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

Failing to evaluate module outputs during destroy #33659

Closed
kbcz1989 opened this issue Aug 10, 2023 · 4 comments · Fixed by #33758
Closed

Failing to evaluate module outputs during destroy #33659

kbcz1989 opened this issue Aug 10, 2023 · 4 comments · Fixed by #33758
Assignees
Labels
bug confirmed a Terraform Core team member has reproduced this issue

Comments

@kbcz1989
Copy link

Terraform Version

Terraform v1.5.3
on linux_amd64

Terraform Configuration Files

main.tf:

terraform {
  required_providers {
    netbox = {
      source  = "e-breuninger/netbox"
      version = "~> 3.4.0"
    }
  }
}

module "netbox_provider_secret" {
  source = "./secret"
}

provider "netbox" {
  server_url           = <netbox_url>
  api_token            = module.netbox_provider_secret.secret
  allow_insecure_https = true
}

provider "aws" {}

resource "netbox_ip_address" "this" {
  ip_address = "10.0.0.50/24"
  status     = "reserved"
}

secret/main.tf

variable "create_secret" {
  type    = bool
  default = false
}

data "aws_ssm_parameter" "this" {
  count = var.create_secret ? 0 : 1
  name  = "/tf-netbox/api_key"
}

resource "aws_ssm_parameter" "this" {
  count          = var.create_secret ? 1 : 0
  name           = "/tf-netbox/api_key"
  type           = "SecureString"
  insecure_value = "bla"
}

# DOES NOT WORK!
output "secret" {
  value = try(coalesce(
    var.create_secret ? null : data.aws_ssm_parameter.this[0].value,
    var.create_secret ? aws_ssm_parameter.this[0].value : null
  ), null)
}

# WORKS!
#output "secret" {
#  value = coalesce(
#    var.create_secret ? null : data.aws_ssm_parameter.this[0].value,
#    var.create_secret ? aws_ssm_parameter.this[0].value : null
#  )
#}

Debug Output

gist

Expected Behavior

Destroy should work.

Actual Behavior

Planning failed. Terraform encountered an error while generating this plan.

╷
│ Error: missing netbox API key
│ 
│   with provider["registry.terraform.io/e-breuninger/netbox"],
│   on main.tf line 14, in provider "netbox":
│   14: provider "netbox" {
│ 
╵

Steps to Reproduce

  1. terraform init
  2. terraform apply
  3. terraform destroy

Additional Context

This is probably related to this change.

While this can be fixed with refactoring, it would be really interesting to see what caused it to fail. We are noticing random similar issues with "try/coalesce" functions.
This was working fine up until v1.5.3

References

#33462

@kbcz1989 kbcz1989 added bug new new issue not yet triaged labels Aug 10, 2023
@jbardin jbardin added confirmed a Terraform Core team member has reproduced this issue and removed new new issue not yet triaged labels Aug 10, 2023
@jbardin
Copy link
Member

jbardin commented Aug 10, 2023

Hi @kbcz1989,

Thanks for filing the issue! This is actually the intersection of multiple bugs and behaviors, all combining to make for a very confusing issue.

  • This configuration will still fail prior to v1.5.3 if you don't allow refresh during the destroy plan, which is a situation also fixed by always evaluate module outputs during destroy #33462, but cannot be remedied by removing try. The fact that it worked at all is sort of a bug itself due to destroy requiring two separate plans which are dependent on a shared state.

  • Another pending patch gives us a slightly more precise value for the evaluation of aws_ssm_parameter.this during the plan because we know there are exactly 0 instances which allows try to give a known value, but that will still fail if you remove the count from the resource.

  • As you can see, removing try helps the most here. This is due to the unique requirements for thetry and can functions, which need to be able to handle evaluation errors (you can see more about this in Issues with combining try and for_each #31868 and can function does not work properly with nonsensitive function #31646). The fact that aws_ssm_parameter.this[0].value can't produce a known value during the plan forces try to return an unknown value, which is propagated to the netbox provider configuration.

  • Most providers won't notice this, because they don't need a complete configuration for the planning process. I don't think the netbox provider needs api_token to plan this resource either, but I'm not sure about the rest of it's resources.

I'm going to leave this open to see if I can find any other confounding factors, but I think most of the unintended behavior here is due to try, which should only be used as necessary, and not as a safety wrapper around all evaluations.

@jbardin jbardin self-assigned this Aug 11, 2023
@kbcz1989
Copy link
Author

Great information @jbardin, thank you!

We are noticing this issue with many providers that require configuration similar to Netbox. I will list some of them here for future reference:

  • aci
  • aviatrix
  • azapi
  • grafana
  • guacamole
  • mongodbatlas
  • mysql
  • opsgenie
  • panos
  • postgresql
  • proxmox
  • restapi
  • vsphere

I will read up more on the details you provided as I am not sure I understand the exact issue. It would be very beneficial to have a function similar to coalesce() which would allow returning null as the last resort.

@jbardin
Copy link
Member

jbardin commented Aug 15, 2023

Sorry, I think I buried the lede there a bit. The try function is what is really at fault -- the destroy operation, module output, coalesce, provider config, etc are all just adding to the confusion. I have a new idea to test with the try and can functions, but in the meantime breaking out the list of values to be coalesced into a local value may work for you:

locals {
  vals = [
    var.create_secret ? null : data.aws_ssm_parameter.this[0].value,
    var.create_secret ? aws_ssm_parameter.this[0].value : null,
  ]
}
output "secret" {
  value = try(coalesce(local.vals...), null)
}

Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug confirmed a Terraform Core team member has reproduced this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants