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*
aws · Aug 8, 2024 · 1e7c690 · 1e7c690
1 parent 79b5cd2
commit 1e7c690
Show file tree

Hide file tree

Showing 8 changed files with 115 additions and 20 deletions.
diff --git a/...nteg.instance-init-multiple.js.snapshot/integ-ec2-multiple-instances-in-stack.assets.json b/...nteg.instance-init-multiple.js.snapshot/integ-ec2-multiple-instances-in-stack.assets.json
diff --git a/...eg.instance-init-multiple.js.snapshot/integ-ec2-multiple-instances-in-stack.template.json b/...eg.instance-init-multiple.js.snapshot/integ-ec2-multiple-instances-in-stack.template.json
@@ -906,7 +906,7 @@
       "Fn::Join": [
        "",
        [
-        "#!/bin/bash\n# fingerprint: 8787022e9944cbeb\n(\n  set +e\n  /opt/aws/bin/cfn-init -v --region ",
+        "#!/bin/bash\n# fingerprint: 370d9b2dcf8bf44b\n(\n  set +e\n  /opt/aws/bin/cfn-init -v --region ",
         {
          "Ref": "AWS::Region"
         },
@@ -955,6 +955,29 @@
         "owner": "root",
         "group": "root"
        }
+      },
+      "commands": {
+       "000": {
+        "command": [
+         "useradd",
+         "-u",
+         "1001",
+         "-g",
+         "1001",
+         "eguser"
+        ]
+       },
+       "001": {
+        "command": [
+         "useradd",
+         "-a",
+         "-u",
+         "1001",
+         "-g",
+         "1001",
+         "eguser"
+        ]
+       }
       }
      }
     },

diff --git a/.../framework-integ/test/aws-ec2/test/integ.instance-init-multiple.js.snapshot/manifest.json b/.../framework-integ/test/aws-ec2/test/integ.instance-init-multiple.js.snapshot/manifest.json
diff --git a/...ting/framework-integ/test/aws-ec2/test/integ.instance-init-multiple.js.snapshot/tree.json b/...ting/framework-integ/test/aws-ec2/test/integ.instance-init-multiple.js.snapshot/tree.json
diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-ec2/test/integ.instance-init-multiple.ts b/packages/@aws-cdk-testing/framework-integ/test/aws-ec2/test/integ.instance-init-multiple.ts
@@ -55,6 +55,12 @@ class TestStack extends cdk.Stack {
               '/target/path/config.json',
               path.join(tmpDir, 'testConfigFile2'),
             ),
+            ec2.InitCommand.argvCommand([
+              'useradd', '-u', '1001', '-g', '1001', 'eguser',
+            ]),
+            ec2.InitCommand.argvCommand([
+              'useradd', '-a', '-u', '1001', '-g', '1001', 'eguser',
+            ]),
           ]),
         },
       }),

diff --git a/...k-testing/framework-integ/test/aws-ec2/test/integ.instance-init.js.snapshot/manifest.json b/...k-testing/framework-integ/test/aws-ec2/test/integ.instance-init.js.snapshot/manifest.json
diff --git a/packages/aws-cdk-lib/aws-ec2/lib/cfn-init.ts b/packages/aws-cdk-lib/aws-ec2/lib/cfn-init.ts
@@ -311,10 +311,17 @@ function deepMerge(target?: Record<string, any>, src?: Record<string, any>) {
       if (target[key] && !Array.isArray(target[key])) {
         throw new Error(`Trying to merge array [${value}] into a non-array '${target[key]}'`);
       }
-      target[key] = Array.from(new Set([
-        ...target[key] ?? [],
-        ...value,
-      ]));
+      if (key === 'command') { // don't deduplicate command arguments
+        target[key] = new Array(
+          ...target[key] ?? [],
+          ...value,
+        );
+      } else {
+        target[key] = Array.from(new Set([
+          ...target[key] ?? [],
+          ...value,
+        ]));
+      }
       continue;
     }
     if (typeof value === 'object' && value) {

diff --git a/packages/aws-cdk-lib/aws-ec2/test/cfn-init.test.ts b/packages/aws-cdk-lib/aws-ec2/test/cfn-init.test.ts
@@ -136,6 +136,68 @@ test('empty configs are not rendered', () => {
   });
 });
 
+test('duplicate config arguments not deduplicated', () => {
+  //GIVEN
+  const config = new ec2.InitConfig([
+    ec2.InitCommand.argvCommand([
+      'useradd', '-u', '1001', '-g', '1001', 'eguser',
+    ]),
+    ec2.InitCommand.argvCommand([
+      'useradd', '-a', '-u', '1001', '-g', '1001', 'eguser',
+    ]),
+  ]);
+
+  // WHEN
+  const init = ec2.CloudFormationInit.fromConfigSets({
+    configSets: { default: ['config'] },
+    configs: { config },
+  });
+  init.attach(resource, linuxOptions());
+
+  // THEN
+  expectMetadataLike({
+    'AWS::CloudFormation::Init': {
+      configSets: {
+        default: ['config'],
+      },
+      config: {
+        commands: {
+          '000': {
+            command: ['useradd', '-u', '1001', '-g', '1001', 'eguser'],
+          },
+          '001': {
+            command: ['useradd', '-a', '-u', '1001', '-g', '1001', 'eguser'],
+          },
+        },
+      },
+    },
+  });
+});
+
+test('deepMerge properly deduplicates non-command arguments', () => {
+  // WHEN
+  const config = new ec2.InitConfig([
+    ec2.InitSource.fromUrl('/tmp/blinky', 'https://amazon.com/blinky.zip'),
+    ec2.InitSource.fromUrl('/tmp/blinky', 'https://amazon.com/blinky.zip'),
+    ec2.InitSource.fromUrl('/tmp/pinky', 'https://amazon.com/pinky.zip'),
+    ec2.InitSource.fromUrl('/tmp/pinky', 'https://amazon.com/pinky.zip'),
+    ec2.InitSource.fromUrl('/tmp/inky', 'https://amazon.com/inky.zip'),
+    ec2.InitSource.fromUrl('/tmp/clyde', 'https://amazon.com/blinky.zip'),
+    ec2.InitSource.fromUrl('/tmp/clyde', 'https://amazon.com/blinky.zip'),
+    ec2.InitSource.fromUrl('/tmp/clyde', 'https://amazon.com/blinky.zip'),
+  ]);
+
+  // THEN
+  expect(config._bind(stack, linuxOptions()).config).toEqual(expect.objectContaining({
+    sources: {
+      '/tmp/blinky': 'https://amazon.com/blinky.zip',
+      '/tmp/pinky': 'https://amazon.com/pinky.zip',
+      '/tmp/inky': 'https://amazon.com/inky.zip',
+      '/tmp/clyde': 'https://amazon.com/blinky.zip',
+    },
+  }));
+});
+
 describe('userdata', () => {
   let simpleInit: ec2.CloudFormationInit;
   beforeEach(() => {