Availability to disable timestamp for lambda package

jBouyoud commented 1 year ago

Is your request related to a new offering from AWS?

Is this functionality available in the AWS provider for Terraform? See CHANGELOG.md, too.

No 🛑: please wait to file a request until the functionality is avaialble in the AWS provider
Yes ✅: please list the AWS provider version which introduced this functionality

Is your request related to a problem? Please describe.

When using this module in a CI environment that aims to detect drift. If the package is freshly checkout the timestamp is always drifting even the checksum doesn't move.

Describe the solution you'd like.

Give a way to disable timestamp checks (probably with a terraform variable)

Describe alternatives you've considered.

Manually touch with a specific timestamp all file that are targeted by the module instance

Additional context

Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
-/+ destroy and then create replacement

Terraform will perform the following actions:

  # module.lambda-export-elasticache-exports.null_resource.archive[0] must be replaced
-/+ resource "null_resource" "archive" {
      ~ id       = "4346580655420454952" -> (known after apply)
      ~ triggers = { # forces replacement
          ~ "timestamp" = "1673343927935659800" -> "1673447867144085800"
            # (1 unchanged element hidden)
        }
    }

Plan: 1 to add, 0 to change, 1 to destroy.

evya123 commented 1 year ago

Would be super helpful since currently every run detects a change :\

andrewhharmon commented 1 year ago

plus one for this, I thought maybe i was doing something wrong. Is there any workaround at the moment?

andrewhharmon commented 1 year ago

curious if this is caught anyone's attention. Does it really need to use the timestamp if it has the file hash?

jussapaavo commented 1 year ago

This does seem to be related to the issues #51, #63 and #323. We're also having this issue, but only when using Terrafrom with multiple envs (users and CI). When deploying from the same env we don't receive these changes with terraform plan.

We tried the suggested fix by using TF_RECREATE_MISSING_LAMBDA_PACKAGE=false env var, but it didn't work for us and instead these Terraform changes persisted when running from multiple envs.

github-actions[bot] commented 1 year ago

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

msrdjan commented 1 year ago

Setting recreate_missing_package module variable to false worked for me.

Module lambda.null_resource.archive[0] was updated once more, due to a change in the timestamp trigger ("1679200526172839000" change to "<WARNING: Missing lambda zip artifacts wouldn't be restored>"), but not in the following runs.

nwalters512 commented 1 year ago

Does anyone understand the implications of the "Missing lambda zip artifacts wouldn't be restored" warning?

github-actions[bot] commented 1 year ago

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

spectorar commented 1 year ago

Bumping this issue. Just spent hours debugging a scenario. A terraform plan/apply partially failed in CI and subsequent builds continued to fail because the package wasn't being regenerated. We have to have recreate_missing_package set to false for CI purposes, however in the event that null_resource.archive hasn't changed while aws_lambda_function.this has (admittedly an edge case), nothing triggers the package to be generated. An option to ignore the timestamp for package comparison would allow recreate_missing_package to be set to true while not causing terraform to detect a change when their hasn't been one to the actual source code.

@nwalters512 This scenario my team managed to get ourselves into seems to be one such implication.

github-actions[bot] commented 1 year ago

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

mickutz commented 1 year ago

yeah, can confirm this issue is a bit of a PITA.

hsdp-smulford commented 1 year ago

+1, booo!

marcofranssen commented 1 year ago

👍 would love to see this implemented! That will save lots of provisioning time when there are actually no changes.

mkielar commented 1 year ago

Wouldn't it be possible to refactor that logic and use some kind of hash (sha512, for example) instead of the timestamp? I mean, let the thing re-generate the ZIP, but if it's the same, don't trigger anything?

p0fi commented 1 year ago

We are having the same problem. Every CI run detects a change in lambda even the code was never touched. Please let's find a solution for this!

mkielar commented 1 year ago

I played with the module a bit, and there are following workarounds for this issue, that you can do as a module user:

Set recreate_missing_package = false. This way, if the source file did not change, terraform will not attempt to re-create the ZIP File, and will not re-deploy the Function. If the sources change, the ZIP will still be created, and the Function redeployed. The downside of this approach is, that if anyone changes the source of the Function using AWS Console, Terraform will likely not detect that, and not re-deploy the Function.
Implement your own ZIPping functionality in a CICD Pipeline, and upload the file to S3. Then point the module to pre-packaged ZIP file in S3. This is of course way more work compared to simply pointing the module to a local script, but it has the additional advantage of versioning your "executable packages"
The same as pt. 2, but use Docker.

