hashicorp / terraform

Terraform enables you to safely and predictably create, change, and improve infrastructure. It is a source-available tool that codifies APIs into declarative configuration files that can be shared amongst team members, treated as code, edited, reviewed, and versioned.
https://www.terraform.io/
Other
42.54k stars 9.52k forks source link

Allow {} to be coerced to object of any type #33303

Open Nuru opened 1 year ago

Nuru commented 1 year ago

Terraform Version

Terraform v1.3.9
on darwin_amd64

Use Cases

I would like to be able to provide an empty object value that can be used in code rather than have to use conditionals in multiple places.

Attempted Solutions

What I would like to work:

This is, of course, a simplified example. In practice, the objects are more complex, and local.enabled is further refined in additional statements.

locals {
  coin_flip = false // random_integer.coin.result > 1

  # Simulate a data source with
  # count = local.coin_flip ? 1 : 0
  datasource = local.coin_flip ? null : {
    enabled = true
    obj = {
      disabled = {
        enabled = false
        list    = []
      }
      enabled = {
        enabled = true
      }
    }
  }
  obj = local.coin_flip ? {} : local.datasource.obj

  enabled = { for k, v in local.obj : k => v if v.enabled }
}

output "grr" {
  value = local.enabled
}

But it fails:

$ terraform apply
╷
│ Error: Inconsistent conditional result types
│ 
│   on objects.tf line 18, in locals:
│   18:   obj = local.coin_flip ? {} : local.datasource.obj
│     ├────────────────
│     │ local.coin_flip is false
│     │ local.datasource.obj is object with 2 attributes
│ 
│ The true and false result expressions must have consistent types. The 'false'
│ value includes object attribute "disabled", which is absent in the 'true'
│ value.
╵

What I have had to resort to:

  // Workaround for inconsistent types in ternary
  // obj = local.coin_flip ? {} : local.datasource.obj
  obj_map = {
    true  = {}
    false = local.coin_flip ? null : local.datasource.obj
  }

  obj = local.obj_map[local.coin_flip]

Proposal

Allow {} to be cast to map or any type of object.

References

apparentlymart commented 1 year ago

Hi @Nuru! Thanks for sharing this use-case.

Unfortunately I don't think exactly what you've proposed is an appropriate answer to the use-case: {} cannot be considered as a subtype of any object type that has attributes, because it doesn't have any attributes. Conversion to an object type with an attribute foo cannot meaningfully succeed unless it would be valid to access .foo on the resulting value.

In the case you've described, I would expect to use null as the representation of "no object", like this:

  obj = local.coin_flip ? null : local.datasource.obj

null is already specified as being convertible to any type, so it is always available as a way to represent the absence of something.

Alternatively, a list of zero or one elements if in a particular situation it's more convenient to think of this as being a bounded list rather than a single value that's either present or not:

  obj = local.coin_flip ? tolist([]) : tolist([local.datasource.obj])

Finally, if you are intending your local.datasource.obj to really be a map of objects rather than an object itself (unclear from the example) then you can help Terraform understand that by writing an explicit conversion to remove the ambiguity:

  obj = local.coin_flip ? tomap({}) : tomap(local.datasource.obj)

However, the given example for local.datasource.obj cannot convert to a map because the element types don't match. It would be necessary to add list = null or list = tolist([]) to the enabled element so that the result type can be map(object({enabled = bool, list = list(unknown)})). (The list element type is unknown here because neither of the objects have an example element to infer the element type from, but that's okay when the list is empty anyway since accessing an element of an empty list can never succeed regardless of type.)

Would any of these three solutions be suitable for what you are trying to achieve? If not, can you say more about why? Thanks!


Another possible way to interpret this request would be to have some way to request an explicit type conversion to an object type with optional attributes. Here's a hypothetical syntax for that which we've prototyped in the past:

# INVALID: This is just an example of a possible future syntax
convert({}, map(object({
  enabled = optional(bool, false)
  list    = optional(list(string), [])
})))

