Terraform: 0.12.0-rc1 '0.12upgrade' converts lists of maps incorrectly

[Hons]

I was upgrading a module to work with 0.12 rc1: https://github.com/terraform-aws-modules/terraform-aws-eks
And encountered some upgrade errors.

Terraform Version

$ terraform version
Terraform v0.12.0-rc1
+ provider.aws v2.10.0
+ provider.local v1.2.2
+ provider.null v2.1.2
+ provider.template v2.1.2

Terraform Configuration Files

variable "worker_groups" {
  description = "A list of maps defining worker group configurations to be defined using AWS Launch Configurations. See workers_group_defaults for valid keys."
  type        = "list"

  default = [
    {
      "name" = "default"
    },
  ]
}

Expected Behavior

Either retain the type list, or convert to a list(map).

Actual Behavior

Conversion to a list(string)

variable "worker_groups" {
  description = "A list of maps defining worker group configurations to be defined using AWS Launch Configurations. See workers_group_defaults for valid keys."
  type        = list(string)
  default = [    {
      "name" = "default"
    },
  ]
}

Steps to Reproduce

1. Put the sample above in a .tf file
2. Run terraform 0.12upgrade
3. Check result

[apparentlymart]

Hi @Hons! Thanks for reporting this.

The current behavior is reflective of the fact that lists and maps of anything other than string weren't intentionally supported in prior versions of Terraform and didn't work fully. However, as you saw it did work partially, and thus there are configurations out there relying on the partial support.

A tension we have here is that the previous behavior was resulting from a total lack of type-checking rather than explicit support for lists of maps, and so it isn't possible to specify exactly the same behavior as before within the new type system that now _is_ doing type checking. If we were to rewrite this as list(map(any)), list(map(string)) or list(any) then it would still be imposing some additional constraints compared to Terraform 0.11, because list(any) and map(any) mean "of any _single_ type" -- all of the elements must have the same type -- rather than that each element can have its own type.

The _best_ migration for what it looks like you intended here would be to define this as a list of objects where all of the expected attributes are enumerated:

variable "worker_groups" {
  description = "A list of maps defining worker group configurations to be defined using AWS Launch Configurations. See workers_group_defaults for valid keys."
  type        = list(object({
    name = string
    # ...and any other attributes you expect to see here
  }))
  default = [
    {
      name = "default"
    },
  ]
}

...but the upgrade tool doesn't have enough information to understand that intent; it's currently mentioned only in the human-readable description, so it can't know that an object type is needed and what attributes should be defined in it.

With that said, I think the best approximation of the old behavior we could manage right now is to detect that the default value isn't a list of strings and generate type = any instead, which would then allow freeform values and disable type checking entirely, which is closer to what Terraform v0.11 would've done. That's not ideal because then the caller could potentially pass something that isn't a list at all, but it would at least avoid breaking something that was working in Terraform v0.11.

variable "worker_groups" {
  description = "A list of maps defining worker group configurations to be defined using AWS Launch Configurations. See workers_group_defaults for valid keys."

  # TF-UPGRADE-TODO: Terraform could not infer an exact element type for this list,
  # so it has disabled type checking on this variable altogether. Consider changing this
  # to a more precise type, such as a list of an object type.
  type        = any
  default = [
    {
      name = "default"
    },
  ]
}

If a variable is defined with no default at all, or if the default is an empty list, then we don't have enough information to make this decision at all, and so we'd still generate list(string). I think for that case we'll need to just rely on the upgrade guide to cover why that occurs and what to do about it; this is a situation where the input doesn't have enough machine-readable information available to automatically upgrade it in all cases.

[Hons]

Thanks @apparentlymart for the extensive response!
That sounds sensible, some of those upgrade TODOs showed up when i was doing this and were extremely helpful.

[hashibot[bot]]

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.