Skip to content

Commit 2f1e2e9

Browse files
patricksuterpatricksuter
authored
fix(cloudfront): custom originId not used for multiple behaviors with same origin (#22830)
When setting custom `originId`s for CloudFront origins und the new `Distribution` construct there is an issue when using the same origin for multiple behaviours. The first behaviour uses the correct custom `originId` as `TargetOriginId` but every behaviour after that use the auto-generated `originId` instead of the correct custom id. fixes #22758 Improved the handling of the `originId` to correctly pick either the auto-generated `originId` or the explicitly set `originId` of the origin. The issue was caused by the auto-generated `originId` to always be set on the internal list of origins which caused issues when adding more than one behaviour since the wrong id was returned when re-using an existing origin from the internal list. ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent c936002 commit 2f1e2e9

12 files changed

+494
-4
lines changed

‎packages/@aws-cdk/aws-cloudfront/lib/distribution.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -354,8 +354,9 @@ export class Distribution extends Resource implements IDistribution {
354354
} else {
355355
const originIndex = this.boundOrigins.length + 1;
356356
const scope = new Construct(this, `Origin${originIndex}`);
357-
const originId = Names.uniqueId(scope).slice(-ORIGIN_ID_MAX_LENGTH);
358-
const originBindConfig = origin.bind(scope, { originId });
357+
const generatedId = Names.uniqueId(scope).slice(-ORIGIN_ID_MAX_LENGTH);
358+
const originBindConfig = origin.bind(scope, { originId: generatedId });
359+
const originId = originBindConfig.originProperty?.id ?? generatedId;
359360
const duplicateId = this.boundOrigins.find(boundOrigin => boundOrigin.originProperty?.id === originBindConfig.originProperty?.id);
360361
if (duplicateId) {
361362
throw new Error(`Origin with id ${duplicateId.originProperty?.id} already exists. OriginIds must be unique within a distribution`);

‎packages/@aws-cdk/aws-cloudfront/test/distribution.test.ts

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -975,6 +975,60 @@ describe('origin IDs', () => {
975975
});
976976
});
977977

978+
describe('custom origin ids', () => {
979+
test('test that originId param is respected', () => {
980+
const origin = defaultOrigin(undefined, 'custom-origin-id');
981+
982+
const distribution = new Distribution(stack, 'Http1Distribution', {
983+
defaultBehavior: { origin },
984+
additionalBehaviors: {
985+
secondUsage: {
986+
origin,
987+
},
988+
},
989+
});
990+
distribution.addBehavior(
991+
'thirdUsage',
992+
origin,
993+
);
994+
995+
Template.fromStack(stack).hasResourceProperties('AWS::CloudFront::Distribution', {
996+
DistributionConfig: {
997+
DefaultCacheBehavior: {
998+
CachePolicyId: '658327ea-f89d-4fab-a63d-7e88639e58f6',
999+
Compress: true,
1000+
TargetOriginId: 'custom-origin-id',
1001+
ViewerProtocolPolicy: 'allow-all',
1002+
},
1003+
CacheBehaviors: [{
1004+
CachePolicyId: '658327ea-f89d-4fab-a63d-7e88639e58f6',
1005+
Compress: true,
1006+
PathPattern: 'secondUsage',
1007+
TargetOriginId: 'custom-origin-id',
1008+
ViewerProtocolPolicy: 'allow-all',
1009+
},
1010+
{
1011+
CachePolicyId: '658327ea-f89d-4fab-a63d-7e88639e58f6',
1012+
Compress: true,
1013+
PathPattern: 'thirdUsage',
1014+
TargetOriginId: 'custom-origin-id',
1015+
ViewerProtocolPolicy: 'allow-all',
1016+
}],
1017+
Enabled: true,
1018+
HttpVersion: 'http2',
1019+
IPV6Enabled: true,
1020+
Origins: [{
1021+
DomainName: 'www.example.com',
1022+
Id: 'custom-origin-id',
1023+
CustomOriginConfig: {
1024+
OriginProtocolPolicy: 'https-only',
1025+
},
1026+
}],
1027+
},
1028+
});
1029+
});
1030+
});
1031+
9781032
describe('supported HTTP versions', () => {
9791033
test('setting HTTP/1.1 renders HttpVersion correctly', () => {
9801034
new Distribution(stack, 'Http1Distribution', {

‎packages/@aws-cdk/aws-cloudfront/test/integ.distribution-origin-id.js.snapshot/DistributionOriginIdDefaultTestDeployAssert16FC3109.assets.json

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
{
2+
"version": "21.0.0",
3+
"files": {
4+
"21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22": {
5+
"source": {
6+
"path": "DistributionOriginIdDefaultTestDeployAssert16FC3109.template.json",
7+
"packaging": "file"
8+
},
9+
"destinations": {
10+
"current_account-current_region": {
11+
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
12+
"objectKey": "21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22.json",
13+
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
14+
}
15+
}
16+
}
17+
},
18+
"dockerImages": {}
19+
}

‎packages/@aws-cdk/aws-cloudfront/test/integ.distribution-origin-id.js.snapshot/DistributionOriginIdDefaultTestDeployAssert16FC3109.template.json

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
{
2+
"Parameters": {
3+
"BootstrapVersion": {
4+
"Type": "AWS::SSM::Parameter::Value<String>",
5+
"Default": "/cdk-bootstrap/hnb659fds/version",
6+
"Description": "Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store. [cdk:skip]"
7+
}
8+
},
9+
"Rules": {
10+
"CheckBootstrapVersion": {
11+
"Assertions": [
12+
{
13+
"Assert": {
14+
"Fn::Not": [
15+
{
16+
"Fn::Contains": [
17+
[
18+
"1",
19+
"2",
20+
"3",
21+
"4",
22+
"5"
23+
],
24+
{
25+
"Ref": "BootstrapVersion"
26+
}
27+
]
28+
}
29+
]
30+
},
31+
"AssertDescription": "CDK bootstrap stack version 6 required. Please run 'cdk bootstrap' with a recent version of the CDK CLI."
32+
}
33+
]
34+
}
35+
}
36+
}

‎packages/@aws-cdk/aws-cloudfront/test/integ.distribution-origin-id.js.snapshot/cdk.out

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{"version":"21.0.0"}

‎packages/@aws-cdk/aws-cloudfront/test/integ.distribution-origin-id.js.snapshot/integ-distribution-origin-id.assets.json

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
{
2+
"version": "21.0.0",
3+
"files": {
4+
"413425a9c32869faf78d5031f15a0e8f5eed244599a8c576f6f4c025d23505e6": {
5+
"source": {
6+
"path": "integ-distribution-origin-id.template.json",
7+
"packaging": "file"
8+
},
9+
"destinations": {
10+
"current_account-current_region": {
11+
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
12+
"objectKey": "413425a9c32869faf78d5031f15a0e8f5eed244599a8c576f6f4c025d23505e6.json",
13+
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
14+
}
15+
}
16+
}
17+
},
18+
"dockerImages": {}
19+
}

‎packages/@aws-cdk/aws-cloudfront/test/integ.distribution-origin-id.js.snapshot/integ-distribution-origin-id.template.json

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
{
2+
"Resources": {
3+
"TestDistribution94EC811C": {
4+
"Type": "AWS::CloudFront::Distribution",
5+
"Properties": {
6+
"DistributionConfig": {
7+
"CacheBehaviors": [
8+
{
9+
"CachePolicyId": "658327ea-f89d-4fab-a63d-7e88639e58f6",
10+
"Compress": true,
11+
"PathPattern": "/second",
12+
"TargetOriginId": "my-custom-origin-id",
13+
"ViewerProtocolPolicy": "allow-all"
14+
},
15+
{
16+
"CachePolicyId": "658327ea-f89d-4fab-a63d-7e88639e58f6",
17+
"Compress": true,
18+
"PathPattern": "/third",
19+
"TargetOriginId": "my-custom-origin-id",
20+
"ViewerProtocolPolicy": "allow-all"
21+
}
22+
],
23+
"DefaultCacheBehavior": {
24+
"CachePolicyId": "658327ea-f89d-4fab-a63d-7e88639e58f6",
25+
"Compress": true,
26+
"TargetOriginId": "my-custom-origin-id",
27+
"ViewerProtocolPolicy": "allow-all"
28+
},
29+
"Enabled": true,
30+
"HttpVersion": "http2",
31+
"IPV6Enabled": true,
32+
"Origins": [
33+
{
34+
"CustomOriginConfig": {
35+
"OriginProtocolPolicy": "https-only"
36+
},
37+
"DomainName": "www.example.com",
38+
"Id": "my-custom-origin-id"
39+
}
40+
]
41+
}
42+
}
43+
}
44+
},
45+
"Parameters": {
46+
"BootstrapVersion": {
47+
"Type": "AWS::SSM::Parameter::Value<String>",
48+
"Default": "/cdk-bootstrap/hnb659fds/version",
49+
"Description": "Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store. [cdk:skip]"
50+
}
51+
},
52+
"Rules": {
53+
"CheckBootstrapVersion": {
54+
"Assertions": [
55+
{
56+
"Assert": {
57+
"Fn::Not": [
58+
{
59+
"Fn::Contains": [
60+
[
61+
"1",
62+
"2",
63+
"3",
64+
"4",
65+
"5"
66+
],
67+
{
68+
"Ref": "BootstrapVersion"
69+
}
70+
]
71+
}
72+
]
73+
},
74+
"AssertDescription": "CDK bootstrap stack version 6 required. Please run 'cdk bootstrap' with a recent version of the CDK CLI."
75+
}
76+
]
77+
}
78+
}
79+
}

‎packages/@aws-cdk/aws-cloudfront/test/integ.distribution-origin-id.js.snapshot/integ.json

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
{
2+
"version": "21.0.0",
3+
"testCases": {
4+
"DistributionOriginId/DefaultTest": {
5+
"stacks": [
6+
"integ-distribution-origin-id"
7+
],
8+
"assertionStack": "DistributionOriginId/DefaultTest/DeployAssert",
9+
"assertionStackName": "DistributionOriginIdDefaultTestDeployAssert16FC3109"
10+
}
11+
}
12+
}

‎packages/@aws-cdk/aws-cloudfront/test/integ.distribution-origin-id.js.snapshot/manifest.json

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
{
2+
"version": "21.0.0",
3+
"artifacts": {
4+
"Tree": {
5+
"type": "cdk:tree",
6+
"properties": {
7+
"file": "tree.json"
8+
}
9+
},
10+
"integ-distribution-origin-id.assets": {
11+
"type": "cdk:asset-manifest",
12+
"properties": {
13+
"file": "integ-distribution-origin-id.assets.json",
14+
"requiresBootstrapStackVersion": 6,
15+
"bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version"
16+
}
17+
},
18+
"integ-distribution-origin-id": {
19+
"type": "aws:cloudformation:stack",
20+
"environment": "aws://unknown-account/unknown-region",
21+
"properties": {
22+
"templateFile": "integ-distribution-origin-id.template.json",
23+
"validateOnSynth": false,
24+
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}",
25+
"cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}",
26+
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/413425a9c32869faf78d5031f15a0e8f5eed244599a8c576f6f4c025d23505e6.json",
27+
"requiresBootstrapStackVersion": 6,
28+
"bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version",
29+
"additionalDependencies": [
30+
"integ-distribution-origin-id.assets"
31+
],
32+
"lookupRole": {
33+
"arn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-lookup-role-${AWS::AccountId}-${AWS::Region}",
34+
"requiresBootstrapStackVersion": 8,
35+
"bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version"
36+
}
37+
},
38+
"dependencies": [
39+
"integ-distribution-origin-id.assets"
40+
],
41+
"metadata": {
42+
"/integ-distribution-origin-id/TestDistribution/Resource": [
43+
{
44+
"type": "aws:cdk:logicalId",
45+
"data": "TestDistribution94EC811C"
46+
}
47+
],
48+
"/integ-distribution-origin-id/BootstrapVersion": [
49+
{
50+
"type": "aws:cdk:logicalId",
51+
"data": "BootstrapVersion"
52+
}
53+
],
54+
"/integ-distribution-origin-id/CheckBootstrapVersion": [
55+
{
56+
"type": "aws:cdk:logicalId",
57+
"data": "CheckBootstrapVersion"
58+
}
59+
]
60+
},
61+
"displayName": "integ-distribution-origin-id"
62+
},
63+
"DistributionOriginIdDefaultTestDeployAssert16FC3109.assets": {
64+
"type": "cdk:asset-manifest",
65+
"properties": {
66+
"file": "DistributionOriginIdDefaultTestDeployAssert16FC3109.assets.json",
67+
"requiresBootstrapStackVersion": 6,
68+
"bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version"
69+
}
70+
},
71+
"DistributionOriginIdDefaultTestDeployAssert16FC3109": {
72+
"type": "aws:cloudformation:stack",
73+
"environment": "aws://unknown-account/unknown-region",
74+
"properties": {
75+
"templateFile": "DistributionOriginIdDefaultTestDeployAssert16FC3109.template.json",
76+
"validateOnSynth": false,
77+
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}",
78+
"cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}",
79+
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22.json",
80+
"requiresBootstrapStackVersion": 6,
81+
"bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version",
82+
"additionalDependencies": [
83+
"DistributionOriginIdDefaultTestDeployAssert16FC3109.assets"
84+
],
85+
"lookupRole": {
86+
"arn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-lookup-role-${AWS::AccountId}-${AWS::Region}",
87+
"requiresBootstrapStackVersion": 8,
88+
"bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version"
89+
}
90+
},
91+
"dependencies": [
92+
"DistributionOriginIdDefaultTestDeployAssert16FC3109.assets"
93+
],
94+
"metadata": {
95+
"/DistributionOriginId/DefaultTest/DeployAssert/BootstrapVersion": [
96+
{
97+
"type": "aws:cdk:logicalId",
98+
"data": "BootstrapVersion"
99+
}
100+
],
101+
"/DistributionOriginId/DefaultTest/DeployAssert/CheckBootstrapVersion": [
102+
{
103+
"type": "aws:cdk:logicalId",
104+
"data": "CheckBootstrapVersion"
105+
}
106+
]
107+
},
108+
"displayName": "DistributionOriginId/DefaultTest/DeployAssert"
109+
}
110+
}
111+
}

0 commit comments

Comments
 (0)