From eec849b92eb32641ddba630ca736172f86a1de99 Mon Sep 17 00:00:00 2001 From: Neta Nir Date: Tue, 28 Jul 2020 16:34:03 -0700 Subject: [PATCH 01/19] chore(core): copy test reports from coverage folder (#9311) Reverts aws/aws-cdk#9284 --- .../test/integ/test-cli-regression-against-current-code.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/aws-cdk/test/integ/test-cli-regression-against-current-code.sh b/packages/aws-cdk/test/integ/test-cli-regression-against-current-code.sh index fbef55098288e..1a62b61d04b33 100755 --- a/packages/aws-cdk/test/integ/test-cli-regression-against-current-code.sh +++ b/packages/aws-cdk/test/integ/test-cli-regression-against-current-code.sh @@ -14,7 +14,7 @@ temp_dir=$(mktemp -d) function cleanup { # keep junit file to allow report creation - cp ${integ_under_test}/junit.xml . + cp ${integ_under_test}/coverage/junit.xml . rm -rf ${temp_dir} rm -rf ${integ_under_test} } From d8f3ce803b3961100863a6999b485fa8f516c209 Mon Sep 17 00:00:00 2001 From: Adam Ruka Date: Tue, 28 Jul 2020 23:02:43 -0700 Subject: [PATCH 02/19] build: skip compatibility check for Gitpod (#9315) There is no point in doing the compatibility check when spinning up a new Gitpod workspace, so add an option to build.sh to skip it, and use it for the Gitpod build. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .gitpod.yml | 2 +- build.sh | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/.gitpod.yml b/.gitpod.yml index 92cf51b5ccdd3..61bf069e34517 100644 --- a/.gitpod.yml +++ b/.gitpod.yml @@ -1,6 +1,6 @@ image: jsii/superchain tasks: - - init: yarn build --skip-test --no-bail --skip-prereqs + - init: yarn build --skip-test --no-bail --skip-prereqs --skip-compat vscode: extensions: diff --git a/build.sh b/build.sh index 754f7ce10111e..a17b80797c8ec 100755 --- a/build.sh +++ b/build.sh @@ -4,6 +4,7 @@ set -euo pipefail bail="--bail" runtarget="build+test" check_prereqs="true" +check_compat="true" while [[ "${1:-}" != "" ]]; do case $1 in -h|--help) @@ -22,6 +23,9 @@ while [[ "${1:-}" != "" ]]; do --skip-prereqs) check_prereqs="false" ;; + --skip-compat) + check_compat="false" + ;; *) echo "Unrecognized parameter: $1" exit 1 @@ -70,6 +74,8 @@ echo "========================================================================== echo "building..." time lerna run $bail --stream $runtarget || fail -/bin/bash scripts/check-api-compatibility.sh +if [ "$check_compat" == "true" ]; then + /bin/bash scripts/check-api-compatibility.sh +fi touch $BUILD_INDICATOR From cbfdc15959c5d5209d4fed6ac281f9897f44d4c5 Mon Sep 17 00:00:00 2001 From: Adam Ruka Date: Wed, 29 Jul 2020 02:06:17 -0700 Subject: [PATCH 03/19] chore(cloudfront): small refactoring of the Origin API (#9281) Change the Origin to an interface, from an abstract class, and change its `bind` protocol to return an `OriginBindConfig` interface. This is in preparation for handling Origin Groups in #9109 - when time comes to handle Origin Groups, we will add a new (optional) property to `OriginBindConfig`, of type `CfnDistribution.OriginGroupProperty`, and handle it in `Distribution`. BREAKING CHANGE: the property Origin.domainName has been removed ---- *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/s3-origin.ts | 21 ++----- .../test/http-origin.test.ts | 8 +-- .../test/integ.s3-origin.expected.json | 8 +-- .../test/load-balancer-origin.test.ts | 8 +-- .../test/s3-origin.test.ts | 14 ++--- .../aws-cloudfront/lib/distribution.ts | 43 +++++++++----- .../@aws-cdk/aws-cloudfront/lib/origin.ts | 58 +++++++++++-------- .../lib/private/cache-behavior.ts | 17 ++---- .../aws-cloudfront/test/distribution.test.ts | 19 +++--- .../aws-cloudfront/test/origin.test.ts | 24 ++++---- .../test/private/cache-behavior.test.ts | 21 +++---- 11 files changed, 121 insertions(+), 120 deletions(-) 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 b976e482b2276..2896ba2a5df79 100644 --- a/packages/@aws-cdk/aws-cloudfront-origins/lib/s3-origin.ts +++ b/packages/@aws-cdk/aws-cloudfront-origins/lib/s3-origin.ts @@ -26,9 +26,8 @@ export interface S3OriginProps { * * @experimental */ -export class S3Origin extends cloudfront.Origin { - - private readonly origin: cloudfront.Origin; +export class S3Origin implements cloudfront.IOrigin { + private readonly origin: cloudfront.IOrigin; constructor(bucket: s3.IBucket, props: S3OriginProps = {}) { let proxyOrigin; @@ -43,22 +42,10 @@ export class S3Origin extends cloudfront.Origin { ...props, }); } - - super(proxyOrigin.domainName); - this.origin = proxyOrigin; } - public get id() { - return this.origin.id; - } - - public bind(scope: cdk.Construct, options: cloudfront.OriginBindOptions) { - this.origin.bind(scope, options); - } - - public renderOrigin() { - return this.origin.renderOrigin(); + public bind(scope: cdk.Construct, options: cloudfront.OriginBindOptions): cloudfront.OriginBindConfig { + return this.origin.bind(scope, options); } - } 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 a5475dc4c760b..246392355586e 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 @@ -15,9 +15,9 @@ beforeEach(() => { test('Renders minimal example with just a domain name', () => { const origin = new HttpOrigin('www.example.com'); - origin.bind(stack, { originIndex: 0 }); + const originBindConfig = origin.bind(stack, { originId: 'StackOrigin029E19582' }); - expect(origin.renderOrigin()).toEqual({ + expect(originBindConfig.originProperty).toEqual({ id: 'StackOrigin029E19582', domainName: 'www.example.com', customOriginConfig: { @@ -32,9 +32,9 @@ test('Can customize properties of the origin', () => { readTimeout: Duration.seconds(10), protocolPolicy: cloudfront.OriginProtocolPolicy.MATCH_VIEWER, }); - origin.bind(stack, { originIndex: 0 }); + const originBindConfig = origin.bind(stack, { originId: 'StackOrigin029E19582' }); - expect(origin.renderOrigin()).toEqual({ + expect(originBindConfig.originProperty).toEqual({ id: 'StackOrigin029E19582', domainName: 'www.example.com', originCustomHeaders: [{ diff --git a/packages/@aws-cdk/aws-cloudfront-origins/test/integ.s3-origin.expected.json b/packages/@aws-cdk/aws-cloudfront-origins/test/integ.s3-origin.expected.json index 64455613ffcbc..173044c7a0dab 100644 --- a/packages/@aws-cdk/aws-cloudfront-origins/test/integ.s3-origin.expected.json +++ b/packages/@aws-cdk/aws-cloudfront-origins/test/integ.s3-origin.expected.json @@ -23,7 +23,7 @@ "Principal": { "CanonicalUser": { "Fn::GetAtt": [ - "DistributionS3Origin115FD918D", + "DistributionOrigin1S3Origin5F5C0696", "S3CanonicalUserId" ] } @@ -56,7 +56,7 @@ } } }, - "DistributionS3Origin115FD918D": { + "DistributionOrigin1S3Origin5F5C0696": { "Type": "AWS::CloudFront::CloudFrontOriginAccessIdentity", "Properties": { "CloudFrontOriginAccessIdentityConfig": { @@ -92,7 +92,7 @@ [ "origin-access-identity/cloudfront/", { - "Ref": "DistributionS3Origin115FD918D" + "Ref": "DistributionOrigin1S3Origin5F5C0696" } ] ] @@ -104,4 +104,4 @@ } } } -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-cloudfront-origins/test/load-balancer-origin.test.ts b/packages/@aws-cdk/aws-cloudfront-origins/test/load-balancer-origin.test.ts index c06c9cc7a7b84..e4ad46372fbef 100644 --- a/packages/@aws-cdk/aws-cloudfront-origins/test/load-balancer-origin.test.ts +++ b/packages/@aws-cdk/aws-cloudfront-origins/test/load-balancer-origin.test.ts @@ -21,9 +21,9 @@ test('Renders minimal example with just a load balancer', () => { }); const origin = new LoadBalancerV2Origin(loadBalancer); - origin.bind(stack, { originIndex: 0 }); + const originBindConfig = origin.bind(stack, { originId: 'StackOrigin029E19582' }); - expect(origin.renderOrigin()).toEqual({ + expect(originBindConfig.originProperty).toEqual({ id: 'StackOrigin029E19582', domainName: loadBalancer.loadBalancerDnsName, customOriginConfig: { @@ -43,9 +43,9 @@ test('Can customize properties of the origin', () => { connectionTimeout: Duration.seconds(5), protocolPolicy: cloudfront.OriginProtocolPolicy.MATCH_VIEWER, }); - origin.bind(stack, { originIndex: 0 }); + const originBindConfig = origin.bind(stack, { originId: 'StackOrigin029E19582' }); - expect(origin.renderOrigin()).toEqual({ + expect(originBindConfig.originProperty).toEqual({ id: 'StackOrigin029E19582', domainName: loadBalancer.loadBalancerDnsName, connectionAttempts: 3, 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 c851f53b122a0..9c0380411e494 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 @@ -17,9 +17,9 @@ test('With non-website bucket, renders all required properties, including S3Orig const bucket = new s3.Bucket(stack, 'Bucket'); const origin = new S3Origin(bucket); - origin.bind(stack, { originIndex: 0 }); + const originBindConfig = origin.bind(stack, { originId: 'StackOrigin029E19582' }); - expect(origin.renderOrigin()).toEqual({ + expect(originBindConfig.originProperty).toEqual({ id: 'StackOrigin029E19582', domainName: bucket.bucketRegionalDomainName, s3OriginConfig: { @@ -34,9 +34,9 @@ test('With website bucket, renders all required properties, including custom ori }); const origin = new S3Origin(bucket); - origin.bind(stack, { originIndex: 0 }); + const originBindConfig = origin.bind(stack, { originId: 'StackOrigin029E19582' }); - expect(origin.renderOrigin()).toEqual({ + expect(originBindConfig.originProperty).toEqual({ id: 'StackOrigin029E19582', domainName: bucket.bucketWebsiteDomainName, customOriginConfig: { @@ -51,9 +51,9 @@ test('Respects props passed down to underlying origin', () => { }); const origin = new S3Origin(bucket, { originPath: '/website' }); - origin.bind(stack, { originIndex: 0 }); + const originBindConfig = origin.bind(stack, { originId: 'StackOrigin029E19582' }); - expect(origin.renderOrigin()).toEqual({ + expect(originBindConfig.originProperty).toEqual({ id: 'StackOrigin029E19582', domainName: bucket.bucketWebsiteDomainName, originPath: '/website', @@ -61,4 +61,4 @@ test('Respects props passed down to underlying origin', () => { originProtocolPolicy: 'http-only', }, }); -}); \ No newline at end of file +}); diff --git a/packages/@aws-cdk/aws-cloudfront/lib/distribution.ts b/packages/@aws-cdk/aws-cloudfront/lib/distribution.ts index e32693aa79888..19516b8a59ae8 100644 --- a/packages/@aws-cdk/aws-cloudfront/lib/distribution.ts +++ b/packages/@aws-cdk/aws-cloudfront/lib/distribution.ts @@ -2,7 +2,7 @@ import * as acm from '@aws-cdk/aws-certificatemanager'; import * as lambda from '@aws-cdk/aws-lambda'; import { Construct, IResource, Lazy, Resource, Stack, Token, Duration } from '@aws-cdk/core'; import { CfnDistribution } from './cloudfront.generated'; -import { Origin } from './origin'; +import { IOrigin, OriginBindConfig, OriginBindOptions } from './origin'; import { CacheBehavior } from './private/cache-behavior'; /** @@ -53,6 +53,10 @@ export interface DistributionAttributes { readonly distributionId: string; } +interface BoundOrigin extends OriginBindOptions, OriginBindConfig { + readonly origin: IOrigin; +} + /** * Properties for a Distribution * @@ -127,7 +131,7 @@ export class Distribution extends Resource implements IDistribution { private readonly defaultBehavior: CacheBehavior; private readonly additionalBehaviors: CacheBehavior[] = []; - private readonly origins: Set = new Set(); + private readonly boundOrigins: BoundOrigin[] = []; private readonly errorResponses: ErrorResponse[]; private readonly certificate?: acm.ICertificate; @@ -142,8 +146,8 @@ export class Distribution extends Resource implements IDistribution { } } - this.defaultBehavior = new CacheBehavior({ pathPattern: '*', ...props.defaultBehavior }); - this.addOrigin(this.defaultBehavior.origin); + const originId = this.addOrigin(props.defaultBehavior.origin); + this.defaultBehavior = new CacheBehavior(originId, { pathPattern: '*', ...props.defaultBehavior }); if (props.additionalBehaviors) { Object.entries(props.additionalBehaviors).forEach(([pathPattern, behaviorOptions]) => { this.addBehavior(pathPattern, behaviorOptions.origin, behaviorOptions); @@ -172,26 +176,38 @@ export class Distribution extends Resource implements IDistribution { * Adds a new behavior to this distribution for the given pathPattern. * * @param pathPattern the path pattern (e.g., 'images/*') that specifies which requests to apply the behavior to. + * @param origin the origin to use for this behavior * @param behaviorOptions the options for the behavior at this path. */ - public addBehavior(pathPattern: string, origin: Origin, behaviorOptions: AddBehaviorOptions = {}) { + public addBehavior(pathPattern: string, origin: IOrigin, behaviorOptions: AddBehaviorOptions = {}) { if (pathPattern === '*') { throw new Error('Only the default behavior can have a path pattern of \'*\''); } - this.additionalBehaviors.push(new CacheBehavior({ pathPattern, origin, ...behaviorOptions })); - this.addOrigin(origin); + const originId = this.addOrigin(origin); + this.additionalBehaviors.push(new CacheBehavior(originId, { pathPattern, ...behaviorOptions })); } - private addOrigin(origin: Origin) { - if (!this.origins.has(origin)) { - this.origins.add(origin); - origin.bind(this, { originIndex: this.origins.size }); + private addOrigin(origin: IOrigin): string { + const existingOrigin = this.boundOrigins.find(boundOrigin => boundOrigin.origin === origin); + if (existingOrigin) { + return existingOrigin.originId; + } else { + const originIndex = this.boundOrigins.length + 1; + const scope = new Construct(this, `Origin${originIndex}`); + const originId = scope.node.uniqueId; + const originBindConfig = origin.bind(scope, { originId }); + this.boundOrigins.push({ origin, originId, ...originBindConfig }); + return originId; } } private renderOrigins(): CfnDistribution.OriginProperty[] { const renderedOrigins: CfnDistribution.OriginProperty[] = []; - this.origins.forEach(origin => renderedOrigins.push(origin.renderOrigin())); + this.boundOrigins.forEach(boundOrigin => { + if (boundOrigin.originProperty) { + renderedOrigins.push(boundOrigin.originProperty); + } + }); return renderedOrigins; } @@ -229,7 +245,6 @@ export class Distribution extends Resource implements IDistribution { minimumProtocolVersion: SecurityPolicyProtocol.TLS_V1_2_2018, }; } - } /** @@ -443,5 +458,5 @@ export interface BehaviorOptions extends AddBehaviorOptions { /** * The origin that you want CloudFront to route requests to when they match this behavior. */ - readonly origin: Origin; + readonly origin: IOrigin; } diff --git a/packages/@aws-cdk/aws-cloudfront/lib/origin.ts b/packages/@aws-cdk/aws-cloudfront/lib/origin.ts index ee15b4106f0e1..5e680f5e29f48 100644 --- a/packages/@aws-cdk/aws-cloudfront/lib/origin.ts +++ b/packages/@aws-cdk/aws-cloudfront/lib/origin.ts @@ -4,6 +4,28 @@ 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 { + /** + * The CloudFormation OriginProperty configuration for this Origin. + * + * @default - nothing is returned + */ + readonly originProperty?: CfnDistribution.OriginProperty; +} + +/** + * Represents the concept of a CloudFront Origin. + * You provide one or more origins when creating a Distribution. + */ +export interface IOrigin { + /** + * The method called when a given Origin is added + * (for the first time) to a Distribution. + */ + bind(scope: Construct, options: OriginBindOptions): OriginBindConfig; +} + /** * Properties to define an Origin. * @@ -48,9 +70,10 @@ export interface OriginProps { */ export interface OriginBindOptions { /** - * The positional index of this origin within the distribution. Used for ensuring unique IDs. + * The identifier of this Origin, + * as assigned by the Distribution this Origin has been used added to. */ - readonly originIndex: number; + readonly originId: string; } /** @@ -59,13 +82,8 @@ export interface OriginBindOptions { * * @experimental */ -export abstract class Origin { - - /** - * The domain name of the origin. - */ - public readonly domainName: string; - +export abstract class Origin implements IOrigin { + private readonly domainName: string; private readonly originPath?: string; private readonly connectionTimeout?: Duration; private readonly connectionAttempts?: number; @@ -97,14 +115,9 @@ export abstract class Origin { /** * Binds the origin to the associated Distribution. Can be used to grant permissions, create dependent resources, etc. */ - public bind(scope: Construct, options: OriginBindOptions): void { - this.originId = new Construct(scope, `Origin${options.originIndex}`).node.uniqueId; - } + public bind(_scope: Construct, options: OriginBindOptions): OriginBindConfig { + this.originId = options.originId; - /** - * Creates and returns the CloudFormation representation of this origin. - */ - public renderOrigin(): CfnDistribution.OriginProperty { const s3OriginConfig = this.renderS3OriginConfig(); const customOriginConfig = this.renderCustomOriginConfig(); @@ -112,7 +125,7 @@ export abstract class Origin { throw new Error('Subclass must override and provide either s3OriginConfig or customOriginConfig'); } - return { + return { originProperty: { domainName: this.domainName, id: this.id, originPath: this.originPath, @@ -121,7 +134,7 @@ export abstract class Origin { originCustomHeaders: this.renderCustomHeaders(), s3OriginConfig, customOriginConfig, - }; + }}; } // Overridden by sub-classes to provide S3 origin config. @@ -153,7 +166,6 @@ export abstract class Origin { if (path.endsWith('/')) { path = path.substr(0, path.length - 1); } return path; } - } /** @@ -184,12 +196,12 @@ export class S3Origin extends Origin { this.bucket = props.bucket; } - public bind(scope: Construct, options: OriginBindOptions) { - super.bind(scope, options); + public bind(scope: Construct, options: OriginBindOptions): OriginBindConfig { if (!this.originAccessIdentity) { - this.originAccessIdentity = new OriginAccessIdentity(scope, `S3Origin${options.originIndex}`); + this.originAccessIdentity = new OriginAccessIdentity(scope, 'S3Origin'); this.bucket.grantRead(this.originAccessIdentity); } + return super.bind(scope, options); } protected renderS3OriginConfig(): CfnDistribution.S3OriginConfigProperty | undefined { @@ -275,4 +287,4 @@ function validateIntInRangeOrUndefined(name: string, min: number, max: number, v const seconds = isDuration ? ' seconds' : ''; throw new Error(`${name}: Must be an int between ${min} and ${max}${seconds} (inclusive); received ${value}.`); } -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-cloudfront/lib/private/cache-behavior.ts b/packages/@aws-cdk/aws-cloudfront/lib/private/cache-behavior.ts index 363590333e777..2e44abcfc05c0 100644 --- a/packages/@aws-cdk/aws-cloudfront/lib/private/cache-behavior.ts +++ b/packages/@aws-cdk/aws-cloudfront/lib/private/cache-behavior.ts @@ -1,11 +1,10 @@ import { CfnDistribution } from '../cloudfront.generated'; -import { BehaviorOptions, ViewerProtocolPolicy } from '../distribution'; -import { Origin } from '../origin'; +import { AddBehaviorOptions, ViewerProtocolPolicy } from '../distribution'; /** * Properties for specifying custom behaviors for origins. */ -export interface CacheBehaviorProps extends BehaviorOptions { +export interface CacheBehaviorProps extends AddBehaviorOptions { /** * The pattern (e.g., `images/*.jpg`) that specifies which requests to apply the behavior to. * There must be exactly one behavior associated with each `Distribution` that has a path pattern @@ -21,14 +20,10 @@ export interface CacheBehaviorProps extends BehaviorOptions { * CloudFrontWebDistribution implementation. */ export class CacheBehavior { + private readonly originId: string; - /** - * Origin that this behavior will route traffic to. - */ - public readonly origin: Origin; - - constructor(private readonly props: CacheBehaviorProps) { - this.origin = props.origin; + constructor(originId: string, private readonly props: CacheBehaviorProps) { + this.originId = originId; } /** @@ -42,7 +37,7 @@ export class CacheBehavior { public _renderBehavior(): CfnDistribution.CacheBehaviorProperty { return { pathPattern: this.props.pathPattern, - targetOriginId: this.origin.id, + targetOriginId: this.originId, allowedMethods: this.props.allowedMethods?.methods ?? undefined, forwardedValues: { queryString: this.props.forwardQueryString ?? false, diff --git a/packages/@aws-cdk/aws-cloudfront/test/distribution.test.ts b/packages/@aws-cdk/aws-cloudfront/test/distribution.test.ts index 3db81d0b7ae1c..aa29d8da5e8ce 100644 --- a/packages/@aws-cdk/aws-cloudfront/test/distribution.test.ts +++ b/packages/@aws-cdk/aws-cloudfront/test/distribution.test.ts @@ -3,7 +3,7 @@ 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, LambdaEdgeEventType, Origin, PriceClass, S3Origin } from '../lib'; +import { Distribution, IOrigin, LambdaEdgeEventType, S3Origin, PriceClass } from '../lib'; let app: App; let stack: Stack; @@ -32,7 +32,7 @@ test('minimal example renders correctly', () => { Id: 'StackMyDistOrigin1D6D5E535', S3OriginConfig: { OriginAccessIdentity: { 'Fn::Join': [ '', - [ 'origin-access-identity/cloudfront/', { Ref: 'MyDistS3Origin1ED86A27E' } ], + [ 'origin-access-identity/cloudfront/', { Ref: 'MyDistOrigin1S3Origin071F2793' } ], ]}, }, }], @@ -83,7 +83,7 @@ describe('multiple behaviors', () => { Id: 'StackMyDistOrigin1D6D5E535', S3OriginConfig: { OriginAccessIdentity: { 'Fn::Join': [ '', - [ 'origin-access-identity/cloudfront/', { Ref: 'MyDistS3Origin1ED86A27E' } ], + [ 'origin-access-identity/cloudfront/', { Ref: 'MyDistOrigin1S3Origin071F2793' } ], ]}, }, }], @@ -121,7 +121,7 @@ describe('multiple behaviors', () => { Id: 'StackMyDistOrigin1D6D5E535', S3OriginConfig: { OriginAccessIdentity: { 'Fn::Join': [ '', - [ 'origin-access-identity/cloudfront/', { Ref: 'MyDistS3Origin1ED86A27E' } ], + [ 'origin-access-identity/cloudfront/', { Ref: 'MyDistOrigin1S3Origin071F2793' } ], ]}, }, }, @@ -130,7 +130,7 @@ describe('multiple behaviors', () => { Id: 'StackMyDistOrigin20B96F3AD', S3OriginConfig: { OriginAccessIdentity: { 'Fn::Join': [ '', - [ 'origin-access-identity/cloudfront/', { Ref: 'MyDistS3Origin2E88F08BB' } ], + [ 'origin-access-identity/cloudfront/', { Ref: 'MyDistOrigin2S3Origin26A1EB27' } ], ]}, }, }], @@ -175,7 +175,7 @@ describe('multiple behaviors', () => { Id: 'StackMyDistOrigin1D6D5E535', S3OriginConfig: { OriginAccessIdentity: { 'Fn::Join': [ '', - [ 'origin-access-identity/cloudfront/', { Ref: 'MyDistS3Origin1ED86A27E' } ], + [ 'origin-access-identity/cloudfront/', { Ref: 'MyDistOrigin1S3Origin071F2793' } ], ]}, }, }, @@ -184,14 +184,13 @@ describe('multiple behaviors', () => { Id: 'StackMyDistOrigin20B96F3AD', S3OriginConfig: { OriginAccessIdentity: { 'Fn::Join': [ '', - [ 'origin-access-identity/cloudfront/', { Ref: 'MyDistS3Origin2E88F08BB' } ], + [ 'origin-access-identity/cloudfront/', { Ref: 'MyDistOrigin2S3Origin26A1EB27' } ], ]}, }, }], }, }); }); - }); describe('certificates', () => { @@ -291,7 +290,7 @@ describe('custom error responses', () => { describe('with Lambda@Edge functions', () => { let lambdaFunction: lambda.Function; - let origin: Origin; + let origin: IOrigin; beforeEach(() => { lambdaFunction = new lambda.Function(stack, 'Function', { @@ -398,6 +397,6 @@ test('price class is included if provided', () => { }); }); -function defaultS3Origin(): Origin { +function defaultS3Origin(): IOrigin { return new S3Origin({ bucket: new s3.Bucket(stack, 'Bucket') }); } diff --git a/packages/@aws-cdk/aws-cloudfront/test/origin.test.ts b/packages/@aws-cdk/aws-cloudfront/test/origin.test.ts index 716af7e3225f5..fb260dc221c9b 100644 --- a/packages/@aws-cdk/aws-cloudfront/test/origin.test.ts +++ b/packages/@aws-cdk/aws-cloudfront/test/origin.test.ts @@ -1,7 +1,7 @@ import '@aws-cdk/assert/jest'; import * as s3 from '@aws-cdk/aws-s3'; import { App, Stack, Duration } from '@aws-cdk/core'; -import { CfnDistribution, Distribution, Origin, OriginProps, HttpOrigin, OriginProtocolPolicy, S3Origin } from '../lib'; +import { CfnDistribution, Distribution, HttpOrigin, OriginProtocolPolicy, S3Origin, Origin, OriginProps } from '../lib'; let app: App; let stack: Stack; @@ -18,9 +18,9 @@ describe('S3Origin', () => { const bucket = new s3.Bucket(stack, 'Bucket'); const origin = new S3Origin({ bucket }); - origin.bind(stack, { originIndex: 0 }); + const originBindConfig = origin.bind(stack, { originId: 'StackOrigin029E19582' }); - expect(origin.renderOrigin()).toEqual({ + expect(originBindConfig.originProperty).toEqual({ id: 'StackOrigin029E19582', domainName: bucket.bucketRegionalDomainName, s3OriginConfig: { @@ -44,7 +44,7 @@ describe('S3Origin', () => { PolicyDocument: { Statement: [{ Principal: { - CanonicalUser: { 'Fn::GetAtt': [ 'DistS3Origin1C4519663', 'S3CanonicalUserId' ] }, + CanonicalUser: { 'Fn::GetAtt': [ 'DistOrigin1S3Origin87D64058', 'S3CanonicalUserId' ] }, }, }], }, @@ -55,9 +55,9 @@ describe('S3Origin', () => { describe('HttpOrigin', () => { test('renders a minimal example with required props', () => { const origin = new HttpOrigin('www.example.com'); - origin.bind(stack, { originIndex: 0 }); + const originBindConfig = origin.bind(stack, { originId: 'StackOrigin029E19582' }); - expect(origin.renderOrigin()).toEqual({ + expect(originBindConfig.originProperty).toEqual({ id: 'StackOrigin029E19582', domainName: 'www.example.com', customOriginConfig: { @@ -78,9 +78,9 @@ describe('HttpOrigin', () => { readTimeout: Duration.seconds(45), keepaliveTimeout: Duration.seconds(3), }); - origin.bind(stack, { originIndex: 0 }); + const originBindConfig = origin.bind(stack, { originId: 'StackOrigin029E19582' }); - expect(origin.renderOrigin()).toEqual({ + expect(originBindConfig.originProperty).toEqual({ id: 'StackOrigin029E19582', domainName: 'www.example.com', originPath: '/app', @@ -127,7 +127,7 @@ describe('HttpOrigin', () => { }); }).toThrow(`keepaliveTimeout: Must be an int between 1 and 60 seconds (inclusive); received ${keepaliveTimeout.toSeconds()}.`); }); -});; +}); describe('Origin', () => { test.each([ @@ -158,9 +158,9 @@ describe('Origin', () => { const origin = new TestOrigin('www.example.com', { originPath, }); - origin.bind(stack, { originIndex: 0 }); + const originBindConfig = origin.bind(stack, { originId: '0' }); - expect(origin.renderOrigin().originPath).toEqual('/api'); + expect(originBindConfig.originProperty?.originPath).toEqual('/api'); }); }); @@ -170,4 +170,4 @@ class TestOrigin extends Origin { protected renderS3OriginConfig(): CfnDistribution.S3OriginConfigProperty | undefined { return { originAccessIdentity: 'origin-access-identity/cloudfront/MyOAIName' }; } -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-cloudfront/test/private/cache-behavior.test.ts b/packages/@aws-cdk/aws-cloudfront/test/private/cache-behavior.test.ts index 81dd84d716dd9..4477a411cf790 100644 --- a/packages/@aws-cdk/aws-cloudfront/test/private/cache-behavior.test.ts +++ b/packages/@aws-cdk/aws-cloudfront/test/private/cache-behavior.test.ts @@ -1,28 +1,24 @@ import '@aws-cdk/assert/jest'; import { App, Stack } from '@aws-cdk/core'; -import { AllowedMethods, HttpOrigin } from '../../lib'; +import { AllowedMethods } from '../../lib'; import { CacheBehavior } from '../../lib/private/cache-behavior'; let app: App; -let stack: Stack; beforeEach(() => { app = new App(); - stack = new Stack(app, 'Stack', { + new Stack(app, 'Stack', { env: { account: '1234', region: 'testregion' }, }); }); test('renders the minimum template with an origin and path specified', () => { - const origin = new HttpOrigin('www.example.com'); - const behavior = new CacheBehavior({ - origin, + const behavior = new CacheBehavior('origin_id', { pathPattern: '*', }); - origin.bind(stack, { originIndex: 0 }); expect(behavior._renderBehavior()).toEqual({ - targetOriginId: behavior.origin.id, + targetOriginId: 'origin_id', pathPattern: '*', forwardedValues: { queryString: false }, viewerProtocolPolicy: 'allow-all', @@ -30,18 +26,15 @@ test('renders the minimum template with an origin and path specified', () => { }); test('renders with all properties specified', () => { - const origin = new HttpOrigin('www.example.com'); - const behavior = new CacheBehavior({ - origin, + const behavior = new CacheBehavior('origin_id', { pathPattern: '*', allowedMethods: AllowedMethods.ALLOW_ALL, forwardQueryString: true, forwardQueryStringCacheKeys: ['user_id', 'auth'], }); - origin.bind(stack, { originIndex: 0 }); expect(behavior._renderBehavior()).toEqual({ - targetOriginId: behavior.origin.id, + targetOriginId: 'origin_id', pathPattern: '*', allowedMethods: ['GET', 'HEAD', 'OPTIONS', 'PUT', 'PATCH', 'POST', 'DELETE'], forwardedValues: { @@ -50,4 +43,4 @@ test('renders with all properties specified', () => { }, viewerProtocolPolicy: 'allow-all', }); -}); \ No newline at end of file +}); From 9f5b0bc293caed0d10c427fb46b966cfbff7af91 Mon Sep 17 00:00:00 2001 From: Nick Lynch Date: Wed, 29 Jul 2020 14:24:27 +0100 Subject: [PATCH 04/19] 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 Date: Wed, 29 Jul 2020 07:38:19 -0700 Subject: [PATCH 05/19] 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 06/19] 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 07/19] 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>(); /** - * 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; } From 88d563fab6971c20754d76c5d2eb9f4463aeae6c Mon Sep 17 00:00:00 2001 From: Nick Lynch Date: Thu, 30 Jul 2020 13:17:00 +0100 Subject: [PATCH 08/19] fix(pipelines): reduce assets IAM policy size (#9333) Collapse the PipelineAssetsRoleDefaultPolicy into a single up-front policy that doesn't grow per-asset. This relaxes some of the permissions in exchange for avoiding an O(N) policy size. fixes #9316 _Testing notes:_ Successfully deployed a pipeline with 49 assets (25 file assets and 24 docker assets). 50 assets is the limit for a single stage of the pipeline. To scale out past 50 assets, we will need to segment the assets pipeline stage into multiple stages. I'm considering that out-of-scope for this bugfix. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- packages/@aws-cdk/pipelines/lib/pipeline.ts | 89 +++++++-- .../integ.pipeline-with-assets.expected.json | 183 +++--------------- .../pipelines/test/pipeline-assets.test.ts | 107 ++++++++++ 3 files changed, 213 insertions(+), 166 deletions(-) diff --git a/packages/@aws-cdk/pipelines/lib/pipeline.ts b/packages/@aws-cdk/pipelines/lib/pipeline.ts index 6dd7af9e472ed..03120bf7c9866 100644 --- a/packages/@aws-cdk/pipelines/lib/pipeline.ts +++ b/packages/@aws-cdk/pipelines/lib/pipeline.ts @@ -242,10 +242,11 @@ interface AssetPublishingProps { */ class AssetPublishing extends Construct { private readonly publishers: Record = {}; + private readonly assetRoles: Record = {}; private readonly myCxAsmRoot: string; private readonly stage: codepipeline.IStage; - private assetRole?: iam.Role; + private readonly pipeline: codepipeline.Pipeline; private _fileAssetCtr = 1; private _dockerAssetCtr = 1; @@ -256,6 +257,7 @@ class AssetPublishing extends Construct { // We MUST add the Stage immediately here, otherwise it will be in the wrong place // in the pipeline! this.stage = this.props.pipeline.addStage({ stageName: 'Assets' }); + this.pipeline = this.props.pipeline; } /** @@ -269,15 +271,9 @@ class AssetPublishing extends Construct { // FIXME: this is silly, we need the relative path here but no easy way to get it const relativePath = path.relative(this.myCxAsmRoot, command.assetManifestPath); - // This role is used by both the CodePipeline build action and related CodeBuild project. Consolidating these two - // roles into one, and re-using across all assets, saves significant size of the final synthesized output. - // Modeled after the CodePipeline role and 'CodePipelineActionRole' roles. - // Late-binding here to prevent creating the role in cases where no asset actions are created. - if (!this.assetRole) { - this.assetRole = new iam.Role(this, 'Role', { - roleName: PhysicalName.GENERATE_IF_NEEDED, - assumedBy: new iam.CompositePrincipal(new iam.ServicePrincipal('codebuild.amazonaws.com'), new iam.AccountPrincipal(Stack.of(this).account)), - }); + // Late-binding here (rather than in the constructor) to prevent creating the role in cases where no asset actions are created. + if (!this.assetRoles[command.assetType]) { + this.generateAssetRole(command.assetType); } let action = this.publishers[command.assetId]; @@ -298,7 +294,7 @@ class AssetPublishing extends Construct { cloudAssemblyInput: this.props.cloudAssemblyInput, cdkCliVersion: this.props.cdkCliVersion, assetType: command.assetType, - role: this.assetRole, + role: this.assetRoles[command.assetType], }); this.stage.addAction(action); } @@ -321,9 +317,78 @@ class AssetPublishing extends Construct { } } } + + /** + * This role is used by both the CodePipeline build action and related CodeBuild project. Consolidating these two + * roles into one, and re-using across all assets, saves significant size of the final synthesized output. + * Modeled after the CodePipeline role and 'CodePipelineActionRole' roles. + * Generates one role per asset type to separate file and Docker/image-based permissions. + */ + private generateAssetRole(assetType: AssetType) { + if (this.assetRoles[assetType]) { return this.assetRoles[assetType]; } + + const rolePrefix = assetType === AssetType.DOCKER_IMAGE ? 'Docker' : 'File'; + const assetRole = new iam.Role(this, `${rolePrefix}Role`, { + roleName: PhysicalName.GENERATE_IF_NEEDED, + assumedBy: new iam.CompositePrincipal(new iam.ServicePrincipal('codebuild.amazonaws.com'), new iam.AccountPrincipal(Stack.of(this).account)), + }); + + // Logging permissions + const logGroupArn = Stack.of(this).formatArn({ + service: 'logs', + resource: 'log-group', + sep: ':', + resourceName: '/aws/codebuild/*', + }); + assetRole.addToPolicy(new iam.PolicyStatement({ + resources: [logGroupArn], + actions: ['logs:CreateLogGroup', 'logs:CreateLogStream', 'logs:PutLogEvents'], + })); + + // CodeBuild report groups + const codeBuildArn = Stack.of(this).formatArn({ + service: 'codebuild', + resource: 'report-group', + resourceName: '*', + }); + assetRole.addToPolicy(new iam.PolicyStatement({ + actions: [ + 'codebuild:CreateReportGroup', + 'codebuild:CreateReport', + 'codebuild:UpdateReport', + 'codebuild:BatchPutTestCases', + ], + resources: [codeBuildArn], + })); + + // CodeBuild start/stop + assetRole.addToPolicy(new iam.PolicyStatement({ + resources: ['*'], + actions: [ + 'codebuild:BatchGetBuilds', + 'codebuild:StartBuild', + 'codebuild:StopBuild', + ], + })); + + // Publishing role access + const rolePattern = assetType === AssetType.DOCKER_IMAGE + ? 'arn:*:iam::*:role/*-image-publishing-role-*' + : 'arn:*:iam::*:role/*-file-publishing-role-*'; + assetRole.addToPolicy(new iam.PolicyStatement({ + actions: ['sts:AssumeRole'], + resources: [rolePattern], + })); + + // Artifact access + this.pipeline.artifactBucket.grantRead(assetRole); + + this.assetRoles[assetType] = assetRole.withoutPolicyUpdates(); + return this.assetRoles[assetType]; + } } function maybeSuffix(x: string | undefined, suffix: string): string | undefined { if (x === undefined) { return undefined; } return `${x}${suffix}`; -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/pipelines/test/integ.pipeline-with-assets.expected.json b/packages/@aws-cdk/pipelines/test/integ.pipeline-with-assets.expected.json index 7795321cf451a..058eb255d030a 100644 --- a/packages/@aws-cdk/pipelines/test/integ.pipeline-with-assets.expected.json +++ b/packages/@aws-cdk/pipelines/test/integ.pipeline-with-assets.expected.json @@ -344,7 +344,7 @@ "Principal": { "AWS": { "Fn::GetAtt": [ - "PipelineAssetsRole9B011B83", + "PipelineAssetsFileRole59943A77", "Arn" ] } @@ -362,7 +362,7 @@ "Principal": { "AWS": { "Fn::GetAtt": [ - "PipelineAssetsRole9B011B83", + "PipelineAssetsFileRole59943A77", "Arn" ] } @@ -561,7 +561,7 @@ "Effect": "Allow", "Resource": { "Fn::GetAtt": [ - "PipelineAssetsRole9B011B83", + "PipelineAssetsFileRole59943A77", "Arn" ] } @@ -723,7 +723,7 @@ "Name": "FileAsset1", "RoleArn": { "Fn::GetAtt": [ - "PipelineAssetsRole9B011B83", + "PipelineAssetsFileRole59943A77", "Arn" ] }, @@ -749,7 +749,7 @@ "Name": "FileAsset2", "RoleArn": { "Fn::GetAtt": [ - "PipelineAssetsRole9B011B83", + "PipelineAssetsFileRole59943A77", "Arn" ] }, @@ -1413,7 +1413,7 @@ } } }, - "PipelineAssetsRole9B011B83": { + "PipelineAssetsFileRole59943A77": { "Type": "AWS::IAM::Role", "Properties": { "AssumeRolePolicyDocument": { @@ -1442,7 +1442,7 @@ } } }, - "PipelineAssetsRoleDefaultPolicyB41726CA": { + "PipelineAssetsFileRoleDefaultPolicy14DB8755": { "Type": "AWS::IAM::Policy", "Properties": { "PolicyDocument": { @@ -1454,39 +1454,18 @@ "logs:PutLogEvents" ], "Effect": "Allow", - "Resource": [ - { - "Fn::Join": [ - "", - [ - "arn:", - { - "Ref": "AWS::Partition" - }, - ":logs:test-region:12345678:log-group:/aws/codebuild/", - { - "Ref": "PipelineAssetsFileAsset185A67CB4" - } - ] - ] - }, - { - "Fn::Join": [ - "", - [ - "arn:", - { - "Ref": "AWS::Partition" - }, - ":logs:test-region:12345678:log-group:/aws/codebuild/", - { - "Ref": "PipelineAssetsFileAsset185A67CB4" - }, - ":*" - ] + "Resource": { + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":logs:test-region:12345678:log-group:/aws/codebuild/*" ] - } - ] + ] + } }, { "Action": [ @@ -1504,20 +1483,11 @@ { "Ref": "AWS::Partition" }, - ":codebuild:test-region:12345678:report-group/", - { - "Ref": "PipelineAssetsFileAsset185A67CB4" - }, - "-*" + ":codebuild:test-region:12345678:report-group/*" ] ] } }, - { - "Action": "sts:AssumeRole", - "Effect": "Allow", - "Resource": "arn:*:iam::*:role/*-file-publishing-role-*" - }, { "Action": [ "codebuild:BatchGetBuilds", @@ -1525,12 +1495,12 @@ "codebuild:StopBuild" ], "Effect": "Allow", - "Resource": { - "Fn::GetAtt": [ - "PipelineAssetsFileAsset185A67CB4", - "Arn" - ] - } + "Resource": "*" + }, + { + "Action": "sts:AssumeRole", + "Effect": "Allow", + "Resource": "arn:*:iam::*:role/*-file-publishing-role-*" }, { "Action": [ @@ -1574,109 +1544,14 @@ "Arn" ] } - }, - { - "Action": [ - "kms:Decrypt", - "kms:Encrypt", - "kms:ReEncrypt*", - "kms:GenerateDataKey*" - ], - "Effect": "Allow", - "Resource": { - "Fn::GetAtt": [ - "PipelineArtifactsBucketEncryptionKeyF5BF0670", - "Arn" - ] - } - }, - { - "Action": [ - "logs:CreateLogGroup", - "logs:CreateLogStream", - "logs:PutLogEvents" - ], - "Effect": "Allow", - "Resource": [ - { - "Fn::Join": [ - "", - [ - "arn:", - { - "Ref": "AWS::Partition" - }, - ":logs:test-region:12345678:log-group:/aws/codebuild/", - { - "Ref": "PipelineAssetsFileAsset24D2D639B" - } - ] - ] - }, - { - "Fn::Join": [ - "", - [ - "arn:", - { - "Ref": "AWS::Partition" - }, - ":logs:test-region:12345678:log-group:/aws/codebuild/", - { - "Ref": "PipelineAssetsFileAsset24D2D639B" - }, - ":*" - ] - ] - } - ] - }, - { - "Action": [ - "codebuild:CreateReportGroup", - "codebuild:CreateReport", - "codebuild:UpdateReport", - "codebuild:BatchPutTestCases" - ], - "Effect": "Allow", - "Resource": { - "Fn::Join": [ - "", - [ - "arn:", - { - "Ref": "AWS::Partition" - }, - ":codebuild:test-region:12345678:report-group/", - { - "Ref": "PipelineAssetsFileAsset24D2D639B" - }, - "-*" - ] - ] - } - }, - { - "Action": [ - "codebuild:BatchGetBuilds", - "codebuild:StartBuild", - "codebuild:StopBuild" - ], - "Effect": "Allow", - "Resource": { - "Fn::GetAtt": [ - "PipelineAssetsFileAsset24D2D639B", - "Arn" - ] - } } ], "Version": "2012-10-17" }, - "PolicyName": "PipelineAssetsRoleDefaultPolicyB41726CA", + "PolicyName": "PipelineAssetsFileRoleDefaultPolicy14DB8755", "Roles": [ { - "Ref": "PipelineAssetsRole9B011B83" + "Ref": "PipelineAssetsFileRole59943A77" } ] } @@ -1695,7 +1570,7 @@ }, "ServiceRole": { "Fn::GetAtt": [ - "PipelineAssetsRole9B011B83", + "PipelineAssetsFileRole59943A77", "Arn" ] }, @@ -1725,7 +1600,7 @@ }, "ServiceRole": { "Fn::GetAtt": [ - "PipelineAssetsRole9B011B83", + "PipelineAssetsFileRole59943A77", "Arn" ] }, diff --git a/packages/@aws-cdk/pipelines/test/pipeline-assets.test.ts b/packages/@aws-cdk/pipelines/test/pipeline-assets.test.ts index 084fe467413bb..41c05e322fa54 100644 --- a/packages/@aws-cdk/pipelines/test/pipeline-assets.test.ts +++ b/packages/@aws-cdk/pipelines/test/pipeline-assets.test.ts @@ -173,6 +173,60 @@ test('can control fix/CLI version used in pipeline selfupdate', () => { }); }); +describe('asset roles and policies', () => { + test('includes file publishing assets role for apps with file assets', () => { + pipeline.addApplicationStage(new FileAssetApp(app, 'App1')); + + expect(pipelineStack).toHaveResourceLike('AWS::IAM::Role', { + AssumeRolePolicyDocument: { + Statement: [{ + Action: 'sts:AssumeRole', + Effect: 'Allow', + Principal: { + Service: 'codebuild.amazonaws.com', + AWS: { 'Fn::Join': [ '', [ + 'arn:', { Ref: 'AWS::Partition' }, `:iam::${PIPELINE_ENV.account}:root`, + ] ] }, + }, + }], + }, + }); + expect(pipelineStack).toHaveResourceLike('AWS::IAM::Policy', + expectedAssetRolePolicy('arn:*:iam::*:role/*-file-publishing-role-*', 'CdkAssetsFileRole6BE17A07')); + }); + + test('includes image publishing assets role for apps with Docker assets', () => { + pipeline.addApplicationStage(new DockerAssetApp(app, 'App1')); + + expect(pipelineStack).toHaveResourceLike('AWS::IAM::Role', { + AssumeRolePolicyDocument: { + Statement: [{ + Action: 'sts:AssumeRole', + Effect: 'Allow', + Principal: { + Service: 'codebuild.amazonaws.com', + AWS: { 'Fn::Join': [ '', [ + 'arn:', { Ref: 'AWS::Partition' }, `:iam::${PIPELINE_ENV.account}:root`, + ] ] }, + }, + }], + }, + }); + expect(pipelineStack).toHaveResourceLike('AWS::IAM::Policy', + expectedAssetRolePolicy('arn:*:iam::*:role/*-image-publishing-role-*', 'CdkAssetsDockerRole484B6DD3')); + }); + + test('includes both roles for apps with both file and Docker assets', () => { + pipeline.addApplicationStage(new FileAssetApp(app, 'App1')); + pipeline.addApplicationStage(new DockerAssetApp(app, 'App2')); + + expect(pipelineStack).toHaveResourceLike('AWS::IAM::Policy', + expectedAssetRolePolicy('arn:*:iam::*:role/*-file-publishing-role-*', 'CdkAssetsFileRole6BE17A07')); + expect(pipelineStack).toHaveResourceLike('AWS::IAM::Policy', + expectedAssetRolePolicy('arn:*:iam::*:role/*-image-publishing-role-*', 'CdkAssetsDockerRole484B6DD3')); + }); +}); + class PlainStackApp extends Stage { constructor(scope: Construct, id: string, props?: StageProps) { super(scope, id, props); @@ -212,3 +266,56 @@ class DockerAssetApp extends Stage { }); } } + +function expectedAssetRolePolicy(assumeRolePattern: string, attachedRole: string) { + return { + PolicyDocument: { + Statement: [{ + Action: ['logs:CreateLogGroup', 'logs:CreateLogStream', 'logs:PutLogEvents'], + Effect: 'Allow', + Resource: { + 'Fn::Join': ['', [ + 'arn:', + {Ref: 'AWS::Partition'}, + `:logs:${PIPELINE_ENV.region}:${PIPELINE_ENV.account}:log-group:/aws/codebuild/*`, + ]], + }, + }, + { + Action: ['codebuild:CreateReportGroup', 'codebuild:CreateReport', 'codebuild:UpdateReport', 'codebuild:BatchPutTestCases'], + Effect: 'Allow', + Resource: { + 'Fn::Join': ['', [ + 'arn:', + {Ref: 'AWS::Partition'}, + `:codebuild:${PIPELINE_ENV.region}:${PIPELINE_ENV.account}:report-group/*`, + ]], + }, + }, + { + Action: ['codebuild:BatchGetBuilds', 'codebuild:StartBuild', 'codebuild:StopBuild'], + Effect: 'Allow', + Resource: '*', + }, + { + Action: 'sts:AssumeRole', + Effect: 'Allow', + Resource: assumeRolePattern, + }, + { + Action: ['s3:GetObject*', 's3:GetBucket*', 's3:List*'], + Effect: 'Allow', + Resource: [ + { 'Fn::GetAtt': ['CdkPipelineArtifactsBucket7B46C7BF', 'Arn' ] }, + { 'Fn::Join': ['', [{'Fn::GetAtt': ['CdkPipelineArtifactsBucket7B46C7BF', 'Arn']}, '/*']] }, + ], + }, + { + Action: ['kms:Decrypt', 'kms:DescribeKey'], + Effect: 'Allow', + Resource: { 'Fn::GetAtt': [ 'CdkPipelineArtifactsBucketEncryptionKeyDDD3258C', 'Arn' ]}, + }], + }, + Roles: [ {Ref: attachedRole} ], + }; +} From 419278bf88c16d5519ba63c822e4af52157e8c67 Mon Sep 17 00:00:00 2001 From: Nick Lynch Date: Thu, 30 Jul 2020 14:04:00 +0100 Subject: [PATCH 09/19] fix(certificatemanager): DNS validation for wildcard certificates (#9291) If a certificate with automatic (Route53) DNS validation contains both a base domain name and the wildcard for that domain (e.g., `example.com` and `*.example.com`), the corresponding DNS validation records are identical. This seems to have caused problems for the automated CloudFormation DNS validation. Solving the problem by removing the redundant wildcard entries from the DomainValidationOption. fixes #9248 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../aws-certificatemanager/lib/certificate.ts | 13 +- .../test/certificate.test.ts | 130 ++++++++++++++---- 2 files changed, 113 insertions(+), 30 deletions(-) diff --git a/packages/@aws-cdk/aws-certificatemanager/lib/certificate.ts b/packages/@aws-cdk/aws-certificatemanager/lib/certificate.ts index 77e6247f38198..9bd10343e1321 100644 --- a/packages/@aws-cdk/aws-certificatemanager/lib/certificate.ts +++ b/packages/@aws-cdk/aws-certificatemanager/lib/certificate.ts @@ -238,7 +238,7 @@ function renderDomainValidation(validation: CertificateValidation, domainNames: switch (validation.method) { case ValidationMethod.DNS: - for (const domainName of domainNames) { + for (const domainName of getUniqueDnsDomainNames(domainNames)) { const hostedZone = validation.props.hostedZones?.[domainName] ?? validation.props.hostedZone; if (hostedZone) { domainValidation.push({ domainName, hostedZoneId: hostedZone.hostedZoneId }); @@ -260,3 +260,14 @@ function renderDomainValidation(validation: CertificateValidation, domainNames: return domainValidation.length !== 0 ? domainValidation : undefined; } + +/** + * Removes wildcard domains (*.example.com) where the base domain (example.com) is present. + * This is because the DNS validation treats them as the same thing, and the automated CloudFormation + * DNS validation errors out with the duplicate records. + */ +function getUniqueDnsDomainNames(domainNames: string[]) { + return domainNames.filter(domain => { + return Token.isUnresolved(domain) || !domain.startsWith('*.') || !domainNames.includes(domain.replace('*.', '')); + }); +} diff --git a/packages/@aws-cdk/aws-certificatemanager/test/certificate.test.ts b/packages/@aws-cdk/aws-certificatemanager/test/certificate.test.ts index 4b122b7eae754..5bbeadf007f1e 100644 --- a/packages/@aws-cdk/aws-certificatemanager/test/certificate.test.ts +++ b/packages/@aws-cdk/aws-certificatemanager/test/certificate.test.ts @@ -121,46 +121,118 @@ test('CertificateValidation.fromEmail', () => { }); }); -test('CertificateValidation.fromDns', () => { - const stack = new Stack(); +describe('CertificateValidation.fromDns', () => { - new Certificate(stack, 'Certificate', { - domainName: 'test.example.com', - subjectAlternativeNames: ['extra.example.com'], - validation: CertificateValidation.fromDns(), - }); + test('without a hosted zone', () => { + const stack = new Stack(); - expect(stack).toHaveResource('AWS::CertificateManager::Certificate', { - DomainName: 'test.example.com', - SubjectAlternativeNames: ['extra.example.com'], - ValidationMethod: 'DNS', + new Certificate(stack, 'Certificate', { + domainName: 'test.example.com', + subjectAlternativeNames: ['extra.example.com'], + validation: CertificateValidation.fromDns(), + }); + + expect(stack).toHaveResource('AWS::CertificateManager::Certificate', { + DomainName: 'test.example.com', + SubjectAlternativeNames: ['extra.example.com'], + ValidationMethod: 'DNS', + }); }); -}); -test('CertificateValidation.fromDns with hosted zone', () => { - const stack = new Stack(); + test('with a hosted zone', () => { + const stack = new Stack(); - const exampleCom = new route53.HostedZone(stack, 'ExampleCom', { - zoneName: 'example.com', + const exampleCom = new route53.HostedZone(stack, 'ExampleCom', { + zoneName: 'example.com', + }); + + new Certificate(stack, 'Certificate', { + domainName: 'test.example.com', + validation: CertificateValidation.fromDns(exampleCom), + }); + + expect(stack).toHaveResource('AWS::CertificateManager::Certificate', { + DomainName: 'test.example.com', + DomainValidationOptions: [ + { + DomainName: 'test.example.com', + HostedZoneId: { + Ref: 'ExampleCom20E1324B', + }, + }, + ], + ValidationMethod: 'DNS', + }); }); - new Certificate(stack, 'Certificate', { - domainName: 'test.example.com', - validation: CertificateValidation.fromDns(exampleCom), + test('with hosted zone and a wildcard name', () => { + const stack = new Stack(); + + const exampleCom = new route53.HostedZone(stack, 'ExampleCom', { + zoneName: 'example.com', + }); + + new Certificate(stack, 'Certificate', { + domainName: 'test.example.com', + validation: CertificateValidation.fromDns(exampleCom), + subjectAlternativeNames: ['*.test.example.com'], + }); + + //Wildcard domain names are de-duped. + expect(stack).toHaveResource('AWS::CertificateManager::Certificate', { + DomainName: 'test.example.com', + DomainValidationOptions: [ + { + DomainName: 'test.example.com', + HostedZoneId: { + Ref: 'ExampleCom20E1324B', + }, + }, + ], + ValidationMethod: 'DNS', + }); }); - expect(stack).toHaveResource('AWS::CertificateManager::Certificate', { - DomainName: 'test.example.com', - DomainValidationOptions: [ - { - DomainName: 'test.example.com', - HostedZoneId: { - Ref: 'ExampleCom20E1324B', + test('with hosted zone and multiple wildcard names', () => { + const stack = new Stack(); + + const exampleCom = new route53.HostedZone(stack, 'ExampleCom', { + zoneName: 'example.com', + }); + + new Certificate(stack, 'Certificate', { + domainName: 'test.example.com', + validation: CertificateValidation.fromDns(exampleCom), + subjectAlternativeNames: ['*.test.example.com', '*.foo.test.example.com', 'bar.test.example.com'], + }); + + //Wildcard domain names are de-duped. + expect(stack).toHaveResource('AWS::CertificateManager::Certificate', { + DomainName: 'test.example.com', + DomainValidationOptions: [ + { + DomainName: 'test.example.com', + HostedZoneId: { + Ref: 'ExampleCom20E1324B', + }, }, - }, - ], - ValidationMethod: 'DNS', + { + DomainName: '*.foo.test.example.com', + HostedZoneId: { + Ref: 'ExampleCom20E1324B', + }, + }, + { + DomainName: 'bar.test.example.com', + HostedZoneId: { + Ref: 'ExampleCom20E1324B', + }, + }, + ], + ValidationMethod: 'DNS', + }); }); + }); test('CertificateValidation.fromDnsMultiZone', () => { From bfc13164b93bad453839a12615f1c1c213de66ca Mon Sep 17 00:00:00 2001 From: Jerry Kindall <52084730+Jerry-AWS@users.noreply.github.com> Date: Thu, 30 Jul 2020 08:19:19 -0700 Subject: [PATCH 10/19] docs: add some missing imports and update some property names in code examples in pipelines README (#9344) Ran across some of these when preparing the How-To topic for the CDK Dev Guide; backporting to the README. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- packages/@aws-cdk/pipelines/README.md | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/packages/@aws-cdk/pipelines/README.md b/packages/@aws-cdk/pipelines/README.md index 6b171a06dea98..ba5085e94e1d0 100644 --- a/packages/@aws-cdk/pipelines/README.md +++ b/packages/@aws-cdk/pipelines/README.md @@ -27,12 +27,24 @@ same, the *CDK Pipelines* library takes care of the details. will work, see the section **CDK Environment Bootstrapping** below). ```ts -import { Construct, Stage } from '@aws-cdk/core'; +/** The stacks for our app are defined in my-stacks.ts. The internals of these + * stacks aren't important, except that DatabaseStack exposes an attribute + * "table" for a database table it defines, and ComputeStack accepts a reference + * to this table in its properties. + */ +import { DatabaseStack, ComputeStack } from '../lib/my-stacks'; + +import { Construct, Stage, Stack, StackProps, StageProps } from '@aws-cdk/core'; +import { CdkPipeline } from '@aws-cdk/pipelines'; +import * as codepipeline from '@aws-cdk/aws-codepipeline'; /** * Your application * - * May consist of one or more Stacks + * May consist of one or more Stacks (here, two) + * + * By declaring our DatabaseStack and our ComputeStack inside a Stage, + * we make sure they are deployed together, or not at all. */ class MyApplication extends Stage { constructor(scope: Construct, id: string, props?: StageProps) { @@ -292,7 +304,7 @@ In its simplest form, adding validation actions looks like this: const stage = pipeline.addApplicationStage(new MyApplication(/* ... */)); stage.addActions(new ShellScriptAction({ - name: 'MyValidation', + actionName: 'MyValidation', commands: ['curl -Ssf https://my.webservice.com/'], // ... more configuration ... })); @@ -403,7 +415,7 @@ const pipeline = new CdkPipeline(this, 'Pipeline', { }); const validationAction = new ShellScriptAction({ - name: 'TestUsingBuildArtifact', + actionName: 'TestUsingBuildArtifact', additionalArtifacts: [integTestsArtifact], // 'test.js' was produced from 'test/test.ts' during the synth step commands: ['node ./test.js'], From d0b32897ef72762e045c41b988997032a02ebef7 Mon Sep 17 00:00:00 2001 From: Jerry Kindall <52084730+Jerry-AWS@users.noreply.github.com> Date: Thu, 30 Jul 2020 09:08:52 -0700 Subject: [PATCH 11/19] ShellScriptAction prop name -> actionName name is the old name for actionName --- packages/@aws-cdk/pipelines/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@aws-cdk/pipelines/README.md b/packages/@aws-cdk/pipelines/README.md index ba5085e94e1d0..ff92b365d56e0 100644 --- a/packages/@aws-cdk/pipelines/README.md +++ b/packages/@aws-cdk/pipelines/README.md @@ -376,7 +376,7 @@ const pipeline = new CdkPipeline(this, 'Pipeline', { }); const validationAction = new ShellScriptAction({ - name: 'TestUsingSourceArtifact', + actionName: 'TestUsingSourceArtifact', additionalArtifacts: [sourceArtifact], // 'test.sh' comes from the source repository From 3cf6a946d7c2d3766fbfa97ef9504af62f7e1a5b Mon Sep 17 00:00:00 2001 From: Bryan Pan Date: Thu, 30 Jul 2020 09:30:28 -0700 Subject: [PATCH 12/19] chore(scripts): allow --reset --up/--down to automatically run script (#8826) **[ISSUE]** Honestly, kind of miss being able to use `lerna` for any kind of npm test through parameters with the old `builddown`. Wanted to be able to run something like `builddown build+test` or `builddown test`. **[APPROACH]** Added a `-r | --reset` flag to allow the `foreach.sh` script to reset and run in one line like `builddown`/`buildup`. **[NOTE]** Won't change anyone's current workflow because `--reset` is still in codebase. Also arguments for the script aren't restricted by order (i.e. `foreach.sh --reset --up yarn build ` and `builddown yarn build --up --reset` will produce the same results)! ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- scripts/builddown | 6 +-- scripts/buildup | 6 +-- scripts/foreach.sh | 105 +++++++++++++++++++++++++++------------------ 3 files changed, 68 insertions(+), 49 deletions(-) diff --git a/scripts/builddown b/scripts/builddown index 181f08dbca389..55c98e0fa9140 100755 --- a/scripts/builddown +++ b/scripts/builddown @@ -11,17 +11,15 @@ echo " for advanced usage, see ${scriptdir}/foreach.sh" echo "************************************************************" if [ "$#" -eq 0 ]; then - ${scriptdir}/foreach.sh --reset + ${scriptdir}/foreach.sh --reset --down yarn build else if [ "$1" != "--resume" ]; then echo "Unknown option: $1" exit 1 fi + ${scriptdir}/foreach.sh --down yarn build fi -${scriptdir}/foreach.sh --down yarn build -${scriptdir}/foreach.sh --reset - echo "************************************************************" echo "builddown done" echo "************************************************************" diff --git a/scripts/buildup b/scripts/buildup index 3fea629af9976..e7dcd324d91ff 100755 --- a/scripts/buildup +++ b/scripts/buildup @@ -11,17 +11,15 @@ echo " for advanced usage, see ${scriptdir}/foreach.sh" echo "************************************************************" if [ "$#" -eq 0 ]; then - ${scriptdir}/foreach.sh --reset + ${scriptdir}/foreach.sh --reset --up yarn build else if [ "$1" != "--resume" ]; then echo "Unknown option: $1" exit 1 fi + ${scriptdir}/foreach.sh --up yarn build fi -${scriptdir}/foreach.sh --up yarn build -${scriptdir}/foreach.sh --reset - echo "************************************************************" echo "buildup done" echo "************************************************************" diff --git a/scripts/foreach.sh b/scripts/foreach.sh index 1a389a5d60a3a..19f8ccd8e6a02 100755 --- a/scripts/foreach.sh +++ b/scripts/foreach.sh @@ -13,13 +13,16 @@ # if a task fails, it will stop, and then to resume, simply run `foreach.sh` again (with or without the same command). # # to reset the session (either when all tasks finished or if you wish to run a different session), run: -# foreach.sh --reset +# foreach.sh [-r | --reset] +# +# to force a reset and run a session with the current, run: +# foreach.sh [-r | --reset] [-u | --up || -d | --down] COMMAND # # to run the command only against the current module and its dependencies: -# foreach.sh --up COMMAND +# foreach.sh [-u | --up] COMMAND # # to run the command only against the current module and its consumers: -# foreach.sh --down COMMAND +# foreach.sh [-d | --down] COMMAND # # -------------------------------------------------------------------------------------------------- set -euo pipefail @@ -41,52 +44,72 @@ function success { printf "\e[32;5;81m$@\e[0m\n" } -if [[ "${1:-}" == "--reset" ]]; then - rm -f "${statedir}/.foreach."* - success "state cleared. you are free to start a new command." - exit 0 +function reset { + rm -f "${statedir}/.foreach."* + success "state cleared. you are free to start a new command." +} + +DIRECTION="" +RESET=0 +SKIP=0 +command_arg="" + +for arg in "$@" +do + case "$arg" in + -r | --reset) RESET=1 ;; + -s | --skip) SKIP=1 ;; + -u | --up) DIRECTION="UP" ;; + -d | --down) DIRECTION="DOWN" ;; + *) command_arg="$command_arg$arg " ;; + esac + shift +done + +if [[ "$RESET" -eq 1 ]]; then + reset fi -if [[ "${1:-}" == "--skip" ]]; then - if [ ! -f ${statefile} ]; then - error "skip failed. no active sessions found." - exit 1 - fi - next=$(head -1 ${statefile}) - if [ -z "${next}" ]; then - error "skip failed. queue is empty. to reset:" - error " $0 --reset" - exit 1 - fi - tail -n +2 "${statefile}" > "${statefile}.tmp" - cp "${statefile}.tmp" "${statefile}" - success "directory '$next' skipped. re-run the original foreach command to resume." - exit 0 +if [[ "$RESET" -eq 1 && "$DIRECTION" == "" ]]; then + exit 0 +fi + +if [[ "$SKIP" -eq 1 ]]; then + if [ ! -f ${statefile} ]; then + error "skip failed. no active sessions found." + exit 1 + fi + next=$(head -1 ${statefile}) + if [ -z "${next}" ]; then + error "skip failed. queue is empty. to reset:" + error " $0 --reset" + exit 1 + fi + tail -n +2 "${statefile}" > "${statefile}.tmp" + cp "${statefile}.tmp" "${statefile}" + success "directory '$next' skipped. re-run the original foreach command (without --reset) to resume." + exit 0 fi direction="" direction_desc="" -if [[ "${1:-}" == "--up" || "${1:-}" == "--down" ]]; then - if [ ! -f package.json ]; then - echo "--up or --down can only be executed from within a module directory (looking for package.json)" - exit 1 - fi - - scope=$(node -p "require('./package.json').name") +if [[ "$DIRECTION" == "UP" || "$DIRECTION" == "DOWN" ]]; then + if [ ! -f package.json ]; then + error "--up or --down can only be executed from within a module directory (looking for package.json)" + exit 1 + fi - if [[ "${1:-}" == "--up" ]]; then - direction=" --scope ${scope} --include-dependencies" - direction_desc="('${scope}' and its dependencies)" - else # --down - direction=" --scope ${scope} --include-dependents" - direction_desc="('${scope}' and its consumers)" - fi + scope=$(node -p "require('./package.json').name") - shift + if [[ "$DIRECTION" == "UP" ]]; then + direction=" --scope ${scope} --include-dependencies" + direction_desc="('${scope}' and its dependencies)" + else # --down + direction=" --scope ${scope} --include-dependents" + direction_desc="('${scope}' and its consumers)" + fi fi -command_arg="${@:-}" - if [ -f "${statefile}" ] && [ -f "${commandfile}" ]; then command="$(cat ${commandfile})" if [ ! -z "${command_arg}" ] && [ "${command}" != "${command_arg}" ]; then @@ -110,8 +133,8 @@ fi next="$(head -n1 ${statefile})" if [ -z "${next}" ]; then - success "done (queue is empty). to reset:" - success " $0 --reset" + success "done (queue is empty). reseting queue:" + reset exit 0 fi From fbb04183ea77bcf630c39fa22893039865782a12 Mon Sep 17 00:00:00 2001 From: Nick Lynch Date: Thu, 30 Jul 2020 18:07:32 +0100 Subject: [PATCH 13/19] chore(cloudfront): README updates and notes of breaking changes (#9356) Making a small -- but useful -- README change as an excuse to capture the breaking changes made in #9326, but that were missed in that commit message. BREAKING CHANGE: Removed origin classes from the aws-cloudfront module. * **aws-cloudfront:** Removed S3Origin and HttpOrigin from the aws-cloudfront module. Use the S3Origin and HttpOrigin classes in the aws-cloudfront-origins module instead. * **aws-cloudfront:** Renamed Origin to OriginBase. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- packages/@aws-cdk/aws-cloudfront/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@aws-cdk/aws-cloudfront/README.md b/packages/@aws-cdk/aws-cloudfront/README.md index e0a38d797088d..44bfe03e680eb 100644 --- a/packages/@aws-cdk/aws-cloudfront/README.md +++ b/packages/@aws-cdk/aws-cloudfront/README.md @@ -36,7 +36,8 @@ for more complex use cases. CloudFront distributions deliver your content from one or more origins; an origin is the location where you store the original version of your content. Origins can be created from S3 buckets or a custom origin (HTTP server). Each distribution has a default behavior which applies to all -requests to that distribution, and routes requests to a primary origin. +requests to that distribution, and routes requests to a primary origin. Constructs to define origins are in the `@aws-cdk/aws-cloudfront-origins` +module. #### From an S3 Bucket @@ -323,7 +324,6 @@ const distribution = new CloudFrontWebDistribution(this, 'MyDistribution', { In case the origin source is not available and answers with one of the specified status code the failover origin source will be used. - ```ts new CloudFrontWebDistribution(stack, 'ADistribution', { originConfigs: [ From b46aa998ce3aedd3c5cfc5b4eef08859a6dc0d2a Mon Sep 17 00:00:00 2001 From: Bryan Pan Date: Thu, 30 Jul 2020 17:47:32 -0700 Subject: [PATCH 14/19] refactor(appsync): strongly type schema definition mode (#9283) **[ISSUE]** schema definition mode is not strongly typed. **[APPROACH]** Make input prop `schemaDefinition` take a enum that allows users to choose between 2 modes: `CODE` and `FILE`. **[NOTE]** This approach was taken in preparation for a code-first approach to schema generation with CDK. All of the functions tagged `@experimental` are subject to change. BREAKING CHANGE: **appsync** prop `schemaDefinition` no longer takes string, instead it is required to configure schema definition mode. - **appsync**: schemaDefinition takes param `SchemaDefinition.XXX` to declare how schema will be configured - **SchemaDefinition.CODE** allows schema definition through CDK - **SchemaDefinition.FILE** allows schema definition through schema.graphql file Fixes #9301 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- packages/@aws-cdk/aws-appsync/README.md | 1 + .../@aws-cdk/aws-appsync/lib/graphqlapi.ts | 75 ++++++++++--- packages/@aws-cdk/aws-appsync/package.json | 2 + .../aws-appsync/test/appsync-apikey.test.ts | 4 + .../aws-appsync/test/appsync-grant.test.ts | 1 + .../aws-appsync/test/appsync-schema.test.ts | 104 ++++++++++++++++++ .../@aws-cdk/aws-appsync/test/appsync.test.ts | 4 + .../aws-appsync/test/integ.graphql-iam.ts | 2 + .../aws-appsync/test/integ.graphql.ts | 2 + 9 files changed, 180 insertions(+), 15 deletions(-) create mode 100644 packages/@aws-cdk/aws-appsync/test/appsync-schema.test.ts diff --git a/packages/@aws-cdk/aws-appsync/README.md b/packages/@aws-cdk/aws-appsync/README.md index f9811ddd9c1be..40064e0822e02 100644 --- a/packages/@aws-cdk/aws-appsync/README.md +++ b/packages/@aws-cdk/aws-appsync/README.md @@ -47,6 +47,7 @@ import * as db from '@aws-cdk/aws-dynamodb'; const api = new appsync.GraphQLApi(stack, 'Api', { name: 'demo', + schemaDefinition: appsync.SchemaDefinition.FILE, schemaDefinitionFile: join(__dirname, 'schema.graphql'), authorizationConfig: { defaultAuthorization: { diff --git a/packages/@aws-cdk/aws-appsync/lib/graphqlapi.ts b/packages/@aws-cdk/aws-appsync/lib/graphqlapi.ts index 9678c0dd3fee9..ce9c380d3707a 100644 --- a/packages/@aws-cdk/aws-appsync/lib/graphqlapi.ts +++ b/packages/@aws-cdk/aws-appsync/lib/graphqlapi.ts @@ -202,6 +202,21 @@ export interface LogConfig { readonly fieldLogLevel?: FieldLogLevel; } +/** + * Enum containing the different modes of schema definition + */ +export enum SchemaDefinition { + /** + * Define schema through functions like addType, addQuery, etc. + */ + CODE = 'CODE', + + /** + * Define schema in a file, i.e. schema.graphql + */ + FILE = 'FILE', +} + /** * Properties for an AppSync GraphQL API */ @@ -227,11 +242,14 @@ export interface GraphQLApiProps { readonly logConfig?: LogConfig; /** - * GraphQL schema definition. You have to specify a definition or a file containing one. + * GraphQL schema definition. Specify how you want to define your schema. + * + * SchemaDefinition.CODE allows schema definition through CDK + * SchemaDefinition.FILE allows schema definition through schema.graphql file * - * @default - Use schemaDefinitionFile + * @experimental */ - readonly schemaDefinition?: string; + readonly schemaDefinition: SchemaDefinition; /** * File containing the GraphQL schema definition. You have to specify a definition or a file containing one. * @@ -332,6 +350,7 @@ export class GraphQLApi extends Construct { return this._apiKey; } + private schemaMode: SchemaDefinition; private api: CfnGraphQLApi; private _apiKey?: string; @@ -389,6 +408,7 @@ export class GraphQLApi extends Construct { this.arn = this.api.attrArn; this.graphQlUrl = this.api.attrGraphQlUrl; this.name = this.api.name; + this.schemaMode = props.schemaDefinition; if ( defaultAuthorizationType === AuthorizationType.API_KEY || @@ -404,18 +424,7 @@ export class GraphQLApi extends Construct { this._apiKey = this.createAPIKey(apiKeyConfig); } - let definition; - if (props.schemaDefinition) { - definition = props.schemaDefinition; - } else if (props.schemaDefinitionFile) { - definition = readFileSync(props.schemaDefinitionFile).toString('UTF-8'); - } else { - throw new Error('Missing Schema definition. Provide schemaDefinition or schemaDefinitionFile'); - } - this.schema = new CfnGraphQLSchema(this, 'Schema', { - apiId: this.apiId, - definition, - }); + this.schema = this.defineSchema(props.schemaDefinitionFile); } /** @@ -666,4 +675,40 @@ export class GraphQLApi extends Construct { const authModes = props.authorizationConfig?.additionalAuthorizationModes; return authModes ? this.formatAdditionalAuthorizationModes(authModes) : undefined; } + + /** + * Sets schema defintiion to input if schema mode is configured with SchemaDefinition.CODE + * + * @param definition string that is the graphql representation of schema + * @experimental temporary + */ + public updateDefinition (definition: string): void{ + if ( this.schemaMode != SchemaDefinition.CODE ) { + throw new Error('API cannot add type because schema definition mode is not configured as CODE.'); + } + this.schema.definition = definition; + } + + /** + * Define schema based on props configuration + * @param file the file name/s3 location of Schema + */ + private defineSchema(file?: string): CfnGraphQLSchema { + let definition; + + if ( this.schemaMode == SchemaDefinition.FILE && !file) { + throw new Error('schemaDefinitionFile must be configured if using FILE definition mode.'); + } else if ( this.schemaMode == SchemaDefinition.FILE && file ) { + definition = readFileSync(file).toString('UTF-8'); + } else if ( this.schemaMode == SchemaDefinition.CODE && !file ) { + definition = ''; + } else if ( this.schemaMode == SchemaDefinition.CODE && file) { + throw new Error('definition mode CODE is incompatible with file definition. Change mode to FILE/S3 or unconfigure schemaDefinitionFile'); + } + + return new CfnGraphQLSchema(this, 'Schema', { + apiId: this.apiId, + definition, + }); + } } diff --git a/packages/@aws-cdk/aws-appsync/package.json b/packages/@aws-cdk/aws-appsync/package.json index ed431e29e7367..07fa3726c0f9b 100644 --- a/packages/@aws-cdk/aws-appsync/package.json +++ b/packages/@aws-cdk/aws-appsync/package.json @@ -75,6 +75,7 @@ "@aws-cdk/aws-dynamodb": "0.0.0", "@aws-cdk/aws-iam": "0.0.0", "@aws-cdk/aws-lambda": "0.0.0", + "@aws-cdk/aws-s3-assets": "0.0.0", "@aws-cdk/core": "0.0.0", "constructs": "^3.0.2" }, @@ -84,6 +85,7 @@ "@aws-cdk/aws-dynamodb": "0.0.0", "@aws-cdk/aws-iam": "0.0.0", "@aws-cdk/aws-lambda": "0.0.0", + "@aws-cdk/aws-s3-assets": "0.0.0", "@aws-cdk/core": "0.0.0", "constructs": "^3.0.2" }, diff --git a/packages/@aws-cdk/aws-appsync/test/appsync-apikey.test.ts b/packages/@aws-cdk/aws-appsync/test/appsync-apikey.test.ts index da49317ee5a64..39a09c1f1c955 100644 --- a/packages/@aws-cdk/aws-appsync/test/appsync-apikey.test.ts +++ b/packages/@aws-cdk/aws-appsync/test/appsync-apikey.test.ts @@ -11,6 +11,7 @@ describe('AppSync Authorization Config', () => { // WHEN new appsync.GraphQLApi(stack, 'api', { name: 'api', + schemaDefinition: appsync.SchemaDefinition.FILE, schemaDefinitionFile: path.join(__dirname, 'appsync.test.graphql'), }); @@ -25,6 +26,7 @@ describe('AppSync Authorization Config', () => { // WHEN new appsync.GraphQLApi(stack, 'api', { name: 'api', + schemaDefinition: appsync.SchemaDefinition.FILE, schemaDefinitionFile: path.join(__dirname, 'appsync.test.graphql'), authorizationConfig: { defaultAuthorization: { @@ -47,6 +49,7 @@ describe('AppSync Authorization Config', () => { // WHEN new appsync.GraphQLApi(stack, 'api', { name: 'api', + schemaDefinition: appsync.SchemaDefinition.FILE, schemaDefinitionFile: path.join(__dirname, 'appsync.test.graphql'), authorizationConfig: { defaultAuthorization: { @@ -66,6 +69,7 @@ describe('AppSync Authorization Config', () => { // WHEN new appsync.GraphQLApi(stack, 'api', { name: 'api', + schemaDefinition: appsync.SchemaDefinition.FILE, schemaDefinitionFile: path.join(__dirname, 'appsync.test.graphql'), authorizationConfig: { defaultAuthorization: { diff --git a/packages/@aws-cdk/aws-appsync/test/appsync-grant.test.ts b/packages/@aws-cdk/aws-appsync/test/appsync-grant.test.ts index f770cbad26319..5f8efefcb9f89 100644 --- a/packages/@aws-cdk/aws-appsync/test/appsync-grant.test.ts +++ b/packages/@aws-cdk/aws-appsync/test/appsync-grant.test.ts @@ -15,6 +15,7 @@ beforeEach(() => { }); api = new appsync.GraphQLApi(stack, 'API', { name: 'demo', + schemaDefinition: appsync.SchemaDefinition.FILE, schemaDefinitionFile: join(__dirname, 'appsync.test.graphql'), authorizationConfig: { defaultAuthorization: { diff --git a/packages/@aws-cdk/aws-appsync/test/appsync-schema.test.ts b/packages/@aws-cdk/aws-appsync/test/appsync-schema.test.ts new file mode 100644 index 0000000000000..d031dbf77d542 --- /dev/null +++ b/packages/@aws-cdk/aws-appsync/test/appsync-schema.test.ts @@ -0,0 +1,104 @@ +import { join } from 'path'; +import '@aws-cdk/assert/jest'; +import * as cdk from '@aws-cdk/core'; +import * as appsync from '../lib'; + +// Schema Definitions +const type = 'type test {\n version: String!\n}\n\n'; +const query = 'type Query {\n getTests: [ test! ]!\n}\n\n'; +const mutation = 'type Mutation {\n addTest(version: String!): test\n}\n'; + +let stack: cdk.Stack; +beforeEach(() => { + // GIVEN + stack = new cdk.Stack(); +}); + +describe('testing schema definition mode `code`', () => { + + test('definition mode `code` produces empty schema definition', () => { + // WHEN + new appsync.GraphQLApi(stack, 'API', { + name: 'demo', + schemaDefinition: appsync.SchemaDefinition.CODE, + }); + + //THEN + expect(stack).toHaveResourceLike('AWS::AppSync::GraphQLSchema', { + Definition: '', + }); + }); + + test('definition mode `code` generates correct schema with updateDefinition', () => { + // WHEN + const api = new appsync.GraphQLApi(stack, 'API', { + name: 'demo', + schemaDefinition: appsync.SchemaDefinition.CODE, + }); + api.updateDefinition(`${type}${query}${mutation}`); + + //THEN + expect(stack).toHaveResourceLike('AWS::AppSync::GraphQLSchema', { + Definition: `${type}${query}${mutation}`, + }); + }); + + test('definition mode `code` errors when schemaDefinitionFile is configured', () => { + // WHEN + const when = () => { + new appsync.GraphQLApi(stack, 'API', { + name: 'demo', + schemaDefinition: appsync.SchemaDefinition.CODE, + schemaDefinitionFile: join(__dirname, 'appsync.test.graphql'), + }); + }; + + //THEN + expect(when).toThrowError('definition mode CODE is incompatible with file definition. Change mode to FILE/S3 or unconfigure schemaDefinitionFile'); + }); + +}); + +describe('testing schema definition mode `file`', () => { + + test('definition mode `file` produces correct output', () => { + // WHEN + new appsync.GraphQLApi(stack, 'API', { + name: 'demo', + schemaDefinition: appsync.SchemaDefinition.FILE, + schemaDefinitionFile: join(__dirname, 'appsync.test.graphql'), + }); + + //THEN + expect(stack).toHaveResourceLike('AWS::AppSync::GraphQLSchema', { + Definition: `${type}${query}${mutation}`, + }); + }); + + test('definition mode `file` errors when calling updateDefiniton function', () => { + // WHEN + const api = new appsync.GraphQLApi(stack, 'API', { + name: 'demo', + schemaDefinition: appsync.SchemaDefinition.FILE, + schemaDefinitionFile: join(__dirname, 'appsync.test.graphql'), + }); + const when = () => { api.updateDefinition('error'); }; + + //THEN + expect(when).toThrowError('API cannot add type because schema definition mode is not configured as CODE.'); + }); + + test('definition mode `file` errors when schemaDefinitionFile is not configured', () => { + // WHEN + const when = () => { + new appsync.GraphQLApi(stack, 'API', { + name: 'demo', + schemaDefinition: appsync.SchemaDefinition.FILE, + }); + }; + + //THEN + expect(when).toThrowError('schemaDefinitionFile must be configured if using FILE definition mode.'); + }); + +}); \ No newline at end of file diff --git a/packages/@aws-cdk/aws-appsync/test/appsync.test.ts b/packages/@aws-cdk/aws-appsync/test/appsync.test.ts index bc37922c85f82..70e8ef0082b89 100644 --- a/packages/@aws-cdk/aws-appsync/test/appsync.test.ts +++ b/packages/@aws-cdk/aws-appsync/test/appsync.test.ts @@ -11,6 +11,7 @@ test('should not throw an Error', () => { const when = () => { new appsync.GraphQLApi(stack, 'api', { authorizationConfig: {}, + schemaDefinition: appsync.SchemaDefinition.FILE, name: 'api', schemaDefinitionFile: path.join(__dirname, 'appsync.test.graphql'), }); @@ -28,6 +29,7 @@ test('appsync should configure pipeline when pipelineConfig has contents', () => const api = new appsync.GraphQLApi(stack, 'api', { authorizationConfig: {}, name: 'api', + schemaDefinition: appsync.SchemaDefinition.FILE, schemaDefinitionFile: path.join(__dirname, 'appsync.test.graphql'), }); @@ -53,6 +55,7 @@ test('appsync should configure resolver as unit when pipelineConfig is empty', ( const api = new appsync.GraphQLApi(stack, 'api', { authorizationConfig: {}, name: 'api', + schemaDefinition: appsync.SchemaDefinition.FILE, schemaDefinitionFile: path.join(__dirname, 'appsync.test.graphql'), }); @@ -76,6 +79,7 @@ test('appsync should configure resolver as unit when pipelineConfig is empty arr const api = new appsync.GraphQLApi(stack, 'api', { authorizationConfig: {}, name: 'api', + schemaDefinition: appsync.SchemaDefinition.FILE, schemaDefinitionFile: path.join(__dirname, 'appsync.test.graphql'), }); diff --git a/packages/@aws-cdk/aws-appsync/test/integ.graphql-iam.ts b/packages/@aws-cdk/aws-appsync/test/integ.graphql-iam.ts index 710975379030d..d55a63adcfb62 100644 --- a/packages/@aws-cdk/aws-appsync/test/integ.graphql-iam.ts +++ b/packages/@aws-cdk/aws-appsync/test/integ.graphql-iam.ts @@ -12,6 +12,7 @@ import { UserPoolDefaultAction, Values, IamResource, + SchemaDefinition, } from '../lib'; /* @@ -37,6 +38,7 @@ const userPool = new UserPool(stack, 'Pool', { const api = new GraphQLApi(stack, 'Api', { name: 'Integ_Test_IAM', + schemaDefinition: SchemaDefinition.FILE, schemaDefinitionFile: join(__dirname, 'integ.graphql-iam.graphql'), authorizationConfig: { defaultAuthorization: { diff --git a/packages/@aws-cdk/aws-appsync/test/integ.graphql.ts b/packages/@aws-cdk/aws-appsync/test/integ.graphql.ts index 7b3444cda5842..3b8564dba6865 100644 --- a/packages/@aws-cdk/aws-appsync/test/integ.graphql.ts +++ b/packages/@aws-cdk/aws-appsync/test/integ.graphql.ts @@ -10,6 +10,7 @@ import { PrimaryKey, UserPoolDefaultAction, Values, + SchemaDefinition, } from '../lib'; /* @@ -35,6 +36,7 @@ const userPool = new UserPool(stack, 'Pool', { const api = new GraphQLApi(stack, 'Api', { name: 'demoapi', + schemaDefinition: SchemaDefinition.FILE, schemaDefinitionFile: join(__dirname, 'integ.graphql.graphql'), authorizationConfig: { defaultAuthorization: { From 8ed9a7fb402fe11d2da3cd1286a3609aa07af73c Mon Sep 17 00:00:00 2001 From: Somaya Date: Fri, 31 Jul 2020 12:40:36 -0700 Subject: [PATCH 15/19] update mergify config with new team members (#9234) --- .mergify.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.mergify.yml b/.mergify.yml index 8e19f26cd586a..bc7c0a843d100 100644 --- a/.mergify.yml +++ b/.mergify.yml @@ -6,7 +6,7 @@ pull_request_rules: label: add: [ contribution/core ] conditions: - - author~=^(eladb|RomainMuller|garnaat|nija-at|shivlaks|skinny85|rix0rrr|NGL321|Jerry-AWS|SomayaB|MrArnoldPalmer|NetaNir|iliapolo|njlynch)$ + - author~=^(eladb|RomainMuller|garnaat|nija-at|shivlaks|skinny85|rix0rrr|NGL321|Jerry-AWS|SomayaB|MrArnoldPalmer|NetaNir|iliapolo|njlynch|ericzbeard|ccfife|fulghum|pkandasamy91|SoManyHs|uttarasridhar|BryanPan342|comcalvi|kaizen3031593|)$ - -label~="contribution/core" - name: automatic merge actions: From 90b901479b2c3d444da1c89d8cec83eca9af4756 Mon Sep 17 00:00:00 2001 From: comcalvi <66279577+comcalvi@users.noreply.github.com> Date: Fri, 31 Jul 2020 16:51:38 -0400 Subject: [PATCH 16/19] docs(cfn-include): fixed CfnOutput comment docs (#9373) ---- Closes #9368 *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- packages/@aws-cdk/core/lib/cfn-output.ts | 36 +++++++++++------------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/packages/@aws-cdk/core/lib/cfn-output.ts b/packages/@aws-cdk/core/lib/cfn-output.ts index 387c5216ead91..3f2784f890073 100644 --- a/packages/@aws-cdk/core/lib/cfn-output.ts +++ b/packages/@aws-cdk/core/lib/cfn-output.ts @@ -60,61 +60,57 @@ export class CfnOutput extends CfnElement { } /** - * Returns the description of this Output + * A String type that describes the output value. + * The description can be a maximum of 4 K in length. + * + * @default - No description. */ public get description() { return this._description; } - /** - * Sets this output's description to the parameter - * @param description the description to update this Output's description to - */ public set description(description: string | undefined) { this._description = description; } /** - * Returns the value of this Output + * The value of the property returned by the aws cloudformation describe-stacks command. + * The value of an output can include literals, parameter references, pseudo-parameters, + * a mapping value, or intrinsic functions. */ public get value() { return this._value; } - /** - * Sets this output's value to the parameter - * @param value the value to update this Output's value to - */ public set value(value: any) { this._value = value; } /** - * Returns the condition of this Output + * A condition to associate with this output value. If the condition evaluates + * to `false`, this output value will not be included in the stack. + * + * @default - No condition is associated with the output. */ public get condition() { return this._condition; } - /** - * Sets this output's condition to the parameter - * @param condition the condition to update this Output's condition to - */ public set condition(condition: CfnCondition | undefined) { this._condition = condition; } /** - * Returns the export of this Output + * The name used to export the value of this output across stacks. + * + * To import the value from another stack, use `Fn.importValue(exportName)`. + * + * @default - the output is not exported */ public get exportName() { return this._exportName; } - /** - * Sets this output's export to the parameter - * @param exportName the export to update this Output's export to - */ public set exportName(exportName: string | undefined) { this._exportName = exportName; } From 975efbed5518c4b4b982038ee6a4ad79fe11935d Mon Sep 17 00:00:00 2001 From: Nick Lynch Date: Fri, 31 Jul 2020 22:34:44 +0100 Subject: [PATCH 17/19] chore(cloudfront-origins): adding integ tests for HTTP and LB origins (#9359) Expanding integ test coverage by ensuring there is one integ test per origin type. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../aws-cloudfront-origins/package.json | 1 + .../test/integ.http-origin.expected.json | 28 ++ .../test/integ.http-origin.ts | 13 + .../integ.load-balancer-origin.expected.json | 433 ++++++++++++++++++ .../test/integ.load-balancer-origin.ts | 17 + 5 files changed, 492 insertions(+) create mode 100644 packages/@aws-cdk/aws-cloudfront-origins/test/integ.http-origin.expected.json create mode 100644 packages/@aws-cdk/aws-cloudfront-origins/test/integ.http-origin.ts create mode 100644 packages/@aws-cdk/aws-cloudfront-origins/test/integ.load-balancer-origin.expected.json create mode 100644 packages/@aws-cdk/aws-cloudfront-origins/test/integ.load-balancer-origin.ts diff --git a/packages/@aws-cdk/aws-cloudfront-origins/package.json b/packages/@aws-cdk/aws-cloudfront-origins/package.json index df97c4987294f..c67cc58d43705 100644 --- a/packages/@aws-cdk/aws-cloudfront-origins/package.json +++ b/packages/@aws-cdk/aws-cloudfront-origins/package.json @@ -63,6 +63,7 @@ "license": "Apache-2.0", "devDependencies": { "@aws-cdk/assert": "0.0.0", + "@aws-cdk/aws-ec2": "0.0.0", "aws-sdk": "^2.715.0", "cdk-build-tools": "0.0.0", "cdk-integ-tools": "0.0.0", diff --git a/packages/@aws-cdk/aws-cloudfront-origins/test/integ.http-origin.expected.json b/packages/@aws-cdk/aws-cloudfront-origins/test/integ.http-origin.expected.json new file mode 100644 index 0000000000000..c5f7da26599c6 --- /dev/null +++ b/packages/@aws-cdk/aws-cloudfront-origins/test/integ.http-origin.expected.json @@ -0,0 +1,28 @@ +{ + "Resources": { + "DistributionCFDistribution882A7313": { + "Type": "AWS::CloudFront::Distribution", + "Properties": { + "DistributionConfig": { + "DefaultCacheBehavior": { + "ForwardedValues": { + "QueryString": false + }, + "TargetOriginId": "cloudfronthttporiginDistributionOrigin162B02709", + "ViewerProtocolPolicy": "allow-all" + }, + "Enabled": true, + "Origins": [ + { + "CustomOriginConfig": { + "OriginProtocolPolicy": "https-only" + }, + "DomainName": "www.example.com", + "Id": "cloudfronthttporiginDistributionOrigin162B02709" + } + ] + } + } + } + } +} diff --git a/packages/@aws-cdk/aws-cloudfront-origins/test/integ.http-origin.ts b/packages/@aws-cdk/aws-cloudfront-origins/test/integ.http-origin.ts new file mode 100644 index 0000000000000..8806ca3aafdc4 --- /dev/null +++ b/packages/@aws-cdk/aws-cloudfront-origins/test/integ.http-origin.ts @@ -0,0 +1,13 @@ +import * as cloudfront from '@aws-cdk/aws-cloudfront'; +import * as cdk from '@aws-cdk/core'; +import * as origins from '../lib'; + +const app = new cdk.App(); + +const stack = new cdk.Stack(app, 'cloudfront-http-origin'); + +new cloudfront.Distribution(stack, 'Distribution', { + defaultBehavior: { origin: new origins.HttpOrigin('www.example.com') }, +}); + +app.synth(); diff --git a/packages/@aws-cdk/aws-cloudfront-origins/test/integ.load-balancer-origin.expected.json b/packages/@aws-cdk/aws-cloudfront-origins/test/integ.load-balancer-origin.expected.json new file mode 100644 index 0000000000000..8c97abc155066 --- /dev/null +++ b/packages/@aws-cdk/aws-cloudfront-origins/test/integ.load-balancer-origin.expected.json @@ -0,0 +1,433 @@ +{ + "Resources": { + "Vpc8378EB38": { + "Type": "AWS::EC2::VPC", + "Properties": { + "CidrBlock": "10.0.0.0/16", + "EnableDnsHostnames": true, + "EnableDnsSupport": true, + "InstanceTenancy": "default", + "Tags": [ + { + "Key": "Name", + "Value": "cloudfront-load-balancer-origin/Vpc" + } + ] + } + }, + "VpcPublicSubnet1Subnet5C2D37C4": { + "Type": "AWS::EC2::Subnet", + "Properties": { + "CidrBlock": "10.0.0.0/18", + "VpcId": { + "Ref": "Vpc8378EB38" + }, + "AvailabilityZone": "test-region-1a", + "MapPublicIpOnLaunch": true, + "Tags": [ + { + "Key": "aws-cdk:subnet-name", + "Value": "Public" + }, + { + "Key": "aws-cdk:subnet-type", + "Value": "Public" + }, + { + "Key": "Name", + "Value": "cloudfront-load-balancer-origin/Vpc/PublicSubnet1" + } + ] + } + }, + "VpcPublicSubnet1RouteTable6C95E38E": { + "Type": "AWS::EC2::RouteTable", + "Properties": { + "VpcId": { + "Ref": "Vpc8378EB38" + }, + "Tags": [ + { + "Key": "Name", + "Value": "cloudfront-load-balancer-origin/Vpc/PublicSubnet1" + } + ] + } + }, + "VpcPublicSubnet1RouteTableAssociation97140677": { + "Type": "AWS::EC2::SubnetRouteTableAssociation", + "Properties": { + "RouteTableId": { + "Ref": "VpcPublicSubnet1RouteTable6C95E38E" + }, + "SubnetId": { + "Ref": "VpcPublicSubnet1Subnet5C2D37C4" + } + } + }, + "VpcPublicSubnet1DefaultRoute3DA9E72A": { + "Type": "AWS::EC2::Route", + "Properties": { + "RouteTableId": { + "Ref": "VpcPublicSubnet1RouteTable6C95E38E" + }, + "DestinationCidrBlock": "0.0.0.0/0", + "GatewayId": { + "Ref": "VpcIGWD7BA715C" + } + }, + "DependsOn": [ + "VpcVPCGWBF912B6E" + ] + }, + "VpcPublicSubnet1EIPD7E02669": { + "Type": "AWS::EC2::EIP", + "Properties": { + "Domain": "vpc", + "Tags": [ + { + "Key": "Name", + "Value": "cloudfront-load-balancer-origin/Vpc/PublicSubnet1" + } + ] + } + }, + "VpcPublicSubnet1NATGateway4D7517AA": { + "Type": "AWS::EC2::NatGateway", + "Properties": { + "AllocationId": { + "Fn::GetAtt": [ + "VpcPublicSubnet1EIPD7E02669", + "AllocationId" + ] + }, + "SubnetId": { + "Ref": "VpcPublicSubnet1Subnet5C2D37C4" + }, + "Tags": [ + { + "Key": "Name", + "Value": "cloudfront-load-balancer-origin/Vpc/PublicSubnet1" + } + ] + } + }, + "VpcPublicSubnet2Subnet691E08A3": { + "Type": "AWS::EC2::Subnet", + "Properties": { + "CidrBlock": "10.0.64.0/18", + "VpcId": { + "Ref": "Vpc8378EB38" + }, + "AvailabilityZone": "test-region-1b", + "MapPublicIpOnLaunch": true, + "Tags": [ + { + "Key": "aws-cdk:subnet-name", + "Value": "Public" + }, + { + "Key": "aws-cdk:subnet-type", + "Value": "Public" + }, + { + "Key": "Name", + "Value": "cloudfront-load-balancer-origin/Vpc/PublicSubnet2" + } + ] + } + }, + "VpcPublicSubnet2RouteTable94F7E489": { + "Type": "AWS::EC2::RouteTable", + "Properties": { + "VpcId": { + "Ref": "Vpc8378EB38" + }, + "Tags": [ + { + "Key": "Name", + "Value": "cloudfront-load-balancer-origin/Vpc/PublicSubnet2" + } + ] + } + }, + "VpcPublicSubnet2RouteTableAssociationDD5762D8": { + "Type": "AWS::EC2::SubnetRouteTableAssociation", + "Properties": { + "RouteTableId": { + "Ref": "VpcPublicSubnet2RouteTable94F7E489" + }, + "SubnetId": { + "Ref": "VpcPublicSubnet2Subnet691E08A3" + } + } + }, + "VpcPublicSubnet2DefaultRoute97F91067": { + "Type": "AWS::EC2::Route", + "Properties": { + "RouteTableId": { + "Ref": "VpcPublicSubnet2RouteTable94F7E489" + }, + "DestinationCidrBlock": "0.0.0.0/0", + "GatewayId": { + "Ref": "VpcIGWD7BA715C" + } + }, + "DependsOn": [ + "VpcVPCGWBF912B6E" + ] + }, + "VpcPublicSubnet2EIP3C605A87": { + "Type": "AWS::EC2::EIP", + "Properties": { + "Domain": "vpc", + "Tags": [ + { + "Key": "Name", + "Value": "cloudfront-load-balancer-origin/Vpc/PublicSubnet2" + } + ] + } + }, + "VpcPublicSubnet2NATGateway9182C01D": { + "Type": "AWS::EC2::NatGateway", + "Properties": { + "AllocationId": { + "Fn::GetAtt": [ + "VpcPublicSubnet2EIP3C605A87", + "AllocationId" + ] + }, + "SubnetId": { + "Ref": "VpcPublicSubnet2Subnet691E08A3" + }, + "Tags": [ + { + "Key": "Name", + "Value": "cloudfront-load-balancer-origin/Vpc/PublicSubnet2" + } + ] + } + }, + "VpcPrivateSubnet1Subnet536B997A": { + "Type": "AWS::EC2::Subnet", + "Properties": { + "CidrBlock": "10.0.128.0/18", + "VpcId": { + "Ref": "Vpc8378EB38" + }, + "AvailabilityZone": "test-region-1a", + "MapPublicIpOnLaunch": false, + "Tags": [ + { + "Key": "aws-cdk:subnet-name", + "Value": "Private" + }, + { + "Key": "aws-cdk:subnet-type", + "Value": "Private" + }, + { + "Key": "Name", + "Value": "cloudfront-load-balancer-origin/Vpc/PrivateSubnet1" + } + ] + } + }, + "VpcPrivateSubnet1RouteTableB2C5B500": { + "Type": "AWS::EC2::RouteTable", + "Properties": { + "VpcId": { + "Ref": "Vpc8378EB38" + }, + "Tags": [ + { + "Key": "Name", + "Value": "cloudfront-load-balancer-origin/Vpc/PrivateSubnet1" + } + ] + } + }, + "VpcPrivateSubnet1RouteTableAssociation70C59FA6": { + "Type": "AWS::EC2::SubnetRouteTableAssociation", + "Properties": { + "RouteTableId": { + "Ref": "VpcPrivateSubnet1RouteTableB2C5B500" + }, + "SubnetId": { + "Ref": "VpcPrivateSubnet1Subnet536B997A" + } + } + }, + "VpcPrivateSubnet1DefaultRouteBE02A9ED": { + "Type": "AWS::EC2::Route", + "Properties": { + "RouteTableId": { + "Ref": "VpcPrivateSubnet1RouteTableB2C5B500" + }, + "DestinationCidrBlock": "0.0.0.0/0", + "NatGatewayId": { + "Ref": "VpcPublicSubnet1NATGateway4D7517AA" + } + } + }, + "VpcPrivateSubnet2Subnet3788AAA1": { + "Type": "AWS::EC2::Subnet", + "Properties": { + "CidrBlock": "10.0.192.0/18", + "VpcId": { + "Ref": "Vpc8378EB38" + }, + "AvailabilityZone": "test-region-1b", + "MapPublicIpOnLaunch": false, + "Tags": [ + { + "Key": "aws-cdk:subnet-name", + "Value": "Private" + }, + { + "Key": "aws-cdk:subnet-type", + "Value": "Private" + }, + { + "Key": "Name", + "Value": "cloudfront-load-balancer-origin/Vpc/PrivateSubnet2" + } + ] + } + }, + "VpcPrivateSubnet2RouteTableA678073B": { + "Type": "AWS::EC2::RouteTable", + "Properties": { + "VpcId": { + "Ref": "Vpc8378EB38" + }, + "Tags": [ + { + "Key": "Name", + "Value": "cloudfront-load-balancer-origin/Vpc/PrivateSubnet2" + } + ] + } + }, + "VpcPrivateSubnet2RouteTableAssociationA89CAD56": { + "Type": "AWS::EC2::SubnetRouteTableAssociation", + "Properties": { + "RouteTableId": { + "Ref": "VpcPrivateSubnet2RouteTableA678073B" + }, + "SubnetId": { + "Ref": "VpcPrivateSubnet2Subnet3788AAA1" + } + } + }, + "VpcPrivateSubnet2DefaultRoute060D2087": { + "Type": "AWS::EC2::Route", + "Properties": { + "RouteTableId": { + "Ref": "VpcPrivateSubnet2RouteTableA678073B" + }, + "DestinationCidrBlock": "0.0.0.0/0", + "NatGatewayId": { + "Ref": "VpcPublicSubnet2NATGateway9182C01D" + } + } + }, + "VpcIGWD7BA715C": { + "Type": "AWS::EC2::InternetGateway", + "Properties": { + "Tags": [ + { + "Key": "Name", + "Value": "cloudfront-load-balancer-origin/Vpc" + } + ] + } + }, + "VpcVPCGWBF912B6E": { + "Type": "AWS::EC2::VPCGatewayAttachment", + "Properties": { + "VpcId": { + "Ref": "Vpc8378EB38" + }, + "InternetGatewayId": { + "Ref": "VpcIGWD7BA715C" + } + } + }, + "LB8A12904C": { + "Type": "AWS::ElasticLoadBalancingV2::LoadBalancer", + "Properties": { + "Scheme": "internet-facing", + "SecurityGroups": [ + { + "Fn::GetAtt": [ + "LBSecurityGroup8A41EA2B", + "GroupId" + ] + } + ], + "Subnets": [ + { + "Ref": "VpcPublicSubnet1Subnet5C2D37C4" + }, + { + "Ref": "VpcPublicSubnet2Subnet691E08A3" + } + ], + "Type": "application" + }, + "DependsOn": [ + "VpcPublicSubnet1DefaultRoute3DA9E72A", + "VpcPublicSubnet2DefaultRoute97F91067" + ] + }, + "LBSecurityGroup8A41EA2B": { + "Type": "AWS::EC2::SecurityGroup", + "Properties": { + "GroupDescription": "Automatically created Security Group for ELB cloudfrontloadbalanceroriginLB8CFBA9DF", + "SecurityGroupEgress": [ + { + "CidrIp": "255.255.255.255/32", + "Description": "Disallow all traffic", + "FromPort": 252, + "IpProtocol": "icmp", + "ToPort": 86 + } + ], + "VpcId": { + "Ref": "Vpc8378EB38" + } + } + }, + "DistributionCFDistribution882A7313": { + "Type": "AWS::CloudFront::Distribution", + "Properties": { + "DistributionConfig": { + "DefaultCacheBehavior": { + "ForwardedValues": { + "QueryString": false + }, + "TargetOriginId": "cloudfrontloadbalanceroriginDistributionOrigin1BCC75186", + "ViewerProtocolPolicy": "allow-all" + }, + "Enabled": true, + "Origins": [ + { + "CustomOriginConfig": { + "OriginProtocolPolicy": "https-only" + }, + "DomainName": { + "Fn::GetAtt": [ + "LB8A12904C", + "DNSName" + ] + }, + "Id": "cloudfrontloadbalanceroriginDistributionOrigin1BCC75186" + } + ] + } + } + } + } +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-cloudfront-origins/test/integ.load-balancer-origin.ts b/packages/@aws-cdk/aws-cloudfront-origins/test/integ.load-balancer-origin.ts new file mode 100644 index 0000000000000..85685500993ae --- /dev/null +++ b/packages/@aws-cdk/aws-cloudfront-origins/test/integ.load-balancer-origin.ts @@ -0,0 +1,17 @@ +import * as cloudfront from '@aws-cdk/aws-cloudfront'; +import * as ec2 from '@aws-cdk/aws-ec2'; +import * as elbv2 from '@aws-cdk/aws-elasticloadbalancingv2'; +import * as cdk from '@aws-cdk/core'; +import * as origins from '../lib'; + +const app = new cdk.App(); +const stack = new cdk.Stack(app, 'cloudfront-load-balancer-origin'); + +const vpc = new ec2.Vpc(stack, 'Vpc', { maxAzs: 2 }); +const loadbalancer = new elbv2.ApplicationLoadBalancer(stack, 'LB', { vpc, internetFacing: true }); + +new cloudfront.Distribution(stack, 'Distribution', { + defaultBehavior: { origin: new origins.LoadBalancerV2Origin(loadbalancer) }, +}); + +app.synth(); From 860966a3945e1b667f92b19b49b92f7a1b1b8b33 Mon Sep 17 00:00:00 2001 From: comcalvi <66279577+comcalvi@users.noreply.github.com> Date: Fri, 31 Jul 2020 18:11:19 -0400 Subject: [PATCH 18/19] feat(core): make the CfnParameter class mutable (#9365) Closes #9364 ---- 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 | 18 ++ .../test/valid-templates.test.ts | 46 +++++ packages/@aws-cdk/core/lib/cfn-parameter.ts | 186 ++++++++++++++++-- 3 files changed, 236 insertions(+), 14 deletions(-) diff --git a/packages/@aws-cdk/cloudformation-include/README.md b/packages/@aws-cdk/cloudformation-include/README.md index 0e08b7f5fb745..535846de608fb 100644 --- a/packages/@aws-cdk/cloudformation-include/README.md +++ b/packages/@aws-cdk/cloudformation-include/README.md @@ -110,6 +110,24 @@ Note that [Custom Resources](https://docs.aws.amazon.com/AWSCloudFormation/lates will be of type CfnResource, and hence won't need to be casted. This holds for any resource that isn't in the CloudFormation schema. +## Parameters + +If your template uses [CloudFormation Parameters] (https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/parameters-section-structure.html), +you can retrieve them from your template: + +```typescript +import * as core from '@aws-cdk/core'; + +const param: core.CfnParameter = cfnTemplate.getParameter('MyParameter'); +``` + +The `CfnParameter` object is mutable, +and any changes you make to it will be reflected in the resulting template: + +```typescript +param.default = 'MyDefault'; +``` + ## Conditions If your template uses [CloudFormation Conditions](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/conditions-section-structure.html), 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 07529248be53b..329cda9e759c8 100644 --- a/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts +++ b/packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts @@ -397,6 +397,52 @@ describe('CDK Include', () => { }).toThrow(/Parameter with name 'FakeBucketNameThatDoesNotExist' was not found in the template/); }); + test('reflects changes to a retrieved CfnParameter object in the resulting template', () => { + const cfnTemplate = includeTestTemplate(stack, 'bucket-with-parameters.json'); + const stringParam = cfnTemplate.getParameter('BucketName'); + const numberParam = cfnTemplate.getParameter('CorsMaxAge'); + + stringParam.default = 'MyDefault'; + stringParam.allowedPattern = '[0-9]*$'; + stringParam.allowedValues = ['123123', '456789']; + stringParam.constraintDescription = 'MyNewConstraint'; + stringParam.description = 'a string of numeric characters'; + stringParam.maxLength = 6; + stringParam.minLength = 2; + + numberParam.maxValue = 100; + numberParam.minValue = 4; + numberParam.noEcho = false; + numberParam.type = "NewType"; + const originalTemplate = loadTestFileToJsObject('bucket-with-parameters.json'); + + expect(stack).toMatchTemplate({ + "Resources": { + ...originalTemplate.Resources, + }, + "Parameters": { + ...originalTemplate.Parameters, + "BucketName": { + ...originalTemplate.Parameters.BucketName, + "Default": "MyDefault", + "AllowedPattern": "[0-9]*$", + "AllowedValues": [ "123123", "456789" ], + "ConstraintDescription": "MyNewConstraint", + "Description": "a string of numeric characters", + "MaxLength": 6, + "MinLength": 2, + }, + "CorsMaxAge": { + ...originalTemplate.Parameters.CorsMaxAge, + "MaxValue": 100, + "MinValue": 4, + "NoEcho": false, + "Type": "NewType", + }, + }, + }); + }); + test('reflects changes to a retrieved CfnCondition object in the resulting template', () => { const cfnTemplate = includeTestTemplate(stack, 'resource-attribute-condition.json'); const alwaysFalseCondition = cfnTemplate.getCondition('AlwaysFalseCond'); diff --git a/packages/@aws-cdk/core/lib/cfn-parameter.ts b/packages/@aws-cdk/core/lib/cfn-parameter.ts index 8d91f7c006777..6226f451c2fb8 100644 --- a/packages/@aws-cdk/core/lib/cfn-parameter.ts +++ b/packages/@aws-cdk/core/lib/cfn-parameter.ts @@ -97,7 +97,17 @@ export interface CfnParameterProps { * update a stack. */ export class CfnParameter extends CfnElement { - private readonly type: string; + private _type: string; + private _default?: any; + private _allowedPattern?: string; + private _allowedValues?: string[]; + private _constraintDescription?: string; + private _description?: string; + private _maxLength?: number; + private _maxValue?: number; + private _minLength?: number; + private _minValue?: number; + private _noEcho?: boolean; /** * Creates a parameter construct. @@ -107,17 +117,165 @@ export class CfnParameter extends CfnElement { * @param scope The parent construct. * @param props The parameter properties. */ - constructor(scope: Construct, id: string, private readonly props: CfnParameterProps = {}) { + constructor(scope: Construct, id: string, props: CfnParameterProps = {}) { super(scope, id); - this.type = props.type || 'String'; + this._type = props.type || 'String'; + this._default = props.default; + this._allowedPattern = props.allowedPattern; + this._allowedValues = props.allowedValues; + this._constraintDescription = props.constraintDescription; + this._description = props.description; + this._maxLength = props.maxLength; + this._maxValue = props.maxValue; + this._minLength = props.minLength; + this._minValue = props.minValue; + this._noEcho = props.noEcho; + } + + /** + * The data type for the parameter (DataType). + * + * @default String + */ + public get type(): string { + return this._type; + } + + public set type(type: string) { + this._type = type; + } + + /** + * A value of the appropriate type for the template to use if no value is specified + * when a stack is created. If you define constraints for the parameter, you must specify + * a value that adheres to those constraints. + * + * @default - No default value for parameter. + */ + public get default(): any { + return this._default; + } + + public set default(value: any) { + this._default = value; + } + + /** + * A regular expression that represents the patterns to allow for String types. + * + * @default - No constraints on patterns allowed for parameter. + */ + public get allowedPattern(): string | undefined { + return this._allowedPattern; + } + + public set allowedPattern(pattern: string | undefined) { + this._allowedPattern = pattern; + } + + /** + * An array containing the list of values allowed for the parameter. + * + * @default - No constraints on values allowed for parameter. + */ + public get allowedValues(): string[] | undefined { + return this._allowedValues; + } + + public set allowedValues(values: string[] | undefined) { + this._allowedValues = values; + } + + /** + * A string that explains a constraint when the constraint is violated. + * For example, without a constraint description, a parameter that has an allowed + * pattern of [A-Za-z0-9]+ displays the following error message when the user specifies + * an invalid value: + * + * @default - No description with customized error message when user specifies invalid values. + */ + public get constraintDescription(): string | undefined { + return this._constraintDescription; + } + + public set constraintDescription(desc: string | undefined) { + this._constraintDescription = desc; + } + + /** + * A string of up to 4000 characters that describes the parameter. + * + * @default - No description for the parameter. + */ + public get description(): string | undefined { + return this._description; + } + + public set description(desc: string | undefined) { + this._description = desc; + } + + /** + * An integer value that determines the largest number of characters you want to allow for String types. + * + * @default - None. + */ + public get maxLength(): number | undefined { + return this._maxLength; + } + + public set maxLength(len: number | undefined) { + this._maxLength = len; + } + + /** + * An integer value that determines the smallest number of characters you want to allow for String types. + * + * @default - None. + */ + public get minLength(): number | undefined { + return this._minLength; + } + + public set minLength(len: number | undefined) { + this._minLength = len; + } + + /** + * A numeric value that determines the largest numeric value you want to allow for Number types. + * + * @default - None. + */ + public get maxValue(): number | undefined { + return this._maxValue; + } + + public set maxValue(len: number | undefined) { + this._maxValue = len; + } + /** + * A numeric value that determines the smallest numeric value you want to allow for Number types. + * + * @default - None. + */ + public get minValue(): number | undefined { + return this._minValue; + } + + public set minValue(len: number | undefined) { + this._minValue = len; } /** * Indicates if this parameter is configured with "NoEcho" enabled. */ public get noEcho(): boolean { - return !!this.props.noEcho; + return !!this._noEcho; + } + + public set noEcho(echo: boolean) { + this._noEcho = echo; } /** @@ -165,16 +323,16 @@ export class CfnParameter extends CfnElement { Parameters: { [this.logicalId]: { Type: this.type, - Default: this.props.default, - AllowedPattern: this.props.allowedPattern, - AllowedValues: this.props.allowedValues, - ConstraintDescription: this.props.constraintDescription, - Description: this.props.description, - MaxLength: this.props.maxLength, - MaxValue: this.props.maxValue, - MinLength: this.props.minLength, - MinValue: this.props.minValue, - NoEcho: this.props.noEcho, + Default: this.default, + AllowedPattern: this.allowedPattern, + AllowedValues: this.allowedValues, + ConstraintDescription: this.constraintDescription, + Description: this.description, + MaxLength: this.maxLength, + MaxValue: this.maxValue, + MinLength: this.minLength, + MinValue: this.minValue, + NoEcho: this._noEcho, }, }, }; From 2b00bf56f43110723b6c525605f116e6ef924b1f Mon Sep 17 00:00:00 2001 From: AWS CDK Team Date: Fri, 31 Jul 2020 22:45:24 +0000 Subject: [PATCH 19/19] chore(release): 1.56.0 --- CHANGELOG.md | 30 ++++++++++++++++++++++++++++++ lerna.json | 2 +- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c6a19b6edba20..6b54895aee553 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,36 @@ All notable changes to this project will be documented in this file. See [standard-version](https://github.com/conventional-changelog/standard-version) for commit guidelines. +## [1.56.0](https://github.com/aws/aws-cdk/compare/v1.55.0...v1.56.0) (2020-07-31) + + +### ⚠ BREAKING CHANGES + +* **appsync:** **appsync** prop `schemaDefinition` no longer takes string, instead it is required to configure schema definition mode. +- **appsync**: schemaDefinition takes param `SchemaDefinition.XXX` to declare how schema will be configured + - **SchemaDefinition.CODE** allows schema definition through CDK + - **SchemaDefinition.FILE** allows schema definition through schema.graphql file +* **cloudfront:** Removed origin classes from the aws-cloudfront module. +* **aws-cloudfront:** Removed S3Origin and HttpOrigin from the aws-cloudfront module. Use the S3Origin and HttpOrigin classes in the aws-cloudfront-origins module instead. +* **aws-cloudfront:** Renamed Origin to OriginBase. +* **cloudfront:** the property Origin.domainName has been removed + +### Features + +* **cfn-include:** add support for the Fn::Sub function ([#9275](https://github.com/aws/aws-cdk/issues/9275)) ([2a48495](https://github.com/aws/aws-cdk/commit/2a48495093dc33d88554aaa0a033338e798f9d5f)) +* **core:** make the CfnParameter class mutable ([#9365](https://github.com/aws/aws-cdk/issues/9365)) ([860966a](https://github.com/aws/aws-cdk/commit/860966a3945e1b667f92b19b49b92f7a1b1b8b33)), closes [#9364](https://github.com/aws/aws-cdk/issues/9364) + + +### Bug Fixes + +* **certificatemanager:** DNS validation for wildcard certificates ([#9291](https://github.com/aws/aws-cdk/issues/9291)) ([419278b](https://github.com/aws/aws-cdk/commit/419278bf88c16d5519ba63c822e4af52157e8c67)), closes [#9248](https://github.com/aws/aws-cdk/issues/9248) +* **pipelines:** reduce assets IAM policy size ([#9333](https://github.com/aws/aws-cdk/issues/9333)) ([88d563f](https://github.com/aws/aws-cdk/commit/88d563fab6971c20754d76c5d2eb9f4463aeae6c)), closes [#9316](https://github.com/aws/aws-cdk/issues/9316) + + +* **appsync:** strongly type schema definition mode ([#9283](https://github.com/aws/aws-cdk/issues/9283)) ([b46aa99](https://github.com/aws/aws-cdk/commit/b46aa998ce3aedd3c5cfc5b4eef08859a6dc0d2a)), closes [#9301](https://github.com/aws/aws-cdk/issues/9301) +* **cloudfront:** README updates and notes of breaking changes ([#9356](https://github.com/aws/aws-cdk/issues/9356)) ([fbb0418](https://github.com/aws/aws-cdk/commit/fbb04183ea77bcf630c39fa22893039865782a12)), closes [#9326](https://github.com/aws/aws-cdk/issues/9326) +* **cloudfront:** small refactoring of the Origin API ([#9281](https://github.com/aws/aws-cdk/issues/9281)) ([cbfdc15](https://github.com/aws/aws-cdk/commit/cbfdc15959c5d5209d4fed6ac281f9897f44d4c5)), closes [#9109](https://github.com/aws/aws-cdk/issues/9109) + ## [1.55.0](https://github.com/aws/aws-cdk/compare/v1.54.0...v1.55.0) (2020-07-28) diff --git a/lerna.json b/lerna.json index 0a38efa354fad..855e51ed977ca 100644 --- a/lerna.json +++ b/lerna.json @@ -10,5 +10,5 @@ "tools/*" ], "rejectCycles": "true", - "version": "1.55.0" + "version": "1.56.0" }