Skip to content

Commit 6cffca0

Browse files
authored
fix(cli): ecs hotswap fails on log configuration enabled (#26876)
## 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 #26871. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 702d9d5 commit 6cffca0

File tree

2 files changed

+15
-5
lines changed

2 files changed

+15
-5
lines changed

packages/aws-cdk/lib/api/hotswap/ecs-services.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,8 +132,9 @@ export async function isHotswappableEcsServiceChange(
132132
},
133133
},
134134
} as const;
135+
const excludeFromTransformLowercased = transformObjectKeys(excludeFromTransform, lowerCaseFirstCharacter);
135136
// We first uppercase the task definition to properly merge it with the one from CloudFormation template.
136-
const upperCasedTaskDef = transformObjectKeys(target.taskDefinition, upperCaseFirstCharacter, excludeFromTransform);
137+
const upperCasedTaskDef = transformObjectKeys(target.taskDefinition, upperCaseFirstCharacter, excludeFromTransformLowercased);
137138
// merge evaluatable diff from CloudFormation template.
138139
const updatedTaskDef = applyPropertyUpdates(changes.updates, upperCasedTaskDef);
139140
// lowercase the merged task definition to use it in AWS SDK.

packages/aws-cdk/test/api/hotswap/ecs-services-hotswap-deployments.test.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -812,13 +812,22 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot
812812
});
813813
});
814814

815-
test('should call registerTaskDefinition with certain properties not lowercased', async () => {
815+
test('should call registerTaskDefinition with certain properties not lowercased nor uppercased', async () => {
816816
// GIVEN
817817
setupCurrentTaskDefinition({
818818
taskDefinitionProperties: {
819819
Family: 'my-task-def',
820820
ContainerDefinitions: [
821-
{ Image: 'image1' },
821+
{
822+
Image: 'image1',
823+
DockerLabels: { Label1: 'label1' },
824+
FirelensConfiguration: {
825+
Options: { Name: 'cloudwatch' },
826+
},
827+
LogConfiguration: {
828+
Options: { Option1: 'option1', option2: 'option2' },
829+
},
830+
},
822831
],
823832
Volumes: [
824833
{
@@ -846,7 +855,7 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot
846855
Options: { Name: 'cloudwatch' },
847856
},
848857
LogConfiguration: {
849-
Options: { Option1: 'option1' },
858+
Options: { Option1: 'option1', option2: 'option2' },
850859
},
851860
},
852861
],
@@ -887,7 +896,7 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot
887896
},
888897
},
889898
logConfiguration: {
890-
options: { Option1: 'option1' },
899+
options: { Option1: 'option1', option2: 'option2' },
891900
},
892901
},
893902
],

0 commit comments

Comments
 (0)