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

Provide a count-like mechanism for variable data that only checks against null. #30816

Closed
theherk opened this issue Apr 7, 2022 · 7 comments
Closed

Comments

@theherk
Copy link
Contributor

theherk commented Apr 7, 2022

Current Terraform Version

➜ terraform version
Terraform v1.1.7
on darwin_amd64
+ provider registry.terraform.io/hashicorp/aws v4.8.0

Use-cases

There are many situations where variable data contains values that can't be known until apply-time. Reasonably in these cases, one can't rely on this data. For example, often we pass a policy document as a variable whose default is null. If the policy is not null, we need to create the policy resource. That can be done with count = var.policy != null ? 1 : 0. However, if that contains data that cannot be determined until apply time, you get the error with which many of us have wrestled: The "count" value depends on resource attributes that cannot be determined until apply.

There are cases where doesn't make sense though. Even if a value in that policy is unknown before applying, it is still crystal clear that the variable is not null.

What we need is a simple mechanism like:

count = var.policy != null ? 1 : 0
# or
for_each = var.policy != null ? [1] : []

but that doesn't choke if there are undetermined values in the variable. There is no question here if this value is null of not, so there should be no issue determining that the count should be precisely one.

Attempted Solutions

There is a workaround in a small few cases, where you can create an "empty" resource (empty policy for example) that can be used where you don't use count, always create the resource, but select which policy (var.policy or empty policy) is actually used for the resource. That is not sufficient though.

Proposal

I'd suggest either improving the count feature to succeed in minimal null checks or add an additional feature which can simply detect the presence of a value without considering its contents.

References

These are a few examples, but you find many on stackoverflow.com and scattered around. Again, there are many cases where this behavior is perfectly sensible, but in some cases like checking against null, the contents really do not matter.

Minimal Example

Consider the following:

data "aws_iam_policy_document" "this" {
  statement {
    actions   = ["s3:PutObject"]
    resources = ["${aws_s3_bucket.this.arn}/*"]

    principals {
      type = "AWS"
      identifiers = [
        "arn:aws:iam::156460612806:root", # eu-west-1
        "arn:aws:iam::897822967062:root", # eu-north-1
      ]
    }
  }
}

resource "aws_s3_bucket" "this" {
  bucket = "test-h4s-bucket"
}

resource "aws_s3_bucket_policy" "this" {
  count = data.aws_iam_policy_document.this.json != null ? 1 : 0
  # for_each = data.aws_iam_policy_document.this.json != null ? [1] : []

  bucket = aws_s3_bucket.this.id
  policy = data.aws_iam_policy_document.this.json
}

In this scenario, the fact that the policy contains a value (arn) that can't be determined until apply is irrelevant in determining if the policy is null or not. Are there other ways to deal with this that I am overlooking? If not, I hope this can be improved somehow.

@theherk theherk added enhancement new new issue not yet triaged labels Apr 7, 2022
@jbardin
Copy link
Member

jbardin commented Apr 7, 2022

Hi @theherk,

Thanks for the enhancement idea. The techniques for dealing with unknown values vary by circumstances, but the more flexible expansion method of for_each is a common way to deal with unknown values, putting the unknown value in the map and using a known key for planning the instance.

Taking your example though, if you are certain in the configuration that data.aws_iam_policy_document.this.json cannot be null, then the comparison to null is unwarranted and the deciding factor for creating the instance should come from elsewhere. Since the output of data.aws_iam_policy_document cannot be null without raising an error (at least that is the common policy for singular data sources for this very reason), it looks like there should be no reason for the conditional in count at all.

If the requirement is to ensure that a value might be unknown, but guaranteed to be non-null, modules can use non-nullable inputs. Extending this idea to resource schemas is covered in #26755, along with similar advice.

Fundamentally terraform must know which instances it's planning, which is why count and for_each keys must be known during the plan. As far as allowing progressive application of configuration goes, we have issue #4149 to track that idea. It's not something that can happen with a single walk. through the configuration, but may evolve into a new workflow for this type of problem.

Since these ideas are already covered in-depth in other issues, I'm going to close this one out. If you have more questions, feel free to use the community forum where there are more people ready to help.

Thanks!

@jbardin jbardin closed this as completed Apr 7, 2022
@theherk
Copy link
Contributor Author

theherk commented Apr 7, 2022

I don't think my contrived example clarified the issue. There are cases where a policy document is a variable, that should be nullable. If it is not passed in the policy resource should not be created. If it is passed in the resource should be created with zero concern for the values contained therein. As in, the number is known to be exactly one in that case, but that fails with an error claiming the count cannot be known.

The requirement is simply that count should be able to compare null vs not null regardless of the computability of the values within the non-null value.

I'm trying to wrap my head around your for_each suggestion.

@theherk
Copy link
Contributor Author

theherk commented Apr 7, 2022

The question has been asked as recommended in the community forum, here. I do hope somebody will take the time to clarify this, as it really doesn't make sense, and I've seen people struggle with it for years, precisely because it makes so little sense.

@jbardin
Copy link
Member

jbardin commented Apr 7, 2022

If a nullable value is passed in in some way, then that is what should be compared for the count value, not the output of the data source. That does still leave the edge case of dealing with "unknown-but-not-null", which we don't have a universal method for dealing with yet. We do have the linked issue 26755 for tracking ideas there.

To help clarify, count doesn't do anything with the expression, it is simply assigned an expression which evaluates to a number. It's a seemingly minor detail, but important for how to formulate a solution. In order to complete the plan, that number cannot be unknown. Terraform needs to know what to plan, so we need a way to formulate an expression which evaluates to a number even when part of that expression is still unknown. In most cases that is accomplished by restructuring the configuration to compare a value higher up in the hierarchy which can determine if the resource will be created or not (it must be defined somewhere in the configuration if we expect the plan to be able to succeed at all). We can explore the non-nullable value idea in 26755, and for instances where even that would not be possible, multiple configurations can be used pending some new workflow like in 4149.

@theherk
Copy link
Contributor Author

theherk commented Apr 7, 2022

Okay, excellent. I didn't find #26755 earlier. I have added it to the references. Thank you for taking the time.

@theherk
Copy link
Contributor Author

theherk commented Apr 7, 2022

For posterity, I think I have found a workaround; kludge but seems to work. This moves the "unknown-but-not-null" value to the value of the for expression.

My workaround was a lie, so I'm editing with the working solution given by @apparentlymart. Take the variable wrapped in an object:

variable "policy_obj" {
  type = object({
    json = string
  })
  default = null
}

resource "some_resource" "x" {
  count = var.policy != null ? 1 : 0

  policy = var.policy.json
}

@crw crw removed the new new issue not yet triaged label Apr 7, 2022
@github-actions
Copy link
Contributor

github-actions bot commented May 8, 2022

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 May 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants