feat(ec2): `Volume` construct #8219

ddneilson · 2020-05-27T02:35:18Z

This adds an L2 construct for AWS::EC2:Volume that supports encryption with a customer-owned KMS key, or service-owned key. It provides methods for importing an existing Volume to the stack, and for granting AttachVolume and DetachVolume to a role.

Resolves #8218

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

ddneilson · 2020-05-27T03:19:22Z

Testing

Aside from a pile of unit tests, I also deployed the following and used the AWS CLI to attach/detach the volume. With the Volume attached, I also mounted it, formatted, and wrote/read data to/from it to ensure that the instance could write/read the Volume.

import * as cdk from '@aws-cdk/core';
import * as kms from '@aws-cdk/aws-kms';
import * as ec2 from '@aws-cdk/aws-ec2';
import { EbsDeviceVolumeType } from '@aws-cdk/aws-ec2';

const app = new cdk.App();

const env = {
    account: process.env.CDK_DEFAULT_ACCOUNT,
    region: process.env.CDK_DEFAULT_REGION
}

const network = new cdk.Stack(app, 'CDKTestNetwork', { env });
const vpc = new ec2.Vpc(network, 'Vpc');
const key = new kms.Key(network, 'Key', {
    removalPolicy: cdk.RemovalPolicy.DESTROY,
});

const testing = new cdk.Stack(app, 'CDKTestTest', { env });
const instance = new ec2.BastionHostLinux(testing, 'MountInstance', {
    vpc,
    instanceType: new ec2.InstanceType('t3.small'),
    subnetSelection: {
        subnetType: ec2.SubnetType.PRIVATE,
    },
});
const bastion = new ec2.BastionHostLinux(testing, 'Bastion', {
    vpc,
    instanceType: new ec2.InstanceType('t3.small'),
    subnetSelection: {
        subnetType: ec2.SubnetType.PRIVATE,
    },
});

const volume1 = new ec2.Volume(testing, 'Volume', {
    availabilityZone: `${env.region}a`,
    sizeGiB: 8,
    encrypted: true,
    encryptionKey: key,
    autoEnableIo: true,
    volumeType: EbsDeviceVolumeType.GENERAL_PURPOSE_SSD,
})
volume1.grantAttachVolume(bastion, instance);
volume1.grantDetachVolume(bastion, instance);

app.synth();

aws-cdk-automation · 2020-05-27T03:24:05Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: ba3f1d8
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-05-27T16:12:34Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 5c26dd8
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

ddneilson · 2020-06-04T16:36:14Z

@rix0rrr This is ready & waiting for your feedback.

packages/@aws-cdk/aws-ec2/lib/volume.ts

rix0rrr · 2020-06-05T15:03:05Z

packages/@aws-cdk/aws-ec2/lib/volume.ts

