fix(cli): ecs hotswap fails on log configuration enabled (aws#26876)

tmokmss · web-flow · commit 6cffca06d972 · 2023-08-25T07:10:36.000Z
## Problem ECS hotswap fails on a task definition containing log configuration like the following. ```ts const taskDefinition = new ecs.FargateTaskDefinition(stack, 'Task', {}); taskDefinition.addContainer('EcsApp', { image: ecs.ContainerImage.fromRegistry('nginx:stable'), logging: ecs.LogDriver.awsLogs({ streamPrefix: 'log' }), }); ``` ## Root cause When we transform object keys in a task definition, we pass `excludeFromTransform` to avoid from transforming keys with arbitrary string, such as [`logDriver.options`](https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_LogConfiguration.html#ECS-Type-LogConfiguration-options). However, it was not working when we transform keys with upperCaseFirstCharacter, because the keys in `excludeFromTransform` was uppercased, where the source object was lowercased. ``` // source task definition { "logConfiguration": { "logDriver": "awslogs", "options": { "awslogs-group": "my-test-stack-TaskEcsAppLogGroupD5C9C1DD-BPB6zgX4S0wU", "awslogs-region": "ap-northeast-1", }, }, } // excludeFromTransform { LogConfiguration: { Options: true, }, }, } // where it should be { logConfiguration: { options: true, }, }, } ``` This misconfiguration resulted in the output task definition uppercased in an unexpected way: ```json { "logConfiguration": { "logDriver": "awslogs", "options": { "Awslogs-group": "my-test-stack-TaskEcsAppLogGroupD5C9C1DD-BPB6zgX4S0wU", "Awslogs-region": "ap-northeast-1", }, }, } ``` The problem was not detected by unit tests because they only contained cases with uppercase keys in a source task definition. ## Fix Use lowercased `excludeFromTransform` when we use it with `upperCaseFirstCharacter`, also adding a test case with lowercased keys in a source task definition. Closes aws#26871. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
diff --git a/packages/aws-cdk/lib/api/hotswap/ecs-services.ts b/packages/aws-cdk/lib/api/hotswap/ecs-services.ts
@@ -132,8 +132,9 @@ export async function isHotswappableEcsServiceChange(
             },
           },
         } as const;
+        const excludeFromTransformLowercased = transformObjectKeys(excludeFromTransform, lowerCaseFirstCharacter);
         // We first uppercase the task definition to properly merge it with the one from CloudFormation template.
-        const upperCasedTaskDef = transformObjectKeys(target.taskDefinition, upperCaseFirstCharacter, excludeFromTransform);
+        const upperCasedTaskDef = transformObjectKeys(target.taskDefinition, upperCaseFirstCharacter, excludeFromTransformLowercased);
         // merge evaluatable diff from CloudFormation template.
         const updatedTaskDef = applyPropertyUpdates(changes.updates, upperCasedTaskDef);
         // lowercase the merged task definition to use it in AWS SDK.
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
@@ -812,13 +812,22 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot
     });
   });
 
-  test('should call registerTaskDefinition with certain properties not lowercased', async () => {
+  test('should call registerTaskDefinition with certain properties not lowercased nor uppercased', async () => {
     // GIVEN
     setupCurrentTaskDefinition({
       taskDefinitionProperties: {
         Family: 'my-task-def',
         ContainerDefinitions: [
-          { Image: 'image1' },
+          {
+            Image: 'image1',
+            DockerLabels: { Label1: 'label1' },
+            FirelensConfiguration: {
+              Options: { Name: 'cloudwatch' },
+            },
+            LogConfiguration: {
+              Options: { Option1: 'option1', option2: 'option2' },
+            },
+          },
         ],
         Volumes: [
           {
@@ -846,7 +855,7 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot
                     Options: { Name: 'cloudwatch' },
                   },
                   LogConfiguration: {
-                    Options: { Option1: 'option1' },
+                    Options: { Option1: 'option1', option2: 'option2' },
                   },
                 },
               ],
@@ -887,7 +896,7 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot
             },
           },
           logConfiguration: {
-            options: { Option1: 'option1' },
+            options: { Option1: 'option1', option2: 'option2' },
           },
         },
       ],
