From 9f5b0bc293caed0d10c427fb46b966cfbff7af91 Mon Sep 17 00:00:00 2001
From: Nick Lynch <nlynch@amazon.com>
Date: Wed, 29 Jul 2020 14:24:27 +0100
Subject: [PATCH 1/4] chore(cloudfront): Removed duplicate origins in
 aws-cloudfront module (#9326)

Prior to this change, there were both HttpOrigin and S3Origin classes in both
the aws-cloudfront and aws-cloudfront-origins module. The behaviors of the
S3Origin classes were also slightly different.

This change removes the duplication by removing the aws-cloudfront versions of
the origins.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
---
 .../aws-cloudfront-origins/lib/http-origin.ts |  73 +++++++-
 .../lib/load-balancer-origin.ts               |   6 +-
 .../aws-cloudfront-origins/lib/s3-origin.ts   |  42 +++--
 .../test/http-origin.test.ts                  |  48 ++++-
 .../test/s3-origin.test.ts                    | 111 +++++++----
 .../@aws-cdk/aws-cloudfront/lib/origin.ts     | 115 +-----------
 .../aws-cloudfront/test/distribution.test.ts  |  92 +++++-----
 .../aws-cloudfront/test/origin.test.ts        | 173 +++---------------
 8 files changed, 292 insertions(+), 368 deletions(-)

diff --git a/packages/@aws-cdk/aws-cloudfront-origins/lib/http-origin.ts b/packages/@aws-cdk/aws-cloudfront-origins/lib/http-origin.ts
index 19bfb9cb15e95..63181acb169a3 100644
--- a/packages/@aws-cdk/aws-cloudfront-origins/lib/http-origin.ts
+++ b/packages/@aws-cdk/aws-cloudfront-origins/lib/http-origin.ts
@@ -1,21 +1,82 @@
 import * as cloudfront from '@aws-cdk/aws-cloudfront';
+import * as cdk from '@aws-cdk/core';
 
 /**
- * Properties for an Origin backed by any HTTP server.
+ * Properties for an Origin backed by an S3 website-configured bucket, load balancer, or custom HTTP server.
  *
  * @experimental
  */
-export interface HttpOriginProps extends cloudfront.HttpOriginProps { }
+export interface HttpOriginProps extends cloudfront.OriginProps {
+  /**
+   * Specifies the protocol (HTTP or HTTPS) that CloudFront uses to connect to the origin.
+   *
+   * @default OriginProtocolPolicy.HTTPS_ONLY
+   */
+  readonly protocolPolicy?: cloudfront.OriginProtocolPolicy;
+
+  /**
+   * The HTTP port that CloudFront uses to connect to the origin.
+   *
+   * @default 80
+   */
+  readonly httpPort?: number;
+
+  /**
+   * The HTTPS port that CloudFront uses to connect to the origin.
+   *
+   * @default 443
+   */
+  readonly httpsPort?: number;
+
+  /**
+   * Specifies how long, in seconds, CloudFront waits for a response from the origin, also known as the origin response timeout.
+   * The valid range is from 1 to 60 seconds, inclusive.
+   *
+   * @default Duration.seconds(30)
+   */
+  readonly readTimeout?: cdk.Duration;
+
+  /**
+   * Specifies how long, in seconds, CloudFront persists its connection to the origin.
+   * The valid range is from 1 to 60 seconds, inclusive.
+   *
+   * @default Duration.seconds(5)
+   */
+  readonly keepaliveTimeout?: cdk.Duration;
+}
 
 /**
- * An Origin for an HTTP server.
+ * An Origin for an HTTP server or S3 bucket configured for website hosting.
  *
  * @experimental
  */
-export class HttpOrigin extends cloudfront.HttpOrigin {
+export class HttpOrigin extends cloudfront.OriginBase {
+
+  constructor(domainName: string, private readonly props: HttpOriginProps = {}) {
+    super(domainName, props);
 
-  constructor(domainName: string, props: HttpOriginProps = {}) {
-    super(domainName, { ...props });
+    validateSecondsInRangeOrUndefined('readTimeout', 1, 60, props.readTimeout);
+    validateSecondsInRangeOrUndefined('keepaliveTimeout', 1, 60, props.keepaliveTimeout);
   }
 
+  protected renderCustomOriginConfig(): cloudfront.CfnDistribution.CustomOriginConfigProperty | undefined {
+    return {
+      originProtocolPolicy: this.props.protocolPolicy ?? cloudfront.OriginProtocolPolicy.HTTPS_ONLY,
+      httpPort: this.props.httpPort,
+      httpsPort: this.props.httpsPort,
+      originReadTimeout: this.props.readTimeout?.toSeconds(),
+      originKeepaliveTimeout: this.props.keepaliveTimeout?.toSeconds(),
+    };
+  }
+}
+
+/**
+ * Throws an error if a duration is defined and not an integer number of seconds within a range.
+ */
+function validateSecondsInRangeOrUndefined(name: string, min: number, max: number, duration?: cdk.Duration) {
+  if (duration === undefined) { return; }
+  const value = duration.toSeconds();
+  if (!Number.isInteger(value) || value < min || value > max) {
+    throw new Error(`${name}: Must be an int between ${min} and ${max} seconds (inclusive); received ${value}.`);
+  }
 }
diff --git a/packages/@aws-cdk/aws-cloudfront-origins/lib/load-balancer-origin.ts b/packages/@aws-cdk/aws-cloudfront-origins/lib/load-balancer-origin.ts
index de3f2d17593cd..0bedd6ff8948d 100644
--- a/packages/@aws-cdk/aws-cloudfront-origins/lib/load-balancer-origin.ts
+++ b/packages/@aws-cdk/aws-cloudfront-origins/lib/load-balancer-origin.ts
@@ -1,19 +1,19 @@
-import * as cloudfront from '@aws-cdk/aws-cloudfront';
 import * as elbv2 from '@aws-cdk/aws-elasticloadbalancingv2';
+import { HttpOrigin, HttpOriginProps } from './http-origin';
 
 /**
  * Properties for an Origin backed by a v2 load balancer.
  *
  * @experimental
  */
-export interface LoadBalancerV2OriginProps extends cloudfront.HttpOriginProps { }
+export interface LoadBalancerV2OriginProps extends HttpOriginProps { }
 
 /**
  * An Origin for a v2 load balancer.
  *
  * @experimental
  */
-export class LoadBalancerV2Origin extends cloudfront.HttpOrigin {
+export class LoadBalancerV2Origin extends HttpOrigin {
 
   constructor(loadBalancer: elbv2.ILoadBalancerV2, props: LoadBalancerV2OriginProps = {}) {
     super(loadBalancer.loadBalancerDnsName, { ...props });
diff --git a/packages/@aws-cdk/aws-cloudfront-origins/lib/s3-origin.ts b/packages/@aws-cdk/aws-cloudfront-origins/lib/s3-origin.ts
index 2896ba2a5df79..5c74b2b59382e 100644
--- a/packages/@aws-cdk/aws-cloudfront-origins/lib/s3-origin.ts
+++ b/packages/@aws-cdk/aws-cloudfront-origins/lib/s3-origin.ts
@@ -1,6 +1,7 @@
 import * as cloudfront from '@aws-cdk/aws-cloudfront';
 import * as s3 from '@aws-cdk/aws-s3';
 import * as cdk from '@aws-cdk/core';
+import { HttpOrigin } from './http-origin';
 
 /**
  * Properties to use to customize an S3 Origin.
@@ -30,22 +31,41 @@ export class S3Origin implements cloudfront.IOrigin {
   private readonly origin: cloudfront.IOrigin;
 
   constructor(bucket: s3.IBucket, props: S3OriginProps = {}) {
-    let proxyOrigin;
-    if (bucket.isWebsite) {
-      proxyOrigin = new cloudfront.HttpOrigin(bucket.bucketWebsiteDomainName, {
+    this.origin = bucket.isWebsite ?
+      new HttpOrigin(bucket.bucketWebsiteDomainName, {
         protocolPolicy: cloudfront.OriginProtocolPolicy.HTTP_ONLY, // S3 only supports HTTP for website buckets
         ...props,
-      });
-    } else {
-      proxyOrigin = new cloudfront.S3Origin({
-        bucket,
-        ...props,
-      });
-    }
-    this.origin = proxyOrigin;
+      }) :
+      new S3BucketOrigin(bucket, props);
   }
 
   public bind(scope: cdk.Construct, options: cloudfront.OriginBindOptions): cloudfront.OriginBindConfig {
     return this.origin.bind(scope, options);
   }
+
+}
+
+/**
+ * An Origin specific to a S3 bucket (not configured for website hosting).
+ *
+ * Contains additional logic around bucket permissions and origin access identities.
+ */
+class S3BucketOrigin extends cloudfront.OriginBase {
+  private originAccessIdentity!: cloudfront.OriginAccessIdentity;
+
+  constructor(private readonly bucket: s3.IBucket, props: S3OriginProps) {
+    super(bucket.bucketRegionalDomainName, props);
+  }
+
+  public bind(scope: cdk.Construct, options: cloudfront.OriginBindOptions): cloudfront.OriginBindConfig {
+    if (!this.originAccessIdentity) {
+      this.originAccessIdentity = new cloudfront.OriginAccessIdentity(scope, 'S3Origin');
+      this.bucket.grantRead(this.originAccessIdentity);
+    }
+    return super.bind(scope, options);
+  }
+
+  protected renderS3OriginConfig(): cloudfront.CfnDistribution.S3OriginConfigProperty | undefined {
+    return { originAccessIdentity: `origin-access-identity/cloudfront/${this.originAccessIdentity.originAccessIdentityName}` };
+  }
 }
diff --git a/packages/@aws-cdk/aws-cloudfront-origins/test/http-origin.test.ts b/packages/@aws-cdk/aws-cloudfront-origins/test/http-origin.test.ts
index 246392355586e..5da48e33879c5 100644
--- a/packages/@aws-cdk/aws-cloudfront-origins/test/http-origin.test.ts
+++ b/packages/@aws-cdk/aws-cloudfront-origins/test/http-origin.test.ts
@@ -8,7 +8,7 @@ let stack: Stack;
 
 beforeEach(() => {
   app = new App();
-  stack = new Stack(app, 'Stack', {
+  new Stack(app, 'Stack', {
     env: { account: '1234', region: 'testregion' },
   });
 });
@@ -26,24 +26,64 @@ test('Renders minimal example with just a domain name', () => {
   });
 });
 
-test('Can customize properties of the origin', () => {
+test('renders an example with all available props', () => {
   const origin = new HttpOrigin('www.example.com', {
+    originPath: '/app',
+    connectionTimeout: Duration.seconds(5),
+    connectionAttempts: 2,
     customHeaders: { AUTH: 'NONE' },
-    readTimeout: Duration.seconds(10),
     protocolPolicy: cloudfront.OriginProtocolPolicy.MATCH_VIEWER,
+    httpPort: 8080,
+    httpsPort: 8443,
+    readTimeout: Duration.seconds(45),
+    keepaliveTimeout: Duration.seconds(3),
   });
   const originBindConfig = origin.bind(stack, { originId: 'StackOrigin029E19582' });
 
   expect(originBindConfig.originProperty).toEqual({
     id: 'StackOrigin029E19582',
     domainName: 'www.example.com',
+    originPath: '/app',
+    connectionTimeout: 5,
+    connectionAttempts: 2,
     originCustomHeaders: [{
       headerName: 'AUTH',
       headerValue: 'NONE',
     }],
     customOriginConfig: {
       originProtocolPolicy: 'match-viewer',
-      originReadTimeout: 10,
+      httpPort: 8080,
+      httpsPort: 8443,
+      originReadTimeout: 45,
+      originKeepaliveTimeout: 3,
     },
   });
 });
+
+test.each([
+  Duration.seconds(0),
+  Duration.seconds(0.5),
+  Duration.seconds(60.5),
+  Duration.seconds(61),
+  Duration.minutes(5),
+])('validates readTimeout is an integer between 1 and 60 seconds', (readTimeout) => {
+  expect(() => {
+    new HttpOrigin('www.example.com', {
+      readTimeout,
+    });
+  }).toThrow(`readTimeout: Must be an int between 1 and 60 seconds (inclusive); received ${readTimeout.toSeconds()}.`);
+});
+
+test.each([
+  Duration.seconds(0),
+  Duration.seconds(0.5),
+  Duration.seconds(60.5),
+  Duration.seconds(61),
+  Duration.minutes(5),
+])('validates keepaliveTimeout is an integer between 1 and 60 seconds', (keepaliveTimeout) => {
+  expect(() => {
+    new HttpOrigin('www.example.com', {
+      keepaliveTimeout,
+    });
+  }).toThrow(`keepaliveTimeout: Must be an int between 1 and 60 seconds (inclusive); received ${keepaliveTimeout.toSeconds()}.`);
+});
diff --git a/packages/@aws-cdk/aws-cloudfront-origins/test/s3-origin.test.ts b/packages/@aws-cdk/aws-cloudfront-origins/test/s3-origin.test.ts
index 9c0380411e494..9e8efaa990079 100644
--- a/packages/@aws-cdk/aws-cloudfront-origins/test/s3-origin.test.ts
+++ b/packages/@aws-cdk/aws-cloudfront-origins/test/s3-origin.test.ts
@@ -1,4 +1,5 @@
 import '@aws-cdk/assert/jest';
+import * as cloudfront from '@aws-cdk/aws-cloudfront';
 import * as s3 from '@aws-cdk/aws-s3';
 import { App, Stack } from '@aws-cdk/core';
 import { S3Origin } from '../lib';
@@ -13,52 +14,94 @@ beforeEach(() => {
   });
 });
 
-test('With non-website bucket, renders all required properties, including S3Origin config', () => {
-  const bucket = new s3.Bucket(stack, 'Bucket');
+describe('With bucket', () => {
+  test('renders minimal example', () => {
+    const bucket = new s3.Bucket(stack, 'Bucket');
 
-  const origin = new S3Origin(bucket);
-  const originBindConfig = origin.bind(stack, { originId: 'StackOrigin029E19582' });
+    const origin = new S3Origin(bucket);
+    const originBindConfig = origin.bind(stack, { originId: 'StackOrigin029E19582' });
 
-  expect(originBindConfig.originProperty).toEqual({
-    id: 'StackOrigin029E19582',
-    domainName: bucket.bucketRegionalDomainName,
-    s3OriginConfig: {
-      originAccessIdentity: 'origin-access-identity/cloudfront/${Token[TOKEN.69]}',
-    },
+    expect(originBindConfig.originProperty).toEqual({
+      id: 'StackOrigin029E19582',
+      domainName: bucket.bucketRegionalDomainName,
+      s3OriginConfig: {
+        originAccessIdentity: 'origin-access-identity/cloudfront/${Token[TOKEN.69]}',
+      },
+    });
   });
-});
 
-test('With website bucket, renders all required properties, including custom origin config', () => {
-  const bucket = new s3.Bucket(stack, 'Bucket', {
-    websiteIndexDocument: 'index.html',
+  test('can customize properties', () => {
+    const bucket = new s3.Bucket(stack, 'Bucket');
+
+    const origin = new S3Origin(bucket, { originPath: '/assets' });
+    const originBindConfig = origin.bind(stack, { originId: 'StackOrigin029E19582' });
+
+    expect(originBindConfig.originProperty).toEqual({
+      id: 'StackOrigin029E19582',
+      domainName: bucket.bucketRegionalDomainName,
+      originPath: '/assets',
+      s3OriginConfig: {
+        originAccessIdentity: 'origin-access-identity/cloudfront/${Token[TOKEN.89]}',
+      },
+    });
   });
 
-  const origin = new S3Origin(bucket);
-  const originBindConfig = origin.bind(stack, { originId: 'StackOrigin029E19582' });
+  test('creates an OriginAccessIdentity and grants read permissions on the bucket', () => {
+    const bucket = new s3.Bucket(stack, 'Bucket');
+
+    const origin = new S3Origin(bucket);
+    new cloudfront.Distribution(stack, 'Dist', { defaultBehavior: { origin } });
 
-  expect(originBindConfig.originProperty).toEqual({
-    id: 'StackOrigin029E19582',
-    domainName: bucket.bucketWebsiteDomainName,
-    customOriginConfig: {
-      originProtocolPolicy: 'http-only',
-    },
+    expect(stack).toHaveResourceLike('AWS::CloudFront::CloudFrontOriginAccessIdentity', {
+      CloudFrontOriginAccessIdentityConfig: {
+        Comment: 'Allows CloudFront to reach the bucket',
+      },
+    });
+    expect(stack).toHaveResourceLike('AWS::S3::BucketPolicy', {
+      PolicyDocument: {
+        Statement: [{
+          Principal: {
+            CanonicalUser: { 'Fn::GetAtt': [ 'DistOrigin1S3Origin87D64058', 'S3CanonicalUserId' ] },
+          },
+        }],
+      },
+    });
   });
 });
 
-test('Respects props passed down to underlying origin', () => {
-  const bucket = new s3.Bucket(stack, 'Bucket', {
-    websiteIndexDocument: 'index.html',
+describe('With website-configured bucket', () => {
+  test('renders all required properties, including custom origin config', () => {
+    const bucket = new s3.Bucket(stack, 'Bucket', {
+      websiteIndexDocument: 'index.html',
+    });
+
+    const origin = new S3Origin(bucket);
+    const originBindConfig = origin.bind(stack, { originId: 'StackOrigin029E19582' });
+
+    expect(originBindConfig.originProperty).toEqual({
+      id: 'StackOrigin029E19582',
+      domainName: bucket.bucketWebsiteDomainName,
+      customOriginConfig: {
+        originProtocolPolicy: 'http-only',
+      },
+    });
   });
 
-  const origin = new S3Origin(bucket, { originPath: '/website' });
-  const originBindConfig = origin.bind(stack, { originId: 'StackOrigin029E19582' });
+  test('can customize properties', () => {
+    const bucket = new s3.Bucket(stack, 'Bucket', {
+      websiteIndexDocument: 'index.html',
+    });
+
+    const origin = new S3Origin(bucket, { originPath: '/assets' });
+    const originBindConfig = origin.bind(stack, { originId: 'StackOrigin029E19582' });
 
-  expect(originBindConfig.originProperty).toEqual({
-    id: 'StackOrigin029E19582',
-    domainName: bucket.bucketWebsiteDomainName,
-    originPath: '/website',
-    customOriginConfig: {
-      originProtocolPolicy: 'http-only',
-    },
+    expect(originBindConfig.originProperty).toEqual({
+      id: 'StackOrigin029E19582',
+      domainName: bucket.bucketWebsiteDomainName,
+      originPath: '/assets',
+      customOriginConfig: {
+        originProtocolPolicy: 'http-only',
+      },
+    });
   });
 });
diff --git a/packages/@aws-cdk/aws-cloudfront/lib/origin.ts b/packages/@aws-cdk/aws-cloudfront/lib/origin.ts
index 5e680f5e29f48..416b9f5bcc179 100644
--- a/packages/@aws-cdk/aws-cloudfront/lib/origin.ts
+++ b/packages/@aws-cdk/aws-cloudfront/lib/origin.ts
@@ -1,8 +1,5 @@
-import { IBucket } from '@aws-cdk/aws-s3';
 import { Construct, Duration, Token } from '@aws-cdk/core';
 import { CfnDistribution } from './cloudfront.generated';
-import { OriginProtocolPolicy } from './distribution';
-import { OriginAccessIdentity } from './origin_access_identity';
 
 /** The struct returned from {@link IOrigin.bind}. */
 export interface OriginBindConfig {
@@ -82,7 +79,7 @@ export interface OriginBindOptions {
  *
  * @experimental
  */
-export abstract class Origin implements IOrigin {
+export abstract class OriginBase implements IOrigin {
   private readonly domainName: string;
   private readonly originPath?: string;
   private readonly connectionTimeout?: Duration;
@@ -168,116 +165,6 @@ export abstract class Origin implements IOrigin {
   }
 }
 
-/**
- * Properties for an Origin backed by an S3 bucket
- *
- * @experimental
- */
-export interface S3OriginProps extends OriginProps {
-  /**
-   * The bucket to use as an origin.
-   */
-  readonly bucket: IBucket;
-}
-
-/**
- * An Origin specific to a S3 bucket (not configured for website hosting).
- *
- * Contains additional logic around bucket permissions and origin access identities.
- *
- * @experimental
- */
-export class S3Origin extends Origin {
-  private readonly bucket: IBucket;
-  private originAccessIdentity!: OriginAccessIdentity;
-
-  constructor(props: S3OriginProps) {
-    super(props.bucket.bucketRegionalDomainName, props);
-    this.bucket = props.bucket;
-  }
-
-  public bind(scope: Construct, options: OriginBindOptions): OriginBindConfig {
-    if (!this.originAccessIdentity) {
-      this.originAccessIdentity = new OriginAccessIdentity(scope, 'S3Origin');
-      this.bucket.grantRead(this.originAccessIdentity);
-    }
-    return super.bind(scope, options);
-  }
-
-  protected renderS3OriginConfig(): CfnDistribution.S3OriginConfigProperty | undefined {
-    return { originAccessIdentity: `origin-access-identity/cloudfront/${this.originAccessIdentity.originAccessIdentityName}` };
-  }
-}
-
-/**
- * Properties for an Origin backed by an S3 website-configured bucket, load balancer, or custom HTTP server.
- *
- * @experimental
- */
-export interface HttpOriginProps extends OriginProps {
-  /**
-   * Specifies the protocol (HTTP or HTTPS) that CloudFront uses to connect to the origin.
-   *
-   * @default OriginProtocolPolicy.HTTPS_ONLY
-   */
-  readonly protocolPolicy?: OriginProtocolPolicy;
-
-  /**
-   * The HTTP port that CloudFront uses to connect to the origin.
-   *
-   * @default 80
-   */
-  readonly httpPort?: number;
-
-  /**
-   * The HTTPS port that CloudFront uses to connect to the origin.
-   *
-   * @default 443
-   */
-  readonly httpsPort?: number;
-
-  /**
-   * Specifies how long, in seconds, CloudFront waits for a response from the origin, also known as the origin response timeout.
-   * The valid range is from 1 to 60 seconds, inclusive.
-   *
-   * @default Duration.seconds(30)
-   */
-  readonly readTimeout?: Duration;
-
-  /**
-   * Specifies how long, in seconds, CloudFront persists its connection to the origin.
-   * The valid range is from 1 to 60 seconds, inclusive.
-   *
-   * @default Duration.seconds(5)
-   */
-  readonly keepaliveTimeout?: Duration;
-}
-
-/**
- * An Origin for an HTTP server or S3 bucket configured for website hosting.
- *
- * @experimental
- */
-export class HttpOrigin extends Origin {
-
-  constructor(domainName: string, private readonly props: HttpOriginProps = {}) {
-    super(domainName, props);
-
-    validateIntInRangeOrUndefined('readTimeout', 1, 60, props.readTimeout?.toSeconds());
-    validateIntInRangeOrUndefined('keepaliveTimeout', 1, 60, props.keepaliveTimeout?.toSeconds());
-  }
-
-  protected renderCustomOriginConfig(): CfnDistribution.CustomOriginConfigProperty | undefined {
-    return {
-      originProtocolPolicy: this.props.protocolPolicy ?? OriginProtocolPolicy.HTTPS_ONLY,
-      httpPort: this.props.httpPort,
-      httpsPort: this.props.httpsPort,
-      originReadTimeout: this.props.readTimeout?.toSeconds(),
-      originKeepaliveTimeout: this.props.keepaliveTimeout?.toSeconds(),
-    };
-  }
-}
-
 /**
  * Throws an error if a value is defined and not an integer or not in a range.
  */
diff --git a/packages/@aws-cdk/aws-cloudfront/test/distribution.test.ts b/packages/@aws-cdk/aws-cloudfront/test/distribution.test.ts
index aa29d8da5e8ce..e4f91e7bcd58d 100644
--- a/packages/@aws-cdk/aws-cloudfront/test/distribution.test.ts
+++ b/packages/@aws-cdk/aws-cloudfront/test/distribution.test.ts
@@ -1,9 +1,8 @@
 import '@aws-cdk/assert/jest';
 import * as acm from '@aws-cdk/aws-certificatemanager';
 import * as lambda from '@aws-cdk/aws-lambda';
-import * as s3 from '@aws-cdk/aws-s3';
 import { App, Duration, Stack } from '@aws-cdk/core';
-import { Distribution, IOrigin, LambdaEdgeEventType, S3Origin, PriceClass } from '../lib';
+import { CfnDistribution, Distribution, IOrigin, LambdaEdgeEventType, OriginBase, OriginProps, OriginProtocolPolicy, PriceClass } from '../lib';
 
 let app: App;
 let stack: Stack;
@@ -16,7 +15,7 @@ beforeEach(() => {
 });
 
 test('minimal example renders correctly', () => {
-  const origin = defaultS3Origin();
+  const origin = defaultOrigin();
   new Distribution(stack, 'MyDist', { defaultBehavior: { origin } });
 
   expect(stack).toHaveResource('AWS::CloudFront::Distribution', {
@@ -28,12 +27,10 @@ test('minimal example renders correctly', () => {
       },
       Enabled: true,
       Origins: [{
-        DomainName: { 'Fn::GetAtt': [ 'Bucket83908E77', 'RegionalDomainName' ] },
+        DomainName: 'www.example.com',
         Id: 'StackMyDistOrigin1D6D5E535',
-        S3OriginConfig: {
-          OriginAccessIdentity: { 'Fn::Join': [ '',
-            [ 'origin-access-identity/cloudfront/', { Ref: 'MyDistOrigin1S3Origin071F2793' } ],
-          ]},
+        CustomOriginConfig: {
+          OriginProtocolPolicy: 'https-only',
         },
       }],
     },
@@ -43,7 +40,7 @@ test('minimal example renders correctly', () => {
 describe('multiple behaviors', () => {
 
   test('a second behavior can\'t be specified with the catch-all path pattern', () => {
-    const origin = defaultS3Origin();
+    const origin = defaultOrigin();
 
     expect(() => {
       new Distribution(stack, 'MyDist', {
@@ -56,7 +53,7 @@ describe('multiple behaviors', () => {
   });
 
   test('a second behavior can be added to the original origin', () => {
-    const origin = defaultS3Origin();
+    const origin = defaultOrigin();
     new Distribution(stack, 'MyDist', {
       defaultBehavior: { origin },
       additionalBehaviors: {
@@ -79,12 +76,10 @@ describe('multiple behaviors', () => {
         }],
         Enabled: true,
         Origins: [{
-          DomainName: { 'Fn::GetAtt': [ 'Bucket83908E77', 'RegionalDomainName' ] },
+          DomainName: 'www.example.com',
           Id: 'StackMyDistOrigin1D6D5E535',
-          S3OriginConfig: {
-            OriginAccessIdentity: { 'Fn::Join': [ '',
-              [ 'origin-access-identity/cloudfront/', { Ref: 'MyDistOrigin1S3Origin071F2793' } ],
-            ]},
+          CustomOriginConfig: {
+            OriginProtocolPolicy: 'https-only',
           },
         }],
       },
@@ -92,9 +87,8 @@ describe('multiple behaviors', () => {
   });
 
   test('a second behavior can be added to a secondary origin', () => {
-    const origin = defaultS3Origin();
-    const bucket2 = new s3.Bucket(stack, 'Bucket2');
-    const origin2 = new S3Origin({ bucket: bucket2 });
+    const origin = defaultOrigin();
+    const origin2 = defaultOrigin('origin2.example.com');
     new Distribution(stack, 'MyDist', {
       defaultBehavior: { origin },
       additionalBehaviors: {
@@ -117,21 +111,17 @@ describe('multiple behaviors', () => {
         }],
         Enabled: true,
         Origins: [{
-          DomainName: { 'Fn::GetAtt': [ 'Bucket83908E77', 'RegionalDomainName' ] },
+          DomainName: 'www.example.com',
           Id: 'StackMyDistOrigin1D6D5E535',
-          S3OriginConfig: {
-            OriginAccessIdentity: { 'Fn::Join': [ '',
-              [ 'origin-access-identity/cloudfront/', { Ref: 'MyDistOrigin1S3Origin071F2793' } ],
-            ]},
+          CustomOriginConfig: {
+            OriginProtocolPolicy: 'https-only',
           },
         },
         {
-          DomainName: { 'Fn::GetAtt': [ 'Bucket25524B414', 'RegionalDomainName' ] },
+          DomainName: 'origin2.example.com',
           Id: 'StackMyDistOrigin20B96F3AD',
-          S3OriginConfig: {
-            OriginAccessIdentity: { 'Fn::Join': [ '',
-              [ 'origin-access-identity/cloudfront/', { Ref: 'MyDistOrigin2S3Origin26A1EB27' } ],
-            ]},
+          CustomOriginConfig: {
+            OriginProtocolPolicy: 'https-only',
           },
         }],
       },
@@ -139,9 +129,8 @@ describe('multiple behaviors', () => {
   });
 
   test('behavior creation order is preserved', () => {
-    const origin = defaultS3Origin();
-    const bucket2 = new s3.Bucket(stack, 'Bucket2');
-    const origin2 = new S3Origin({ bucket: bucket2 });
+    const origin = defaultOrigin();
+    const origin2 = defaultOrigin('origin2.example.com');
     const dist = new Distribution(stack, 'MyDist', {
       defaultBehavior: { origin },
       additionalBehaviors: {
@@ -171,21 +160,17 @@ describe('multiple behaviors', () => {
         }],
         Enabled: true,
         Origins: [{
-          DomainName: { 'Fn::GetAtt': [ 'Bucket83908E77', 'RegionalDomainName' ] },
+          DomainName: 'www.example.com',
           Id: 'StackMyDistOrigin1D6D5E535',
-          S3OriginConfig: {
-            OriginAccessIdentity: { 'Fn::Join': [ '',
-              [ 'origin-access-identity/cloudfront/', { Ref: 'MyDistOrigin1S3Origin071F2793' } ],
-            ]},
+          CustomOriginConfig: {
+            OriginProtocolPolicy: 'https-only',
           },
         },
         {
-          DomainName: { 'Fn::GetAtt': [ 'Bucket25524B414', 'RegionalDomainName' ] },
+          DomainName: 'origin2.example.com',
           Id: 'StackMyDistOrigin20B96F3AD',
-          S3OriginConfig: {
-            OriginAccessIdentity: { 'Fn::Join': [ '',
-              [ 'origin-access-identity/cloudfront/', { Ref: 'MyDistOrigin2S3Origin26A1EB27' } ],
-            ]},
+          CustomOriginConfig: {
+            OriginProtocolPolicy: 'https-only',
           },
         }],
       },
@@ -196,7 +181,7 @@ describe('multiple behaviors', () => {
 describe('certificates', () => {
 
   test('should fail if using an imported certificate from outside of us-east-1', () => {
-    const origin = defaultS3Origin();
+    const origin = defaultOrigin();
     const certificate = acm.Certificate.fromCertificateArn(stack, 'Cert', 'arn:aws:acm:eu-west-1:123456789012:certificate/12345678-1234-1234-1234-123456789012');
 
     expect(() => {
@@ -211,7 +196,7 @@ describe('certificates', () => {
     const certificate = acm.Certificate.fromCertificateArn(stack, 'Cert', 'arn:aws:acm:us-east-1:123456789012:certificate/12345678-1234-1234-1234-123456789012');
 
     new Distribution(stack, 'Dist', {
-      defaultBehavior: { origin: defaultS3Origin() },
+      defaultBehavior: { origin: defaultOrigin() },
       certificate,
     });
 
@@ -230,7 +215,7 @@ describe('certificates', () => {
 describe('custom error responses', () => {
 
   test('should fail if responsePagePath is defined but responseCode is not', () => {
-    const origin = defaultS3Origin();
+    const origin = defaultOrigin();
 
     expect(() => {
       new Distribution(stack, 'Dist', {
@@ -244,7 +229,7 @@ describe('custom error responses', () => {
   });
 
   test('should fail if only the error code is provided', () => {
-    const origin = defaultS3Origin();
+    const origin = defaultOrigin();
 
     expect(() => {
       new Distribution(stack, 'Dist', {
@@ -255,7 +240,7 @@ describe('custom error responses', () => {
   });
 
   test('should render the array of error configs if provided', () => {
-    const origin = defaultS3Origin();
+    const origin = defaultOrigin();
     new Distribution(stack, 'Dist', {
       defaultBehavior: { origin },
       errorResponses: [{
@@ -299,7 +284,7 @@ describe('with Lambda@Edge functions', () => {
       handler: 'index.handler',
     });
 
-    origin = defaultS3Origin();
+    origin = defaultOrigin();
   });
 
   test('can add an edge lambdas to the default behavior', () => {
@@ -384,7 +369,7 @@ describe('with Lambda@Edge functions', () => {
 });
 
 test('price class is included if provided', () => {
-  const origin = defaultS3Origin();
+  const origin = defaultOrigin();
   new Distribution(stack, 'Dist', {
     defaultBehavior: { origin },
     priceClass: PriceClass.PRICE_CLASS_200,
@@ -397,6 +382,13 @@ test('price class is included if provided', () => {
   });
 });
 
-function defaultS3Origin(): IOrigin {
-  return new S3Origin({ bucket: new s3.Bucket(stack, 'Bucket') });
+function defaultOrigin(domainName?: string): IOrigin {
+  return new TestOrigin(domainName ?? 'www.example.com');
+}
+
+class TestOrigin extends OriginBase {
+  constructor(domainName: string, props: OriginProps = {}) { super(domainName, props); }
+  protected renderCustomOriginConfig(): CfnDistribution.CustomOriginConfigProperty | undefined {
+    return { originProtocolPolicy: OriginProtocolPolicy.HTTPS_ONLY };
+  }
 }
diff --git a/packages/@aws-cdk/aws-cloudfront/test/origin.test.ts b/packages/@aws-cdk/aws-cloudfront/test/origin.test.ts
index fb260dc221c9b..a9d553dbe01c0 100644
--- a/packages/@aws-cdk/aws-cloudfront/test/origin.test.ts
+++ b/packages/@aws-cdk/aws-cloudfront/test/origin.test.ts
@@ -1,7 +1,6 @@
 import '@aws-cdk/assert/jest';
-import * as s3 from '@aws-cdk/aws-s3';
 import { App, Stack, Duration } from '@aws-cdk/core';
-import { CfnDistribution, Distribution, HttpOrigin, OriginProtocolPolicy, S3Origin, Origin, OriginProps } from '../lib';
+import { CfnDistribution, OriginProtocolPolicy, OriginBase, OriginProps } from '../lib';
 
 let app: App;
 let stack: Stack;
@@ -13,161 +12,43 @@ beforeEach(() => {
   });
 });
 
-describe('S3Origin', () => {
-  test('as bucket, renders all required properties, including S3Origin config', () => {
-    const bucket = new s3.Bucket(stack, 'Bucket');
-
-    const origin = new S3Origin({ bucket });
-    const originBindConfig = origin.bind(stack, { originId: 'StackOrigin029E19582' });
-
-    expect(originBindConfig.originProperty).toEqual({
-      id: 'StackOrigin029E19582',
-      domainName: bucket.bucketRegionalDomainName,
-      s3OriginConfig: {
-        originAccessIdentity: 'origin-access-identity/cloudfront/${Token[TOKEN.69]}',
-      },
-    });
-  });
-
-  test('as bucket, creates an OriginAccessIdentity and grants read permissions on the bucket', () => {
-    const bucket = new s3.Bucket(stack, 'Bucket');
-
-    const origin = new S3Origin({ bucket });
-    new Distribution(stack, 'Dist', { defaultBehavior: { origin } });
-
-    expect(stack).toHaveResourceLike('AWS::CloudFront::CloudFrontOriginAccessIdentity', {
-      CloudFrontOriginAccessIdentityConfig: {
-        Comment: 'Allows CloudFront to reach the bucket',
-      },
+test.each([
+  Duration.seconds(0),
+  Duration.seconds(0.5),
+  Duration.seconds(10.5),
+  Duration.seconds(11),
+  Duration.minutes(5),
+])('validates connectionTimeout is an int between 1 and 10 seconds', (connectionTimeout) => {
+  expect(() => {
+    new TestOrigin('www.example.com', {
+      connectionTimeout,
     });
-    expect(stack).toHaveResourceLike('AWS::S3::BucketPolicy', {
-      PolicyDocument: {
-        Statement: [{
-          Principal: {
-            CanonicalUser: { 'Fn::GetAtt': [ 'DistOrigin1S3Origin87D64058', 'S3CanonicalUserId' ] },
-          },
-        }],
-      },
-    });
-  });
+  }).toThrow(`connectionTimeout: Must be an int between 1 and 10 seconds (inclusive); received ${connectionTimeout.toSeconds()}.`);
 });
 
-describe('HttpOrigin', () => {
-  test('renders a minimal example with required props', () => {
-    const origin = new HttpOrigin('www.example.com');
-    const originBindConfig = origin.bind(stack, { originId: 'StackOrigin029E19582' });
-
-    expect(originBindConfig.originProperty).toEqual({
-      id: 'StackOrigin029E19582',
-      domainName: 'www.example.com',
-      customOriginConfig: {
-        originProtocolPolicy: 'https-only',
-      },
-    });
-  });
-
-  test('renders an example with all available props', () => {
-    const origin = new HttpOrigin('www.example.com', {
-      originPath: '/app',
-      connectionTimeout: Duration.seconds(5),
-      connectionAttempts: 2,
-      customHeaders: { AUTH: 'NONE' },
-      protocolPolicy: OriginProtocolPolicy.MATCH_VIEWER,
-      httpPort: 8080,
-      httpsPort: 8443,
-      readTimeout: Duration.seconds(45),
-      keepaliveTimeout: Duration.seconds(3),
+test.each([-0.5, 0.5, 1.5, 4])
+('validates connectionAttempts is an int between 1 and 3', (connectionAttempts) => {
+  expect(() => {
+    new TestOrigin('www.example.com', {
+      connectionAttempts,
     });
-    const originBindConfig = origin.bind(stack, { originId: 'StackOrigin029E19582' });
-
-    expect(originBindConfig.originProperty).toEqual({
-      id: 'StackOrigin029E19582',
-      domainName: 'www.example.com',
-      originPath: '/app',
-      connectionTimeout: 5,
-      connectionAttempts: 2,
-      originCustomHeaders: [{
-        headerName: 'AUTH',
-        headerValue: 'NONE',
-      }],
-      customOriginConfig: {
-        originProtocolPolicy: 'match-viewer',
-        httpPort: 8080,
-        httpsPort: 8443,
-        originReadTimeout: 45,
-        originKeepaliveTimeout: 3,
-      },
-    });
-  });
-
-  test.each([
-    Duration.seconds(0),
-    Duration.seconds(0.5),
-    Duration.seconds(60.5),
-    Duration.seconds(61),
-    Duration.minutes(5),
-  ])('validates readTimeout is an integer between 1 and 60 seconds', (readTimeout) => {
-    expect(() => {
-      new HttpOrigin('www.example.com', {
-        readTimeout,
-      });
-    }).toThrow(`readTimeout: Must be an int between 1 and 60 seconds (inclusive); received ${readTimeout.toSeconds()}.`);
-  });
-
-  test.each([
-    Duration.seconds(0),
-    Duration.seconds(0.5),
-    Duration.seconds(60.5),
-    Duration.seconds(61),
-    Duration.minutes(5),
-  ])('validates keepaliveTimeout is an integer between 1 and 60 seconds', (keepaliveTimeout) => {
-    expect(() => {
-      new HttpOrigin('www.example.com', {
-        keepaliveTimeout,
-      });
-    }).toThrow(`keepaliveTimeout: Must be an int between 1 and 60 seconds (inclusive); received ${keepaliveTimeout.toSeconds()}.`);
-  });
+  }).toThrow(`connectionAttempts: Must be an int between 1 and 3 (inclusive); received ${connectionAttempts}.`);
 });
 
-describe('Origin', () => {
-  test.each([
-    Duration.seconds(0),
-    Duration.seconds(0.5),
-    Duration.seconds(10.5),
-    Duration.seconds(11),
-    Duration.minutes(5),
-  ])('validates connectionTimeout is an int between 1 and 10 seconds', (connectionTimeout) => {
-    expect(() => {
-      new TestOrigin('www.example.com', {
-        connectionTimeout,
-      });
-    }).toThrow(`connectionTimeout: Must be an int between 1 and 10 seconds (inclusive); received ${connectionTimeout.toSeconds()}.`);
-  });
-
-  test.each([-0.5, 0.5, 1.5, 4])
-  ('validates connectionAttempts is an int between 1 and 3', (connectionAttempts) => {
-    expect(() => {
-      new TestOrigin('www.example.com', {
-        connectionAttempts,
-      });
-    }).toThrow(`connectionAttempts: Must be an int between 1 and 3 (inclusive); received ${connectionAttempts}.`);
+test.each(['api', '/api', '/api/', 'api/'])
+('enforces that originPath starts but does not end, with a /', (originPath) => {
+  const origin = new TestOrigin('www.example.com', {
+    originPath,
   });
+  const originBindConfig = origin.bind(stack, { originId: '0' });
 
-  test.each(['api', '/api', '/api/', 'api/'])
-  ('enforces that originPath starts but does not end, with a /', (originPath) => {
-    const origin = new TestOrigin('www.example.com', {
-      originPath,
-    });
-    const originBindConfig = origin.bind(stack, { originId: '0' });
-
-    expect(originBindConfig.originProperty?.originPath).toEqual('/api');
-  });
+  expect(originBindConfig.originProperty?.originPath).toEqual('/api');
 });
 
 /** Used for testing common Origin functionality */
-class TestOrigin extends Origin {
+class TestOrigin extends OriginBase {
   constructor(domainName: string, props: OriginProps = {}) { super(domainName, props); }
-  protected renderS3OriginConfig(): CfnDistribution.S3OriginConfigProperty | undefined {
-    return { originAccessIdentity: 'origin-access-identity/cloudfront/MyOAIName' };
+  protected renderCustomOriginConfig(): CfnDistribution.CustomOriginConfigProperty | undefined {
+    return { originProtocolPolicy: OriginProtocolPolicy.HTTPS_ONLY };
   }
 }

From 3cad6a367bd954bdfe5115bce6c0e70b9bc39ad4 Mon Sep 17 00:00:00 2001
From: Adam Ruka <adamruka@amazon.com>
Date: Wed, 29 Jul 2020 07:38:19 -0700
Subject: [PATCH 2/4] chore(cfn-include): update ReadMe for Nested Stacks
 (#9314)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
---
 .../@aws-cdk/cloudformation-include/README.md | 19 +++----------------
 1 file changed, 3 insertions(+), 16 deletions(-)

diff --git a/packages/@aws-cdk/cloudformation-include/README.md b/packages/@aws-cdk/cloudformation-include/README.md
index 4a591bc74f150..160bddd94cf6c 100644
--- a/packages/@aws-cdk/cloudformation-include/README.md
+++ b/packages/@aws-cdk/cloudformation-include/README.md
@@ -158,10 +158,7 @@ For example, if you have the following parent template:
     "ChildStack": {
       "Type": "AWS::CloudFormation::Stack",
       "Properties": {
-        "TemplateURL": "https://my-s3-template-source.s3.amazonaws.com/child-import-stack.json",
-        "Parameters": {
-          "MyBucketParameter": "my-bucket-name"
-        }
+        "TemplateURL": "https://my-s3-template-source.s3.amazonaws.com/child-import-stack.json"
       }
     }
   }
@@ -172,19 +169,9 @@ where the child template pointed to by `https://my-s3-template-source.s3.amazona
 
 ```json
 {
-  "Parameters": {
-    "MyBucketParameter": {
-      "Type": "String",
-      "Default": "default-bucket-param-name"
-    }
-  },
   "Resources": {
-    "BucketImport": {
-      "Type": "AWS::S3::Bucket",
-      "Properties": {
-        "BucketName": {
-          "Ref": "MyBucketParameter"
-        }
+    "MyBucket": {
+      "Type": "AWS::S3::Bucket"
       }
     }
   }

From 239e686d20ee95758e5952fbde5073d53c25b535 Mon Sep 17 00:00:00 2001
From: Daniel Neilson <53624638+ddneilson@users.noreply.github.com>
Date: Wed, 29 Jul 2020 16:19:37 -0500
Subject: [PATCH 3/4] chore(core): include aws-rfdk in version reporting
 (#9337)

Implements https://github.com/aws/aws-cdk/issues/9336
----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
---
 .../@aws-cdk/core/lib/private/runtime-info.ts |  3 ++
 .../@aws-cdk/core/test/test.runtime-info.ts   | 28 +++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/packages/@aws-cdk/core/lib/private/runtime-info.ts b/packages/@aws-cdk/core/lib/private/runtime-info.ts
index 7edf871a5f183..25fc3ccaf1818 100644
--- a/packages/@aws-cdk/core/lib/private/runtime-info.ts
+++ b/packages/@aws-cdk/core/lib/private/runtime-info.ts
@@ -4,6 +4,8 @@ import { major as nodeMajorVersion } from './node-version';
 
 // list of NPM scopes included in version reporting e.g. @aws-cdk and @aws-solutions-konstruk
 const WHITELIST_SCOPES = ['@aws-cdk', '@aws-solutions-konstruk', '@aws-solutions-constructs'];
+// list of NPM packages included in version reporting
+const WHITELIST_PACKAGES = ['aws-rfdk'];
 
 /**
  * Returns a list of loaded modules and their versions.
@@ -26,6 +28,7 @@ export function collectRuntimeInformation(): cxschema.RuntimeInfo {
         foundMatch = true;
       }
     }
+    foundMatch = foundMatch || WHITELIST_PACKAGES.includes(name);
 
     if (!foundMatch) {
       delete libraries[name];
diff --git a/packages/@aws-cdk/core/test/test.runtime-info.ts b/packages/@aws-cdk/core/test/test.runtime-info.ts
index 8f619ce9f991d..45c60d0162f40 100644
--- a/packages/@aws-cdk/core/test/test.runtime-info.ts
+++ b/packages/@aws-cdk/core/test/test.runtime-info.ts
@@ -32,6 +32,34 @@ export = {
     test.done();
   },
 
+  'version reporting finds aws-rfdk package'(test: Test) {
+    const pkgdir = fs.mkdtempSync(path.join(os.tmpdir(), 'runtime-info-rfdk'));
+    const mockVersion = '1.2.3';
+
+    fs.writeFileSync(path.join(pkgdir, 'index.js'), 'module.exports = \'this is foo\';');
+    fs.writeFileSync(path.join(pkgdir, 'package.json'), JSON.stringify({
+      name: 'aws-rfdk',
+      version: mockVersion,
+    }));
+
+    // eslint-disable-next-line @typescript-eslint/no-require-imports, import/no-extraneous-dependencies
+    require(pkgdir);
+
+    const runtimeInfo = collectRuntimeInformation();
+
+    // eslint-disable-next-line @typescript-eslint/no-require-imports
+    const version = require('../package.json').version;
+    test.deepEqual(runtimeInfo.libraries , {
+      '@aws-cdk/core': version,
+      '@aws-cdk/cx-api': version,
+      '@aws-cdk/cloud-assembly-schema': version,
+      '@aws-solutions-konstruk/foo': mockVersion, // picks up the module from the other test.
+      'aws-rfdk': mockVersion,
+      'jsii-runtime': `node.js/${process.version}`,
+    });
+    test.done();
+  },
+
   'version reporting finds no version with no associated package.json'(test: Test) {
     const pkgdir = fs.mkdtempSync(path.join(os.tmpdir(), 'runtime-info-find-npm-package-fixture'));
     const mockVersion = '1.2.3';

From 2a48495093dc33d88554aaa0a033338e798f9d5f Mon Sep 17 00:00:00 2001
From: comcalvi <66279577+comcalvi@users.noreply.github.com>
Date: Wed, 29 Jul 2020 17:54:21 -0400
Subject: [PATCH 4/4] feat(cfn-include): add support for the Fn::Sub function
 (#9275)

----

Closes  #9228

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
---
 .../@aws-cdk/cloudformation-include/README.md | 44 -----------
 .../cloudformation-include/lib/file-utils.ts  |  4 +-
 .../test/invalid-templates.test.ts            | 12 +++
 .../test-templates/fn-sub-brace-edges.json    | 55 +++++++++++++
 .../test/test-templates/fn-sub-escaping.json  | 10 +++
 .../fn-sub-map-dotted-attributes.json         | 25 ++++++
 .../test/test-templates/fn-sub-override.json  | 16 ++++
 .../test-templates/fn-sub-parsing-edges.json  |  0
 .../fn-sub-shadow-attribute.json              | 20 +++++
 .../test/test-templates/fn-sub-shadow.json    | 20 +++++
 .../test/test-templates/fn-sub-string.json    | 16 ++++
 .../invalid/fn-sub-${}-only.json              | 12 +++
 .../fn-sub-key-not-in-template-string.json    | 10 +++
 .../yaml/invalid/short-form-import-sub.yaml   |  7 ++
 .../test-templates/yaml/long-form-vpc.yaml    |  8 ++
 .../yaml/short-form-fnsub-string.yaml         | 11 +++
 .../yaml/short-form-sub-map.yaml              | 15 ++++
 .../test/valid-templates.test.ts              | 77 +++++++++++++++++++
 .../test/yaml-templates.test.ts               | 22 ++++++
 packages/@aws-cdk/core/lib/cfn-parse.ts       | 63 +++++++++++++++
 .../core/lib/private/cfn-reference.ts         | 23 ++++--
 21 files changed, 415 insertions(+), 55 deletions(-)
 create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-brace-edges.json
 create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-escaping.json
 create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-map-dotted-attributes.json
 create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-override.json
 create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-parsing-edges.json
 create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-shadow-attribute.json
 create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-shadow.json
 create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-string.json
 create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-${}-only.json
 create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-key-not-in-template-string.json
 create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/invalid/short-form-import-sub.yaml
 create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/short-form-fnsub-string.yaml
 create mode 100644 packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/short-form-sub-map.yaml

diff --git a/packages/@aws-cdk/cloudformation-include/README.md b/packages/@aws-cdk/cloudformation-include/README.md
index 160bddd94cf6c..0e08b7f5fb745 100644
--- a/packages/@aws-cdk/cloudformation-include/README.md
+++ b/packages/@aws-cdk/cloudformation-include/README.md
@@ -218,47 +218,3 @@ bucketReadRole.addToPolicy(new iam.PolicyStatement({
   resources: [bucket.attrArn],
 }));
 ```
-
-## Known limitations
-
-This module is still in its early, experimental stage,
-and so does not implement all features of CloudFormation templates.
-All items unchecked below are currently not supported.
-
-### Ability to retrieve CloudFormation objects from the template:
-
-- [x] Resources
-- [x] Parameters
-- [x] Conditions
-- [x] Outputs
-
-### [Resource attributes](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-product-attribute-reference.html):
-
-- [x] Properties
-- [x] Condition
-- [x] DependsOn
-- [x] CreationPolicy
-- [x] UpdatePolicy
-- [x] UpdateReplacePolicy
-- [x] DeletionPolicy
-- [x] Metadata
-
-### [CloudFormation functions](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/intrinsic-function-reference.html):
-
-- [x] Ref
-- [x] Fn::GetAtt
-- [x] Fn::Join
-- [x] Fn::If
-- [x] Fn::And
-- [x] Fn::Equals
-- [x] Fn::Not
-- [x] Fn::Or
-- [x] Fn::Base64
-- [x] Fn::Cidr
-- [x] Fn::FindInMap
-- [x] Fn::GetAZs
-- [x] Fn::ImportValue
-- [x] Fn::Select
-- [x] Fn::Split
-- [ ] Fn::Sub
-- [x] Fn::Transform
diff --git a/packages/@aws-cdk/cloudformation-include/lib/file-utils.ts b/packages/@aws-cdk/cloudformation-include/lib/file-utils.ts
index dd58b76777d5d..65a05101bcb90 100644
--- a/packages/@aws-cdk/cloudformation-include/lib/file-utils.ts
+++ b/packages/@aws-cdk/cloudformation-include/lib/file-utils.ts
@@ -31,11 +31,9 @@ function makeTagForCfnIntrinsic(
 }
 
 const shortForms: yaml_types.Schema.CustomTag[] = [
-  'Base64', 'Cidr', 'FindInMap', 'GetAZs', 'ImportValue', 'Join',
+  'Base64', 'Cidr', 'FindInMap', 'GetAZs', 'ImportValue', 'Join', 'Sub',
   'Select', 'Split', 'Transform', 'And', 'Equals', 'If', 'Not', 'Or',
 ].map(name => makeTagForCfnIntrinsic(name)).concat(
-  // ToDo: special logic for ImportValue will be needed when support for Fn::Sub is added. See
-  // https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/intrinsic-function-reference-importvalue.html
   makeTagForCfnIntrinsic('Ref', false),
   makeTagForCfnIntrinsic('GetAtt', true, (_doc: yaml.Document, cstNode: yaml_cst.CST.Node): any => {
     // The position of the leftmost period and opening bracket tell us what syntax is being used
diff --git a/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts b/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts
index ef93cd9809d82..113e58773a242 100644
--- a/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts
+++ b/packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts
@@ -100,6 +100,18 @@ describe('CDK Include', () => {
       includeTestTemplate(stack, 'output-referencing-nonexistant-condition.json');
     }).toThrow(/Output with name 'SomeOutput' refers to a Condition with name 'NonexistantCondition' which was not found in this template/);
   });
+
+  test("throws a validation exception when Fn::Sub in string form uses a key that isn't in the template", () => {
+    expect(() => {
+      includeTestTemplate(stack, 'fn-sub-key-not-in-template-string.json');
+    }).toThrow(/Element referenced in Fn::Sub expression with logical ID: 'AFakeResource' was not found in the template/);
+  });
+
+  test('throws a validation exception when Fn::Sub has an empty ${} reference', () => {
+    expect(() => {
+      includeTestTemplate(stack, 'fn-sub-${}-only.json');
+    }).toThrow(/Element referenced in Fn::Sub expression with logical ID: '' was not found in the template/);
+  });
 });
 
 function includeTestTemplate(scope: core.Construct, testTemplate: string): inc.CfnInclude {
diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-brace-edges.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-brace-edges.json
new file mode 100644
index 0000000000000..b8ef634ac6cd0
--- /dev/null
+++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-brace-edges.json
@@ -0,0 +1,55 @@
+{
+  "Resources": {
+    "Bucket": {
+      "Type": "Custom::ManyStrings",
+      "Properties": {
+        "SymbolsOnly": {
+          "DollarSign": {
+            "Fn::Sub": "$"
+          },
+          "OpeningBrace": {
+            "Fn::Sub": "{"
+          },
+          "ClosingBrace": {
+            "Fn::Sub": "}"
+          },
+          "DollarOpeningBrace": {
+            "Fn::Sub": "${"
+          },
+          "DollarClosingBrace": {
+            "Fn::Sub": "$}"
+          },
+          "OpeningBraceDollar": {
+            "Fn::Sub": "{$"
+          },
+          "ClosingBraceDollar": {
+            "Fn::Sub": "}$"
+          }
+        },
+        "SymbolsAndResources": {
+          "DollarSign": {
+            "Fn::Sub": "DoesNotExist$DoesNotExist"
+          },
+          "OpeningBrace": {
+            "Fn::Sub": "DoesNotExist{DoesNotExist"
+          },
+          "ClosingBrace": {
+            "Fn::Sub": "DoesNotExist}DoesNotExist"
+          },
+          "DollarOpeningBrace": {
+            "Fn::Sub": "DoesNotExist${DoesNotExist"
+          },
+          "DollarClosingBrace": {
+            "Fn::Sub": "DoesNotExist$}DoesNotExist"
+          },
+          "OpeningBraceDollar": {
+            "Fn::Sub": "DoesNotExist{$DoesNotExist"
+          },
+          "ClosingBraceDollar": {
+            "Fn::Sub": "DoesNotExist}$DoesNotExist"
+          }
+        }
+      }
+    }
+  }
+}
diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-escaping.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-escaping.json
new file mode 100644
index 0000000000000..915a65819aec7
--- /dev/null
+++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-escaping.json
@@ -0,0 +1,10 @@
+{
+  "Resources": {
+    "Bucket": {
+      "Type": "AWS::S3::Bucket",
+      "Properties": {
+        "BucketName": { "Fn::Sub": "some-bucket${!AWS::AccountId}7896${ ! DoesNotExist}${!Immediate}234" }
+      }
+    }
+  }
+}
diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-map-dotted-attributes.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-map-dotted-attributes.json
new file mode 100644
index 0000000000000..5bc772b8f5860
--- /dev/null
+++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-map-dotted-attributes.json
@@ -0,0 +1,25 @@
+{
+  "Resources": {
+    "Bucket": {
+      "Type": "AWS::S3::Bucket",
+      "Properties": {
+        "BucketName": { 
+          "Fn::Sub": "${ELB.SourceSecurityGroup.GroupName}"
+        }
+      }
+    },
+    "ELB": {
+      "Type": "AWS::ElasticLoadBalancing::LoadBalancer",
+      "Properties": {
+        "AvailabilityZones": [
+          "us-east-1a"
+        ],
+        "Listeners": [{
+          "LoadBalancerPort": "80",
+          "InstancePort": "80",
+          "Protocol": "HTTP"
+        }]
+      }
+    }
+  }
+}
diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-override.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-override.json
new file mode 100644
index 0000000000000..cdaa623bfd7fa
--- /dev/null
+++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-override.json
@@ -0,0 +1,16 @@
+{
+  "Resources": {
+    "Bucket": {
+      "Type": "AWS::S3::Bucket",
+      "Properties": {
+        "BucketName": "bucket"
+      }
+    },
+    "AnotherBucket": {
+      "Type": "AWS::S3::Bucket",
+      "Properties": {
+        "BucketName": { "Fn::Sub": "${Bucket}-${!Bucket}-${Bucket.DomainName}" }
+      }
+    }
+  }
+}
diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-parsing-edges.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-parsing-edges.json
new file mode 100644
index 0000000000000..e69de29bb2d1d
diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-shadow-attribute.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-shadow-attribute.json
new file mode 100644
index 0000000000000..8401ee9a79ccb
--- /dev/null
+++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-shadow-attribute.json
@@ -0,0 +1,20 @@
+{
+  "Resources": {
+    "Bucket": {
+      "Type": "AWS::S3::Bucket",
+      "Properties": {
+        "BucketName": {
+          "Fn::Sub": [
+            "${AnotherBucket.DomainName}",
+            {
+               "AnotherBucket": "whatever"
+            }
+          ]
+        }
+      }
+    },
+    "AnotherBucket": {
+      "Type": "AWS::S3::Bucket"
+    }
+  }
+}
diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-shadow.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-shadow.json
new file mode 100644
index 0000000000000..6e5cdbee0be2c
--- /dev/null
+++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-shadow.json
@@ -0,0 +1,20 @@
+{
+  "Resources": {
+    "Bucket": {
+      "Type": "AWS::S3::Bucket",
+      "Properties": {
+        "BucketName": { 
+          "Fn::Sub": [ 
+            "${AnotherBucket}", 
+            { 
+              "AnotherBucket": { "Ref" : "AnotherBucket" }
+            } 
+          ]
+        }
+      }
+    },
+    "AnotherBucket": {
+      "Type": "AWS::S3::Bucket"
+    }
+  }
+}
diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-string.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-string.json
new file mode 100644
index 0000000000000..2936a59bb55fc
--- /dev/null
+++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-string.json
@@ -0,0 +1,16 @@
+{
+  "Resources": {
+    "Bucket": {
+      "Type": "AWS::S3::Bucket",
+      "Properties": {
+        "BucketName": "bucket"
+      }
+    },
+    "AnotherBucket": {
+      "Type": "AWS::S3::Bucket",
+      "Properties": {
+        "BucketName": { "Fn::Sub": "1-${AWS::Region}-foo-${Bucket}-${!Literal}-${Bucket.DomainName}-${AWS::Region}" }
+      }
+    }
+  }
+}
diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-${}-only.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-${}-only.json
new file mode 100644
index 0000000000000..87f9556e5a6b0
--- /dev/null
+++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-${}-only.json
@@ -0,0 +1,12 @@
+{
+  "Resources": {
+    "Bucket": {
+      "Type": "AWS::S3::Bucket",
+      "Properties": {
+        "BucketName": { 
+          "Fn::Sub": "${}"
+        }
+      }
+    }
+  }
+}
diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-key-not-in-template-string.json b/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-key-not-in-template-string.json
new file mode 100644
index 0000000000000..c5e9ff5b13b8d
--- /dev/null
+++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/fn-sub-key-not-in-template-string.json
@@ -0,0 +1,10 @@
+{
+  "Resources": {
+    "Bucket": {
+      "Type": "AWS::S3::Bucket",
+      "Properties": {
+        "AccessControl": { "Fn::Sub": "${AFakeResource}" }
+      }
+    }
+  }
+}
diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/invalid/short-form-import-sub.yaml b/packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/invalid/short-form-import-sub.yaml
new file mode 100644
index 0000000000000..899904f61a8cf
--- /dev/null
+++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/invalid/short-form-import-sub.yaml
@@ -0,0 +1,7 @@
+Resources:
+  Bucket:
+    Type: AWS::S3::Bucket
+    Properties:
+      BucketName:
+        !ImportValue
+          !Sub ${AWS::Region}
diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/long-form-vpc.yaml b/packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/long-form-vpc.yaml
index de8b072887d23..f32def7fd072a 100644
--- a/packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/long-form-vpc.yaml
+++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/long-form-vpc.yaml
@@ -42,3 +42,11 @@ Resources:
               Location: location,
               AnotherParameter:
                 Fn::Base64: AnotherValue
+      AccessControl:
+        Fn::ImportValue: 
+          Fn::Sub:
+            - "${Region}-foo-${!Immediate}-foo-${Vpc}-${Vpc.Id}-${Name}"
+            - Name:
+                Ref: Vpc
+              Region:
+                Fn::Base64: AWS::Region
diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/short-form-fnsub-string.yaml b/packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/short-form-fnsub-string.yaml
new file mode 100644
index 0000000000000..72ccedb2c61c9
--- /dev/null
+++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/short-form-fnsub-string.yaml
@@ -0,0 +1,11 @@
+Resources:
+  Bucket:
+    Type: AWS::S3::Bucket
+    Properties:
+      BucketName:
+        Fn::Sub: some-bucket${!AWS::AccountId}7896${ ! AWS::Region}1-1${!Immediate}234
+  AnotherBucket:
+    Type: AWS::S3::Bucket
+    Properties:
+      BucketName:
+        !Sub 1-${AWS::Region}-foo-${Bucket}-${!Literal}-${Bucket.DomainName}-${AWS::Region}
diff --git a/packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/short-form-sub-map.yaml b/packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/short-form-sub-map.yaml
new file mode 100644
index 0000000000000..80450b205abba
--- /dev/null
+++ b/packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/short-form-sub-map.yaml
@@ -0,0 +1,15 @@
+Resources:
+  Bucket:
+    Type: AWS::S3::Bucket
+    Properties:
+      BucketName:
+        !Sub
+        - "${Region}-foo-${!Immediate}-foo-${AnotherBucket}-${AnotherBucket.DomainName}-${Name}"
+        - Name:
+            Ref: AnotherBucket
+          Region:
+            Fn::Base64: AWS::Region
+  AnotherBucket:
+    Type: AWS::S3::Bucket
+    Properties:
+      BucketName: another-bucket
diff --git a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts
index 912525b7c369f..07529248be53b 100644
--- a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts
+++ b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts
@@ -192,6 +192,83 @@ describe('CDK Include', () => {
     );
   });
 
+  test('can ingest a template with Fn::Sub in string form with escaped and unescaped references and output it unchanged', () => {
+    includeTestTemplate(stack, 'fn-sub-string.json');
+
+    expect(stack).toMatchTemplate(
+      loadTestFileToJsObject('fn-sub-string.json'),
+    );
+  });
+
+  test('can parse the string argument Fn::Sub with escaped references that contain whitespace', () => {
+    includeTestTemplate(stack, 'fn-sub-escaping.json');
+
+    expect(stack).toMatchTemplate(
+      loadTestFileToJsObject('fn-sub-escaping.json'),
+    );
+  });
+
+  test('can ingest a template with Fn::Sub in map form and output it unchanged', () => {
+    includeTestTemplate(stack, 'fn-sub-map-dotted-attributes.json');
+
+    expect(stack).toMatchTemplate(
+      loadTestFileToJsObject('fn-sub-map-dotted-attributes.json'),
+    );
+  });
+
+  test('can ingest a template with Fn::Sub shadowing a logical ID from the template and output it unchanged', () => {
+    includeTestTemplate(stack, 'fn-sub-shadow.json');
+
+    expect(stack).toMatchTemplate(
+      loadTestFileToJsObject('fn-sub-shadow.json'),
+    );
+  });
+
+  test('can ingest a template with Fn::Sub attribute expression shadowing a logical ID from the template, and output it unchanged', () => {
+    includeTestTemplate(stack, 'fn-sub-shadow-attribute.json');
+
+    expect(stack).toMatchTemplate(
+      loadTestFileToJsObject('fn-sub-shadow-attribute.json'),
+    );
+  });
+
+  test('can modify resources used in Fn::Sub in map form references and see the changes in the template', () => {
+    const cfnTemplate = includeTestTemplate(stack, 'fn-sub-shadow.json');
+
+    cfnTemplate.getResource('AnotherBucket').overrideLogicalId('NewBucket');
+
+    expect(stack).toHaveResourceLike('AWS::S3::Bucket', {
+      "BucketName": {
+        "Fn::Sub": [
+          "${AnotherBucket}",
+          {
+            "AnotherBucket": { "Ref": "NewBucket" },
+          },
+        ],
+      },
+    });
+  });
+
+  test('can modify resources used in Fn::Sub in string form and see the changes in the template', () => {
+    const cfnTemplate = includeTestTemplate(stack, 'fn-sub-override.json');
+
+    cfnTemplate.getResource('Bucket').overrideLogicalId('NewBucket');
+
+    expect(stack).toHaveResourceLike('AWS::S3::Bucket', {
+      "BucketName": {
+        "Fn::Sub": "${NewBucket}-${!Bucket}-${NewBucket.DomainName}",
+      },
+    });
+  });
+
+  test('can ingest a template with Fn::Sub with brace edge cases and output it unchanged', () => {
+    includeTestTemplate(stack, 'fn-sub-brace-edges.json');
+
+    expect(stack).toMatchTemplate(
+      loadTestFileToJsObject('fn-sub-brace-edges.json'),
+    );
+  });
+
   test('can ingest a template with a Ref expression for an array value, and output it unchanged', () => {
     includeTestTemplate(stack, 'ref-array-property.json');
 
diff --git a/packages/@aws-cdk/cloudformation-include/test/yaml-templates.test.ts b/packages/@aws-cdk/cloudformation-include/test/yaml-templates.test.ts
index 01bba1610a9b5..11180842bdb34 100644
--- a/packages/@aws-cdk/cloudformation-include/test/yaml-templates.test.ts
+++ b/packages/@aws-cdk/cloudformation-include/test/yaml-templates.test.ts
@@ -323,6 +323,28 @@ describe('CDK Include', () => {
       loadTestFileToJsObject('long-form-subnet.yaml'),
     );
   });
+
+  test('can ingest a YAML tempalte with Fn::Sub in string form and output it unchanged', () => {
+    includeTestTemplate(stack, 'short-form-fnsub-string.yaml');
+
+    expect(stack).toMatchTemplate(
+      loadTestFileToJsObject('short-form-fnsub-string.yaml'),
+    );
+  });
+
+  test('can ingest a YAML tmeplate with Fn::Sub in map form and output it unchanged', () => {
+    includeTestTemplate(stack, 'short-form-sub-map.yaml');
+
+    expect(stack).toMatchTemplate(
+      loadTestFileToJsObject('short-form-sub-map.yaml'),
+    );
+  });
+
+  test('the parser throws an error on a YAML tmeplate with short form import value that uses short form sub', () => {
+    expect(() => {
+      includeTestTemplate(stack, 'invalid/short-form-import-sub.yaml');
+    }).toThrow(/A node can have at most one tag/);
+  });
 });
 
 function includeTestTemplate(scope: core.Construct, testTemplate: string): inc.CfnInclude {
diff --git a/packages/@aws-cdk/core/lib/cfn-parse.ts b/packages/@aws-cdk/core/lib/cfn-parse.ts
index 6c880b80cd420..c821b66971073 100644
--- a/packages/@aws-cdk/core/lib/cfn-parse.ts
+++ b/packages/@aws-cdk/core/lib/cfn-parse.ts
@@ -419,6 +419,20 @@ export class CfnParser {
         const value = this.parseValue(object[key]);
         return Fn.conditionOr(...value);
       }
+      case 'Fn::Sub': {
+        const value = this.parseValue(object[key]);
+        let fnSubString: string;
+        let map: { [key: string]: any } | undefined;
+        if (typeof value === 'string') {
+          fnSubString = value;
+          map = undefined;
+        } else {
+          fnSubString = value[0];
+          map = value[1];
+        }
+
+        return Fn.sub(this.parseFnSubString(fnSubString, map), map);
+      }
       case 'Condition': {
         // a reference to a Condition from another Condition
         const condition = this.options.finder.findCondition(object[key]);
@@ -446,6 +460,51 @@ export class CfnParser {
       ? key
       : undefined;
   }
+
+  private parseFnSubString(value: string, map: { [key: string]: any } = {}): string {
+    const leftBrace = value.indexOf('${');
+    const rightBrace = value.indexOf('}') + 1;
+    // don't include left and right braces when searching for the target of the reference
+    if (leftBrace === -1 || leftBrace >= rightBrace) {
+      return value;
+    }
+
+    const leftHalf = value.substring(0, leftBrace);
+    const rightHalf = value.substring(rightBrace);
+    const refTarget = value.substring(leftBrace + 2, rightBrace - 1).trim();
+    if (refTarget[0] === '!') {
+      return value.substring(0, rightBrace) + this.parseFnSubString(rightHalf, map);
+    }
+
+    // lookup in map
+    if (refTarget in map) {
+      return leftHalf + '${' + refTarget + '}' + this.parseFnSubString(rightHalf, map);
+    }
+
+    // since it's not in the map, check if it's a pseudo parameter
+    const specialRef = specialCaseSubRefs(refTarget);
+    if (specialRef) {
+      return leftHalf + specialRef + this.parseFnSubString(rightHalf, map);
+    }
+
+    const dotIndex = refTarget.indexOf('.');
+    const isRef = dotIndex === -1;
+    if (isRef) {
+      const refElement = this.options.finder.findRefTarget(refTarget);
+      if (!refElement) {
+        throw new Error(`Element referenced in Fn::Sub expression with logical ID: '${refTarget}' was not found in the template`);
+      }
+      return leftHalf + CfnReference.for(refElement, 'Ref', true).toString() + this.parseFnSubString(rightHalf, map);
+    } else {
+      const targetId = refTarget.substring(0, dotIndex);
+      const refResource = this.options.finder.findResource(targetId);
+      if (!refResource) {
+        throw new Error(`Resource referenced in Fn::Sub expression with logical ID: '${targetId}' was not found in the template`);
+      }
+      const attribute = refTarget.substring(dotIndex + 1);
+      return leftHalf + CfnReference.for(refResource, attribute, true).toString() + this.parseFnSubString(rightHalf, map);
+    }
+  }
 }
 
 function specialCaseRefs(value: any): any {
@@ -462,6 +521,10 @@ function specialCaseRefs(value: any): any {
   }
 }
 
+function specialCaseSubRefs(value: string): string | undefined {
+  return value.indexOf('::') === -1 ? undefined: '${' + value + '}';
+}
+
 function undefinedIfAllValuesAreEmpty(object: object): object | undefined {
   return Object.values(object).some(v => v !== undefined) ? object : undefined;
 }
diff --git a/packages/@aws-cdk/core/lib/private/cfn-reference.ts b/packages/@aws-cdk/core/lib/private/cfn-reference.ts
index cd28a221958c5..491232e344840 100644
--- a/packages/@aws-cdk/core/lib/private/cfn-reference.ts
+++ b/packages/@aws-cdk/core/lib/private/cfn-reference.ts
@@ -33,10 +33,15 @@ export class CfnReference extends Reference {
    * important that the state isn't lost if it's lazily created, like so:
    *
    *     Lazy.stringValue({ produce: () => new CfnReference(...) })
+   *
+   * If fnSub is true, then this reference will resolve as ${logicalID}.
+   * This allows cloudformation-include to correctly handle Fn::Sub.
    */
-  public static for(target: CfnElement, attribute: string) {
-    return CfnReference.singletonReference(target, attribute, () => {
-      const cfnIntrinsic = attribute === 'Ref' ? { Ref: target.logicalId } : { 'Fn::GetAtt': [ target.logicalId, attribute ]};
+  public static for(target: CfnElement, attribute: string, fnSub: boolean = false) {
+    return CfnReference.singletonReference(target, attribute, fnSub, () => {
+      const cfnIntrinsic = fnSub
+        ? ('${' + target.logicalId + (attribute === 'Ref' ? '' :  `.${attribute}`) + '}')
+        : (attribute === 'Ref' ? { Ref: target.logicalId } : { 'Fn::GetAtt': [target.logicalId, attribute] });
       return new CfnReference(cfnIntrinsic, attribute, target);
     });
   }
@@ -45,7 +50,7 @@ export class CfnReference extends Reference {
    * Return a CfnReference that references a pseudo referencd
    */
   public static forPseudo(pseudoName: string, scope: Construct) {
-    return CfnReference.singletonReference(scope, `Pseudo:${pseudoName}`, () => {
+    return CfnReference.singletonReference(scope, `Pseudo:${pseudoName}`, false, () => {
       const cfnIntrinsic = { Ref: pseudoName };
       return new CfnReference(cfnIntrinsic, pseudoName, scope);
     });
@@ -57,18 +62,20 @@ export class CfnReference extends Reference {
   private static referenceTable = new Map<Construct, Map<string, CfnReference>>();
 
   /**
-   * Get or create the table
+   * Get or create the table.
+   * Passing fnSub = true allows cloudformation-include to correctly handle Fn::Sub.
    */
-  private static singletonReference(target: Construct, attribKey: string, fresh: () => CfnReference) {
+  private static singletonReference(target: Construct, attribKey: string, fnSub: boolean, fresh: () => CfnReference) {
     let attribs = CfnReference.referenceTable.get(target);
     if (!attribs) {
       attribs = new Map();
       CfnReference.referenceTable.set(target, attribs);
     }
-    let ref = attribs.get(attribKey);
+    const cacheKey = attribKey + (fnSub ? 'Fn::Sub' : '');
+    let ref = attribs.get(cacheKey);
     if (!ref) {
       ref = fresh();
-      attribs.set(attribKey, ref);
+      attribs.set(cacheKey, ref);
     }
     return ref;
   }