Terraform: setting defaults() for optional object attribute using experimental `module_variable_optional_attrs` causes panic

[marshall7m]

Terraform Version

0.14.3

Terraform Configuration Files

terraform {
  required_version = "0.14.3"
  experiments = [module_variable_optional_attrs]
}

variable "foo" {
  type = list(object({
    bar_one = string
    bar_two = string
    bar_three = optional(list(string))

  }))
  default = [
      {
          bar_one = "test_one"
          bar_two = "test_two"
      }
  ]
}

locals {
    y = [for mapping in var.foo: defaults(mapping, {bar_three = "test_three"})]
}

output "y" {
    value = local.y
}

Debug Output

Gist

Crash Output

Expected Behavior

Changes to Outputs:
  + y = [
      + {
          + bar_one     = "test_one"
          + bar_two     = "test_two"
          + bar_three  = ["test_three"]
        },
    ]

Actual Behavior

See Debug Output

Steps to Reproduce

Copy and paste code from Terraform Configuration Files

terraform init

terraform apply

Additional Context

References

[danieldreier]

Thank you for reporting this with such a clear reproduction case! I have reproduced this on Terraform 0.14.3 on OS X.

[jalaziz]

I'm also getting a panic. Not sure if it's the same cause, but the trace is:

Call to function "defaults" failed: panic in function implementation: a bool
is required
goroutine 373 [running]:
runtime/debug.Stack(0xc0012c2d70, 0x2f83a80, 0xc000945330)
    runtime/debug/stack.go:24 +0x9f
github.com/zclconf/go-cty/cty/function.errorForPanic(...)
    github.com/zclconf/[email protected]/cty/function/error.go:44
github.com/zclconf/go-cty/cty/function.Function.Call.func1(0xc0012c42f0,
0xc0012c4310)
    github.com/zclconf/[email protected]/cty/function/function.go:291 +0x95
panic(0x2f83a80, 0xc000945330)
    runtime/panic.go:969 +0x1b9
github.com/hashicorp/terraform/lang/funcs.defaultsApply(0x38644c0,
0xc00005842a, 0x0, 0x0, 0x38644c0, 0xc000058429, 0x2f83a80, 0xc000944d00,
0x2f83a80, 0xc000944d00, ...)
    github.com/hashicorp/terraform/lang/funcs/defaults.go:94 +0x1489
github.com/hashicorp/terraform/lang/funcs.defaultsApply(0x38645c0,
0xc000944b40, 0x3063c20, 0xc00097d650, 0x38645c0, 0xc000944d30, 0x3063c20,
0xc00097daa0, 0x3063c20, 0xc00097daa0, ...)
    github.com/hashicorp/terraform/lang/funcs/defaults.go:107 +0x45c
github.com/hashicorp/terraform/lang/funcs.defaultsApply(0x38645c0,
0xc000944b90, 0x3063c20, 0xc00097d6b0, 0x38645c0, 0xc000944d70, 0x3063c20,
0xc00097db60, 0x0, 0xc0012c40d8, ...)
    github.com/hashicorp/terraform/lang/funcs/defaults.go:107 +0x45c
github.com/hashicorp/terraform/lang/funcs.glob..func29(0xc000950840, 0x2, 0x2,
0x38645c0, 0xc000944b90, 0xc000945300, 0x3063c20, 0xc00097de30, 0xc00097de90,
0xc0012c4190, ...)
    github.com/hashicorp/terraform/lang/funcs/defaults.go:65 +0xb3
