terraform-aws-modules / terraform-aws-lambda

Terraform module, which takes care of a lot of AWS Lambda/serverless tasks (build dependencies, packages, updates, deployments) in countless combinations πŸ‡ΊπŸ‡¦
https://registry.terraform.io/modules/terraform-aws-modules/lambda/aws
Apache License 2.0
886 stars 656 forks source link

fix: Pass `--platform=manylinux2014` to `pip install` #444

Closed pdecat closed 1 year ago

pdecat commented 1 year ago

Description

As mentioned in AWS documentation, the --platform=manylinux2014 parameter must be passed to pip install for versions of packages with compiled binaries compatible with Lambda runtimes to be installed.

Not doing so causes wrong versions of python wheel packages to be installed when applying a terraform configuration with Lambda resources on platforms with a newer glibc than Lambda supports.

This results in runtime errors like this one with recent cryptography packages:

/lib64/libc.so.6: version `GLIBC_2.28' not found (required by /opt/python/cryptography/hazmat/bindings/_rust.abi3.so

Motivation and Context

Resolves https://github.com/terraform-aws-modules/terraform-aws-lambda/issues/346

For reference:

https://repost.aws/knowledge-center/lambda-python-package-compatible https://docs.aws.amazon.com/lambda/latest/dg/lambda-runtimes.html https://aws.amazon.com/fr/amazon-linux-2/faqs/ https://peps.python.org/pep-0599/#the-manylinux2014-policy https://github.com/pyca/cryptography/issues/6391

Breaking Changes

No.

How Has This Been Tested?

pdecat commented 1 year ago

Marked as draft as testing is still in progress.

pdecat commented 1 year ago

CI seems broken because of network issues:

β”‚ Error: Failed to install provider
β”‚ 
β”‚ Error while installing hashicorp/external v2.3.1: read tcp
β”‚ 10.1.1.20:40466->108.138.85.65:443: read: connection reset by peer
antonbabenko commented 1 year ago

@pdecat Rerun helped.

pdecat commented 1 year ago

So, everything is working fine in my development environment with this change, the proper manylinux2014 version of cryptography is packaged in the Lambda zip archive.

It took a bit of time to confirm because since usage of hash_extra_paths was disabled, the content of package.py is no longer taken into account for the hash computation, so no changes where triggered.

Reading through the original issue, I fail to understand why hash_extra_paths was disabled as terraform plans shown in that issue mention changes on the timestamp, but not the filename which uses the computed hash.

pdecat commented 1 year ago

Turns out it is trickier than expected as the target instruction set architecture must be specified in the --platform parameter passed to pip, i.e. manylinux2014_x86_64 or manylinux2014_aarch64.

I could have used the value passed in the architectures field for Lambda functions, but layers can support more than one architecture in compatible_architectures.

pdecat commented 1 year ago

In the meantime, the work-around when building Lambda targeting a python runtime, and not using docker, is to export environment variable before running terraform plan/apply:

# export PIP_PLATFORM="manylinux2014_x86_64"
# export PIP_ONLY_BINARY=":all:"

Note: it is necessary to taint/replace existing null_resource.archive[0] and local_file.archive_plan[0] resources from the terraform-aws-lambda module instance, and to delete existing local files from artifacts_dir.

The easier way to ensure the package are properly rebuilt with the environment variables taken into account is to also export TF_LAMBDA_PACKAGE_LOG_LEVEL=DEBUG3, and lookup which versions of the Wheel archives are downloaded.

pdecat commented 1 year ago

I guess I'll close this PR, and use the aforementioned work-around until Amazon releases support for Python 3.10, or better yet 3.11, with runtimes based on Amazon Linux 2023 that comes with glibc 2.34.

pdecat commented 1 year ago

After thinking a bit more about it, I believe this can be achieved by:

This should avoid breaking any existing usages of the module. And to opt-in the new behavior, one would have to define a single architecture for its function or layer.

antonbabenko commented 1 year ago

@pdecat Thank you very much for the work you do on this module!

I think, I like the workaround described in https://github.com/terraform-aws-modules/terraform-aws-lambda/pull/444#issuecomment-1494592708 very much because all user will have to do is just set a couple of environment variables and we avoid putting extra logic into already big package.py.

If it is documented properly, it can be a decent solution. What do you think?

pdecat commented 1 year ago

@pdecat Thank you very much for the work you do on this module!

Thanks @antonbabenko, I'm glad to contribute to your awesome modules :)

I think, I like the workaround described in #444 (comment) very much because all user will have to do is just set a couple of environment variables and we avoid putting extra logic into already big package.py.

If it is documented properly, it can be a decent solution. What do you think?

I do agree package.py is already relatively complex, but I believe my last proposed change should not be too intrusive or risky (in the sense of breaking existing configurations).

IMO, the work-around using environment variables can be acceptable, but it would be better if their presence could be enforced one way or another. Indeed, one thing that I find less desirable about using environment variables is that they are not taken into account when computing the package hash sum, which is key to detect something as changed. Maybe using pip configuration files instead of environments variables could help get a better work-around as they could probably be included in the hash computation using source_path.

Also, what's your take on restoring hash_extra_paths behavior, maybe also allowing the user to customize it by exposing it as variable with a default value? Should I open a new issue to discuss that matter further?

github-actions[bot] commented 1 year ago

This PR has been automatically marked as stale because it has been open 30 days with no activity. Remove stale label or comment or this PR will be closed in 10 days

github-actions[bot] commented 1 year ago

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.