Unfortunately the only reason we haven't already implemented something like this is legacy constraints: this would be the first time that a type constraint could appear anywhere other than the type argument of a variable block and Terraform's dependency analyzer doesn't understand how to ignore it in this position and avoid e.g. misinterpreting bool as a resource type rather than a keyword. We hope to address that at some point, but are not currently prioritizing it.

If we were able to complete this then it would have the same behavior as passing a value through in input variable with the same type constraint.

(Note for future maintainers reading this: the HCL part of the convert function is still available in HCL's source code. What we haven't yet solved is how to integrate that into Terraform in a way that doesn't upset Terraform's own dependency analyzer, which HCL is not aware of. One candidate solution is to teach the expression analyzer that string, bool, number, and any are all valid symbols that it should ignore, but that then raises the question of how to properly handle the error of using those keywords in a non-type-expression context, which remains unsolved at the time I'm writing this.)

Nuru commented 1 year ago

@apparentlymart Thank you for your detailed response. Unfortunately, none of your suggestions help me.

I want some kind of value I can put in the conditional

obj = local.coin_flip ? <SOMETHING> : local.datasource.obj

that will ensure

enabled = { for k, v in local.obj : k => v if v.enabled }

works regardless of the coin flip. This is because in the actual code, there are close to 30 lines of additional transformations performed on enabled, and I do not want to have to have conditionals on every line. Unfortunately, null does not satisfy the need here.

I understand your comment about {} not being a subtype of object, but perhaps you could code it as a special case. I think of it more as an empty object, similar to [] being an empty list. I can write

obj = local.coin_flip ? [] : [local.datasource.obj]

and Terraform does not complain about the empty tuple as being a different type. To me it is a parallel construction to write

obj = local.coin_flip ? {} : local.datasource.obj

I would NOT expect

obj = local.coin_flip ? {foo = "bar"} : local.datasource.obj

to work, but like I said, I think {} should be a special case.

Side note: When the placeholder is a local (click to reveal) BTW, I understand the problem with the following (`placeholder` has a type, even though the value is `null`), but the error message is very confusing: ```hcl placeholder = local.coin_flip ? {} : null obj = local.coin_flip ? local.placeholder : local.datasource.obj ``` ``` │ Error: Inconsistent conditional result types │ │ on objects.tf line 20, in locals: │ 20: obj = local.coin_flip ? local.placeholder : local.datasource.obj │ ├──────────────── │ │ local.coin_flip is false │ │ local.datasource.obj is object with 2 attributes │ │ local.placeholder is null │ │ The true and false result expressions must have consistent types. The 'false' │ value includes object attribute "disabled", which is absent in the 'true' │ value. ```

In my particular case, I do not know the full type that will be returned by the data source, so I could not cast it into an object type even if that feature were available. It is not a map, so I cannot cast it to a map.

An alternative solution would be to allow null in a for statement, so that for k, v in null works like for k, v in {}, but that seems more drastic to me.

apparentlymart commented 1 year ago

In order for v.enabled to work as you showed in your first example, the following would need to be true:

Therefore to make that work would require Terraform to automatically set that attribute to either false or true, but Terraform doesn't have enough information to know which of those values would be appropriate here.

Therefore I don't think an automatic approach is appropriate here. You will always need to at least write out an enabled attribute that is either true or false.

With today's Terraform to meet your requirement I would define another local value that has the placeholder object to use to represent the absence of an object, and then use that in your expression:

locals {
  placeholder_something = {
    enabled = false
    list    = tolist([]) # or null, if that seems better
  }
  obj = local.coin_flip ? local.datasource.obj : local.placeholder_something
}

(I called this "placeholder something" just because the example we're discussing here is so contrived that I don't know what kind of thing local.datasource.obj represents to choose a better name.)

Alternatively, you can make the conditional produce null and then handle the fallback to the placeholder at the reference point instead:

enabled = { for k, v in local.obj : k => v if try(v.enabled, false) }

This is a more concise approach but it's also less precise, and would mask other errors such as v not being an object at all. But it seems like you are prioritizing writing less code over being explicit, so you might prefer this option.

Nuru commented 1 year ago

@apparentlymart wrote:

In order for v.enabled to work as you showed in your first example, the following would need to be true:

v has an attribute named enabled v.enabled is either true or false. (null would not work because if requires a non-null condition)

No, the code works fine if local.obj is an empty object or map. There are no keys, so for k, v in local.obj does not execute the generator statement.

I cannot flesh out placeholder_something because I do not know all the elements local.obj is going to have. I know it has some I am looking for (or else it is fine for that to be an error), but it has extraneous stuff that can vary.

I am looking for a more precise solution, otherwise I could solve this with try() at the expense of masking a lot of errors I would rather see.

apparentlymart commented 1 year ago

Thanks for the extra context, @Nuru. I thought you were intending v to be the empty object, not local.obj.

It isn't a goal of the Terraform language to help you write code that accepts arbitrary input regardless of type. We generally expect that you, as the module author, will decide exactly what type your module expects as input and return errors if the given input doesn't match that type.

That seems in direct conflict with your goal of having "extraneous stuff that can vary". I would suggest choosing a different approach where you design your API explicitly, and so that every expression has a well-defined result type regardless of input. Consider the Terraform language as having a static type system, not a dynamic type system.

Nuru commented 1 year ago

@apparentlymart This is not about an input I can declare and control, this is about data returned by a data source, which is beyond my control, much like Terraform allows for JSON input.

I think Hashicorp opened the door by allowing objects to be treated like maps in for statements (for which I am grateful). We have the empty list/tuple, which the ternary operator accepts as an alternative to a tuple of any type, and which for accepts as an empty list. I am just asking for something analogous for objects. Otherwise I have to jump through this indirection via an intermediate object, which both shows Terraform can handle the situation internally and how awkward it is currently.

hp-andrewk commented 5 months ago

There is an inconsistency here which, regardless of the previous comments, needs to be considered and possibly addressed.

Consider a module with a variable defined thus:

variable "anything" {
  type        = any
  default     = {}
}

Note the type and the default. Now I want to be able to define 'anything' to have some value or not according to a flag:

anything = var.enable_anything ? {<complex object defn>} : {}

Nope, not allowed. Ok, let me try this:

anything = var.enable_anything ? {<complex object defn>} : null

Also not allowed. Fair enough, the module that consumes 'anything' doesn't expect 'null', that's why it defines a default of {} (which has a type of 'any', right?). But, in my first attempt, am I not providing the same default value on the rhs of ternary expression?

To my mind, if the variable defines a default value/type and the same default value/type is explicitly provided, there should be no issue whatsoever - Terraform should accept that because they are clearly identical, semantically. I suspect this would be the case if it wasn't for the use of the ternary operator. Ergo, the issue here is the ternary operator - it is making an optimistic evaluation which, in my view, it shouldn't and possibly doesn't need to do. Why not simply pass the appropriate value through and allow the consumer to determine whether it's valid or not?

shaneholder commented 1 week ago

I hit something similar to this today but possibly even more odd. Note that the result in both cases is returning an object with 2 properties, but I am finding that if the types of one of the properties is an object and the other properties are not objects then I get the inconsistent type error.

terraform {
  backend "local" {}
}

variable amitrue {
  type = bool
}

locals {
  result = var.amitrue ? {x = {}, y: ""} : {} # Fails
  #result = var.amitrue ? {x = "1", y: 2} : {} # Works
  #result = var.amitrue ? {x = {}, y: {}} : {} # Works
}

output "result" {
  value = local.result
}

Using the above I get an error:

 $ terraform plan -var="amitrue=true"
╷
│ Error: Inconsistent conditional result types
│ 
│   on main.tf line 10, in locals:
│   10:   result = var.amitrue ? {x = {}, y: ""} : {} # Fails
│     ├────────────────
│     │ var.amitrue is a bool
│ 
│ The true and false result expressions must have consistent types. The 'true' value includes object attribute "x", which is absent in the 'false' value.

However, if I comment out the Fails line and uncomment the Works line I get the below.

$ terraform plan -var="amitrue=true"

Changes to Outputs:
  + result = {
      + x = {}
      + y = {}
    }

You can apply this plan to save these new output values to the Terraform state, without changing any real infrastructure.
$ /usr/bin/terraform --version
Terraform v1.9.7
on linux_amd64

Something feels off.