github.com/zclconf/go-cty/cty/function.Function.Call(0xc0002429c0,
0xc000950840, 0x2, 0x2, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
    github.com/zclconf/[email protected]/cty/function/function.go:295 +0x51a
github.com/hashicorp/hcl/v2/hclsyntax.(*FunctionCallExpr).Value(0xc0005660f0,
0xc001105480, 0x0, 0xc0012c5800, 0x1, 0x1, 0x0, 0x0, 0x0)
    github.com/hashicorp/hcl/[email protected]/hclsyntax/expression.go:442 +0x10c5
github.com/hashicorp/terraform/lang.(*Scope).EvalExpr(0xc000c0e6e0, 0x3863040,
0xc0005660f0, 0x3864500, 0x4949fa0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
    github.com/hashicorp/terraform/lang/eval.go:171 +0x1b7
github.com/hashicorp/terraform/terraform.(*BuiltinEvalContext).EvaluateExpr(0xc0008fc410,
0x3863040, 0xc0005660f0, 0x3864500, 0x4949fa0, 0x0, 0x0, 0x0, 0x106a5b6,
0x106fa20, ...)
    github.com/hashicorp/terraform/terraform/eval_context_builtin.go:287 +0xbb
github.com/hashicorp/terraform/terraform.(*NodeLocal).Execute(0xc00097c900,
0x389fde0, 0xc0008fc410, 0xc00003c006, 0x30b14e0, 0x3237b60)
    github.com/hashicorp/terraform/terraform/node_local.go:156 +0x71d
github.com/hashicorp/terraform/terraform.(*ContextGraphWalker).Execute(0xc001323c70,
0x389fde0, 0xc0008fc410, 0xd2ca258, 0xc00097c900, 0x0, 0x0, 0x0)
    github.com/hashicorp/terraform/terraform/graph_walk_context.go:127 +0xbc
github.com/hashicorp/terraform/terraform.(*Graph).walk.func1(0x3237b60,
0xc00097c900, 0x0, 0x0, 0x0)
    github.com/hashicorp/terraform/terraform/graph.go:59 +0x962
github.com/hashicorp/terraform/dag.(*Walker).walkVertex(0xc000a72f00,
0x3237b60, 0xc00097c900, 0xc000950700)
    github.com/hashicorp/terraform/dag/walk.go:387 +0x375
created by github.com/hashicorp/terraform/dag.(*Walker).Update
    github.com/hashicorp/terraform/dag/walk.go:309 +0x1246
.

[apparentlymart]

I think @jalaziz's report here is a different problem, but it's in the same area so will probably be easiest to address them both together.

I think what both of these have in common a missed check in the defaultsAssertSuitableFallback function, which is meant to return an error if the given value for the defaults argument (the second argument to the defaults function) isn't a suitable default value for a corresponding value in the input argument (the first argument).

For the original report I think the fix would actually need to be in the defaultsApply function rather than in the defaultsAssertSuitableFallback function, because in that case the default value _does_ have the correct type but the collection _itself_ ends up being null, which defaultsApply doesn't currently handle at all. I think for consistency with the other handling of collections here the correct interpretation of a null input would be to return an empty collection of the appropriate kind, such as an empty map as the default for a null map, which would then ensure that other code working with this value can apply splat or for expressions to it and get an empty result in that case.

For the other case where the error message was "a bool is required", @jalaziz it'd be helpful if you could share the Terraform configuration that reproduces that error because it wasn't clear to me just from reading the code how the function got into that situation. There's already some logic in defaultsAssertSuitableFallback that aims to ensure that the default value for a boolean is itself a boolean, but it seems like you've found some input where that check isn't working right.

[jalaziz]

@apparentlymart This is part of a bigger module, but I believe these should be the relevant parts:

variable "postgres" {
  type = object({
    enabled                = optional(bool)
    version                = optional(string)
    instance_type          = optional(string)
    multi_az               = optional(bool)
    auto_upgrade           = optional(bool)
    backup_retention_limit = optional(number)
    publicly_accessible    = optional(bool)
    export_logs            = optional(bool)
    subnet_group_name      = string
    security_group_id      = string
    storage = object({
      encrypted = optional(bool)
      type      = optional(bool)
      iops      = optional(number)
      size      = optional(number)
      max_size  = optional(number)
    })
    db = object({
      name     = optional(string)
      username = string
    })
  })
}

locals {
  postgres = defaults(var.postgres, {
    enabled                = true
    version                = "11.8"
    instance_type          = "db.t3.small"
    multi_az               = false
    auto_upgrade           = true
    backup_retention_limit = 3
    publicly_accessible    = false
    export_logs            = false
    storage = {
      encrypted = true
      type      = "gp2"
      size      = 100
      max_size  = 1000
    }
    db = {
      name = var.name
    }
  })
}

Ultimately, the issue was that type in storage was incorrectly defined as optional(bool).

Also, I'm not sure if this helps, but if I extract the nested objects and call defaults on them individually, I don't see the panic.