feat(bootstrap): add kms-key-id option to cdk bootstrap command #2245
Conversation
|
Is it possible to see why the build failed? |
|
Seems failed due to an ongoing issue we're experiencing with the |
|
Still need to make a change to the code in case there is no key. And add a test case. Haven't had the time yet. |
|
That's cool. I was just making sure this PR does grow excessively stale :) |
|
@abelmokadem are you till intending to push further commits on there? If you're unable to, it'd be nice if you can lay out the remaining work you intend to be done before this is merge-able. |
|
Hi guys, I have been quite busy with work and our new born daughter :( Sorry I can't pick this up any further. Still hoping to see this feature in a future release. |
|
Can you outline what is missing? I can try to pick up the PR. |
| Properties: { | ||
| AccessControl: "Private", | ||
| BucketEncryption: { ServerSideEncryptionConfiguration: [{ ServerSideEncryptionByDefault: { SSEAlgorithm: "aws:kms" } }] } | ||
| AccessControl: 'Private', |
There was a problem hiding this comment.
@hoegertn there is no condition here for when no kmsKeyId is provided.
There was a problem hiding this comment.
right, you'd want to default that to a KMS managed server-side encryption
| @@ -57,6 +57,7 @@ async function parseCommandLineArguments() { | |||
| .option('output', { type: 'string', alias: 'o', desc: 'write CloudFormation template for requested stacks to the given directory', requiresArg: true }) | |||
| .option('numbered', { type: 'boolean', alias: 'n', desc: 'prefix filenames with numbers to indicate deployment ordering' })) | |||
| .command('bootstrap [ENVIRONMENTS..]', 'Deploys the CDK toolkit stack into an AWS environment', yargs => yargs | |||
| .option('toolkit-bucket-encryption-key-id', { type: 'string', alias: 'k', desc: 'AWS KMS master key ID used for the SSE-KMS encryption', default: 'aws:kms' }) | |||
There was a problem hiding this comment.
-
I think the default should be no key right / a KMS managed key right? - S3 Bucket Encryption options for reference.
aws:kmsis not quite the default of selecting this option. -
I'd prefer if we didn't add an alias as I don't think it's intuitive that
kwould map to the bootstrap bucket's KMS key ID
| readonly bucketName?: string; | ||
|
|
||
| /** | ||
| * The ID of an existing KMS key to be used for encrypting items in the bucket. |
There was a problem hiding this comment.
by existing, do we mean a user managed KMS key?
| Properties: { | ||
| AccessControl: "Private", | ||
| BucketEncryption: { ServerSideEncryptionConfiguration: [{ ServerSideEncryptionByDefault: { SSEAlgorithm: "aws:kms" } }] } | ||
| AccessControl: 'Private', |
There was a problem hiding this comment.
right, you'd want to default that to a KMS managed server-side encryption
Closes #2203
Pull Request Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.