Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ec2): Volume grants have an overly complicated API #9115

Merged
merged 2 commits into from
Aug 10, 2020
Merged

fix(ec2): Volume grants have an overly complicated API #9115

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fd4eefd
fix(ec2): Fixes design flaw with Volume grant by ResourceTag
Jul 16, 2020
bdfd60a
Merge branch 'master' into fix_ebs_2
mergify[bot] Aug 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 6 additions & 15 deletions packages/@aws-cdk/aws-ec2/lib/volume.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,14 +290,10 @@ export interface IVolume extends IResource {
* given the ability to AttachVolume if both the Volume and the destination Instance have that
* tag applied to them.
*
* If you need to call this method multiple times on different sets of constructs, then provide a
* unique `tagKeySuffix` for each call; failure to do so will result in an inability to attach this
* volume to some of the grants because it will overwrite the tag.
*
* @param grantee the principal being granted permission.
* @param constructs The list of constructs that will have the generated resource tag applied to them.
* @param tagKeySuffix A suffix to use on the generated Tag key in place of the generated hash value.
* Defaults to a hash calculated from this volume.
* Defaults to a hash calculated from this volume and list of constructs. (DEPRECATED)
*/
grantAttachVolumeByResourceTag(grantee: IGrantable, constructs: Construct[], tagKeySuffix?: string): Grant;

Expand All @@ -324,7 +320,7 @@ export interface IVolume extends IResource {
* @param grantee the principal being granted permission.
* @param constructs The list of constructs that will have the generated resource tag applied to them.
* @param tagKeySuffix A suffix to use on the generated Tag key in place of the generated hash value.
* Defaults to a hash calculated from this volume.
* Defaults to a hash calculated from this volume and list of constructs. (DEPRECATED)
*/
grantDetachVolumeByResourceTag(grantee: IGrantable, constructs: Construct[], tagKeySuffix?: string): Grant;
}
Expand Down Expand Up @@ -497,8 +493,8 @@ abstract class VolumeBase extends Resource implements IVolume {
}

public grantAttachVolumeByResourceTag(grantee: IGrantable, constructs: Construct[], tagKeySuffix?: string): Grant {
const tagKey = `VolumeGrantAttach-${tagKeySuffix ?? this.stringHash(this.node.uniqueId)}`;
const tagValue = this.calculateResourceTagValue(constructs);
const tagValue = this.calculateResourceTagValue([this, ...constructs]);
const tagKey = `VolumeGrantAttach-${tagKeySuffix ?? tagValue.slice(0,10).toUpperCase()}`;
const grantCondition: { [key: string]: string } = {};
grantCondition[`ec2:ResourceTag/${tagKey}`] = tagValue;

Expand Down Expand Up @@ -526,8 +522,8 @@ abstract class VolumeBase extends Resource implements IVolume {
}

public grantDetachVolumeByResourceTag(grantee: IGrantable, constructs: Construct[], tagKeySuffix?: string): Grant {
const tagKey = `VolumeGrantDetach-${tagKeySuffix ?? this.stringHash(this.node.uniqueId)}`;
const tagValue = this.calculateResourceTagValue(constructs);
const tagValue = this.calculateResourceTagValue([this, ...constructs]);
const tagKey = `VolumeGrantDetach-${tagKeySuffix ?? tagValue.slice(0,10).toUpperCase()}`;
const grantCondition: { [key: string]: string } = {};
grantCondition[`ec2:ResourceTag/${tagKey}`] = tagValue;

Expand Down Expand Up @@ -558,11 +554,6 @@ abstract class VolumeBase extends Resource implements IVolume {
return resourceArns;
}

private stringHash(value: string): string {
const md5 = crypto.createHash('md5').update(value).digest('hex');
return md5.slice(0, 8).toUpperCase();
}

private calculateResourceTagValue(constructs: Construct[]): string {
const md5 = crypto.createHash('md5');
constructs.forEach(construct => md5.update(construct.node.uniqueId));
Expand Down
34 changes: 16 additions & 18 deletions packages/@aws-cdk/aws-ec2/test/volume.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -739,7 +739,7 @@ nodeunitShim({
],
Condition: {
'ForAnyValue:StringEquals': {
'ec2:ResourceTag/VolumeGrantAttach-BD7A9717': 'd9a17c1c9e8ef6866e4dbeef41c741b2',
'ec2:ResourceTag/VolumeGrantAttach-B2376B2BDA': 'b2376b2bda65cb40f83c290dd844c4aa',
},
},
}],
Expand All @@ -748,17 +748,17 @@ nodeunitShim({
cdkExpect(stack).to(haveResourceLike('AWS::EC2::Volume', {
Tags: [
{
Key: 'VolumeGrantAttach-BD7A9717',
Value: 'd9a17c1c9e8ef6866e4dbeef41c741b2',
Key: 'VolumeGrantAttach-B2376B2BDA',
Value: 'b2376b2bda65cb40f83c290dd844c4aa',
},
],
}, ResourcePart.Properties));
cdkExpect(stack).to(haveResourceLike('AWS::EC2::Instance', {
Tags: [
{},
{
Key: 'VolumeGrantAttach-BD7A9717',
Value: 'd9a17c1c9e8ef6866e4dbeef41c741b2',
Key: 'VolumeGrantAttach-B2376B2BDA',
Value: 'b2376b2bda65cb40f83c290dd844c4aa',
},
],
}, ResourcePart.Properties));
Expand Down Expand Up @@ -816,7 +816,7 @@ nodeunitShim({
],
Condition: {
'ForAnyValue:StringEquals': {
'ec2:ResourceTag/VolumeGrantAttach-TestSuffix': 'd9a17c1c9e8ef6866e4dbeef41c741b2',
'ec2:ResourceTag/VolumeGrantAttach-TestSuffix': 'b2376b2bda65cb40f83c290dd844c4aa',
},
},
}],
Expand All @@ -826,7 +826,7 @@ nodeunitShim({
Tags: [
{
Key: 'VolumeGrantAttach-TestSuffix',
Value: 'd9a17c1c9e8ef6866e4dbeef41c741b2',
Value: 'b2376b2bda65cb40f83c290dd844c4aa',
},
],
}, ResourcePart.Properties));
Expand All @@ -835,11 +835,10 @@ nodeunitShim({
{},
{
Key: 'VolumeGrantAttach-TestSuffix',
Value: 'd9a17c1c9e8ef6866e4dbeef41c741b2',
Value: 'b2376b2bda65cb40f83c290dd844c4aa',
},
],
}, ResourcePart.Properties));

test.done();
},

