terraform icon indicating copy to clipboard operation
terraform copied to clipboard

Plan is confusing when output value changes between tuple type and list type with the same elements values

Open Nuru opened this issue 3 years ago • 15 comments

Changing

output "test" {
  value = ["first", "second"]
}

to

output "test" {
  value = true ? ["first", "second"] : [] # ["first", "second"]
}

unexpectedly and incorrectly produces a plan with a change like

Changes to Outputs:
  ~ test = [
      - "first",
      - "second",
    ] -> [
      + "first",
      + "second",
    ]

This occurs in Terraform v1.2.0 and earlier versions, such as 1.1.7.

Terraform Version

Terraform v1.2.0
on darwin_amd64
+ provider registry.terraform.io/hashicorp/aws v4.15.1

Nuru avatar May 21 '22 21:05 Nuru

Hi @Nuru,

Thanks for filing the issue!

Altering the configuration in this way is changing the type of the output value, going from a tuple(string, string) to a list(string). While these differences may be important in many situations (including for nested module outputs), for root module outputs the change is not going to be important for the end user.

Since outputs are dynamically typed, we should be able to check if root module outputs can serialize to the same value and avoid the extra diff in the plan.

Thanks!

jbardin avatar May 23 '22 13:05 jbardin

It could potentially be significant if the output values are consumed elsewhere using terraform_remote_state, so perhaps another way to proceed here would be to use similar annotations as we use in terraform console and, I think, in the final apply success messages, to distinguish a list from a tuple by wrapping it in a tolist( ... ) call, so that in this case the "after" value would at least be rendered differently than the "before" value.

Admittedly a change from a tuple to a list would still not be super clear to someone who isn't yet familiar with the type system details, but it would at least show that the type is changing and thus hopefully give a better clue that there's something to learn about the type system here.

Changes to Outputs:
  ~ test = [
      - "first",
      - "second",
    ] -> tolist([
      + "first",
      + "second",
    ])

apparentlymart avatar May 23 '22 14:05 apparentlymart

Thanks @apparentlymart, for some reason I was thinking we still serialized these without type information. I even said these are dynamically typed, which would indicate that the type information is encoded in the state, therefore the serialized representations wouldn't match anyway.

jbardin avatar May 23 '22 14:05 jbardin

@apparentlymart Thank you for pointing this out. I agree with you that showing the output changed from tuple to list would be helpful. Despite using Terraform practically every working day for 2 years now, I still sometimes get confused about whether I am dealing with a tuple or a list and what causes a tuple to get converted to a list, and it does sometimes cause problems because my tuples are not always valid lists.

More to the point, @jbardin, why is the output changing from tuple to list? How can I keep the output type from changing?

Nuru avatar May 23 '22 18:05 Nuru

What's happening here is that Terraform is noticing that the two arms of your conditional expression have different types and so rather than immediately returning an error Terraform is trying to find a type that they can both convert to, and assuming that you intended to make those conversions.

In the expression true ? ["first", "second"] : [], the "true" result is of type tuple([string, string]) while the "false" result is of type tuple([]) (the empty tuple type). Terraform noticed that a tuple whose elements are all strings and a tuple with no elements at all can both convert to list(string), and so it guessed that what you meant and made the result be a list of strings.

The purpose of tuple types is to make the individual element types be part of the type so that we can support different elements having different types. We have this in the type system primarily so that Terraform's type system will be a superset of JSON's: Terraform's tuple types correspond with JSON arrays. That means there isn't any way for this conditional expression to return a tuple type though, because there is no tuple type that both of these values can convert to. List types, on the other hand, represent any number of elements of the same type, and so are a good type to use if you want to conditionally decide on how many elements there will be, as is the case here.

With that said then, there is no way to write this so that the new expression would return a tuple type, but in this case it could in principle have been a list type in the original example, which would mean that addition the condition would not change the type:

output "test" {
  value = tolist(["first", "second"])
}
output "test" {
  value = true ? tolist(["first", "second"]) : tolist([])
}

apparentlymart avatar May 23 '22 18:05 apparentlymart

@apparentlymart This now seems to me a bigger problem.

In practice, of course, the output is not all constants, and the old value may not be a list. It might be a tuple, in which case the output now breaks:

output "test" {
  value = true ? ["first", { second = "second" }] : []
}

of course breaks with

