From 067da75d1dee20c142b86a6ed56eade2f052894c Mon Sep 17 00:00:00 2001 From: Mike Cowgill Date: Tue, 25 Sep 2018 00:55:38 -0700 Subject: [PATCH] feat(ec2): Add tag support to security groups (#766) --- .../aws-autoscaling/lib/auto-scaling-group.ts | 2 +- .../integ.asg-w-loadbalancer.expected.json | 7 ++ .../test/test.auto-scaling-group.ts | 6 ++ .../@aws-cdk/aws-ec2/lib/security-group.ts | 16 ++++- packages/@aws-cdk/aws-ec2/lib/vpc.ts | 2 +- packages/@aws-cdk/aws-ec2/test/test.vpc.ts | 68 +++++++++++-------- .../test/integ.elb.expected.json | 1 + .../test/integ.vpc-lambda.expected.json | 1 + packages/@aws-cdk/cdk/lib/core/tag-manager.ts | 8 +-- 9 files changed, 74 insertions(+), 37 deletions(-) diff --git a/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts b/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts index 2a3ced6f16e9c..48de5cf026fb5 100644 --- a/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts +++ b/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts @@ -468,7 +468,7 @@ function renderRollingUpdateConfig(config: RollingUpdateConfiguration = {}): cdk class TagManager extends cdk.TagManager { protected tagFormatResolve(tagGroups: cdk.TagGroups): any { - const tags = {...tagGroups.nonSitckyTags, ...tagGroups.ancestorTags, ...tagGroups.stickyTags}; + const tags = {...tagGroups.nonStickyTags, ...tagGroups.ancestorTags, ...tagGroups.stickyTags}; return Object.keys(tags).map( (key) => { const propagateAtLaunch = !!tagGroups.propagateTags[key] || !!tagGroups.ancestorTags[key]; return {key, value: tags[key], propagateAtLaunch}; diff --git a/packages/@aws-cdk/aws-autoscaling/test/integ.asg-w-loadbalancer.expected.json b/packages/@aws-cdk/aws-autoscaling/test/integ.asg-w-loadbalancer.expected.json index 2c79947b3b5f4..f777045972bc5 100644 --- a/packages/@aws-cdk/aws-autoscaling/test/integ.asg-w-loadbalancer.expected.json +++ b/packages/@aws-cdk/aws-autoscaling/test/integ.asg-w-loadbalancer.expected.json @@ -453,6 +453,12 @@ } ], "SecurityGroupIngress": [], + "Tags": [ + { + "Key": "Name", + "Value": "aws-cdk-ec2-integ/Fleet" + } + ], "VpcId": { "Ref": "VPCB9E5F0B4" } @@ -583,6 +589,7 @@ "ToPort": 80 } ], + "Tags": [], "VpcId": { "Ref": "VPCB9E5F0B4" } diff --git a/packages/@aws-cdk/aws-autoscaling/test/test.auto-scaling-group.ts b/packages/@aws-cdk/aws-autoscaling/test/test.auto-scaling-group.ts index c639715bf6c49..e15bfb46c9e10 100644 --- a/packages/@aws-cdk/aws-autoscaling/test/test.auto-scaling-group.ts +++ b/packages/@aws-cdk/aws-autoscaling/test/test.auto-scaling-group.ts @@ -33,6 +33,12 @@ export = { } ], "SecurityGroupIngress": [], + "Tags": [ + { + "Key": "Name", + "Value": "MyFleet" + } + ], "VpcId": "my-vpc" } }, diff --git a/packages/@aws-cdk/aws-ec2/lib/security-group.ts b/packages/@aws-cdk/aws-ec2/lib/security-group.ts index 07d23af92f456..662dde72a7366 100644 --- a/packages/@aws-cdk/aws-ec2/lib/security-group.ts +++ b/packages/@aws-cdk/aws-ec2/lib/security-group.ts @@ -1,4 +1,4 @@ -import { Construct, Output, Token } from '@aws-cdk/cdk'; +import { Construct, ITaggable, Output, TagManager, Tags, Token } from '@aws-cdk/cdk'; import { Connections, IConnectable } from './connections'; import { cloudformation } from './ec2.generated'; import { IPortRange, ISecurityGroupRule } from './security-group-rule'; @@ -89,6 +89,11 @@ export interface SecurityGroupProps { */ description?: string; + /** + * The AWS resource tags to associate with the security group. + */ + tags?: Tags; + /** * The VPC in which to create the security group. */ @@ -102,7 +107,7 @@ export interface SecurityGroupProps { * inline ingress and egress rule (which saves on the total number of resources inside * the template). */ -export class SecurityGroup extends SecurityGroupRef { +export class SecurityGroup extends SecurityGroupRef implements ITaggable { /** * An attribute that represents the security group name. */ @@ -118,6 +123,11 @@ export class SecurityGroup extends SecurityGroupRef { */ public readonly securityGroupId: string; + /** + * Manage tags for this construct and children + */ + public readonly tags: TagManager; + private readonly securityGroup: cloudformation.SecurityGroupResource; private readonly directIngressRules: cloudformation.SecurityGroupResource.IngressProperty[] = []; private readonly directEgressRules: cloudformation.SecurityGroupResource.EgressProperty[] = []; @@ -125,6 +135,7 @@ export class SecurityGroup extends SecurityGroupRef { constructor(parent: Construct, name: string, props: SecurityGroupProps) { super(parent, name); + this.tags = new TagManager(this, { initialTags: props.tags}); const groupDescription = props.description || this.path; this.securityGroup = new cloudformation.SecurityGroupResource(this, 'Resource', { groupName: props.groupName, @@ -132,6 +143,7 @@ export class SecurityGroup extends SecurityGroupRef { securityGroupIngress: new Token(() => this.directIngressRules), securityGroupEgress: new Token(() => this.directEgressRules), vpcId: props.vpc.vpcId, + tags: this.tags, }); this.securityGroupId = this.securityGroup.securityGroupId; diff --git a/packages/@aws-cdk/aws-ec2/lib/vpc.ts b/packages/@aws-cdk/aws-ec2/lib/vpc.ts index b6ba29a9df281..faf8c4b7f0a63 100644 --- a/packages/@aws-cdk/aws-ec2/lib/vpc.ts +++ b/packages/@aws-cdk/aws-ec2/lib/vpc.ts @@ -468,7 +468,7 @@ export class VpcSubnet extends VpcSubnetRef implements cdk.ITaggable { constructor(parent: cdk.Construct, name: string, props: VpcSubnetProps) { super(parent, name); - this.tags = new cdk.TagManager(this, props.tags); + this.tags = new cdk.TagManager(this, {initialTags: props.tags}); this.tags.setTag(NAME_TAG, this.path, {overwrite: false}); this.availabilityZone = props.availabilityZone; diff --git a/packages/@aws-cdk/aws-ec2/test/test.vpc.ts b/packages/@aws-cdk/aws-ec2/test/test.vpc.ts index 459ab504462a5..b9a52dfad6d50 100644 --- a/packages/@aws-cdk/aws-ec2/test/test.vpc.ts +++ b/packages/@aws-cdk/aws-ec2/test/test.vpc.ts @@ -128,6 +128,10 @@ export = { cidrMask: 24, name: 'ingress', subnetType: SubnetType.Public, + tags: { + type: 'Public', + init: 'No', + }, }, { cidrMask: 24, @@ -155,44 +159,50 @@ export = { CidrBlock: `10.0.6.${i * 16}/28` })); } + expect(stack).to(haveResource("AWS::EC2::Subnet", hasTags( + [ + { Key: 'type', Value: 'Public'}, + { Key: 'init', Value: 'No'}, + ], + ))); test.done(); }, "with custom subents and natGateways = 2 there should be only two NATGW"(test: Test) { const stack = getTestStack(); new VpcNetwork(stack, 'TheVPC', { - cidr: '10.0.0.0/21', - natGateways: 2, - subnetConfiguration: [ - { - cidrMask: 24, - name: 'ingress', - subnetType: SubnetType.Public, - }, - { - cidrMask: 24, - name: 'application', - subnetType: SubnetType.Private, - }, - { - cidrMask: 28, - name: 'rds', - subnetType: SubnetType.Isolated, - } - ], - maxAZs: 3 + cidr: '10.0.0.0/21', + natGateways: 2, + subnetConfiguration: [ + { + cidrMask: 24, + name: 'ingress', + subnetType: SubnetType.Public, + }, + { + cidrMask: 24, + name: 'application', + subnetType: SubnetType.Private, + }, + { + cidrMask: 28, + name: 'rds', + subnetType: SubnetType.Isolated, + } + ], + maxAZs: 3 }); expect(stack).to(countResources("AWS::EC2::InternetGateway", 1)); expect(stack).to(countResources("AWS::EC2::NatGateway", 2)); expect(stack).to(countResources("AWS::EC2::Subnet", 9)); for (let i = 0; i < 6; i++) { - expect(stack).to(haveResource("AWS::EC2::Subnet", { - CidrBlock: `10.0.${i}.0/24` - })); + expect(stack).to(haveResource("AWS::EC2::Subnet", { + CidrBlock: `10.0.${i}.0/24` + })); } for (let i = 0; i < 3; i++) { - expect(stack).to(haveResource("AWS::EC2::Subnet", { - CidrBlock: `10.0.6.${i * 16}/28` - })); + expect(stack).to(haveResource("AWS::EC2::Subnet", { + CidrBlock: `10.0.6.${i * 16}/28` + })); } test.done(); }, @@ -229,9 +239,9 @@ export = { expect(stack).to(countResources("AWS::EC2::Subnet", 4)); expect(stack).to(countResources("AWS::EC2::Route", 4)); for (let i = 0; i < 4; i++) { - expect(stack).to(haveResource("AWS::EC2::Subnet", { - CidrBlock: `10.0.${i * 64}.0/18` - })); + expect(stack).to(haveResource("AWS::EC2::Subnet", { + CidrBlock: `10.0.${i * 64}.0/18` + })); } expect(stack).to(haveResource("AWS::EC2::Route", { DestinationCidrBlock: '0.0.0.0/0', diff --git a/packages/@aws-cdk/aws-elasticloadbalancing/test/integ.elb.expected.json b/packages/@aws-cdk/aws-elasticloadbalancing/test/integ.elb.expected.json index 4b05529da2869..0203dcbaccd44 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancing/test/integ.elb.expected.json +++ b/packages/@aws-cdk/aws-elasticloadbalancing/test/integ.elb.expected.json @@ -185,6 +185,7 @@ "ToPort": 80 } ], + "Tags": [], "VpcId": { "Ref": "VPCB9E5F0B4" } diff --git a/packages/@aws-cdk/aws-lambda/test/integ.vpc-lambda.expected.json b/packages/@aws-cdk/aws-lambda/test/integ.vpc-lambda.expected.json index 396988864f19f..93e729c8fe111 100644 --- a/packages/@aws-cdk/aws-lambda/test/integ.vpc-lambda.expected.json +++ b/packages/@aws-cdk/aws-lambda/test/integ.vpc-lambda.expected.json @@ -382,6 +382,7 @@ } ], "SecurityGroupIngress": [], + "Tags": [], "VpcId": { "Ref": "VPCB9E5F0B4" } diff --git a/packages/@aws-cdk/cdk/lib/core/tag-manager.ts b/packages/@aws-cdk/cdk/lib/core/tag-manager.ts index d0b4b5daa00cd..d8763158cedd6 100644 --- a/packages/@aws-cdk/cdk/lib/core/tag-manager.ts +++ b/packages/@aws-cdk/cdk/lib/core/tag-manager.ts @@ -60,7 +60,7 @@ export interface TagGroups { /** * Tags that are overwritten by ancestor tags */ - nonSitckyTags: Tags; + nonStickyTags: Tags; /** * Tags with propagate true not from an ancestor @@ -209,14 +209,14 @@ export class TagManager extends Token { return parentTags; } - const nonSitckyTags = filterTags(this._tags, {sticky: false}); + const nonStickyTags = filterTags(this._tags, {sticky: false}); const stickyTags = filterTags(this._tags, {sticky: true}); const ancestors = this.parent.ancestors(); const ancestorTags = propagatedTags(ancestors); const propagateTags = filterTags(this._tags, {propagate: true}); return this.tagFormatResolve( { ancestorTags, - nonSitckyTags, + nonStickyTags, stickyTags, propagateTags, }); @@ -263,7 +263,7 @@ export class TagManager extends Token { * additional CloudFormation key for `PropagateAtLaunch` */ protected tagFormatResolve(tagGroups: TagGroups): any { - const tags = {...tagGroups.nonSitckyTags, ...tagGroups.ancestorTags, ...tagGroups.stickyTags}; + const tags = {...tagGroups.nonStickyTags, ...tagGroups.ancestorTags, ...tagGroups.stickyTags}; for (const key of this.blockedTags) { delete tags[key]; } return Object.keys(tags).map( key => ({key, value: tags[key]})); }