for_each if conditional deletes resources inside of module if map(any) variable has inconsistent types

[notfromstatefarm]

Terraform Version

Terraform v1.0.11
on darwin_amd64
+ provider registry.terraform.io/hashicorp/null v3.1.0

Terraform Configuration Files

https://github.com/notfromstatefarm/tf-module-var-issue

Debug Output

Output of terraform plan when it tries to delete the resources

Expected Behavior

When adding a new, unreferenced key/value to a subset of objects inside of a map, it shouldn't change anything (or should show an error for inconsistent types?)

Actual Behavior

Terraform plans to delete the resource using the map in a conditional for_each if the key is not added to all objects

Steps to Reproduce

1. Apply as is
2. Uncomment one of the bye parameters in test.tf
3. Apply will now show it deleting the resource inside the module that uses a for_each with an if statement, but not the unconditional for_each, nor the resource outside of the module.

If both byes are uncommented, everything is normal.

Also listed in repo README

Additional Context

I noticed that when there's a map of objects with just a single field and you add another field to one of the objects you receive the following error: The given value is not suitable for child module variable "tests" defined at testmodule/variables.tf:1,1-17: all map elements must have the same type.

Example of this additional context:

No error:

locals {
  tests = {
    ab = {
      enabled   = true
    }
    cd = {
      enabled   = true
    }
  }
}

Has error:

locals {
  tests = {
    ab = {
      enabled   = true
      asdf = 123
    }
    cd = {
      enabled   = true
    }
  }
}

I assume this error should be showing in this issue's scenario too and for some reason it isn't when there's more than two fields.. and even more strangely is only breaking if statements that aren't even using the additional field.

Important things to note

1. for_each not using if and for_each outside of the module works fine
2. setting conditional to false doesn't matter, both true and false result in resource being deleted
3. There are no errors, it is simply planning to delete the resource

References

None that I could find

[apparentlymart]

Hi @notfromstatefarm! Thanks for reporting this.

I think what you've encountered here is a subtlety of Terraform's implicit type inference. By default, Terraform tries to automatically infer what type you "probably" intended to use, with many of those rules motivated by backward-compatibility with older versions of Terraform that didn't yet have separate types for numbers and booleans.

As you noted, all elements of a map must have the same type, and so if you write type = map(any) then Terraform must try to find a single type that all of the given elements can convert to.

In your case, I believe that Terraform is noticing that although each of your elements has a different object type, the values of those attributes all have implicit conversions to string available, and therefore map(map(string)) is an available resolution to force your given value to conform to the type constraint -- the any is resolved by replacing it by map(string). This is an example of a behavior required to be compatible with Terraform v0.11 and earlier, because in those versions all of these attribute values would have already been strings, due to those older versions not yet having any other primitive types.

That type conversion means that the two non-string attribute values will get automatically converted to string, producing a value like this:

locals {
  tests = {
    ab = {
      enabled   = "true"
      something = "else"
      bye       = "1337"
    }
    cd = {
      enabled   = "true"
      something = "else"
    }
  }
}

Unfortunately, while "true" can implicitly convert to true when used in a boolean context, you explicitly compared with the string "true" in your expression:

  for_each = { for name, test in var.tests : name => test if test.enabled == true }

true and "true" are not equal to one another, so the condition here fails. If you'd instead written just if test.enabled then I believe this actually could've worked, because Terraform can see that if is a boolean context and thus implicitly convert "true" back to true again.

So that gives one possible resolution to the quirk you saw here: evaluate your value directly as a boolean value rather than comparing it to one.

The other option would be to write an explicit type constraint so that Terraform doesn't have to guess what type constraint you probably meant:

variable "tests" {
  description = "tests"
  type        = map(object{
    enabled   = bool
    something = string
    bye       = number
  })
}

With that said then, unfortunately I believe what you observed here represents Terraform working as designed, although I would certainly agree it's confusing behavior. One of the consequences of the automatic type conversion and inference behaviors we inherited from Terraform v0.11 and earlier is that sometimes they obscure a problem by preventing Terraform from returning an error.

We are blocked from changing them due to Terraform v1.0 Compatibility Promises, and so something I'm currently prototyping is an extra mode where Terraform can give hints about things that are technically valid but seem likely to be doing something other than what the author intended, over in #29661. Coincidentally, one of the interactions you encountered here happens to be the first thing on my list of ideas for hint rules:

• Conditional expressions where the predicate provably always returns true or always false, such as comparing two values of different types with either == or !=.

If we'd implemented that rule and you'd activated "validation hint" mode then Terraform should've been able to detect that test.enabled is a string and true is a bool and thus that conditional expression can never evaluate to anything other than false. However, I can see that it wouldn't have told the whole story here because it wouldn't have been sufficient to explain why test.enabled is a string here. For that to work I think we'd need an additional hint that an object with non-homogeneous attribute types got converted to a map under automatic type conversion, which seems to be an error more often than it's intentional and if it is intentional can be silenced by explicitly converting everything to string using tostring.

With that said then, I expect that hint mode is unfortunately probably the best we can do to help with this, given compatibility constraints. However, I'm going to leave this open here to record the confusion, in case someone else has an idea of how to resolve it in a way that is backward-compatible.

[notfromstatefarm]

Thanks for the detailed reply @apparentlymart!

In my example code, there was initially a boolean and a string, so wouldn't it have been doing this type conversion already? Or was it able to determine a more specific typing in that case since both ab and cd were the same?

Secondly, is it intended that the type conversion only occurs when passing to a module? I would've assumed it did the conversion when it was initially declared in locals.

I agree there should be a warning/hint simply showing that your map is being typecasted like this. I can't imagine this is commonly desired.

Is there a way to easily display what variables are truly being passed to a module?

[apparentlymart]

Hi @notfromstatefarm,

In your initial example I expect that the inferred type would be map(object({ enabled = bool, something = string })), because in that case the element types all have matching object types and so no further type conversion is necessary to produce a valid type.

This didn't happen when you used the value directly because your local value has no type constraints of its own and so its type is something like this:

object({
  ab = object({
    enabled   = bool
    something = string
    bye       = number
  })
  cd = object({
    enabled   = bool
    something = string
  })
})

It was the type constraint on the input variable that called for converting the top-level object type into a map type, and so that's why this only happened at the boundary into the module. You could get the same effect for your local variable itself by converting it to a map, like this:

locals {
  tests = tomap({
    ab = {
      enabled   = true
      something = "else"
      #bye       = 1337
    }
    cd = {
      enabled   = true
      something = "else"
      #bye       = 1337
    }
  })
}

The tomap conversion function is essentially the same as converting to map(any), forcing the same type inference and potential automatic conversions to figure out what the element type of the resulting map ought to be.

Writing out an explicit type (that is, one that doesn't include any at all) is the best way to avoid this sort of problem today, because it completely disables all of these automatic type inference behaviors and Terraform will instead just immediately return an error if the value can't be directly converted to the given type. The any placeholder exists to tell Terraform to figure it out itself, matching as much as possible Terraform v0.11 and earlier would've allowed, and so it creates more opportunities for Terraform to misunderstand your intent and produce a result other than what you expected.