The true and false result expressions must have consistent types

This would catch me by surprise, and is undesirable. The reason I don't use

output "test" {
  value = true ? ["first", { second = "second" }] : null
}

is because in the false case, null breaks length(module.example.test) (and also because, in a root module, the output does not show up in the results ((null outputs are suppressed)) ).

I would like a way to keep this conditional output a tuple.

Nuru avatar May 23 '22 19:05 Nuru

This issue is currently representing the fact that Terraform's rendering of output values is confusing in cases where the type has changed in a way where the new value renders the same as the old.

It sounds like you also want to talk about ways to make output values have different types depending on other data in the module. I would typically suggest thinking of output values as having static types in most cases, but since Terraform has a hybrid static/dynamic type system in order to ergonomically support working with JSON, so there are some ways to achieve it if you work a bit against the design of the language. If you want to talk more about that, let's do it in the community forum because it's more appropriate for this sort of unstructured discussion.

apparentlymart avatar May 23 '22 20:05 apparentlymart

It sounds like you also want to talk about ways to make output values have different types depending on other data in the module.

Really all I want here is the ability to preserve the output type of tuple while using a conditional. Personally I don't consider tuple(object, object) any more or less dynamic than list(object) but I confess I don't think about it that much or that deeply. At the moment, I do not want to participate in that discussion, so if you just tell me there is no way to keep the output a tuple with a length(), then we can leave it there and limit this issue to the plan output being clearer.

Nuru avatar May 23 '22 20:05 Nuru

There are two sequence-like type kinds in Terraform:

  1. Tuple: a fixed-length sequence of elements that can each be of different types.
  2. List: a variable-length sequence of elements that are all of the same type.

The design intent is that, similar to other languages with explicit typing, you decide for each output value what type it will have and always return a value of that type. Within that design intent, there is no way for the return type of an output value to be both tuple([string, string]) and tuple([]) at the same time, for similar reasons that e.g. in Go you can't write a function that has a different return type depending on the situation, but you can return a []string if you don't know statically how many elements you'll return.

However, the Terraform language does have some escape hatches intended to make it easier to work with dynamic data like JSON results, and so you can potentially exploit those to get the effect you want even though this isn't what those features were intended for:

  1. Literally use JSON as the intermediary, which is the most direct interpretation of using the JSON-oriented features to trick Terraform into doing what you want:

      value = jsondecode(var.condition ? jsonencode(["first", { second = "second" }]) : jsonencode([]))
    

    This can work because the conditional expression always returns string, but Terraform treats jsondecode as special to allow it to return whatever type the JSON string implies, so it gets type-checked dynamically instead of statically.

  2. Perhaps a less obnoxious approach is to use a for expression to dynamically construct a differently-typed value in each case by conditionally filtering out all of the elements:

      value = [for v in ["first", { second = "second" }] : v if var.condition]
    

    Again for expressions are designed to allow construction of arbitrary data structures, so that they can be then sent to functions like jsonencode to produce dynamic JSON arrays. However, even if you don't send the result to a function like jsonencode the result is still a dynamically-selected tuple type, and so the above will effectively construct an empty tuple in any case where var.condition is false.

Intentionally returning a sequence of differently-typed elements is not a typical Terraform module design -- the more common approach would be to return an object type where each of the different elements has a meaningful name, which corresponds with declaring a structure type in languages like Go. However, there isn't any technical reason why you can't do the above if you want to; it'd just be something I expect most Terraform users would find surprising if this were in a public shared module.

apparentlymart avatar May 23 '22 23:05 apparentlymart

Thank you, @apparentlymart , that is very helpful. Some version of your comment should make its way into the official Terraform documentation.

For this issue, I will be satisfied with the change to the plan output you proposed here.

Nuru avatar May 25 '22 17:05 Nuru

I have tested this with various versions of Terraform and noticed that appears fixed in 1.4.0-beta1.

  • 1.2.0: issue occurs
  • 1.3.0: issue occurs
  • 1.3.9: issue occurs
  • 1.4.0-alpha20221207: issue occurs
  • 1.4.0-alpha20221109: issue occurs
  • 1.4.0-beta1: issue resolved
  • 1.4.0-beta2: issue resolved
  • 1.4.0-rc1: issue resolved
  • 1.4.0: issue resolved
  • 1.4.4: issue resolved

