From 4d4b90bd433d489af3f64a7edae7a1b4cd00e143 Mon Sep 17 00:00:00 2001 From: Jungseok Lee Date: Tue, 9 Oct 2018 19:42:10 -0700 Subject: [PATCH 1/2] fix(cdk): fix TagManager to evaluate to undefined if no tags are included It is unnecessary to add "Tags" field to CloudFormation template unless tags are specified. Therefore, this patch fixes TagManager to evaluate to undefined if there are no tags included. --- ...g.asg-w-classic-loadbalancer.expected.json | 1 - .../test/integ.asg-w-elbv2.expected.json | 1 - .../test/integ.dynamodb.expected.json | 9 +-- .../aws-dynamodb/test/test.dynamodb.ts | 74 ++++++------------- packages/@aws-cdk/cdk/lib/core/tag-manager.ts | 3 + .../cdk/test/core/test.tag-manager.ts | 10 +-- 6 files changed, 35 insertions(+), 63 deletions(-) diff --git a/packages/@aws-cdk/aws-autoscaling/test/integ.asg-w-classic-loadbalancer.expected.json b/packages/@aws-cdk/aws-autoscaling/test/integ.asg-w-classic-loadbalancer.expected.json index e1b0eedbe3bc4..12f63b4c44098 100644 --- a/packages/@aws-cdk/aws-autoscaling/test/integ.asg-w-classic-loadbalancer.expected.json +++ b/packages/@aws-cdk/aws-autoscaling/test/integ.asg-w-classic-loadbalancer.expected.json @@ -589,7 +589,6 @@ "ToPort": 80 } ], - "Tags": [], "VpcId": { "Ref": "VPCB9E5F0B4" } diff --git a/packages/@aws-cdk/aws-autoscaling/test/integ.asg-w-elbv2.expected.json b/packages/@aws-cdk/aws-autoscaling/test/integ.asg-w-elbv2.expected.json index 65c1a6fee950f..f02b0c3b4ac4f 100644 --- a/packages/@aws-cdk/aws-autoscaling/test/integ.asg-w-elbv2.expected.json +++ b/packages/@aws-cdk/aws-autoscaling/test/integ.asg-w-elbv2.expected.json @@ -476,7 +476,6 @@ "ToPort": 80 } ], - "Tags": [], "VpcId": { "Ref": "VPCB9E5F0B4" } diff --git a/packages/@aws-cdk/aws-dynamodb/test/integ.dynamodb.expected.json b/packages/@aws-cdk/aws-dynamodb/test/integ.dynamodb.expected.json index fafe5f0a7ef86..12b7020d69c42 100644 --- a/packages/@aws-cdk/aws-dynamodb/test/integ.dynamodb.expected.json +++ b/packages/@aws-cdk/aws-dynamodb/test/integ.dynamodb.expected.json @@ -20,8 +20,7 @@ } ], "GlobalSecondaryIndexes": [], - "LocalSecondaryIndexes": [], - "Tags": [] + "LocalSecondaryIndexes": [] } }, "TableWithGlobalAndLocalSecondaryIndexBC540710": { @@ -313,8 +312,7 @@ } } ], - "LocalSecondaryIndexes": [], - "Tags": [] + "LocalSecondaryIndexes": [] } }, "TableWithLocalSecondaryIndex4DA3D08F": { @@ -366,8 +364,7 @@ "ProjectionType": "ALL" } } - ], - "Tags": [] + ] } } } diff --git a/packages/@aws-cdk/aws-dynamodb/test/test.dynamodb.ts b/packages/@aws-cdk/aws-dynamodb/test/test.dynamodb.ts index 8516074446576..b1d1481307a88 100644 --- a/packages/@aws-cdk/aws-dynamodb/test/test.dynamodb.ts +++ b/packages/@aws-cdk/aws-dynamodb/test/test.dynamodb.ts @@ -87,8 +87,7 @@ export = { KeySchema: [{ AttributeName: 'hashKey', KeyType: 'HASH' }], ProvisionedThroughput: { ReadCapacityUnits: 5, WriteCapacityUnits: 5 }, GlobalSecondaryIndexes: [], - LocalSecondaryIndexes: [], - Tags: [] + LocalSecondaryIndexes: [] } } } @@ -119,8 +118,7 @@ export = { ], ProvisionedThroughput: { ReadCapacityUnits: 5, WriteCapacityUnits: 5 }, GlobalSecondaryIndexes: [], - LocalSecondaryIndexes: [], - Tags: [] + LocalSecondaryIndexes: [] } } } @@ -151,8 +149,7 @@ export = { ], ProvisionedThroughput: { ReadCapacityUnits: 5, WriteCapacityUnits: 5 }, GlobalSecondaryIndexes: [], - LocalSecondaryIndexes: [], - Tags: [] + LocalSecondaryIndexes: [] } } } @@ -183,8 +180,7 @@ export = { ], ProvisionedThroughput: { ReadCapacityUnits: 5, WriteCapacityUnits: 5 }, GlobalSecondaryIndexes: [], - LocalSecondaryIndexes: [], - Tags: [] + LocalSecondaryIndexes: [] } } } @@ -215,8 +211,7 @@ export = { ], ProvisionedThroughput: { ReadCapacityUnits: 5, WriteCapacityUnits: 5 }, GlobalSecondaryIndexes: [], - LocalSecondaryIndexes: [], - Tags: [] + LocalSecondaryIndexes: [] } } } @@ -247,8 +242,7 @@ export = { ], ProvisionedThroughput: { ReadCapacityUnits: 5, WriteCapacityUnits: 5 }, GlobalSecondaryIndexes: [], - LocalSecondaryIndexes: [], - Tags: [] + LocalSecondaryIndexes: [] } } } @@ -286,8 +280,7 @@ export = { ProvisionedThroughput: { ReadCapacityUnits: 42, WriteCapacityUnits: 1337 }, GlobalSecondaryIndexes: [], LocalSecondaryIndexes: [], - TableName: 'MyTable', - Tags: [] + TableName: 'MyTable' } } } @@ -325,8 +318,7 @@ export = { { AttributeName: 'sortKey', AttributeType: 'N' } ], StreamSpecification: { StreamViewType: 'NEW_IMAGE' }, - TableName: 'MyTable', - Tags: [] + TableName: 'MyTable' } } } @@ -364,8 +356,7 @@ export = { { AttributeName: 'sortKey', AttributeType: 'N' } ], StreamSpecification: { StreamViewType: 'OLD_IMAGE' }, - TableName: 'MyTable', - Tags: [] + TableName: 'MyTable' } } } @@ -462,8 +453,7 @@ export = { ProvisionedThroughput: { ReadCapacityUnits: 42, WriteCapacityUnits: 1337 } } ], - LocalSecondaryIndexes: [], - Tags: [] + LocalSecondaryIndexes: [] } } } @@ -514,8 +504,7 @@ export = { ProvisionedThroughput: { ReadCapacityUnits: 42, WriteCapacityUnits: 1337 } } ], - LocalSecondaryIndexes: [], - Tags: [] + LocalSecondaryIndexes: [] } } } @@ -564,8 +553,7 @@ export = { ProvisionedThroughput: { ReadCapacityUnits: 5, WriteCapacityUnits: 5 } } ], - LocalSecondaryIndexes: [], - Tags: [] + LocalSecondaryIndexes: [] } } } @@ -618,8 +606,7 @@ export = { ProvisionedThroughput: { ReadCapacityUnits: 42, WriteCapacityUnits: 1337 } } ], - LocalSecondaryIndexes: [], - Tags: [] + LocalSecondaryIndexes: [] } } } @@ -789,8 +776,7 @@ export = { ProvisionedThroughput: { ReadCapacityUnits: 5, WriteCapacityUnits: 5 } }, ], - LocalSecondaryIndexes: [], - Tags: [] + LocalSecondaryIndexes: [] } } } @@ -851,8 +837,7 @@ export = { ProvisionedThroughput: { ReadCapacityUnits: 5, WriteCapacityUnits: 5 } } ], - LocalSecondaryIndexes: [], - Tags: [] + LocalSecondaryIndexes: [] } } } @@ -897,8 +882,7 @@ export = { ], Projection: { ProjectionType: 'ALL' }, } - ], - Tags: [] + ] } } } @@ -944,8 +928,7 @@ export = { ], Projection: { ProjectionType: 'KEYS_ONLY' }, } - ], - Tags: [] + ] } } } @@ -994,8 +977,7 @@ export = { ], Projection: { NonKeyAttributes: ['lsiNonKey0', 'lsiNonKey1'], ProjectionType: 'INCLUDE' }, } - ], - Tags: [] + ] } } } @@ -1100,8 +1082,7 @@ export = { AttributeDefinitions: [ { AttributeName: 'hashKey', AttributeType: 'S' }, { AttributeName: 'sortKey', AttributeType: 'N' } ], - TableName: 'MyTable', - Tags: [] } }, + TableName: 'MyTable' } }, MyTableReadAutoScalingRoleFEE68E49: { Type: 'AWS::IAM::Role', Properties: @@ -1183,8 +1164,7 @@ export = { AttributeDefinitions: [ { AttributeName: 'hashKey', AttributeType: 'S' }, { AttributeName: 'sortKey', AttributeType: 'N' } ], - TableName: 'MyTable', - Tags: [] } }, + TableName: 'MyTable' } }, MyTableReadAutoScalingRoleFEE68E49: { Type: 'AWS::IAM::Role', Properties: @@ -1294,8 +1274,7 @@ export = { AttributeDefinitions: [ { AttributeName: 'hashKey', AttributeType: 'S' }, { AttributeName: 'sortKey', AttributeType: 'N' } ], - TableName: 'MyTable', - Tags: [] } }, + TableName: 'MyTable' } }, MyTableReadAutoScalingRoleFEE68E49: { Type: 'AWS::IAM::Role', Properties: @@ -1373,7 +1352,6 @@ export = { ProvisionedThroughput: { ReadCapacityUnits: 42, WriteCapacityUnits: 1337 }, GlobalSecondaryIndexes: [], LocalSecondaryIndexes: [], - Tags: [], AttributeDefinitions: [ { AttributeName: 'hashKey', AttributeType: 'S' }, { AttributeName: 'sortKey', AttributeType: 'N' } ] } }, @@ -1581,8 +1559,7 @@ export = { AttributeDefinitions: [ { AttributeName: 'hashKey', AttributeType: 'S' }, { AttributeName: 'sortKey', AttributeType: 'N' } ], - TableName: 'MyTable', - Tags: [] } }, + TableName: 'MyTable' } }, MyTableWriteAutoScalingRoleDF7775DE: { Type: 'AWS::IAM::Role', Properties: @@ -1664,8 +1641,7 @@ export = { AttributeDefinitions: [ { AttributeName: 'hashKey', AttributeType: 'S' }, { AttributeName: 'sortKey', AttributeType: 'N' } ], - TableName: 'MyTable', - Tags: [] } }, + TableName: 'MyTable' } }, MyTableWriteAutoScalingRoleDF7775DE: { Type: 'AWS::IAM::Role', Properties: @@ -1775,8 +1751,7 @@ export = { AttributeDefinitions: [ { AttributeName: 'hashKey', AttributeType: 'S' }, { AttributeName: 'sortKey', AttributeType: 'N' } ], - TableName: 'MyTable', - Tags: [] } }, + TableName: 'MyTable' } }, MyTableWriteAutoScalingRoleDF7775DE: { Type: 'AWS::IAM::Role', Properties: @@ -1854,7 +1829,6 @@ export = { ProvisionedThroughput: { ReadCapacityUnits: 42, WriteCapacityUnits: 1337 }, GlobalSecondaryIndexes: [], LocalSecondaryIndexes: [], - Tags: [], AttributeDefinitions: [ { AttributeName: 'hashKey', AttributeType: 'S' }, { AttributeName: 'sortKey', AttributeType: 'N' } ] } }, diff --git a/packages/@aws-cdk/cdk/lib/core/tag-manager.ts b/packages/@aws-cdk/cdk/lib/core/tag-manager.ts index 7a25100eb47e5..3b5ebf053320d 100644 --- a/packages/@aws-cdk/cdk/lib/core/tag-manager.ts +++ b/packages/@aws-cdk/cdk/lib/core/tag-manager.ts @@ -265,6 +265,9 @@ export class TagManager extends Token { protected tagFormatResolve(tagGroups: TagGroups): any { const tags = {...tagGroups.nonStickyTags, ...tagGroups.ancestorTags, ...tagGroups.stickyTags}; for (const key of this.blockedTags) { delete tags[key]; } + if (Object.keys(tags).length === 0) { + return undefined; + } return Object.keys(tags).map( key => ({key, value: tags[key]})); } } diff --git a/packages/@aws-cdk/cdk/test/core/test.tag-manager.ts b/packages/@aws-cdk/cdk/test/core/test.tag-manager.ts index 5c096ba76a724..b34e57341772e 100644 --- a/packages/@aws-cdk/cdk/test/core/test.tag-manager.ts +++ b/packages/@aws-cdk/cdk/test/core/test.tag-manager.ts @@ -35,7 +35,7 @@ export = { test.deepEqual(construct.tags.resolve(), tagArray); } - test.deepEqual(ctagger2.tags.resolve().length, 0); + test.deepEqual(ctagger2.tags.resolve(), undefined); test.done(); }, 'setTag with propagate false tags do not propagate'(test: Test) { @@ -51,7 +51,7 @@ export = { ctagger.tags.setTag(tag.key, tag.value, {propagate: false}); for (const construct of [ctagger1, ctagger2]) { - test.deepEqual(construct.tags.resolve().length, 0); + test.deepEqual(construct.tags.resolve(), undefined); } test.deepEqual(ctagger.tags.resolve()[0].key, 'Name'); test.deepEqual(ctagger.tags.resolve()[0].value, 'TheCakeIsALie'); @@ -96,7 +96,7 @@ export = { ctagger.tags.setTag(tag.key, tag.value, {propagate: true}); test.deepEqual(ctagger.tags.resolve(), [tag]); ctagger.tags.removeTag(tag.key); - test.deepEqual(ctagger.tags.resolve(), []); + test.deepEqual(ctagger.tags.resolve(), undefined); ctagger.tags.setTag(tag.key, tag.value, {propagate: true}); test.deepEqual(ctagger.tags.resolve(), [tag]); test.done(); @@ -115,7 +115,7 @@ export = { ctagger.tags.removeTag('Name'); for (const construct of [ctagger, ctagger1, ctagger2]) { - test.deepEqual(construct.tags.resolve().length, 0); + test.deepEqual(construct.tags.resolve(), undefined); } test.done(); }, @@ -127,7 +127,7 @@ export = { ctagger1.tags.removeTag('Env', {blockPropagate: true}); const result = ctagger.tags.resolve(); test.deepEqual(result, [{key: 'Env', value: 'Dev'}]); - test.deepEqual(ctagger1.tags.resolve(), []); + test.deepEqual(ctagger1.tags.resolve(), undefined); test.done(); }, 'children can override parent propagated tags'(test: Test) { From 6a055ca1ce093d5dc8a0dfd281640e4b65d45987 Mon Sep 17 00:00:00 2001 From: Jungseok Lee Date: Tue, 9 Oct 2018 21:10:54 -0700 Subject: [PATCH 2/2] Fix build brekage This patch fixes build brekage. --- .../aws-codedeploy/test/integ.deployment-group.expected.json | 1 - .../aws-elasticloadbalancing/test/integ.elb.expected.json | 1 - .../aws-elasticloadbalancingv2/test/integ.alb.expected.json | 1 - packages/@aws-cdk/aws-lambda/test/integ.vpc-lambda.expected.json | 1 - packages/@aws-cdk/aws-rds/test/integ.cluster.expected.json | 1 - 5 files changed, 5 deletions(-) diff --git a/packages/@aws-cdk/aws-codedeploy/test/integ.deployment-group.expected.json b/packages/@aws-cdk/aws-codedeploy/test/integ.deployment-group.expected.json index 8a7c4c281744c..cf70ed442b8c3 100644 --- a/packages/@aws-cdk/aws-codedeploy/test/integ.deployment-group.expected.json +++ b/packages/@aws-cdk/aws-codedeploy/test/integ.deployment-group.expected.json @@ -674,7 +674,6 @@ "ToPort": 80 } ], - "Tags": [], "VpcId": { "Ref": "VPCB9E5F0B4" } 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 458026fd7584e..25da8feca4a7f 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancing/test/integ.elb.expected.json +++ b/packages/@aws-cdk/aws-elasticloadbalancing/test/integ.elb.expected.json @@ -185,7 +185,6 @@ "ToPort": 80 } ], - "Tags": [], "VpcId": { "Ref": "VPCB9E5F0B4" } diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/integ.alb.expected.json b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/integ.alb.expected.json index e56c37e3c76ad..a5c979a0dee9a 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/integ.alb.expected.json +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/integ.alb.expected.json @@ -343,7 +343,6 @@ "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 0c4084d96dda6..84f1345239c11 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,7 +382,6 @@ } ], "SecurityGroupIngress": [], - "Tags": [], "VpcId": { "Ref": "VPCB9E5F0B4" } diff --git a/packages/@aws-cdk/aws-rds/test/integ.cluster.expected.json b/packages/@aws-cdk/aws-rds/test/integ.cluster.expected.json index a6c2dc2522720..c95a923aec045 100644 --- a/packages/@aws-cdk/aws-rds/test/integ.cluster.expected.json +++ b/packages/@aws-cdk/aws-rds/test/integ.cluster.expected.json @@ -382,7 +382,6 @@ "GroupDescription": "RDS security group", "SecurityGroupEgress": [], "SecurityGroupIngress": [], - "Tags": [], "VpcId": { "Ref": "VPCB9E5F0B4" }