New for_each validation rejects secrets as values

[WhyNotHugo]

I use this pattern (and other similar) to handle secrets:

resource "aws_ssm_parameter" "Secrets" {
  for_each = {
    for secret in flatten([values(local.secret_params)]) :
    secret.name => secret.value
  }

  name  = each.key
  type  = "SecureString"
  value = each.value
}

Note that secrets are values in that map but not keys, so resource instance keys include no secrets -- only their names.

Terraform Version

$ terraform version
Terraform v0.15.0
on linux_amd64
+ provider registry.terraform.io/cyrilgdn/postgresql v1.12.0
+ provider registry.terraform.io/cyrilgdn/rabbitmq v1.5.1
+ provider registry.terraform.io/hashicorp/archive v2.1.0
+ provider registry.terraform.io/hashicorp/aws v3.37.0
+ provider registry.terraform.io/hashicorp/external v2.1.0
+ provider registry.terraform.io/hashicorp/null v3.1.0
+ provider registry.terraform.io/hashicorp/random v3.1.0
+ provider registry.terraform.io/hashicorp/template v2.2.0

Terraform Configuration Files

resource "aws_ssm_parameter" "Secrets" {
  for_each = {
    for secret in flatten([values(local.secret_params)]) :
    secret.name => secret.value
  }

  name  = each.key
  type  = "SecureString"
  value = each.value
}

Debug Output

Acquiring state lock. This may take a few moments...
╷
│ Error: Invalid for_each argument
│
│   on ssm.tf line 2, in resource "aws_ssm_parameter" "Secrets":
│    2:   for_each = {
│    3:     for secret in flatten([values(local.secret_params)]) :
│    4:     secret.name => secret.value
│    5:   }
│     ├────────────────
│     │ local.secret_params has a sensitive value
│
│ Sensitive values, or values derived from sensitive values, cannot be used as for_each arguments. If used,
│ the sensitive value could be exposed as a resource instance key.
╵
Releasing state lock. This may take a few moments...

Expected Behavior

Terraform should let me use non-secrets as instance keys.
This worked fine in 0.14.10.

Since the keys have no secrets (only the values do), there are no secrets in the resource instance names.

Actual Behavior

The for_each validation is overzealous, and rejects secrets being present in map values.
It should only reject them being in mapping keys.

Steps to Reproduce

See example above.

Additional Context

This validation actually reduces security. The only way to work around this (and several similar issues) seems to be to UNMARK secrets as such so they'll be allowed here -- but this also means they WILL be exposed.

The pre 0.15 behaviour was actually safer, by no trying to be overprotective.

[piotr-jagiello]

I can confirm that this is a problem - and moreover, it's doubly annoying because 0.15 now marks some provider outputs as sensitive, for example aws_iam_access_key.ses_smtp_password_v4 is now considered sensitive automatically.
So I have to use "nonsensitive" function as a workaround, which is indeed much worse than old behaviour.

[alisdair]

@WhyNotHugo Thanks for the report!

I'm having trouble trying to reproduce this issue. Can you please give a full example of a variable definition and value, and a resource using this? Ideally please use null_resource or random_pet to make reproduction easier.

We have been working on some improvements to sensitivity handling in various functions (see #28460 for more details). The upcoming 0.15.1 release might have already fixed this, but I'd like to verify that.

[piotr-jagiello]

It does seem like newer versions improved things somewhat (I could remove workarounds for some simpler examples), but it's still pretty easy to confuse sensitivity propagation. This is a simplified example of a real configuration that's still breaking on 0.15.3:

locals {
    global_secrets = {
        password = sensitive("aaaaaa")
    }
    service_specific_secrets = {
        unrelated_service = {
            unrelated_password = sensitive("bbbbb")
        }
    }

    service_a_secrets = merge(local.global_secrets, lookup(local.service_specific_secrets, "service_a", {}))
}

resource "null_resource" "service_a_secrets" {
    for_each = local.service_a_secrets
    provisioner "local-exec" {
        command = "echo '${each.key}: saving the secret safely'"
    }
}

[alisdair]

@piotr-jagiello Thanks for that example. In this case the problem is the lookup function, which was fixed in cty to fully support sensitivity marks in zclconf/go-cty#98, but I didn't notice that Terraform has its own implementation of lookup. We need to make the same improvements here.

Can you please open another issue with that reproduction case, so that we can track & fix it separately from this issue? Please @ me in the issue.

[alisdair]

@WhyNotHugo I'm confident that this issue has been resolved in 0.15.3, so I'm going to close this for now. If you still see the same problem, please reopen with a config showing the problem and I'll be happy to look into it. Thanks!

[WhyNotHugo]

Honestly, I can't think of how to use null_resource or random_pet to generate examples.

Here's an actual read-world example though:

resource "aws_ssm_parameter" "Secrets" {
  for_each = {
    for secret in flatten([values(local.secret_params)]) :
    secret.name => secret.value
  }

  name  = each.key
  type  = "SecureString"
  value = each.value
}

secret_params contains, amongst other things:

DATABASE_URL = "postgis://${site}:${postgresql_role.django.password}@${aws_db_instance.django.endpoint}/${site}"

(This is a very simplified version of things though).

[WhyNotHugo]

Oh, looks like this was closed while I was typing 😅

[alisdair]

Are you still seeing the issue on 0.15.3?

[github-actions]

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.