The v1.4.0 included a rewrite of the Terraform plan renderer (#32520) (ref: v1.4/CHANGELOG.md), after which the issue no longer appears.

I can't explain how/why the new plan renderer fixes it, but to convince myself that it does, I compiled terraform and tested the issue at two points: at the commit which enabled the new renderer (8d61c5bfc411a97716e4b6f387cf5b6cb1a53cec), and at the commit right before that.

Issue fixed, as in, the terraform plan now shows no diff:

$ terraform plan

───────────────────────────────────────────────────────────────────────────────────────────────────────────────

Note: You didn't use the -out option to save this plan, so Terraform can't guarantee to take exactly these
actions if you run "terraform apply" now.

Note: This is my first ever contribution to Terraform, I hope it's not unwelcome.

nickyfow avatar Apr 07 '23 15:04 nickyfow

@nickyfow Thanks for this thoughtful write-up! I'll confirm with the maintainers. Thanks!

crw avatar Apr 07 '23 17:04 crw

Thanks indeed for letting us know @nickyfow!

Interestingly it seems like the new plan renderer may have "over-fixed" this, at least in terms of what I was asserting about the solution above.

I tried initially with this configuration:

output "foo" {
  value = ["a"]
}

I applied it to get a starting point:

Changes to Outputs:
  + foo = [
      + "a",
    ]

You can apply this plan to save these new output values to the Terraform state,
without changing any real infrastructure.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes


Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

Outputs:

foo = [
  "a",
]

I then changed the configuration to make the output value a list with the same content, mimicking the situation in the original report above:

output "foo" {
  value = tolist(["a"])
}

I expected to see a plan to change this value from a tuple into a list as we discussed earlier, but instead it said "No changes":


No changes. Your infrastructure matches the configuration.

Terraform has compared your real infrastructure against your configuration and
found no differences, so no changes are needed.

If I add a new element at the same time as changing it to a list:

output "foo" {
  value = tolist(["a", "b"])
}

...then Terraform reports that it is adding the element but still doesn't mark the value as being a list, rendering it as if it were a tuple([string, string]):

Changes to Outputs:
  ~ foo = [
        "a",
      + "b",
    ]

You can apply this plan to save these new output values to the Terraform state, without
changing any real infrastructure.

Amusingly, if I use terraform apply to plan and apply the changes at the same time, Terraform now gets confused because the plan renderer disagrees with the rest of Terraform about what constitutes a change (I reverted back to the list with only "a" before running this):


No changes. Your infrastructure matches the configuration.

Terraform has compared your real infrastructure against your configuration and
found no differences, so no changes are needed.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes


Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

Outputs:

foo = tolist([
  "a",
])

Normally the "No changes." message causes Terraform to exit immediately without the "Do you want to perform these actions?" prompt, but the control logic can still see that the output value has changed even though the plan renderer is pretending it hasn't, and so it still shows the usual prompt to confirm and then updates the state to make it a list anyway.

So my sense is that this bug isn't really fixed, but it's now broken in a slightly different way than before. This new way is arguably slightly worse because it reports that nothing will change but then changes something anyway, so I think we should prioritize fully fixing this, by making the plan renderer explicitly show the type changing from tuple to list as we discussed earlier in this issue.

apparentlymart avatar Apr 07 '23 18:04 apparentlymart

@apparentlymart wrote (in the comment just before this one):

So my sense is that this bug isn't really fixed, but it's now broken in a slightly different way than before.

Do you have an update? Is it fully fixed now?

Nuru avatar May 11 '23 18:05 Nuru

Are there any updates on this? I am experiencing this exact same issue right now with Version 1.4.6. It is extremely annoying when it comes to larger resources and modules, because Terraform wants to change something what is basically the exact same then before, just that there is no count condition to it.

My previous configuration was

module "rds" {
  count    = var.environment == "prod" ? 1 : 0
  source  = "terraform-aws-modules/rds/aws"
  version = "5.1.0"
  // ...

Changed it to

module "rds" {
  source  = "terraform-aws-modules/rds/aws"
  version = "5.1.0"
  // ...

Which leads to:

  # module.rds[0].module.db_instance.aws_db_instance.this[0] will be destroyed
  # (because module.rds[0].module.db_instance is not in configuration)

and creating a new instance.

Derison avatar Sep 08 '23 11:09 Derison