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

Optional arguments in object variable type definition #19898

Closed
prehor opened this issue Dec 30, 2018 · 112 comments
Closed

Optional arguments in object variable type definition #19898

prehor opened this issue Dec 30, 2018 · 112 comments

Comments

@prehor
Copy link

prehor commented Dec 30, 2018

Current Terraform Version

Terraform v0.12.0-alpha4 (2c36829d3265661d8edbd5014de8090ea7e2a076)

Proposal

I like the object variable type and it would be nice to be able to define optional arguments which can carry null value too, to use:

variable "network_rules" {
  default = null
  type = object({
    bypass = optional(list(string))
    ip_rules = optional(list(string))
    virtual_network_subnet_ids = optional(list(string))
  })
}

resource "azurerm_storage_account" "sa" {
  name = random_string.name.result
  location = var.location
  resource_group_name = var.resource_group_name
  account_replication_type = var.account_replication_type
  account_tier = var.account_tier

  dynamic "network_rules" {
    for_each = var.network_rules == null ? [] : list(var.network_rules)

    content {
      bypass = network_rules.value.bypass
      ip_rules = network_rules.value.ip_rules
      virtual_network_subnet_ids = network_rules.value.virtual_network_subnet_ids
    }
  }

instead of:

variable "network_rules" {
  default = null
  type = map(string)
}

resource "azurerm_storage_account" "sa" {
  name = random_string.name.result
  location = var.location
  resource_group_name = var.resource_group_name
  account_replication_type = var.account_replication_type
  account_tier = var.account_tier

  dynamic "network_rules" {
    for_each = var.network_rules == null ? [] : list(var.network_rules)

    content {
      bypass = lookup(network_rules, "bypass", null) == null ? null : split(",", lookup(network_rules, "bypass"))
      ip_rules = lookup(network_rules, "ip_rules", null) == null ? null : split(",", lookup(network_rules, "ip_rules"))
      virtual_network_subnet_ids = lookup(network_rules, "virtual_network_subnet_ids", null) == null ? null : split(",", lookup(network_rules, "virtual_network_subnet_ids"))
    }
  }
}
@prehor prehor changed the title Allow optional arguments in object variable type definition Optional arguments in object variable type definition Dec 30, 2018
@abymsft
Copy link

abymsft commented Jun 25, 2019

I'd really like to have this option too. Any plans of prioritizing this enahancement ?

@rverma-nikiai
Copy link

is there a workaround for this? I found myself using type=any

@sudoforge
Copy link

sudoforge commented Jun 27, 2019

@rverma-nikiai No, there is not currently a workaround for this.

The best thing we can do is define the optional argument as such (where foobar is the optional property):

modules/myfoo/main.tf

variable "foo" {
  type = object({
    name   = string
    id     = number
    foobar = string # or bool, number, etc...
  })
}

resource "null_resource" {
  triggers = {
    some_name   = var.foo.name
    some_id     = var.foo.id
    some_foobar = var.foo.foobar
  }
}

... and then call it, explicitly setting the optional properties to null

main.tf

module "potato" {
  source = "./modules/myfoo"
  foo = {
    name   = "something-cool-is-afoot"
    id     = 1234567890
    foobar = null
  }
}

This allows you to do normal stuff like set defaults using a ternary check, eg.:

modules/myfoo/main.tf

locals {
  foobar_default = var.foo.foobar == null ? "default value for foo.foobar" : null
}

...

@thefotios
Copy link

Along the same lines, it would be useful to be able to declare partial defaults for complex types

For instance, something like

variable "with_default_path" {
  type = object({
    id = string
    path = string
  })
 
  default = {
    path = "/"
  }
}

@mildwonkey
Copy link
Contributor

Hi folks! This is a great feature request, and getting use-cases is absolutely helpful, but so you know it best to react to the original issue comment with 👍which we can and do report on during prioritization.
Thank you :)

