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

[lambda] Function.currentVersion only updates for framework assets #10136

Closed
rix0rrr opened this issue Sep 2, 2020 · 8 comments
Closed

[lambda] Function.currentVersion only updates for framework assets #10136

rix0rrr opened this issue Sep 2, 2020 · 8 comments
Labels
@aws-cdk/aws-lambda Related to AWS Lambda bug This issue is a bug. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. effort/medium Medium work item – several days of effort p1

Comments

@rix0rrr
Copy link
Contributor

rix0rrr commented Sep 2, 2020

This code seems to assume that the Function's CloudFormation representation is good enough to determine the full source hash, which is not true as soon as indirection ​through a { Ref } or something comes into play:

https://github.com/aws/aws-cdk/blob/master/packages/@aws-cdk/aws-lambda/lib/function-hash.ts

The Code class needs a way to mix in another value in there, so that if the Code comes from an asset, even if that asset is referenced through { Ref }s or whatever, it still has an opportunity to mix in a sourceHash of some sort to cause the Version to be updated.

  • It works for new-style assets because the asset sourceHash has been hardcoded into the CFN template fragment.

  • It works for old-style assets because the asset sourceHash has been hardcoded into the parameter name (so the template fragment looks like { Ref: AssetObjectKey035fcdc6941252eb150bf31facc9dc4e }.

  • It does not work for custom assets (such as the ones used to integrate with Amazon's internal build system) which reference parameters that look like { Ref: SomeNameThatsTheSameEveryTime }.

The correct solution is to explicitly work the asset.sourceHash into the content hash somewhere, and not rely on the CloudFormation template fragment alone.


This is 🐛 Bug Report

@rix0rrr rix0rrr added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Sep 2, 2020
@github-actions github-actions bot added the @aws-cdk/aws-lambda Related to AWS Lambda label Sep 2, 2020
@nija-at
Copy link
Contributor

nija-at commented Sep 3, 2020

Can you provide an example?

@nija-at nija-at added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Sep 3, 2020
@rix0rrr
Copy link
Contributor Author

rix0rrr commented Sep 3, 2020

Yes, but not publicly.

@SomayaB SomayaB removed needs-triage This issue or PR still needs to be triaged. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels Sep 3, 2020
@rix0rrr rix0rrr changed the title [lambda] Function.currentVersion only updates for new-style assets [lambda] Function.currentVersion only updates for framework assets Sep 7, 2020
@eladb
Copy link
Contributor

eladb commented Sep 7, 2020

Why not just use environment variables as a way to include additional info in the hash?

@nija-at nija-at added effort/medium Medium work item – several days of effort p1 labels Sep 8, 2020
@flemjame-at-amazon
Copy link
Contributor

Why not just use environment variables as a way to include additional info in the hash?

That is what we have been doing to work around this. It's not elegant, but it's functional.

@eladb
Copy link
Contributor

eladb commented Sep 21, 2020

Why not just use environment variables as a way to include additional info in the hash?

That is what we have been doing to work around this. It's not elegant, but it's functional.

We can add some "formal" way to salt the version if that feels better. What do you think?

@rehos
Copy link

rehos commented Sep 25, 2020

Is it an idea to use currentVersionOptions.codeSha256 as the formal way to salt the version?

@flemjame-at-amazon
Copy link
Contributor

flemjame-at-amazon commented Jan 19, 2021

Why not just use environment variables as a way to include additional info in the hash?

That is what we have been doing to work around this. It's not elegant, but it's functional.

We can add some "formal" way to salt the version if that feels better. What do you think?

Sorry to necro the thread -- yes I think that would be a reasonable approach. Having something in the documentation that mentions this workaround would also be helpful

mergify bot pushed a commit that referenced this issue Feb 1, 2021
This is created to provide an example of a workaround for #10136

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
NovakGu pushed a commit to NovakGu/aws-cdk that referenced this issue Feb 18, 2021
This is created to provide an example of a workaround for aws#10136

----

*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

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 Jun 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-lambda Related to AWS Lambda bug This issue is a bug. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. effort/medium Medium work item – several days of effort p1
Projects
None yet
Development

No branches or pull requests

6 participants