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

1. Copy and paste code from Terraform Configuration Files
2. terraform init
3. 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/go-cty@v1.7.1/cty/function/error.go:44
github.com/zclconf/go-cty/cty/function.Function.Call.func1(0xc0012c42f0,
0xc0012c4310)
    github.com/zclconf/go-cty@v1.7.1/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/go-cty@v1.7.1/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/v2@v2.8.1/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.

[mengesb]

Mine is an extremely similar issue.. I have the following:

variable "command" {
  type = list(object({
    description      = optional(string)
    shell_command    = optional(string)
    inline_script    = optional(string)
    script_file      = optional(string)
    script_file_args = optional(string)

    job = optional(object({
      name              = string
      group_name        = optional(string)
      run_for_each_node = optional(bool)
      args              = optional(string)

      nodefilters = optional(object({
        excludeprecedence = optional(bool)
        filter            = optional(string)
      }))
    }))

    node_step_plugin = optional(object({
      type   = string
      config = optional(map(string))
    }))

    step = optional(object({
      type   = string
      config = optional(map(string))
    }))
  }))
  description = "Nested block defining one step in the job workflow. A job must have one or more commands."
}

locals {
  command = [ for idx in var.command : defaults(idx,
    {
      description = null
      shell_command = null
      inline_script = null
      script_file = null
      script_file_args = null
      job = {}
      node_step_plugin = {}
      step = {}
    }
  )]
}

I've tried various incarnations of that local... most resulting in panics to the effect of:

Call to function "defaults" failed: panic in function implementation:
interface conversion: interface {} is nil, not map[string]interface {}

I'm using version 0.14.5

[mengesb]

I'm thinking that nested objects (declared optional or not) may have an issue... I also tried this

locals {
  command = [ for obj in var.command : defaults(obj,
    {
      description = null
      shell_command = null
      inline_script = null
      script_file = null
      script_file_args = null
      job = obj.job == null ? {} : defaults(obj.job, {
        group_name = null
        run_for_each_node = false
        args = null
        nodefilters = obj.job.nodefilters == null ? {} : defaults(obj.job.nodefilters, {
          excludeprecedence = false
          filter = null
        })
      })
    }
  )]
}

That resulted in

Error: Error in function call

  on ../../main.tf line 67, in locals:
  67:   command = [ for obj in var.command : defaults(obj,
  68:     {
  69:       description = null
  70:       shell_command = null
  71:       inline_script = null
  72:       script_file = null
  73:       script_file_args = null
  74:       job = obj.job == null ? {} : defaults(obj.job, {
  75:         group_name = null
  76:         run_for_each_node = false
  77:         args = null
  78:         nodefilters = obj.job.nodefilters == null ? {} : defaults(obj.job.nodefilters, {
  79:           excludeprecedence = false
  80:           filter = null
  81:         })
  82:       })
  83:     }
  84:   )]
    |----------------
    | obj.job is null

Call to function "defaults" failed: panic in function implementation:
interface conversion: interface {} is nil, not map[string]interface {}
...

[eddytrex]

Hi I have a similar issue with Terraform v0.14.7, using this module

terraform {
    experiments = [module_variable_optional_attrs]
}

variable "test" {
    type = list(object({
        attribute1 = string
        attribute2 = optional(object({
            attribute21 = string
            attribute22 = optional(string)
        }))
    }))
    default = [{
        attribute1 = "value1"
        attribute2 = {
            attribute21 = "value21" 
        }
    },{
        attribute1 = "value1"
    }]
}

The start of the panic

panic: inconsistent list element types (

cty.Object(map[string]
    cty.Type{
        "attribute1":cty.String, 
        "attribute2":cty.Object(
            map[string]cty.Type{"attribute21":cty.String,
                                "attribute22":cty.String}
        )}) 
then 
cty.Object(map[string]
    cty.Type{
        "attribute1":cty.String, 
        "attribute2":cty.ObjectWithOptionalAttrs(
            map[string]cty.Type{"attribute21":cty.String, 
                                "attribute22":cty.String}, 
                                []string{"", "attribute22"}
        )})
)

Thanks.

[alisdair]

@marshall7m I'm working on this issue now. One of the things that tripped me up in your original report is using a default value for an attribute of optional(list(string)). It's not clear to me what purpose this has, and the most logical fix for the issue would not match your "expected result".

Instead, the output would be an empty list:

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

To make sure that we're addressing the real problem, can you give more detail on what your actual use case is here?

[marshall7m]

@marshall7m I'm working on this issue now. One of the things that tripped me up in your original report is using a default value for an attribute of optional(list(string)). It's not clear to me what purpose this has, and the most logical fix for the issue would not match your "expected result".

Instead, the output would be an empty list:

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

To make sure that we're addressing the real problem, can you give more detail on what your actual use case is here?

Thanks @alisdair for working on this. The defaults function was used to define the default conditions attribute within an IAM policy statements variable.

The optional(list(string)) attribute you are asking about is for the conditions attribute (see below). If I remember correctly, I truncated the bar_three attr within the example for simplicity but in reality, it's a list of objects.

variable "statements" {
  description = "IAM policy statements for role permissions"
  type = list(object({
    effect    = string
    resources = list(string)
    actions   = list(string)
    conditions = optional(list(object({
      test     = string
      variable = string
      values   = list(string)
    })))
  }))
  default = []
}

locals {
  statements = [for statement in var.statements :
    defaults(statement, {
      conditions = {}
    }) 
  ]
}

[alisdair]

Ah, that makes more sense, and is in line with how defaults is designed. Good to hear!

I just merged a fix for the issues in this thread, which should go out in the next 0.15.0 beta release.

[marshall7m]

Awesome, thanks @alisdair for fixing this issue!

[ghost]

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.

[korinne]

Hi all, we recently released the Terraform v1.3 alpha, which includes the ability to mark object type attributes as optional and set default values.

I'm following up with issues that have provided feedback on the previous experiment, and wanted to invite you all to try the alpha and provide any feedback on this updated design. You can learn more/add comments here. Thank you so much in advance, and hope you like the feature!