fix(lambda-python-alpha): bundling hash logic includes auth tokens in PIP urls, causing an unnecessary rebuild #27903
Conversation
github-actions
bot
added
beginning-contributor
There was a problem hiding this comment.
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.
kcp-chewie
changed the title
kcp-chewie
changed the title
kcp-chewie
force-pushed
the
sanitize-cache-key
branch
2 times, most recently
from
77c0adb
to
54e89c9
Compare
aws-cdk-automation
dismissed
their stale review
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
There was a problem hiding this comment.
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.
aws-cdk-automation
dismissed
their stale review
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
aws-cdk-automation
added
the
pr/needs-community-review
There was a problem hiding this comment.
Thanks 👍
Just some minor adjustments in my opinion.
Also, can you please change the title to describe the bug (not the solution) as per guidelines?
aws-cdk-automation
removed
the
pr/needs-community-review
kcp-chewie
force-pushed
the
sanitize-cache-key
branch
from
a6d9357
to
dd4144d
Compare
kcp-chewie
changed the title
kcp-chewie
force-pushed
the
sanitize-cache-key
branch
from
dd4144d
to
421481d
Compare
aws-cdk-automation
added
the
pr/needs-maintainer-review
|
Anything I can do to help this one get reviewed/merged? CC @lpizzinidev |
There was a problem hiding this comment.
Thanks @kcp-chewie. I surmise that this will cause unnecessary rebuilds for the first time people using PIP_INDEX_URL update their CDK with this change, but I suppose we are okay with that.
kaizencc
changed the title
github-actions
bot
added
the
effort/medium
aws-cdk-automation
removed
the
pr/needs-maintainer-review
|
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). |
|
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). |
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). |
|
Thanks @kaizencc ! For reference, this should only cause an unnecessary rebuild for anyone PIP_INDEX_URL with basic auth username/password and a static password. Folks using PIP_INDEX_URL without any auth should see no rebuild, and folks using PIP_INDEX_URL with a rotating token (like AWS CodeArtifact users) will see 1 final rebuild. |
… PIP urls, causing an unnecessary rebuild (aws#27903) Update the bundler hash logic to ignore the secret token that is included in the URL when fetching packages from Code Artifact. This token changes constantly, and prevents the results of a previous build from being reused, along with causing lambdas to be unnecessarily redeployed anytime the CDK is built, even if no code is changed. This implementation strips the token from the hash, but does not change anything else about the hash. Open question Currently this logic will error if an invalid URL string is passed for PIP_INDEX_URL or PIP_EXTRA_INDEX_URL. The build would fail later anyway when those URLs are being used, but I'm happy to try/catch this logic block to be more robust if that is preferred. Closes aws#27331. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Update the bundler hash logic to ignore the secret token that is included in the URL when fetching packages from Code Artifact. This token changes constantly, and prevents the results of a previous build from being reused, along with causing lambdas to be unnecessarily redeployed anytime the CDK is built, even if no code is changed.
This implementation strips the token from the hash, but does not change anything else about the hash.
Open question
Currently this logic will error if an invalid URL string is passed for PIP_INDEX_URL or PIP_EXTRA_INDEX_URL. The build would fail later anyway when those URLs are being used, but I'm happy to try/catch this logic block to be more robust if that is preferred.
Closes #27331.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license