feat(ec2): support KMS keys for block device mappings for both instances and launch templates#18326
Conversation
|
|
|
Title does not follow the guidelines of Conventional Commits. Please adjust title before merge. |
| * Properties of an EBS block device | ||
| */ | ||
| export interface EbsDeviceProps extends EbsDeviceSnapshotOptions { | ||
| export interface EbsDeviceProps extends EbsDeviceSnapshotOptions, EbsDeviceOptions { |
There was a problem hiding this comment.
Is this necessary and/or intentional?
It feels like omitting the encrypted property from the EbsDeviceProps was an intentional decision, so I'm a little wary of undoing it. Any details you can provide here?
There was a problem hiding this comment.
I consider this a bug or it was unintentionally, or left over from early days.
The class BlockDeviceVolume offers two ebs related static methods:
public static ebsFromSnapshot(snapshotId: string, options: EbsDeviceSnapshotOptions = {}): BlockDeviceVolume {
public static ebs(volumeSize: number, options: EbsDeviceOptions = {}): BlockDeviceVolume {
both call the constructor of the class
* @param ebsDevice EBS device info
* @param virtualName Virtual device name
*/
protected constructor(public readonly ebsDevice?: EbsDeviceProps, public readonly virtualName?: string) {
}
I thought extending the EbsDeviceProps that are only used within the protected constructor would be the right thing to do. Because both SnapshotOptions and DeviceOptions are the properties of the ebs volume, but you can't specify them at the same time (but this is handled by the mentioned static methods).
So yes, it was intentionally to change this, in order to get kms encryption for device mappings working.
Apart from that, this prop is reused here:
If I don't extend the props, I could not access the encryption properties.
But as already said: The static methods ebs and ebsFromSnapshot take care of not combining incompatible properties.
…hub.com/markussiebert/aws-cdk into feat/block-device-mappings-with-kms-cmk
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
…ces and launch templates (aws#18326) This pullrequest will add the optional ```kmsKeyId``` property to the ```EbsDeviceOptions``` Interface. Whit this, it will be possible to specify the kmsKeyId used for encrypting the ebs volumes when launching instances. At the moment I already use this via an escape hatch in my projects, but it's not that handy as the block device mapping is an array. I don't like to only specify a kmsKeyId (= ARN) but accepting an ```kms.IKey``` in the properties would be a bigger change. fixes: aws#18309 *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
This pullrequest will add the optional
kmsKeyIdproperty to theEbsDeviceOptionsInterface. Whit this, it will be possible to specify the kmsKeyId used for encrypting the ebs volumes when launching instances. At the moment I already use this via an escape hatch in my projects, but it's not that handy as the block device mapping is an array.I don't like to only specify a kmsKeyId (= ARN) but accepting an
kms.IKeyin the properties would be a bigger change.fixes: #18309
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license