Default behavior of `tempfile.mkdtemp(suffix=None, prefix=None, dir=None)` changed in Python 3.12

sassdavid commented 1 month ago

Description

We use a module to create Lambda layers built in Docker. Recently, I tried changing the pip_tmp_dir to a path relative to the module (${path.module}) to avoid constant discrepancies in the plan between my local environment and our CI/CD pipeline. I noticed differences between the local run and the CI/CD pipeline run.

On my computer, the Python version is 3.12, but our pipelines use 3.10. I found that there was a change in Python 3.12: https://docs.python.org/3/library/tempfile.html#tempfile.mkdtemp

It states: "Changed in version 3.12: mkdtemp() now always returns an absolute path, even if dir is relative."

This change is beneficial for us as it allows the use of a relative path for pip_tmp_dir, thus avoiding constant changes in the plan due to ${path.cwd} differences.

Could you suggest a way to achieve the same behavior with older Python versions? Additionally, is there any concern about always returning the absolute path in this file: https://github.com/terraform-aws-modules/terraform-aws-lambda/blob/master/package.py#L123?

If your request is for a new feature, please use the Feature request template.

[x] ✋ I have searched the open/closed issues and my issue is not listed.

⚠️ Note

Before you submit an issue, please perform the following first:

Remove the local .terraform directory (! ONLY if state is stored remotely, which hopefully you are following that best practice!): rm -rf .terraform/
Re-initialize the project root to pull down modules: terraform init
Re-attempt your terraform plan or apply and check if the issue still persists

Versions

Module version [Required]: 7.7.0
Terraform version: 1.9.2
Provider version(s):
- provider registry.terraform.io/hashicorp/aws v5.57.0
- provider registry.terraform.io/hashicorp/external v2.3.3
- provider registry.terraform.io/hashicorp/local v2.5.1
- provider registry.terraform.io/hashicorp/null v3.2.2

Reproduction Code [Required]

module "s3fileloader_lambda_function_layer" {
  source  = "terraform-aws-modules/lambda/aws"
  version = "7.7.0"

  create = local.create_s3fileloader

  create_function = false
  create_package  = true
  create_layer    = true

  layer_name  = "${local.lambda_name}-pip-requirements"
  description = "Layer for supporting 's3fileloader' lambda function"

  compatible_runtimes = ["python3.12"]
  runtime             = "python3.12"
  artifacts_dir       = "${path.root}/builds/lambda_layers/"

  build_in_docker = true
  docker_additional_options = [
    "--network", "host"
  ]

  source_path = [
    {
      path             = "${path.module}/s3fileloader"
      pip_tmp_dir      = "${path.module}/s3fileloader"
      prefix_in_zip    = "python"
      pip_requirements = true
    }
  ]

  store_on_s3               = true
  s3_prefix                 = "lambda_layers/"
  s3_object_storage_class   = "STANDARD"
  s3_bucket                 = module.s3fileloader_lambda_function_sources.s3_bucket_id
  s3_server_side_encryption = "aws:kms"
  s3_object_tags            = local.tags

  recreate_missing_package     = false
  trigger_on_package_timestamp = false

  tags = local.tags
}

Steps to reproduce the behavior:

Run the module definition above with python3.10 and python3.12.

Expected behavior

The Lambda layer was created correctly.

Actual behavior

Python 3.10:

The script fails here, presumably due to the relative path: https://github.com/terraform-aws-modules/terraform-aws-lambda/blob/master/package.py#L1108

[2024-07-11T09:21:27.996Z] module.s3fileloader_lambda_function_layer.null_resource.archive[0] (local-exec): FileNotFoundError: [Errno 2] No such file or directory: './s3fileloader/terraform-aws-lambda-4jrqazb4/requirements.txt'

[2024-07-11T09:21:21.436Z] module.s3fileloader_lambda_function_layer.null_resource.archive[0] (local-exec): zip: creating './builds/lambda_layers/50440c451250a62316026ec292815a320d9bbca0100cf05b80febf54fd137adc.zip' archive
[2024-07-11T09:21:21.436Z] module.s3fileloader_lambda_function_layer.null_resource.archive[0] (local-exec): Installing python requirements: ./s3fileloader/requirements.txt
[2024-07-11T09:21:21.436Z] module.s3fileloader_lambda_function_layer.null_resource.archive[0] (local-exec): > mktemp -d terraform-aws-lambda-XXXXXXXX # ./s3fileloader/terraform-aws-lambda-4jrqazb4
[2024-07-11T09:21:21.436Z] module.s3fileloader_lambda_function_layer.null_resource.archive[0] (local-exec): > cd ./s3fileloader/terraform-aws-lambda-4jrqazb4

Python 3.12:

The script does not fail.

