Fix CloudFront origin error (#514)

* Fix CloudFront origin error When using CloudFront without an origin access identity, we were failing to inlclude the "empty" property S3OriginConfig - this fixes that. Also adds an integ test and a validate that verifies this. Should fix #508
aws · Aug 7, 2018 · 436cc6f · 436cc6f
1 parent 6846b60
commit 436cc6f
Show file tree

Hide file tree

Showing 8 changed files with 270 additions and 6 deletions.
diff --git a/packages/@aws-cdk/aws-cloudfront/README.md b/packages/@aws-cdk/aws-cloudfront/README.md
@@ -6,13 +6,13 @@ Example usage:
 ```ts
 const sourceBucket = new Bucket(this, 'Bucket');
 
-const distribution = new CloudFrontDistribution(this, 'MyDistribution', {
+const distribution = new CloudFrontWebDistribution(this, 'MyDistribution', {
     originConfigs: [
         {
             s3OriginSource: {
                 s3BucketSource: sourceBucket
             },
-            behaviors : [ {isDefaultBehavior}]
+            behaviors : [ {isDefaultBehavior: true}]
         }
     ]
  });

diff --git a/packages/@aws-cdk/aws-cloudfront/lib/web_distribution.ts b/packages/@aws-cdk/aws-cloudfront/lib/web_distribution.ts
@@ -508,7 +508,10 @@ export class CloudFrontWebDistribution extends cdk.Construct {
                         "origin-access-identity/cloudfront/", originConfig.s3OriginSource.originAccessIdentity.ref
                     ),
                 };
+            } else if (originConfig.s3OriginSource) {
+                originProperty.s3OriginConfig = {};
             }
+
             if (originConfig.customOriginSource) {
                 originProperty.customOriginConfig = {
                     httpPort: originConfig.customOriginSource.httpPort || 80,
@@ -525,6 +528,12 @@ export class CloudFrontWebDistribution extends cdk.Construct {
             origins.push(originProperty);
             originIndex++;
         }
+
+        origins.forEach(origin => {
+            if (!origin.s3OriginConfig && !origin.customOriginConfig) {
+                throw new Error(`Origin ${origin.domainName} is missing either S3OriginConfig or CustomOriginConfig. At least 1 must be specified.`);
+            }
+        });
         distributionConfig.origins = origins;
 
         const defaultBehaviors = behaviors.filter(behavior => behavior.isDefaultBehavior);

diff --git a/packages/@aws-cdk/aws-cloudfront/package.json b/packages/@aws-cdk/aws-cloudfront/package.json
@@ -28,6 +28,7 @@
     "watch": "cdk-watch",
     "lint": "cdk-lint",
     "test": "cdk-test",
+    "integ": "cdk-integ",
     "pkglint": "pkglint -f",
     "package": "cdk-package"
   },
@@ -47,10 +48,11 @@
   },
   "license": "Apache-2.0",
   "devDependencies": {
-    "@aws-cdk/assert": "^0.8.0",
     "aws-sdk": "^2.259.1",
-    "cdk-build-tools": "^0.8.0",
     "cfn2ts": "^0.8.0",
+    "@aws-cdk/assert": "^0.8.0",
+    "cdk-build-tools": "^0.8.0",
+    "cdk-integ-tools": "^0.8.0",
     "pkglint": "^0.8.0"
   },
   "dependencies": {

diff --git a/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-custom.expected.json b/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-custom.expected.json
@@ -0,0 +1,60 @@
+{
+  "Resources": {
+    "AnAmazingWebsiteProbablyCFDistribution47E3983B": {
+      "Type": "AWS::CloudFront::Distribution",
+      "Properties": {
+        "DistributionConfig": {
+          "CacheBehaviors": [],
+          "DefaultCacheBehavior": {
+            "AllowedMethods": [
+              "GET",
+              "HEAD"
+            ],
+            "CachedMethods": [
+              "GET",
+              "HEAD"
+            ],
+            "ForwardedValues": {
+              "Cookies": {
+                "Forward": "none"
+              },
+              "QueryString": false
+            },
+            "TargetOriginId": "origin1",
+            "ViewerProtocolPolicy": "https-only"
+          },
+          "DefaultRootObject": "index.html",
+          "Enabled": true,
+          "HttpVersion": "http2",
+          "IPV6Enabled": true,
+          "Origins": [
+            {
+              "CustomOriginConfig": {
+                "HTTPPort": 80,
+                "HTTPSPort": 443,
+                "OriginKeepaliveTimeout": 5,
+                "OriginProtocolPolicy": "https-only",
+                "OriginReadTimeout": 30,
+                "OriginSSLProtocols": [
+                  "TLSv1.2"
+                ]
+              },
+              "DomainName": "brelandm.a2z.com",
+              "Id": "origin1",
+              "OriginCustomHeaders": [
+                {
+                  "HeaderName": "X-Custom-Header",
+                  "HeaderValue": "somevalue"
+                }
+              ]
+            }
+          ],
+          "PriceClass": "PriceClass_100",
+          "ViewerCertificate": {
+            "CloudFrontDefaultCertificate": true
+          }
+        }
+      }
+    }
+  }
+}
diff --git a/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-custom.ts b/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-custom.ts
@@ -0,0 +1,27 @@
+
+import cdk = require('@aws-cdk/cdk');
+import cloudfront = require('../lib');
+
+const app = new cdk.App(process.argv);
+
+const stack = new cdk.Stack(app, 'aws-cdk-cloudfront-custom');
+
+new cloudfront.CloudFrontWebDistribution(stack, 'AnAmazingWebsiteProbably', {
+    originConfigs: [
+        {
+            originHeaders: {
+              "X-Custom-Header": "somevalue",
+            },
+            customOriginSource: {
+              domainName: "brelandm.a2z.com",
+            },
+            behaviors: [
+              {
+                isDefaultBehavior: true,
+              }
+            ]
+        }
+    ]
+});
+
+process.stdout.write(app.run());
diff --git a/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront.expected.json b/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront.expected.json
@@ -0,0 +1,53 @@
+{
+  "Resources": {
+    "Bucket83908E77": {
+      "Type": "AWS::S3::Bucket"
+    },
+    "MyDistributionCFDistributionDE147309": {
+      "Type": "AWS::CloudFront::Distribution",
+      "Properties": {
+        "DistributionConfig": {
+          "CacheBehaviors": [],
+          "DefaultCacheBehavior": {
+            "AllowedMethods": [
+              "GET",
+              "HEAD"
+            ],
+            "CachedMethods": [
+              "GET",
+              "HEAD"
+            ],
+            "ForwardedValues": {
+              "Cookies": {
+                "Forward": "none"
+              },
+              "QueryString": false
+            },
+            "TargetOriginId": "origin1",
+            "ViewerProtocolPolicy": "https-only"
+          },
+          "DefaultRootObject": "index.html",
+          "Enabled": true,
+          "HttpVersion": "http2",
+          "IPV6Enabled": true,
+          "Origins": [
+            {
+              "DomainName": {
+                "Fn::GetAtt": [
+                  "Bucket83908E77",
+                  "DomainName"
+                ]
+              },
+              "Id": "origin1",
+              "S3OriginConfig": {}
+            }
+          ],
+          "PriceClass": "PriceClass_100",
+          "ViewerCertificate": {
+            "CloudFrontDefaultCertificate": true
+          }
+        }
+      }
+    }
+  }
+}
diff --git a/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront.ts b/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront.ts
@@ -0,0 +1,23 @@
+
+import s3 = require('@aws-cdk/aws-s3');
+import cdk = require('@aws-cdk/cdk');
+import cloudfront = require('../lib');
+
+const app = new cdk.App(process.argv);
+
+const stack = new cdk.Stack(app, 'aws-cdk-cloudfront');
+
+const sourceBucket = new s3.Bucket(stack, 'Bucket');
+
+new cloudfront.CloudFrontWebDistribution(stack, 'MyDistribution', {
+    originConfigs: [
+        {
+            s3OriginSource: {
+                s3BucketSource: sourceBucket
+            },
+            behaviors : [ {isDefaultBehavior: true}]
+        }
+    ]
+ });
+
+process.stdout.write(app.run());
diff --git a/packages/@aws-cdk/aws-cloudfront/test/test.basic.ts b/packages/@aws-cdk/aws-cloudfront/test/test.basic.ts
@@ -7,6 +7,94 @@ import { CloudFrontWebDistribution } from '../lib';
 // tslint:disable:object-literal-key-quotes
 
 export = {
+
+    'distribution with custom origin adds custom origin'(test: Test) {
+      const stack = new cdk.Stack();
+
+      new CloudFrontWebDistribution(stack, 'AnAmazingWebsiteProbably', {
+          originConfigs: [
+              {
+                  originHeaders: {
+                    "X-Custom-Header": "somevalue",
+                  },
+                  customOriginSource: {
+                    domainName: "myorigin.com",
+                  },
+                  behaviors: [
+                    {
+                      isDefaultBehavior: true,
+                    }
+                  ],
+              }
+          ]
+      });
+
+      expect(stack).toMatch(
+        {
+          "Resources": {
+          "AnAmazingWebsiteProbablyCFDistribution47E3983B": {
+            "Type": "AWS::CloudFront::Distribution",
+            "Properties": {
+              "DistributionConfig": {
+                "CacheBehaviors": [],
+                "DefaultCacheBehavior": {
+                  "AllowedMethods": [
+                    "GET",
+                    "HEAD"
+                  ],
+                  "CachedMethods": [
+                    "GET",
+                    "HEAD"
+                  ],
+                  "ForwardedValues": {
+                    "Cookies": {
+                      "Forward": "none"
+                    },
+                    "QueryString": false
+                  },
+                  "TargetOriginId": "origin1",
+                  "ViewerProtocolPolicy": "https-only"
+                },
+                "DefaultRootObject": "index.html",
+                "Enabled": true,
+                "HttpVersion": "http2",
+                "IPV6Enabled": true,
+                "Origins": [
+                  {
+                    "CustomOriginConfig": {
+                      "HTTPPort": 80,
+                      "HTTPSPort": 443,
+                      "OriginKeepaliveTimeout": 5,
+                      "OriginProtocolPolicy": "https-only",
+                      "OriginReadTimeout": 30,
+                      "OriginSSLProtocols": [
+                        "TLSv1.2"
+                      ]
+                    },
+                    "DomainName": "myorigin.com",
+                    "Id": "origin1",
+                    "OriginCustomHeaders": [
+                      {
+                        "HeaderName": "X-Custom-Header",
+                        "HeaderValue": "somevalue"
+                      }
+                    ]
+                  }
+                ],
+                "PriceClass": "PriceClass_100",
+                "ViewerCertificate": {
+                  "CloudFrontDefaultCertificate": true
+                }
+              }
+            }
+          }
+        }
+      }
+      );
+
+      test.done();
+    },
+
     'most basic distribution'(test: Test) {
         const stack = new cdk.Stack();
         const sourceBucket = new s3.Bucket(stack, 'Bucket');
@@ -44,7 +132,8 @@ export = {
                             "DomainName"
                           ]
                         },
-                        "Id": "origin1"
+                        "Id": "origin1",
+                        "S3OriginConfig": {}
                       }
                     ],
                     "ViewerCertificate": {
@@ -117,7 +206,8 @@ export = {
                             "DomainName"
                           ]
                         },
-                        "Id": "origin1"
+                        "Id": "origin1",
+                        "S3OriginConfig": {}
                       }
                     ],
                     "ViewerCertificate": {