From 1e412a0d36f92660759aadf8eb30e72181607f46 Mon Sep 17 00:00:00 2001 From: Leonardo Gama Date: Wed, 10 Jul 2024 10:26:31 -0700 Subject: [PATCH 1/9] Prevent deduplication of init command arguments --- packages/aws-cdk-lib/aws-ec2/lib/cfn-init.ts | 12 ++++-- .../aws-cdk-lib/aws-ec2/test/cfn-init.test.ts | 38 +++++++++++++++++++ 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/packages/aws-cdk-lib/aws-ec2/lib/cfn-init.ts b/packages/aws-cdk-lib/aws-ec2/lib/cfn-init.ts index bf055b4b27a41..89a76399c8d0b 100644 --- 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,14 @@ function deepMerge(target?: Record, src?: Record) { 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] = Array.from(new Set([ + ...target[key] ?? [], + ...value, + ])); + } else { + 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 index bf6823a598988..793e2cf166ddd 100644 --- 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,44 @@ 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'], + }, + }, + }, + }, + }); +}); + describe('userdata', () => { let simpleInit: ec2.CloudFormationInit; beforeEach(() => { From d6088a03d08b7962639df18895ee8c872e894f76 Mon Sep 17 00:00:00 2001 From: Leonardo Gama Date: Wed, 10 Jul 2024 11:36:57 -0700 Subject: [PATCH 2/9] Fix linting --- packages/aws-cdk-lib/aws-ec2/lib/cfn-init.ts | 2 +- packages/aws-cdk-lib/aws-ec2/test/cfn-init.test.ts | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/aws-cdk-lib/aws-ec2/lib/cfn-init.ts b/packages/aws-cdk-lib/aws-ec2/lib/cfn-init.ts index 89a76399c8d0b..62b016cb9c63d 100644 --- a/packages/aws-cdk-lib/aws-ec2/lib/cfn-init.ts +++ b/packages/aws-cdk-lib/aws-ec2/lib/cfn-init.ts @@ -311,7 +311,7 @@ function deepMerge(target?: Record, src?: Record) { if (target[key] && !Array.isArray(target[key])) { throw new Error(`Trying to merge array [${value}] into a non-array '${target[key]}'`); } - if (key != 'command') { // don't deduplicate command arguments + if (key != 'command') { // don't deduplicate command arguments target[key] = Array.from(new Set([ ...target[key] ?? [], ...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 index 793e2cf166ddd..3e69071aa114a 100644 --- a/packages/aws-cdk-lib/aws-ec2/test/cfn-init.test.ts +++ b/packages/aws-cdk-lib/aws-ec2/test/cfn-init.test.ts @@ -146,11 +146,11 @@ test('duplicate config arguments not deduplicated', () => { 'useradd', '-a', '-u', '1001', '-g', '1001', 'eguser', ]), ]); - + // WHEN const init = ec2.CloudFormationInit.fromConfigSets({ - configSets: {default: ['config']}, - configs: {config}, + configSets: { default: ['config'] }, + configs: { config }, }); init.attach(resource, linuxOptions()); From 6fea359aed7a36a3a88842349acd2c85e0a9b07e Mon Sep 17 00:00:00 2001 From: Leonardo Gama Date: Wed, 10 Jul 2024 13:53:11 -0700 Subject: [PATCH 3/9] Update integ test snapshot --- .../integ.instance-init.js.snapshot/manifest.json | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-ec2/test/integ.instance-init.js.snapshot/manifest.json b/packages/@aws-cdk-testing/framework-integ/test/aws-ec2/test/integ.instance-init.js.snapshot/manifest.json index b0e1d19683224..e49e7eba966a3 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-ec2/test/integ.instance-init.js.snapshot/manifest.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-ec2/test/integ.instance-init.js.snapshot/manifest.json @@ -199,7 +199,10 @@ "/integ-init/Instance2/Resource": [ { "type": "aws:cdk:logicalId", - "data": "Instance255F3526574cbd507dfce8b71" + "data": "Instance255F3526574cbd507dfce8b71", + "trace": [ + "!!DESTRUCTIVE_CHANGES: WILL_DESTROY" + ] } ], "/integ-init/SsmParameterValue:--aws--service--ami-amazon-linux-latest--amzn-ami-hvm-x86_64-gp2:C96584B6-F00A-464E-AD19-53AFF4B05118.Parameter": [ @@ -219,15 +222,6 @@ "type": "aws:cdk:logicalId", "data": "CheckBootstrapVersion" } - ], - "Instance255F35265a0c5f577d761edb0": [ - { - "type": "aws:cdk:logicalId", - "data": "Instance255F35265a0c5f577d761edb0", - "trace": [ - "!!DESTRUCTIVE_CHANGES: WILL_DESTROY" - ] - } ] }, "displayName": "integ-init" From 3c230a71db5e6808ca98db52f5c2c6728c84b970 Mon Sep 17 00:00:00 2001 From: Leonardo Gama Date: Wed, 10 Jul 2024 14:08:48 -0700 Subject: [PATCH 4/9] Update integ tests --- ...c2-multiple-instances-in-stack.assets.json | 4 +-- ...-multiple-instances-in-stack.template.json | 25 ++++++++++++++++++- .../manifest.json | 7 ++++-- .../tree.json | 2 +- .../test/integ.instance-init-multiple.ts | 6 +++++ 5 files changed, 38 insertions(+), 6 deletions(-) diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-ec2/test/integ.instance-init-multiple.js.snapshot/integ-ec2-multiple-instances-in-stack.assets.json b/packages/@aws-cdk-testing/framework-integ/test/aws-ec2/test/integ.instance-init-multiple.js.snapshot/integ-ec2-multiple-instances-in-stack.assets.json index 37139758282cd..efd673ae6169d 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-ec2/test/integ.instance-init-multiple.js.snapshot/integ-ec2-multiple-instances-in-stack.assets.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-ec2/test/integ.instance-init-multiple.js.snapshot/integ-ec2-multiple-instances-in-stack.assets.json @@ -40,7 +40,7 @@ } } }, - "55adc2a4d264e77d2c794df6cb13dd26ee0a9986d5b00a9bfe3cd48b6e0b2dfe": { + "db1c5d9623e5e22db4e511f52daf89f93fb1cbb8f325df40d03d5c50413e63a3": { "source": { "path": "integ-ec2-multiple-instances-in-stack.template.json", "packaging": "file" @@ -48,7 +48,7 @@ "destinations": { "current_account-current_region": { "bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}", - "objectKey": "55adc2a4d264e77d2c794df6cb13dd26ee0a9986d5b00a9bfe3cd48b6e0b2dfe.json", + "objectKey": "db1c5d9623e5e22db4e511f52daf89f93fb1cbb8f325df40d03d5c50413e63a3.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-ec2/test/integ.instance-init-multiple.js.snapshot/integ-ec2-multiple-instances-in-stack.template.json b/packages/@aws-cdk-testing/framework-integ/test/aws-ec2/test/integ.instance-init-multiple.js.snapshot/integ-ec2-multiple-instances-in-stack.template.json index 787b41606da14..e09958092be8c 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-ec2/test/integ.instance-init-multiple.js.snapshot/integ-ec2-multiple-instances-in-stack.template.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-ec2/test/integ.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/packages/@aws-cdk-testing/framework-integ/test/aws-ec2/test/integ.instance-init-multiple.js.snapshot/manifest.json b/packages/@aws-cdk-testing/framework-integ/test/aws-ec2/test/integ.instance-init-multiple.js.snapshot/manifest.json index 9d805bac6514b..bd244eaf3c6a4 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-ec2/test/integ.instance-init-multiple.js.snapshot/manifest.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-ec2/test/integ.instance-init-multiple.js.snapshot/manifest.json @@ -18,7 +18,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}/55adc2a4d264e77d2c794df6cb13dd26ee0a9986d5b00a9bfe3cd48b6e0b2dfe.json", + "stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/db1c5d9623e5e22db4e511f52daf89f93fb1cbb8f325df40d03d5c50413e63a3.json", "requiresBootstrapStackVersion": 6, "bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version", "additionalDependencies": [ @@ -259,7 +259,10 @@ "/integ-ec2-multiple-instances-in-stack/SecondInstance/Resource": [ { "type": "aws:cdk:logicalId", - "data": "SecondInstance4834A636" + "data": "SecondInstance4834A636", + "trace": [ + "!!DESTRUCTIVE_CHANGES: MAY_REPLACE" + ] } ], "/integ-ec2-multiple-instances-in-stack/BootstrapVersion": [ diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-ec2/test/integ.instance-init-multiple.js.snapshot/tree.json b/packages/@aws-cdk-testing/framework-integ/test/aws-ec2/test/integ.instance-init-multiple.js.snapshot/tree.json index e556e0419844d..11c69d2c333ad 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-ec2/test/integ.instance-init-multiple.js.snapshot/tree.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-ec2/test/integ.instance-init-multiple.js.snapshot/tree.json @@ -1265,7 +1265,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" }, 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 index 87a8ab87c74e6..de01a25b4715f 100644 --- 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', + ]), ]), }, }), From 51131e6eaaaa0c9ba5f1d6a29ab24e7a37758bbc Mon Sep 17 00:00:00 2001 From: Leonardo Gama Date: Fri, 12 Jul 2024 14:50:04 -0700 Subject: [PATCH 5/9] Update dedup array logic --- packages/aws-cdk-lib/aws-ec2/lib/cfn-init.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/aws-cdk-lib/aws-ec2/lib/cfn-init.ts b/packages/aws-cdk-lib/aws-ec2/lib/cfn-init.ts index 62b016cb9c63d..86ee5e3a38ced 100644 --- a/packages/aws-cdk-lib/aws-ec2/lib/cfn-init.ts +++ b/packages/aws-cdk-lib/aws-ec2/lib/cfn-init.ts @@ -317,7 +317,10 @@ function deepMerge(target?: Record, src?: Record) { ...value, ])); } else { - target[key] = value; + target[key] = new Array( + ...target[key] ?? [], + ...value, + ); } continue; } From cb5d97e2de3af107ffe1fceb9b9ab1053dea3def Mon Sep 17 00:00:00 2001 From: Leonardo Gama Date: Fri, 26 Jul 2024 10:23:19 -0700 Subject: [PATCH 6/9] Change flow control --- packages/aws-cdk-lib/aws-ec2/lib/cfn-init.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/aws-cdk-lib/aws-ec2/lib/cfn-init.ts b/packages/aws-cdk-lib/aws-ec2/lib/cfn-init.ts index 86ee5e3a38ced..e97992612b233 100644 --- a/packages/aws-cdk-lib/aws-ec2/lib/cfn-init.ts +++ b/packages/aws-cdk-lib/aws-ec2/lib/cfn-init.ts @@ -311,16 +311,16 @@ function deepMerge(target?: Record, src?: Record) { if (target[key] && !Array.isArray(target[key])) { throw new Error(`Trying to merge array [${value}] into a non-array '${target[key]}'`); } - if (key != 'command') { // don't deduplicate command arguments - target[key] = Array.from(new Set([ + if (key === 'command') { // don't deduplicate command arguments + target[key] = new Array( ...target[key] ?? [], ...value, - ])); + ); } else { - target[key] = new Array( + target[key] = Array.from(new Set([ ...target[key] ?? [], ...value, - ); + ])); } continue; } From 69979248e9692e28866376e9dcf048cd56fd1a2b Mon Sep 17 00:00:00 2001 From: Leonardo Gama Date: Fri, 26 Jul 2024 13:22:55 -0700 Subject: [PATCH 7/9] Test for deduplication --- .../aws-cdk-lib/aws-ec2/test/cfn-init.test.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) 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 index 3e69071aa114a..d4a256b1a1ad3 100644 --- a/packages/aws-cdk-lib/aws-ec2/test/cfn-init.test.ts +++ b/packages/aws-cdk-lib/aws-ec2/test/cfn-init.test.ts @@ -174,6 +174,23 @@ test('duplicate config arguments not deduplicated', () => { }); }); +test('deepMerge properly deduplicates non-command arguments', () => { + // WHEN + const config = new ec2.InitConfig([ + ec2.InitSource.fromUrl('/tmp/foo', 'https://amazon.com/foo.zip'), + ec2.InitSource.fromUrl('/tmp/foo', 'https://amazon.com/foo.zip'), + ec2.InitSource.fromUrl('/tmp/foo', 'https://amazon.com/foo.zip'), + ]); + + // THEN + console.log(config._bind(stack, linuxOptions()).config); + expect(config._bind(stack, linuxOptions()).config).toEqual(expect.objectContaining({ + sources: { + "/tmp/foo": "https://amazon.com/foo.zip", + }, + })); +}); + describe('userdata', () => { let simpleInit: ec2.CloudFormationInit; beforeEach(() => { From 7d38fdc0641f9a175d9a34dc3427887340cc3265 Mon Sep 17 00:00:00 2001 From: Leonardo Gama Date: Fri, 26 Jul 2024 13:35:55 -0700 Subject: [PATCH 8/9] Fix linting errors --- packages/aws-cdk-lib/aws-ec2/test/cfn-init.test.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 index d4a256b1a1ad3..cd56ab2e8ed37 100644 --- a/packages/aws-cdk-lib/aws-ec2/test/cfn-init.test.ts +++ b/packages/aws-cdk-lib/aws-ec2/test/cfn-init.test.ts @@ -183,10 +183,9 @@ test('deepMerge properly deduplicates non-command arguments', () => { ]); // THEN - console.log(config._bind(stack, linuxOptions()).config); expect(config._bind(stack, linuxOptions()).config).toEqual(expect.objectContaining({ sources: { - "/tmp/foo": "https://amazon.com/foo.zip", + '/tmp/foo': 'https://amazon.com/foo.zip', }, })); }); From f2e4ca1dc1d9662c3f4ff05feb41176a90e6aaea Mon Sep 17 00:00:00 2001 From: Leonardo Gama Date: Mon, 29 Jul 2024 10:35:27 -0700 Subject: [PATCH 9/9] Add additional dedup checks --- .../aws-cdk-lib/aws-ec2/test/cfn-init.test.ts | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) 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 index cd56ab2e8ed37..0a7f5f3a7181b 100644 --- a/packages/aws-cdk-lib/aws-ec2/test/cfn-init.test.ts +++ b/packages/aws-cdk-lib/aws-ec2/test/cfn-init.test.ts @@ -177,15 +177,23 @@ test('duplicate config arguments not deduplicated', () => { test('deepMerge properly deduplicates non-command arguments', () => { // WHEN const config = new ec2.InitConfig([ - ec2.InitSource.fromUrl('/tmp/foo', 'https://amazon.com/foo.zip'), - ec2.InitSource.fromUrl('/tmp/foo', 'https://amazon.com/foo.zip'), - ec2.InitSource.fromUrl('/tmp/foo', 'https://amazon.com/foo.zip'), + 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/foo': 'https://amazon.com/foo.zip', + '/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', }, })); });