+  public abstract readonly encryptionKey?: IKey;
+
+  public grantAttachVolume(grantee: IGrantable, instance?: IInstance): Grant {


Is IGrantable here ever expected to be anything other than an instance role?

In fact, is this ever anything other than an instance you would want to grant this to?

Also, in what cases would you want to grant attach separately from detach?

Lack of imagination on my part seems safer to assume that there may be use-cases outside of an instance wanting to do the attach/detach. For instance, maybe there's a reason to have a lambda that's driving some external logic around moving/sharing an EBS volume between instances.

edit: Along that line of thought... maybe the instance arg should be an array of instances...

Also, in what cases would you want to grant attach separately from detach?

Yeah, one of the uses that I have for this is exactly that case. It's an ASG instance that will re-attach an EBS volume if it's replaced by the ASG, but never needs to detach it. The detachment happens automatically when the instance is terminated.

packages/@aws-cdk/aws-ec2/README.md

packages/@aws-cdk/aws-ec2/lib/volume.ts

Pull request has been modified.

ddneilson · 2020-06-06T21:47:58Z

@rix0rrr All set for another round of review.

aws-cdk-automation · 2020-06-06T22:11:26Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: fb2fd78
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

packages/@aws-cdk/aws-ec2/lib/volume.ts

ddneilson · 2020-06-18T05:09:11Z

@rix0rrr This is now ready for, hopefully, final review. I've sorted out the IAM permissions for the KMS key used for encryption. Access to the EBS volume & the KMS key is now properly separated -- you cannot attach the volume without access to the KMS key.

aws-cdk-automation · 2020-06-18T05:54:26Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: f5b045c
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-06-18T20:45:19Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: f741667
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

rix0rrr · 2020-06-24T14:59:24Z

packages/@aws-cdk/aws-ec2/README.md

+
+The following is a sample skeleton of a script that can be used to attach a Volume to the Linux instance that it is running on:
+
+```bash


Can you turn this example into an instance.userData.addCommand()-style example?

People would benefit from the automatic interpolation of EBS_VOL_ID, for example.

packages/@aws-cdk/aws-ec2/README.md

This changes the permissions appended to the KMS key policy such they are as minimal as possible. Permissions are split between a grant when creating the volume -- a grant that allows the Volume to be created -- and a grant when allowing attachment of the Volume. This properly restricts use of the CMK-encrypted volume to only those users that have access to both the EBS volume and, separately, the CMK used for encryption.

aws-cdk-automation · 2020-06-25T06:01:15Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 7b7522d
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

ddneilson · 2020-06-25T13:44:22Z

packages/@aws-cdk/aws-ec2/lib/volume.ts

+   *                     Defaults to a hash calculated from this volume.
+   */
+  grantAttachVolumeToSelf(instance: Instance, tagKeySuffix?: string): Grant;


I'm thinking that I should change this, and similarly the corresponding grantDetachVolumeFromSelf(), to grantAttachVolumeByTag(grantee: IGrantable, constructs: Construct[], tagKeySuffix?: string): Grant

Reason being that there are other instance-sources that this will not cover as it's implemented -- ASGs, EC2-Fleets, and SpotFleetRequests to name a few.

Implementation would be, essentially, the same except that the tag value would be applied to all given constructs.

rix0rrr · 2020-06-25T13:50:14Z

packages/@aws-cdk/aws-ec2/lib/volume.ts

Can you explain in a comment why the tag needs to be applied on this? That one surpised me.

For the other, don't forget to set { appliesToLaunchedInstances: true } (or whatever the option is), otherwise this still won't work for ASGs.

Can you explain in a comment why the tag needs to be applied on this? That one surpised me.

Sure. It's a resource tag. So, all resources involved in the call must have the tag.

For the other, don't forget to set { appliesToLaunchedInstances: true } (or whatever the option is), otherwise this still won't work for ASGs.

Fortunately, it's true by default: https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_core.TagProps.html#applytolaunchedinstances

aws-cdk-automation · 2020-06-25T18:38:05Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 47412bb
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-06-25T18:40:47Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 2da39bf
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2020-06-26T07:10:32Z

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-cdk-automation · 2020-06-26T07:57:26Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 17128cf
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2020-06-26T07:57:45Z

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).

SomayaB assigned rix0rrr May 27, 2020

ddneilson force-pushed the ebs_L2 branch from ba3f1d8 to 5c26dd8 Compare May 27, 2020 15:21

rix0rrr previously requested changes Jun 5, 2020

View reviewed changes

ddneilson force-pushed the ebs_L2 branch from 5c26dd8 to fb2fd78 Compare June 6, 2020 21:24

ddneilson commented Jun 9, 2020

View reviewed changes

packages/@aws-cdk/aws-ec2/lib/volume.ts Show resolved Hide resolved

ddneilson force-pushed the ebs_L2 branch from fb2fd78 to f5b045c Compare June 18, 2020 05:06

rix0rrr reviewed Jun 24, 2020

View reviewed changes

ddneilson added 4 commits June 25, 2020 05:16

feat(ec2): Adds an L2 construct for AWS::EC2::Volume

33f1128

Adds tests for Key.fromKeyArn() & snapshot handling

b7bfcbb

Adds separate methods for self-refrencing grant

7b7522d

ddneilson force-pushed the ebs_L2 branch from f741667 to 7b7522d Compare June 25, 2020 05:17

ddneilson commented Jun 25, 2020

View reviewed changes

rix0rrr reviewed Jun 25, 2020

View reviewed changes

ddneilson added 2 commits June 25, 2020 17:55

Adds grantAttachVolumeByResourceTag/grantDetachVolumeByResourceTag

47412bb

Missed updates to the README.

2da39bf

rix0rrr approved these changes Jun 26, 2020

View reviewed changes

rix0rrr changed the title ~~feat(ec2): Adds an L2 construct for AWS::EC2::Volume~~ feat(ec2): Volume construct Jun 26, 2020

Merge branch 'master' into ebs_L2

17128cf

mergify bot merged commit 7490dee into aws:master Jun 26, 2020

ddneilson mentioned this pull request Jul 15, 2020

[ec2] Volume construct throws with fresh CDK app #9081

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ec2): `Volume` construct #8219

feat(ec2): `Volume` construct #8219

ddneilson commented May 27, 2020 •

edited

ddneilson commented May 27, 2020

aws-cdk-automation commented May 27, 2020

aws-cdk-automation commented May 27, 2020

ddneilson commented Jun 4, 2020

rix0rrr Jun 5, 2020

ddneilson Jun 5, 2020 •

edited

ddneilson commented Jun 6, 2020

aws-cdk-automation commented Jun 6, 2020

ddneilson commented Jun 18, 2020

aws-cdk-automation commented Jun 18, 2020

aws-cdk-automation commented Jun 18, 2020

rix0rrr Jun 24, 2020

ddneilson Jun 25, 2020

aws-cdk-automation commented Jun 25, 2020

ddneilson Jun 25, 2020

rix0rrr Jun 25, 2020

ddneilson Jun 25, 2020

aws-cdk-automation commented Jun 25, 2020

aws-cdk-automation commented Jun 25, 2020

mergify bot commented Jun 26, 2020

aws-cdk-automation commented Jun 26, 2020

mergify bot commented Jun 26, 2020

		public abstract readonly encryptionKey?: IKey;

		public grantAttachVolume(grantee: IGrantable, instance?: IInstance): Grant {


		The following is a sample skeleton of a script that can be used to attach a Volume to the Linux instance that it is running on:

		```bash

feat(ec2): Volume construct #8219

feat(ec2): Volume construct #8219

Conversation

ddneilson commented May 27, 2020 • edited

ddneilson commented May 27, 2020

Testing

aws-cdk-automation commented May 27, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented May 27, 2020

AWS CodeBuild CI Report

ddneilson commented Jun 4, 2020

rix0rrr Jun 5, 2020

Choose a reason for hiding this comment

ddneilson Jun 5, 2020 • edited

Choose a reason for hiding this comment

ddneilson commented Jun 6, 2020

aws-cdk-automation commented Jun 6, 2020

AWS CodeBuild CI Report

ddneilson commented Jun 18, 2020

aws-cdk-automation commented Jun 18, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Jun 18, 2020

AWS CodeBuild CI Report

rix0rrr Jun 24, 2020

Choose a reason for hiding this comment

ddneilson Jun 25, 2020

Choose a reason for hiding this comment

aws-cdk-automation commented Jun 25, 2020

AWS CodeBuild CI Report

ddneilson Jun 25, 2020

Choose a reason for hiding this comment

rix0rrr Jun 25, 2020

Choose a reason for hiding this comment

ddneilson Jun 25, 2020

Choose a reason for hiding this comment

aws-cdk-automation commented Jun 25, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Jun 25, 2020

AWS CodeBuild CI Report

mergify bot commented Jun 26, 2020

aws-cdk-automation commented Jun 26, 2020

AWS CodeBuild CI Report

mergify bot commented Jun 26, 2020

feat(ec2): `Volume` construct #8219

feat(ec2): `Volume` construct #8219

ddneilson commented May 27, 2020 •

edited

ddneilson Jun 5, 2020 •

edited