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

Lookup default value type error for map of objects with optional attributes (1.3.5 regression) #32417

Closed
SpComb opened this issue Dec 20, 2022 · 5 comments
Labels
bug working as designed confirmed as reported and closed because the behavior is intended

Comments

@SpComb
Copy link

SpComb commented Dec 20, 2022

Terraform Version

Terraform v1.3.6
on linux_amd64

Terraform Configuration Files

variable "name" {
  type    = string
  default = "test"
}

variable "test_count" {
  type    = number
  default = 1
}

variable "overrides" {
  type = map(object(
    {
      foo = optional(string)
      bar = optional(string)
    }
  ))
  default = {}
}

output "test" {
  value = {
    for index in range(var.test_count) : "${var.name}${index + 1}" => lookup(var.overrides, "${var.name}${index + 1}", {})
  }
}

Debug Output

https://gist.github.com/SpComb/8d9cc19f6c43c3221f95c21db99c370a

Expected Behavior

terraform 1.3.4 plan output:

Changes to Outputs:
  + test = {
      + test1 = {
          + bar = null
          + foo = null
        }
    }

Actual Behavior

terraform 1.3.5 plan error:

╷
│ Error: Invalid function argument
│ 
│   on test.tf line 23, in output "test":
│   23:     for index in range(var.test_count) : "${var.name}${index + 1}" => lookup(var.overrides, "${var.name}${index + 1}", {})
│ 
│ Invalid value for "default" parameter: the default value must have the same type as the map elements.
╵

Steps to Reproduce

  1. terraform plan

Additional Context

No response

References

No response

@SpComb SpComb added bug new new issue not yet triaged labels Dec 20, 2022
@SpComb
Copy link
Author

SpComb commented Dec 20, 2022

Workaround is to use the default value for a typed input parameter. I don't know of any other way of explicitly expressing {} as an object literal with the correct type.

@@ -8,6 +8,16 @@ variable "test_count" {
   default = 1
 }
 
+variable "override_defaults" {
+  type = object(
+    {
+      foo = optional(string)
+      bar = optional(string)
+    }
+  )
+  default = {}
+}
+
 variable "overrides" {
   type = map(object(
     {
@@ -20,6 +30,6 @@ variable "overrides" {
 
 output "test" {
   value = {
-    for index in range(var.test_count) : "${var.name}${index + 1}" => lookup(var.overrides, "${var.name}${index + 1}", {})
+    for index in range(var.test_count) : "${var.name}${index + 1}" => lookup(var.overrides, "${var.name}${index + 1}", var.override_defaults)
   }
 }
$ docker run -v $PWD:/src -w /src -it hashicorp/terraform:1.3.6 plan

Changes to Outputs:
  + test = {
      + test1 = {
          + bar = null
          + foo = null
        }
    }

`` `

@liamcervante
Copy link
Member

liamcervante commented Dec 20, 2022

Hi @SpComb, thanks for the report and the reproduction. I can confirm that I can see the same thing, and that this worked in 1.3.4, and is broken in 1.3.5 and 1.3.6.

tl;dr: The correct thing for you to do here is to write your lookup as follows: lookup(var.overrides, "${var.name}${index + 1}", {foo = null,bar = null}. Unfortunately, this is an example of useful bug that made something work previously that shouldn't have ever worked. I will close this as working as designed, the rest of this comment is attempting to explain the reasoning behind why it works the way it works.

So the reason this changed is we fixed a downstream bug in the type system that was leaving the metadata about optional attributes in empty collections: zclconf/go-cty#143

Even in 1.3.4 if you populated the default value of overrides with anything (so the default wasn't empty), you would see the same error you are seeing now (I will add another comment below that demonstrates this). This leads me to say that this was unfortunate case of a useful bug that meant this worked previously when it shouldn't have. I do think this is confusing and I'll try to explain a bit more (this comment has good overview of what I'll try to say).

Basically, you can only specify optional attributes in type constraints. This type constraint is what you put in the type field of the variable definition, and where you can specify optional attributes. Terraform and HCL then convert your default (the empty collection) into a concrete value which strips out the optional metadata and replaces the optional attributes with concrete attributes (if the default value had any items, these attributes would be given null values if they weren't populated). Next, when you specify the default for the lookup function, you are specifying an object with no attributes. As it is a concrete value you are specifying, you cannot provide optional attribute metadata at this point. The error you see comes from Terraform treating the value from the variable as objects with two attributes, and the default value in the lookup function as an object with no attributes.

This is similar to #32200.

The correct thing for you to do here is to write your lookup as follows: lookup(var.overrides, "${var.name}${index + 1}", {foo = null,bar = null}. This makes the types match. The trick is that the optional syntax on the type constraint is just syntactic sugar that hides the fact optional attributes are actually just null values when the actual object is created.

@liamcervante
Copy link
Member

This comment contains a modified reproduction that also fails in v1.3.4. I think this demonstrates this isn't a case of regression but actually a change that made empty collections and populated collections behave in the same way.

variable "name" {
  type    = string
  default = "test"
}

variable "test_count" {
  type    = number
  default = 1
}

variable "overrides" {
  type = map(object(
    {
      foo = optional(string)
      bar = optional(string)
    }
  ))
  default = {
    "example" = {},
  }
}

output "test" {
  value = {
    for index in range(var.test_count) : "${var.name}${index + 1}" => lookup(var.overrides, "${var.name}${index + 1}", {})
  }
}

The above fails with the same error in all 1.3.* versions, and is an example of Terraform behaving in the right (if a slightly confusing) way.

@liamcervante liamcervante added working as designed confirmed as reported and closed because the behavior is intended and removed new new issue not yet triaged labels Dec 20, 2022
@liamcervante liamcervante closed this as not planned Won't fix, can't repro, duplicate, stale Dec 20, 2022
@SpComb
Copy link
Author

SpComb commented Dec 20, 2022

Thank you for the explanation!

@github-actions
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 Jan 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug working as designed confirmed as reported and closed because the behavior is intended
Projects
None yet
Development

No branches or pull requests

2 participants