The true and false result expressions must have consistent types. The 'true' value is set of dynamic, but the 'false' value is object.

[FernandoMiguel]

$ terraform apply
╷
│ Warning: Experimental feature "module_variable_optional_attrs" is active
│
│   on .terraform/modules/base_system.eks_blueprints_kubernetes_addons_karpenter.ondat/locals.tf line 2, in terraform:
│    2:   experiments = [module_variable_optional_attrs]
│
│ Experimental features are subject to breaking changes in future minor or patch releases, based on feedback.
│
│ If you have feedback on the design of this feature, please open a GitHub issue to discuss it.
│
│ (and 63 more similar warnings elsewhere)
╵
╷
│ Warning: Redundant ignore_changes element
│
│   on .terraform/modules/base_system/eks_addons.tf line 62, in resource "aws_eks_addon" "aws_eks_addons":
│   62: resource "aws_eks_addon" "aws_eks_addons" {
│
│ Adding an attribute name to ignore_changes tells Terraform to ignore future changes to the argument in configuration after the object has been created, retaining the value originally configured.
│
│ The attribute modified_at is decided by the provider alone and therefore there can be no configured value to compare with. Including this attribute in ignore_changes has no effect. Remove the attribute from ignore_changes to quiet this warning.
╵
╷
│ Error: Inconsistent conditional result types
│
│   on .terraform/modules/base_system/otel-fargate-container-insights.tf line 15, in resource "kubectl_manifest" "otel_fargate_container_insights":
│   15:   for_each  = var.fargate_create == true && var.otel_fargate_container_insights_enabled == true ? toset(data.kubectl_path_documents.otel_fargate_container_insights[0].documents) : {}
│     ├────────────────
│     │ data.kubectl_path_documents.otel_fargate_container_insights[0].documents will be known only after apply
│     │ var.fargate_create is a bool, known only after apply
│     │ var.otel_fargate_container_insights_enabled is a bool, known only after apply
│
│ The true and false result expressions must have consistent types. The 'true' value is set of dynamic, but the 'false' value is object.
╵

Terraform Version

$ terraform -v
Terraform v1.2.0-rc2
on darwin_arm64
+ provider [registry.terraform.io/gavinbunney/kubectl](http://registry.terraform.io/gavinbunney/kubectl) v1.14.0
+ provider [registry.terraform.io/hashicorp/aws](http://registry.terraform.io/hashicorp/aws) v4.13.0
+ provider [registry.terraform.io/hashicorp/cloudinit](http://registry.terraform.io/hashicorp/cloudinit) v2.2.0
+ provider [registry.terraform.io/hashicorp/helm](http://registry.terraform.io/hashicorp/helm) v2.5.1
+ provider [registry.terraform.io/hashicorp/kubernetes](http://registry.terraform.io/hashicorp/kubernetes) v2.11.0
+ provider [registry.terraform.io/hashicorp/local](http://registry.terraform.io/hashicorp/local) v2.2.2
+ provider [registry.terraform.io/hashicorp/null](http://registry.terraform.io/hashicorp/null) v3.1.1
+ provider [registry.terraform.io/hashicorp/random](http://registry.terraform.io/hashicorp/random) v3.1.3
+ provider [registry.terraform.io/hashicorp/time](http://registry.terraform.io/hashicorp/time) v0.7.2
+ provider registry.terraform.io/hashicorp/vault v3.5.0

Terraform Configuration Files

data "kubectl_path_documents" "otel_fargate_container_insights" {
  count   = var.fargate_create == true && var.otel_fargate_container_insights_enabled == true ? 1 : 0
  pattern = "${path.module}/charts/otel-fargate-container-insights/*.yaml"
  vars = {
    ClusterName        = data.aws_eks_cluster.this.id
    namespace          = var.otel_fargate_container_insights_namespace
    ServiceAccountName = var.otel_fargate_container_insights_service_account_name
    region             = var.aws_region
  }
}

resource "kubectl_manifest" "otel_fargate_container_insights" {
  for_each  = var.fargate_create == true && var.otel_fargate_container_insights_enabled == true ? toset(data.kubectl_path_documents.otel_fargate_container_insights[0].documents) : {}
  yaml_body = each.value

  depends_on = [
    aws_iam_role.otel_irsa,
    kubernetes_namespace.otel_fargate_container_insights_namespace,
    kubernetes_service_account.eks_service_account_otel_fargate_container_insights,
    aws_cloudwatch_log_group.containerinsights,
  ]
}

works fine with

$ terraform -v
Terraform v1.1.9
on darwin_arm64
+ provider registry.terraform.io/gavinbunney/kubectl v1.14.0
+ provider registry.terraform.io/hashicorp/aws v4.13.0
+ provider registry.terraform.io/hashicorp/cloudinit v2.2.0
+ provider registry.terraform.io/hashicorp/helm v2.5.1
+ provider registry.terraform.io/hashicorp/kubernetes v2.11.0
+ provider registry.terraform.io/hashicorp/local v2.2.2
+ provider registry.terraform.io/hashicorp/null v3.1.1
+ provider registry.terraform.io/hashicorp/random v3.1.3
+ provider registry.terraform.io/hashicorp/time v0.7.2
+ provider registry.terraform.io/hashicorp/vault v3.5.0

[apparentlymart]

Thanks for reporting this, @FernandoMiguel!

At first glance this seems like a correct error: your conditional expression is indeed defined to produce a set in one case but an empty object in another, and those types are not compatible.

Therefore I think our first step here would be to understand why this didn't return an error in v1.1.9. It suggests that there was some sort of bug in that previous version that is now fixed, so I think we will need to understand what that was to see what is a reasonable path forward here. We'll take a closer look at this soon.

Thanks again!

[apparentlymart]

This one was a pretty interesting one to create a minimal reproduction for, because it seems that it's crucial that the set itself be unknown and of unknown type, leading to what this error message clumsily refers to as a "set of dynamic".

Therefore my reproduction case is pretty contrived and relies on some tricks to synthesize a similar result without relying on a Kubernetes server:

resource "null_resource" "example" {
}

locals {
  list = null_resource.example.id[*]
  set  = toset(local.list)
  pred = null_resource.example.id == "well well well"

  maybe = local.pred ? local.set : {}
}

Some notes about what's going on here:

• During initial planning, null_resource.example.id is an unknown value of type string.
• null_resource.example.id == "well well well" returns an unknown value of type bool, because we can see that the ID is a string but we cannot yet determine whether it equals "well well well".
• null_resource.example.id[*] returns an unknown value of an unknown type, because the splat operator needs to know whether the ID will be null before it can determine what its result type will be.
• toset(local.list) produces an unknown set of an unknown type due to being derived from an unknown value of an unknown type. (Terraform optimistically assumes that the type conversion will succeed during planning, under our usual rule that during apply time all values must either be compatible with the assumptions we made or return an error explaining why not).

With all of that said then, this sufficiently recreates the relevant situation to produce an equivalent error on Terraform v1.2.0-rc2:

╷
│ Error: Inconsistent conditional result types
│ 
│   on conditional-set-and-object.tf line 9, in locals:
│    9:   maybe = local.pred ? local.set : {}
│     ├────────────────
│     │ local.pred is a bool, known only after apply
│     │ local.set is a set of dynamic, known only after apply
│ 
│ The true and false result expressions must have consistent types. The
│ 'true' value is set of dynamic, but the 'false' value is object.
╵

On Terraform v1.1.9 it succeeds during planning and during the subsequent apply step:

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:

  # null_resource.example will be created
  + resource "null_resource" "example" {
      + id = (known after apply)
    }

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

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

null_resource.example: Creating...
null_resource.example: Creation complete after 0s [id=2603811441306929907]

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

We do typically consider it an improvement if an error that was previously detected only during apply were now detected during plan instead due to Terraform's analysis being more precise, but this one is concerning because somehow Terraform v1.1.9 wasn't even detecting this problem during the apply step, at which point Terraform should be able to clearly see that the true and false arms of the conditional are of incompatible types.

I don't yet know why Terraform v1.1.9 wasn't detecting this error, but hopefully the simpler reproduction case above will make it easier to dig in and get to the bottom of it.

[apparentlymart]

One further thing I noticed is that even though the first terraform apply somehow ran to completion without errors with Terraform v1.1.9, if I immediately run terraform plan with Terraform v1.1.9 afterwards (so that there's now a prior state with null_resource.example in it) then it does fail as I would've expected:

null_resource.example: Refreshing state... [id=9049847589891101533]
╷
│ Error: Inconsistent conditional result types
│ 
│   on conditional-set-and-object.tf line 9, in locals:
│    9:   maybe = local.pred ? local.set : {}
│     ├────────────────
│     │ local.pred is false
│     │ local.set is set of string with 1 element
│ 
│ The true and false result expressions must have consistent types. The
│ given expressions are set of string and object, respectively.
╵

(Notice that this is the old-style "Inconsistent conditional result types" message, where says "are set of string and object, respectively" instead of "The 'true' value is set of string, but the false value is object" as v1.2 would've produced.)

It is not clear to me at the moment why terraform apply failed to catch this error even though the subsequent plan caught it, given that the evaluation of the local value during apply ought to have been working with the same final state of null_resource.example as the subsequent plan is. 🤔

(The "Refreshing state..." message did cause me to wonder if the refresh step was the key here, but sadly there's no difference in behavior when I do the subsequent plan with -refresh=false.)

[alisdair]

The change which resulted in this new behaviour is in #30879, where we corrected the conversion functions to cope with values of unknown type. This means that calling toset(…) with arguments of unknown type will now correctly return a set(dynamic) value, rather than the more general dynamic.

I've verified this by cherry-picking the commit from #30879 onto v1.1, which results in the same behaviour as the 1.2 branch.

To achieve the goal I think you're aiming for, the for_each expression here should be updated:

  for_each  = (
    var.fargate_create == true && var.otel_fargate_container_insights_enabled == true ?
    toset(data.kubectl_path_documents.otel_fargate_container_insights[0].documents) : 
    toset([])
  )

That is, changing {} to toset([]). This should work as expected on both the 1.1 and 1.2 releases.

Based on our reproduction, we don't currently believe this is breaking previously-valid behaviour, as while the first apply succeeds, a subsequent plan will (correctly) error. If this is not the case, and you are relying on the previously buggy behaviour, please let us know more and we'll think further about how to move forward.

Thanks again for testing the release candidate and reporting the issue!

[apparentlymart]

From looking closely at the TRACE logs for the apply step, I can see why terraform apply didn't catch this in v1.1.9:

2022-05-12T11:51:32.980-0700 [DEBUG] pruneUnusedNodes: local.maybe (expand) is no longer needed, removing
2022-05-12T11:51:32.980-0700 [DEBUG] pruneUnusedNodes: local.set (expand) is no longer needed, removing
2022-05-12T11:51:32.980-0700 [DEBUG] pruneUnusedNodes: local.pred (expand) is no longer needed, removing
2022-05-12T11:51:32.980-0700 [DEBUG] pruneUnusedNodes: local.list (expand) is no longer needed, removing

Since I used local values for my reproduction, that made them subject to pruning during the apply step and therefore Terraform didn't evaluate them at all. I expect the same would not apply to a resource for_each argument, because those lead to a side-effect and so cannot be safely pruned in this way.

(It's interesting to note that this pruning does appear to allow invalid local values to go undetected as long as they remain unknown until the apply step, which is concerning now that I see it, but is a different concern from what this issue is discussing. An unknown value that never gets used is an edge case anyway, so it's not super concerning but still something we might want to revisit and consider.)

[apparentlymart]

I did try to reproduce this with for_each instead of a local value but ended up blocked by the usual error that for_each itself cannot be unknown:

╷
│ Error: Invalid for_each argument
│ 
│   on conditional-set-and-object.tf line 14, in resource "null_resource" "another":
│   14:   for_each = local.maybe
│     ├────────────────
│     │ local.maybe will be known only after apply
│ 
│ The "for_each" value depends on resource attributes that cannot be
│ determined until apply, so Terraform cannot predict how many instances
│ will be created. To work around this, use the -target argument to first
│ apply only the resources that the for_each depends on.
╵

I find myself unsure about how to write a reproduction that involves for_each here. Even in the original report we can see that the conditional predicate was based on unknown values:

│   on .terraform/modules/base_system/otel-fargate-container-insights.tf line 15, in resource "kubectl_manifest" "otel_fargate_container_insights":
│   15:   for_each  = var.fargate_create == true && var.otel_fargate_container_insights_enabled == true ? toset(data.kubectl_path_documents.otel_fargate_container_insights[0].documents) : {}
│     ├────────────────
│     │ data.kubectl_path_documents.otel_fargate_container_insights[0].documents will be known only after apply
│     │ var.fargate_create is a bool, known only after apply
│     │ var.otel_fargate_container_insights_enabled is a bool, known only after apply

...and so it seems like if the condition hadn't failed type checking then the for_each ought to have failed immediately anyway, because the result of a conditional with an unknown predicate is an unknown value of the conditional result type. (This is, incidentally, why Terraform requires the true and false result types to be the same.)

@FernandoMiguel, when you tried this with Terraform v1.1.9, did you do so in the same context as you tried v1.2.0-rc2? In particular, I'm wondering if you tried v1.2.0-rc2 in a situation where there was no prior state but you tried Terraform v1.1.9 in a situation where there was already a prior state, and therefore Terraform was able to decide a known answer for var.fargate_create == true && var.otel_fargate_container_insights_enabled == true in that case.

[FernandoMiguel]

@FernandoMiguel, when you tried this with Terraform v1.1.9, did you do so in the same context as you tried v1.2.0-rc2? In particular, I'm wondering if you tried v1.2.0-rc2 in a situation where there was no prior state but you tried Terraform v1.1.9 in a situation where there was already a prior state, and therefore Terraform was able to decide a known answer for var.fargate_create == true && var.otel_fargate_container_insights_enabled == true in that case.

all of my plans/applies were of real code with already existing states, and just by switching tfenv version to try to debug the issue between versions

[FernandoMiguel]

After talking with @apparentlymart in slack , I ended up going with
for_each = toset(var.karpenter-enabled == true ? toset(data.kubectl_path_documents.karpenter_provisioners[0].documents) : [])
which feels far more readable than doing
: toset([])

Still, I expect this change of behaviour to impact in some manner a large set of codebases that did not cast type prior.

although looking at https://github.com/search?l=&p=3&q=for_each++%3D+var.+%3A+%7B%7D+language%3AHCL+-dynamic+-toset&ref=advsearch&type=Code , that may not prove true my statement... oh well