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

module output can not be interpolated in local-exec command #17337

Closed
matti opened this issue Feb 13, 2018 · 12 comments
Closed

module output can not be interpolated in local-exec command #17337

matti opened this issue Feb 13, 2018 · 12 comments
Labels
bug config v0.11 Issues (primarily bugs) reported against v0.11 releases v0.12 Issues (primarily bugs) reported against v0.12 releases

Comments

@matti
Copy link

matti commented Feb 13, 2018

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
Copy link
Author

matti commented Feb 13, 2018

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

@catsby
Copy link
Contributor

catsby commented Feb 13, 2018

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?

@catsby catsby added waiting-response An issue/pull request is waiting for a response from the community modules bug labels Feb 13, 2018
@matti
Copy link
Author

matti commented Feb 14, 2018

@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

@apparentlymart
Copy link
Contributor

@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
Copy link
Author

matti commented Feb 15, 2018

@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

@jbardin
Copy link
Member

jbardin commented Feb 15, 2018

@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.

@catsby
Copy link
Contributor

catsby commented Feb 15, 2018

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

@jbardin jbardin removed the waiting-response An issue/pull request is waiting for a response from the community label Feb 15, 2018
@jbardin
Copy link
Member

jbardin commented Feb 16, 2018

@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
Copy link
Author

matti commented Feb 17, 2018

Yes, only on destroy.

@jbardin
Copy link
Member

jbardin commented Feb 19, 2018

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.

@hashibot hashibot added config and removed modules labels Aug 15, 2019
@hashibot hashibot added v0.11 Issues (primarily bugs) reported against v0.11 releases v0.12 Issues (primarily bugs) reported against v0.12 releases labels Aug 29, 2019
@apparentlymart
Copy link
Contributor

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.

@ghost
Copy link

ghost commented Jul 31, 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 Jul 31, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug config v0.11 Issues (primarily bugs) reported against v0.11 releases v0.12 Issues (primarily bugs) reported against v0.12 releases
Projects
None yet
Development

No branches or pull requests

5 participants