-
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
feat(lambda-python): support setting environment vars for bundling #18635
feat(lambda-python): support setting environment vars for bundling #18635
Conversation
# Upgrade pip (required by cryptography v3.4 and above, which is a dependency of poetry) | ||
RUN pip install --upgrade pip | ||
RUN pip install pipenv poetry |
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.
Moved this up since if using Code Artifact, this install uses the private repo, which forces rebuilds of the Docker image every time (and skipping Docker cache).
1847f89
to
2e8ba56
Compare
test('Bundling with custom build arg for `PIP_EXTRA_INDEX_URL`', () => { | ||
const entry = path.join(__dirname, 'lambda-handler'); | ||
const testPypi = 'https://test.pypi.org/simple/'; | ||
Bundling.bundle({ | ||
entry: entry, | ||
runtime: Runtime.PYTHON_3_7, | ||
buildArgs: { PIP_EXTRA_INDEX_URL: testPypi }, | ||
}); | ||
|
||
expect(DockerImage.fromBuild).toHaveBeenCalledWith(expect.stringMatching(path.join(__dirname, '../lib')), expect.objectContaining({ | ||
buildArgs: expect.objectContaining({ | ||
PIP_EXTRA_INDEX_URL: testPypi, | ||
}), | ||
})); | ||
}); |
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.
Added this to pacify the validate-pr CI. Not sure if there's a way to validate env vars in Docker images (the change we are making here).
@corymhall : Any feedback on this? Tiny fix, so would be great to get into the next release. |
I think it should be pretty easy to add environment variable support. You just need to add environment to
|
Neat! I thought I was missing something obvious. Thanks for the pointer, updating the PR. |
2e8ba56
to
220d089
Compare
00005df
to
a8dbafe
Compare
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.
@corymhall : Thanks for the tip! Updated the PR and docs.
}); | ||
``` | ||
|
||
The index URL or the token are only used during bundling and thus not included in the final 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.
Added note about tokens not being persisted in the final asset.
@@ -172,4 +199,33 @@ new lambda.PythonFunction(this, 'function', { | |||
}); | |||
``` | |||
|
|||
This type of an example should work for `pip` and `poetry` based dependencies, but will not work for `pipenv`. | |||
**Note:** Setting custom build args for bundling will force the base bundling image to be rebuilt every time (i.e. skip the Docker cache). |
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.
And about rebuilding Docker images for bundling if using build args.
Setting only environment variable should work for `pip` and `poetry` based dependencies, whereas `pipenv` based dependencies will require **both** build args and environment variables to be set. | ||
|
||
|
||
Example for using Code Artifact with `pipenv`-based dependencies: |
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.
Running into this also made clear how we could use pipenv
with Code Artifact.
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.
Why does pipenv
require both env vars and build args?
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.
Thanks for asking, @corymhall. That was an incorrect note on my part stemming from how bundling was occurring earlier (in the Docker build step), and I didn't completely understand it since I don't use pipenv
.
Over the last couple days, I created a test project with pipenv
and validated installing a package into it that is only on Test PyPI. It works with the PIP_INDEX_URL
environment variable set (same as for pip
and poetry
), which it'll be when the Docker run step occurs. I updated the documentation now to reflect that.
Setting only environment variable should work for `pip` and `poetry` based dependencies, whereas `pipenv` based dependencies will require **both** build args and environment variables to be set. | ||
|
||
|
||
Example for using Code Artifact with `pipenv`-based dependencies: |
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.
Why does pipenv
require both env vars and build args?
Pull request has been modified.
3a75397
to
1a7fc45
Compare
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.
Looks good!
Thank you for contributing! Your pull request will be updated from master 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 master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
…ws#18635) While using the Python Lambda with Code Artifact, discovered that Code Artifact was still inaccessible because bundling occurs at _run_ time, which can only access env vars, not build args. This is not a security issue because bundled output doesn't contain any of the secret values. **Note:** Without this, using Code Artifact (or any other private packaging for Python Lambdas) is currently broken. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
While using the Python Lambda with Code Artifact, discovered that Code Artifact was still inaccessible because bundling occurs at run time, which can only access env vars, not build args.
This is not a security issue because bundled output doesn't contain any of the secret values.
Note: Without this, using Code Artifact (or any other private packaging for Python Lambdas) is currently broken.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license