From 05d2a7cac6461ead8bc527e312da96a1d0884a3c Mon Sep 17 00:00:00 2001 From: tmokmss Date: Sat, 13 May 2023 18:26:14 +0900 Subject: [PATCH 01/19] feat(cli): allow hotswap of ECS task definitions with tokens --- .../aws-cdk/lib/api/hotswap/ecs-services.ts | 160 ++++++++++++++++-- .../ecs-services-hotswap-deployments.test.ts | 119 +++++++++++++ 2 files changed, 269 insertions(+), 10 deletions(-) diff --git a/packages/aws-cdk/lib/api/hotswap/ecs-services.ts b/packages/aws-cdk/lib/api/hotswap/ecs-services.ts index ad62950fc861b..065a5d0147165 100644 --- a/packages/aws-cdk/lib/api/hotswap/ecs-services.ts +++ b/packages/aws-cdk/lib/api/hotswap/ecs-services.ts @@ -33,7 +33,8 @@ export async function isHotswappableEcsServiceChange( // if there are no resources referencing the TaskDefinition, // hotswap is not possible in FALL_BACK mode reportNonHotswappableChange(ret, change, undefined, 'No ECS services reference the changed task definition', false); - } if (resourcesReferencingTaskDef.length > ecsServicesReferencingTaskDef.length) { + } + if (resourcesReferencingTaskDef.length > ecsServicesReferencingTaskDef.length) { // if something besides an ECS Service is referencing the TaskDefinition, // hotswap is not possible in FALL_BACK mode const nonEcsServiceTaskDefRefs = resourcesReferencingTaskDef.filter(r => r.Type !== 'AWS::ECS::Service'); @@ -45,14 +46,18 @@ export async function isHotswappableEcsServiceChange( const namesOfHotswappableChanges = Object.keys(classifiedChanges.hotswappableProps); if (namesOfHotswappableChanges.length > 0) { const taskDefinitionResource = await prepareTaskDefinitionChange(evaluateCfnTemplate, logicalId, change); + if (taskDefinitionResource === undefined) { + reportNonHotswappableChange(ret, change, undefined, 'Found unsupported changes to the task definition', false); + return ret; + } ret.push({ hotswappable: true, resourceType: change.newValue.Type, propsChanged: namesOfHotswappableChanges, service: 'ecs-service', resourceNames: [ - `ECS Task Definition '${await taskDefinitionResource.Family}'`, ...ecsServicesReferencingTaskDef.map(ecsService => `ECS Service '${ecsService.serviceArn.split('/')[2]}'`), + `ECS Task Definition '${taskDefinitionResource.family}'`, ], apply: async (sdk: ISDK) => { // Step 1 - update the changed TaskDefinition, creating a new TaskDefinition Revision @@ -61,8 +66,8 @@ export async function isHotswappableEcsServiceChange( // The SDK requires more properties here than its worth doing explicit typing for // instead, just use all the old values in the diff to fill them in implicitly - const lowercasedTaskDef = transformObjectKeys(taskDefinitionResource, lowerCaseFirstCharacter, { - // All the properties that take arbitrary string as keys i.e. { "string" : "string" } + let lowercasedTaskDef = transformObjectKeys(taskDefinitionResource.taskDefinition, lowerCaseFirstCharacter, { + // Don't transform the properties that take arbitrary string as keys i.e. { "string" : "string" } // https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_RegisterTaskDefinition.html#API_RegisterTaskDefinition_RequestSyntax ContainerDefinitions: { DockerLabels: true, @@ -80,6 +85,22 @@ export async function isHotswappableEcsServiceChange( }, }, }); + + if (taskDefinitionResource.hotswappableWithCopy) { + // get the latest task definition of the family + const describeTaskDefinitionResponse = await sdk + .ecs() + .describeTaskDefinition({ + taskDefinition: taskDefinitionResource.family, + }) + .promise(); + if (describeTaskDefinitionResponse.taskDefinition === undefined) { + throw new Error(`Could not find TaskDefinition ${taskDefinitionResource.family}`); + } + mergeTaskDefinitions(lowercasedTaskDef, describeTaskDefinitionResponse.taskDefinition); + lowercasedTaskDef = describeTaskDefinitionResponse.taskDefinition; + } + const registerTaskDefResponse = await sdk.ecs().registerTaskDefinition(lowercasedTaskDef).promise(); const taskDefRevArn = registerTaskDefResponse.taskDefinition?.taskDefinitionArn; @@ -167,13 +188,39 @@ export async function isHotswappableEcsServiceChange( return ret; } +function mergeTaskDefinitions(patch: {containerDefinitions: {[key:string]: any}[]}, old: AWS.ECS.TaskDefinition) { + const src = patch.containerDefinitions; + const dst = old.containerDefinitions; + if (dst === undefined) return; + // schema: https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_ContainerDefinition.html + for (const i in src) { + for (const key of Object.keys(src[i])) { + (dst[i] as any)[key] = src[i][key]; + } + } + + // the response of describeTaskDefinition API contains several keys that must not exist as + // a request of registerTaskDefinition API. We remove those keys here. + [ + 'taskDefinitionArn', + 'revision', + 'status', + 'requiresAttributes', + 'compatibilities', + 'registeredAt', + 'registeredBy', + ].forEach(key=> delete (old as any)[key]); +} + interface EcsService { readonly serviceArn: string; } async function prepareTaskDefinitionChange( - evaluateCfnTemplate: EvaluateCloudFormationTemplate, logicalId: string, change: HotswappableChangeCandidate, -) { + evaluateCfnTemplate: EvaluateCloudFormationTemplate, + logicalId: string, + change: HotswappableChangeCandidate, +): Promise { const taskDefinitionResource: { [name: string]: any } = { ...change.oldValue.Properties, ContainerDefinitions: change.newValue.Properties?.ContainerDefinitions, @@ -195,11 +242,104 @@ async function prepareTaskDefinitionChange( // otherwise, familyNameOrArn is just the simple name evaluated from the CloudFormation template : familyNameOrArn; // then, let's evaluate the body of the remainder of the TaskDef (without the Family property) - return { - ...await evaluateCfnTemplate.evaluateCfnExpression({ + + let evaluated; + let hotswappableWithCopy = false; + try { + evaluated = await evaluateCfnTemplate.evaluateCfnExpression({ ...(taskDefinitionResource ?? {}), Family: undefined, - }), - Family: family, + }); + } catch (e) { + const deep = await deepCompareContainerDefinitions( + evaluateCfnTemplate, + change.oldValue.Properties?.ContainerDefinitions, + change.newValue.Properties?.ContainerDefinitions); + if (deep === false) { + throw e; + } else { + evaluated = { + ContainerDefinitions: deep, + }; + hotswappableWithCopy = true; + } + } + + return { + taskDefinition: { + ...evaluated, + Family: family, // override the family + }, + family, + hotswappableWithCopy, }; } + +// return false if container definitions can be partially updated +// i.e. there are no changes containing unsupported tokens or we have to update the entire task definition +// if yes, we return only values that should be updated. +async function deepCompareContainerDefinitions(evaluateCfnTemplate: EvaluateCloudFormationTemplate, oldValue: any[], newValue: any[]) { + const res: any[] = []; + // we can assume both objects have the same structure, allowing to simplify the comparison logic. + // schema: https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_ContainerDefinition.html + if (oldValue.length !== newValue.length) { + // one or more containers are added or removed + return false; + } + for (const i in oldValue) { + res.push({}); + const lhs = oldValue[i]; + const rhs = newValue[i]; + + if (!deepCompareObject(Object.keys(lhs), Object.keys(rhs))) { + // one or more fields are added or removed + return false; + } + for (const key of Object.keys(lhs)) { + // compare two objects deeply first + if (deepCompareObject(lhs[key], rhs[key])) { + // if there is no difference, skip the field + continue; + } + // if there is any diff found, check if it can be evaluated without raising an error + try { + res[i][key] = await evaluateCfnTemplate.evaluateCfnExpression(rhs[key]); + } catch (e) { + // the diff contains unsupported tokens + return false; + } + } + } + + return res; +} + +// return true when two objects are identical +function deepCompareObject(lhs: any, rhs: any) { + if (typeof lhs !== 'object') { + return lhs === rhs; + } + if (Array.isArray(lhs)) { + if (!Array.isArray(rhs)) { + return false; + } + if (lhs.length != rhs.length) { + return false; + } + for (const i in lhs) { + if (!deepCompareObject(lhs[i], rhs[i])) { + return false; + } + } + return true; + } + if (!deepCompareObject(Object.keys(lhs), Object.keys(rhs))) { + return false; + } + for (const key of Object.keys(lhs)) { + if (!deepCompareObject(lhs[key], rhs[key])) { + return false; + } + } + return true; +} diff --git a/packages/aws-cdk/test/api/hotswap/ecs-services-hotswap-deployments.test.ts b/packages/aws-cdk/test/api/hotswap/ecs-services-hotswap-deployments.test.ts index e0267635a59d8..304de3968683a 100644 --- a/packages/aws-cdk/test/api/hotswap/ecs-services-hotswap-deployments.test.ts +++ b/packages/aws-cdk/test/api/hotswap/ecs-services-hotswap-deployments.test.ts @@ -5,15 +5,18 @@ import { HotswapMode } from '../../../lib/api/hotswap/common'; let hotswapMockSdkProvider: setup.HotswapMockSdkProvider; let mockRegisterTaskDef: jest.Mock; +let mockDescribeTaskDef: jest.Mock; let mockUpdateService: (params: AWS.ECS.UpdateServiceRequest) => AWS.ECS.UpdateServiceResponse; beforeEach(() => { hotswapMockSdkProvider = setup.setupHotswapTests(); mockRegisterTaskDef = jest.fn(); + mockDescribeTaskDef = jest.fn(); mockUpdateService = jest.fn(); hotswapMockSdkProvider.stubEcs({ registerTaskDefinition: mockRegisterTaskDef, + describeTaskDefinition: mockDescribeTaskDef, updateService: mockUpdateService, }, { // these are needed for the waiter API that the ECS service hotswap uses @@ -519,6 +522,122 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot } }); + test('should call registerTaskDefinition, describeTaskDefinition, and updateService for a difference only in the container image but with environment variables of unsupported intrinsics', async () => { + // GIVEN + setup.setCurrentCfnStackTemplate({ + Resources: { + TaskDef: { + Type: 'AWS::ECS::TaskDefinition', + Properties: { + Family: 'my-task-def', + ContainerDefinitions: [ + { + Image: 'image1', + Environment: [ + { + Name: 'FOO', + Value: { 'Fn::GetAtt': ['Bar', 'Baz'] }, + }, + ], + }, + ], + }, + }, + Service: { + Type: 'AWS::ECS::Service', + Properties: { + TaskDefinition: { Ref: 'TaskDef' }, + }, + }, + }, + }); + setup.pushStackResourceSummaries( + setup.stackSummaryOf('Service', 'AWS::ECS::Service', + 'arn:aws:ecs:region:account:service/my-cluster/my-service'), + ); + mockRegisterTaskDef.mockReturnValue({ + taskDefinition: { + taskDefinitionArn: 'arn:aws:ecs:region:account:task-definition/my-task-def:3', + }, + }); + mockDescribeTaskDef.mockReturnValue({ + taskDefinition: { + taskDefinitionArn: 'arn:aws:ecs:region:account:task-definition/my-task-def:2', + family: 'my-task-def', + containerDefinitions: [ + { + image: 'image1', + environment: [ + { + name: 'FOO', + value: 'value', + }, + ], + }, + ], + }, + }); + const cdkStackArtifact = setup.cdkStackArtifactOf({ + template: { + Resources: { + TaskDef: { + Type: 'AWS::ECS::TaskDefinition', + Properties: { + Family: 'my-task-def', + ContainerDefinitions: [ + { + Image: 'image2', + Environment: [ + { + Name: 'FOO', + Value: { 'Fn::GetAtt': ['Bar', 'Baz'] }, + }, + ], + }, + ], + }, + }, + Service: { + Type: 'AWS::ECS::Service', + Properties: { + TaskDefinition: { Ref: 'TaskDef' }, + }, + }, + }, + }, + }); + + // WHEN + const deployStackResult = await hotswapMockSdkProvider.tryHotswapDeployment(hotswapMode, cdkStackArtifact); + + // THEN + expect(deployStackResult).not.toBeUndefined(); + expect(mockRegisterTaskDef).toBeCalledWith({ + family: 'my-task-def', + containerDefinitions: [ + { + image: 'image2', + environment: [ + { + + name: 'FOO', + value: 'value', + }, + ], + }, + ], + }); + expect(mockUpdateService).toBeCalledWith({ + service: 'arn:aws:ecs:region:account:service/my-cluster/my-service', + cluster: 'my-cluster', + taskDefinition: 'arn:aws:ecs:region:account:task-definition/my-task-def:3', + deploymentConfiguration: { + minimumHealthyPercent: 0, + }, + forceNewDeployment: true, + }); + }); + test('should call registerTaskDefinition with certain properties not lowercased', async () => { // GIVEN setup.setCurrentCfnStackTemplate({ From 694adbd44f93805890199bfff0c61fcaa77e6aa2 Mon Sep 17 00:00:00 2001 From: tmokmss Date: Sun, 14 May 2023 19:55:09 +0900 Subject: [PATCH 02/19] Update ecs-services.ts --- .../aws-cdk/lib/api/hotswap/ecs-services.ts | 35 +++++++++---------- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/packages/aws-cdk/lib/api/hotswap/ecs-services.ts b/packages/aws-cdk/lib/api/hotswap/ecs-services.ts index 065a5d0147165..056f231fbdb63 100644 --- a/packages/aws-cdk/lib/api/hotswap/ecs-services.ts +++ b/packages/aws-cdk/lib/api/hotswap/ecs-services.ts @@ -251,15 +251,15 @@ async function prepareTaskDefinitionChange( Family: undefined, }); } catch (e) { - const deep = await deepCompareContainerDefinitions( + const result = await deepCompareContainerDefinitions( evaluateCfnTemplate, - change.oldValue.Properties?.ContainerDefinitions, - change.newValue.Properties?.ContainerDefinitions); - if (deep === false) { + change.oldValue.Properties?.ContainerDefinitions ?? [], + change.newValue.Properties?.ContainerDefinitions ?? []); + if (result === false) { throw e; } else { evaluated = { - ContainerDefinitions: deep, + ContainerDefinitions: result, }; hotswappableWithCopy = true; } @@ -268,35 +268,34 @@ async function prepareTaskDefinitionChange( return { taskDefinition: { ...evaluated, - Family: family, // override the family + Family: family, }, family, hotswappableWithCopy, }; } -// return false if container definitions can be partially updated -// i.e. there are no changes containing unsupported tokens or we have to update the entire task definition -// if yes, we return only values that should be updated. -async function deepCompareContainerDefinitions(evaluateCfnTemplate: EvaluateCloudFormationTemplate, oldValue: any[], newValue: any[]) { +// return false if the new container definitions cannot be hotswapped +// i.e. there are changes containing unsupported tokens, or additions or removals of properties +// If not (i.e. hotswappable), we return the properties that should be updated (merged with the deployed task definition later) +async function deepCompareContainerDefinitions(evaluateCfnTemplate: EvaluateCloudFormationTemplate, oldDefinition: any[], newDefinition: any[]) { const res: any[] = []; - // we can assume both objects have the same structure, allowing to simplify the comparison logic. // schema: https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_ContainerDefinition.html - if (oldValue.length !== newValue.length) { + if (oldDefinition.length !== newDefinition.length) { // one or more containers are added or removed return false; } - for (const i in oldValue) { + for (const i in oldDefinition) { res.push({}); - const lhs = oldValue[i]; - const rhs = newValue[i]; + const lhs = oldDefinition[i]; + const rhs = newDefinition[i]; if (!deepCompareObject(Object.keys(lhs), Object.keys(rhs))) { // one or more fields are added or removed return false; } for (const key of Object.keys(lhs)) { - // compare two objects deeply first + // compare two properties first if (deepCompareObject(lhs[key], rhs[key])) { // if there is no difference, skip the field continue; @@ -315,7 +314,7 @@ async function deepCompareContainerDefinitions(evaluateCfnTemplate: EvaluateClou } // return true when two objects are identical -function deepCompareObject(lhs: any, rhs: any) { +function deepCompareObject(lhs: object, rhs: object) { if (typeof lhs !== 'object') { return lhs === rhs; } @@ -337,7 +336,7 @@ function deepCompareObject(lhs: any, rhs: any) { return false; } for (const key of Object.keys(lhs)) { - if (!deepCompareObject(lhs[key], rhs[key])) { + if (!deepCompareObject((lhs as any)[key], (rhs as any)[key])) { return false; } } From 6b42be255c58d0250da3feb962e2d39263d6b531 Mon Sep 17 00:00:00 2001 From: tmokmss Date: Tue, 16 May 2023 16:51:53 +0900 Subject: [PATCH 03/19] Update ecs-services.ts --- packages/aws-cdk/lib/api/hotswap/ecs-services.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/aws-cdk/lib/api/hotswap/ecs-services.ts b/packages/aws-cdk/lib/api/hotswap/ecs-services.ts index 056f231fbdb63..e7d63e0394f30 100644 --- a/packages/aws-cdk/lib/api/hotswap/ecs-services.ts +++ b/packages/aws-cdk/lib/api/hotswap/ecs-services.ts @@ -318,6 +318,9 @@ function deepCompareObject(lhs: object, rhs: object) { if (typeof lhs !== 'object') { return lhs === rhs; } + if (typeof rhs !== 'object') { + return false; + } if (Array.isArray(lhs)) { if (!Array.isArray(rhs)) { return false; From 652489edbd7eadae37d614e75330095108564601 Mon Sep 17 00:00:00 2001 From: tmokmss Date: Sun, 4 Jun 2023 22:12:32 +0900 Subject: [PATCH 04/19] add integ diff --- .../AlarmWithLabelIntegrationTest.assets.json | 6 +- ...larmWithLabelIntegrationTest.template.json | 4 +- .../cdk.out | 2 +- ...efaultTestDeployAssert045C663E.assets.json | 2 +- .../integ.json | 2 +- .../manifest.json | 4 +- .../tree.json | 56 +++++++++---------- .../test/integ.alarm-with-label.ts | 2 +- 8 files changed, 39 insertions(+), 39 deletions(-) diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/AlarmWithLabelIntegrationTest.assets.json b/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/AlarmWithLabelIntegrationTest.assets.json index adef293e4db88..418d01f023638 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/AlarmWithLabelIntegrationTest.assets.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/AlarmWithLabelIntegrationTest.assets.json @@ -1,7 +1,7 @@ { - "version": "30.0.0", + "version": "31.0.0", "files": { - "a6f5e41c0212fcbcdb1d7cdb7acbe27cf764613445faf44701294353f7c98c9b": { + "7dd43c047b72efa6e8541bd455e64dcbeab0ecee6da38a518b91cd170a8e4a5f": { "source": { "path": "AlarmWithLabelIntegrationTest.template.json", "packaging": "file" @@ -9,7 +9,7 @@ "destinations": { "current_account-current_region": { "bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}", - "objectKey": "a6f5e41c0212fcbcdb1d7cdb7acbe27cf764613445faf44701294353f7c98c9b.json", + "objectKey": "7dd43c047b72efa6e8541bd455e64dcbeab0ecee6da38a518b91cd170a8e4a5f.json", "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}" } } diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/AlarmWithLabelIntegrationTest.template.json b/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/AlarmWithLabelIntegrationTest.template.json index d753de4a7c2d0..889a06e848423 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/AlarmWithLabelIntegrationTest.template.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/AlarmWithLabelIntegrationTest.template.json @@ -12,7 +12,7 @@ "MetricStat": { "Metric": { "MetricName": "Metric", - "Namespace": "CDK/Test" + "Namespace": "CDK/Test1" }, "Period": 300, "Stat": "Average" @@ -35,7 +35,7 @@ "MetricStat": { "Metric": { "MetricName": "Metric", - "Namespace": "CDK/Test" + "Namespace": "CDK/Test1" }, "Period": 300, "Stat": "Average" diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/cdk.out b/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/cdk.out index ae4b03c54e770..7925065efbcc4 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/cdk.out +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/cdk.out @@ -1 +1 @@ -{"version":"30.0.0"} \ No newline at end of file +{"version":"31.0.0"} \ No newline at end of file diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/cdkcloudwatchalarmswithlabelintegtestDefaultTestDeployAssert045C663E.assets.json b/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/cdkcloudwatchalarmswithlabelintegtestDefaultTestDeployAssert045C663E.assets.json index a650834f19c4e..d1b7d43900c93 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/cdkcloudwatchalarmswithlabelintegtestDefaultTestDeployAssert045C663E.assets.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/cdkcloudwatchalarmswithlabelintegtestDefaultTestDeployAssert045C663E.assets.json @@ -1,5 +1,5 @@ { - "version": "30.0.0", + "version": "31.0.0", "files": { "21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22": { "source": { diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/integ.json b/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/integ.json index c892545d7a29d..dabd3cbbbd040 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/integ.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/integ.json @@ -1,5 +1,5 @@ { - "version": "30.0.0", + "version": "31.0.0", "testCases": { "cdk-cloudwatch-alarms-with-label-integ-test/DefaultTest": { "stacks": [ diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/manifest.json b/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/manifest.json index ccd6946844384..49ac9220ebe63 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/manifest.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/manifest.json @@ -1,5 +1,5 @@ { - "version": "30.0.0", + "version": "32.0.0", "artifacts": { "AlarmWithLabelIntegrationTest.assets": { "type": "cdk:asset-manifest", @@ -17,7 +17,7 @@ "validateOnSynth": false, "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}", "cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}", - "stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/a6f5e41c0212fcbcdb1d7cdb7acbe27cf764613445faf44701294353f7c98c9b.json", + "stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/7dd43c047b72efa6e8541bd455e64dcbeab0ecee6da38a518b91cd170a8e4a5f.json", "requiresBootstrapStackVersion": 6, "bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version", "additionalDependencies": [ diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/tree.json b/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/tree.json index 0413a5583abfe..fb8c38dbde1b0 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/tree.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/tree.json @@ -25,7 +25,7 @@ "metricStat": { "metric": { "metricName": "Metric", - "namespace": "CDK/Test" + "namespace": "CDK/Test1" }, "period": 300, "stat": "Average" @@ -39,14 +39,14 @@ } }, "constructInfo": { - "fqn": "@aws-cdk/aws-cloudwatch.CfnAlarm", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.26" } } }, "constructInfo": { - "fqn": "@aws-cdk/aws-cloudwatch.Alarm", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.26" } }, "Alarm2": { @@ -66,7 +66,7 @@ "metricStat": { "metric": { "metricName": "Metric", - "namespace": "CDK/Test" + "namespace": "CDK/Test1" }, "period": 300, "stat": "Average" @@ -80,36 +80,36 @@ } }, "constructInfo": { - "fqn": "@aws-cdk/aws-cloudwatch.CfnAlarm", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.26" } } }, "constructInfo": { - "fqn": "@aws-cdk/aws-cloudwatch.Alarm", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.26" } }, "BootstrapVersion": { "id": "BootstrapVersion", "path": "AlarmWithLabelIntegrationTest/BootstrapVersion", "constructInfo": { - "fqn": "@aws-cdk/core.CfnParameter", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.26" } }, "CheckBootstrapVersion": { "id": "CheckBootstrapVersion", "path": "AlarmWithLabelIntegrationTest/CheckBootstrapVersion", "constructInfo": { - "fqn": "@aws-cdk/core.CfnRule", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.26" } } }, "constructInfo": { - "fqn": "@aws-cdk/core.Stack", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.26" } }, "cdk-cloudwatch-alarms-with-label-integ-test": { @@ -125,7 +125,7 @@ "path": "cdk-cloudwatch-alarms-with-label-integ-test/DefaultTest/Default", "constructInfo": { "fqn": "constructs.Construct", - "version": "10.1.252" + "version": "10.2.26" } }, "DeployAssert": { @@ -136,33 +136,33 @@ "id": "BootstrapVersion", "path": "cdk-cloudwatch-alarms-with-label-integ-test/DefaultTest/DeployAssert/BootstrapVersion", "constructInfo": { - "fqn": "@aws-cdk/core.CfnParameter", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.26" } }, "CheckBootstrapVersion": { "id": "CheckBootstrapVersion", "path": "cdk-cloudwatch-alarms-with-label-integ-test/DefaultTest/DeployAssert/CheckBootstrapVersion", "constructInfo": { - "fqn": "@aws-cdk/core.CfnRule", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.26" } } }, "constructInfo": { - "fqn": "@aws-cdk/core.Stack", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.26" } } }, "constructInfo": { - "fqn": "@aws-cdk/integ-tests.IntegTestCase", + "fqn": "@aws-cdk/integ-tests-alpha.IntegTestCase", "version": "0.0.0" } } }, "constructInfo": { - "fqn": "@aws-cdk/integ-tests.IntegTest", + "fqn": "@aws-cdk/integ-tests-alpha.IntegTest", "version": "0.0.0" } }, @@ -171,13 +171,13 @@ "path": "Tree", "constructInfo": { "fqn": "constructs.Construct", - "version": "10.1.252" + "version": "10.2.26" } } }, "constructInfo": { - "fqn": "@aws-cdk/core.App", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.26" } } } \ No newline at end of file diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.ts b/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.ts index daa98ddd86198..4dafacbb59b91 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.ts +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.ts @@ -8,7 +8,7 @@ class AlarmWithLabelIntegrationTest extends Stack { super(scope, id, props); const testMetric = new Metric({ - namespace: 'CDK/Test', + namespace: 'CDK/Test1', metricName: 'Metric', label: 'Metric [AVG: ${AVG}]', }); From b79959dcb19c632d8adfbf8e437e016874481978 Mon Sep 17 00:00:00 2001 From: tmokmss Date: Sun, 4 Jun 2023 23:18:26 +0900 Subject: [PATCH 05/19] Revert "add integ diff" This reverts commit 652489edbd7eadae37d614e75330095108564601. --- .../AlarmWithLabelIntegrationTest.assets.json | 6 +- ...larmWithLabelIntegrationTest.template.json | 4 +- .../cdk.out | 2 +- ...efaultTestDeployAssert045C663E.assets.json | 2 +- .../integ.json | 2 +- .../manifest.json | 4 +- .../tree.json | 56 +++++++++---------- .../test/integ.alarm-with-label.ts | 2 +- 8 files changed, 39 insertions(+), 39 deletions(-) diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/AlarmWithLabelIntegrationTest.assets.json b/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/AlarmWithLabelIntegrationTest.assets.json index 418d01f023638..adef293e4db88 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/AlarmWithLabelIntegrationTest.assets.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/AlarmWithLabelIntegrationTest.assets.json @@ -1,7 +1,7 @@ { - "version": "31.0.0", + "version": "30.0.0", "files": { - "7dd43c047b72efa6e8541bd455e64dcbeab0ecee6da38a518b91cd170a8e4a5f": { + "a6f5e41c0212fcbcdb1d7cdb7acbe27cf764613445faf44701294353f7c98c9b": { "source": { "path": "AlarmWithLabelIntegrationTest.template.json", "packaging": "file" @@ -9,7 +9,7 @@ "destinations": { "current_account-current_region": { "bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}", - "objectKey": "7dd43c047b72efa6e8541bd455e64dcbeab0ecee6da38a518b91cd170a8e4a5f.json", + "objectKey": "a6f5e41c0212fcbcdb1d7cdb7acbe27cf764613445faf44701294353f7c98c9b.json", "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}" } } diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/AlarmWithLabelIntegrationTest.template.json b/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/AlarmWithLabelIntegrationTest.template.json index 889a06e848423..d753de4a7c2d0 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/AlarmWithLabelIntegrationTest.template.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/AlarmWithLabelIntegrationTest.template.json @@ -12,7 +12,7 @@ "MetricStat": { "Metric": { "MetricName": "Metric", - "Namespace": "CDK/Test1" + "Namespace": "CDK/Test" }, "Period": 300, "Stat": "Average" @@ -35,7 +35,7 @@ "MetricStat": { "Metric": { "MetricName": "Metric", - "Namespace": "CDK/Test1" + "Namespace": "CDK/Test" }, "Period": 300, "Stat": "Average" diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/cdk.out b/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/cdk.out index 7925065efbcc4..ae4b03c54e770 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/cdk.out +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/cdk.out @@ -1 +1 @@ -{"version":"31.0.0"} \ No newline at end of file +{"version":"30.0.0"} \ No newline at end of file diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/cdkcloudwatchalarmswithlabelintegtestDefaultTestDeployAssert045C663E.assets.json b/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/cdkcloudwatchalarmswithlabelintegtestDefaultTestDeployAssert045C663E.assets.json index d1b7d43900c93..a650834f19c4e 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/cdkcloudwatchalarmswithlabelintegtestDefaultTestDeployAssert045C663E.assets.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/cdkcloudwatchalarmswithlabelintegtestDefaultTestDeployAssert045C663E.assets.json @@ -1,5 +1,5 @@ { - "version": "31.0.0", + "version": "30.0.0", "files": { "21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22": { "source": { diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/integ.json b/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/integ.json index dabd3cbbbd040..c892545d7a29d 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/integ.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/integ.json @@ -1,5 +1,5 @@ { - "version": "31.0.0", + "version": "30.0.0", "testCases": { "cdk-cloudwatch-alarms-with-label-integ-test/DefaultTest": { "stacks": [ diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/manifest.json b/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/manifest.json index 49ac9220ebe63..ccd6946844384 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/manifest.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/manifest.json @@ -1,5 +1,5 @@ { - "version": "32.0.0", + "version": "30.0.0", "artifacts": { "AlarmWithLabelIntegrationTest.assets": { "type": "cdk:asset-manifest", @@ -17,7 +17,7 @@ "validateOnSynth": false, "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}", "cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}", - "stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/7dd43c047b72efa6e8541bd455e64dcbeab0ecee6da38a518b91cd170a8e4a5f.json", + "stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/a6f5e41c0212fcbcdb1d7cdb7acbe27cf764613445faf44701294353f7c98c9b.json", "requiresBootstrapStackVersion": 6, "bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version", "additionalDependencies": [ diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/tree.json b/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/tree.json index fb8c38dbde1b0..0413a5583abfe 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/tree.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.js.snapshot/tree.json @@ -25,7 +25,7 @@ "metricStat": { "metric": { "metricName": "Metric", - "namespace": "CDK/Test1" + "namespace": "CDK/Test" }, "period": 300, "stat": "Average" @@ -39,14 +39,14 @@ } }, "constructInfo": { - "fqn": "constructs.Construct", - "version": "10.2.26" + "fqn": "@aws-cdk/aws-cloudwatch.CfnAlarm", + "version": "0.0.0" } } }, "constructInfo": { - "fqn": "constructs.Construct", - "version": "10.2.26" + "fqn": "@aws-cdk/aws-cloudwatch.Alarm", + "version": "0.0.0" } }, "Alarm2": { @@ -66,7 +66,7 @@ "metricStat": { "metric": { "metricName": "Metric", - "namespace": "CDK/Test1" + "namespace": "CDK/Test" }, "period": 300, "stat": "Average" @@ -80,36 +80,36 @@ } }, "constructInfo": { - "fqn": "constructs.Construct", - "version": "10.2.26" + "fqn": "@aws-cdk/aws-cloudwatch.CfnAlarm", + "version": "0.0.0" } } }, "constructInfo": { - "fqn": "constructs.Construct", - "version": "10.2.26" + "fqn": "@aws-cdk/aws-cloudwatch.Alarm", + "version": "0.0.0" } }, "BootstrapVersion": { "id": "BootstrapVersion", "path": "AlarmWithLabelIntegrationTest/BootstrapVersion", "constructInfo": { - "fqn": "constructs.Construct", - "version": "10.2.26" + "fqn": "@aws-cdk/core.CfnParameter", + "version": "0.0.0" } }, "CheckBootstrapVersion": { "id": "CheckBootstrapVersion", "path": "AlarmWithLabelIntegrationTest/CheckBootstrapVersion", "constructInfo": { - "fqn": "constructs.Construct", - "version": "10.2.26" + "fqn": "@aws-cdk/core.CfnRule", + "version": "0.0.0" } } }, "constructInfo": { - "fqn": "constructs.Construct", - "version": "10.2.26" + "fqn": "@aws-cdk/core.Stack", + "version": "0.0.0" } }, "cdk-cloudwatch-alarms-with-label-integ-test": { @@ -125,7 +125,7 @@ "path": "cdk-cloudwatch-alarms-with-label-integ-test/DefaultTest/Default", "constructInfo": { "fqn": "constructs.Construct", - "version": "10.2.26" + "version": "10.1.252" } }, "DeployAssert": { @@ -136,33 +136,33 @@ "id": "BootstrapVersion", "path": "cdk-cloudwatch-alarms-with-label-integ-test/DefaultTest/DeployAssert/BootstrapVersion", "constructInfo": { - "fqn": "constructs.Construct", - "version": "10.2.26" + "fqn": "@aws-cdk/core.CfnParameter", + "version": "0.0.0" } }, "CheckBootstrapVersion": { "id": "CheckBootstrapVersion", "path": "cdk-cloudwatch-alarms-with-label-integ-test/DefaultTest/DeployAssert/CheckBootstrapVersion", "constructInfo": { - "fqn": "constructs.Construct", - "version": "10.2.26" + "fqn": "@aws-cdk/core.CfnRule", + "version": "0.0.0" } } }, "constructInfo": { - "fqn": "constructs.Construct", - "version": "10.2.26" + "fqn": "@aws-cdk/core.Stack", + "version": "0.0.0" } } }, "constructInfo": { - "fqn": "@aws-cdk/integ-tests-alpha.IntegTestCase", + "fqn": "@aws-cdk/integ-tests.IntegTestCase", "version": "0.0.0" } } }, "constructInfo": { - "fqn": "@aws-cdk/integ-tests-alpha.IntegTest", + "fqn": "@aws-cdk/integ-tests.IntegTest", "version": "0.0.0" } }, @@ -171,13 +171,13 @@ "path": "Tree", "constructInfo": { "fqn": "constructs.Construct", - "version": "10.2.26" + "version": "10.1.252" } } }, "constructInfo": { - "fqn": "constructs.Construct", - "version": "10.2.26" + "fqn": "@aws-cdk/core.App", + "version": "0.0.0" } } } \ No newline at end of file diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.ts b/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.ts index 4dafacbb59b91..daa98ddd86198 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.ts +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-cloudwatch/test/integ.alarm-with-label.ts @@ -8,7 +8,7 @@ class AlarmWithLabelIntegrationTest extends Stack { super(scope, id, props); const testMetric = new Metric({ - namespace: 'CDK/Test1', + namespace: 'CDK/Test', metricName: 'Metric', label: 'Metric [AVG: ${AVG}]', }); From fc897d2fed4f6019276f43363cce6ecfa3b94f81 Mon Sep 17 00:00:00 2001 From: tmokmss Date: Sat, 15 Jul 2023 18:55:50 +0900 Subject: [PATCH 06/19] refactor --- .../aws-cdk/lib/api/hotswap/ecs-services.ts | 73 +++++++++---------- 1 file changed, 35 insertions(+), 38 deletions(-) diff --git a/packages/aws-cdk/lib/api/hotswap/ecs-services.ts b/packages/aws-cdk/lib/api/hotswap/ecs-services.ts index e7d63e0394f30..7c4e84ca94dc9 100644 --- a/packages/aws-cdk/lib/api/hotswap/ecs-services.ts +++ b/packages/aws-cdk/lib/api/hotswap/ecs-services.ts @@ -86,18 +86,19 @@ export async function isHotswappableEcsServiceChange( }, }); - if (taskDefinitionResource.hotswappableWithCopy) { + if (taskDefinitionResource.hotswappableWithMerge) { // get the latest task definition of the family const describeTaskDefinitionResponse = await sdk .ecs() .describeTaskDefinition({ taskDefinition: taskDefinitionResource.family, + include: ['TAGS'], }) .promise(); if (describeTaskDefinitionResponse.taskDefinition === undefined) { throw new Error(`Could not find TaskDefinition ${taskDefinitionResource.family}`); } - mergeTaskDefinitions(lowercasedTaskDef, describeTaskDefinitionResponse.taskDefinition); + mergeTaskDefinitions(lowercasedTaskDef, describeTaskDefinitionResponse); lowercasedTaskDef = describeTaskDefinitionResponse.taskDefinition; } @@ -188,9 +189,9 @@ export async function isHotswappableEcsServiceChange( return ret; } -function mergeTaskDefinitions(patch: {containerDefinitions: {[key:string]: any}[]}, old: AWS.ECS.TaskDefinition) { +function mergeTaskDefinitions(patch: {containerDefinitions: {[key:string]: any}[]}, target: AWS.ECS.DescribeTaskDefinitionResponse) { const src = patch.containerDefinitions; - const dst = old.containerDefinitions; + const dst = target.taskDefinition?.containerDefinitions; if (dst === undefined) return; // schema: https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_ContainerDefinition.html for (const i in src) { @@ -199,9 +200,13 @@ function mergeTaskDefinitions(patch: {containerDefinitions: {[key:string]: any}[ } } - // the response of describeTaskDefinition API contains several keys that must not exist as - // a request of registerTaskDefinition API. We remove those keys here. + // The response of describeTaskDefinition API contains several keys that must not exist as a request of registerTaskDefinition API. + // We remove these keys here. + // Compare these two structs: + // https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_RegisterTaskDefinition.html#API_RegisterTaskDefinition_RequestSyntax + // https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_DescribeTaskDefinition.html#API_DescribeTaskDefinition_ResponseSyntax [ + 'compatibilities', 'taskDefinitionArn', 'revision', 'status', @@ -209,7 +214,12 @@ function mergeTaskDefinitions(patch: {containerDefinitions: {[key:string]: any}[ 'compatibilities', 'registeredAt', 'registeredBy', - ].forEach(key=> delete (old as any)[key]); + ].forEach(key=> delete (target.taskDefinition as any)[key]); + + if (target.tags !== undefined && target.tags.length > 0) { + // tags field is in different layer in describe result, moving to the intended location + (target.taskDefinition as any).tags = target.tags; + } } interface EcsService { @@ -220,7 +230,7 @@ async function prepareTaskDefinitionChange( evaluateCfnTemplate: EvaluateCloudFormationTemplate, logicalId: string, change: HotswappableChangeCandidate, -): Promise { +): Promise { const taskDefinitionResource: { [name: string]: any } = { ...change.oldValue.Properties, ContainerDefinitions: change.newValue.Properties?.ContainerDefinitions, @@ -244,7 +254,7 @@ async function prepareTaskDefinitionChange( // then, let's evaluate the body of the remainder of the TaskDef (without the Family property) let evaluated; - let hotswappableWithCopy = false; + let hotswappableWithMerge = false; try { evaluated = await evaluateCfnTemplate.evaluateCfnExpression({ ...(taskDefinitionResource ?? {}), @@ -259,9 +269,10 @@ async function prepareTaskDefinitionChange( throw e; } else { evaluated = { + // return a partial definition that should be merged with the current definition ContainerDefinitions: result, }; - hotswappableWithCopy = true; + hotswappableWithMerge = true; } } @@ -271,46 +282,46 @@ async function prepareTaskDefinitionChange( Family: family, }, family, - hotswappableWithCopy, + hotswappableWithMerge, }; } // return false if the new container definitions cannot be hotswapped -// i.e. there are changes containing unsupported tokens, or additions or removals of properties -// If not (i.e. hotswappable), we return the properties that should be updated (merged with the deployed task definition later) +// i.e. there are changes containing unsupported tokens or additions/removals of properties +// If not, we return the properties that should be updated (they will be merged with the deployed task definition later) async function deepCompareContainerDefinitions(evaluateCfnTemplate: EvaluateCloudFormationTemplate, oldDefinition: any[], newDefinition: any[]) { - const res: any[] = []; + const result: any[] = []; // schema: https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_ContainerDefinition.html if (oldDefinition.length !== newDefinition.length) { // one or more containers are added or removed return false; } for (const i in oldDefinition) { - res.push({}); - const lhs = oldDefinition[i]; - const rhs = newDefinition[i]; + result.push({}); + const prev = oldDefinition[i]; + const next = newDefinition[i]; - if (!deepCompareObject(Object.keys(lhs), Object.keys(rhs))) { + if (!deepCompareObject(Object.keys(prev), Object.keys(next))) { // one or more fields are added or removed return false; } - for (const key of Object.keys(lhs)) { + for (const key of Object.keys(prev)) { // compare two properties first - if (deepCompareObject(lhs[key], rhs[key])) { + if (deepCompareObject(prev[key], next[key])) { // if there is no difference, skip the field continue; } // if there is any diff found, check if it can be evaluated without raising an error try { - res[i][key] = await evaluateCfnTemplate.evaluateCfnExpression(rhs[key]); + result[i][key] = await evaluateCfnTemplate.evaluateCfnExpression(next[key]); } catch (e) { - // the diff contains unsupported tokens + // Give up hotswap if the diff contains unsupported expression return false; } } } - return res; + return result; } // return true when two objects are identical @@ -321,21 +332,7 @@ function deepCompareObject(lhs: object, rhs: object) { if (typeof rhs !== 'object') { return false; } - if (Array.isArray(lhs)) { - if (!Array.isArray(rhs)) { - return false; - } - if (lhs.length != rhs.length) { - return false; - } - for (const i in lhs) { - if (!deepCompareObject(lhs[i], rhs[i])) { - return false; - } - } - return true; - } - if (!deepCompareObject(Object.keys(lhs), Object.keys(rhs))) { + if (Object.keys(lhs).length != Object.keys(rhs).length) { return false; } for (const key of Object.keys(lhs)) { From 84ebf8cc2569566f1a1c2b5674172fcffd85b606 Mon Sep 17 00:00:00 2001 From: tmokmss Date: Wed, 19 Jul 2023 00:10:34 +0900 Subject: [PATCH 07/19] Update ecs-services.ts --- packages/aws-cdk/lib/api/hotswap/ecs-services.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/aws-cdk/lib/api/hotswap/ecs-services.ts b/packages/aws-cdk/lib/api/hotswap/ecs-services.ts index 7c4e84ca94dc9..4716b212c1836 100644 --- a/packages/aws-cdk/lib/api/hotswap/ecs-services.ts +++ b/packages/aws-cdk/lib/api/hotswap/ecs-services.ts @@ -200,9 +200,8 @@ function mergeTaskDefinitions(patch: {containerDefinitions: {[key:string]: any}[ } } - // The response of describeTaskDefinition API contains several keys that must not exist as a request of registerTaskDefinition API. - // We remove these keys here. - // Compare these two structs: + // The describeTaskDefinition response contains several keys that must not exist in a registerTaskDefinition request. + // We remove these keys here, comparing these two structs: // https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_RegisterTaskDefinition.html#API_RegisterTaskDefinition_RequestSyntax // https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_DescribeTaskDefinition.html#API_DescribeTaskDefinition_ResponseSyntax [ @@ -217,7 +216,7 @@ function mergeTaskDefinitions(patch: {containerDefinitions: {[key:string]: any}[ ].forEach(key=> delete (target.taskDefinition as any)[key]); if (target.tags !== undefined && target.tags.length > 0) { - // tags field is in different layer in describe result, moving to the intended location + // the tags field is in a different location in describeTaskDefinition response, moving it as intended for registerTaskDefinition request. (target.taskDefinition as any).tags = target.tags; } } From 64f2c20310ba09673c37d3d6d8700f730b957bf0 Mon Sep 17 00:00:00 2001 From: Masashi Tomooka Date: Sat, 29 Jul 2023 14:03:49 +0900 Subject: [PATCH 08/19] Update packages/aws-cdk/lib/api/hotswap/ecs-services.ts Co-authored-by: Calvin Combs <66279577+comcalvi@users.noreply.github.com> --- packages/aws-cdk/lib/api/hotswap/ecs-services.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/aws-cdk/lib/api/hotswap/ecs-services.ts b/packages/aws-cdk/lib/api/hotswap/ecs-services.ts index 4716b212c1836..d48e227a1d4ee 100644 --- a/packages/aws-cdk/lib/api/hotswap/ecs-services.ts +++ b/packages/aws-cdk/lib/api/hotswap/ecs-services.ts @@ -192,7 +192,9 @@ export async function isHotswappableEcsServiceChange( function mergeTaskDefinitions(patch: {containerDefinitions: {[key:string]: any}[]}, target: AWS.ECS.DescribeTaskDefinitionResponse) { const src = patch.containerDefinitions; const dst = target.taskDefinition?.containerDefinitions; - if (dst === undefined) return; + if (dst === undefined) { + return; + } // schema: https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_ContainerDefinition.html for (const i in src) { for (const key of Object.keys(src[i])) { From f0284c0f57c23992863df5f788bc6a9e6beeaf6a Mon Sep 17 00:00:00 2001 From: tmokmss Date: Mon, 7 Aug 2023 01:30:03 +0900 Subject: [PATCH 09/19] address review comments --- .../aws-cdk/lib/api/hotswap/ecs-services.ts | 57 ++++++++++++++++--- 1 file changed, 48 insertions(+), 9 deletions(-) diff --git a/packages/aws-cdk/lib/api/hotswap/ecs-services.ts b/packages/aws-cdk/lib/api/hotswap/ecs-services.ts index d48e227a1d4ee..0ffa23829c81c 100644 --- a/packages/aws-cdk/lib/api/hotswap/ecs-services.ts +++ b/packages/aws-cdk/lib/api/hotswap/ecs-services.ts @@ -1,7 +1,7 @@ import * as AWS from 'aws-sdk'; import { ChangeHotswapResult, classifyChanges, HotswappableChangeCandidate, lowerCaseFirstCharacter, reportNonHotswappableChange, transformObjectKeys } from './common'; import { ISDK } from '../aws-auth'; -import { EvaluateCloudFormationTemplate } from '../evaluate-cloudformation-template'; +import { CfnEvaluationException, EvaluateCloudFormationTemplate } from '../evaluate-cloudformation-template'; export async function isHotswappableEcsServiceChange( logicalId: string, change: HotswappableChangeCandidate, evaluateCfnTemplate: EvaluateCloudFormationTemplate, @@ -98,8 +98,11 @@ export async function isHotswappableEcsServiceChange( if (describeTaskDefinitionResponse.taskDefinition === undefined) { throw new Error(`Could not find TaskDefinition ${taskDefinitionResource.family}`); } - mergeTaskDefinitions(lowercasedTaskDef, describeTaskDefinitionResponse); - lowercasedTaskDef = describeTaskDefinitionResponse.taskDefinition; + const merged = mergeTaskDefinitions(lowercasedTaskDef, describeTaskDefinitionResponse); + if (merged === undefined) { + throw new Error('Failed to merge the task definition. Please try deploying without hotswap first.'); + } + lowercasedTaskDef = merged.taskDefinition; } const registerTaskDefResponse = await sdk.ecs().registerTaskDefinition(lowercasedTaskDef).promise(); @@ -191,12 +194,15 @@ export async function isHotswappableEcsServiceChange( function mergeTaskDefinitions(patch: {containerDefinitions: {[key:string]: any}[]}, target: AWS.ECS.DescribeTaskDefinitionResponse) { const src = patch.containerDefinitions; + // deep copy target to avoid side effects. The response of AWS API is in JSON format, so safe to use JSON.stringify. + target = JSON.parse(JSON.stringify(target)); const dst = target.taskDefinition?.containerDefinitions; if (dst === undefined) { return; } // schema: https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_ContainerDefinition.html for (const i in src) { + if (dst[i] === undefined) return; for (const key of Object.keys(src[i])) { (dst[i] as any)[key] = src[i][key]; } @@ -220,18 +226,39 @@ function mergeTaskDefinitions(patch: {containerDefinitions: {[key:string]: any}[ if (target.tags !== undefined && target.tags.length > 0) { // the tags field is in a different location in describeTaskDefinition response, moving it as intended for registerTaskDefinition request. (target.taskDefinition as any).tags = target.tags; + delete target.tags; } + + return target; } interface EcsService { readonly serviceArn: string; } +type TaskDefinitionChange = { + /** + * A new task definition that should be deployed. + * If hotswappableWithMerge equals true, this only contains diffs that should be merged with the currently deployed task definition. + */ + taskDefinition: any; + + /** + * A family for this task definition + */ + family: string; + + /** + * true if the change can be hotswapped by merging with the currently deployed task definition + */ + hotswappableWithMerge: boolean +}; + async function prepareTaskDefinitionChange( evaluateCfnTemplate: EvaluateCloudFormationTemplate, logicalId: string, change: HotswappableChangeCandidate, -): Promise { +): Promise { const taskDefinitionResource: { [name: string]: any } = { ...change.oldValue.Properties, ContainerDefinitions: change.newValue.Properties?.ContainerDefinitions, @@ -262,6 +289,9 @@ async function prepareTaskDefinitionChange( Family: undefined, }); } catch (e) { + if (!(e instanceof CfnEvaluationException)) { + throw e; + } const result = await deepCompareContainerDefinitions( evaluateCfnTemplate, change.oldValue.Properties?.ContainerDefinitions ?? [], @@ -287,9 +317,12 @@ async function prepareTaskDefinitionChange( }; } -// return false if the new container definitions cannot be hotswapped -// i.e. there are changes containing unsupported tokens or additions/removals of properties -// If not, we return the properties that should be updated (they will be merged with the deployed task definition later) +/** + * return false if the new container definitions cannot be hotswapped + * i.e. there are changes containing unsupported tokens or additions/removals of properties + * + * Otherwise we return the properties that should be updated (they will be merged with the deployed task definition later) + */ async function deepCompareContainerDefinitions(evaluateCfnTemplate: EvaluateCloudFormationTemplate, oldDefinition: any[], newDefinition: any[]) { const result: any[] = []; // schema: https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_ContainerDefinition.html @@ -306,6 +339,7 @@ async function deepCompareContainerDefinitions(evaluateCfnTemplate: EvaluateClou // one or more fields are added or removed return false; } + // We don't recurse properties to keep the update logic simple. It should still cover most hotswap use cases. for (const key of Object.keys(prev)) { // compare two properties first if (deepCompareObject(prev[key], next[key])) { @@ -316,6 +350,9 @@ async function deepCompareContainerDefinitions(evaluateCfnTemplate: EvaluateClou try { result[i][key] = await evaluateCfnTemplate.evaluateCfnExpression(next[key]); } catch (e) { + if (!(e instanceof CfnEvaluationException)) { + throw e; + } // Give up hotswap if the diff contains unsupported expression return false; } @@ -325,8 +362,10 @@ async function deepCompareContainerDefinitions(evaluateCfnTemplate: EvaluateClou return result; } -// return true when two objects are identical -function deepCompareObject(lhs: object, rhs: object) { +/** + * return true when two objects are identical + */ +function deepCompareObject(lhs: any, rhs: any): boolean { if (typeof lhs !== 'object') { return lhs === rhs; } From c90504e3e2fa4449be5728d27a17e45ffa04025f Mon Sep 17 00:00:00 2001 From: tmokmss Date: Wed, 9 Aug 2023 13:17:52 +0900 Subject: [PATCH 10/19] extract common functions --- .../aws-cdk/lib/api/hotswap-deployments.ts | 179 ++++++++++- packages/aws-cdk/lib/api/hotswap/common.ts | 7 + .../aws-cdk/lib/api/hotswap/ecs-services.ts | 295 +++++------------- 3 files changed, 270 insertions(+), 211 deletions(-) diff --git a/packages/aws-cdk/lib/api/hotswap-deployments.ts b/packages/aws-cdk/lib/api/hotswap-deployments.ts index eb03bdba5a8fb..adfc627d2f328 100644 --- a/packages/aws-cdk/lib/api/hotswap-deployments.ts +++ b/packages/aws-cdk/lib/api/hotswap-deployments.ts @@ -3,7 +3,7 @@ import * as cxapi from '@aws-cdk/cx-api'; import * as chalk from 'chalk'; import { ISDK, Mode, SdkProvider } from './aws-auth'; import { DeployStackResult } from './deploy-stack'; -import { EvaluateCloudFormationTemplate, LazyListStackResources } from './evaluate-cloudformation-template'; +import { CfnEvaluationException, EvaluateCloudFormationTemplate, LazyListStackResources } from './evaluate-cloudformation-template'; import { isHotswappableAppSyncChange } from './hotswap/appsync-mapping-templates'; import { isHotswappableCodeBuildProjectChange } from './hotswap/code-build-projects'; import { ICON, ChangeHotswapResult, HotswapMode, HotswappableChange, NonHotswappableChange, HotswappableChangeCandidate, ClassifiedResourceChanges, reportNonHotswappableChange } from './hotswap/common'; @@ -376,3 +376,180 @@ function logNonHotswappableChanges(nonHotswappableChanges: NonHotswappableChange print(''); // newline } + +type ChangedProps = { key: string[]; type: 'removed' | 'added'; value?: any }; + +export function detectChangedProps(next: any, prev: any): ChangedProps[] { + const changedProps: ChangedProps[] = []; + changedProps.push(...detectAdditions(next, prev)); + changedProps.push(...detectRemovals(next, prev)); + return changedProps; +} + +function detectAdditions(next: any, prev: any, keys: string[] = []): ChangedProps[] { + const changedProps: ChangedProps[] = []; + // Compare each value of two objects, detect additions (added or modified properties) + // If we encounter CFn intrinsic (key.startsWith('Fn::') || key == 'Ref'), stop recursion + if (typeof next !== 'object') { + if (next !== prev) { + // there is an addition or change to the property + return [{ key: new Array(...keys), type: 'added' }]; + } else { + return []; + } + } + if (typeof prev !== 'object') { + // there is an addition or change to the property + return [{ key: new Array(...keys), type: 'added' }]; + } + + // If the lhs is an object but also a CFn intrinsic, don't recurse further. + const lastKey = keys.length > 0 ? keys[keys.length - 1] : ''; + if (lastKey.startsWith('Fn::') || lastKey == 'Ref') { + if (!deepCompareObject(prev, next)) { + // there is an addition or change to the property + return [{ key: new Array(...keys.slice(0, -1)), type: 'added' }]; + } else { + return []; + } + } + + // compare children + for (const key of Object.keys(next)) { + keys.push(key); + changedProps.push(...detectAdditions((next as any)[key], (prev as any)[key], keys)); + keys.pop(); + } + return changedProps; +} + +function detectRemovals(next: any, prev: any, keys: string[] = []): ChangedProps[] { + // Compare each value of two objects, detect removed properties + // To do this, find any keys that exist only in prev object. + // If we encounter CFn intrinsic (key.startsWith('Fn::') || key == 'Ref'), stop recursion + const changedProps: ChangedProps[] = []; + if (next === undefined) { + return [{ key: new Array(...keys), type: 'removed' }]; + } + + if (typeof prev !== 'object' || typeof next !== 'object') { + // either prev or next is not an object, then the property is modified but not removed + return []; + } + + // If the prev is an object but also a CFn intrinsic, don't recurse further. + const lastKey = keys.length > 0 ? keys[keys.length - 1] : ''; + if (lastKey.startsWith('Fn::') || lastKey == 'Ref') { + // next is not undefined here, so the property is at least not removed + return []; + } + + // compare children + for (const key of Object.keys(prev)) { + keys.push(key); + changedProps.push(...detectRemovals((next as any)[key], (prev as any)[key], keys)); + keys.pop(); + } + return changedProps; +} + +/** + * return true when two objects are identical + */ +function deepCompareObject(lhs: any, rhs: any): boolean { + if (typeof lhs !== 'object') { + return lhs === rhs; + } + if (typeof rhs !== 'object') { + return false; + } + if (Object.keys(lhs).length != Object.keys(rhs).length) { + return false; + } + for (const key of Object.keys(lhs)) { + if (!deepCompareObject((lhs as any)[key], (rhs as any)[key])) { + return false; + } + } + return true; +} + +interface HotswappablePropertyUpdates { + readonly updates: ChangedProps[]; + readonly unevaluatableUpdates: ChangedProps[]; +} + +/** + * Diff each property of the changes, and check if each diff can be actually hotswapped (i.e. evaluated by EvaluateCloudFormationTemplate.) + * If any diff cannot be evaluated, they are reported by unevaluatableUpdates. + * This method works on more granular level than HotswappableChangeCandidate.propertyUpdates. + */ +export async function hotswappableProperties( + evaluate: EvaluateCloudFormationTemplate, + change: HotswappableChangeCandidate, + PropertiesToInclude: string[], + transform?: (obj: any) => any, +): Promise { + transform = transform ?? ((obj: any) => obj); + const prev = transform(change.oldValue.Properties!); + const next = transform(change.newValue.Properties!); + const changedProps = detectChangedProps(next, prev).filter( + prop => PropertiesToInclude.includes(prop.key[0]), + ); + const evaluatedUpdates = await Promise.all( + changedProps + .filter((prop) => prop.type === 'added') + .map(async (prop) => { + const val = getPropertyFromKey(prop.key, next); + try { + const evaluated = await evaluate.evaluateCfnExpression(val); + return { + ...prop, + value: evaluated, + }; + } catch (e) { + if (e instanceof CfnEvaluationException) { + return prop; + } + throw e; + } + })); + const unevaluatableUpdates = evaluatedUpdates.filter(update => update.value === undefined); + evaluatedUpdates.push(...changedProps.filter(prop => prop.type == 'removed')); + + return { + updates: evaluatedUpdates, + unevaluatableUpdates, + }; +} + +function getPropertyFromKey(key: string[], obj: object) { + return key.reduce((prev, cur) => (prev as any)?.[cur], obj); +} + +function overwriteProperty(key: string[], newValue: any, target: object) { + for (const next of key.slice(0, -1)) { + target = (target as any)?.[next]; + } + if (target === undefined) return false; + if (newValue === undefined) { + delete (target as any)[key[key.length - 1]]; + } else { + (target as any)[key[key.length - 1]] = newValue; + } + return true; +} + +/** + * Take the old template and property updates, and synthesize a new template. + */ +export function applyPropertyUpdates(patches: ChangedProps[], target: any) { + target = JSON.parse(JSON.stringify(target)); + for (const patch of patches) { + const res = overwriteProperty(patch.key, patch.value, target); + if (!res) { + throw new Error(`failed to applying patch to ${patch.key.join('.')}. Please try deploying without hotswap first.`); + } + } + return target; +} diff --git a/packages/aws-cdk/lib/api/hotswap/common.ts b/packages/aws-cdk/lib/api/hotswap/common.ts index d72d7b270fc85..fba277b73ac4f 100644 --- a/packages/aws-cdk/lib/api/hotswap/common.ts +++ b/packages/aws-cdk/lib/api/hotswap/common.ts @@ -135,6 +135,13 @@ export function lowerCaseFirstCharacter(str: string): string { return str.length > 0 ? `${str[0].toLowerCase()}${str.slice(1)}` : str; } +/** + * This function upper cases the first character of the string provided. + */ +export function upperCaseFirstCharacter(str: string): string { + return str.length > 0 ? `${str[0].toUpperCase()}${str.slice(1)}` : str; +} + export type PropDiffs = Record>; export class ClassifiedChanges { diff --git a/packages/aws-cdk/lib/api/hotswap/ecs-services.ts b/packages/aws-cdk/lib/api/hotswap/ecs-services.ts index 0ffa23829c81c..a86441a1d2f31 100644 --- a/packages/aws-cdk/lib/api/hotswap/ecs-services.ts +++ b/packages/aws-cdk/lib/api/hotswap/ecs-services.ts @@ -1,7 +1,8 @@ import * as AWS from 'aws-sdk'; -import { ChangeHotswapResult, classifyChanges, HotswappableChangeCandidate, lowerCaseFirstCharacter, reportNonHotswappableChange, transformObjectKeys } from './common'; +import { ChangeHotswapResult, classifyChanges, HotswappableChangeCandidate, lowerCaseFirstCharacter, reportNonHotswappableChange, transformObjectKeys, upperCaseFirstCharacter } from './common'; import { ISDK } from '../aws-auth'; -import { CfnEvaluationException, EvaluateCloudFormationTemplate } from '../evaluate-cloudformation-template'; +import { EvaluateCloudFormationTemplate } from '../evaluate-cloudformation-template'; +import { applyPropertyUpdates, hotswappableProperties } from '../hotswap-deployments'; export async function isHotswappableEcsServiceChange( logicalId: string, change: HotswappableChangeCandidate, evaluateCfnTemplate: EvaluateCloudFormationTemplate, @@ -16,7 +17,8 @@ export async function isHotswappableEcsServiceChange( // We only allow a change in the ContainerDefinitions of the TaskDefinition for now - // it contains the image and environment variables, so seems like a safe bet for now. // We might revisit this decision in the future though! - const classifiedChanges = classifyChanges(change, ['ContainerDefinitions']); + const propertiesToHotswap = ['ContainerDefinitions']; + const classifiedChanges = classifyChanges(change, propertiesToHotswap); classifiedChanges.reportNonHotswappablePropertyChanges(ret); // find all ECS Services that reference the TaskDefinition that changed @@ -45,11 +47,40 @@ export async function isHotswappableEcsServiceChange( const namesOfHotswappableChanges = Object.keys(classifiedChanges.hotswappableProps); if (namesOfHotswappableChanges.length > 0) { - const taskDefinitionResource = await prepareTaskDefinitionChange(evaluateCfnTemplate, logicalId, change); - if (taskDefinitionResource === undefined) { - reportNonHotswappableChange(ret, change, undefined, 'Found unsupported changes to the task definition', false); + const familyName = await getFamilyName(evaluateCfnTemplate, logicalId, change); + if (familyName === undefined) { + reportNonHotswappableChange(ret, change, undefined, 'Failed to determine family name of the task definition', false); return ret; } + + // Don't transform the properties that take arbitrary string as keys i.e. { "string" : "string" } + // https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_RegisterTaskDefinition.html#API_RegisterTaskDefinition_RequestSyntax + const excludeFromTransform = { + ContainerDefinitions: { + DockerLabels: true, + FirelensConfiguration: { + Options: true, + }, + LogConfiguration: { + Options: true, + }, + }, + Volumes: { + DockerVolumeConfiguration: { + DriverOpts: true, + Labels: true, + }, + }, + } as const; + + const changes = await hotswappableProperties(evaluateCfnTemplate, change, propertiesToHotswap); + if (changes.unevaluatableUpdates.length > 0) { + reportNonHotswappableChange(ret, change, undefined, `Found changes that cannot be evaluated locally in the task definition - ${ + changes.unevaluatableUpdates.map(p => p.key.join('.')).join(', ') + }`, false); + return ret; + } + ret.push({ hotswappable: true, resourceType: change.newValue.Type, @@ -57,53 +88,55 @@ export async function isHotswappableEcsServiceChange( service: 'ecs-service', resourceNames: [ ...ecsServicesReferencingTaskDef.map(ecsService => `ECS Service '${ecsService.serviceArn.split('/')[2]}'`), - `ECS Task Definition '${taskDefinitionResource.family}'`, + `ECS Task Definition '${familyName}'`, ], apply: async (sdk: ISDK) => { // Step 1 - update the changed TaskDefinition, creating a new TaskDefinition Revision // we need to lowercase the evaluated TaskDef from CloudFormation, // as the AWS SDK uses lowercase property names for these + // get the latest task definition of the family + const target = await sdk + .ecs() + .describeTaskDefinition({ + taskDefinition: familyName, + include: ['TAGS'], + }) + .promise(); + if (target.taskDefinition === undefined) { + throw new Error(`Could not find a task definition: ${familyName}`); + } + + // The describeTaskDefinition response contains several keys that must not exist in a registerTaskDefinition request. + // We remove these keys here, comparing these two structs: + // https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_RegisterTaskDefinition.html#API_RegisterTaskDefinition_RequestSyntax + // https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_DescribeTaskDefinition.html#API_DescribeTaskDefinition_ResponseSyntax + [ + 'compatibilities', + 'taskDefinitionArn', + 'revision', + 'status', + 'requiresAttributes', + 'compatibilities', + 'registeredAt', + 'registeredBy', + ].forEach(key=> delete (target.taskDefinition as any)[key]); + + if (target.tags !== undefined && target.tags.length > 0) { + // the tags field is in a different location in describeTaskDefinition response, moving it as intended for registerTaskDefinition request. + (target.taskDefinition as any).tags = target.tags; + delete target.tags; + } + + const upperCasedTaskDef = transformObjectKeys(target.taskDefinition, upperCaseFirstCharacter, excludeFromTransform); + const updatedTaskDef = applyPropertyUpdates(changes.updates, upperCasedTaskDef); + const lowercasedTaskDef = transformObjectKeys(updatedTaskDef, lowerCaseFirstCharacter, excludeFromTransform); + // The SDK requires more properties here than its worth doing explicit typing for // instead, just use all the old values in the diff to fill them in implicitly - let lowercasedTaskDef = transformObjectKeys(taskDefinitionResource.taskDefinition, lowerCaseFirstCharacter, { - // Don't transform the properties that take arbitrary string as keys i.e. { "string" : "string" } - // https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_RegisterTaskDefinition.html#API_RegisterTaskDefinition_RequestSyntax - ContainerDefinitions: { - DockerLabels: true, - FirelensConfiguration: { - Options: true, - }, - LogConfiguration: { - Options: true, - }, - }, - Volumes: { - DockerVolumeConfiguration: { - DriverOpts: true, - Labels: true, - }, - }, - }); - if (taskDefinitionResource.hotswappableWithMerge) { - // get the latest task definition of the family - const describeTaskDefinitionResponse = await sdk - .ecs() - .describeTaskDefinition({ - taskDefinition: taskDefinitionResource.family, - include: ['TAGS'], - }) - .promise(); - if (describeTaskDefinitionResponse.taskDefinition === undefined) { - throw new Error(`Could not find TaskDefinition ${taskDefinitionResource.family}`); - } - const merged = mergeTaskDefinitions(lowercasedTaskDef, describeTaskDefinitionResponse); - if (merged === undefined) { - throw new Error('Failed to merge the task definition. Please try deploying without hotswap first.'); - } - lowercasedTaskDef = merged.taskDefinition; - } + // eslint-disable-next-line no-console + console.log(lowercasedTaskDef); const registerTaskDefResponse = await sdk.ecs().registerTaskDefinition(lowercasedTaskDef).promise(); const taskDefRevArn = registerTaskDefResponse.taskDefinition?.taskDefinitionArn; @@ -192,78 +225,19 @@ export async function isHotswappableEcsServiceChange( return ret; } -function mergeTaskDefinitions(patch: {containerDefinitions: {[key:string]: any}[]}, target: AWS.ECS.DescribeTaskDefinitionResponse) { - const src = patch.containerDefinitions; - // deep copy target to avoid side effects. The response of AWS API is in JSON format, so safe to use JSON.stringify. - target = JSON.parse(JSON.stringify(target)); - const dst = target.taskDefinition?.containerDefinitions; - if (dst === undefined) { - return; - } - // schema: https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_ContainerDefinition.html - for (const i in src) { - if (dst[i] === undefined) return; - for (const key of Object.keys(src[i])) { - (dst[i] as any)[key] = src[i][key]; - } - } - - // The describeTaskDefinition response contains several keys that must not exist in a registerTaskDefinition request. - // We remove these keys here, comparing these two structs: - // https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_RegisterTaskDefinition.html#API_RegisterTaskDefinition_RequestSyntax - // https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_DescribeTaskDefinition.html#API_DescribeTaskDefinition_ResponseSyntax - [ - 'compatibilities', - 'taskDefinitionArn', - 'revision', - 'status', - 'requiresAttributes', - 'compatibilities', - 'registeredAt', - 'registeredBy', - ].forEach(key=> delete (target.taskDefinition as any)[key]); - - if (target.tags !== undefined && target.tags.length > 0) { - // the tags field is in a different location in describeTaskDefinition response, moving it as intended for registerTaskDefinition request. - (target.taskDefinition as any).tags = target.tags; - delete target.tags; - } - - return target; -} - interface EcsService { readonly serviceArn: string; } -type TaskDefinitionChange = { - /** - * A new task definition that should be deployed. - * If hotswappableWithMerge equals true, this only contains diffs that should be merged with the currently deployed task definition. - */ - taskDefinition: any; - - /** - * A family for this task definition - */ - family: string; - - /** - * true if the change can be hotswapped by merging with the currently deployed task definition - */ - hotswappableWithMerge: boolean -}; - -async function prepareTaskDefinitionChange( +async function getFamilyName( evaluateCfnTemplate: EvaluateCloudFormationTemplate, logicalId: string, - change: HotswappableChangeCandidate, -): Promise { + change: HotswappableChangeCandidate) { const taskDefinitionResource: { [name: string]: any } = { ...change.oldValue.Properties, ContainerDefinitions: change.newValue.Properties?.ContainerDefinitions, }; - // first, let's get the name of the family + // first, let's get the name of the family const familyNameOrArn = await evaluateCfnTemplate.establishResourcePhysicalName(logicalId, taskDefinitionResource?.Family); if (!familyNameOrArn) { // if the Family property has not been provided, and we can't find it in the current Stack, @@ -274,111 +248,12 @@ async function prepareTaskDefinitionChange( // remove it if needed const familyNameOrArnParts = familyNameOrArn.split(':'); const family = familyNameOrArnParts.length > 1 - // familyNameOrArn is actually an ARN, of the format 'arn:aws:ecs:region:account:task-definition/:' - // so, take the 6th element, at index 5, and split it on '/' + // familyNameOrArn is actually an ARN, of the format 'arn:aws:ecs:region:account:task-definition/:' + // so, take the 6th element, at index 5, and split it on '/' ? familyNameOrArnParts[5].split('/')[1] - // otherwise, familyNameOrArn is just the simple name evaluated from the CloudFormation template + // otherwise, familyNameOrArn is just the simple name evaluated from the CloudFormation template : familyNameOrArn; - // then, let's evaluate the body of the remainder of the TaskDef (without the Family property) - - let evaluated; - let hotswappableWithMerge = false; - try { - evaluated = await evaluateCfnTemplate.evaluateCfnExpression({ - ...(taskDefinitionResource ?? {}), - Family: undefined, - }); - } catch (e) { - if (!(e instanceof CfnEvaluationException)) { - throw e; - } - const result = await deepCompareContainerDefinitions( - evaluateCfnTemplate, - change.oldValue.Properties?.ContainerDefinitions ?? [], - change.newValue.Properties?.ContainerDefinitions ?? []); - if (result === false) { - throw e; - } else { - evaluated = { - // return a partial definition that should be merged with the current definition - ContainerDefinitions: result, - }; - hotswappableWithMerge = true; - } - } + // then, let's evaluate the body of the remainder of the TaskDef (without the Family property) - return { - taskDefinition: { - ...evaluated, - Family: family, - }, - family, - hotswappableWithMerge, - }; -} - -/** - * return false if the new container definitions cannot be hotswapped - * i.e. there are changes containing unsupported tokens or additions/removals of properties - * - * Otherwise we return the properties that should be updated (they will be merged with the deployed task definition later) - */ -async function deepCompareContainerDefinitions(evaluateCfnTemplate: EvaluateCloudFormationTemplate, oldDefinition: any[], newDefinition: any[]) { - const result: any[] = []; - // schema: https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_ContainerDefinition.html - if (oldDefinition.length !== newDefinition.length) { - // one or more containers are added or removed - return false; - } - for (const i in oldDefinition) { - result.push({}); - const prev = oldDefinition[i]; - const next = newDefinition[i]; - - if (!deepCompareObject(Object.keys(prev), Object.keys(next))) { - // one or more fields are added or removed - return false; - } - // We don't recurse properties to keep the update logic simple. It should still cover most hotswap use cases. - for (const key of Object.keys(prev)) { - // compare two properties first - if (deepCompareObject(prev[key], next[key])) { - // if there is no difference, skip the field - continue; - } - // if there is any diff found, check if it can be evaluated without raising an error - try { - result[i][key] = await evaluateCfnTemplate.evaluateCfnExpression(next[key]); - } catch (e) { - if (!(e instanceof CfnEvaluationException)) { - throw e; - } - // Give up hotswap if the diff contains unsupported expression - return false; - } - } - } - - return result; -} - -/** - * return true when two objects are identical - */ -function deepCompareObject(lhs: any, rhs: any): boolean { - if (typeof lhs !== 'object') { - return lhs === rhs; - } - if (typeof rhs !== 'object') { - return false; - } - if (Object.keys(lhs).length != Object.keys(rhs).length) { - return false; - } - for (const key of Object.keys(lhs)) { - if (!deepCompareObject((lhs as any)[key], (rhs as any)[key])) { - return false; - } - } - return true; + return family; } From 9e7e7aaaed760fa295fac03af6c94289f9d4a517 Mon Sep 17 00:00:00 2001 From: tmokmss Date: Wed, 9 Aug 2023 15:25:56 +0900 Subject: [PATCH 11/19] fix --- .../aws-cdk/lib/api/hotswap-deployments.ts | 42 ++++++++++++------- .../aws-cdk/lib/api/hotswap/ecs-services.ts | 18 ++++---- 2 files changed, 38 insertions(+), 22 deletions(-) diff --git a/packages/aws-cdk/lib/api/hotswap-deployments.ts b/packages/aws-cdk/lib/api/hotswap-deployments.ts index adfc627d2f328..cc5c8d9bae012 100644 --- a/packages/aws-cdk/lib/api/hotswap-deployments.ts +++ b/packages/aws-cdk/lib/api/hotswap-deployments.ts @@ -387,9 +387,9 @@ export function detectChangedProps(next: any, prev: any): ChangedProps[] { } function detectAdditions(next: any, prev: any, keys: string[] = []): ChangedProps[] { - const changedProps: ChangedProps[] = []; // Compare each value of two objects, detect additions (added or modified properties) // If we encounter CFn intrinsic (key.startsWith('Fn::') || key == 'Ref'), stop recursion + if (typeof next !== 'object') { if (next !== prev) { // there is an addition or change to the property @@ -398,24 +398,27 @@ function detectAdditions(next: any, prev: any, keys: string[] = []): ChangedProp return []; } } + if (typeof prev !== 'object') { // there is an addition or change to the property return [{ key: new Array(...keys), type: 'added' }]; } - // If the lhs is an object but also a CFn intrinsic, don't recurse further. - const lastKey = keys.length > 0 ? keys[keys.length - 1] : ''; - if (lastKey.startsWith('Fn::') || lastKey == 'Ref') { + // If the next is a CFn intrinsic, don't recurse further. + // We take the first key with [0] because a CfnIntrinsic object only has one key, e.g. { 'Ref': 'SomeResource' } + const childKeys = Object.keys(next); + if (childKeys.length === 1 && (childKeys[0].startsWith('Fn::') || childKeys[0] === 'Ref')) { if (!deepCompareObject(prev, next)) { // there is an addition or change to the property - return [{ key: new Array(...keys.slice(0, -1)), type: 'added' }]; + return [{ key: new Array(...keys), type: 'added' }]; } else { return []; } } + const changedProps: ChangedProps[] = []; // compare children - for (const key of Object.keys(next)) { + for (const key of childKeys) { keys.push(key); changedProps.push(...detectAdditions((next as any)[key], (prev as any)[key], keys)); keys.pop(); @@ -427,25 +430,26 @@ function detectRemovals(next: any, prev: any, keys: string[] = []): ChangedProps // Compare each value of two objects, detect removed properties // To do this, find any keys that exist only in prev object. // If we encounter CFn intrinsic (key.startsWith('Fn::') || key == 'Ref'), stop recursion - const changedProps: ChangedProps[] = []; if (next === undefined) { return [{ key: new Array(...keys), type: 'removed' }]; } if (typeof prev !== 'object' || typeof next !== 'object') { - // either prev or next is not an object, then the property is modified but not removed + // either prev or next is not an object nor undefined, then the property is not removed return []; } - // If the prev is an object but also a CFn intrinsic, don't recurse further. - const lastKey = keys.length > 0 ? keys[keys.length - 1] : ''; - if (lastKey.startsWith('Fn::') || lastKey == 'Ref') { - // next is not undefined here, so the property is at least not removed + // If the next is a CFn intrinsic, don't recurse further. + // We take the first key with [0] because a CfnIntrinsic object only has one key, e.g. { 'Ref': 'SomeResource' } + const childKeys = Object.keys(prev); + if (childKeys.length === 1 && (childKeys[0].startsWith('Fn::') || childKeys[0] === 'Ref')) { + // next is not undefined here, so it is at least not removed return []; } + const changedProps: ChangedProps[] = []; // compare children - for (const key of Object.keys(prev)) { + for (const key of childKeys) { keys.push(key); changedProps.push(...detectRemovals((next as any)[key], (prev as any)[key], keys)); keys.pop(); @@ -529,9 +533,17 @@ function getPropertyFromKey(key: string[], obj: object) { function overwriteProperty(key: string[], newValue: any, target: object) { for (const next of key.slice(0, -1)) { - target = (target as any)?.[next]; + if (next in target) { + target = (target as any)[next]; + } else if (Array.isArray(target)) { + // When an element is added to an array, we need to explicitly allocate the new element. + target = {}; + (target as any)[next] = {}; + } else { + // This is an unexpected condition. Perhaps the deployed task definition is modified outside of CFn. + return false; + } } - if (target === undefined) return false; if (newValue === undefined) { delete (target as any)[key[key.length - 1]]; } else { diff --git a/packages/aws-cdk/lib/api/hotswap/ecs-services.ts b/packages/aws-cdk/lib/api/hotswap/ecs-services.ts index a86441a1d2f31..735a84d053ff4 100644 --- a/packages/aws-cdk/lib/api/hotswap/ecs-services.ts +++ b/packages/aws-cdk/lib/api/hotswap/ecs-services.ts @@ -122,22 +122,20 @@ export async function isHotswappableEcsServiceChange( 'registeredBy', ].forEach(key=> delete (target.taskDefinition as any)[key]); + // the tags field is in a different location in describeTaskDefinition response, + // moving it as intended for registerTaskDefinition request. if (target.tags !== undefined && target.tags.length > 0) { - // the tags field is in a different location in describeTaskDefinition response, moving it as intended for registerTaskDefinition request. (target.taskDefinition as any).tags = target.tags; delete target.tags; } + // We first uppercase the task definition to properly merge it with the one from CloudFormation template. const upperCasedTaskDef = transformObjectKeys(target.taskDefinition, upperCaseFirstCharacter, excludeFromTransform); + // merge evaluatable diff from CloudFormation template. const updatedTaskDef = applyPropertyUpdates(changes.updates, upperCasedTaskDef); + // lowercase the merged task definition to use it in AWS SDK. const lowercasedTaskDef = transformObjectKeys(updatedTaskDef, lowerCaseFirstCharacter, excludeFromTransform); - // The SDK requires more properties here than its worth doing explicit typing for - // instead, just use all the old values in the diff to fill them in implicitly - - // eslint-disable-next-line no-console - console.log(lowercasedTaskDef); - const registerTaskDefResponse = await sdk.ecs().registerTaskDefinition(lowercasedTaskDef).promise(); const taskDefRevArn = registerTaskDefResponse.taskDefinition?.taskDefinitionArn; @@ -175,6 +173,12 @@ export async function isHotswappableEcsServiceChange( }), ); + if (taskDefRevArn !== undefined) { + // Make the task definition registered by hotswap INACTIVE immediately. + // This is to prevent describeTaskDefinition API from returning a drifted task definition. + await sdk.ecs().deregisterTaskDefinition({ taskDefinition: taskDefRevArn }).promise(); + } + // Step 3 - wait for the service deployments triggered in Step 2 to finish // configure a custom Waiter (sdk.ecs() as any).api.waiters.deploymentToFinish = { From c69cbaa1397089c236432a3e23da9d9ff10176e2 Mon Sep 17 00:00:00 2001 From: tmokmss Date: Wed, 9 Aug 2023 15:33:58 +0900 Subject: [PATCH 12/19] add comments --- packages/aws-cdk/lib/api/hotswap-deployments.ts | 12 +++++------- packages/aws-cdk/lib/api/hotswap/ecs-services.ts | 4 ++-- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/packages/aws-cdk/lib/api/hotswap-deployments.ts b/packages/aws-cdk/lib/api/hotswap-deployments.ts index cc5c8d9bae012..bce267d464675 100644 --- a/packages/aws-cdk/lib/api/hotswap-deployments.ts +++ b/packages/aws-cdk/lib/api/hotswap-deployments.ts @@ -405,7 +405,6 @@ function detectAdditions(next: any, prev: any, keys: string[] = []): ChangedProp } // If the next is a CFn intrinsic, don't recurse further. - // We take the first key with [0] because a CfnIntrinsic object only has one key, e.g. { 'Ref': 'SomeResource' } const childKeys = Object.keys(next); if (childKeys.length === 1 && (childKeys[0].startsWith('Fn::') || childKeys[0] === 'Ref')) { if (!deepCompareObject(prev, next)) { @@ -439,8 +438,7 @@ function detectRemovals(next: any, prev: any, keys: string[] = []): ChangedProps return []; } - // If the next is a CFn intrinsic, don't recurse further. - // We take the first key with [0] because a CfnIntrinsic object only has one key, e.g. { 'Ref': 'SomeResource' } + // If the prev is a CFn intrinsic, don't recurse further. const childKeys = Object.keys(prev); if (childKeys.length === 1 && (childKeys[0].startsWith('Fn::') || childKeys[0] === 'Ref')) { // next is not undefined here, so it is at least not removed @@ -478,7 +476,7 @@ function deepCompareObject(lhs: any, rhs: any): boolean { return true; } -interface HotswappablePropertyUpdates { +interface EvaluatedPropertyUpdates { readonly updates: ChangedProps[]; readonly unevaluatableUpdates: ChangedProps[]; } @@ -488,12 +486,12 @@ interface HotswappablePropertyUpdates { * If any diff cannot be evaluated, they are reported by unevaluatableUpdates. * This method works on more granular level than HotswappableChangeCandidate.propertyUpdates. */ -export async function hotswappableProperties( +export async function evaluatableProperties( evaluate: EvaluateCloudFormationTemplate, change: HotswappableChangeCandidate, PropertiesToInclude: string[], transform?: (obj: any) => any, -): Promise { +): Promise { transform = transform ?? ((obj: any) => obj); const prev = transform(change.oldValue.Properties!); const next = transform(change.newValue.Properties!); @@ -536,7 +534,7 @@ function overwriteProperty(key: string[], newValue: any, target: object) { if (next in target) { target = (target as any)[next]; } else if (Array.isArray(target)) { - // When an element is added to an array, we need to explicitly allocate the new element. + // When an element is added to an array, we need explicitly allocate the new element. target = {}; (target as any)[next] = {}; } else { diff --git a/packages/aws-cdk/lib/api/hotswap/ecs-services.ts b/packages/aws-cdk/lib/api/hotswap/ecs-services.ts index 735a84d053ff4..04d0947cc278c 100644 --- a/packages/aws-cdk/lib/api/hotswap/ecs-services.ts +++ b/packages/aws-cdk/lib/api/hotswap/ecs-services.ts @@ -2,7 +2,7 @@ import * as AWS from 'aws-sdk'; import { ChangeHotswapResult, classifyChanges, HotswappableChangeCandidate, lowerCaseFirstCharacter, reportNonHotswappableChange, transformObjectKeys, upperCaseFirstCharacter } from './common'; import { ISDK } from '../aws-auth'; import { EvaluateCloudFormationTemplate } from '../evaluate-cloudformation-template'; -import { applyPropertyUpdates, hotswappableProperties } from '../hotswap-deployments'; +import { applyPropertyUpdates, evaluatableProperties } from '../hotswap-deployments'; export async function isHotswappableEcsServiceChange( logicalId: string, change: HotswappableChangeCandidate, evaluateCfnTemplate: EvaluateCloudFormationTemplate, @@ -73,7 +73,7 @@ export async function isHotswappableEcsServiceChange( }, } as const; - const changes = await hotswappableProperties(evaluateCfnTemplate, change, propertiesToHotswap); + const changes = await evaluatableProperties(evaluateCfnTemplate, change, propertiesToHotswap); if (changes.unevaluatableUpdates.length > 0) { reportNonHotswappableChange(ret, change, undefined, `Found changes that cannot be evaluated locally in the task definition - ${ changes.unevaluatableUpdates.map(p => p.key.join('.')).join(', ') From 1c6009226514ec66280ccbfcb277f91637a49632 Mon Sep 17 00:00:00 2001 From: tmokmss Date: Wed, 9 Aug 2023 16:28:57 +0900 Subject: [PATCH 13/19] add comments --- .../aws-cdk/lib/api/hotswap-deployments.ts | 21 +++++++++++++++++-- .../aws-cdk/lib/api/hotswap/ecs-services.ts | 2 +- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/packages/aws-cdk/lib/api/hotswap-deployments.ts b/packages/aws-cdk/lib/api/hotswap-deployments.ts index bce267d464675..5490ad4015967 100644 --- a/packages/aws-cdk/lib/api/hotswap-deployments.ts +++ b/packages/aws-cdk/lib/api/hotswap-deployments.ts @@ -377,9 +377,26 @@ function logNonHotswappableChanges(nonHotswappableChanges: NonHotswappableChange print(''); // newline } -type ChangedProps = { key: string[]; type: 'removed' | 'added'; value?: any }; +type ChangedProps = { + /** + * Array to specify the property from an object. + * e.g. Given this object `{ 'a': { 'b': 1 } }`, the key array for the element `1` will be `['a', 'b']` + */ + key: string[]; + + /** + * Whether the property is added (also modified) or removed. + */ + type: 'removed' | 'added'; + + /** + * evaluated value of the property. + * undefined if type == 'removed' + */ + value?: any +}; -export function detectChangedProps(next: any, prev: any): ChangedProps[] { +function detectChangedProps(next: any, prev: any): ChangedProps[] { const changedProps: ChangedProps[] = []; changedProps.push(...detectAdditions(next, prev)); changedProps.push(...detectRemovals(next, prev)); diff --git a/packages/aws-cdk/lib/api/hotswap/ecs-services.ts b/packages/aws-cdk/lib/api/hotswap/ecs-services.ts index 04d0947cc278c..fb49b2b973230 100644 --- a/packages/aws-cdk/lib/api/hotswap/ecs-services.ts +++ b/packages/aws-cdk/lib/api/hotswap/ecs-services.ts @@ -87,8 +87,8 @@ export async function isHotswappableEcsServiceChange( propsChanged: namesOfHotswappableChanges, service: 'ecs-service', resourceNames: [ - ...ecsServicesReferencingTaskDef.map(ecsService => `ECS Service '${ecsService.serviceArn.split('/')[2]}'`), `ECS Task Definition '${familyName}'`, + ...ecsServicesReferencingTaskDef.map(ecsService => `ECS Service '${ecsService.serviceArn.split('/')[2]}'`), ], apply: async (sdk: ISDK) => { // Step 1 - update the changed TaskDefinition, creating a new TaskDefinition Revision From 4407b05e82964f7a3a6ea4e11b37b698867239d9 Mon Sep 17 00:00:00 2001 From: tmokmss Date: Wed, 9 Aug 2023 16:53:45 +0900 Subject: [PATCH 14/19] refactor --- .../aws-cdk/lib/api/hotswap-deployments.ts | 12 +++--- .../aws-cdk/lib/api/hotswap/ecs-services.ts | 39 +++++++++---------- 2 files changed, 25 insertions(+), 26 deletions(-) diff --git a/packages/aws-cdk/lib/api/hotswap-deployments.ts b/packages/aws-cdk/lib/api/hotswap-deployments.ts index 5490ad4015967..8229334fc37ed 100644 --- a/packages/aws-cdk/lib/api/hotswap-deployments.ts +++ b/packages/aws-cdk/lib/api/hotswap-deployments.ts @@ -502,18 +502,18 @@ interface EvaluatedPropertyUpdates { * Diff each property of the changes, and check if each diff can be actually hotswapped (i.e. evaluated by EvaluateCloudFormationTemplate.) * If any diff cannot be evaluated, they are reported by unevaluatableUpdates. * This method works on more granular level than HotswappableChangeCandidate.propertyUpdates. + * + * If propertiesToInclude is specified, we only compare properties that are under keys in the argument. */ export async function evaluatableProperties( evaluate: EvaluateCloudFormationTemplate, change: HotswappableChangeCandidate, - PropertiesToInclude: string[], - transform?: (obj: any) => any, + propertiesToInclude?: string[], ): Promise { - transform = transform ?? ((obj: any) => obj); - const prev = transform(change.oldValue.Properties!); - const next = transform(change.newValue.Properties!); + const prev = change.oldValue.Properties!; + const next = change.newValue.Properties!; const changedProps = detectChangedProps(next, prev).filter( - prop => PropertiesToInclude.includes(prop.key[0]), + prop => propertiesToInclude?.includes(prop.key[0]) ?? true, ); const evaluatedUpdates = await Promise.all( changedProps diff --git a/packages/aws-cdk/lib/api/hotswap/ecs-services.ts b/packages/aws-cdk/lib/api/hotswap/ecs-services.ts index fb49b2b973230..ceb41a30603bb 100644 --- a/packages/aws-cdk/lib/api/hotswap/ecs-services.ts +++ b/packages/aws-cdk/lib/api/hotswap/ecs-services.ts @@ -53,26 +53,6 @@ export async function isHotswappableEcsServiceChange( return ret; } - // Don't transform the properties that take arbitrary string as keys i.e. { "string" : "string" } - // https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_RegisterTaskDefinition.html#API_RegisterTaskDefinition_RequestSyntax - const excludeFromTransform = { - ContainerDefinitions: { - DockerLabels: true, - FirelensConfiguration: { - Options: true, - }, - LogConfiguration: { - Options: true, - }, - }, - Volumes: { - DockerVolumeConfiguration: { - DriverOpts: true, - Labels: true, - }, - }, - } as const; - const changes = await evaluatableProperties(evaluateCfnTemplate, change, propertiesToHotswap); if (changes.unevaluatableUpdates.length > 0) { reportNonHotswappableChange(ret, change, undefined, `Found changes that cannot be evaluated locally in the task definition - ${ @@ -129,6 +109,25 @@ export async function isHotswappableEcsServiceChange( delete target.tags; } + // Don't transform the properties that take arbitrary string as keys i.e. { "string" : "string" } + // https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_RegisterTaskDefinition.html#API_RegisterTaskDefinition_RequestSyntax + const excludeFromTransform = { + ContainerDefinitions: { + DockerLabels: true, + FirelensConfiguration: { + Options: true, + }, + LogConfiguration: { + Options: true, + }, + }, + Volumes: { + DockerVolumeConfiguration: { + DriverOpts: true, + Labels: true, + }, + }, + } as const; // We first uppercase the task definition to properly merge it with the one from CloudFormation template. const upperCasedTaskDef = transformObjectKeys(target.taskDefinition, upperCaseFirstCharacter, excludeFromTransform); // merge evaluatable diff from CloudFormation template. From 4d7ce4af3f3131928b364349ea8816c3d596f7ae Mon Sep 17 00:00:00 2001 From: tmokmss Date: Fri, 11 Aug 2023 06:16:28 +0900 Subject: [PATCH 15/19] use physicalResourceId to determine ARN of the task definition --- .../aws-cdk/lib/api/hotswap/ecs-services.ts | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/packages/aws-cdk/lib/api/hotswap/ecs-services.ts b/packages/aws-cdk/lib/api/hotswap/ecs-services.ts index ceb41a30603bb..bf9a4085bae81 100644 --- a/packages/aws-cdk/lib/api/hotswap/ecs-services.ts +++ b/packages/aws-cdk/lib/api/hotswap/ecs-services.ts @@ -52,6 +52,11 @@ export async function isHotswappableEcsServiceChange( reportNonHotswappableChange(ret, change, undefined, 'Failed to determine family name of the task definition', false); return ret; } + const oldTaskDefinitionArn = await evaluateCfnTemplate.findPhysicalNameFor(logicalId); + if (oldTaskDefinitionArn === undefined) { + reportNonHotswappableChange(ret, change, undefined, 'Failed to determine ARN of the task definition', false); + return ret; + } const changes = await evaluatableProperties(evaluateCfnTemplate, change, propertiesToHotswap); if (changes.unevaluatableUpdates.length > 0) { @@ -75,16 +80,16 @@ export async function isHotswappableEcsServiceChange( // we need to lowercase the evaluated TaskDef from CloudFormation, // as the AWS SDK uses lowercase property names for these - // get the latest task definition of the family + // get the task definition of the family and revision corresponding to the old CFn template const target = await sdk .ecs() .describeTaskDefinition({ - taskDefinition: familyName, + taskDefinition: oldTaskDefinitionArn, include: ['TAGS'], }) .promise(); if (target.taskDefinition === undefined) { - throw new Error(`Could not find a task definition: ${familyName}`); + throw new Error(`Could not find a task definition: ${oldTaskDefinitionArn}. Try deploying without hotswap first.`); } // The describeTaskDefinition response contains several keys that must not exist in a registerTaskDefinition request. @@ -172,12 +177,6 @@ export async function isHotswappableEcsServiceChange( }), ); - if (taskDefRevArn !== undefined) { - // Make the task definition registered by hotswap INACTIVE immediately. - // This is to prevent describeTaskDefinition API from returning a drifted task definition. - await sdk.ecs().deregisterTaskDefinition({ taskDefinition: taskDefRevArn }).promise(); - } - // Step 3 - wait for the service deployments triggered in Step 2 to finish // configure a custom Waiter (sdk.ecs() as any).api.waiters.deploymentToFinish = { From 31427f36a60a0682d6fa67b2d161ba8718bd69ae Mon Sep 17 00:00:00 2001 From: tmokmss Date: Fri, 11 Aug 2023 07:01:59 +0900 Subject: [PATCH 16/19] fix unit tests --- .../ecs-services-hotswap-deployments.test.ts | 360 ++++++++---------- 1 file changed, 154 insertions(+), 206 deletions(-) diff --git a/packages/aws-cdk/test/api/hotswap/ecs-services-hotswap-deployments.test.ts b/packages/aws-cdk/test/api/hotswap/ecs-services-hotswap-deployments.test.ts index 304de3968683a..369d32a007505 100644 --- a/packages/aws-cdk/test/api/hotswap/ecs-services-hotswap-deployments.test.ts +++ b/packages/aws-cdk/test/api/hotswap/ecs-services-hotswap-deployments.test.ts @@ -1,7 +1,7 @@ /* eslint-disable import/order */ import * as AWS from 'aws-sdk'; import * as setup from './hotswap-test-setup'; -import { HotswapMode } from '../../../lib/api/hotswap/common'; +import { HotswapMode, lowerCaseFirstCharacter, transformObjectKeys } from '../../../lib/api/hotswap/common'; let hotswapMockSdkProvider: setup.HotswapMockSdkProvider; let mockRegisterTaskDef: jest.Mock; @@ -33,36 +33,71 @@ beforeEach(() => { }); }); -describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hotswapMode) => { - test('should call registerTaskDefinition and updateService for a difference only in the TaskDefinition with a Family property', async () => { - // GIVEN - setup.setCurrentCfnStackTemplate({ - Resources: { - TaskDef: { - Type: 'AWS::ECS::TaskDefinition', - Properties: { - Family: 'my-task-def', - ContainerDefinitions: [ - { Image: 'image1' }, - ], - }, - }, +function setupCurrentTaskDefinition(props: {taskDefinitionProperties: any, includeService: boolean, otherResources?: any}) { + setup.setCurrentCfnStackTemplate({ + Resources: { + TaskDef: { + Type: 'AWS::ECS::TaskDefinition', + Properties: props.taskDefinitionProperties, + }, + ...(props.includeService ? { Service: { Type: 'AWS::ECS::Service', Properties: { TaskDefinition: { Ref: 'TaskDef' }, }, }, - }, - }); + } : {}), + ...(props.otherResources ?? {}), + }, + }); + if (props.includeService) { setup.pushStackResourceSummaries( setup.stackSummaryOf('Service', 'AWS::ECS::Service', 'arn:aws:ecs:region:account:service/my-cluster/my-service'), ); - mockRegisterTaskDef.mockReturnValue({ - taskDefinition: { - taskDefinitionArn: 'arn:aws:ecs:region:account:task-definition/my-task-def:3', + } + setup.pushStackResourceSummaries( + setup.stackSummaryOf('TaskDef', 'AWS::ECS::TaskDefinition', + 'arn:aws:ecs:region:account:task-definition/my-task-def:2'), + ); + mockRegisterTaskDef.mockReturnValue({ + taskDefinition: { + taskDefinitionArn: 'arn:aws:ecs:region:account:task-definition/my-task-def:3', + }, + }); + mockDescribeTaskDef.mockReturnValue({ + taskDefinition: transformObjectKeys(props.taskDefinitionProperties, lowerCaseFirstCharacter, { + ContainerDefinitions: { + DockerLabels: true, + FirelensConfiguration: { + Options: true, + }, + LogConfiguration: { + Options: true, + }, + }, + Volumes: { + DockerVolumeConfiguration: { + DriverOpts: true, + Labels: true, + }, }, + }), + }); +} + +describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hotswapMode) => { + test('should call registerTaskDefinition and updateService for a difference only in the TaskDefinition with a Family property', async () => { + // GIVEN + setupCurrentTaskDefinition({ + taskDefinitionProperties: { + Family: 'my-task-def', + ContainerDefinitions: [ + { Image: 'image1' }, + ], + }, + includeService: true, }); const cdkStackArtifact = setup.cdkStackArtifactOf({ template: { @@ -97,6 +132,10 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot { image: 'image2' }, ], }); + expect(mockDescribeTaskDef).toBeCalledWith({ + taskDefinition: 'arn:aws:ecs:region:account:task-definition/my-task-def:2', + include: ['TAGS'], + }); expect(mockUpdateService).toBeCalledWith({ service: 'arn:aws:ecs:region:account:service/my-cluster/my-service', cluster: 'my-cluster', @@ -110,34 +149,15 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot test('any other TaskDefinition property change besides ContainerDefinition cannot be hotswapped in CLASSIC mode but does not block HOTSWAP_ONLY mode deployments', async () => { // GIVEN - setup.setCurrentCfnStackTemplate({ - Resources: { - TaskDef: { - Type: 'AWS::ECS::TaskDefinition', - Properties: { - Family: 'my-task-def', - ContainerDefinitions: [ - { Image: 'image1' }, - ], - Cpu: '256', - }, - }, - Service: { - Type: 'AWS::ECS::Service', - Properties: { - TaskDefinition: { Ref: 'TaskDef' }, - }, - }, - }, - }); - setup.pushStackResourceSummaries( - setup.stackSummaryOf('Service', 'AWS::ECS::Service', - 'arn:aws:ecs:region:account:service/my-cluster/my-service'), - ); - mockRegisterTaskDef.mockReturnValue({ - taskDefinition: { - taskDefinitionArn: 'arn:aws:ecs:region:account:task-definition/my-task-def:3', + setupCurrentTaskDefinition({ + taskDefinitionProperties: { + Family: 'my-task-def', + ContainerDefinitions: [ + { Image: 'image1' }, + ], + Cpu: '256', }, + includeService: true, }); const cdkStackArtifact = setup.cdkStackArtifactOf({ template: { @@ -169,6 +189,7 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot // THEN expect(deployStackResult).toBeUndefined(); expect(mockRegisterTaskDef).not.toHaveBeenCalled(); + expect(mockDescribeTaskDef).not.toHaveBeenCalled(); expect(mockUpdateService).not.toHaveBeenCalled(); } else if (hotswapMode === HotswapMode.HOTSWAP_ONLY) { // WHEN @@ -183,6 +204,10 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot ], cpu: '256', // this uses the old value because a new value could cause a service replacement }); + expect(mockDescribeTaskDef).toBeCalledWith({ + taskDefinition: 'arn:aws:ecs:region:account:task-definition/my-task-def:2', + include: ['TAGS'], + }); expect(mockUpdateService).toBeCalledWith({ service: 'arn:aws:ecs:region:account:service/my-cluster/my-service', cluster: 'my-cluster', @@ -197,34 +222,15 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot test('deleting any other TaskDefinition property besides ContainerDefinition results in a full deployment in CLASSIC mode and a hotswap deployment in HOTSWAP_ONLY mode', async () => { // GIVEN - setup.setCurrentCfnStackTemplate({ - Resources: { - TaskDef: { - Type: 'AWS::ECS::TaskDefinition', - Properties: { - Family: 'my-task-def', - ContainerDefinitions: [ - { Image: 'image1' }, - ], - Cpu: '256', - }, - }, - Service: { - Type: 'AWS::ECS::Service', - Properties: { - TaskDefinition: { Ref: 'TaskDef' }, - }, - }, - }, - }); - setup.pushStackResourceSummaries( - setup.stackSummaryOf('Service', 'AWS::ECS::Service', - 'arn:aws:ecs:region:account:service/my-cluster/my-service'), - ); - mockRegisterTaskDef.mockReturnValue({ - taskDefinition: { - taskDefinitionArn: 'arn:aws:ecs:region:account:task-definition/my-task-def:3', + setupCurrentTaskDefinition({ + taskDefinitionProperties: { + Family: 'my-task-def', + ContainerDefinitions: [ + { Image: 'image1' }, + ], + Cpu: '256', }, + includeService: true, }); const cdkStackArtifact = setup.cdkStackArtifactOf({ template: { @@ -255,6 +261,7 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot // THEN expect(deployStackResult).toBeUndefined(); expect(mockRegisterTaskDef).not.toHaveBeenCalled(); + expect(mockDescribeTaskDef).not.toHaveBeenCalled(); expect(mockUpdateService).not.toHaveBeenCalled(); } else if (hotswapMode === HotswapMode.HOTSWAP_ONLY) { // WHEN @@ -262,6 +269,10 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot // THEN expect(deployStackResult).not.toBeUndefined(); + expect(mockDescribeTaskDef).toBeCalledWith({ + taskDefinition: 'arn:aws:ecs:region:account:task-definition/my-task-def:2', + include: ['TAGS'], + }); expect(mockRegisterTaskDef).toBeCalledWith({ family: 'my-task-def', containerDefinitions: [ @@ -283,33 +294,23 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot test('should call registerTaskDefinition and updateService for a difference only in the TaskDefinition without a Family property', async () => { // GIVEN - setup.setCurrentCfnStackTemplate({ - Resources: { - TaskDef: { - Type: 'AWS::ECS::TaskDefinition', - Properties: { - ContainerDefinitions: [ - { Image: 'image1' }, - ], - }, - }, - Service: { - Type: 'AWS::ECS::Service', - Properties: { - TaskDefinition: { Ref: 'TaskDef' }, - }, - }, + setupCurrentTaskDefinition({ + taskDefinitionProperties: { + ContainerDefinitions: [ + { Image: 'image1' }, + ], }, + includeService: true, }); - setup.pushStackResourceSummaries( - setup.stackSummaryOf('TaskDef', 'AWS::ECS::TaskDefinition', - 'arn:aws:ecs:region:account:task-definition/my-task-def:2'), - setup.stackSummaryOf('Service', 'AWS::ECS::Service', - 'arn:aws:ecs:region:account:service/my-cluster/my-service'), - ); - mockRegisterTaskDef.mockReturnValue({ + mockDescribeTaskDef.mockReturnValue({ taskDefinition: { - taskDefinitionArn: 'arn:aws:ecs:region:account:task-definition/my-task-def:3', + taskDefinitionArn: 'arn:aws:ecs:region:account:task-definition/my-task-def:2', + family: 'my-task-def', + containerDefinitions: [ + { + image: 'image1', + }, + ], }, }); const cdkStackArtifact = setup.cdkStackArtifactOf({ @@ -338,6 +339,10 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot // THEN expect(deployStackResult).not.toBeUndefined(); + expect(mockDescribeTaskDef).toBeCalledWith({ + taskDefinition: 'arn:aws:ecs:region:account:task-definition/my-task-def:2', + include: ['TAGS'], + }); expect(mockRegisterTaskDef).toBeCalledWith({ family: 'my-task-def', containerDefinitions: [ @@ -357,26 +362,14 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot test('a difference just in a TaskDefinition, without any services using it, is not hotswappable in FALL_BACK mode', async () => { // GIVEN - setup.setCurrentCfnStackTemplate({ - Resources: { - TaskDef: { - Type: 'AWS::ECS::TaskDefinition', - Properties: { - ContainerDefinitions: [ - { Image: 'image1' }, - ], - }, - }, - }, - }); - setup.pushStackResourceSummaries( - setup.stackSummaryOf('TaskDef', 'AWS::ECS::TaskDefinition', - 'arn:aws:ecs:region:account:task-definition/my-task-def:2'), - ); - mockRegisterTaskDef.mockReturnValue({ - taskDefinition: { - taskDefinitionArn: 'arn:aws:ecs:region:account:task-definition/my-task-def:3', + setupCurrentTaskDefinition({ + taskDefinitionProperties: { + Family: 'my-task-def', + ContainerDefinitions: [ + { Image: 'image1' }, + ], }, + includeService: false, }); const cdkStackArtifact = setup.cdkStackArtifactOf({ template: { @@ -399,6 +392,7 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot // THEN expect(deployStackResult).toBeUndefined(); + expect(mockDescribeTaskDef).not.toHaveBeenCalled(); expect(mockRegisterTaskDef).not.toHaveBeenCalled(); expect(mockUpdateService).not.toHaveBeenCalled(); } else if (hotswapMode === HotswapMode.HOTSWAP_ONLY) { @@ -407,6 +401,10 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot // THEN expect(deployStackResult).not.toBeUndefined(); + expect(mockDescribeTaskDef).toBeCalledWith({ + taskDefinition: 'arn:aws:ecs:region:account:task-definition/my-task-def:2', + include: ['TAGS'], + }); expect(mockRegisterTaskDef).toBeCalledWith({ family: 'my-task-def', containerDefinitions: [ @@ -420,23 +418,15 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot test('if anything besides an ECS Service references the changed TaskDefinition, hotswapping is not possible in CLASSIC mode but is possible in HOTSWAP_ONLY', async () => { // GIVEN - setup.setCurrentCfnStackTemplate({ - Resources: { - TaskDef: { - Type: 'AWS::ECS::TaskDefinition', - Properties: { - Family: 'my-task-def', - ContainerDefinitions: [ - { Image: 'image1' }, - ], - }, - }, - Service: { - Type: 'AWS::ECS::Service', - Properties: { - TaskDefinition: { Ref: 'TaskDef' }, - }, - }, + setupCurrentTaskDefinition({ + taskDefinitionProperties: { + Family: 'my-task-def', + ContainerDefinitions: [ + { Image: 'image1' }, + ], + }, + includeService: true, + otherResources: { Function: { Type: 'AWS::Lambda::Function', Properties: { @@ -449,15 +439,6 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot }, }, }); - setup.pushStackResourceSummaries( - setup.stackSummaryOf('Service', 'AWS::ECS::Service', - 'arn:aws:ecs:region:account:service/my-cluster/my-service'), - ); - mockRegisterTaskDef.mockReturnValue({ - taskDefinition: { - taskDefinitionArn: 'arn:aws:ecs:region:account:task-definition/my-task-def:3', - }, - }); const cdkStackArtifact = setup.cdkStackArtifactOf({ template: { Resources: { @@ -496,6 +477,7 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot // THEN expect(deployStackResult).toBeUndefined(); + expect(mockDescribeTaskDef).not.toHaveBeenCalled(); expect(mockRegisterTaskDef).not.toHaveBeenCalled(); expect(mockUpdateService).not.toHaveBeenCalled(); } else if (hotswapMode === HotswapMode.HOTSWAP_ONLY) { @@ -504,6 +486,10 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot // THEN expect(deployStackResult).not.toBeUndefined(); + expect(mockDescribeTaskDef).toBeCalledWith({ + taskDefinition: 'arn:aws:ecs:region:account:task-definition/my-task-def:2', + include: ['TAGS'], + }); expect(mockRegisterTaskDef).toBeCalledWith({ family: 'my-task-def', containerDefinitions: [ @@ -524,41 +510,22 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot test('should call registerTaskDefinition, describeTaskDefinition, and updateService for a difference only in the container image but with environment variables of unsupported intrinsics', async () => { // GIVEN - setup.setCurrentCfnStackTemplate({ - Resources: { - TaskDef: { - Type: 'AWS::ECS::TaskDefinition', - Properties: { - Family: 'my-task-def', - ContainerDefinitions: [ + setupCurrentTaskDefinition({ + taskDefinitionProperties: { + Family: 'my-task-def', + ContainerDefinitions: [ + { + Image: 'image1', + Environment: [ { - Image: 'image1', - Environment: [ - { - Name: 'FOO', - Value: { 'Fn::GetAtt': ['Bar', 'Baz'] }, - }, - ], + Name: 'FOO', + Value: { 'Fn::GetAtt': ['Bar', 'Baz'] }, }, ], }, - }, - Service: { - Type: 'AWS::ECS::Service', - Properties: { - TaskDefinition: { Ref: 'TaskDef' }, - }, - }, - }, - }); - setup.pushStackResourceSummaries( - setup.stackSummaryOf('Service', 'AWS::ECS::Service', - 'arn:aws:ecs:region:account:service/my-cluster/my-service'), - ); - mockRegisterTaskDef.mockReturnValue({ - taskDefinition: { - taskDefinitionArn: 'arn:aws:ecs:region:account:task-definition/my-task-def:3', + ], }, + includeService: true, }); mockDescribeTaskDef.mockReturnValue({ taskDefinition: { @@ -640,41 +607,22 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot test('should call registerTaskDefinition with certain properties not lowercased', async () => { // GIVEN - setup.setCurrentCfnStackTemplate({ - Resources: { - TaskDef: { - Type: 'AWS::ECS::TaskDefinition', - Properties: { - Family: 'my-task-def', - ContainerDefinitions: [ - { Image: 'image1' }, - ], - Volumes: [ - { - DockerVolumeConfiguration: { - DriverOpts: { Option1: 'option1' }, - Labels: { Label1: 'label1' }, - }, - }, - ], - }, - }, - Service: { - Type: 'AWS::ECS::Service', - Properties: { - TaskDefinition: { Ref: 'TaskDef' }, + setupCurrentTaskDefinition({ + taskDefinitionProperties: { + Family: 'my-task-def', + ContainerDefinitions: [ + { Image: 'image1' }, + ], + Volumes: [ + { + DockerVolumeConfiguration: { + DriverOpts: { Option1: 'option1' }, + Labels: { Label1: 'label1' }, + }, }, - }, - }, - }); - setup.pushStackResourceSummaries( - setup.stackSummaryOf('Service', 'AWS::ECS::Service', - 'arn:aws:ecs:region:account:service/my-cluster/my-service'), - ); - mockRegisterTaskDef.mockReturnValue({ - taskDefinition: { - taskDefinitionArn: 'arn:aws:ecs:region:account:task-definition/my-task-def:3', + ], }, + includeService: true, }); const cdkStackArtifact = setup.cdkStackArtifactOf({ template: { From 76af98f8b717fd03d78d4d768961eb818b0d70f5 Mon Sep 17 00:00:00 2001 From: tmokmss Date: Fri, 11 Aug 2023 07:06:40 +0900 Subject: [PATCH 17/19] add unit tests --- .../ecs-services-hotswap-deployments.test.ts | 207 ++++++++++++++++++ 1 file changed, 207 insertions(+) diff --git a/packages/aws-cdk/test/api/hotswap/ecs-services-hotswap-deployments.test.ts b/packages/aws-cdk/test/api/hotswap/ecs-services-hotswap-deployments.test.ts index 369d32a007505..b9be0725315c8 100644 --- a/packages/aws-cdk/test/api/hotswap/ecs-services-hotswap-deployments.test.ts +++ b/packages/aws-cdk/test/api/hotswap/ecs-services-hotswap-deployments.test.ts @@ -586,7 +586,214 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot image: 'image2', environment: [ { + name: 'FOO', + value: 'value', + }, + ], + }, + ], + }); + expect(mockUpdateService).toBeCalledWith({ + service: 'arn:aws:ecs:region:account:service/my-cluster/my-service', + cluster: 'my-cluster', + taskDefinition: 'arn:aws:ecs:region:account:task-definition/my-task-def:3', + deploymentConfiguration: { + minimumHealthyPercent: 0, + }, + forceNewDeployment: true, + }); + }); + test('should call registerTaskDefinition, describeTaskDefinition, and updateService for a simple environment variable addition', async () => { + // GIVEN + setupCurrentTaskDefinition({ + taskDefinitionProperties: { + Family: 'my-task-def', + ContainerDefinitions: [ + { + Image: 'image1', + Environment: [ + { + Name: 'FOO', + Value: { 'Fn::GetAtt': ['Bar', 'Baz'] }, + }, + ], + }, + ], + }, + includeService: true, + }); + mockDescribeTaskDef.mockReturnValue({ + taskDefinition: { + taskDefinitionArn: 'arn:aws:ecs:region:account:task-definition/my-task-def:2', + family: 'my-task-def', + containerDefinitions: [ + { + image: 'image1', + environment: [ + { + name: 'FOO', + value: 'value', + }, + ], + }, + ], + }, + }); + const cdkStackArtifact = setup.cdkStackArtifactOf({ + template: { + Resources: { + TaskDef: { + Type: 'AWS::ECS::TaskDefinition', + Properties: { + Family: 'my-task-def', + ContainerDefinitions: [ + { + Image: 'image2', + Environment: [ + { + Name: 'FOO', + Value: { 'Fn::GetAtt': ['Bar', 'Baz'] }, + }, + { + Name: 'BAR', + Value: '1', + }, + ], + }, + ], + }, + }, + Service: { + Type: 'AWS::ECS::Service', + Properties: { + TaskDefinition: { Ref: 'TaskDef' }, + }, + }, + }, + }, + }); + + // WHEN + const deployStackResult = await hotswapMockSdkProvider.tryHotswapDeployment(hotswapMode, cdkStackArtifact); + + // THEN + expect(deployStackResult).not.toBeUndefined(); + expect(mockRegisterTaskDef).toBeCalledWith({ + family: 'my-task-def', + containerDefinitions: [ + { + image: 'image2', + environment: [ + { + name: 'FOO', + value: 'value', + }, + { + name: 'BAR', + value: '1', + }, + ], + }, + ], + }); + expect(mockUpdateService).toBeCalledWith({ + service: 'arn:aws:ecs:region:account:service/my-cluster/my-service', + cluster: 'my-cluster', + taskDefinition: 'arn:aws:ecs:region:account:task-definition/my-task-def:3', + deploymentConfiguration: { + minimumHealthyPercent: 0, + }, + forceNewDeployment: true, + }); + }); + + test('should call registerTaskDefinition, describeTaskDefinition, and updateService for a environment variable deletion', async () => { + // GIVEN + setupCurrentTaskDefinition({ + taskDefinitionProperties: { + Family: 'my-task-def', + ContainerDefinitions: [ + { + Image: 'image1', + Environment: [ + { + Name: 'FOO', + Value: { 'Fn::GetAtt': ['Bar', 'Baz'] }, + }, + { + Name: 'BAR', + Value: '1', + }, + ], + }, + ], + }, + includeService: true, + }); + mockDescribeTaskDef.mockReturnValue({ + taskDefinition: { + taskDefinitionArn: 'arn:aws:ecs:region:account:task-definition/my-task-def:2', + family: 'my-task-def', + containerDefinitions: [ + { + image: 'image1', + environment: [ + { + name: 'FOO', + value: 'value', + }, + { + name: 'BAR', + value: '1', + }, + ], + }, + ], + }, + }); + const cdkStackArtifact = setup.cdkStackArtifactOf({ + template: { + Resources: { + TaskDef: { + Type: 'AWS::ECS::TaskDefinition', + Properties: { + Family: 'my-task-def', + ContainerDefinitions: [ + { + Image: 'image2', + Environment: [ + { + Name: 'FOO', + Value: { 'Fn::GetAtt': ['Bar', 'Baz'] }, + }, + ], + }, + ], + }, + }, + Service: { + Type: 'AWS::ECS::Service', + Properties: { + TaskDefinition: { Ref: 'TaskDef' }, + }, + }, + }, + }, + }); + + // WHEN + const deployStackResult = await hotswapMockSdkProvider.tryHotswapDeployment(hotswapMode, cdkStackArtifact); + + // THEN + expect(deployStackResult).not.toBeUndefined(); + expect(mockRegisterTaskDef).toBeCalledWith({ + family: 'my-task-def', + containerDefinitions: [ + { + image: 'image2', + environment: [ + { name: 'FOO', value: 'value', }, From f08d1d2a9efd539a3a2dbf8352aece1ba8ca5ce0 Mon Sep 17 00:00:00 2001 From: tmokmss Date: Fri, 11 Aug 2023 07:16:15 +0900 Subject: [PATCH 18/19] fix circular import --- .../aws-cdk/lib/api/hotswap-deployments.ts | 204 ----------------- packages/aws-cdk/lib/api/hotswap/common.ts | 205 ++++++++++++++++++ .../aws-cdk/lib/api/hotswap/ecs-services.ts | 3 +- 3 files changed, 206 insertions(+), 206 deletions(-) diff --git a/packages/aws-cdk/lib/api/hotswap-deployments.ts b/packages/aws-cdk/lib/api/hotswap-deployments.ts index 8229334fc37ed..47df91950555c 100644 --- a/packages/aws-cdk/lib/api/hotswap-deployments.ts +++ b/packages/aws-cdk/lib/api/hotswap-deployments.ts @@ -376,207 +376,3 @@ function logNonHotswappableChanges(nonHotswappableChanges: NonHotswappableChange print(''); // newline } - -type ChangedProps = { - /** - * Array to specify the property from an object. - * e.g. Given this object `{ 'a': { 'b': 1 } }`, the key array for the element `1` will be `['a', 'b']` - */ - key: string[]; - - /** - * Whether the property is added (also modified) or removed. - */ - type: 'removed' | 'added'; - - /** - * evaluated value of the property. - * undefined if type == 'removed' - */ - value?: any -}; - -function detectChangedProps(next: any, prev: any): ChangedProps[] { - const changedProps: ChangedProps[] = []; - changedProps.push(...detectAdditions(next, prev)); - changedProps.push(...detectRemovals(next, prev)); - return changedProps; -} - -function detectAdditions(next: any, prev: any, keys: string[] = []): ChangedProps[] { - // Compare each value of two objects, detect additions (added or modified properties) - // If we encounter CFn intrinsic (key.startsWith('Fn::') || key == 'Ref'), stop recursion - - if (typeof next !== 'object') { - if (next !== prev) { - // there is an addition or change to the property - return [{ key: new Array(...keys), type: 'added' }]; - } else { - return []; - } - } - - if (typeof prev !== 'object') { - // there is an addition or change to the property - return [{ key: new Array(...keys), type: 'added' }]; - } - - // If the next is a CFn intrinsic, don't recurse further. - const childKeys = Object.keys(next); - if (childKeys.length === 1 && (childKeys[0].startsWith('Fn::') || childKeys[0] === 'Ref')) { - if (!deepCompareObject(prev, next)) { - // there is an addition or change to the property - return [{ key: new Array(...keys), type: 'added' }]; - } else { - return []; - } - } - - const changedProps: ChangedProps[] = []; - // compare children - for (const key of childKeys) { - keys.push(key); - changedProps.push(...detectAdditions((next as any)[key], (prev as any)[key], keys)); - keys.pop(); - } - return changedProps; -} - -function detectRemovals(next: any, prev: any, keys: string[] = []): ChangedProps[] { - // Compare each value of two objects, detect removed properties - // To do this, find any keys that exist only in prev object. - // If we encounter CFn intrinsic (key.startsWith('Fn::') || key == 'Ref'), stop recursion - if (next === undefined) { - return [{ key: new Array(...keys), type: 'removed' }]; - } - - if (typeof prev !== 'object' || typeof next !== 'object') { - // either prev or next is not an object nor undefined, then the property is not removed - return []; - } - - // If the prev is a CFn intrinsic, don't recurse further. - const childKeys = Object.keys(prev); - if (childKeys.length === 1 && (childKeys[0].startsWith('Fn::') || childKeys[0] === 'Ref')) { - // next is not undefined here, so it is at least not removed - return []; - } - - const changedProps: ChangedProps[] = []; - // compare children - for (const key of childKeys) { - keys.push(key); - changedProps.push(...detectRemovals((next as any)[key], (prev as any)[key], keys)); - keys.pop(); - } - return changedProps; -} - -/** - * return true when two objects are identical - */ -function deepCompareObject(lhs: any, rhs: any): boolean { - if (typeof lhs !== 'object') { - return lhs === rhs; - } - if (typeof rhs !== 'object') { - return false; - } - if (Object.keys(lhs).length != Object.keys(rhs).length) { - return false; - } - for (const key of Object.keys(lhs)) { - if (!deepCompareObject((lhs as any)[key], (rhs as any)[key])) { - return false; - } - } - return true; -} - -interface EvaluatedPropertyUpdates { - readonly updates: ChangedProps[]; - readonly unevaluatableUpdates: ChangedProps[]; -} - -/** - * Diff each property of the changes, and check if each diff can be actually hotswapped (i.e. evaluated by EvaluateCloudFormationTemplate.) - * If any diff cannot be evaluated, they are reported by unevaluatableUpdates. - * This method works on more granular level than HotswappableChangeCandidate.propertyUpdates. - * - * If propertiesToInclude is specified, we only compare properties that are under keys in the argument. - */ -export async function evaluatableProperties( - evaluate: EvaluateCloudFormationTemplate, - change: HotswappableChangeCandidate, - propertiesToInclude?: string[], -): Promise { - const prev = change.oldValue.Properties!; - const next = change.newValue.Properties!; - const changedProps = detectChangedProps(next, prev).filter( - prop => propertiesToInclude?.includes(prop.key[0]) ?? true, - ); - const evaluatedUpdates = await Promise.all( - changedProps - .filter((prop) => prop.type === 'added') - .map(async (prop) => { - const val = getPropertyFromKey(prop.key, next); - try { - const evaluated = await evaluate.evaluateCfnExpression(val); - return { - ...prop, - value: evaluated, - }; - } catch (e) { - if (e instanceof CfnEvaluationException) { - return prop; - } - throw e; - } - })); - const unevaluatableUpdates = evaluatedUpdates.filter(update => update.value === undefined); - evaluatedUpdates.push(...changedProps.filter(prop => prop.type == 'removed')); - - return { - updates: evaluatedUpdates, - unevaluatableUpdates, - }; -} - -function getPropertyFromKey(key: string[], obj: object) { - return key.reduce((prev, cur) => (prev as any)?.[cur], obj); -} - -function overwriteProperty(key: string[], newValue: any, target: object) { - for (const next of key.slice(0, -1)) { - if (next in target) { - target = (target as any)[next]; - } else if (Array.isArray(target)) { - // When an element is added to an array, we need explicitly allocate the new element. - target = {}; - (target as any)[next] = {}; - } else { - // This is an unexpected condition. Perhaps the deployed task definition is modified outside of CFn. - return false; - } - } - if (newValue === undefined) { - delete (target as any)[key[key.length - 1]]; - } else { - (target as any)[key[key.length - 1]] = newValue; - } - return true; -} - -/** - * Take the old template and property updates, and synthesize a new template. - */ -export function applyPropertyUpdates(patches: ChangedProps[], target: any) { - target = JSON.parse(JSON.stringify(target)); - for (const patch of patches) { - const res = overwriteProperty(patch.key, patch.value, target); - if (!res) { - throw new Error(`failed to applying patch to ${patch.key.join('.')}. Please try deploying without hotswap first.`); - } - } - return target; -} diff --git a/packages/aws-cdk/lib/api/hotswap/common.ts b/packages/aws-cdk/lib/api/hotswap/common.ts index fba277b73ac4f..5bbb69535faf1 100644 --- a/packages/aws-cdk/lib/api/hotswap/common.ts +++ b/packages/aws-cdk/lib/api/hotswap/common.ts @@ -1,5 +1,6 @@ import * as cfn_diff from '@aws-cdk/cloudformation-diff'; import { ISDK } from '../aws-auth'; +import { CfnEvaluationException, EvaluateCloudFormationTemplate } from '../evaluate-cloudformation-template'; export const ICON = '✨'; @@ -220,3 +221,207 @@ export function reportNonHotswappableResource( reason, }]; } + +type ChangedProps = { + /** + * Array to specify the property from an object. + * e.g. Given this object `{ 'a': { 'b': 1 } }`, the key array for the element `1` will be `['a', 'b']` + */ + key: string[]; + + /** + * Whether the property is added (also modified) or removed. + */ + type: 'removed' | 'added'; + + /** + * evaluated value of the property. + * undefined if type == 'removed' + */ + value?: any +}; + +function detectChangedProps(next: any, prev: any): ChangedProps[] { + const changedProps: ChangedProps[] = []; + changedProps.push(...detectAdditions(next, prev)); + changedProps.push(...detectRemovals(next, prev)); + return changedProps; +} + +function detectAdditions(next: any, prev: any, keys: string[] = []): ChangedProps[] { + // Compare each value of two objects, detect additions (added or modified properties) + // If we encounter CFn intrinsic (key.startsWith('Fn::') || key == 'Ref'), stop recursion + + if (typeof next !== 'object') { + if (next !== prev) { + // there is an addition or change to the property + return [{ key: new Array(...keys), type: 'added' }]; + } else { + return []; + } + } + + if (typeof prev !== 'object') { + // there is an addition or change to the property + return [{ key: new Array(...keys), type: 'added' }]; + } + + // If the next is a CFn intrinsic, don't recurse further. + const childKeys = Object.keys(next); + if (childKeys.length === 1 && (childKeys[0].startsWith('Fn::') || childKeys[0] === 'Ref')) { + if (!deepCompareObject(prev, next)) { + // there is an addition or change to the property + return [{ key: new Array(...keys), type: 'added' }]; + } else { + return []; + } + } + + const changedProps: ChangedProps[] = []; + // compare children + for (const key of childKeys) { + keys.push(key); + changedProps.push(...detectAdditions((next as any)[key], (prev as any)[key], keys)); + keys.pop(); + } + return changedProps; +} + +function detectRemovals(next: any, prev: any, keys: string[] = []): ChangedProps[] { + // Compare each value of two objects, detect removed properties + // To do this, find any keys that exist only in prev object. + // If we encounter CFn intrinsic (key.startsWith('Fn::') || key == 'Ref'), stop recursion + if (next === undefined) { + return [{ key: new Array(...keys), type: 'removed' }]; + } + + if (typeof prev !== 'object' || typeof next !== 'object') { + // either prev or next is not an object nor undefined, then the property is not removed + return []; + } + + // If the prev is a CFn intrinsic, don't recurse further. + const childKeys = Object.keys(prev); + if (childKeys.length === 1 && (childKeys[0].startsWith('Fn::') || childKeys[0] === 'Ref')) { + // next is not undefined here, so it is at least not removed + return []; + } + + const changedProps: ChangedProps[] = []; + // compare children + for (const key of childKeys) { + keys.push(key); + changedProps.push(...detectRemovals((next as any)[key], (prev as any)[key], keys)); + keys.pop(); + } + return changedProps; +} + +/** + * return true when two objects are identical + */ +function deepCompareObject(lhs: any, rhs: any): boolean { + if (typeof lhs !== 'object') { + return lhs === rhs; + } + if (typeof rhs !== 'object') { + return false; + } + if (Object.keys(lhs).length != Object.keys(rhs).length) { + return false; + } + for (const key of Object.keys(lhs)) { + if (!deepCompareObject((lhs as any)[key], (rhs as any)[key])) { + return false; + } + } + return true; +} + +interface EvaluatedPropertyUpdates { + readonly updates: ChangedProps[]; + readonly unevaluatableUpdates: ChangedProps[]; +} + +/** + * Diff each property of the changes, and check if each diff can be actually hotswapped (i.e. evaluated by EvaluateCloudFormationTemplate.) + * If any diff cannot be evaluated, they are reported by unevaluatableUpdates. + * This method works on more granular level than HotswappableChangeCandidate.propertyUpdates. + * + * If propertiesToInclude is specified, we only compare properties that are under keys in the argument. + */ +export async function evaluatableProperties( + evaluate: EvaluateCloudFormationTemplate, + change: HotswappableChangeCandidate, + propertiesToInclude?: string[], +): Promise { + const prev = change.oldValue.Properties!; + const next = change.newValue.Properties!; + const changedProps = detectChangedProps(next, prev).filter( + prop => propertiesToInclude?.includes(prop.key[0]) ?? true, + ); + const evaluatedUpdates = await Promise.all( + changedProps + .filter((prop) => prop.type === 'added') + .map(async (prop) => { + const val = getPropertyFromKey(prop.key, next); + try { + const evaluated = await evaluate.evaluateCfnExpression(val); + return { + ...prop, + value: evaluated, + }; + } catch (e) { + if (e instanceof CfnEvaluationException) { + return prop; + } + throw e; + } + })); + const unevaluatableUpdates = evaluatedUpdates.filter(update => update.value === undefined); + evaluatedUpdates.push(...changedProps.filter(prop => prop.type == 'removed')); + + return { + updates: evaluatedUpdates, + unevaluatableUpdates, + }; +} + +function getPropertyFromKey(key: string[], obj: object) { + return key.reduce((prev, cur) => (prev as any)?.[cur], obj); +} + +function overwriteProperty(key: string[], newValue: any, target: object) { + for (const next of key.slice(0, -1)) { + if (next in target) { + target = (target as any)[next]; + } else if (Array.isArray(target)) { + // When an element is added to an array, we need explicitly allocate the new element. + target = {}; + (target as any)[next] = {}; + } else { + // This is an unexpected condition. Perhaps the deployed task definition is modified outside of CFn. + return false; + } + } + if (newValue === undefined) { + delete (target as any)[key[key.length - 1]]; + } else { + (target as any)[key[key.length - 1]] = newValue; + } + return true; +} + +/** + * Take the old template and property updates, and synthesize a new template. + */ +export function applyPropertyUpdates(patches: ChangedProps[], target: any) { + target = JSON.parse(JSON.stringify(target)); + for (const patch of patches) { + const res = overwriteProperty(patch.key, patch.value, target); + if (!res) { + throw new Error(`failed to applying patch to ${patch.key.join('.')}. Please try deploying without hotswap first.`); + } + } + return target; +} diff --git a/packages/aws-cdk/lib/api/hotswap/ecs-services.ts b/packages/aws-cdk/lib/api/hotswap/ecs-services.ts index bf9a4085bae81..7f2dea19ea493 100644 --- a/packages/aws-cdk/lib/api/hotswap/ecs-services.ts +++ b/packages/aws-cdk/lib/api/hotswap/ecs-services.ts @@ -1,8 +1,7 @@ import * as AWS from 'aws-sdk'; -import { ChangeHotswapResult, classifyChanges, HotswappableChangeCandidate, lowerCaseFirstCharacter, reportNonHotswappableChange, transformObjectKeys, upperCaseFirstCharacter } from './common'; +import { ChangeHotswapResult, classifyChanges, HotswappableChangeCandidate, lowerCaseFirstCharacter, reportNonHotswappableChange, transformObjectKeys, upperCaseFirstCharacter, applyPropertyUpdates, evaluatableProperties } from './common'; import { ISDK } from '../aws-auth'; import { EvaluateCloudFormationTemplate } from '../evaluate-cloudformation-template'; -import { applyPropertyUpdates, evaluatableProperties } from '../hotswap-deployments'; export async function isHotswappableEcsServiceChange( logicalId: string, change: HotswappableChangeCandidate, evaluateCfnTemplate: EvaluateCloudFormationTemplate, From 052d4794ede1b5d73d9cecaf7163700f637ae3b4 Mon Sep 17 00:00:00 2001 From: tmokmss Date: Fri, 11 Aug 2023 07:17:22 +0900 Subject: [PATCH 19/19] Update hotswap-deployments.ts --- packages/aws-cdk/lib/api/hotswap-deployments.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/aws-cdk/lib/api/hotswap-deployments.ts b/packages/aws-cdk/lib/api/hotswap-deployments.ts index 47df91950555c..eb03bdba5a8fb 100644 --- a/packages/aws-cdk/lib/api/hotswap-deployments.ts +++ b/packages/aws-cdk/lib/api/hotswap-deployments.ts @@ -3,7 +3,7 @@ import * as cxapi from '@aws-cdk/cx-api'; import * as chalk from 'chalk'; import { ISDK, Mode, SdkProvider } from './aws-auth'; import { DeployStackResult } from './deploy-stack'; -import { CfnEvaluationException, EvaluateCloudFormationTemplate, LazyListStackResources } from './evaluate-cloudformation-template'; +import { EvaluateCloudFormationTemplate, LazyListStackResources } from './evaluate-cloudformation-template'; import { isHotswappableAppSyncChange } from './hotswap/appsync-mapping-templates'; import { isHotswappableCodeBuildProjectChange } from './hotswap/code-build-projects'; import { ICON, ChangeHotswapResult, HotswapMode, HotswappableChange, NonHotswappableChange, HotswappableChangeCandidate, ClassifiedResourceChanges, reportNonHotswappableChange } from './hotswap/common';