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

Boolean operators should be capable of converting unknown values to known #31078

Closed
Nuru opened this issue May 19, 2022 · 3 comments · Fixed by #33234 · May be fixed by #36224
Closed

Boolean operators should be capable of converting unknown values to known #31078

Nuru opened this issue May 19, 2022 · 3 comments · Fixed by #33234 · May be fixed by #36224
Labels
config enhancement unknown-values Issues related to Terraform's treatment of unknown values

Comments

@Nuru
Copy link

Nuru commented May 19, 2022

Boolean operators && and || should be capable of converting unknown values to known values based on standard boolean logic.

&& true false unknown
true true false unknown
false false false false
unknown unknown false unknown
|| true false unknown
true true true true
false true false unknown
unknown true unknown unknown

Terraform Version

1.1.7

Terraform Configuration Files

This is obviously contrived to show the issue, but in practice this sort of thing allows certain configurations to tolerate certain unknowns. For example, a configuration could require either a list of AWS Availability Zones (AZs) or the number of zones to use, with the final list of AZs being either the configured list or the list from a data source, and the final count being either the provided count or the size of the configured list of AZs. Currently this has to be carefully managed with the tertiary operator ? : but it should be able to be managed with straightforward boolean arithmetic.

variable "known" {
  type = bool
  default = false
}

resource "null_resource" "unknown" {
}

locals {
  computed = var.known && (length(null_resource.unknown.id) == 3)
  # The following statement avoid the problem:
  # computed = var.known ? length(null_resource.unknown.id) == 3 : false
}

resource "null_resource" "count" {
  count = local.computed ? 1 : 0
}

Expected Behavior

count should be known at plan time.

Actual Behavior

Error: Invalid count argument

Steps to Reproduce

  1. terraform init
  2. terraform apply
@Nuru Nuru added bug new new issue not yet triaged labels May 19, 2022
@apparentlymart
Copy link
Contributor

Hi @Nuru! Thanks for sharing this.

I think this is roughly the same as the existing issue #24128. Although that one was talking about short circuit in general (not evaluating the second operand at all if the first gives enough information), solving that one would contribute to solving this one in the case you showed where the known value appears first.

You are right to note though that the order of the operands doesn't matter when it comes to dealing with unknown values, so this is not entirely covered by the short circuit behavior and so for now I'll leave them both open with links between them in case that helps us to resolve one without the other. In practice though, I would suggest that anyone trying to solve one of them aim to solve them both.

So far we've not tried to make the Terraform language very "clever" with spotting special cases like this where one operand has a value that makes the other operand irrelevant to the result, though it does seem to be backward compatible to do so, since we typically consider replacing an unknown value with a compatible known value as an enhancement rather than as a regression.

The one concern I have is that it might make it easier to accidentally write a module that works with some input but fails (with "invalid for_each") on other input, and for the module author to not realize that until it is too late to fix without a breaking change. That is already possible, but typically today only with explicit conditionals that tend to make it clearer that there are two possibilities to test when verifying the module behavior. That doesn't mean that we should not do it, but it does suggest that we ought to prioritize improving Terraform's handling of unknown values (#30937) and/or have a more prescriptive solution for testing modules comprehensively in conjunction with making it easier to write expressions that might hide the possibility of a conditional unknown.

@apparentlymart apparentlymart added enhancement config and removed bug new new issue not yet triaged labels May 19, 2022
@Nuru
Copy link
Author

Nuru commented May 19, 2022

@apparentlymart While I agree with you that it is appropriate to link this issue to the short-circuit feature request, they are different and one is not a substitute for the other. The short-circuit case should cover not just unknown values, but known invalid values, too, such as an index past the end of the array, while this boolean operator issue should cover unknown values whether they come first or second in the expression.

The number of ways to fall into invalid for_each is breathtaking. I run into it all the time with optional inputs to modules, where some uses get statically configured inputs and some uses get dynamically generated inputs. (AWS Availability Zones for subnets for VPCs being the most recent example.) As I have mentioned elsewhere, I had the hardest time figuring out that one trigger for this problem was sort(). While I could imagine a tool that traces all the dependencies that go into the expression of a for_each clause, I wonder how useful the output would be. I just spent a day going through TF_LOG output looking for what was causing the Kubernetes provider to try to talk to localhost and it was brutal, and I can imagine the dependency output being somewhat the same.

Side note: this is what triggered this report. If the Kubernetes provider is fed an unknown value for host it considers that invalid and falls back to localhost. I could not force it to use example.com instead of local.x even with

host = local.x == null || local.x != null ? "example.com" : local.x

during terraform destroy because local.x was unknown at the time, but super hard to figure out since any output I tried, e.g. executing /bin/sh -c "echo '${local.x}' > /tmp/local-x" in a provider local-exec, would not show up in the log or anywhere else when local.x is unknown. I also note ironically that fixing both the shortcut issue and this boolean operation issue would still not resolve this situation as it is a different special case.

@Nuru Nuru changed the title Boolean operators should be capable of converting unknown values to known Boolean operators should be capable of converting unknown or null values to known Oct 16, 2022
@Nuru Nuru changed the title Boolean operators should be capable of converting unknown or null values to known Boolean operators should be capable of converting unknown values to known Oct 16, 2022
@apparentlymart apparentlymart added the unknown-values Issues related to Terraform's treatment of unknown values label Feb 7, 2023
@github-actions
Copy link
Contributor

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 Jun 24, 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
2 participants