terraform icon indicating copy to clipboard operation
terraform copied to clipboard

"inconsistent set element types" crash

Open smelchior opened this issue 1 year ago • 9 comments

Terraform Version

Terraform v1.3.2
on darwin_arm64
+ provider registry.terraform.io/hashicorp/aws v4.34.0

Terraform Configuration Files


variable "backup_plans" {
  type = set(object({
    name = string
    schedules = set(object({
      name                     = string
      cron_expression          = string
      retention_days           = number
      cold_storage_after       = optional(number, 0)
      enable_continuous_backup = bool
    }))
    conditions          = set(map(string))
    recovery_point_tags = map(string)
    resources           = list(string)
    not_resources       = list(string)
  }))
default = [
    {
      name = "test"
      schedules = [
        {
          name                     = "daily"
          cron_expression          = "cron(0 1 * * ? *)"
          retention_days           = 7
          enable_continuous_backup = true
        },
        {
          name                     = "weekly"
          cron_expression          = "cron(0 3 ? * MON *)"
          retention_days           = 13 * 7
          cold_storage_after       = 1
          enable_continuous_backup = false
        }
      ]
      conditions = [
        {
          key   = "datatype"
          value = "business-critical"
        }
      ]
      not_resources = []
      resources = [
        "arn:aws:dynamodb:*:*:table/*"
      ]

      recovery_point_tags = {
        "backuptype" = "fallback"
      }
    },

  ]
}

resource "aws_backup_plan" "this" {
  for_each = { for plan in var.backup_plans : plan.name => plan }
  name     = each.key

  dynamic "rule" {
    for_each = each.value.schedules
    content {
      rule_name                = rule.value.name
      target_vault_name        = aws_backup_vault.this.name
      schedule                 = rule.value.cron_expression
      enable_continuous_backup = rule.value.enable_continuous_backup
      lifecycle {
        delete_after       = rule.value.retention_days
        cold_storage_after = rule.value.cold_storage_after
      }
      recovery_point_tags = each.value.recovery_point_tags
    }
  }
}


resource "aws_backup_selection" "this" {
  for_each     = { for plan in var.backup_plans : plan.name => plan }
  iam_role_arn = aws_iam_role.this.arn
  name         = "default"
  plan_id      = aws_backup_plan.this[each.key].id
  condition {
    dynamic "string_equals" {
      for_each = each.value.conditions
      content {
        key   = "aws:ResourceTag/${string_equals.value.key}"
        value = string_equals.value.value
      }
    }
  }
  not_resources = each.value.not_resources
  resources     = each.value.resources
}

Debug Output

.

Expected Behavior

no crash

Actual Behavior

inconsistent set element types (cty.Object(map[string]cty.Type{"conditions":cty.Tuple([]cty.Type{cty.Object(map[string]cty.Type{"key":cty.String, "value":cty.String})}), "name":cty.String, "not_resources":cty.List(cty.String), "recovery_point_tags":cty.Object(map[string]cty.Type{"ts-backuptype":cty.String}), "resources":cty.Tuple([]cty.Type{cty.String}), "schedules":cty.List(cty.Object(map[string]cty.Type{"cold_storage_after":cty.Number, "cron_expression":cty.String, "enable_continuous_backup":cty.String, "name":cty.String, "retention_days":cty.String}))}) then cty.Object(map[string]cty.Type{"conditions":cty.Tuple([]cty.Type{cty.Object(map[string]cty.Type{"key":cty.String, "value":cty.String})}), "name":cty.String, "not_resources":cty.List(cty.String), "recovery_point_tags":cty.Object(map[string]cty.Type{"ts-backuptype":cty.String}), "resources":cty.Tuple([]cty.Type{cty.String}), "schedules":cty.List(cty.Object(map[string]cty.Type{"cold_storage_after":cty.String, "cron_expression":cty.String, "enable_continuous_backup":cty.String, "name":cty.String, "retention_days":cty.String}))})) goroutine 31 [running]: runtime/debug.Stack() /usr/local/go/src/runtime/debug/stack.go:24 +0x64 runtime/debug.PrintStack() /usr/local/go/src/runtime/debug/stack.go:16 +0x1c github.com/hashicorp/terraform/internal/logging.PanicHandler() /Users/distiller/project/project/internal/logging/panic.go:55 +0x170 panic({0x104c352e0, 0x1400149ad20}) /usr/local/go/src/runtime/panic.go:884 +0x204 github.com/zclconf/go-cty/cty.SetVal({0x140031d2280, 0x4, 0x1?}) /Users/distiller/go/pkg/mod/github.com/zclconf/[email protected]/cty/value_init.go:281 +0x608 github.com/zclconf/go-cty/cty.transform({0x0?, 0x0, 0x0}, {{{0x105039f78?, 0x14001479280?}}, {0x104f49d60?, 0x14003856a50?}}, {0x105021360, 0x14000634390}) /Users/distiller/go/pkg/mod/github.com/zclconf/[email protected]/cty/walk.go:177 +0xbe0 github.com/zclconf/go-cty/cty.TransformWithTransformer(...) /Users/distiller/go/pkg/mod/github.com/zclconf/[email protected]/cty/walk.go:130 github.com/hashicorp/hcl/v2/ext/typeexpr.(*Defaults).Apply(0x140008297e0?, {{{0x105039f78?, 0x14001479280?}}, {0x104f49d60?, 0x14003856a50?}}) /Users/distiller/go/pkg/mod/github.com/hashicorp/hcl/[email protected]/ext/typeexpr/defaults.go:38 +0x9c github.com/hashicorp/terraform/internal/terraform.prepareFinalInputVariableValue({{0x14000ab2a60, 0x1, 0x1}, {{}, {0x1400039eaf0, 0xc}}}, 0x14003f15a30, 0x1400077a750) /Users/distiller/project/project/internal/terraform/eval_variable.go:98 +0x88c github.com/hashicorp/terraform/internal/terraform.(*nodeModuleVariable).evalModuleVariable(0x140036324e0?, {0x10504e2f8?, 0x140023ca1c0?}, 0xc?) /Users/distiller/project/project/internal/terraform/node_module_variable.go:239 +0x3e4 github.com/hashicorp/terraform/internal/terraform.(*nodeModuleVariable).Execute(0x140036324e0, {0x10504e2f8, 0x140023ca1c0}, 0x2) /Users/distiller/project/project/internal/terraform/node_module_variable.go:155 +0x100 github.com/hashicorp/terraform/internal/terraform.(*ContextGraphWalker).Execute(0x1400465e870, {0x10504e2f8, 0x140023ca1c0}, {0x1304d1a38, 0x140036324e0}) /Users/distiller/project/project/internal/terraform/graph_walk_context.go:136 +0xa8 github.com/hashicorp/terraform/internal/terraform.(*Graph).walk.func1({0x104e04d40, 0x140036324e0}) /Users/distiller/project/project/internal/terraform/graph.go:74 +0x208 github.com/hashicorp/terraform/internal/dag.(*Walker).walkVertex(0x14003632540, {0x104e04d40, 0x140036324e0}, 0x140010e6a80) /Users/distiller/project/project/internal/dag/walk.go:381 +0x2e0 created by github.com/hashicorp/terraform/internal/dag.(*Walker).Update /Users/distiller/project/project/internal/dag/walk.go:304 +0xbf0

Steps to Reproduce

terraform plan

Additional Context

The issue only happens if cold_storage_after is set in a schedule.

References

No response

smelchior avatar Oct 10 '22 18:10 smelchior

Hi @smelchior,

Thanks for filing the issue. I'm not able to replicate the crash from the example given. Could you supply the exact value being assigned to the backup_plans variable which caused the problem? One thing to note in the output, is that one of the values for cold_storage_after is a string, which implies that the type is being incorrectly inferred from another location.

Thanks!

jbardin avatar Oct 11 '22 14:10 jbardin

Hi @jbardin, thanks for taking a look :)! I did some more tests to find a minimal example for the bug: https://github.com/smelchior/tf-variable-bug With this i can reproduce it consistently, can you give it a try?