Expand Down Expand Up @@ -1051,7 +1050,7 @@ nodeunitShim({
],
Condition: {
'ForAnyValue:StringEquals': {
'ec2:ResourceTag/VolumeGrantDetach-BD7A9717': 'd9a17c1c9e8ef6866e4dbeef41c741b2',
'ec2:ResourceTag/VolumeGrantDetach-B2376B2BDA': 'b2376b2bda65cb40f83c290dd844c4aa',
},
},
}],
Expand All @@ -1060,17 +1059,17 @@ nodeunitShim({
cdkExpect(stack).to(haveResourceLike('AWS::EC2::Volume', {
Tags: [
{
Key: 'VolumeGrantDetach-BD7A9717',
Value: 'd9a17c1c9e8ef6866e4dbeef41c741b2',
Key: 'VolumeGrantDetach-B2376B2BDA',
Value: 'b2376b2bda65cb40f83c290dd844c4aa',
},
],
}, ResourcePart.Properties));
cdkExpect(stack).to(haveResourceLike('AWS::EC2::Instance', {
Tags: [
{},
{
Key: 'VolumeGrantDetach-BD7A9717',
Value: 'd9a17c1c9e8ef6866e4dbeef41c741b2',
Key: 'VolumeGrantDetach-B2376B2BDA',
Value: 'b2376b2bda65cb40f83c290dd844c4aa',
},
],
}, ResourcePart.Properties));
Expand Down Expand Up @@ -1128,7 +1127,7 @@ nodeunitShim({
],
Condition: {
'ForAnyValue:StringEquals': {
'ec2:ResourceTag/VolumeGrantDetach-TestSuffix': 'd9a17c1c9e8ef6866e4dbeef41c741b2',
'ec2:ResourceTag/VolumeGrantDetach-TestSuffix': 'b2376b2bda65cb40f83c290dd844c4aa',
},
},
}],
Expand All @@ -1138,7 +1137,7 @@ nodeunitShim({
Tags: [
{
Key: 'VolumeGrantDetach-TestSuffix',
Value: 'd9a17c1c9e8ef6866e4dbeef41c741b2',
Value: 'b2376b2bda65cb40f83c290dd844c4aa',
},
],
}, ResourcePart.Properties));
Expand All @@ -1147,11 +1146,10 @@ nodeunitShim({
{},
{
Key: 'VolumeGrantDetach-TestSuffix',
Value: 'd9a17c1c9e8ef6866e4dbeef41c741b2',
Value: 'b2376b2bda65cb40f83c290dd844c4aa',
},
],
}, ResourcePart.Properties));

test.done();
},

Expand Down