fix(cdk): move apply() from Construct to ConstructNode (#1738)

moofish32 · Elad Ben-Israel · commit 642c8a616ed2 · 2019-02-12T10:20:14.000+02:00
This change moves the aspect `apply()` function from the `Construct` to the `ConstructNode`. All other functionality remains the same. BREAKING CHANGE: Tag aspects use this feature and any consumers of this implementation must change from `myConstruct.apply( ... )` to `myConstruct.node.apply( ... )`. fixes #1732
diff --git a/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts b/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts
@@ -230,7 +230,7 @@ export class AutoScalingGroup extends cdk.Construct implements IAutoScalingGroup
     });
     this.connections = new ec2.Connections({ securityGroups: [this.securityGroup] });
     this.securityGroups.push(this.securityGroup);
-    this.apply(new cdk.Tag(NAME_TAG, this.node.path));
+    this.node.apply(new cdk.Tag(NAME_TAG, this.node.path));
 
     this.role = props.role || new iam.Role(this, 'InstanceRole', {
       assumedBy: new iam.ServicePrincipal('ec2.amazonaws.com')
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
@@ -380,8 +380,8 @@ export = {
         pauseTimeSec: 345
       },
     });
-    asg.apply( new cdk.Tag('superfood', 'acai'));
-    asg.apply( new cdk.Tag('notsuper', 'caramel', { applyToLaunchedInstances: false }));
+    asg.node.apply(new cdk.Tag('superfood', 'acai'));
+    asg.node.apply(new cdk.Tag('notsuper', 'caramel', { applyToLaunchedInstances: false }));
 
     // THEN
     expect(stack).to(haveResource("AWS::AutoScaling::AutoScalingGroup", {
diff --git a/packages/@aws-cdk/aws-dynamodb/test/integ.dynamodb.ondemand.ts b/packages/@aws-cdk/aws-dynamodb/test/integ.dynamodb.ondemand.ts
@@ -52,7 +52,7 @@ const tableWithGlobalAndLocalSecondaryIndex = new Table(stack, TABLE_WITH_GLOBAL
   ttlAttributeName: 'timeToLive'
 });
 
-tableWithGlobalAndLocalSecondaryIndex.apply(new Tag('Environment', 'Production'));
+tableWithGlobalAndLocalSecondaryIndex.node.apply(new Tag('Environment', 'Production'));
 
 tableWithGlobalAndLocalSecondaryIndex.addPartitionKey(TABLE_PARTITION_KEY);
 tableWithGlobalAndLocalSecondaryIndex.addSortKey(TABLE_SORT_KEY);
diff --git a/packages/@aws-cdk/aws-dynamodb/test/integ.dynamodb.ts b/packages/@aws-cdk/aws-dynamodb/test/integ.dynamodb.ts
@@ -51,7 +51,7 @@ const tableWithGlobalAndLocalSecondaryIndex = new Table(stack, TABLE_WITH_GLOBAL
   ttlAttributeName: 'timeToLive'
 });
 
-tableWithGlobalAndLocalSecondaryIndex.apply(new Tag('Environment', 'Production'));
+tableWithGlobalAndLocalSecondaryIndex.node.apply(new Tag('Environment', 'Production'));
 tableWithGlobalAndLocalSecondaryIndex.addPartitionKey(TABLE_PARTITION_KEY);
 tableWithGlobalAndLocalSecondaryIndex.addSortKey(TABLE_SORT_KEY);
 tableWithGlobalAndLocalSecondaryIndex.addGlobalSecondaryIndex({
diff --git a/packages/@aws-cdk/aws-dynamodb/test/test.dynamodb.ts b/packages/@aws-cdk/aws-dynamodb/test/test.dynamodb.ts
@@ -317,7 +317,7 @@ export = {
     });
     table.addPartitionKey(TABLE_PARTITION_KEY);
     table.addSortKey(TABLE_SORT_KEY);
-    table.apply(new Tag('Environment', 'Production'));
+    table.node.apply(new Tag('Environment', 'Production'));
 
     expect(stack).to(haveResource('AWS::DynamoDB::Table',
       {
diff --git a/packages/@aws-cdk/aws-ec2/lib/vpc.ts b/packages/@aws-cdk/aws-ec2/lib/vpc.ts
@@ -299,7 +299,7 @@ export class VpcNetwork extends VpcNetworkBase {
       instanceTenancy,
     });
 
-    this.apply(new cdk.Tag(NAME_TAG, this.node.path));
+    this.node.apply(new cdk.Tag(NAME_TAG, this.node.path));
 
     this.availabilityZones = new cdk.AvailabilityZoneProvider(this).availabilityZones;
     this.availabilityZones.sort();
@@ -452,8 +452,8 @@ export class VpcNetwork extends VpcNetworkBase {
 
       // These values will be used to recover the config upon provider import
       const includeResourceTypes = [CfnSubnet.resourceTypeName];
-      subnet.apply(new cdk.Tag(SUBNETNAME_TAG, subnetConfig.name, {includeResourceTypes}));
-      subnet.apply(new cdk.Tag(SUBNETTYPE_TAG, subnetTypeTagValue(subnetConfig.subnetType), {includeResourceTypes}));
+      subnet.node.apply(new cdk.Tag(SUBNETNAME_TAG, subnetConfig.name, {includeResourceTypes}));
+      subnet.node.apply(new cdk.Tag(SUBNETTYPE_TAG, subnetTypeTagValue(subnetConfig.subnetType), {includeResourceTypes}));
     });
   }
 }
@@ -529,7 +529,7 @@ export class VpcSubnet extends cdk.Construct implements IVpcSubnet {
 
   constructor(scope: cdk.Construct, id: string, props: VpcSubnetProps) {
     super(scope, id);
-    this.apply(new cdk.Tag(NAME_TAG, this.node.path));
+    this.node.apply(new cdk.Tag(NAME_TAG, this.node.path));
 
     this.availabilityZone = props.availabilityZone;
     const subnet = new CfnSubnet(this, 'Subnet', {
diff --git a/packages/@aws-cdk/aws-ec2/test/test.vpc.ts b/packages/@aws-cdk/aws-ec2/test/test.vpc.ts
@@ -349,8 +349,8 @@ export = {
 
       const vpc = new VpcNetwork(stack, 'TheVPC');
       // overwrite to set propagate
-      vpc.apply(new Tag('BusinessUnit', 'Marketing', {includeResourceTypes: [CfnVPC.resourceTypeName]}));
-      vpc.apply(new Tag('VpcType', 'Good'));
+      vpc.node.apply(new Tag('BusinessUnit', 'Marketing', {includeResourceTypes: [CfnVPC.resourceTypeName]}));
+      vpc.node.apply(new Tag('VpcType', 'Good'));
       expect(stack).to(haveResource("AWS::EC2::VPC", hasTags(toCfnTags(allTags))));
       const taggables = ['Subnet', 'InternetGateway', 'NatGateway', 'RouteTable'];
       const propTags = toCfnTags(tags);
@@ -381,7 +381,7 @@ export = {
       const vpc = new VpcNetwork(stack, 'TheVPC');
       const tag = {Key: 'Late', Value: 'Adder'};
       expect(stack).notTo(haveResource('AWS::EC2::VPC', hasTags([tag])));
-      vpc.apply(new Tag(tag.Key, tag.Value));
+      vpc.node.apply(new Tag(tag.Key, tag.Value));
       expect(stack).to(haveResource('AWS::EC2::VPC', hasTags([tag])));
       test.done();
     },
diff --git a/packages/@aws-cdk/aws-eks/lib/cluster.ts b/packages/@aws-cdk/aws-eks/lib/cluster.ts
@@ -247,7 +247,7 @@ export class Cluster extends ClusterBase {
     autoScalingGroup.role.attachManagedPolicy(new iam.AwsManagedPolicy('AmazonEC2ContainerRegistryReadOnly', this).policyArn);
 
     // EKS Required Tags
-    autoScalingGroup.apply(new cdk.Tag(`kubernetes.io/cluster/${this.clusterName}`, 'owned', { applyToLaunchedInstances: true }));
+    autoScalingGroup.node.apply(new cdk.Tag(`kubernetes.io/cluster/${this.clusterName}`, 'owned', { applyToLaunchedInstances: true }));
 
     // Create an Output for the Instance Role ARN (need to paste it into aws-auth-cm.yaml)
     new cdk.Output(autoScalingGroup, 'InstanceRoleARN', {
@@ -273,7 +273,7 @@ export class Cluster extends ClusterBase {
         return;
       }
 
-      subnet.apply(new cdk.Tag("kubernetes.io/role/internal-elb", "1"));
+      subnet.node.apply(new cdk.Tag("kubernetes.io/role/internal-elb", "1"));
     }
   }
 }
@@ -339,4 +339,4 @@ function flatMap<T, U>(xs: T[], f: (x: T) => U[]): U[] {
     ret.push(...f(x));
   }
   return ret;
-}
+}
diff --git a/packages/@aws-cdk/aws-kms/test/test.key.ts b/packages/@aws-cdk/aws-kms/test/test.key.ts
@@ -147,9 +147,9 @@ export = {
     p.addAwsPrincipal('arn');
     key.addToResourcePolicy(p);
 
-    key.apply(new Tag('tag1', 'value1'));
-    key.apply(new Tag('tag2', 'value2'));
-    key.apply(new Tag('tag3', ''));
+    key.node.apply(new Tag('tag1', 'value1'));
+    key.node.apply(new Tag('tag2', 'value2'));
+    key.node.apply(new Tag('tag3', ''));
 
     expect(stack).to(exactlyMatchTemplate({
       Resources: {
diff --git a/packages/@aws-cdk/aws-s3/test/test.notifications.ts b/packages/@aws-cdk/aws-s3/test/test.notifications.ts
@@ -44,7 +44,7 @@ export = {
   },
   'when notification are added, you can tag the lambda'(test: Test) {
     const stack = new cdk.Stack();
-    stack.apply(new cdk.Tag('Lambda', 'AreTagged'));
+    stack.node.apply(new cdk.Tag('Lambda', 'AreTagged'));
 
     const bucket = new s3.Bucket(stack, 'MyBucket');
 
diff --git a/packages/@aws-cdk/cdk/README.md b/packages/@aws-cdk/cdk/README.md
@@ -39,7 +39,7 @@ import cdk = require('@aws-cdk/cdk');
 
 const app = new cdk.App();
 const theBestStack = new cdk.Stack(app, 'MarketingSystem');
-theBestStack.apply(new cdk.Tag('StackType', 'TheBest'));
+theBestStack.node.apply(new cdk.Tag('StackType', 'TheBest'));
 
 // any resources added that support tags will get them
 ```
@@ -105,8 +105,8 @@ const vpc = new ec2.VpcNetwork(marketingStack, 'MarketingVpc', {
   });
 // override the VPC tags with Platform
 // this will tag the VPC, Subnets, Route Tables, IGW, and NatGWs
-vpc.apply(new cdk.Tag(COST_CENTER_KEY, 'Platform'));
-vpc.apply(new cdk.RemoveTag('Name'));
+vpc.node.apply(new cdk.Tag(COST_CENTER_KEY, 'Platform'));
+vpc.node.apply(new cdk.RemoveTag('Name'));
 // snip //
 ```
 
@@ -131,7 +131,7 @@ true. If false the property is set accordingly.
 ```ts
 // ... snip
 const vpc = new ec2.VpcNetwork(this, 'MyVpc', { ... });
-vpc.apply(new cdk.Tag('MyKey', 'MyValue', { applyToLaunchedInstances: false }));
+vpc.node.apply(new cdk.Tag('MyKey', 'MyValue', { applyToLaunchedInstances: false }));
 // ... snip
 ```
 
@@ -145,7 +145,7 @@ interpreted as apply to any resource type.
 ```ts
 // ... snip
 const vpc = new ec2.VpcNetwork(this, 'MyVpc', { ... });
-vpc.apply(new cdk.Tag('MyKey', 'MyValue', { includeResourceTypes: ['AWS::EC2::Subnet']}));
+vpc.node.apply(new cdk.Tag('MyKey', 'MyValue', { includeResourceTypes: ['AWS::EC2::Subnet']}));
 // ... snip
 ```
 
@@ -160,7 +160,7 @@ over include in the event of a collision.
 ```ts
 // ... snip
 const vpc = new ec2.VpcNetwork(this, 'MyVpc', { ... });
-vpc.apply(new cdk.Tag('MyKey', 'MyValue', { exludeResourceTypes: ['AWS::EC2::Subnet']}));
+vpc.node.apply(new cdk.Tag('MyKey', 'MyValue', { exludeResourceTypes: ['AWS::EC2::Subnet']}));
 // ... snip
 ```
 
@@ -174,6 +174,6 @@ setting for removing tags uses a higher priority than the standard tag.
 ```ts
 // ... snip
 const vpc = new ec2.VpcNetwork(this, 'MyVpc', { ... });
-vpc.apply(new cdk.Tag('MyKey', 'MyValue', { priority: 2 }));
+vpc.node.apply(new cdk.Tag('MyKey', 'MyValue', { priority: 2 }));
 // ... snip
 ```
diff --git a/packages/@aws-cdk/cdk/lib/core/construct.ts b/packages/@aws-cdk/cdk/lib/core/construct.ts
@@ -312,6 +312,13 @@ export class ConstructNode {
     }
   }
 
+  /**
+   * Applies the aspect to this Constructs node
+   */
+  public apply(aspect: IAspect): void {
+    this.aspects.push(aspect);
+    return;
+  }
   /**
    * Return the ancestors (including self) of this Construct up until and excluding the indicated component
    *
@@ -576,14 +583,6 @@ export class Construct implements IConstruct {
     return this.node.typename + (path.length > 0 ? ` [${path}]` : '');
   }
 
-  /**
-   * Applies the aspect to this Constructs node
-   */
-  public apply(aspect: IAspect): void {
-    this.node.aspects.push(aspect);
-    return;
-  }
-
   /**
    * Validate the current construct.
    *
diff --git a/packages/@aws-cdk/cdk/test/aspects/test.tag-aspect.ts b/packages/@aws-cdk/cdk/test/aspects/test.tag-aspect.ts
@@ -32,7 +32,7 @@ export = {
     const map = new MapTaggableResource(res, 'MapFakeResource', {
       type: 'AWS::Fake::Thing',
     });
-    res.apply(new Tag('foo', 'bar'));
+    res.node.apply(new Tag('foo', 'bar'));
     test.deepEqual(res.node.aspects.length, 1);
     root.node.prepareTree();
     test.deepEqual(res.tags.renderTags(), [{key: 'foo', value: 'bar'}]);
@@ -49,10 +49,10 @@ export = {
     const res2 = new TaggableResource(res, 'FakeResource', {
       type: 'AWS::Fake::Thing',
     });
-    res.apply(new Tag('foo', 'bar'));
-    res.apply(new Tag('foo', 'foobar'));
-    res.apply(new Tag('foo', 'baz'));
-    res2.apply(new Tag('foo', 'good'));
+    res.node.apply(new Tag('foo', 'bar'));
+    res.node.apply(new Tag('foo', 'foobar'));
+    res.node.apply(new Tag('foo', 'baz'));
+    res2.node.apply(new Tag('foo', 'good'));
     root.node.prepareTree();
     test.deepEqual(res.tags.renderTags(), [{key: 'foo', value: 'baz'}]);
     test.deepEqual(res2.tags.renderTags(), [{key: 'foo', value: 'good'}]);
@@ -73,10 +73,10 @@ export = {
     const map = new MapTaggableResource(res, 'MapFakeResource', {
       type: 'AWS::Fake::Thing',
     });
-    root.apply(new Tag('root', 'was here'));
-    res.apply(new Tag('first', 'there is only 1'));
-    res.apply(new RemoveTag('root'));
-    res.apply(new RemoveTag('doesnotexist'));
+    root.node.apply(new Tag('root', 'was here'));
+    res.node.apply(new Tag('first', 'there is only 1'));
+    res.node.apply(new RemoveTag('root'));
+    res.node.apply(new RemoveTag('doesnotexist'));
     root.node.prepareTree();
 
     test.deepEqual(res.tags.renderTags(), [{key: 'first', value: 'there is only 1'}]);
@@ -91,7 +91,7 @@ export = {
       type: 'AWS::Fake::Thing',
     });
 
-    res.apply(new Tag('foo', 'bar'));
+    res.node.apply(new Tag('foo', 'bar'));
     root.node.prepareTree();
     test.deepEqual(res.tags.renderTags(), [{key: 'foo', value: 'bar'}]);
     root.node.prepareTree();
@@ -108,8 +108,8 @@ export = {
     const res2 = new TaggableResource(res, 'FakeResource', {
       type: 'AWS::Fake::Thing',
     });
-    res.apply(new RemoveTag('key'));
-    res2.apply(new Tag('key', 'value'));
+    res.node.apply(new RemoveTag('key'));
+    res2.node.apply(new Tag('key', 'value'));
     root.node.prepareTree();
     test.deepEqual(res.tags.renderTags(), undefined);
     test.deepEqual(res2.tags.renderTags(), undefined);
@@ -123,8 +123,8 @@ export = {
     const res2 = new TaggableResource(res, 'FakeResource', {
       type: 'AWS::Fake::Thing',
     });
-    res.apply(new RemoveTag('key', {priority: 0}));
-    res2.apply(new Tag('key', 'value'));
+    res.node.apply(new RemoveTag('key', {priority: 0}));
+    res2.node.apply(new Tag('key', 'value'));
     root.node.prepareTree();
     test.deepEqual(res.tags.renderTags(), undefined);
     test.deepEqual(res2.tags.renderTags(), [{key: 'key', value: 'value'}]);
@@ -148,7 +148,7 @@ export = {
         ],
       },
     });
-    aspectBranch.apply(new Tag('aspects', 'rule'));
+    aspectBranch.node.apply(new Tag('aspects', 'rule'));
     root.node.prepareTree();
     test.deepEqual(aspectBranch.tags.renderTags(), [{key: 'aspects', value: 'rule'}]);
     test.deepEqual(cfnBranch.testProperties().tags, [{key: 'cfn', value: 'is cool'}]);
diff --git a/packages/@aws-cdk/cdk/test/test.aspect.ts b/packages/@aws-cdk/cdk/test/test.aspect.ts
@@ -19,7 +19,7 @@ class VisitOnce implements IAspect {
 export = {
   'Aspects are invoked only once'(test: Test) {
     const root = new MyConstruct();
-    root.apply(new VisitOnce());
+    root.node.apply(new VisitOnce());
     root.node.prepareTree();
     test.deepEqual(root.visitCounter, 1);
     root.node.prepareTree();

Original file line number	Diff line number	Diff line change
`@@ -317,7 +317,7 @@ export = {`
`317`	`317`	`});`
`318`	`318`	`table.addPartitionKey(TABLE_PARTITION_KEY);`
`319`	`319`	`table.addSortKey(TABLE_SORT_KEY);`
`320`		`- table.apply(new Tag('Environment', 'Production'));`
	`320`	`+ table.node.apply(new Tag('Environment', 'Production'));`
`321`	`321`
`322`	`322`	`expect(stack).to(haveResource('AWS::DynamoDB::Table',`
`323`	`323`	`{`
Original file line number	Diff line number	Diff line change
`@@ -247,7 +247,7 @@ export class Cluster extends ClusterBase {`
`247`	`247`	`autoScalingGroup.role.attachManagedPolicy(new iam.AwsManagedPolicy('AmazonEC2ContainerRegistryReadOnly', this).policyArn);`
`248`	`248`
`249`	`249`	`// EKS Required Tags`
`250`		- autoScalingGroup.apply(new cdk.Tag(`kubernetes.io/cluster/${this.clusterName}`, 'owned', { applyToLaunchedInstances: true }));
	`250`	+ autoScalingGroup.node.apply(new cdk.Tag(`kubernetes.io/cluster/${this.clusterName}`, 'owned', { applyToLaunchedInstances: true }));
`251`	`251`
`252`	`252`	`// Create an Output for the Instance Role ARN (need to paste it into aws-auth-cm.yaml)`
`253`	`253`	`new cdk.Output(autoScalingGroup, 'InstanceRoleARN', {`
`@@ -273,7 +273,7 @@ export class Cluster extends ClusterBase {`
`273`	`273`	`return;`
`274`	`274`	`}`
`275`	`275`
`276`		`- subnet.apply(new cdk.Tag("kubernetes.io/role/internal-elb", "1"));`
	`276`	`+ subnet.node.apply(new cdk.Tag("kubernetes.io/role/internal-elb", "1"));`
`277`	`277`	`}`
`278`	`278`	`}`
`279`	`279`	`}`
`@@ -339,4 +339,4 @@ function flatMap<T, U>(xs: T[], f: (x: T) => U[]): U[] {`
`339`	`339`	`ret.push(...f(x));`
`340`	`340`	`}`
`341`	`341`	`return ret;`
`342`		`-}`
	`342`	`+}`