Skip to content
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

Using ${path.module} in aws_lambda_function filename results in incorrect "difference" between user machines #7613

Closed
joshma opened this issue Jul 12, 2016 · 13 comments

Comments

@joshma
Copy link

joshma commented Jul 12, 2016

I'm writing this as a bug report, but it's possible I'm just not using path.module correctly (I followed the recommendation https://www.terraform.io/docs/modules/create.html). Let me know if there's an easy workaround!

Terraform Version

Terraform v0.6.16

Affected Resource(s)

  • aws_lambda_function

Terraform Configuration Files

This is logging/main.tf, which is used as a module by parent:

resource "aws_lambda_function" "logs-lambdaCloudWatchToElasticSearch" {
  filename = "${path.module}/lambda.zip"

  function_name = "logs-lambdaCloudWatchToElasticSearch"
  role = "${aws_iam_role.cwToEsRole.arn}"
  handler = "lambda.handler"

  source_code_hash = "${base64sha256(file("${path.module}/lambda.zip"))}"
}

Expected Behavior

After a terraform apply is run by user A, new tfstate is committed to repo. User B pulls and runs terraform plan. It is expected that there are no changes.

Actual Behavior

Because filename = "${path.module}/lambda.zip", the filename stored in state looks something like "filename": "/Users/josh/path/to/terraform/.terraform/modules/79cd4d17db98b26baebfcf1024aade92/lambda.zip". As a result, when damon runs terraform plan, he gets:

~ module.logging.elasticsearch.aws_lambda_function.logs-lambdaCloudWatchToElasticSearch
    filename: "/Users/josh/path/to/terraform/.terraform/modules/79cd4d17db98b26baebfcf1024aade92/lambda.zip" => "/Users/damon/path/to/terraform/.terraform/modules/79cd4d17db98b26baebfcf1024aade92/lambda.zip"

Steps to Reproduce

Please list the steps required to reproduce the issue, for example:

  1. Create a lambda function with filename containing path.module
  2. Move/copy terraform file and state to a new folder, run terraform plan, and verify that terraform thinks the lambda function has changed.
@leg100
Copy link

leg100 commented Jul 25, 2016

+1 I'm seeing this too.

@apparentlymart
Copy link
Contributor

Ahh yes, we've seen this sort of issue before with paths in config. It was one of the reasons why we moved to primarily using the file() function to interpolate file contents rather than referring to filenames directly, but that doesn't make sense here since we're talking about a binary file and Terraform's internals aren't designed to represent raw binary content.

A different workaround could be to write a StateFunc that tries to normalize the given path to be relative to the current working directory. That's still not 100% right since it's not guaranteed that the current working directory will be the configuration root when Terraform is run, but it's at least more likely to be correct than the current approach.

A different way to do it would be to make the path.module variable itself be normalized as a path relative to the current working directory, which is potentially more invasive but would address any other situations where filenames are directly provided in config.

A further way we addressed this in some cases in the past was to make the StateFunc try to read the file and return a hash of it, so the diffing works in terms of contents rather than filename... but that feels weird to do here when we already have the separate source_code_hash attribute that effectively does the same thing. Really what we want here is to simply disregard the path altogether during diffing, since the exact path is not meaningful for recognizing differences... perhaps there's room here for a core change to allow attributes to be marked as "non-diffable", though we'd need to think hard about how to do that right so it doesn't run into similar sorts of issues as the ignore_changes feature did.

@geoffreyprice
Copy link

I'm running into this as well. Using the path.module saves the variable in the tfstate file. Is there a command that will not save a path in the tfstate but instead always interpolate the path at runtime?

@apparentlymart
Copy link
Contributor

A possible workaround in the short term would be to use the ignore_changes feature to explicitly ask Terraform not to diff this argument, by adding the following to the aws_lambda_function resource block:

  ignore_changes = ["filename"]

I didn't test this, but I believe it should cause Terraform to not generate a diff when only the filename has changed.

@joshuaspence
Copy link
Contributor

I had a similar issue with the aws_lambda_function resource except that I was using ${path.cwd} instead of ${path.module}. I noticed that removing ${path.cwd} seemed to work (as in, the file was still correctly uploaded as a Lambda function), and also ensured that the path in the tfstate file was relative rather than absolute. Is this expected or am I relying on an implementation detail (that relative paths seem to be assumed to be relative to the current working directory) that may break in the future?

