hashicorp / terraform

Terraform enables you to safely and predictably create, change, and improve infrastructure. It is a source-available tool that codifies APIs into declarative configuration files that can be shared amongst team members, treated as code, edited, reviewed, and versioned.
https://www.terraform.io/
Other
42.31k stars 9.49k forks source link

Can't use sensitive value in dynamic for_each #29744

Open camlow325 opened 2 years ago

camlow325 commented 2 years ago

Terraform Version

› terraform version
Terraform v1.0.8
on darwin_amd64
+ provider registry.terraform.io/datadog/datadog v3.2.0
+ provider registry.terraform.io/hashicorp/aws v3.55.0
+ provider registry.terraform.io/hashicorp/helm v2.3.

Terraform Configuration Files

variable "secrets" {
  type = map(string)
  sensitive = true
}

resource "helm_release" "example" {
  name = "redis"

  repository = "https://charts.bitnami.com/bitnami"
  chart = "redis"

  set {
    name = "replica.replicaCount"
    value = 0
  }

  set_sensitive {
    name = "auth.password"
    value = "foo"
  }

  dynamic "set_sensitive" {
    for_each = var.secrets
    content {
      name = set_sensitive.key
      value = set_sensitive.value
    }
  }
}

Note that this example uses an arbitrary Helm chart, just to demonstrate the behavior around processing of the dynamic set_sensitive attribute.

Debug Output

Crash Output

Expected Behavior

In prior versions of Terraform (we last tried 0.13.5) — albeit without the input variable being marked as sensitive since that wasn't supported yet — the Terraform apply succeeded.

Actual Behavior

In Terraform 1.0.8, we see the following error:

│ Error: Invalid dynamic for_each value
│
│   on test.tf line 6, in resource "helm_release" "example":
│  6:      for_each = var.secrets
│     ├────────────────
│     │ var.secrets has a sensitive value
│
│ Cannot use a map of string value in for_each. An iterable collection is required.

As described in for_each documentation, we understand that this new behavior is intentional for resources (and, therefore, presumably to modules as well):

Sensitive values, such as sensitive input variables, sensitive outputs, or sensitive resource attributes (if the provider_sensitive_attrs experiment is enabled), cannot be used as arguments to for_each. The value used in for_each is used to identify the resource instance and will always be disclosed in UI output, which is why sensitive values are not allowed.

