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.

Sign up for GitHub

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

Jump to bottom

[aws-ec2] Asset based file fingerprint is not included in ec2 user data. #10206

Closed
allanlw opened this issue Sep 6, 2020 · 2 comments · Fixed by #10216
Closed

[aws-ec2] Asset based file fingerprint is not included in ec2 user data. #10206

allanlw opened this issue Sep 6, 2020 · 2 comments · Fixed by #10216
Assignees
@njlynch
Labels
@aws-cdk/aws-autoscaling Related to Amazon EC2 Auto Scaling @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 needs-triage This issue or PR still needs to be triaged. p1

Comments

@allanlw
Copy link
Contributor

allanlw commented Sep 6, 2020 •
edited
Loading

When using CFN init with an ec2.Instance, setting userDataCausesReplacement (not default) and initOptions.embedFingerprint (the default) normally means modifying the provided CloudFormationInit will replace the instance.

However, with InitFile (added in #9664) using InitFile.FromAsset to specify a local file to be delivered as an asset, updating the local file does not update the fingerprint embedded in the user data, so instances are not replaced, even though the new asset is deployed.

Reproduction Steps

  1. Create an ec2.Instance with userDataCausesReplacement and initOptions.embedFingerprint and a CloudFormationInit containing an InitFile.FromAsset with a local file.
  2. Deploy.
  3. Modify the local file.
  4. Deploy

What did you expect to happen?

The instance should be replaced.

What actually happened?

The instance is not replaced, even though the asset is.

Environment

  • CLI Version : 1.62.0
  • Framework Version: 1.62.0
  • Node.js Version: v12.18.0
  • OS : Linux
  • Language (Version): Typescript

Other

This might be intended and/or desirable behavior. Regardless, a flag like includeFileAssetsInFingerprint on initOptions or includeInFingerprint on initFileOptions would be prudent.

This is 🐛 Bug Report
@allanlw allanlw added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Sep 6, 2020
@github-actions github-actions bot added the @aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud label Sep 6, 2020
@rix0rrr
Copy link
Contributor

rix0rrr commented Sep 7, 2020

No, you are correct that this is a bug.

@rix0rrr rix0rrr added @aws-cdk/aws-autoscaling Related to Amazon EC2 Auto Scaling effort/small Small work item – less than a day of effort p1 labels Sep 7, 2020
@njlynch njlynch assigned njlynch and unassigned rix0rrr Sep 7, 2020
njlynch added a commit that referenced this issue Sep 7, 2020
@njlynch
fix(ec2): cfn-init - asset-based file fingerprint not included in use…
360ad5c 
…r data

We fingerprint the cfn-init configuration to insert into the user data so
changes to the cfn-init config can trigger an instance replacement; however,
the fingerprint was being calculated on the raw config, including tokens,
so the asset hash was not being considered in the fingerprint.

This fix resolves the tokens so the fingerprint takes asset hashes into
consideration.

fixes #10206
@njlynch njlynch mentioned this issue Sep 7, 2020
Merged
@mergify mergify bot closed this as completed in #10216 Sep 7, 2020
mergify bot pushed a commit that referenced this issue Sep 7, 2020
@njlynch
fix(ec2): cfn-init user data hash not updated if file asset changes (#…
0d7ca63 
…10216)

We fingerprint the cfn-init configuration to insert into the user data so
changes to the cfn-init config can trigger an instance replacement; however,
the fingerprint was being calculated on the raw config, including tokens,
so the asset hash was not being considered in the fingerprint.

This fix resolves the tokens so the fingerprint takes asset hashes into
consideration.

fixes #10206

----

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

allanlw commented Sep 7, 2020

Wow, thanks for the quick turn around! Cheers!

@Harper04 Harper04 mentioned this issue Nov 25, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@njlynch njlynch

Labels
@aws-cdk/aws-autoscaling Related to Amazon EC2 Auto Scaling @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 needs-triage This issue or PR still needs to be triaged. p1
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(ec2): cfn-init user data hash not updated if file asset changes aws/aws-cdk
3 participants
@rix0rrr @njlynch @allanlw