Skip to content

Commit

Permalink
fix(cdk): move apply() from Construct to ConstructNode (#1738)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
moofish32 authored and Elad Ben-Israel committed Feb 12, 2019
1 parent 49080c6 commit 642c8a6
Show file tree
Hide file tree
Showing 14 changed files with 50 additions and 51 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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", {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-dynamodb/test/integ.dynamodb.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-dynamodb/test/test.dynamodb.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
{
Expand Down
8 changes: 4 additions & 4 deletions packages/@aws-cdk/aws-ec2/lib/vpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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}));
});
}
}
Expand Down Expand Up @@ -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', {
Expand Down
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-ec2/test/test.vpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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();
},
Expand Down
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-eks/lib/cluster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', {
Expand All @@ -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"));
}
}
}
Expand Down Expand Up @@ -339,4 +339,4 @@ function flatMap<T, U>(xs: T[], f: (x: T) => U[]): U[] {
ret.push(...f(x));
}
return ret;
}
}
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-kms/test/test.key.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-s3/test/test.notifications.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down
14 changes: 7 additions & 7 deletions packages/@aws-cdk/cdk/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
```
Expand Down Expand Up @@ -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 //
```

Expand All @@ -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
```

Expand All @@ -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
```

Expand All @@ -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
```

Expand All @@ -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
```
15 changes: 7 additions & 8 deletions packages/@aws-cdk/cdk/lib/core/construct.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down Expand Up @@ -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.
*
Expand Down
30 changes: 15 additions & 15 deletions packages/@aws-cdk/cdk/test/aspects/test.tag-aspect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'}]);
Expand All @@ -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'}]);
Expand All @@ -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'}]);
Expand All @@ -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();
Expand All @@ -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);
Expand All @@ -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'}]);
Expand All @@ -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'}]);
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/cdk/test/test.aspect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit 642c8a6

Please sign in to comment.