smelchior avatar Oct 12 '22 05:10 smelchior

Thanks @smelchior, that is a great example! We'll take a look.

jbardin avatar Oct 12 '22 12:10 jbardin

Hi @smelchior,

The root problem in the configuration here is the root module variable declaration. Because you specified a type of set(any), Terraform must infer a single unified type for the given set elements. The object values don't have a common type, so the closest type it finds with the any constraint is map(string) (because conversions from bool and number to string are allowed).

Removing the set(any) constraint will prevent the issue, and allow the given tuple to be converted correctly, and assign defaults when assigned to the module variable. Defining an identical type for the root module variable will also of course fix the problem.

This still should not crash however, so we can work on that upstream, and hopefully return a more reasonable error to help diagnose the problem more easily.

jbardin avatar Oct 12 '22 17:10 jbardin

Thanks for looking into it! We fixed it now on our side so it does not crash anymore, but a more helpful error message would definitely be nice!

smelchior avatar Oct 13 '22 07:10 smelchior

@jbardin Commenting here because you closed #32048 as a duplicate.

I understand your rationale regarding conversion to set(any), but I have the same problem (in Terraform 1.3.3) as #32048 where the troublesome member is optional(number). This should not cause a problem with type inference, and your suggested workaround is inapplicable.

Can you provide a workaround for optional(number) as described in #32048?

Nuru avatar Oct 22 '22 21:10 Nuru

@Nuru, it is the optional attribute which is the trigger for the crash, but the problem has to do with the optional attribute being applied to an incompatible type. This is usually caused by earlier implicit conversion to a collection with a different type, e.g. a set(map(string)), list(map(string)), etc. In your example, the coalesce function must try to find a unified type and convert all the arguments, which is probably resulting in something you are not expecting. You should be able to remove coalesce to get the behavior you need. If you have null values being passed in, and the the module input cannot be null, set nullable = false in the variable declaration to use the default.

jbardin avatar Oct 24 '22 13:10 jbardin

@jbardin

Thank you for the additional explanation. Unfortunately, #32048 is not my example, I just referenced it because I hoped it was close enough. I'm not using coalesce and the attributes in the object in question are nullable, so I cannot use nullable = false. (Can you use nullable at the object attribute level with optional? What is the syntax for that?)

My example is here. You can ignore most of the tfvars files. Check out the linked directory/commit and run

terraform init
terraform plan -var-file lifecycle.us-east-2.tfvars

and you will get the crash. The crash is due to lifecycle_configuration_rules, which is meant to be an improvement over lifecycle_rules. The whole point of this PR is to remove the burden on the caller of specifying objects of exactly the same type in a list of objects (allowing optional attributes to be truly optional), so if this cannot be fixed in the module source, I will have to wait for the Terraform bugfix, but if there is a way to fix this in the source (of the module, not the example), then please let me know and I will.

Nuru avatar Oct 25 '22 22:10 Nuru

@Nuru,

I don't think there is any guaranteed workaround which does not involve altering the input value in some way, because it is the input value type which is triggering the bug.

jbardin avatar Oct 26 '22 17:10 jbardin

Just to be clear, this is not fixed in Terraform version 1.3.4

Nuru avatar Nov 03 '22 04:11 Nuru

Hi @Nuru, I've checked the minimal example provided for this issue and the example provided in the duplicate issue #32048 and both are now working for me.

I haven't been able to reproduce your issue as I need an AWS account to run your example. I should have one soon, when I get it I'll take another look. In the meantime, could you file a new issue and include the stack trace from your crash. If you could also try and create a minimal example that doesn't require an AWS account that would speed things up for me to be able to reproduce.

Either way, we'll carry on considering this issue closed and can discuss your crash further in a new issue.

liamcervante avatar Nov 03 '22 12:11 liamcervante

@liamcervante I opened #32163 for you, requires no providers.

Nuru avatar Nov 04 '22 02:11 Nuru

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[bot] avatar Dec 04 '22 02:12 github-actions[bot]