@apparentlymart
Copy link
Contributor

We now have a feature in helper/schema to allow attributes to customize their diffing behavior, so we might be in a better position to do what I described earlier around ignoring this attribute for diffing purposes, or changing it to normalize the path in some way. What do you think, @jen20? ( I am thinking of the thing you did to allow us to normalize IAM policies when diffing.)

@mitchellh
Copy link
Contributor

@apparentlymart I actually think the diffing should be based on the hash of the contents of the file, unfortunately. Normalizing the path would likely be fairly tedious/difficult. The particularly annoying part about this function is that it is sometimes used to upload fairly large (megabytes) files and if you have a lot, then reading and hashing all of them on a terraform plan is kind of terrible...

Hm.

@mitchellh
Copy link
Contributor

Dup of #8204, we'll use that there.

@etaty
Copy link

etaty commented Sep 26, 2017

I found a workaround to this using the current working directory path.cwd
If you remove the current working directory from the state file, then the state doesn't change!

data "archive_file" "slack_notification_zip" {
  type = "zip"
  output_path = "${path.module}/lambda.zip"

  source_dir = "${path.module}/source/"
}

resource "aws_lambda_function" "slack_notification" {
  filename = "${substr(data.archive_file.slack_notification_zip.output_path, length(path.cwd) + 1, -1)}"
  // +1 for removing the "/"
}

@ibrahima
Copy link

ibrahima commented Apr 25, 2018

For reference, ignore_changes does not work to solve this problem because it then tries to load the file from the path on the other user's machine, which then fails.

You can also solve this problem by doing something like

data "null_data_source" "path-to-some-file" {
  inputs {
    filename = "${substr("${path.module}/path/to/file", length(path.cwd) + 1, -1)}"
  }
}

resource "aws_lambda_function" "slack_notification" {
  filename = "${data.null_data_source.path-to-some-file.outputs.filename}"
}

(useful in the general case when it's not necessarily an archive_file)

@sprutner
Copy link
Contributor

Just ran into this issue using shared state on Terraform Community Module https://github.com/terraform-aws-modules/terraform-aws-notify-slack/

@davehowell
Copy link

@etaty I assume in your workaround that the state does change for the data.archive_file.slack_notification_zip.output_path and not for the aws_lambda_function.slack_notification.filename

In the docs on paths and embedded files, it says

you can't use a relative path, since paths in Terraform are generally relative to the working directory from which Terraform was executed. Instead, you want to use a module-relative path

In the OP's question the example is
/Users/josh/path/to/terraform/.terraform/modules/79cd4d17db98b26baebfcf1024aade92/lambda.zip
And the change from /Users/josh... to '/Users/damon...` in the statefile diff triggers the new resource.

The way I read it, terraform apply would be executed from the /Users/josh/path/to/terraform/ directory, so the substring trick would be removing exactly that from the filename attribute, which leaves the path as .terraform/modules/79cd4d17db98b26baebfcf1024aade92/lambda.zip which looks like a relative path, ...

...I think I've just answered my question which was why does this work, or more to the point, how is this workaround any different from just using a relative path? Seems it is because Terraform is caching modules into these UUID directories which {path.module} is able to re-constitute. I guess it also helps with referencing remote modules which have no local path apart from where they are cached, and also with re-organising module directories so embedded file paths don't need to be changed.

umbrant added a commit to umbrant/aws-lambda-es-cleanup that referenced this issue Apr 29, 2019
The terraform filename parameter encodes an absolute path to the module,
which can be different when run on different machines. This changes it
to key on a relative path, per the recommendation here:

hashicorp/terraform#7613 (comment)
giuliocalzolari pushed a commit to cloudreach/aws-lambda-es-cleanup that referenced this issue May 21, 2019
* Use a relative path to prevent spurious terraform changes.

The terraform filename parameter encodes an absolute path to the module,
which can be different when run on different machines. This changes it
to key on a relative path, per the recommendation here:

hashicorp/terraform#7613 (comment)

* Add debug logging
@ghost
Copy link

ghost commented Mar 30, 2020

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.

@ghost ghost locked and limited conversation to collaborators Mar 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests