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

In an unknown choice of 1 of 2 lists, if the lists are the same length, the list length should be known #33237

Closed
Nuru opened this issue May 22, 2023 · 3 comments · Fixed by #33294
Labels
config enhancement unknown-values Issues related to Terraform's treatment of unknown values

Comments

@Nuru
Copy link

Nuru commented May 22, 2023

Terraform Version

Terraform v1.4.6
on darwin_amd64
+ provider registry.terraform.io/hashicorp/null v3.2.1
+ provider registry.terraform.io/hashicorp/random v3.5.1

Terraform Configuration Files

Of course this is very pared down from useful code:

bug.tf
variable "bug" {
  type = bool
  description = "set to true to trigger the bug"
  default = true
}

resource "random_integer" "coin" {
  max   = 2
  min   = 1
}

locals {
  coin_flip = random_integer.coin.result > 1

  rules = [
    {
      list        = (var.bug ? local.coin_flip : true) ? [null] : [null]
      description = "example description"
    }
  ]

  # Use concat to flatten the list of lists into a single list
  # instead of flatten because flatten depends on values.
  expanded_rules = concat(concat([[]], [for n, rule in local.rules : [for i, v in rule.list : {
    key         = "exp.${n}.${i}" 
    description = rule.description
    item        = "known" // In useful code, this would be `v`
  }]])...)

  keyed_rules = { for rule in local.expanded_rules : rule.key => rule }
}

resource "null_resource" "count" {
  count = length(local.keyed_rules)
  triggers = {
    description = values(local.keyed_rules)[count.index].description
  }
}

resource "null_resource" "for_each" {
  for_each = local.keyed_rules
  triggers = {
    description = each.value.description
  }
}

Debug Output

n/a

Expected Behavior

Terraform should be able to plan

Actual Behavior

The dreaded count and for_each errors:

Click to see Error Message
│ Error: Invalid count argument
│ 
│   on bug.tf line 32, in resource "null_resource" "count":
│   32:   count = length(local.keyed_rules)
│ 
│ The "count" 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 count depends on.
╵
╷
│ Error: Invalid for_each argument
│ 
│   on bug.tf line 39, in resource "null_resource" "for_each":
│   39:   for_each = local.keyed_rules
│     ├────────────────
│     │ local.keyed_rules will be known only after apply
│ 
│ The "for_each" map includes keys derived from resource attributes that cannot
│ be determined until apply, and so Terraform cannot determine the full set of
│ keys that will identify the instances of this resource.

The unknown value of local.rules[0].list should not make it a list of unknown length, because both options are the same length, but it does.

Steps to Reproduce

  1. terraform init
  2. terraform plan -var bug=false # succeeds
  3. terraform plan -var bug=true # fails

Additional Context

This is part of the ongoing saga of not being able to use count or for_each because of derived values not known at plan time.

References

Originally posted by @apparentlymart in #26755 (comment)

The known-ness of an attribute value is independent of the known-ness of the object value it's contained within.

At first I thought that the unknown value of local.rules[0].list was making the other attributes in the object unknown. However, replacing

      list        = (var.bug ? local.coin_flip : true) ? [null] : [null]

with

      list        = [(var.bug ? local.coin_flip : true) ? null : null]

allows the plan to succeed, which indicates the problem is with the length of the list.

@Nuru Nuru added bug new new issue not yet triaged labels May 22, 2023
@apparentlymart apparentlymart added the unknown-values Issues related to Terraform's treatment of unknown values label May 22, 2023
@apparentlymart
Copy link
Contributor

Thanks for reporting this, @Nuru!

Terraform is behaving as designed here -- the documentation never promised that the conditional operator would produce anything other than an unknown value when the predicate is itself unknown -- so I'm going to relabel this as an enhancement to reflect that supporting what you've described requires changing Terraform's design to treat unknown values differently and make additional promises about them.

However, it's good timing that today I opened #33234 which introduces the initial building blocks required to solve this: the possibility for an unknown list to have bounds on its length, including possibly having its lower bound and upper bound equal which then allows it to become a known list with all elements unknown instead.

That PR doesn't include a change to the conditional operator to achieve exactly what you called for here, but I mentioned in that PR that I have a commit for HCL to teach its various operators to refine their outputs, and I already implemented a rule in that commit so that if a conditional operator is choosing between two different values of the same collection type then it'll refine an unknown result to have length bounds set to the range between the true and the false expression. If the true and false expression are both known to have the same length then that will allow the result to become a known list with all elements unknown, just as you described here.

