From a77eaaa0592e57bb85c54280b26f22ab8439c173 Mon Sep 17 00:00:00 2001 From: corymhall <43035978+corymhall@users.noreply.github.com> Date: Tue, 4 Oct 2022 19:52:22 +0000 Subject: [PATCH] feat(cloudfront-origins): allow setting a user defined origin id This PR adds the ability to set a custom user defined origin identifier. The default identifier that is generated is not very descriptive and some users would like the ability to define a custom identifier. Since ids have to be unique per distribution, I also added validation that will throw an error if it is not. fixes #2756 --- .../test/http-origin.test.ts | 3 +- .../test/load-balancer-origin.test.ts | 3 +- .../test/origin-group.test.ts | 79 +++++++++++++++++++ .../test/rest-api-origin.test.ts | 53 +++++++++++++ .../test/s3-origin.test.ts | 70 ++++++++++++++++ .../aws-cloudfront/lib/distribution.ts | 6 +- .../@aws-cdk/aws-cloudfront/lib/origin.ts | 11 ++- 7 files changed, 221 insertions(+), 4 deletions(-) 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 626f9f0cbe9ed..b04c6fa1b0e2b 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 @@ -30,6 +30,7 @@ test('Renders minimal example with just a domain name', () => { test('renders an example with all available props', () => { const origin = new HttpOrigin('www.example.com', { + originId: 'MyCustomOrigin', originPath: '/app', connectionTimeout: Duration.seconds(5), connectionAttempts: 2, @@ -44,7 +45,7 @@ test('renders an example with all available props', () => { const originBindConfig = origin.bind(stack, { originId: 'StackOrigin029E19582' }); expect(originBindConfig.originProperty).toEqual({ - id: 'StackOrigin029E19582', + id: 'MyCustomOrigin', domainName: 'www.example.com', originPath: '/app', connectionTimeout: 5, 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 899fe27ce2e39..6bbe38f976e34 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 @@ -44,11 +44,12 @@ test('Can customize properties of the origin', () => { connectionAttempts: 3, connectionTimeout: Duration.seconds(5), protocolPolicy: cloudfront.OriginProtocolPolicy.MATCH_VIEWER, + originId: 'MyCustomOrigin', }); const originBindConfig = origin.bind(stack, { originId: 'StackOrigin029E19582' }); expect(originBindConfig.originProperty).toEqual({ - id: 'StackOrigin029E19582', + id: 'MyCustomOrigin', domainName: loadBalancer.loadBalancerDnsName, connectionAttempts: 3, connectionTimeout: 5, diff --git a/packages/@aws-cdk/aws-cloudfront-origins/test/origin-group.test.ts b/packages/@aws-cdk/aws-cloudfront-origins/test/origin-group.test.ts index 8289c9c2794e3..a864a03fa7c82 100644 --- a/packages/@aws-cdk/aws-cloudfront-origins/test/origin-group.test.ts +++ b/packages/@aws-cdk/aws-cloudfront-origins/test/origin-group.test.ts @@ -94,6 +94,85 @@ describe('Origin Groups', () => { }); }); + test('correctly render the OriginGroups property of DistributionConfig with originId set', () => { + const failoverOrigin = new origins.S3Origin(s3.Bucket.fromBucketName(stack, 'ImportedBucket', 'imported-bucket'), { originId: 'MyCustomOrigin1' }); + const originGroup = new origins.OriginGroup({ + primaryOrigin, + fallbackOrigin: failoverOrigin, + fallbackStatusCodes: [500], + }); + + new cloudfront.Distribution(stack, 'Distribution', { + defaultBehavior: { origin: originGroup }, + }); + + const primaryOriginId = 'DistributionOrigin13547B94F'; + const originGroupId = 'DistributionOriginGroup1A1A31B49'; + Template.fromStack(stack).hasResourceProperties('AWS::CloudFront::Distribution', { + DistributionConfig: { + DefaultCacheBehavior: { + TargetOriginId: originGroupId, + }, + Origins: [ + { + Id: primaryOriginId, + DomainName: { + 'Fn::GetAtt': ['Bucket83908E77', 'RegionalDomainName'], + }, + S3OriginConfig: { + OriginAccessIdentity: { + 'Fn::Join': ['', [ + 'origin-access-identity/cloudfront/', + { Ref: 'DistributionOrigin1S3Origin5F5C0696' }, + ]], + }, + }, + }, + { + Id: 'MyCustomOrigin1', + DomainName: { + 'Fn::Join': ['', [ + 'imported-bucket.s3.', + { Ref: 'AWS::Region' }, + '.', + { Ref: 'AWS::URLSuffix' }, + ]], + }, + S3OriginConfig: { + OriginAccessIdentity: { + 'Fn::Join': ['', [ + 'origin-access-identity/cloudfront/', + { Ref: 'DistributionOrigin2S3OriginE484D4BF' }, + ]], + }, + }, + }, + ], + OriginGroups: { + Items: [ + { + FailoverCriteria: { + StatusCodes: { + Items: [500], + Quantity: 1, + }, + }, + Id: 'DistributionOriginGroup1A1A31B49', + Members: { + Items: [ + { OriginId: primaryOriginId }, + { OriginId: 'MyCustomOrigin1' }, + ], + Quantity: 2, + }, + }, + ], + Quantity: 1, + }, + }, + }); + }); + test('cannot have an Origin with their own failover configuration as the primary Origin', () => { const failoverOrigin = new origins.S3Origin(s3.Bucket.fromBucketName(stack, 'ImportedBucket', 'imported-bucket')); const originGroup = new origins.OriginGroup({ diff --git a/packages/@aws-cdk/aws-cloudfront-origins/test/rest-api-origin.test.ts b/packages/@aws-cdk/aws-cloudfront-origins/test/rest-api-origin.test.ts index cc89ec8e59976..e84c63ddb8a9d 100644 --- a/packages/@aws-cdk/aws-cloudfront-origins/test/rest-api-origin.test.ts +++ b/packages/@aws-cdk/aws-cloudfront-origins/test/rest-api-origin.test.ts @@ -60,3 +60,56 @@ test('Correctly renders the origin', () => { }, }); }); + +test('Correctly renders the origin, with custom originId', () => { + const api = new apigateway.RestApi(stack, 'RestApi'); + api.root.addMethod('GET'); + + const origin = new RestApiOrigin(api, { originId: 'MyCustomOrigin' }); + const originBindConfig = origin.bind(stack, { originId: 'StackOrigin029E19582' }); + + expect(stack.resolve(originBindConfig.originProperty)).toEqual({ + id: 'MyCustomOrigin', + domainName: { + 'Fn::Select': [2, { + 'Fn::Split': ['/', { + 'Fn::Join': ['', [ + 'https://', { Ref: 'RestApi0C43BF4B' }, + '.execute-api.', + { Ref: 'AWS::Region' }, + '.', + { Ref: 'AWS::URLSuffix' }, + '/', + { Ref: 'RestApiDeploymentStageprod3855DE66' }, + '/', + ]], + }], + }], + }, + originPath: { + 'Fn::Join': ['', ['/', { + 'Fn::Select': [3, { + 'Fn::Split': ['/', { + 'Fn::Join': ['', [ + 'https://', + { Ref: 'RestApi0C43BF4B' }, + '.execute-api.', + { Ref: 'AWS::Region' }, + '.', + { Ref: 'AWS::URLSuffix' }, + '/', + { Ref: 'RestApiDeploymentStageprod3855DE66' }, + '/', + ]], + }], + }], + }]], + }, + customOriginConfig: { + originProtocolPolicy: 'https-only', + originSslProtocols: [ + 'TLSv1.2', + ], + }, + }); +}); 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 5d2d27a1bce0e..7889659db5420 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 @@ -152,6 +152,61 @@ describe('With bucket', () => { }, }); }); + + test('Can set a custom originId', () => { + const bucket = new s3.Bucket(stack, 'Bucket'); + const bucket2 = new s3.Bucket(stack, 'Bucket2'); + const origin2 = new S3Origin(bucket2, { originId: 'MyOtherCustomOrigin' }); + const origin = new S3Origin(bucket, { originId: 'MyCustomOrigin' }); + const distro = new cloudfront.Distribution(stack, 'Dist', { + defaultBehavior: { origin }, + }); + distro.addBehavior('/test', origin2); + + Template.fromStack(stack).hasResourceProperties('AWS::CloudFront::Distribution', { + DistributionConfig: { + CacheBehaviors: [ + { + CachePolicyId: '658327ea-f89d-4fab-a63d-7e88639e58f6', + Compress: true, + PathPattern: '/test', + TargetOriginId: 'MyOtherCustomOrigin', + ViewerProtocolPolicy: 'allow-all', + }, + ], + DefaultCacheBehavior: { + CachePolicyId: '658327ea-f89d-4fab-a63d-7e88639e58f6', + Compress: true, + TargetOriginId: 'MyCustomOrigin', + ViewerProtocolPolicy: 'allow-all', + }, + Origins: [ + { + Id: 'MyCustomOrigin', + }, + { + Id: 'MyOtherCustomOrigin', + }, + ], + }, + }); + }); + test('Cannot set an originId duplicates', () => { + const bucket = new s3.Bucket(stack, 'Bucket'); + const bucket2 = new s3.Bucket(stack, 'Bucket2'); + const origin = new S3Origin(bucket, { originId: 'MyCustomOrigin' }); + const origin2 = new S3Origin(bucket2, { originId: 'MyCustomOrigin' }); + expect(() => { + new cloudfront.Distribution(stack, 'Dist', { + defaultBehavior: { origin }, + additionalBehaviors: { + Origin2: { + origin: origin2, + }, + }, + }); + }).toThrow(/Origin with id MyCustomOrigin already exists/); + }); }); describe('With website-configured bucket', () => { @@ -195,4 +250,19 @@ describe('With website-configured bucket', () => { }, }); }); + + test('Can set an originId', () => { + const bucket = new s3.Bucket(stack, 'Bucket', { + websiteIndexDocument: 'index.html', + }); + const origin = new S3Origin(bucket, { originId: 'MyCustomOrigin' }); + new cloudfront.Distribution(stack, 'Dist', { defaultBehavior: { origin } }); + Template.fromStack(stack).hasResourceProperties('AWS::CloudFront::Distribution', { + DistributionConfig: { + Origins: [{ + Id: 'MyCustomOrigin', + }], + }, + }); + }); }); diff --git a/packages/@aws-cdk/aws-cloudfront/lib/distribution.ts b/packages/@aws-cdk/aws-cloudfront/lib/distribution.ts index 3a6464a7415e3..345477524e4e7 100644 --- a/packages/@aws-cdk/aws-cloudfront/lib/distribution.ts +++ b/packages/@aws-cdk/aws-cloudfront/lib/distribution.ts @@ -356,6 +356,10 @@ export class Distribution extends Resource implements IDistribution { const scope = new Construct(this, `Origin${originIndex}`); const originId = Names.uniqueId(scope).slice(-ORIGIN_ID_MAX_LENGTH); const originBindConfig = origin.bind(scope, { originId }); + const duplicateId = this.boundOrigins.find(boundOrigin => boundOrigin.originProperty?.id === originBindConfig.originProperty?.id); + if (duplicateId) { + throw new Error(`Origin with id ${duplicateId.originProperty?.id} already exists. OriginIds must be unique within a distribution`); + } if (!originBindConfig.failoverConfig) { this.boundOrigins.push({ origin, originId, ...originBindConfig }); } else { @@ -370,7 +374,7 @@ export class Distribution extends Resource implements IDistribution { this.addOriginGroup(originGroupId, originBindConfig.failoverConfig.statusCodes, originId, failoverOriginId); return originGroupId; } - return originId; + return originBindConfig.originProperty?.id ?? originId; } } diff --git a/packages/@aws-cdk/aws-cloudfront/lib/origin.ts b/packages/@aws-cdk/aws-cloudfront/lib/origin.ts index 0640a19a2e5ba..a5a85cde311c6 100644 --- a/packages/@aws-cdk/aws-cloudfront/lib/origin.ts +++ b/packages/@aws-cdk/aws-cloudfront/lib/origin.ts @@ -81,6 +81,13 @@ export interface OriginOptions { * @default - origin shield not enabled */ readonly originShieldRegion?: string; + + /** + * A unique identifier for the origin. This value must be unique within the distribution. + * + * @default - an originid will be generated for you + */ + readonly originId?: string; } /** @@ -118,6 +125,7 @@ export abstract class OriginBase implements IOrigin { private readonly connectionAttempts?: number; private readonly customHeaders?: Record; private readonly originShieldRegion?: string + private readonly originId?: string; protected constructor(domainName: string, props: OriginProps = {}) { validateIntInRangeOrUndefined('connectionTimeout', 1, 10, props.connectionTimeout?.toSeconds()); @@ -130,6 +138,7 @@ export abstract class OriginBase implements IOrigin { this.connectionAttempts = props.connectionAttempts; this.customHeaders = props.customHeaders; this.originShieldRegion = props.originShieldRegion; + this.originId = props.originId; } /** @@ -146,7 +155,7 @@ export abstract class OriginBase implements IOrigin { return { originProperty: { domainName: this.domainName, - id: options.originId, + id: this.originId ?? options.originId, originPath: this.originPath, connectionAttempts: this.connectionAttempts, connectionTimeout: this.connectionTimeout?.toSeconds(),