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

Regression: Incorrect marshaling of data types in JSON #31894

Closed
jcarlson opened this issue Sep 28, 2022 · 3 comments
Closed

Regression: Incorrect marshaling of data types in JSON #31894

jcarlson opened this issue Sep 28, 2022 · 3 comments
Assignees
Labels
bug working as designed confirmed as reported and closed because the behavior is intended

Comments

@jcarlson
Copy link

Terraform Version

Terraform v0.15.5
on darwin_amd64
+ provider registry.terraform.io/hashicorp/local v2.2.3

Terraform Configuration Files

variable "foo" {
  type = list(object({
    name = string
  }))

  default = []
}

variable "bar" {
  type = list(object({
    name      = string
    read_only = bool
  }))

  default = [
    {
      name      = "anything"
      read_only = false
    }
  ]
}

locals {
  foobar = [for volume in concat(var.foo, var.bar) : {
    name     = volume.name
    readOnly = try(volume.read_only, false)
  }]
}

resource "local_file" "container_definitions" {
  filename = "foobar.json"
  content  = jsonencode(local.foobar)
}

output "foobar" {
  value = local.foobar
}

output "json" {
  value = jsonencode(local.foobar)
}

Debug Output

none

Expected Behavior

readOnly should be encoded as a boolean

Actual Behavior

readOnly is encoded as a string

Steps to Reproduce

  1. terraform init
  2. terraform apply

Additional Context

Under Terraform 0.14.11, the plan produced by this configuration is as follows:

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  # local_file.container_definitions will be created
  + resource "local_file" "container_definitions" {
      + content              = jsonencode(
            [
              + {
                  + name     = "anything"
                  + readOnly = false
                },
            ]
        )
      + directory_permission = "0777"
      + file_permission      = "0777"
      + filename             = "foobar.json"
      + id                   = (known after apply)
    }

Plan: 1 to add, 0 to change, 0 to destroy.

Changes to Outputs:
  + foobar = [
      + {
          + name     = "anything"
          + readOnly = false
        },
    ]
  + json   = jsonencode(
        [
          + {
              + name     = "anything"
              + readOnly = false
            },
        ]
    )

Using Terraform 0.15.5, the plan is different:

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  # local_file.container_definitions will be created
  + resource "local_file" "container_definitions" {
      + content              = jsonencode(
            [
              + {
                  + name     = "anything"
                  + readOnly = "false"
                },
            ]
        )
      + directory_permission = "0777"
      + file_permission      = "0777"
      + filename             = "foobar.json"
      + id                   = (known after apply)
    }

Plan: 1 to add, 0 to change, 0 to destroy.

Changes to Outputs:
  + foobar = [
      + {
          + name     = "anything"
          + readOnly = "false"
        },
    ]
  + json   = jsonencode(
        [
          + {
              + name     = "anything"
              + readOnly = "false"
            },
        ]
    )

Note that the readOnly attribute in 0.14.11 is encoded as a boolean, while in 0.15.5 it is encoded as a string. Replacing concat(var.foo, var.bar) with var.bar in this example resolves this issue, so there appears to be some sort of regression in the way objects are iterated after concatenation.

Terraform v1.2.9 produces a similar plan to v0.15.5.

References

cloudposse/terraform-aws-ecs-container-definition#108

@alisdair
Copy link
Member

Thanks for the report and the reduced reproduction case!

The change in behaviour you've found is unfortunate, but the previous behaviour was incorrect, and Terraform is now working as designed. I believe the change responsible is the fix for #27966, which is described in zclconf/go-cty#89.

First I'd like to explain what's happening here, and then later offer some suggestions as to how you can update your configuration to work with the new behaviour.

Using the recently-added type function in terraform console, you can see what's happening:

> var.foo
tolist([])
> type(var.foo)
list(
    object({
        name: string,
    }),
)

var.foo is as defined, an empty list of object({ name: string }).

> var.bar
tolist([
  {
    "name" = "anything"
    "read_only" = false
  },
])
> type(var.bar)
list(
    object({
        name: string,
        read_only: bool,
    }),
)

Similarly var.bar is a one-element list of the specified type.

> type(concat(var.foo, var.bar))
list(map(string))

When concatenating these two different typed values, Terraform must determine a unified type for the list. Lists in Terraform must have a single element type, and all elements must convert to that type.

With the information in the configuration, the only unified type available for these two object types is map(string). Terraform cannot automatically convert object({ name: string }) to object({ name: string, read_only: bool }), as it is invalid to add properties to an object. The alternative would be to drop the read_only attribute, which is also undesirable.

Because bool can be converted safely to string, Terraform's type unification does so, which results in the problematic behaviour you're seeing.


There are several ways that you can work around this behaviour to achieve the result I think you are looking for.

First, you can use the new optional object attributes feature in Terraform 1.3. Adding read-only = optional(bool) to the type constraint for var.foo gives the intended result:

> var.foo
tolist([])
> type(concat(var.foo, var.bar))
list(
    object({
        name: string,
        read_only: bool,
    }),
)
> concat(var.foo, var.bar)
tolist([
  {
    "name" = "anything"
    "read_only" = false
  },
])

Alternatively, you can use a for expression to map the elements of var.foo into the appropriate type for concatenation:

locals {
  foo_converted = [for f in var.foo : merge({ read_only : false }, f)]
  foobar        = concat(local.foo_converted, var.bar)
}
> local.foobar
[
  {
    "name" = "anything"
    "read_only" = false
  },
]

Finally, you can explicitly type-convert the read_only attribute to bool using the tobool() function:

locals {
  foobar = [for volume in concat(var.foo, var.bar) : {
    name     = volume.name
    readOnly = tobool(try(volume.read_only, false))
  }]
}
> local.foobar
[
  {
    "name" = "anything"
    "read_only" = false
  },
]

I hope one of these options works for your real use case, and let me know if you have any follow-up questions.

Since this doesn't represent a bug in Terraform, I'm going to close this issue. Thanks again for the report.

@alisdair alisdair added working as designed confirmed as reported and closed because the behavior is intended and removed new new issue not yet triaged labels Sep 29, 2022
@jcarlson
Copy link
Author

@alisdair That is an unfortunate change in behavior from 0.14 to 0.15, but I understand why, and I appreciate the detailed explanation. I was able to remediate my configuration quite easily by, as you suggested, explicitly casting the value as follows: readOnly = try(tobool(volume.read_only), false).

This particular module where I ran into this is still designed to be compatible with TF 0.13+, so I can't yet take advantage of optional object attributes yet, but I will keep that in mind as we get upgraded.

Thanks!

@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 Oct 30, 2022
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