fix(aws-cloudfront): Fix loggingConfig being ignored

feat(aws-s3): Add support for domainName on BucketRef instead of just Bucket LoggingConfiguration now requires bucket instead of it being optional. There is no point to including a LoggingConfiguration without a bucket (no-breaking). Fixes aws#721
ZeldoKavira · Sep 28, 2018 · 2b88d1b · 2b88d1b
1 parent 2a2b7a1
commit 2b88d1b
Show file tree

Hide file tree

Showing 6 changed files with 187 additions and 1 deletion.
diff --git a/packages/@aws-cdk/aws-cloudfront/lib/web_distribution.ts b/packages/@aws-cdk/aws-cloudfront/lib/web_distribution.ts
@@ -74,7 +74,7 @@ export enum SSLMethod {
  * @default prefix: no prefix is set by default.
  */
 export interface LoggingConfiguration {
-  readonly bucket?: s3.BucketRef,
+  readonly bucket: s3.BucketRef,
   readonly includeCookies?: boolean,
   readonly prefix?: string
 }
@@ -561,6 +561,14 @@ export class CloudFrontWebDistribution extends cdk.Construct {
       };
     }
 
+    if (props.loggingConfig) {
+      distributionConfig.logging = {
+        bucket: props.loggingConfig.bucket.domainName!,
+        includeCookies: props.loggingConfig.includeCookies || false,
+        prefix: props.loggingConfig.prefix
+      };
+    }
+
     const distribution = new cloudformation.DistributionResource(this, 'CFDistribution', {distributionConfig});
     this.domainName = distribution.distributionDomainName;
 

diff --git a/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-bucket-logging.expected.json b/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-bucket-logging.expected.json
@@ -0,0 +1,75 @@
+{
+  "Resources": {
+    "Bucket83908E77": {
+      "Type": "AWS::S3::Bucket"
+    },
+    "AnAmazingWebsiteProbablyCFDistribution47E3983B": {
+      "Type": "AWS::CloudFront::Distribution",
+      "Properties": {
+        "DistributionConfig": {
+          "CacheBehaviors": [],
+          "DefaultCacheBehavior": {
+            "AllowedMethods": [
+              "GET",
+              "HEAD"
+            ],
+            "CachedMethods": [
+              "GET",
+              "HEAD"
+            ],
+            "ForwardedValues": {
+              "Cookies": {
+                "Forward": "none"
+              },
+              "QueryString": false
+            },
+            "TargetOriginId": "origin1",
+            "ViewerProtocolPolicy": "redirect-to-https"
+          },
+          "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": {
+            "AcmCertificateArn": "testACM",
+            "MinimumProtocolVersion": "TLSv1",
+            "SslSupportMethod": "sni-only"
+          },
+          "Aliases": [
+            "test.test.com"
+          ],
+          "Logging": {
+            "Bucket": {
+              "Fn::GetAtt":["Bucket83908E77","DomainName"]
+            },
+            "IncludeCookies": true,
+            "Prefix": "test-prefix"
+          }
+        }
+      }
+    }
+  }
+}
diff --git a/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-bucket-logging.ts b/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-bucket-logging.ts
@@ -0,0 +1,40 @@
+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-custom');
+
+const loggingBucket = new s3.Bucket(stack, 'Bucket');
+
+new cloudfront.CloudFrontWebDistribution(stack, 'AnAmazingWebsiteProbably', {
+  originConfigs: [
+    {
+      originHeaders: {
+        "X-Custom-Header": "somevalue",
+      },
+      customOriginSource: {
+        domainName: "brelandm.a2z.com",
+      },
+      behaviors: [
+        {
+        isDefaultBehavior: true,
+        }
+      ]
+    }
+  ],
+  aliasConfiguration: {
+    acmCertRef: 'testACM',
+    names: ['test.test.com'],
+    sslMethod: cloudfront.SSLMethod.SNI,
+    securityPolicy: cloudfront.SecurityPolicyProtocol.TLSv1
+  },
+  loggingConfig: {
+    bucket: loggingBucket,
+    includeCookies: true,
+    prefix: 'test-prefix'
+  }
+});
+
+process.stdout.write(app.run());
diff --git a/packages/@aws-cdk/aws-s3/lib/bucket.ts b/packages/@aws-cdk/aws-s3/lib/bucket.ts
@@ -30,6 +30,12 @@ export interface BucketRefProps {
    * policy, won't work.
    */
   bucketName?: string;
+
+  /**
+   * The domain of the bucket. This is not required and will be infered from
+   * the bucket name.
+   */
+  bucketDomainName?: string;
 }
 
 /**
@@ -72,6 +78,11 @@ export abstract class BucketRef extends cdk.Construct {
    */
   public abstract readonly bucketName: string;
 
+  /**
+   * The domain of the bucket.
+   */
+  public abstract readonly domainName: string;
+
   /**
    * Optional KMS encryption key associated with this bucket.
    */
@@ -701,6 +712,7 @@ export interface NotificationKeyFilter {
 class ImportedBucketRef extends BucketRef {
   public readonly bucketArn: string;
   public readonly bucketName: string;
+  public readonly domainName: string;
   public readonly encryptionKey?: kms.EncryptionKey;
 
   protected policy?: BucketPolicy;
@@ -716,7 +728,12 @@ class ImportedBucketRef extends BucketRef {
 
     this.bucketArn = parseBucketArn(props);
     this.bucketName = bucketName;
+    this.domainName = props.bucketDomainName || this.generateDomainName();
     this.autoCreatePolicy = false;
     this.policy = undefined;
   }
+
+  private generateDomainName() {
+    return `${this.bucketName}.s3.amazonaws.com`;
+  }
 }
diff --git a/packages/@aws-cdk/aws-s3/test/integ.bucket.domain-name.expected.json b/packages/@aws-cdk/aws-s3/test/integ.bucket.domain-name.expected.json
@@ -0,0 +1,24 @@
+{
+  "Resources": {
+    "MyBucketF68F3FF0": {
+      "Type": "AWS::S3::Bucket"
+    }
+  },
+  "Outputs": {
+    "RealBucketDomain": {
+      "Value": {
+        "Fn::GetAtt":["MyBucketF68F3FF0","DomainName"]
+      },
+      "Export": {
+        "Name": "aws-cdk-s3-urls:RealBucketDomain"
+      }
+    },
+    "ImportedBucketDomain": {
+      "Value": "my-bucket-test.s3.amazonaws.com",
+      "Export": {
+        "Name": "aws-cdk-s3-urls:ImportedBucketDomain"
+      }
+    }
+  }
+}
+
diff --git a/packages/@aws-cdk/aws-s3/test/integ.bucket.domain-name.ts b/packages/@aws-cdk/aws-s3/test/integ.bucket.domain-name.ts
@@ -0,0 +1,22 @@
+import cdk = require('@aws-cdk/cdk');
+import s3 = require('../lib');
+
+class TestStack extends cdk.Stack {
+  constructor(parent: cdk.App, id: string) {
+    super(parent, id);
+
+    /// !show
+    const bucket = new s3.Bucket(this, 'MyBucket');
+    const bucket2 = s3.Bucket.import(this, "MyBucket2", {
+      bucketArn: "arn:aws:s3:::my-bucket-test"
+    });
+
+    new cdk.Output(this, 'RealBucketDomain', { value: bucket.domainName });
+    new cdk.Output(this, 'ImportedBucketDomain', { value: bucket2.domainName });
+    /// !hide
+  }
+}
+
+const app = new cdk.App(process.argv);
+new TestStack(app, 'aws-cdk-s3-urls');
+process.stdout.write(app.run());