Skip to content

Commit 1e7c690

Browse files
authored
fix(ec2): prevent deduplication of init command args (#30821)
### Issue # (if applicable) Closes #26221 ### Reason for this change Previously, using `ec2.InitCommand.argvCommand()` would remove some duplicate strings in the input array. This produces an incorrect command in the template, leading to unexpected behaviour. ### Description of changes An additional line was added to the `deepMerge` function that is called in the `InitConfig.bindForType()` method, which checks the key of the input array, preventing it from becoming a set (removing duplicates) if it is a list of commands. ### Description of how you validated changes A unit test was added to generate an `AWS::CloudFormation::Init` resource identical to the one reproduced in the issue. The test was run and failed before the changes were made, and following the changes in `cfn-init.ts`, the test passed. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 79b5cd2 commit 1e7c690

File tree

8 files changed

+115
-20
lines changed

8 files changed

+115
-20
lines changed

packages/@aws-cdk-testing/framework-integ/test/aws-ec2/test/integ.instance-init-multiple.js.snapshot/integ-ec2-multiple-instances-in-stack.assets.json

+2-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/framework-integ/test/aws-ec2/test/integ.instance-init-multiple.js.snapshot/integ-ec2-multiple-instances-in-stack.template.json

+24-1
Original file line numberDiff line numberDiff line change
@@ -906,7 +906,7 @@
906906
"Fn::Join": [
907907
"",
908908
[
909-
"#!/bin/bash\n# fingerprint: 8787022e9944cbeb\n(\n set +e\n /opt/aws/bin/cfn-init -v --region ",
909+
"#!/bin/bash\n# fingerprint: 370d9b2dcf8bf44b\n(\n set +e\n /opt/aws/bin/cfn-init -v --region ",
910910
{
911911
"Ref": "AWS::Region"
912912
},
@@ -955,6 +955,29 @@
955955
"owner": "root",
956956
"group": "root"
957957
}
958+
},
959+
"commands": {
960+
"000": {
961+
"command": [
962+
"useradd",
963+
"-u",
964+
"1001",
965+
"-g",
966+
"1001",
967+
"eguser"
968+
]
969+
},
970+
"001": {
971+
"command": [
972+
"useradd",
973+
"-a",
974+
"-u",
975+
"1001",
976+
"-g",
977+
"1001",
978+
"eguser"
979+
]
980+
}
958981
}
959982
}
960983
},

packages/@aws-cdk-testing/framework-integ/test/aws-ec2/test/integ.instance-init-multiple.js.snapshot/manifest.json

+5-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/framework-integ/test/aws-ec2/test/integ.instance-init-multiple.js.snapshot/tree.json

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/framework-integ/test/aws-ec2/test/integ.instance-init-multiple.ts

+6
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,12 @@ class TestStack extends cdk.Stack {
5555
'/target/path/config.json',
5656
path.join(tmpDir, 'testConfigFile2'),
5757
),
58+
ec2.InitCommand.argvCommand([
59+
'useradd', '-u', '1001', '-g', '1001', 'eguser',
60+
]),
61+
ec2.InitCommand.argvCommand([
62+
'useradd', '-a', '-u', '1001', '-g', '1001', 'eguser',
63+
]),
5864
]),
5965
},
6066
}),

packages/@aws-cdk-testing/framework-integ/test/aws-ec2/test/integ.instance-init.js.snapshot/manifest.json

+4-10
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/aws-cdk-lib/aws-ec2/lib/cfn-init.ts

+11-4
Original file line numberDiff line numberDiff line change
@@ -311,10 +311,17 @@ function deepMerge(target?: Record<string, any>, src?: Record<string, any>) {
311311
if (target[key] && !Array.isArray(target[key])) {
312312
throw new Error(`Trying to merge array [${value}] into a non-array '${target[key]}'`);
313313
}
314-
target[key] = Array.from(new Set([
315-
...target[key] ?? [],
316-
...value,
317-
]));
314+
if (key === 'command') { // don't deduplicate command arguments
315+
target[key] = new Array(
316+
...target[key] ?? [],
317+
...value,
318+
);
319+
} else {
320+
target[key] = Array.from(new Set([
321+
...target[key] ?? [],
322+
...value,
323+
]));
324+
}
318325
continue;
319326
}
320327
if (typeof value === 'object' && value) {

packages/aws-cdk-lib/aws-ec2/test/cfn-init.test.ts

+62
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,68 @@ test('empty configs are not rendered', () => {
136136
});
137137
});
138138

139+
test('duplicate config arguments not deduplicated', () => {
140+
//GIVEN
141+
const config = new ec2.InitConfig([
142+
ec2.InitCommand.argvCommand([
143+
'useradd', '-u', '1001', '-g', '1001', 'eguser',
144+
]),
145+
ec2.InitCommand.argvCommand([
146+
'useradd', '-a', '-u', '1001', '-g', '1001', 'eguser',
147+
]),
148+
]);
149+
150+
// WHEN
151+
const init = ec2.CloudFormationInit.fromConfigSets({
152+
configSets: { default: ['config'] },
153+
configs: { config },
154+
});
155+
init.attach(resource, linuxOptions());
156+
157+
// THEN
158+
expectMetadataLike({
159+
'AWS::CloudFormation::Init': {
160+
configSets: {
161+
default: ['config'],
162+
},
163+
config: {
164+
commands: {
165+
'000': {
166+
command: ['useradd', '-u', '1001', '-g', '1001', 'eguser'],
167+
},
168+
'001': {
169+
command: ['useradd', '-a', '-u', '1001', '-g', '1001', 'eguser'],
170+
},
171+
},
172+
},
173+
},
174+
});
175+
});
176+
177+
test('deepMerge properly deduplicates non-command arguments', () => {
178+
// WHEN
179+
const config = new ec2.InitConfig([
180+
ec2.InitSource.fromUrl('/tmp/blinky', 'https://amazon.com/blinky.zip'),
181+
ec2.InitSource.fromUrl('/tmp/blinky', 'https://amazon.com/blinky.zip'),
182+
ec2.InitSource.fromUrl('/tmp/pinky', 'https://amazon.com/pinky.zip'),
183+
ec2.InitSource.fromUrl('/tmp/pinky', 'https://amazon.com/pinky.zip'),
184+
ec2.InitSource.fromUrl('/tmp/inky', 'https://amazon.com/inky.zip'),
185+
ec2.InitSource.fromUrl('/tmp/clyde', 'https://amazon.com/blinky.zip'),
186+
ec2.InitSource.fromUrl('/tmp/clyde', 'https://amazon.com/blinky.zip'),
187+
ec2.InitSource.fromUrl('/tmp/clyde', 'https://amazon.com/blinky.zip'),
188+
]);
189+
190+
// THEN
191+
expect(config._bind(stack, linuxOptions()).config).toEqual(expect.objectContaining({
192+
sources: {
193+
'/tmp/blinky': 'https://amazon.com/blinky.zip',
194+
'/tmp/pinky': 'https://amazon.com/pinky.zip',
195+
'/tmp/inky': 'https://amazon.com/inky.zip',
196+
'/tmp/clyde': 'https://amazon.com/blinky.zip',
197+
},
198+
}));
199+
});
200+
139201
describe('userdata', () => {
140202
let simpleInit: ec2.CloudFormationInit;
141203
beforeEach(() => {

0 commit comments

Comments
 (0)