@LinguineCode
Copy link

Along the same lines, it would be useful to be able to declare partial defaults for complex types

For instance, something like

variable "with_default_path" {
  type = object({
    id = string
    path = string
  })
 
  default = {
    path = "/"
  }
}

Absolutely, this should be part of the original post as well

👍 Optional values
👍 Partial default values

@sudoforge
Copy link

sudoforge commented Aug 3, 2019

@solsglasses - please add reactions to comments, pull requests, or issues instead of creating a new comment simply to show agreement with someone. Thanks for helping to keep the community awesome, and the notification feed of -- well, everyone -- free of fluff!

@thefotios
Copy link

I'm not sure of the internals, but this may not be that difficult. Since type and defaults are already well defined, the parser would just need to allow both of them and then merge the defaults with the provided variable before validation.

My proposal mostly dealt with optional values inside the type definition, not the argument itself being optional. The might get a little trickier. Having the argument be optional is going to lead to a lot of lines like:

for_each = var.network_rules == null ? [] : list(var.network_rules)

It may make more sense for the default value to match the type definition. So a list(object(...)) would return an empty list. This way you can let the parser handle the null check and you can be guaranteed to get the proper type for iteration. This is assuming that an empty list is valid for the type definition of list(object(...)).

It could reduce a bunch of boilerplate code if there was an optional flag in the variable definition.

@atavakoliyext
Copy link

atavakoliyext commented Aug 7, 2019

Update: I created #22449 to track this.


One more proposal to add to the mix:

object({
  <KEY> = <TYPE | { [type = <TYPE>,][ default = ...] }>
})

I think would allow more readable complex object definitions in cases where defaults could appear at any level, and would have parity with how defaults in variable blocks are defined. For a contrived example:

variable "pipeline" {
  type = object({
    name = string

    pipeline_type = {
      type = string        // inferred from default & can be omitted
      default = "STANDARD"
    }

    stages = {
      type = list(object({
        name      = string
        cmd       = string
        must_pass = { default = false }
        }
      }))

      default = [
        {
          name      = "build"
          cmd       = "make all"
          must_pass = true
        },
        {
          name = "deploy"
          cmd  = "make deploy"
        },
      ]
    }
  })

  default = {
    name = "standard_build_pipeline"
    // everything else is taken from the type defaults
  }
}

