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

(aws-ec2): InitFile construct IDs are interpolated from target_filename when using fromAsset() or fromSource() which can cause naming collisions #16891

Closed
its-mirus-lu opened this issue Oct 10, 2021 · 5 comments · Fixed by #27468
Assignees
Labels
@aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud bug This issue is a bug. effort/small Small work item – less than a day of effort p1

Comments

@its-mirus-lu
Copy link

its-mirus-lu commented Oct 10, 2021

What is the problem?

TLDR: Naming collision of InitFile constructs for multiple ec2 instances defined in the same stack definition when using the fromAsset or fromSource methods. Affected file: https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk/aws-ec2/lib/cfn-init-elements.ts. Cause is most likely due to the underlying InitFile fromAsset and fromSource methods creating an s3 asset with interpolated id of ${targetFileName}Asset.

The Scenario

I'm deploying a fleet of instances, each requiring a configuration file (unique to each instance) to be placed in the same directory in each instance. For example instance-a has a config file that has a config file that is different to that of instance-b. In both instances, the file is placed in the path /etc/gitlab-runner/config.toml.

I've leveraged cfn-init and the cdk aws-ec2.InitFile.fromAsset() construct and method to create the file on each instance.

When synthesizing, the following error is returned:

jsii.errors.JSIIError: There is already a Construct with name '--etc--gitlab-runner--config.tomlAsset' in Stack

Reproduction Steps

  1. Create a stack via cdk
  2. Define an ec2 instance instance-a in the stack
  3. Define a cfn-init configuration for instance-a via aws_ec2.InitConfig
  4. Within the InitConfig for this instance, add an InitFile element like this:
ec2.InitFile.from_asset(
    target_file_name="/etc/gitlab-runner/config.toml",
    path=toml_file1.toml,
)
  1. In the same stack, define a second ec2 for instance-b
  2. Define a cfn-init configuration for instance-a via another aws_ec2.InitConfig,
  3. Within the InitConfig for this instance, add an InitFile element like this:
ec2.InitFile.from_asset(
    target_file_name="/etc/gitlab-runner/config.toml",
    path=toml_file2.toml,
)
  1. Attempt to synthesize CloudFormation from the code

What did you expect to happen?

I expected each InitFile to have a uniquely interpolated construct Id, and for there not to be a collision in names.

What actually happened?

Both constructs for InitFile were named --etc--gitlab-runner--config.tomlAsset resulting in the synth error jsii.errors.JSIIError: There is already a Construct with name '--etc--gitlab-runner--config.tomlAsset' in Stack [...]

The construct Id for Init file is interpolated from the target_file_name, which will cause an issue if multiple EC2 instances are defined in the same stack, each which use the InitFile construct to place files in the same folder in their respective instances.

CDK CLI Version

1.24.0

Framework Version

1.24.0

Node.js Version

v16.3.0

OS

MacOS Catalina 10.15.7

Language

Python

Language Version

Python 3.9.7

Other information

Workaround

In an InitConfig:

  1. When defining an InitFile element, specify a target_file_name that contains the instance-id,
  2. Add another element of type InitCommand and move the file from step 1 to the final destination

Example:

config_toml = ec2.InitConfig(
    elements=[
        
        ec2.InitFile.from_asset(
            target_file_name="/etc/gitlab-runner/config" + logical_id + ".toml",
            path=toml_path,
        ),
        ec2.InitCommand.shell_command(
            shell_command="mv /etc/gitlab-runner/config"
            + logical_id
            + ".toml /etc/gitlab-runner/config.toml"
        ),
    ]
)

Possible Solution

Looking at https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk/aws-ec2/lib/cfn-init-elements.ts there are two places where an s3 asset is used as a staging location for files before being placed on an instance. This s3 asset construct's ID is causing the naming collision in cdk.

Classes InitFile and InitSource both use the same naming convention for the s3_asset construct id:

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`_**, {
          path,
          ...bindOptions,
        });

A fix might be to interpolate the s3_assets.Asset id as a combination of the filename and a unique hash that wouldn't change on each synth.

@its-mirus-lu its-mirus-lu added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Oct 10, 2021
@github-actions github-actions bot added the @aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud label Oct 10, 2021
@its-mirus-lu its-mirus-lu changed the title (aws-ec2): InitFile construct ID's are interpolated from target_filename which can cause naming collisions (aws-ec2): InitFile construct IDs are interpolated from target_filename which can cause naming collisions Oct 10, 2021
@its-mirus-lu its-mirus-lu changed the title (aws-ec2): InitFile construct IDs are interpolated from target_filename which can cause naming collisions (aws-ec2): InitFile construct IDs are interpolated from target_filename when using fromAsset() or fromSource() which can cause naming collisions Oct 10, 2021
@njlynch njlynch added effort/small Small work item – less than a day of effort p1 and removed needs-triage This issue or PR still needs to be triaged. labels Oct 15, 2021
@njlynch njlynch removed their assignment Oct 15, 2021
@njlynch
Copy link
Contributor

njlynch commented Oct 15, 2021

Thanks for the bug report and the detailed write-up!

@github-actions
Copy link

This issue has not received any attention in 1 year. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added closing-soon This issue will automatically close in 4 days unless further comments are made. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels Oct 15, 2022
@peterwoodworth peterwoodworth reopened this May 1, 2023
@peterwoodworth
Copy link
Contributor

This is still an issue, reopening. More repro code can be found here thanks to @bpauwels

@sumupitchayan sumupitchayan added p1.5 and removed p1 labels May 16, 2023
@otaviomacedo otaviomacedo added p2 and removed p1.5 labels May 22, 2023
@peterwoodworth peterwoodworth added p1 needs-review and removed closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. p2 labels Jul 20, 2023
@macdrorepo
Copy link

Any progress here?

@sumupitchayan sumupitchayan self-assigned this Oct 4, 2023
@mergify mergify bot closed this as completed in #27468 Oct 23, 2023
mergify bot pushed a commit that referenced this issue Oct 23, 2023
…multiple instances in the same stack (#27468)

Closes #16891 

If a user creates more than one EC2 instance in the same stack and defines an InitConfig for each instance using `ec2.InitFile.fromAsset()`, the user will get an error if they pass in the same `targetFilePath` or `targetDirectory`. This bug is due to [how the asset is uploaded to S3](https://github.com/aws/aws-cdk/blob/main/packages/aws-cdk-lib/aws-ec2/lib/cfn-init-elements.ts#L427).

This PR fixes this issue by adding a hash to the `targetFileName` before being uploaded as an s3 asset.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud bug This issue is a bug. effort/small Small work item – less than a day of effort p1
Projects
None yet
7 participants