fix(cloudfront): ensures origin groups are added with their own ID as…

… a target (#9593) This is to fix the issue I created yesterday #9561 - mostly to get the CI running and see if the test pass already. The whole issue is described in #9561. Fixes #9561 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
aws · Aug 13, 2020 · 246842f · 246842f
1 parent 014c13a
commit 246842f
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 8 deletions.
diff --git a/packages/@aws-cdk/aws-cloudfront-origins/test/integ.origin-group.expected.json b/packages/@aws-cdk/aws-cloudfront-origins/test/integ.origin-group.expected.json
@@ -72,9 +72,19 @@
             "ForwardedValues": {
               "QueryString": false
             },
-            "TargetOriginId": "cloudfrontorigingroupDistributionOrigin137659A54",
+            "TargetOriginId": "cloudfrontorigingroupDistributionOriginGroup10B57F1D1",
             "ViewerProtocolPolicy": "allow-all"
           },
+          "CacheBehaviors": [
+            {
+              "ForwardedValues": {
+                "QueryString": false
+              },
+              "PathPattern": "/api",
+              "TargetOriginId": "cloudfrontorigingroupDistributionOriginGroup10B57F1D1",
+              "ViewerProtocolPolicy": "allow-all"
+            }
+          ],
           "Enabled": true,
           "HttpVersion": "http2",
           "IPV6Enabled": true,

diff --git a/packages/@aws-cdk/aws-cloudfront-origins/test/integ.origin-group.ts b/packages/@aws-cdk/aws-cloudfront-origins/test/integ.origin-group.ts
@@ -17,6 +17,11 @@ const originGroup = new origins.OriginGroup({
 
 new cloudfront.Distribution(stack, 'Distribution', {
   defaultBehavior: { origin: originGroup },
+  additionalBehaviors: {
+    '/api': {
+      origin: originGroup,
+    },
+  },
 });
 
 app.synth();
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
@@ -28,8 +28,12 @@ describe('Origin Groups', () => {
 
     const primaryOriginId = 'DistributionOrigin13547B94F';
     const failoverOriginId = 'DistributionOrigin2C85CC43B';
+    const originGroupId = 'DistributionOriginGroup1A1A31B49';
     expect(stack).toHaveResourceLike('AWS::CloudFront::Distribution', {
       DistributionConfig: {
+        DefaultCacheBehavior: {
+          TargetOriginId: originGroupId,
+        },
         Origins: [
           {
             Id: primaryOriginId,

diff --git a/packages/@aws-cdk/aws-cloudfront/lib/distribution.ts b/packages/@aws-cdk/aws-cloudfront/lib/distribution.ts
@@ -57,6 +57,7 @@ export interface DistributionAttributes {
 
 interface BoundOrigin extends OriginBindOptions, OriginBindConfig {
   readonly origin: IOrigin;
+  readonly originGroupId?: string;
 }
 
 /**
@@ -291,38 +292,43 @@ export class Distribution extends Resource implements IDistribution {
   private addOrigin(origin: IOrigin, isFailoverOrigin: boolean = false): string {
     const existingOrigin = this.boundOrigins.find(boundOrigin => boundOrigin.origin === origin);
     if (existingOrigin) {
-      return existingOrigin.originId;
+      return existingOrigin.originGroupId ?? 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 });
-      if (originBindConfig.failoverConfig) {
+      if (!originBindConfig.failoverConfig) {
+        this.boundOrigins.push({origin, originId, ...originBindConfig});
+      } else {
         if (isFailoverOrigin) {
           throw new Error('An Origin cannot use an Origin with its own failover configuration as its fallback origin!');
         }
+        const groupIndex = this.originGroups.length + 1;
+        const originGroupId = new Construct(this, `OriginGroup${groupIndex}`).node.uniqueId;
+        this.boundOrigins.push({origin, originId, originGroupId, ...originBindConfig});
+
         const failoverOriginId = this.addOrigin(originBindConfig.failoverConfig.failoverOrigin, true);
-        this.addOriginGroup(originBindConfig.failoverConfig.statusCodes, originId, failoverOriginId);
+        this.addOriginGroup(originGroupId, originBindConfig.failoverConfig.statusCodes, originId, failoverOriginId);
+        return originGroupId;
       }
       return originId;
     }
   }
 
-  private addOriginGroup(statusCodes: number[] | undefined, originId: string, failoverOriginId: string): void {
+  private addOriginGroup(originGroupId: string, statusCodes: number[] | undefined, originId: string, failoverOriginId: string): void {
     statusCodes = statusCodes ?? [500, 502, 503, 504];
     if (statusCodes.length === 0) {
       throw new Error('fallbackStatusCodes cannot be empty');
     }
-    const groupIndex = this.originGroups.length + 1;
     this.originGroups.push({
       failoverCriteria: {
         statusCodes: {
           items: statusCodes,
           quantity: statusCodes.length,
         },
       },
-      id: new Construct(this, `OriginGroup${groupIndex}`).node.uniqueId,
+      id: originGroupId,
       members: {
         items: [
           { originId },