All of them, however, render the functionality of simply pointing to a script file, either useless or broken and some of them (docker) require quite significant amount of extra effort and infrastructure provisioning. That somehow contradicts the idea of using this module to simplify lambda provisioning.

I still think, the best way out of this would be to refactor the module to use some kind of hash on the ZIP file (instead of it's creation timestamp). @antonbabenko, WDYT?

p0fi commented 1 year ago

Are there more downsides to setting recreate_missing_package = false to false?

mkielar commented 1 year ago

I did not spot any. But to be fair, I set it to false, left a comment in code explaining why, so that somebody, someday wouldn't think it's weird and "fix" it ;-), and moved on, never thinking about it again. We do, however have a quite strict policy of not allowing people to mess around manually via AWS Console, so the setup is relatively safe for us. YMMV.

qpleple commented 1 year ago

I have the same issue. If different users alternatively do terraform apply, it will reconstruct constantly the package, even with the setting recreate_missing_package = false.

orolega commented 12 months ago

I was getting the Error: Provider produced inconsistent final plan error when recreate_missing_package = false. removing that and using the default true value resolved the error but now the timestamp always results in the plan stating there is a change

OlesYudin commented 11 months ago

I have a similar problem, I am using the terraform-aws-modules/lambda/aws module in my module which is used in several sub-environments. This means that the same workspace contains the same modules, only with different names and slightly different configurations, but the source code for AWS Lambda is the same. I ran into two problems:

If you update the source code, in my case it will have to update several modules in one workspace, which leads to an error, which is described in this issue:
476
If you set recreate_missing_package = true (default value) - AWS Lambda module will be updated every time

github-actions[bot] commented 10 months ago

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

p0fi commented 10 months ago

This issue still exists and the workaround with recreating package does not work in CI environments when there was a deploy problem. During a subsequent run terraform will say it does not need to create the zip files as the archive resource seem to exist. But in reality they don't. Could we just get the module to use a hash instead of the timestamp as suggested? This would solve so many headaches!

brokenthumbs commented 9 months ago

Does this pull request require additional work in order to be considered for merging? From what I can tell, this pull request would change the logic to use hash instead of timestamp, when comparing zip file archives.

https://github.com/terraform-aws-modules/terraform-aws-lambda/pull/331

github-actions[bot] commented 8 months ago

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

brokenthumbs commented 8 months ago

Not stale. Is there any contributor who might know more reasoning to why the proposed change would be acceptable or unacceptable?

antonbabenko commented 8 months ago

@brokenthumbs Lack of time - the only reason for most delays.

brokenthumbs commented 8 months ago

@antonbabenko That's fair 🙏

Keeping this issue open for when you / another contributor potentially has a chance to review.

I have tested https://github.com/terraform-aws-modules/terraform-aws-lambda/pull/331 and the code seems to work okay, but being new to using this module, I might also be missing edge cases or unforeseen side effects.

brokenthumbs commented 8 months ago

For those reading, more context here: https://github.com/terraform-aws-modules/terraform-aws-lambda/issues/323#issuecomment-1234144646

antonbabenko commented 8 months ago

Can someone please provide a complete set of instructions (including 100% failing code)? I have an extremely hard time realizing WHAT IS WRONG with this and without being able to reproduce it locally I can't do much.

brokenthumbs commented 8 months ago

I'm not sure what https://github.com/terraform-aws-modules/terraform-aws-lambda/pull/509 is attempting to solve.
I do know what https://github.com/terraform-aws-modules/terraform-aws-lambda/pull/331 is attempting to solve.

For clarification, there is no failing code in the repository. What is being suggested is an alternative to a specific behavior of Terraform triggers in specific scenarios.

Here's my attempt at explaining why timestamp as a Terraform trigger can potentially cause problems.

Scenario:

I create my initial instantiation of the terraform-aws-lambda module.
I execute terraform plan and terraform apply.
I reset my local environment by deleting the .terraform directory.
- This simulates a Continuous Integration environment, where subsequent executions of Terraform do not guarantee there to be any cached .terraform directory.
- In many cases, a Continuous Integration environment will always start with a fresh Git clone of a repository.
Without making any changes to my terraform-aws-lambda resource, I execute terraform plan.
- What I expect to happen:
- No changes required and terraform plan indicates no differences.
- What currently happens:
- terraform plan indicates null_resource.archive[0] must be replaced.
- This is because the triggers include timestamp (https://github.com/terraform-aws-modules/terraform-aws-lambda/blob/master/package.tf#L62-L65).
- This forces terraform apply to re-deploy the Lambda function, even if there have been no changes to the terraform-aws-lambda resource or function code.

Local Reproduction:

Instantiate the terraform-aws-lambda module, using terraform init.
Execute terraform plan and terraform apply.
Delete the .terraform directory.
Recreate the .terraform directory using terraform init.
Make no changes to the terraform-aws-lambda module resource or to the function code.
Execute terraform plan.

Expected Result:

terraform plan indicates that there are no changes required.

Current Result:

terraform plan indicates null_resource.archive[0] must be replaced.

Recommended Upgrade:

The timestamp trigger will always be different, when recreating the build.zip.
- This is only a problem when dealing with newly cloned repositories, where the .terraform directory is missing and must be recreated using terraform init.
- Because the timestamp will always be different, then every terraform plan in a newly cloned repository will force a deployment of the terraform-aws-lambda module, regardless of whether any changes have been made to the module usage or function source code.
By replacing the timestamp trigger with hash, we can execute terraform plan in a newly cloned repository and not trigger any deployments of the terraform-aws-lambda module, as expected when not making any changes to the module usage or function source code.

Per an earlier comment, one potential fix is as such. https://github.com/terraform-aws-modules/terraform-aws-lambda/issues/396#issuecomment-1609078149

This would effectively turn this:

  triggers = {
    filename  = data.external.archive_prepare[0].result.filename
    timestamp = data.external.archive_prepare[0].result.timestamp
  }

into this:

  triggers = {
    filename  = data.external.archive_prepare[0].result.filename
    hash      = md5(data.external.archive_prepare[0].result.build_plan)
  }

There isn't any failing code, because the code itself is working. The issue described is with a unique process, involving freshly cloned repositories (for example, when executing Terraform in a Continuous Integration environment).

Let me know if there's something else I can do or test to help merge https://github.com/terraform-aws-modules/terraform-aws-lambda/pull/331

🙏

EDIT: Tagging @mkielar to review my explanation and proposed fix. ^^^

github-actions[bot] commented 7 months ago

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

brokenthumbs commented 7 months ago

Not stale, looking for review on https://github.com/terraform-aws-modules/terraform-aws-lambda/issues/396#issuecomment-1792787797 from @antonbabenko

re:

Can someone please provide a complete set of instructions (including 100% failing code)? I have an extremely hard time realizing WHAT IS WRONG with this and without being able to reproduce it locally I can't do much.

github-actions[bot] commented 6 months ago

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

nwalters512 commented 6 months ago

I don't believe this should be closed yet.

brokenthumbs commented 6 months ago

I believe my explanation of the situation is still relevant: https://github.com/terraform-aws-modules/terraform-aws-lambda/issues/396#issuecomment-1792787797

antonbabenko commented 5 months ago

This issue has been resolved in version 6.7.0 :tada:

antonbabenko commented 5 months ago

I think it is fixed now by @samuel-phan in #521. Please let us know if there is still an issue.

brokenthumbs commented 5 months ago

Does filename change if the hash changes?

I'm thinking about if I were to avoid updating on timestamp, would the filename of the package change trigger updates instead?

EDIT:

Filename does change on update! Disabling timestamp should work great 👍 I'll give this a try in my production environment this week.

  # module.module.module.lambda_function.null_resource.archive[0] must be replaced
-/+ resource "null_resource" "archive" {
      ~ id       = "6348977012088056442" -> (known after apply)
      ~ triggers = { # forces replacement
          ~ "filename" = "builds/d2b8d4bd26f21d3c56d6b53988959701aa861149f4286853c3e3f57ef7ebea35.zip" -> "builds/2da1928cc4a1c4909daf48a87c1381093c8ffd8fc7219f63b435fa239cd5f989.zip"
        }
    }

EDIT:

Thanks all for helping solve the issue!

github-actions[bot] commented 4 months 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