It wasn't clear to us, however, if this behavior was intentionally intended to apply for dynamic attributes of a resource. We can work around the error by using the nonsensitive function, like this:

 dynamic "set_sensitive" {
    for_each = nonsensitive(var.secrets)

That could work here since the set_sensitive attribute is inherently sensitive in a helm_release resource and so would be redacted from UI output. It just seemed incorrect to have to do that when using content that fundamentally is sensitive.

Steps to Reproduce

  1. terraform init
  2. TF_VAR_secrets='{secret_1_key: "secret_1_val", secret_2_key: "secret_2_val"}' terraform apply

Additional Context

References

alisdair commented 2 years ago

Hi @camlow325, thanks for reporting this. I can reproduce this behaviour with dynamic.

The cause is that we haven't added support for sensitive for_each values in the hcl/dynblock package. Doing so seems possible, but not trivial. Sensitive collections are not iterable, and removing the sensitivity without later reapply it to the individual values is not an acceptable solution to that.

We would also have to determine how to prevent sensitive iterator values from being used in the list of labels, which (like resource instance keys) would result in sensitive value escaping.

danieljkemp commented 2 years ago

I just ran face-first into this.

Sensitive collections are not iterable

This is not intuitive, either from the error message or the documentation on input variables. If there are no plans to support sensitive values in dynamic for_each, a more clear warning message, or updated documentation or both would help prevent someone else from the same frustration.

alisdair commented 2 years ago

That's a good point, and I agree that this error message is poor. An interim improvement for this issue could detect a sensitive for_each value and present more useful suggestions about what to do next.

willmorgan commented 1 year ago

TF noob here. I spent a good hour messing around with different kinds of types to try and work around this problem, when in fact the error was due to the input being sensitive.

My use case is mapping environment settings from a secrets file decrypted with SOPS straight into the env block of a Docker container definition. Iterating directly over a map of secrets is far easier to manage and reason about than specifying each environment variable block twice: once for the actual key/value map, and again for the block.

I'm using the nonsensitive workaround for now, which is good enough - but I won't be able to get that hour back of my life 😁

pdecat commented 1 year ago

For the record, there's a variant of the error message with apparently the same cause:

╷
│ Error: Invalid dynamic for_each value
│
│   on pagerduty/webhook_subscriptions_onedash.tf line 132, in resource "pagerduty_webhook_subscription" "webhook_onedash_preprod_bh":
│  132:       for_each = { for header_name, header_value in {
│  133:         "Authorization"         = "Bearer ${var.webhooks[split("/", each.key)[1]].bearer_token}"
│  134:         "x-alerting-doc-url"    = each.value.doc
│  135:         "x-alerting-project-id" = each.value.bh_project
│  136:         "x-alerting-source"     = each.value.bh_source
│  137:         }
│  138:         :
│  139:         header_name => header_value if header_value != ""
│  140:       }
│     ├────────────────
│     │ each.key is "Sodexo CIAM/onedash_preprod"
│     │ each.value.bh_project is "PRJ-M9FV44"
│     │ each.value.bh_source is "RMP_Pagerduty_HO"
│     │ each.value.doc is ""
│     │ var.webhooks is map of object with 15 elements
│
│ Cannot use a object value in for_each. An iterable collection is required.

Searching for that error message gave no result, so I figured adding a comment here would not hurt. Especially as it is even less understandable as the sensitive keyword does not appear.

When I added a tomap() function call to try and troubleshoot the issue, I got the same error message as the OP, and finally found this Github issue:

│ Error: Invalid dynamic for_each value
│
│   on pagerduty/webhook_subscriptions_onedash.tf line 132, in resource "pagerduty_webhook_subscription" "webhook_onedash_preprod_bh":
│  132:       for_each = tomap({ for header_name, header_value in {
│  133:         "Authorization"         = "Bearer ${var.webhooks[split("/", each.key)[1]].bearer_token}"
│  134:         "x-alerting-doc-url"    = each.value.doc
│  135:         "x-alerting-project-id" = each.value.bh_project
│  136:         "x-alerting-source"     = each.value.bh_source
│  137:         }
│  138:         :
│  139:         header_name => header_value if header_value != ""
│  140:       })
│     ├────────────────
│     │ each.key is "a/b"
│     │ each.value.bh_project is "PRJ-123"
│     │ each.value.bh_source is "SRC-123"
│     │ each.value.doc is ""
│     │ var.webhooks is map of object with 15 elements
│
│ Cannot use a map of string value in for_each. An iterable collection is required.
╵

The work-around to use nonsensitive() also works in this case:

    dynamic "custom_header" {
      for_each = { for header_name, header_value in {
        "Authorization"         = "Bearer ${nonsensitive(var.webhooks[split("/", each.key)[1]].bearer_token)}"
        "x-alerting-doc-url"    = each.value.doc
        "x-alerting-project-id" = each.value.bh_project
        "x-alerting-source"     = each.value.bh_source
        }
        :
        header_name => header_value if header_value != ""
      }
      content {
        name  = custom_header.key
        value = custom_header.value
      }
    }

Edit: But now, the secret obviously appears in plan :facepalm: ~In this precise use case, I'll just process the bearer token out of the dynamic block because I know it will always be defined.~

Edit 2: Better yet, I'll just call sensitive() on the whole header with an exception:

    dynamic "custom_header" {
      for_each = { for header_name, header_value in {
        "Authorization"         = "Bearer ${nonsensitive(var.webhooks[split("/", each.key)[1]].bearer_token)}"
        "x-alerting-doc-url"    = each.value.doc
        "x-alerting-project-id" = each.value.bh_project
        "x-alerting-source"     = each.value.bh_source
        }
        :
        header_name => header_value if header_value != ""
      }
      content {
        name  = custom_header.key
        value = custom_header.key == "Authorization" ? sensitive(custom_header.value) : custom_header.value
      }
    }
lenalebt commented 1 year ago

In a complex scenario, it took me around 5 hours (and a rubberduck companion for an hour) to finally end up here and see that the issue was due to my inputs being sensitive. To make it short, I think it would already be a lot easier to debug if the error message would at least say something along the lines of "reminder: sensitive values are not iterable".

maxhillaert commented 1 year ago

I'm counting 12 hours , burning the midnight oil trying to find an obscure sensitive value breaking my for_each. Any tips for doing this? Many modules involved. Can't see any sensitive = true in the code we own. I'm left thinking the azurerm provider is baking in sensitive value and it is part of the dependency graph.

lodotek commented 8 months ago

still no solution here here? 😢

gRizzlyGR commented 7 months ago

Had the same problem for an ALB listener rule with dynamic query strings. I solved this by splitting the original variable definition. From this:

variable "basic_auth" {
  type = object({
    username = string
    password = string
    enabled = bool
    query_strings = list(object({
      key   = string
      value = string
    }))
  })

  sensitive = true
}

To this:

variable "basic_auth_credentials" {
  type = object({
    username = string
    password = string
  })

  sensitive = true
}

variable "basic_auth_config" {
  type = object({
    enabled = bool
    query_strings = list(object({
      key   = string
      value = string
    }))
  })
}

So I can use var.basic_auth_config.query_strings with dynamic and for_each.

jeacott1 commented 1 month ago

3 years later this is still an issue.