Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix: incorporate review comments
Browse files Browse the repository at this point in the history
mazyu36 committed Jun 17, 2024

Verified

This commit was signed with the committer’s verified signature.
addaleax Anna Henningsen
1 parent 6b0b467 commit 6f06849
Showing 4 changed files with 32 additions and 30 deletions.
Original file line number Diff line number Diff line change
@@ -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', {
2 changes: 1 addition & 1 deletion packages/aws-cdk-lib/aws-synthetics/README.md
Original file line number Diff line number Diff line change
@@ -271,6 +271,6 @@ const canary = new synthetics.Canary(this, 'MyCanary', {
expiration: Duration.days(30),
}],
artifactS3EncryptionMode: synthetics.ArtifactsEncryptionMode.KMS,
kmsKey: key,
artifactS3KmsKey: key,
});
```
41 changes: 19 additions & 22 deletions packages/aws-cdk-lib/aws-synthetics/lib/canary.ts
Original file line number Diff line number Diff line change
@@ -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
*
@@ -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,
@@ -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;

@@ -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,
},
};
}
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
@@ -844,7 +844,12 @@ describe('artifact encryption test', () => {
ArtifactConfig: {
S3Encryption: {
EncryptionMode: 'SSE_KMS',
KmsKeyArn: stack.resolve(canary.encryptionKey?.keyArn),
KmsKeyArn: {
'Fn::GetAtt': [
'CanaryKey36A631B4',
'Arn',
],
},
},
},
});
@@ -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
@@ -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.');
});
@@ -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 6f06849

Please sign in to comment.