conditional fails type unification with nested dynamic values

[dr-yd]

Terraform Version

Terraform v1.0.9
on darwin_amd64
+ provider registry.terraform.io/hashicorp/aws v3.35.0
+ provider registry.terraform.io/hashicorp/local v2.1.0
+ provider registry.terraform.io/hashicorp/null v3.1.0

Issue seems to exists in any version after v1.0.4. I finally traced it down with 1.0.9 so please check with that - in previous versions it may behave differently.

Terraform Configuration Files

terraform {
}

resource "null_resource" "test" {
  count = 2
}

locals {
  t1 = formatlist("%s", null_resource.test.*.id)
  t2 = false ? [
    {
      c = []
    },
    ] : [
    {
      c = []
    },
    {
      c = local.t1
    }
  ]
}

Debug Output

https://gist.github.com/dr-yd/ac0c838bdac435052865ec21562f716e

Expected Behavior

• The value of the check should not influence the validity of the code.
• The code should be validated successfully in any case.

Actual Behavior

• If the value being tested is true, the code is successfully validated.
• If the value being tested is false, the validation fails as follows:

│ Error: Inconsistent conditional result types
│
│   on main.tf line 15, in locals:
│   11:   t2 = local.prod ? [
│   12:     {
│   13:       c = []
│   14:     },
│   15:     ] : [
│   16:     {
│   17:       c = []
│   18:     },
│   19:     {
│   20:       c = local.t1
│   21:     }
│   22:   ]
│     ├────────────────
│     │ local.t1 will be known only after apply
│
│ The false result value has the wrong type: element types must all match for conversion to list.

• If the ternary is removed, the code validates successfully:

terraform {
}

resource "null_resource" "test" {
  count = 2
}

locals {
  t1 = formatlist("%s", null_resource.test.*.id)
  t3 = [
    {
      c = []
    },
    {
      c = local.t1
    }
  ]
}

Steps to Reproduce

1. terraform init
2. terraform validate

Additional Context

Local CLI, local state.

[jbardin]

Thanks for example @dr-yd!

The conditional expressions is working as designed here, because it must find a unified type to return for evaluation, so that both the true and false conditions result in the same type. The change in 1.0.5 was the upstream change https://github.com/zclconf/go-cty/pull/115, which fixes a bug in formatlist where it was not be able to determine the correct type to return. What you see here may be a manifestation of the related function call bug mentioned in that PR's comments, but will need further investigation.

What's also interesting here is that even when writing out the explicit list comprehension of

[ for v in null_resource.test: v.id ]

to avoid the splat behavior, we still see a single DynamicVal being passed to formatist, even though that expression is technically known to convert to list(string). This may be because validation is not evaluating count and for_each, since they may not be statically defined, but it may be worth looking into concurrently.

[dr-yd]

Regarding the conditional expression working as designed - as I understand your reply, it should also fail if the value being tested is true. Since the condition being evaluated can be arbitrarily complex, failing based on the value provided sounds like a source for potentially untraceable bugs.

Even narrowing it down this far took me two hours since I absolutely didn't expect this behavior. And that condition was simple, just terraform.workspace == "master" to achieve different values on stage and prod. If the types of the list elements don't end up being the same, the code is not more valid because I switched environments.

As for the rest - thank you for looking into the issue!

[jbardin]

Thanks for the extra info @dr-yd! Optimally the types should unify in the same manner regardless of whether the condition is true or false, so the fact that false here is changing the behavior is surprising. I'll add that to the list of things to investigate.

[jbardin]

Updating here to show what each step of the process is doing for future work:

1. In order to process null_resource.test.*.id, the evaluation state data must first fetch the entire resource value, but since we can't expand the resource, and there is no state to reference, we only return early with a DynamicVal

2. The change referenced earlier is for formatlist to return a DynamicVal when any arguments are of an unknown type, because formatlist has always been able to accept either lists or single values as arguments which changes the function behavior. The prior incorrect result only happen to match the type desired here, and was hiding the underlying issue.

3. The conditional expression must unify the result types in order for the expression itself to have a single type. The given types return a unified type, however that unification process is unsafe and fails during conversion.

4. The cty type unification for

   • True: cty.TupleVal([]cty.Value{cty.ObjectVal(map[string]cty.Value{"c": cty.EmptyTupleVal})})
   • False: cty.TupleVal([]cty.Value{cty.ObjectVal(map[string]cty.Value{"c": cty.EmptyTupleVal}), cty.ObjectVal(map[string]cty.Value{"c": cty.DynamicVal})})

   is cty.List(cty.Object(map[string]cty.Type{"c": cty.DynamicPseudoType})), but unfortunately the False value cannot convert to that type, resulting in the error above.

The problem with unification in this case is that the expression type needs to be a list due to the possibility of different numbers of values in the container, however the conversion will always fail because while the values each may satisfy the type constraint, the resulting list cannot contain multiple types. There probably cannot be a general solution in the Terraform language until we have inline type conversions to specify the desired types more directly.

One improvement which may be possible now within Terraform, is if we can return a more specific type for the resource data accessed during the validate walk. We can determine the mode of expansion from the config, so if we can get an absolute provider and schema, we can get the resource type ty and return cty.UnknownVal(ty), cty.UnknownVal(cty.List(ty)) or cty.UnknownVal(cty.Map(ty)) accordingly. A problem here may be that the source of truth for evaluation is the state, and we may not have access to the absolute provider to get the schema without any resource state.

[jbardin]

The linked PR #29862 is aiming to handle the case shown here by creating more precisely typed values during validation. This however does not solve the overall problem, since we can't always have exact type information. For example, just switching from a resource to a variable would still produce the same result:

variable "in" {
}

locals {
  t2 = false ? [{ c = [] }] : [{ c = [] }, { c =  formatlist("%s", var.in) }]
}

[jbardin]

I should probably add that in the last example we can make it work by ensuring there is as type for var.in:

variable "in" {
  type = list(string)
}

Which illustrates why I mentioned before that we probably can't solve all these cases until there is a general solution for defining type constraints/conversions inline, since the usual reason why seemingly valid conditional expressions fail to validate is because the inferred types don't match exactly what was intended.

[arekt]

I think I hit the same problem on terraform 1.1

   source  = "terraform-aws-modules/rds/aws"
   version = "~> 3.0"

+  count                           = var.environment == "production" ? 1 : 0

!!!!!!!!!!!!!!!!!!!!!!!!!!! TERRAFORM CRASH !!!!!!!!!!!!!!!!!!!!!!!!!!!!

Terraform crashed! This is always indicative of a bug within Terraform.
Please report the crash with Terraform[1] so that we can fix this.

When reporting bugs, please include your terraform version, the stack trace
shown below, and any additional information which may help replicate the issue.

[1]: https://github.com/hashicorp/terraform/issues

!!!!!!!!!!!!!!!!!!!!!!!!!!! TERRAFORM CRASH !!!!!!!!!!!!!!!!!!!!!!!!!!!!

runtime error: invalid memory address or nil pointer dereferencegoroutine 6406 [running]:
runtime/debug.Stack()
	/usr/local/go/src/runtime/debug/stack.go:24 +0x88
runtime/debug.PrintStack()
	/usr/local/go/src/runtime/debug/stack.go:16 +0x20
github.com/hashicorp/terraform/internal/logging.PanicHandler()
	/Users/distiller/project/project/internal/logging/panic.go:44 +0xe0
panic({0x1028b7a00, 0x104074910})
	/usr/local/go/src/runtime/panic.go:1038 +0x21c
github.com/hashicorp/terraform/internal/instances.(*expanderModule).resourceInstances(0x0, {0x14000a2e6c0, 0x1, 0x2}, {{}, 0x4d, {0x1400005a9f0, 0x16}, {0x14000a0cf88, 0x4}}, ...)
	/Users/distiller/project/project/internal/instances/expander.go:376 +0x64
github.com/hashicorp/terraform/internal/instances.(*expanderModule).resourceInstances(0x14001493740, {0x14000a2e6a0, 0x2, 0x3}, {{}, 0x4d, {0x1400005a9f0, 0x16}, {0x14000a0cf88, 0x4}}, ...)
	/Users/distiller/project/project/internal/instances/expander.go:385 +0x204
github.com/hashicorp/terraform/internal/instances.(*expanderModule).resourceInstances(0x140028ef590, {0x14000a2e680, 0x3, 0x4}, {{}, 0x4d, {0x1400005a9f0, 0x16}, {0x14000a0cf88, 0x4}}, ...)
	/Users/distiller/project/project/internal/instances/expander.go:385 +0x204
github.com/hashicorp/terraform/internal/instances.(*Expander).ExpandResource(0x14000fbc620, {{}, {0x14000a2e680, 0x3, 0x4}, {{}, 0x4d, {0x1400005a9f0, 0x16}, {0x14000a0cf88, ...}}})
	/Users/distiller/project/project/internal/instances/expander.go:157 +0x11c
github.com/hashicorp/terraform/internal/terraform.(*NodePlannableResourceInstanceOrphan).deleteActionReason(0x140020f8830, {0x102d19620, 0x1400210e380})
	/Users/distiller/project/project/internal/terraform/node_resource_plan_orphan.go:182 +0x2c0
github.com/hashicorp/terraform/internal/terraform.(*NodePlannableResourceInstanceOrphan).managedResourceExecute(0x140020f8830, {0x102d19620, 0x1400210e380})
	/Users/distiller/project/project/internal/terraform/node_resource_plan_orphan.go:140 +0x640
github.com/hashicorp/terraform/internal/terraform.(*NodePlannableResourceInstanceOrphan).Execute(0x140020f8830, {0x102d19620, 0x1400210e380}, 0x2)
	/Users/distiller/project/project/internal/terraform/node_resource_plan_orphan.go:49 +0x94
github.com/hashicorp/terraform/internal/terraform.(*ContextGraphWalker).Execute(0x14001830000, {0x102d19620, 0x1400210e380}, {0x12ba7ada0, 0x140020f8830})
	/Users/distiller/project/project/internal/terraform/graph_walk_context.go:133 +0x9c
github.com/hashicorp/terraform/internal/terraform.(*Graph).walk.func1({0x102be0300, 0x140020f8830})
	/Users/distiller/project/project/internal/terraform/graph.go:74 +0x244
github.com/hashicorp/terraform/internal/dag.(*Walker).walkVertex(0x14000c68840, {0x102be0300, 0x140020f8830}, 0x14003bda480)
	/Users/distiller/project/project/internal/dag/walk.go:381 +0x31c
created by github.com/hashicorp/terraform/internal/dag.(*Walker).Update
	/Users/distiller/project/project/internal/dag/walk.go:304 +0xe0c
	

It works without problem on terraform 1.0.7

In my case environment variable is a string

variable "environment" {
  type = string
}

[crw]

Hi @arekt, just FYI it may also be related to https://github.com/hashicorp/terraform/issues/30184 -- I mention this because the original report in this ticket did not mention a crash-to-stack-trace, which makes me think this is related to the new issue.