fix: incorporat review comments and refactor

aws · Jun 16, 2024 · fd44f6f · fd44f6f
1 parent 88a8e79
commit fd44f6f
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 77 deletions.
diff --git a/...k-testing/framework-integ/test/aws-synthetics/test/integ.canary-artifact-s3-encryption.ts b/...k-testing/framework-integ/test/aws-synthetics/test/integ.canary-artifact-s3-encryption.ts
@@ -1,7 +1,7 @@
 /// !cdk-integ canary-one
 import * as s3 from 'aws-cdk-lib/aws-s3';
 import * as cdk from 'aws-cdk-lib/core';
-import { Canary, Cleanup, Code, EncryptionMode, Runtime, Schedule, Test } from 'aws-cdk-lib/aws-synthetics';
+import { ArtifactsEncryptionMode, Canary, Cleanup, Code, Runtime, Schedule, Test } from 'aws-cdk-lib/aws-synthetics';
 import { IntegTest } from '@aws-cdk/integ-tests-alpha';
 import { RemovalPolicy } from 'aws-cdk-lib';
 import { Key } from 'aws-cdk-lib/aws-kms';
@@ -27,9 +27,7 @@ new Canary(stack, 'CanarySseS3', {
   artifactsBucketLocation: { bucket, prefix },
   runtime: Runtime.SYNTHETICS_NODEJS_PUPPETEER_7_0,
   cleanup: Cleanup.LAMBDA,
-  artifactS3Encryption: {
-    encryptionMode: EncryptionMode.S3_MANAGED,
-  },
+  artifactS3EncryptionMode: ArtifactsEncryptionMode.S3_MANAGED,
 });
 
 new Canary(stack, 'CanarySseKmsWithoutKeySetting', {
@@ -44,9 +42,7 @@ new Canary(stack, 'CanarySseKmsWithoutKeySetting', {
   artifactsBucketLocation: { bucket, prefix },
   runtime: Runtime.SYNTHETICS_NODEJS_PUPPETEER_7_0,
   cleanup: Cleanup.LAMBDA,
-  artifactS3Encryption: {
-    encryptionMode: EncryptionMode.KMS,
-  },
+  artifactS3EncryptionMode: ArtifactsEncryptionMode.KMS,
 });
 
 const encryptKey = new Key(stack, 'Key', { removalPolicy: RemovalPolicy.DESTROY });
@@ -63,10 +59,8 @@ new Canary(stack, 'CanarySseKmsWith', {
   artifactsBucketLocation: { bucket, prefix },
   runtime: Runtime.SYNTHETICS_NODEJS_PUPPETEER_7_0,
   cleanup: Cleanup.LAMBDA,
-  artifactS3Encryption: {
-    encryptionMode: EncryptionMode.KMS,
-    kmsKey: encryptKey,
-  },
+  artifactS3EncryptionMode: ArtifactsEncryptionMode.KMS,
+  kmsKey: encryptKey,
 });
 
 new IntegTest(app, 'IntegCanaryTest', {

diff --git a/packages/aws-cdk-lib/aws-synthetics/README.md b/packages/aws-cdk-lib/aws-synthetics/README.md
@@ -251,9 +251,9 @@ const canary = new synthetics.Canary(this, 'MyCanary', {
 
 Canary artifacts are encrypted at rest using an AWS-managed key by default.
 
-Additionally, you can choose the encryption options SSE-S3 or SSE-KMS by setting the `artifactS3Encryption` property.
+Additionally, you can choose the encryption options SSE-S3 or SSE-KMS by setting the `artifactS3EncryptionMode` property.
 
-You can also supply your own external KMS key by specifying the `kmsKey` property. If you don't, a KMS key will be automatically created and associated with the canary.
+When you use SSE-KMS, you can also supply your own external KMS key by specifying the `kmsKey` property. If you don't, a KMS key will be automatically created and associated with the canary.
 
 ```ts
 import * as kms from 'aws-cdk-lib/aws-kms';
@@ -270,9 +270,7 @@ const canary = new synthetics.Canary(this, 'MyCanary', {
   artifactsBucketLifecycleRules: [{
     expiration: Duration.days(30),
   }],
-  artifactS3Encryption: {
-    encryptionMode: synthetics.EncryptionMode.KMS,
-    kmsKey: key, // external KMS Key set
-  },
+  artifactS3EncryptionMode: synthetics.ArtifactsEncryptionMode.KMS,
+  kmsKey: key,
 });
 ```
diff --git a/packages/aws-cdk-lib/aws-synthetics/lib/canary.ts b/packages/aws-cdk-lib/aws-synthetics/lib/canary.ts
@@ -232,47 +232,35 @@ export interface CanaryProps {
   readonly artifactsBucketLifecycleRules?: Array<s3.LifecycleRule>;
 
   /**
-   * Canary Artifacts in S3 encryption configuration.
-   * Artifact encryption is only supported for Node.js runtime.
+   * Canary Artifacts in S3 encryption mode.
+   * Artifact encryption is only supported for canaries that use Synthetics runtime
+   * version `syn-nodejs-puppeteer-3.3` or later.
    *
    * @default - Artifacts are encrypted at rest using an AWS managed key
    *
    * @see https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/CloudWatch_Synthetics_artifact_encryption.html
    */
-  readonly artifactS3Encryption?: ArtifactS3Encryption;
-}
-
-/**
- * Canary Artifacts in S3 encryption configuration.
- */
-export interface ArtifactS3Encryption {
-  /**
-    * Encryption mode.
-    * @default - Artifacts are encrypted at rest using an AWS managed key
-    */
-  readonly encryptionMode?: EncryptionMode;
+  readonly artifactS3EncryptionMode?: ArtifactsEncryptionMode;
 
   /**
    * The KMS key to be used to encrypt the data.
-   * @default - no kms key if mode = S3_MANAGED.
-   * A key will be created if one is not provided and mode = KMS.
+   *
+   * @default - no kms key if mode = S3_MANAGED. A key will be created if one is not provided and mode = KMS.
    */
   readonly kmsKey?: kms.IKey;
 }
 
 /**
  * Encryption mode for canary artifacts.
  */
-export enum EncryptionMode {
+export enum ArtifactsEncryptionMode {
   /**
    * Server-side encryption (SSE) with an Amazon S3-managed key.
-   *
    */
   S3_MANAGED = 'SSE_S3',
 
   /**
    * Server-side encryption (SSE) with an AWS KMS customer managed key.
-   *
    */
   KMS = 'SSE_KMS',
 }
@@ -347,29 +335,16 @@ export class Canary extends cdk.Resource implements ec2.IConnectable {
       this._connections = new ec2.Connections({});
     }
 
-    if (props.runtime.family !== RuntimeFamily.NODEJS && props.artifactS3Encryption) {
-      throw new Error('Artifact encryption is only supported for Node.js runtime.');
-    }
-
-    if (props.artifactS3Encryption?.encryptionMode !== EncryptionMode.KMS && props.artifactS3Encryption?.kmsKey) {
+    if (!cdk.Token.isUnresolved(props.artifactS3EncryptionMode) &&
+      props.artifactS3EncryptionMode !== ArtifactsEncryptionMode.KMS &&
+      !cdk.Token.isUnresolved(props.kmsKey) &&
+      props.kmsKey) {
       throw new Error('A customer-managed KMS key was provided, but the encryption mode is not set to SSE-KMS.');
     }
 
-    let encryptionKey;
-    if (props.artifactS3Encryption?.encryptionMode === EncryptionMode.KMS) {
-      encryptionKey = props.artifactS3Encryption?.kmsKey ?? new kms.Key(this, 'Key', { description: `Created by ${this.node.path}` });
-    }
-
-    encryptionKey?.grantEncryptDecrypt(this.role);
-
-    let artifactConfig;
-    if (props.artifactS3Encryption) {
-      artifactConfig = {
-        s3Encryption: {
-          encryptionMode: props.artifactS3Encryption.encryptionMode,
-          kmsKeyArn: encryptionKey?.keyArn ?? undefined,
-        },
-      };
+    if (props.artifactS3EncryptionMode === ArtifactsEncryptionMode.KMS) {
+      // Kms Key is set or generated for using `createArtifactConfig`
+      this.encryptionKey = props.kmsKey ?? new kms.Key(this, 'Key', { description: `Created by ${this.node.path}` });
     }
 
     const resource: CfnCanary = new CfnCanary(this, 'Resource', {
@@ -384,16 +359,14 @@ export class Canary extends cdk.Resource implements ec2.IConnectable {
       code: this.createCode(props),
       runConfig: this.createRunConfig(props),
       vpcConfig: this.createVpcConfig(props),
-      artifactConfig,
+      artifactConfig: this.createArticatConfig(props),
     });
     this._resource = resource;
 
     this.canaryId = resource.attrId;
     this.canaryState = resource.attrState;
     this.canaryName = this.getResourceNameAttribute(resource.ref);
 
-    this.encryptionKey = encryptionKey;
-
     if (props.cleanup === Cleanup.LAMBDA) {
       this.cleanupUnderlyingResources();
     }
@@ -643,6 +616,29 @@ export class Canary extends cdk.Resource implements ec2.IConnectable {
     };
   }
 
+  private createArticatConfig(props: CanaryProps): CfnCanary.ArtifactConfigProperty | undefined {
+    if (!props.artifactS3EncryptionMode) {
+      return undefined;
+    }
+
+    const isNodeRuntime = !cdk.Token.isUnresolved(props.runtime) && props.runtime.family === RuntimeFamily.NODEJS;
+    const isArtifactS3EncryptionModeDefined = !cdk.Token.isUnresolved(props.artifactS3EncryptionMode) && props.artifactS3EncryptionMode;
+
+    // Only check runtime family is nodejs because versions prior to syn-nodejs-puppeteer-3.3 are deprecated and can no longer be configured.
+    if (!isNodeRuntime && isArtifactS3EncryptionModeDefined) {
+      throw new Error(`Artifact encryption is only supported for canaries that use Synthetics runtime version syn-nodejs-puppeteer-3.3 or later, got ${props.runtime.name}.`);
+    }
+
+    this.encryptionKey?.grantEncryptDecrypt(this.role);
+
+    return {
+      s3Encryption: {
+        encryptionMode: props.artifactS3EncryptionMode,
+        kmsKeyArn: this.encryptionKey?.keyArn ?? undefined,
+      },
+    };
+  }
+
   /**
    * Creates a unique name for the canary. The generated name is the physical ID of the canary.
    */

diff --git a/packages/aws-cdk-lib/aws-synthetics/test/canary.test.ts b/packages/aws-cdk-lib/aws-synthetics/test/canary.test.ts
@@ -804,9 +804,7 @@ describe('artifact encryption test', () => {
           };`),
       }),
       runtime: synthetics.Runtime.SYNTHETICS_NODEJS_PUPPETEER_7_0,
-      artifactS3Encryption: {
-        encryptionMode: synthetics.EncryptionMode.S3_MANAGED,
-      },
+      artifactS3EncryptionMode: synthetics.ArtifactsEncryptionMode.S3_MANAGED,
     });
 
     // THEN
@@ -834,9 +832,7 @@ describe('artifact encryption test', () => {
           };`),
       }),
       runtime: synthetics.Runtime.SYNTHETICS_NODEJS_PUPPETEER_7_0,
-      artifactS3Encryption: {
-        encryptionMode: synthetics.EncryptionMode.KMS,
-      },
+      artifactS3EncryptionMode: synthetics.ArtifactsEncryptionMode.KMS,
     });
 
     Template.fromStack(stack).hasResourceProperties('AWS::KMS::Key', {
@@ -869,10 +865,8 @@ describe('artifact encryption test', () => {
           };`),
       }),
       runtime: synthetics.Runtime.SYNTHETICS_NODEJS_PUPPETEER_7_0,
-      artifactS3Encryption: {
-        encryptionMode: synthetics.EncryptionMode.KMS,
-        kmsKey: key,
-      },
+      artifactS3EncryptionMode: synthetics.ArtifactsEncryptionMode.KMS,
+      kmsKey: key,
     });
 
     // THEN
@@ -897,10 +891,8 @@ describe('artifact encryption test', () => {
           code: synthetics.Code.fromAsset(path.join(__dirname, 'canaries')),
         }),
         runtime: synthetics.Runtime.SYNTHETICS_NODEJS_PUPPETEER_7_0,
-        artifactS3Encryption: {
-          encryptionMode: synthetics.EncryptionMode.S3_MANAGED,
-          kmsKey: key,
-        },
+        artifactS3EncryptionMode: synthetics.ArtifactsEncryptionMode.S3_MANAGED,
+        kmsKey: key,
       });
     }).toThrow('A customer-managed KMS key was provided, but the encryption mode is not set to SSE-KMS.');
   });
@@ -915,10 +907,8 @@ describe('artifact encryption test', () => {
           handler: 'index.handler',
           code: synthetics.Code.fromInline('# Synthetics handler code'),
         }),
-        artifactS3Encryption: {
-          encryptionMode: synthetics.EncryptionMode.S3_MANAGED,
-        },
+        artifactS3EncryptionMode: synthetics.ArtifactsEncryptionMode.S3_MANAGED,
       });
-    }).toThrow('Artifact encryption is only supported for Node.js runtime.');
+    }).toThrow('Artifact encryption is only supported for canaries that use Synthetics runtime version syn-nodejs-puppeteer-3.3 or later, got syn-python-selenium-3.0.');
   });
 });