(the use-case for having both type and default in the block would be for when the type can't be inferred from the default, such as when default = null).

@morgante
Copy link

For what it's worth, even the minimal behavior (accepting objects with omitted values and setting the omitted values to null) would be a major usability improvement. Defaults could be implemented within modules themselves.

@binlab
Copy link
Contributor

binlab commented Sep 22, 2019

Very needed possibility for maps and objects types.

@sidewinder12s
Copy link

Being able to set a default = null type I think would allow modules to set object type constraints for input variables or resources that have optional attributes, while still being able to set some type/object constraints.

@atavakoliyext
Copy link

My personal vote is also to add support for default values, rather than support for marking some fields as optional.

Defaults imply optionality, so you get optionals for free by supporting defaults. OTOH, if we add optional(...) support, and then later add defaults, then we'll have two different ways of defining optionality (specifying a default vs specifying optional(...), the latter being equivalent to specifying a null default), which I think would be less preferred.

@cadavre
Copy link

cadavre commented Oct 7, 2019

Definitely @atavakoliyext proposal is best.

@prehor thefotios proposal:

variable "with_default_path" {
  type = object({
    id = string
    path = string
  })
 
  default = {
    path = "/"
  }
}

Why we cannot use it:

variable "with_default_path" {
  type = list(object({
    id = string
    path = string
  }))
 
  default = [{
    path = "/"
  }] # ???
  # or how?
  # this doesn't look reasonable
}

@lemontreeran
Copy link

lemontreeran commented Nov 1, 2019

Actually there are workarounds for optional arguments in object variables. The tricky part is still on the default value of the variable and using local variables. Use the locals variable to verify the optional parameter in "network_rules". Put it into null if not existing. The other resources would be able to refer to those local variables.

variable "network_rules" {
  default = {
    bypass = null,
    ip_rules = null,
    virtual_network_subnet_ids = null
  }
  type = object({
    bypass = list(string)
    ip_rules = list(string)
    virtual_network_subnet_ids = list(string)
  })
}

locals {
    network_rules = lookup(var.network_rules, "bypass", "no_bypass") == "no_bypass" ? null : var.network_rules,
    ip_rules = lookup(var.ip_rules, "ip_rules", "no_ip_rules") == "no_ip_rules" ? null : var.network_rules,
    virtual_network_subnet_ids = lookup(var.virtual_network_subnet_ids, "virtual_network_subnet_ids", "no_virtual_network_subnet_ids") == "no_virtual_network_subnet_ids" ? null : var.network_rules
}

@grimm26
Copy link

grimm26 commented Nov 5, 2019

@lemontreeran This does not work. I cleaned up your example:

variable "network_rules" {
  default = {
    bypass                     = null,
    ip_rules                   = null,
    virtual_network_subnet_ids = null
  }
  type = object({
    bypass                     = list(string)
    ip_rules                   = list(string)
    virtual_network_subnet_ids = list(string)
  })
}

locals {
  network_rules              = lookup(var.network_rules, "bypass", "no_bypass") == "no_bypass" ? null : var.network_rules
  ip_rules                   = lookup(var.network_rules, "ip_rules", "no_ip_rules") == "no_ip_rules" ? null : var.network_rules
  virtual_network_subnet_ids = lookup(var.network_rules, "virtual_network_subnet_ids", "no_virtual_network_subnet_ids") == "no_virtual_network_subnet_ids" ? null : var.network_rules
}

And provided a terraform.tfvars like so:

network_rules = {
  bypass = ["nope"]
}

The result:

> $ terraform plan
Error: Invalid value for input variable

  on terraform.tfvars line 1:
   1: network_rules = {
   2:   bypass = ["nope"]
   3: }

The given value is not valid for variable "network_rules": attributes
"ip_rules" and "virtual_network_subnet_ids" are required.

You cannot have optional elements of a well-defined variable object.

@lemontreeran
Copy link

lemontreeran commented Nov 6, 2019

@lemontreeran This does not work. I cleaned up your example:

variable "network_rules" {
  default = {
    bypass                     = null,
    ip_rules                   = null,
    virtual_network_subnet_ids = null
  }
  type = object({
    bypass                     = list(string)
    ip_rules                   = list(string)
    virtual_network_subnet_ids = list(string)
  })
}

locals {
  network_rules              = lookup(var.network_rules, "bypass", "no_bypass") == "no_bypass" ? null : var.network_rules
  ip_rules                   = lookup(var.network_rules, "ip_rules", "no_ip_rules") == "no_ip_rules" ? null : var.network_rules
  virtual_network_subnet_ids = lookup(var.network_rules, "virtual_network_subnet_ids", "no_virtual_network_subnet_ids") == "no_virtual_network_subnet_ids" ? null : var.network_rules
}

And provided a terraform.tfvars like so:

network_rules = {
  bypass = ["nope"]
}

The result:

> $ terraform plan
Error: Invalid value for input variable

  on terraform.tfvars line 1:
   1: network_rules = {
   2:   bypass = ["nope"]
   3: }

The given value is not valid for variable "network_rules": attributes
"ip_rules" and "virtual_network_subnet_ids" are required.

You cannot have optional elements of a well-defined variable object.

Try adding the null value for those vars. I know it is ugly......

network_rules = {
  bypass                      = ["nope"],
  ip_rules                    = null,
  virtual_network_subnet_ids  = null
}

@grimm26
Copy link

grimm26 commented Nov 6, 2019

Try adding the null value for those vars. I know it is ugly......

network_rules = {
  bypass                      = ["nope"],
  ip_rules                    = null,
  virtual_network_subnet_ids  = null
}

It's not that it is ugly, it is not optional if you have to supply a value, which is the whole point of this issue.

@sudoforge
Copy link

@grimm26 Yes, that is why the issue is still open. As @lemontreeran noted, and as I wrote earlier in the thread, providing null values for the "optional" parameters is the only way to circumvent the issue.

@igorbrites
Copy link

Or following the first post, I'd like to pass default values like this:

variable "network_rules" {
  default = null
  type = object({
    bypass                     = optional(list(string), ["teste"])
    ip_rules                   = optional(list(string), null)
    virtual_network_subnet_ids = optional(list(string), [])
    enabled                    = optional(bool, true)
  })
}

The optional could work like the lookup function (without the first map parameter, off course), receiving the type on the first parameter and the default value on the second one.

@mutt13y
Copy link

mutt13y commented Dec 3, 2019

Where we have a map or list of map the solution should include the capability for the map key to be absent.

type = list(object({
  foo = optional(string,"foo_default")
  bar = optional(string)
}))

so if bar is not provided by the caller the key would be completely absent.

this way you can use the absence of the key to infer some more complex default action rather than having to use a rouge value for default.

@jonwtech
Copy link

jonwtech commented Jan 4, 2022

Hey Guys! I'm facing similar issue when creating optional variables inside a list of objects. Is this Issue related to that or did I missing something in the comments for workarounds.

terraform version: 0.13.7

variable "foo" { type = list(object({ bar= optional(bool), hello= optional(bool), bye= optional(bool), name = string, description = string, birthyear = optional(number), age = optional(number), text = string, records= list(object({ name = string description = string alias = string })) })) }

Issue: Keyword "optional" is not a valid type constructor.

You need to be on at least v0.14

@gmauleon
Copy link

Another 2022 bump on this, would definitely need it in my map of object setup where I don't want advanced options to be mandatory.

@prowlaiii
Copy link

prowlaiii commented Mar 31, 2022

I find the separate declaration of the types and the defaults a bit cumbersome and the "optional" keyword superfluous.

I think this has already been suggested, but the syntax for declaring the attributes of a variable is already defined, so it would be consistent to just make them recursive, eg.

variable "simple1" {
  default     = "blah"
  description = "A simple variable"
  type        = string
}
variable "complex1" {
  default = null
  description = "A complex variable"
  type = object({
    mystring1 = {
      default = "blah"
      description = "A string in an object variable (optional)"
      type = string
    }
    mystring2 = {
      description = "A string in an object variable (required)"
      sensitive = true
      type = string
    }
    mylist1 = {
      default = []
      description = "A list in an object variable (optional)"
      type = list(string)
    }
    mymap1 = {
      default = {}
      description = "A map in an object variable (optional)"
      type = map(string)
    }
    myobject1 = {
      default = null
      description = "An object within an object variable (optional)"
      type = object({
        ....
      )}
    }
    mystring3 = { description = "Another string in an object variable (required)", type = string }
  })
}

@ImIOImI
Copy link

ImIOImI commented Apr 7, 2022

I was surprised that when specifying an object as a type it filters out all inputs that aren't declared. I love all the ideas about specifying optional variables and stuff... but this has been open for 4 years. Maybe just don't kill things not declared in the type, then give us the awesome later?

tfvars:

test = {
  map0 = {
    required = "string"
  }
  map1 = {
    required = "string"
    optional = "string"
  }
}

variable:

variable "test" {
  type = map(
    object(
      {
        required = string
      }
    )
  )
}

outputs:

test = tomap({
  "map0" = {
    "required" = "string"
  }
  "map1" = {
    "required" = "string"
  }
})

In my opinion this should either error or let optional go through. I'm 100% in favor of making this declared explicitly, but it's current behavior is very surprising.

@pascal-hofmann
Copy link

pascal-hofmann commented Apr 8, 2022

I was surprised that when specifying an object as a type it filters out all inputs that aren't declared. I love all the ideas about specifying optional variables and stuff... but this has been open for 4 years. Maybe just don't kill things not declared in the type, then give us the awesome later?

I prefer to have them declared explicitly. This way you can look at the type declaration and see which fields are supported.

@dylanturn
Copy link

I prefer to have them declared explicitly. This way you can look at the type declaration and see which fields are supported.

It also means that the language server (terraform-ls/terraform-lsp) can introspect the modules and provide much more accurate auto-complete suggestions...

@ImIOImI
Copy link

ImIOImI commented Apr 8, 2022

I prefer to have them declared explicitly. This way you can look at the type declaration and see which fields are supported.

Me too... but the five stages of grief are "denial, anger, bargaining, depression and acceptance" and right now I'm at the bargaining stage. If I can't have something perfect, can I at least have something less nice?

@mponton-cn
Copy link

I'm at the bargaining phase myself. Simply getting rid of the "experimental" warning would please me.

@tikicoder
Copy link

For me this has been experimental for how long, I just would like to know what is the reason, what is causing issues, and is it really something that should be a blocker versus a hey every FYI this WILL break if you do this you know it will break.

@Nuru
Copy link

Nuru commented Apr 18, 2022

@apparentlymart Can we get an ETA (or declaration of lack of interest in an ETA) for taking this feature out of experimental and into production? I would really like to be using it in a lot of places now. It has been experimental in 0.14, 0.15, 1.0, and 1.1. Can we get it into production at 1.2? CC: @alisdair

Until the experiment is concluded, the behavior of this feature may see breaking changes even in minor releases. We recommend using this feature only in prerelease versions of modules as long as it remains experimental.

@apparentlymart
Copy link
Contributor

Hi all,

Based on feedback about the existing experiment, we don't intend to graduate it to be a stable feature exactly as currently designed. Instead, we intend to make another design iteration which better integrates the idea of default values into the constraints system, and deals with some other concerns that folks raised in feedback on the existing proposal.

We don't intend to take the experiment away until there's a new experiment to replace it, but if you do wish to use it in production in spite of its experimental status I suggest keeping in mind that you will probably need to change your configuration at least a little in response to the changes for the second round of experiment.

The purpose of experiments is to gather feedback before finalizing a design, so in this case the experiment process worked as intended but since the feedback led to us needing another round of design we had to wait until there was time to spend on that design work, since the team was already wholly assigned to other work. We'll share more here when there's more to share.

@joe-a-t
Copy link

joe-a-t commented Apr 20, 2022

Flagging #30608 as a related enhancement request that might merit consideration for future state consistency when refactoring of the optional arguments stuff here.

@erikeckhardt
Copy link

It seems like long-term, to enforce types in a way that works for reasonable use cases, following in the pattern of TypeScript could be extremely useful. Let's say there are two properties that are mutually exclusive. Even with optional(), there's no way to express with the type system that either ONE or the OTHER must be present—that would have to be a validation error that occurs at runtime instead of giving an error at design time.

Here's a concrete example of a real use-case for creating AWS CloudWatch dashboard widget JSON: properties query and metrics are mutually exclusive, but one is required. Using optional() might look like this:

variable widget_definition {
  type = object({
    metrics = optional(list(list(string)))
    query = optional(string)
    ... other attributes here
  })
  description = "An object similar in structure to the JSON for CloudWatch dashboard widgets, but with some default values and simplifications/helpers to make the input simpler."
  validation {
    condition = count([ for key in var.widget_definition: key if contains(["metrics", "query"], key) ]) == 1
    error_message = "Must provide exactly one of properties 'metrics' and 'query' in variable widget_definition."
  }

But TypeScript handles this with type unions. That might look something like this:

  type = object({
     ... other attributes here
  }) & (
    { metrics = list(list(string) }
    | { query = string }
  )

Using something like this would avoid the need for validation to enforce the mutual exclusivity, allowing the type system itself to represent the strict expected types and the either/or nature of both metrics and query.

@fabiendelpierre
Copy link

Looks like this is happening with TF v1.3.0.

https://github.com/hashicorp/terraform/releases/tag/v1.3.0-alpha20220608

@korinne
Copy link

korinne commented Jun 8, 2022

@fabiendelpierre You beat me to it :) Hi all, we'd love your feedback on the latest v1.3 alpha, which includes the ability to mark object type attributes as optional and set default values. You can learn more/provide feedback here. Thank you in advance!

@prowlaiii
Copy link

prowlaiii commented Aug 23, 2022

Further to my previous post, I would suggest that (though the v1.3.0 adding of the 2nd default field in the "optional()" function is an improvement) the "optional()" keyword/function itself is really a bit of a red-herring and simply expanding the variable structure to allow nesting would be a more natural and syntactically consistent approach.

A simple variable declaration:

variable "name" {
  type    = string
  default = "blah"
}

A complex variable declaration:

variable "settings" {
  type = object({
    name          = string       # shorthand for "{ type = string }"
    description   = { type = string, default = "The name" }
    how_many      = { type = number, default = 1, description = "Count of items" }
    list_of_items = { type = list, default = [] }
  })
}

Just as with normal variables, the act of declaring a default makes it optional; there is no need for an "optional()" keyword/function and conversely not specifying a "default" makes it a required item.

Thinking about it, the "object()" declaration could become redundant too, as the fact that there is a mix of types would implicitly make that so.

variable "settings" {
  type = {
    name          = string       # shorthand for "{ type = string }"
    description   = { type = string, default = "The name" }
    how_many      = { type = number, default = 1, description = "Count of items" }
    list_of_items = { type = list, default = [] }
  }
}

Extrapolating that further, the "type =" lines could be recursive and thus support sub-objects .

@atavakoliyext
Copy link

@prowlaiii is what you're describing similar to the feature request in #22449?

@prowlaiii
Copy link

@prowlaiii is what you're describing similar to the feature request in #22449?

I think it is indeed; I hadn't seen it, but I knew I couldn't have been the first person on the planet to think of it!

(I'm also suggesting the "description =" item and removing the need for "object()" there.)

FYI, I also raised this in the v1.3 feedback and have now included a link to yours there.
https://discuss.hashicorp.com/t/request-for-feedback-optional-object-type-attributes-with-defaults-in-v1-3-alpha/40550/47

@dylanturn
Copy link

A complex variable declaration:

variable "settings" {
  type = object({
    name          = string       # shorthand for "{ type = string }"
    description   = { type = string, default = "The name" }
    how_many      = { type = number, default = 1, description = "Count of items" }
    list_of_items = { type = list, default = [] }
  })
}

If we're going to take it that far then why not just go all the way and create a new type that lets you simply specify an openapi schema?

The openapi shema variable type could be defined inline, sourced from a file in the same directory, or maybe even from a remote source (like we source modules today). That capability would make it easy to create, document, and reuse complex data classes in our input and output blocks.

@apparentlymart
Copy link
Contributor

Hi all,

An evolution of the optional attributes experiment, with built-in support for default values and no separate defaults function, is going to graduate to stable in the forthcoming v1.3.0 release.

For that reason, I'm going to close this issue to represent that the base use-case is met. I see that there is some discussion here about further additions to the model for object attributes, but I think it'll be better to discuss those in more specific issues once the initial features are included in a stable release and we can learn from experiences of using the first iteration of this functionality in a broad set of Terraform modules with differing design needs.

Thanks for the great discussion here!

@github-actions
Copy link

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 Sep 29, 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