Skip to content

Commit

Permalink
fix: incorporate review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
mazyu36 committed Jun 17, 2024
1 parent fd44f6f commit 5f26026
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ new Canary(stack, 'CanarySseKmsWith', {
runtime: Runtime.SYNTHETICS_NODEJS_PUPPETEER_7_0,
cleanup: Cleanup.LAMBDA,
artifactS3EncryptionMode: ArtifactsEncryptionMode.KMS,
kmsKey: encryptKey,
artifactS3KmsKey: encryptKey,
});

new IntegTest(app, 'IntegCanaryTest', {
Expand Down
2 changes: 1 addition & 1 deletion packages/aws-cdk-lib/aws-synthetics/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,6 @@ const canary = new synthetics.Canary(this, 'MyCanary', {
expiration: Duration.days(30),
}],
artifactS3EncryptionMode: synthetics.ArtifactsEncryptionMode.KMS,
kmsKey: key,
artifactS3KmsKey: key,
});
```
47 changes: 22 additions & 25 deletions packages/aws-cdk-lib/aws-synthetics/lib/canary.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,11 +243,11 @@ export interface CanaryProps {
readonly artifactS3EncryptionMode?: ArtifactsEncryptionMode;

/**
* The KMS key to be used to encrypt the data.
* The KMS key used to encrypt canary artifacts.
*
* @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 `artifactS3EncryptionMode` is set to `S3_MANAGED`. A key will be created if one is not provided and `artifactS3EncryptionMode` is set to `KMS`.
*/
readonly kmsKey?: kms.IKey;
readonly artifactS3KmsKey?: kms.IKey;
}

/**
Expand Down Expand Up @@ -297,11 +297,6 @@ export class Canary extends cdk.Resource implements ec2.IConnectable {
*/
public readonly artifactsBucket: s3.IBucket;

/**
* Optional KMS encryption key associated with this canary.
*/
public readonly encryptionKey?: kms.IKey;

/**
* Actual connections object for the underlying Lambda
*
Expand Down Expand Up @@ -335,18 +330,6 @@ export class Canary extends cdk.Resource implements ec2.IConnectable {
this._connections = new ec2.Connections({});
}

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.');
}

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', {
artifactS3Location: this.artifactsBucket.s3UrlForObject(props.artifactsBucketLocation?.prefix),
executionRoleArn: this.role.roleArn,
Expand All @@ -359,7 +342,7 @@ export class Canary extends cdk.Resource implements ec2.IConnectable {
code: this.createCode(props),
runConfig: this.createRunConfig(props),
vpcConfig: this.createVpcConfig(props),
artifactConfig: this.createArticatConfig(props),
artifactConfig: this.createArtifactConfig(props),
});
this._resource = resource;

Expand Down Expand Up @@ -616,25 +599,39 @@ export class Canary extends cdk.Resource implements ec2.IConnectable {
};
}

private createArticatConfig(props: CanaryProps): CfnCanary.ArtifactConfigProperty | undefined {
private createArtifactConfig(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;
const isArtifactS3KmsKeyDefined = !cdk.Token.isUnresolved(props.artifactS3KmsKey) && props.artifactS3KmsKey;

if (isArtifactS3EncryptionModeDefined &&
props.artifactS3EncryptionMode !== ArtifactsEncryptionMode.KMS &&
isArtifactS3KmsKeyDefined
) {
throw new Error('A customer-managed KMS key was provided, but the encryption mode is not set to SSE-KMS.');
}

// 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}.`);
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}\`.`);
}

let encryptionKey: kms.IKey | undefined;
if (props.artifactS3EncryptionMode === ArtifactsEncryptionMode.KMS) {
// Kms Key is set or generated for using `createArtifactConfig`
encryptionKey = props.artifactS3KmsKey ?? new kms.Key(this, 'Key', { description: `Created by ${this.node.path}` });
}

this.encryptionKey?.grantEncryptDecrypt(this.role);
encryptionKey?.grantEncryptDecrypt(this.role);

return {
s3Encryption: {
encryptionMode: props.artifactS3EncryptionMode,
kmsKeyArn: this.encryptionKey?.keyArn ?? undefined,
kmsKeyArn: encryptionKey?.keyArn,
},
};
}
Expand Down
17 changes: 11 additions & 6 deletions packages/aws-cdk-lib/aws-synthetics/test/canary.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -844,7 +844,12 @@ describe('artifact encryption test', () => {
ArtifactConfig: {
S3Encryption: {
EncryptionMode: 'SSE_KMS',
KmsKeyArn: stack.resolve(canary.encryptionKey?.keyArn),
KmsKeyArn: {
'Fn::GetAtt': [
'CanaryKey36A631B4',
'Arn',
],
},
},
},
});
Expand All @@ -866,7 +871,7 @@ describe('artifact encryption test', () => {
}),
runtime: synthetics.Runtime.SYNTHETICS_NODEJS_PUPPETEER_7_0,
artifactS3EncryptionMode: synthetics.ArtifactsEncryptionMode.KMS,
kmsKey: key,
artifactS3KmsKey: key,
});

// THEN
Expand All @@ -887,12 +892,12 @@ describe('artifact encryption test', () => {
expect(() => {
new synthetics.Canary(stack, 'Canary', {
test: synthetics.Test.custom({
handler: 'index.functionName',
code: synthetics.Code.fromAsset(path.join(__dirname, 'canaries')),
handler: 'index.handler',
code: synthetics.Code.fromInline('/* Synthetics handler code */'),
}),
runtime: synthetics.Runtime.SYNTHETICS_NODEJS_PUPPETEER_7_0,
artifactS3EncryptionMode: synthetics.ArtifactsEncryptionMode.S3_MANAGED,
kmsKey: key,
artifactS3KmsKey: key,
});
}).toThrow('A customer-managed KMS key was provided, but the encryption mode is not set to SSE-KMS.');
});
Expand All @@ -909,6 +914,6 @@ describe('artifact encryption test', () => {
}),
artifactS3EncryptionMode: synthetics.ArtifactsEncryptionMode.S3_MANAGED,
});
}).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.');
}).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`.');
});
});

0 comments on commit 5f26026

Please sign in to comment.