-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Conversation
Signed-off-by: Sumu <sumughan@amazon.com>
…still failing Signed-off-by: Sumu <sumughan@amazon.com>
…n.toString()) -- test now passes successfully Signed-off-by: Sumu <sumughan@amazon.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.
A comment requesting an exemption should contain the text Exemption Request
. Additionally, if clarification is needed add Clarification Request
to a comment.
ec2.InitFile.fromAsset()
on multiple instances in the same stackec2.InitFile.fromAsset()
on multiple instances in the same stack
Signed-off-by: Sumu <sumughan@amazon.com>
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
Signed-off-by: Sumu <sumughan@amazon.com>
Signed-off-by: Sumu <sumughan@amazon.com>
nit: typo in the description? |
@@ -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`, { |
There was a problem hiding this comment.
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.
Signed-off-by: Sumu <sumughan@amazon.com>
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
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 sametargetFilePath
ortargetDirectory
. This bug is due to how the asset is uploaded to S3.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