Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ec2): naming collisions when using ec2.InitFile.fromAsset() on multiple instances in the same stack #27468

Merged
merged 17 commits into from
Oct 23, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
b9a7e9f
add first test case from issue repro code
sumupitchayan Oct 6, 2023
c30f22b
add md5hash(bindOptions.scope.node.id) to s3 asset name, but test is …
sumupitchayan Oct 9, 2023
c19797a
change bind id hash to hash of md5hash(bindOptions.scope.node.childre…
sumupitchayan Oct 9, 2023
9ec3c62
Merge branch 'main' into sumughan/fix-initfiles-with-same-filename
sumupitchayan Oct 9, 2023
f0fbafd
write initial integ test and create snapshots
sumupitchayan Oct 9, 2023
1d3077a
Merge branch 'main' into sumughan/fix-initfiles-with-same-filename
sumupitchayan Oct 10, 2023
9d61abd
update integ test to use two different testConfigFiles
sumupitchayan Oct 10, 2023
ffff9c3
update integ snapshots
sumupitchayan Oct 10, 2023
530b23a
Merge branch 'main' into sumughan/fix-initfiles-with-same-filename
sumupitchayan Oct 10, 2023
8f55c58
Merge branch 'main' into sumughan/fix-initfiles-with-same-filename
sumupitchayan Oct 10, 2023
a0a2c0a
add comment explaining why the children hash is used
sumupitchayan Oct 11, 2023
956cc54
Merge branch 'main' into sumughan/fix-initfiles-with-same-filename
sumupitchayan Oct 11, 2023
15276b7
Merge branch 'main' into sumughan/fix-initfiles-with-same-filename
sumupitchayan Oct 11, 2023
ae90224
Merge branch 'main' into sumughan/fix-initfiles-with-same-filename
sumupitchayan Oct 12, 2023
f7e98af
Merge branch 'main' into sumughan/fix-initfiles-with-same-filename
sumupitchayan Oct 16, 2023
ed7e007
Merge branch 'main' into sumughan/fix-initfiles-with-same-filename
sumupitchayan Oct 16, 2023
bc6d73c
Merge branch 'main' into sumughan/fix-initfiles-with-same-filename
mikewrighton Oct 23, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions packages/aws-cdk-lib/aws-ec2/lib/cfn-init-elements.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import * as iam from '../../aws-iam';
import * as s3 from '../../aws-s3';
import * as s3_assets from '../../aws-s3-assets';
import { Duration } from '../../core';
import { md5hash } from '../../core/lib/helpers-internal';

/**
* An object that represents reasons to restart an InitService
Expand Down Expand Up @@ -424,7 +425,7 @@ export abstract class InitFile extends InitElement {
public static fromAsset(targetFileName: string, path: string, options: InitFileAssetOptions = {}): InitFile {
return new class extends InitFile {
protected _doBind(bindOptions: InitBindOptions) {
const asset = new s3_assets.Asset(bindOptions.scope, `${targetFileName}Asset`, {
const asset = new s3_assets.Asset(bindOptions.scope, `${md5hash(bindOptions.scope.node.children.toString())}${targetFileName}Asset`, {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I'd find a comment useful here explaining why the hash is being taken this way.

path,
...options,
});
Expand Down Expand Up @@ -941,7 +942,7 @@ export abstract class InitSource extends InitElement {
public static fromAsset(targetDirectory: string, path: string, options: InitSourceAssetOptions = {}): InitSource {
return new class extends InitSource {
protected _doBind(bindOptions: InitBindOptions) {
const asset = new s3_assets.Asset(bindOptions.scope, `${targetDirectory}Asset`, {
const asset = new s3_assets.Asset(bindOptions.scope, `${md5hash(bindOptions.scope.node.children.toString())}${targetDirectory}Asset`, {
path,
...bindOptions,
});
Expand Down
57 changes: 57 additions & 0 deletions packages/aws-cdk-lib/aws-ec2/test/cfn-init-element.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,63 @@ describe('InitFile', () => {
});
});

test('does not have naming collision when multiple EC2 instances are defined in the same stack with using InitFile.fromAsset with the same targetFileName', () => {
// GIVEN
const myApp = new App();
const myStack = new Stack(myApp, 'myStack');
const vpc = new ec2.Vpc(myStack, 'vpc');

// WHEN
new ec2.Instance(myStack, 'FirstInstance', {
vpc,
instanceType:
ec2.InstanceType.of(ec2.InstanceClass.T3A, ec2.InstanceSize.MICRO),
machineImage: new ec2.AmazonLinuxImage({
generation: ec2.AmazonLinuxGeneration.AMAZON_LINUX_2,
}),
init: ec2.CloudFormationInit.fromConfigSets({
configSets: {
default: ['default'],
},
configs: {
default: new ec2.InitConfig([
ec2.InitFile.fromAsset(
'/target/path/config.json',
path.join(__dirname, 'init-configs/configFileForFirstInstance.json'),
),
]),
},
}),
});

// THEN
expect(() => {

new ec2.Instance(myStack, 'SecondInstance', {
vpc,
instanceType:
ec2.InstanceType.of(ec2.InstanceClass.T3A, ec2.InstanceSize.MICRO),
machineImage: new ec2.AmazonLinuxImage({
generation: ec2.AmazonLinuxGeneration.AMAZON_LINUX_2,
}),
init: ec2.CloudFormationInit.fromConfigSets({
configSets: {
default: ['default'],
},
configs: {
default: new ec2.InitConfig([
ec2.InitFile.fromAsset(
'/target/path/config.json',
path.join(__dirname, 'init-configs/configFileForSecondInstance.json'),
),
]),
},
}),
});

}).not.toThrow();
});

});

describe('InitGroup', () => {
Expand Down
Loading