module.s3fileloader_lambda_function_layer.null_resource.archive[0] (local-exec): zip: creating './builds/lambda_layers/50440c451250a62316026ec292815a320d9bbca0100cf05b80febf54fd137adc.zip' archive
module.s3fileloader_lambda_function_layer.null_resource.archive[0] (local-exec): Installing python requirements: ./s3fileloader/requirements.txt
module.s3fileloader_lambda_function_layer.null_resource.archive[0] (local-exec): > mktemp -d terraform-aws-lambda-XXXXXXXX # /home/user/src/bitbucket/company/Terraform-AWS/tf-scripts/.terragrunt-cache/J5cPGP4LIGaljTfIVUC_seYg5ek/37ujJTU5vwGX_XeoGQN7WeHvNDI/s3fileloader/terraform-aws-lambda-79x0pniy
module.s3fileloader_lambda_function_layer.null_resource.archive[0] (local-exec): > cd /home/user/src/bitbucket/company/Terraform-AWS/tf-scripts/.terragrunt-cache/J5cPGP4LIGaljTfIVUC_seYg5ek/37ujJTU5vwGX_XeoGQN7WeHvNDI/s3fileloader/terraform-aws-lambda-79x0pniy

Terminal Output Screenshot(s)

Additional context

pdecat commented 1 month ago

Hi,

interesting finding about tempfile.mkdtemp() behavior change in python 3.12!

A few findings below:

This change is beneficial for us as it allows the use of a relative path for pip_tmp_dir, thus avoiding constant changes in the plan due to ${path.cwd} differences.

I do not believe pip_tmp_dir is taken into account when computing the hash of the lambda content, is it?

However, one should probably avoid to point pip_tmp_dir to the same location as the source path like in your reproduction case, as it will probably include temporary artifacts in the output, and thus alter the hash value.

Also, if pip_tmp_dir is not set at all, I believe the path is normally already absolute with all python versions, e.g. /tmp/terraform-aws-lambda-xxxxx on Linux:

If dir is not None, the file will be created in that directory; otherwise, a default directory is used. The default directory is chosen from a platform-dependent list, but the user of the application can control the directory location by setting the TMPDIR, TEMP or TMP environment variables. There is thus no guarantee that the generated filename will have any nice properties, such as not requiring quoting when passed to external commands via os.popen().

# python3.10
Python 3.10.14 (main, Mar 22 2024, 15:42:52) [GCC 13.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import tempfile
>>> tempfile.mkdtemp()
'/tmp/tmpscec8nji'

# python3.11
Python 3.11.9 (main, Apr 15 2024, 18:24:23) [Clang 17.0.6 ] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import tempfile
>>> tempfile.mkdtemp()
'/tmp/tmprp7_k1h3'

# python3.12
Python 3.12.4 (main, Jun 17 2024, 16:05:13) [GCC 13.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import tempfile
>>> tempfile.mkdtemp()
'/tmp/tmpnsxqju_s'

In the end, we should probably recommend that pip_tmp_dir must be an absolute path, and outside the source path.

WDYT?

sassdavid commented 1 month ago

Hi,

thanks for the suggestions.

I do not believe pip_tmp_dir is taken into account when computing the hash of the lambda content, is it?

It may be true in some aspects, but in the case of creating a Lambda layer, it triggers recreation due to changes in the archive_plan. As a result, only the layer is recreated, including its replacement for the Lambda.

However, one should probably avoid to point pip_tmp_dir to the same location as the source path like in your reproduction case, as it will probably include temporary artifacts in the output, and thus alter the hash value.

Thank you for this suggestion. We will make modifications accordingly.

Also, if pip_tmp_dir is not set at all, I believe the path is normally already absolute with all python versions, e.g. /tmp/terraform-aws-lambda-xxxxx on Linux:

Yes, I'm aware that when the pip_tmp_dir is not defined, it creates a temporary folder in /tmp/, resulting in an absolute path, which is also correct. However, we had other issues with it.

So, we will most likely run another round without defining the pip_tmp_dir and proceed with the default path: /tmp/terraform-aws-lambda-xxxxx.

Based on your suggestion that the pip_tmp_dir should be absolute, do you think we can modify this to return os.path.abspath(path)?

sassdavid commented 1 month ago

Hi @pdecat,

In the meantime, we managed to move forward without using pip_tmp_dir and instead used /tmp/ as a temporary folder, which resolved our issue.

If you don't mind or if you agree, I would like to open a PR for this change:

Based on your suggestion that the pip_tmp_dir should be absolute, do you think we can modify this to return os.path.abspath(path)?

Please let me know your opinion; otherwise, I will close this issue.

pdecat commented 1 month ago

Hi @sassdavid, sure, please open a PR!

antonbabenko commented 1 month ago

This issue has been resolved in version 7.7.1 :tada:

github-actions[bot] commented 1 week ago

I'm going to lock this issue 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 similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

terraform-aws-modules / terraform-aws-lambda