hashicorp/terraform

module output can not be interpolated in local-exec command

matti opened this issue · 12 comments

matti commented

Terraform Version

Terraform v0.11.3
+ provider.local v1.1.0
+ provider.null v1.0.0

Terraform Configuration Files

module "ls" {
  source = "matti/resource/shell"
  command = "uptime"
}

resource "null_resource" "wat" {
  provisioner "local-exec" {
    when = "destroy"

    command = "echo ${local.thisdoesnt}"
  }
}

locals {
  thisdoesnt = "${module.ls.stdout}"
  thisworks = "wet"
}

output "ls" {
  value = "${module.ls.stdout}"
}

Debug Output

module.ls.null_resource.shell: Provisioning with 'local-exec'...
module.ls.null_resource.shell (local-exec): Executing: ["/bin/sh" "-c" "rm /Users/mpa/dev/tf-test/.terraform/modules/8f515d54eea339107c30d6a35253921e/matti-terraform-shell-resource-14e7ecb/stdout.9210179467979770770"]
2018/02/13 19:05:45 [TRACE] root: eval: *terraform.EvalIf
2018/02/13 19:05:45 [TRACE] root: eval: *terraform.EvalApplyPost
2018/02/13 19:05:45 [ERROR] root: eval: *terraform.EvalApplyPost, err: 1 error(s) occurred:

* local-exec provisioner command must be a non-empty string
2018/02/13 19:05:45 [ERROR] root: eval: *terraform.EvalIf, err: 1 error(s) occurred:

* local-exec provisioner command must be a non-empty string
2018/02/13 19:05:45 [ERROR] root: eval: *terraform.EvalSequence, err: 1 error(s) occurred:

* local-exec provisioner command must be a non-empty string
2018/02/13 19:05:45 [ERROR] root: eval: *terraform.EvalOpFilter, err: 1 error(s) occurred:

* local-exec provisioner command must be a non-empty string
2018/02/13 19:05:45 [TRACE] [walkDestroy] Exiting eval tree: null_resource.wat (destroy)
2018/02/13 19:05:45 [TRACE] dag/walk: upstream errored, not walking "local.thisdoesnt"
2018/02/13 19:05:45 [TRACE] dag/walk: upstream errored, not walking "module.ls.output.stdout"
2018/02/13 19:05:45 [TRACE] dag/walk: upstream errored, not walking "output.ls"

Expected Bahviour

String would be interpolated.

Actual Behaviour

* local-exec provisioner command must be a non-empty string

Steps to Reproduce

  1. the file contents above to main.tf
  2. terraform init
  3. terraform apply

Also just directly interpolating module output won't work. Variable interpolation works.

matti commented

I have a feeling that this is some kind of a security feature?

Hey @matti can you give me more information about how the module matti/resource/shell works, specifically around how the output stdout is defined?

Perhaps I'm mistaken, but I believed locals functionality to require being defined in a module?

matti commented

@catsby matti/resource/shell is a public terraform module at https://registry.terraform.io/modules/matti/resource/shell/0.1.0

I did some additional testing and a clear repro repository (without that module) here: https://github.com/matti/terraform-repro-issue-17337

@jbardin this reminds me of the issue you were looking at recently where locals and outputs were not being handled properly in some situations, which led to you reworking how we build the graph nodes for these. Does it look like something similar to you, or am I off-base here?

matti commented

@apparentlymart just narrowed down the repro: this bug only happens on destroy and this depends_on triggers it https://github.com/matti/terraform-repro-issue-17337/blob/master/anymodule/main.tf#L18

the repro doesn't have locals at all

@apparentlymart,

Yes, it appears there are still some more edge cases in destroy.
I have a similar case already, and I'll be taking a closer look asap.

@matti Looks like James or Martin will be picking this up, thank you so much for the above and beyond digging and information providing 😄

@matti,

Can I assume here that you're encountering the errors during destroy, though your example only mentions apply? (The reference is in a destroy provisioner, and I can't replicate this on apply). If that's the case, then the good news is it is already fixed in master! The less good news is that this hits the other destroy-time edge case I'm working on right now and depends_on isn't part of the issue.

Also, while it should not return an error, adding a "depends_on" to a data source isn't very useful right now, since it's always going to show a diff in the plan.

matti commented

Yes, only on destroy.

The root cause here turns out to be the combined behavior of a destroy provisioner and a data source using depends_on. Because of the depends_on field, the data source can't be read during refresh and therefor is removed from the state and has no diff. This prevents it from being added to the destroy graph, the only time the destroy provisioner is evaluated.

#17034 Should take care of the bulk of this issue for us. There may be some additional details to consider, but we need to get the datasource lifecycle fixed up first.

Hi all! Sorry for the long silence here, and thanks for sharing the reproduction cases.

During the 0.12 cycle we (the Terraform team at HashiCorp) looked into the design and implementation of destroy-time provisioners and found that there were some irreconcilable flaws in the way they were conceived, and this issue is a symptom of those flaws.

In order to retain as much of the functionality of destroy-time provisioners as possible while addressing the design flaws, one of the 0.12 minor releases began a deprecation cycle for referring to objects other than self, count.index and each.key inside a destroy-time provisioner block or its associated connection block. The forthcoming Terraform 0.13 release will conclude that deprecation cycle by promoting the earlier warnings to configuration validation errors.

As a consequence of that, the example given in the original framing of this bug will no longer be considered valid in Terraform 0.13, because the destroy-time provisioner refers to local.thisdoesnt. To get a similar result it will be required to copy the desired value into the resource configuration so that during the destroy phase it can be obtained via self, like this:

resource "null_resource" "wat" {
  triggers = {
    message = local.thisdoesnt
  }

  provisioner "local-exec" {
    when = "destroy"

    command = "echo ${self.triggers.message}"
  }
}

For null_resource we can use triggers as a convenient place to cache the values to use during destroy. For other resource types some more creative solutions will likely be required, because most resource types don't have an attribute like this triggers where the given values are meaningless to the resource type.

The 0.13 upgrade guide is not published on the main website yet because the release is still in beta as I write this, but we have a draft of the upgrade guide in a temporary location which includes a section Destroy-time provisioners may not refer to other resources that includes some additional details. (Similar content will appear in the main 0.13 upgrade guide after the 0.13.0 final release.)

Since this broader change has now intentionally made the original reproduction case invalid, I'm going to close this issue. While these new restrictions are inconvenient, they are necessary to fix fundamental problems with how destroy-time provisioners were designed, and so various other issues around ordering of operations during destroy have either already been resolved for 0.13.0 or will be addressed by ongoing improvements in future releases.

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.