(In order for this to work out as I described you'd need to make sure your condition actually returns a list type and not a tuple type, so it might be necessary to add one or to tolist calls to force that conversion; we'll see once the changes are in place and we can try real examples like yours.)

With that said then: I think we already know what needs to happen to make Terraform behave as you described, but it's a pretty big set of changes so we'll need to review and merge them carefully. Once we get to the point where I'm opening a PR to introduce the HCL-side changes into Terraform I'll mark that PR as closing this issue.

Thanks again!

@apparentlymart
Copy link
Contributor

apparentlymart commented May 31, 2023

I've now opened #33294 with the HCL update I mentioned.

As I suspected before, it was necessary to explicitly tell Terraform that the conditional expression should return a list in order to activate the new behavior, so I've modified the conditional to use tolist but otherwise used the original example as given:

variable "bug" {
  type = bool
  description = "set to true to trigger the bug"
  default = true
}

resource "random_integer" "coin" {
  max   = 2
  min   = 1
}

locals {
  coin_flip = random_integer.coin.result > 1

  rules = [
    {
      list        = (var.bug ? local.coin_flip : true) ? tolist([null]) : tolist([null])
      description = "example description"
    }
  ]

  # Use concat to flatten the list of lists into a single list
  # instead of flatten because flatten depends on values.
  expanded_rules = concat(concat([[]], [for n, rule in local.rules : [for i, v in rule.list : {
    key         = "exp.${n}.${i}" 
    description = rule.description
    item        = "known" // In useful code, this would be `v`
  }]])...)

  keyed_rules = { for rule in local.expanded_rules : rule.key => rule }
}

resource "null_resource" "count" {
  count = length(local.keyed_rules)
  triggers = {
    description = values(local.keyed_rules)[count.index].description
  }
}

resource "null_resource" "for_each" {
  for_each = local.keyed_rules
  triggers = {
    description = each.value.description
  }
}

Without that change, we can see the unknown value error as reported:

$ terraform plan -var bug=true

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

Terraform planned the following actions, but then encountered a problem:

  # random_integer.coin will be created
  + resource "random_integer" "coin" {
      + id     = (known after apply)
      + max    = 2
      + min    = 1
      + result = (known after apply)
    }

Plan: 1 to add, 0 to change, 0 to destroy.
╷
│ Error: Invalid count argument
│ 
│   on conditional-lists-of-same-length.tf line 34, in resource "null_resource" "count":
│   34:   count = length(local.keyed_rules)
│ 
│ The "count" 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 count depends on.
╵
╷
│ Error: Invalid for_each argument
│ 
│   on conditional-lists-of-same-length.tf line 41, in resource "null_resource" "for_each":
│   41:   for_each = local.keyed_rules
│     ├────────────────
│     │ local.keyed_rules will be known only after apply
│ 
│ The "for_each" map includes keys derived from resource attributes
│ that cannot be determined until apply, and so Terraform cannot
│ determine the full set of keys that will identify the instances of
│ this resource.
│ 
│ When working with unknown values in for_each, it's better to define
│ the map keys statically in your configuration and place apply-time
│ results only in the map values.
│ 
│ Alternatively, you could use the -target planning option to first
│ apply only the resources that the for_each value depends on, and then
│ apply a second time to fully converge.
╵

With the new HCL changes included, it successfully completes planning with a known list whose elements are all unknown:

$ terraform plan -var bug=true

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.count[0] will be created
  + resource "null_resource" "count" {
      + id       = (known after apply)
      + triggers = {
          + "description" = "example description"
        }
    }

  # null_resource.for_each["exp.0.0"] will be created
  + resource "null_resource" "for_each" {
      + id       = (known after apply)
      + triggers = {
          + "description" = "example description"
        }
    }

  # random_integer.coin will be created
  + resource "random_integer" "coin" {
      + id     = (known after apply)
      + max    = 2
      + min    = 1
      + result = (known after apply)
    }

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

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

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.

I expect that more improvements in this area will follow later on, now that we have this new "refinements" primitive to work with, but for now there's at least one way to make this work as described.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 1, 2023

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 Jul 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
config enhancement unknown-values Issues related to Terraform's treatment of unknown values
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants