From 11e77c41733109a76007c5da1d1ad06c897dbdfe Mon Sep 17 00:00:00 2001 From: ren-yamanashi Date: Fri, 22 Nov 2024 23:50:57 +0900 Subject: [PATCH 1/6] feat(cloudfront): validate attachWebAclId --- .../aws-cloudfront/lib/distribution.ts | 5 +++++ .../aws-cloudfront/test/distribution.test.ts | 22 +++++++++++++++---- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/packages/aws-cdk-lib/aws-cloudfront/lib/distribution.ts b/packages/aws-cdk-lib/aws-cloudfront/lib/distribution.ts index ba8bac6636cd9..98b3c00afd142 100644 --- a/packages/aws-cdk-lib/aws-cloudfront/lib/distribution.ts +++ b/packages/aws-cdk-lib/aws-cloudfront/lib/distribution.ts @@ -606,12 +606,17 @@ export class Distribution extends Resource implements IDistribution { /** * Attach WAF WebACL to this CloudFront distribution * + * WebACL must be in us-east-1 region * @param webAclId The WAF WebACL to associate with this distribution */ public attachWebAclId(webAclId: string) { if (this.webAclId) { throw new Error('A WebACL has already been attached to this distribution'); } + const arnParts = Stack.of(this).splitArn(webAclId, ArnFormat.SLASH_RESOURCE_NAME); + if (!Token.isUnresolved(arnParts.region) && arnParts.region !== 'us-east-1') { + throw new Error(`WebACL for CloudFront distributions must be created in the us-east-1 region; received ${arnParts.region}`); + } this.webAclId = webAclId; } diff --git a/packages/aws-cdk-lib/aws-cloudfront/test/distribution.test.ts b/packages/aws-cdk-lib/aws-cloudfront/test/distribution.test.ts index a6dc892975c0d..1a4af8b6c1596 100644 --- a/packages/aws-cdk-lib/aws-cloudfront/test/distribution.test.ts +++ b/packages/aws-cdk-lib/aws-cloudfront/test/distribution.test.ts @@ -1343,11 +1343,11 @@ describe('attachWebAclId', () => { defaultBehavior: { origin }, }); - distribution.attachWebAclId('473e64fd-f30b-4765-81a0-62ad96dd167a'); + distribution.attachWebAclId('arn:aws:wafv2:us-east-1:123456789012:global/web-acl/MyWebAcl/473e64fd-f30b-4765-81a0-62ad96dd167a'); Template.fromStack(stack).hasResourceProperties('AWS::CloudFront::Distribution', { DistributionConfig: { - WebACLId: '473e64fd-f30b-4765-81a0-62ad96dd167a', + WebACLId: 'arn:aws:wafv2:us-east-1:123456789012:global/web-acl/MyWebAcl/473e64fd-f30b-4765-81a0-62ad96dd167a', }, }); }); @@ -1357,11 +1357,25 @@ describe('attachWebAclId', () => { const distribution = new Distribution(stack, 'MyDist', { defaultBehavior: { origin }, - webAclId: '473e64fd-f30b-4765-81a0-62ad96dd167a', + webAclId: 'arn:aws:wafv2:us-east-1:123456789012:global/web-acl/MyWebAcl/473e64fd-f30b-4765-81a0-62ad96dd167a', }); expect(() => { - distribution.attachWebAclId('473e64fd-f30b-4765-81a0-62ad96dd167b'); + distribution.attachWebAclId('arn:aws:wafv2:us-east-1:123456789012:global/web-acl/MyWebAcl/473e64fd-f30b-4765-81a0-62ad96dd167b'); }).toThrow(/A WebACL has already been attached to this distribution/); }); + + test('throws if the WebAcl is not in us-east-1 region', () => { + const origin = defaultOrigin(); + + const distribution = new Distribution(stack, 'MyDist', { + defaultBehavior: { origin }, + }); + + // ap-northeast-1リージョンのWebACL ARNを指定 + expect(() => { + distribution.attachWebAclId('arn:aws:wafv2:ap-northeast-1:123456789012:global/web-acl/MyWebAcl/473e64fd-f30b-4765-81a0-62ad96dd167a'); + }).toThrow(/WebACL for CloudFront distributions must be created in the us-east-1 region; received ap-northeast-1/); + }); + }); From 845859620a6f5a8c9bb2a931229f5f0a8a5b81d0 Mon Sep 17 00:00:00 2001 From: ren-yamanashi Date: Sat, 23 Nov 2024 00:10:15 +0900 Subject: [PATCH 2/6] feat(cloudfront): fix validation logic and test --- .../aws-cdk-lib/aws-cloudfront/lib/distribution.ts | 11 +++++++---- .../aws-cloudfront/test/distribution.test.ts | 8 ++++---- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/packages/aws-cdk-lib/aws-cloudfront/lib/distribution.ts b/packages/aws-cdk-lib/aws-cloudfront/lib/distribution.ts index 98b3c00afd142..5fae54954084d 100644 --- a/packages/aws-cdk-lib/aws-cloudfront/lib/distribution.ts +++ b/packages/aws-cdk-lib/aws-cloudfront/lib/distribution.ts @@ -606,16 +606,19 @@ export class Distribution extends Resource implements IDistribution { /** * Attach WAF WebACL to this CloudFront distribution * - * WebACL must be in us-east-1 region + * WebACL must be in the us-east-1 region. + * * @param webAclId The WAF WebACL to associate with this distribution */ public attachWebAclId(webAclId: string) { if (this.webAclId) { throw new Error('A WebACL has already been attached to this distribution'); } - const arnParts = Stack.of(this).splitArn(webAclId, ArnFormat.SLASH_RESOURCE_NAME); - if (!Token.isUnresolved(arnParts.region) && arnParts.region !== 'us-east-1') { - throw new Error(`WebACL for CloudFront distributions must be created in the us-east-1 region; received ${arnParts.region}`); + if (webAclId.startsWith('arn:')) { + const arnParts = Stack.of(this).splitArn(webAclId, ArnFormat.SLASH_RESOURCE_NAME); + if (!Token.isUnresolved(arnParts.region) && arnParts.region !== 'us-east-1') { + throw new Error(`WebACL for CloudFront distributions must be created in the us-east-1 region; received ${arnParts.region}`); + } } this.webAclId = webAclId; } diff --git a/packages/aws-cdk-lib/aws-cloudfront/test/distribution.test.ts b/packages/aws-cdk-lib/aws-cloudfront/test/distribution.test.ts index 1a4af8b6c1596..5bbbbdfc19c94 100644 --- a/packages/aws-cdk-lib/aws-cloudfront/test/distribution.test.ts +++ b/packages/aws-cdk-lib/aws-cloudfront/test/distribution.test.ts @@ -1343,11 +1343,11 @@ describe('attachWebAclId', () => { defaultBehavior: { origin }, }); - distribution.attachWebAclId('arn:aws:wafv2:us-east-1:123456789012:global/web-acl/MyWebAcl/473e64fd-f30b-4765-81a0-62ad96dd167a'); + distribution.attachWebAclId('473e64fd-f30b-4765-81a0-62ad96dd167a'); Template.fromStack(stack).hasResourceProperties('AWS::CloudFront::Distribution', { DistributionConfig: { - WebACLId: 'arn:aws:wafv2:us-east-1:123456789012:global/web-acl/MyWebAcl/473e64fd-f30b-4765-81a0-62ad96dd167a', + WebACLId: '473e64fd-f30b-4765-81a0-62ad96dd167a', }, }); }); @@ -1357,11 +1357,11 @@ describe('attachWebAclId', () => { const distribution = new Distribution(stack, 'MyDist', { defaultBehavior: { origin }, - webAclId: 'arn:aws:wafv2:us-east-1:123456789012:global/web-acl/MyWebAcl/473e64fd-f30b-4765-81a0-62ad96dd167a', + webAclId: '473e64fd-f30b-4765-81a0-62ad96dd167a', }); expect(() => { - distribution.attachWebAclId('arn:aws:wafv2:us-east-1:123456789012:global/web-acl/MyWebAcl/473e64fd-f30b-4765-81a0-62ad96dd167b'); + distribution.attachWebAclId('473e64fd-f30b-4765-81a0-62ad96dd167b'); }).toThrow(/A WebACL has already been attached to this distribution/); }); From 4416463e23dda467bf2fe0dbfc19b35d8a9816b6 Mon Sep 17 00:00:00 2001 From: ren-yamanashi Date: Sat, 23 Nov 2024 00:51:11 +0900 Subject: [PATCH 3/6] feat(cloudfront): fix comment --- packages/aws-cdk-lib/aws-cloudfront/test/distribution.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/aws-cdk-lib/aws-cloudfront/test/distribution.test.ts b/packages/aws-cdk-lib/aws-cloudfront/test/distribution.test.ts index 5bbbbdfc19c94..aa828b538510c 100644 --- a/packages/aws-cdk-lib/aws-cloudfront/test/distribution.test.ts +++ b/packages/aws-cdk-lib/aws-cloudfront/test/distribution.test.ts @@ -1372,7 +1372,6 @@ describe('attachWebAclId', () => { defaultBehavior: { origin }, }); - // ap-northeast-1リージョンのWebACL ARNを指定 expect(() => { distribution.attachWebAclId('arn:aws:wafv2:ap-northeast-1:123456789012:global/web-acl/MyWebAcl/473e64fd-f30b-4765-81a0-62ad96dd167a'); }).toThrow(/WebACL for CloudFront distributions must be created in the us-east-1 region; received ap-northeast-1/); From 1407e7747e5599d4fbd4593bc8fcd831b6fa9a48 Mon Sep 17 00:00:00 2001 From: ren-yamanashi Date: Sat, 23 Nov 2024 00:57:37 +0900 Subject: [PATCH 4/6] delete period --- packages/aws-cdk-lib/aws-cloudfront/lib/distribution.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/aws-cdk-lib/aws-cloudfront/lib/distribution.ts b/packages/aws-cdk-lib/aws-cloudfront/lib/distribution.ts index 5fae54954084d..e1c45be3561e9 100644 --- a/packages/aws-cdk-lib/aws-cloudfront/lib/distribution.ts +++ b/packages/aws-cdk-lib/aws-cloudfront/lib/distribution.ts @@ -606,7 +606,7 @@ export class Distribution extends Resource implements IDistribution { /** * Attach WAF WebACL to this CloudFront distribution * - * WebACL must be in the us-east-1 region. + * WebACL must be in the us-east-1 region * * @param webAclId The WAF WebACL to associate with this distribution */ From 8e16c2e663776d9a580e2602b684361a1adc9416 Mon Sep 17 00:00:00 2001 From: ren-yamanashi Date: Sat, 23 Nov 2024 00:59:20 +0900 Subject: [PATCH 5/6] refactor: validation logic --- packages/aws-cdk-lib/aws-cloudfront/lib/distribution.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/aws-cdk-lib/aws-cloudfront/lib/distribution.ts b/packages/aws-cdk-lib/aws-cloudfront/lib/distribution.ts index e1c45be3561e9..456b2a334869c 100644 --- a/packages/aws-cdk-lib/aws-cloudfront/lib/distribution.ts +++ b/packages/aws-cdk-lib/aws-cloudfront/lib/distribution.ts @@ -615,9 +615,9 @@ export class Distribution extends Resource implements IDistribution { throw new Error('A WebACL has already been attached to this distribution'); } if (webAclId.startsWith('arn:')) { - const arnParts = Stack.of(this).splitArn(webAclId, ArnFormat.SLASH_RESOURCE_NAME); - if (!Token.isUnresolved(arnParts.region) && arnParts.region !== 'us-east-1') { - throw new Error(`WebACL for CloudFront distributions must be created in the us-east-1 region; received ${arnParts.region}`); + const webAclRegion = Stack.of(this).splitArn(webAclId, ArnFormat.SLASH_RESOURCE_NAME).region; + if (!Token.isUnresolved(webAclRegion) && webAclRegion !== 'us-east-1') { + throw new Error(`WebACL for CloudFront distributions must be created in the us-east-1 region; received ${webAclRegion}`); } } this.webAclId = webAclId; From 4a02e215cee5c9bd3f325e63ae51c96ee985cf75 Mon Sep 17 00:00:00 2001 From: ren-yamanashi Date: Wed, 27 Nov 2024 21:51:31 +0900 Subject: [PATCH 6/6] feat: add validation --- .../aws-cloudfront/lib/distribution.ts | 12 +++++++-- .../aws-cloudfront/test/distribution.test.ts | 27 ++++++++++++++----- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/packages/aws-cdk-lib/aws-cloudfront/lib/distribution.ts b/packages/aws-cdk-lib/aws-cloudfront/lib/distribution.ts index 456b2a334869c..35f081faafc6b 100644 --- a/packages/aws-cdk-lib/aws-cloudfront/lib/distribution.ts +++ b/packages/aws-cdk-lib/aws-cloudfront/lib/distribution.ts @@ -336,10 +336,14 @@ export class Distribution extends Resource implements IDistribution { }); } + if (props.webAclId) { + this.validateWebAclId(props.webAclId); + this.webAclId = props.webAclId; + } + this.certificate = props.certificate; this.errorResponses = props.errorResponses ?? []; this.publishAdditionalMetrics = props.publishAdditionalMetrics; - this.webAclId = props.webAclId; // Comments have an undocumented limit of 128 characters const trimmedComment = @@ -614,13 +618,17 @@ export class Distribution extends Resource implements IDistribution { if (this.webAclId) { throw new Error('A WebACL has already been attached to this distribution'); } + this.validateWebAclId(webAclId); + this.webAclId = webAclId; + } + + private validateWebAclId(webAclId: string) { if (webAclId.startsWith('arn:')) { const webAclRegion = Stack.of(this).splitArn(webAclId, ArnFormat.SLASH_RESOURCE_NAME).region; if (!Token.isUnresolved(webAclRegion) && webAclRegion !== 'us-east-1') { throw new Error(`WebACL for CloudFront distributions must be created in the us-east-1 region; received ${webAclRegion}`); } } - this.webAclId = webAclId; } private addOrigin(origin: IOrigin, isFailoverOrigin: boolean = false): string { diff --git a/packages/aws-cdk-lib/aws-cloudfront/test/distribution.test.ts b/packages/aws-cdk-lib/aws-cloudfront/test/distribution.test.ts index aa828b538510c..517dc7237dff0 100644 --- a/packages/aws-cdk-lib/aws-cloudfront/test/distribution.test.ts +++ b/packages/aws-cdk-lib/aws-cloudfront/test/distribution.test.ts @@ -1365,16 +1365,29 @@ describe('attachWebAclId', () => { }).toThrow(/A WebACL has already been attached to this distribution/); }); - test('throws if the WebAcl is not in us-east-1 region', () => { - const origin = defaultOrigin(); + describe('throws if the WebAcl is not in us-east-1 region', () => { + test('when try to attach WebACL using `attachWebAclId` method', () => { + const origin = defaultOrigin(); - const distribution = new Distribution(stack, 'MyDist', { - defaultBehavior: { origin }, + const distribution = new Distribution(stack, 'MyDist', { + defaultBehavior: { origin }, + }); + + expect(() => { + distribution.attachWebAclId('arn:aws:wafv2:ap-northeast-1:123456789012:global/web-acl/MyWebAcl/473e64fd-f30b-4765-81a0-62ad96dd167a'); + }).toThrow(/WebACL for CloudFront distributions must be created in the us-east-1 region; received ap-northeast-1/); }); - expect(() => { - distribution.attachWebAclId('arn:aws:wafv2:ap-northeast-1:123456789012:global/web-acl/MyWebAcl/473e64fd-f30b-4765-81a0-62ad96dd167a'); - }).toThrow(/WebACL for CloudFront distributions must be created in the us-east-1 region; received ap-northeast-1/); + test('when try to attach WebACL by specifying value for props', () => { + const origin = defaultOrigin(); + + expect(() => { + new Distribution(stack, 'MyDist', { + defaultBehavior: { origin }, + webAclId: 'arn:aws:wafv2:ap-northeast-1:123456789012:global/web-acl/MyWebAcl/473e64fd-f30b-4765-81a0-62ad96dd167a', + }); + }).toThrow(/WebACL for CloudFront distributions must be created in the us-east-1 region; received ap-northeast-1/); + }); }); });