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
41.99k stars 9.46k forks source link

Allow better `lookup` default value for map of objects with optional attributes #35027

Open bholzer opened 4 months ago

bholzer commented 4 months ago

Terraform Version

1.7.3

Use Cases

I would like to provide a sane default lookup value for a map of objects with optional values.

My current specific use case:

I have a module that creates some AWS eventbridge schedules. I have a variable that I am using to allow consumers to override values of default schedules while at the same time providing custom schedules of their own.

Here's what it looks like right now:

variable "schedules" {
  description = <<-DESC
    EventBridge schedule overrides for the application.
    These values will be merged with the default schedules or can create new schedules if no matching key is found.
  DESC
  type = map(object({
    schedule_expression = optional(string)
    description = optional(string)
    enabled = optional(bool, true)
    command = optional(list(string))
    cpu = optional(number)
    memory = optional(number)
  }))
  default = {}
}

locals {
  default_schedules = {
    schedule1 = {
      schedule_expression = "cron(0 8 ? * 3 *)" # Every Tuesday at 8:00 AM
      description = "Schedule 1"
      enabled = true
      command = ["command1"]
    }
    schedule2 = {
      schedule_expression = "cron(0/15 * ? * * *)" # Every 15 minutes
      description = "Schedule 2"
      enabled = true
      command = ["command2"]
    }
  }

  # Merge default schedules with var.schedules
  # If matching default schedule does not exist, create the provided schedule
  schedules = {
    for name in setunion(keys(var.schedules), keys(local.default_schedules)) :
      name => merge(
        try(local.default_schedules[name], {}),
        { for k, v in try(var.schedules[name], {}): k => v if v != null } # Remove null values that result from optional object values
      )
  }
}

I'm doing this to provide a flexible and easily configurable abstraction

Attempted Solutions

In my use case, note my usage of try(local.default_schedules[name], {})

I'm doing this because lookup(local.default_schedules, name, {}) does not work. I get an error: the default value must have the same type as the map elements

try seems like a suboptimal solution here because I don't want to swallow other unspecified errors unexpectedly or anything like that.

The solution suggested in the referenced issue also feels really unsatisfying. Fully specifying the entire object with null values is not something I'm going to do in this case.

Proposal

No response

References

This has been discussed a bit before, and it's said that this currently behaves as designed. That may be the case, but if so, I think the design should be changed to support a reasonable alternative.

https://github.com/hashicorp/terraform/issues/32417

apparentlymart commented 4 months ago

Hi @bholzer,

When I consider your whole writeup here, I understand the underlying need as something like: I'm trying to generate a value for a remote system that requires optional properties to be omitted when unset, rather than explicitly set to null, and it's hard to get from Terraform's idea of optional attributes to what the remote system expects.

Do you think that's a valid interpretation of the problem you've encountered?

I ask because when thinking about that topmost goal a more direct solution comes to my mind: a function that takes an arbitrary value of any type, visits all of the nested values if any, and for any object type or map type it removes any attribute/element whose value is null.

Do you think that would be a viable replacement for the complicated expression you are trying to write in that local value?


If we did want to offer such a function there are some tricky details to handle, but I think all solvable. Some initial thoughts:

This function therefore seems subtle enough that it deserves to be a built-in, and I've heard about this use-case of removing null values several times before so I think it's also common enough to justify being a built-in even if it were not so subtle.

bholzer commented 4 months ago

Thanks for taking the the time to take a look at this @apparentlymart. I realize this is somewhat unclear, sorry about that.

I'm trying to generate a value for a remote system that requires optional properties to be omitted when unset, rather than explicitly set to null, and it's hard to get from Terraform's idea of optional attributes to what the remote system expects. Do you think that's a valid interpretation of the problem you've encountered?

Not quite, although I think you've identified another thing that I encounter with some frequency:

I find myself doing { for k, v in some_object: k => v if v != null } often enough that I would appreciate having the function you suggest to deal with this. For this module, I'm doing this to remove null-valued properties for remote system compatibility and to ensure my own expressions are working properly. I think of it as compact for objects/maps

--

Trying to restate the problem that prompted this issue more concisely:

There is no way to provide an empty object as a default when looking up objects that only have optional properties.

A slimmed down example:

variable "example" {
  type = map(object({
    prop = optional(string)
  }))
  default = {}
}

output "example" {
  value = lookup(var.example, "missing", {})
}

I recognize that the empty object is of a different type than the object specified in the variable and that's what causes the problem here, but it also seems reasonable to me that an object type with only optional properties can be thought of as "compactable" to an empty object.

If I wanted to use lookup properly here to avoid overusing try, the solution feels excessive (and gets much worse with more object properties):

output "example" {
  value = {
    for k, v in lookup(var.example, "missing", { prop = null }):
      k => v if v != null
  }
}
apparentlymart commented 4 months ago

Hi @bholzer,

Thanks for the additional context!

Unfortunately in your example the lookup function can only see the concrete type of the current value of var.example, and not the type constraint that it was converted to earlier by the variable block. That function only knows that the map's element type is object({ prop = string }), because the optional attribute was already dealt with upstream.

I think what this problem really seems to want is the general-purpose convert function we've wanted to add for a long time:

convert({}, object({
  prop = optional(string)
}))

This hypothetical function would allow performing the same type conversion behavior that is built into variable blocks at any location where a general expression is expected.

Unfortunately we've not been able to implement this function so far because it requires a breaking change to the language: it means that Terraform must treat identifiers like string, number, etc as type constraints when they appear in value expressions, whereas historically Terraform has treated them as the beginning of a reference to a resource like resource "string" "whatever", belonging to a hypothetical provider called "string". Although we know of no such provider, we cannot prove that one does not exist privately inside a company somewhere, and so must preserve it for as long as the Terraform v1.x Compatibility Promises remain in effect.

(This function broadly requires some kind of unification of the "value expression" and "type expression" concepts, so that type constraint expressions can be valid in locations where value expressions are expected. Today type constraint expressions are valid only in limited situations, such as in the type argument in a variable block, so there is no ambiguity between the meaning of string in a type constraint vs. the meaning of string in a value expression.)

In the meantime then I think try remains the best option, despite its disadvantages, because it's built for this sort of "I don't care about types, just do what I want" sort of situation.

I'm sorry I don't have a better answer. At least we can use this issue to represent the use-case.

crw commented 4 months ago

Note for future viewers: if you are viewing this issue and would like to indicate your interest, please use the 👍 reaction on the issue description to upvote this issue. We also welcome additional use case descriptions. Thanks!