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

"inconsistent set element types" crash #31978

Closed
smelchior opened this issue Oct 10, 2022 · 13 comments · Fixed by #32027
Closed

"inconsistent set element types" crash #31978

smelchior opened this issue Oct 10, 2022 · 13 comments · Fixed by #32027
Labels
bug confirmed a Terraform Core team member has reproduced this issue explained a Terraform Core team member has described the root cause of this issue in code upstream

Comments

@smelchior
Copy link

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 smelchior added bug new new issue not yet triaged labels Oct 10, 2022
@jbardin
Copy link
Member

jbardin commented Oct 11, 2022

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 jbardin added waiting-response An issue/pull request is waiting for a response from the community waiting for reproduction unable to reproduce issue without further information and removed new new issue not yet triaged labels Oct 11, 2022
@smelchior
Copy link
Author

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?

@jbardin
Copy link
Member

jbardin commented Oct 12, 2022

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

@jbardin jbardin added confirmed a Terraform Core team member has reproduced this issue and removed waiting-response An issue/pull request is waiting for a response from the community waiting for reproduction unable to reproduce issue without further information labels Oct 12, 2022
@jbardin
Copy link
Member

jbardin commented Oct 12, 2022

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 jbardin added upstream explained a Terraform Core team member has described the root cause of this issue in code labels Oct 12, 2022
@smelchior
Copy link
Author

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!

@Nuru
Copy link

Nuru commented Oct 22, 2022

@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?

@jbardin
Copy link
Member

jbardin commented Oct 24, 2022

@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.

@Nuru
Copy link

Nuru commented Oct 25, 2022

@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.

@jbardin
Copy link
Member

jbardin commented Oct 26, 2022

@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.

@Nuru
Copy link

Nuru commented Nov 3, 2022

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

@liamcervante
Copy link
Member

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.

@Nuru
Copy link

Nuru commented Nov 4, 2022

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

@github-actions
Copy link
Contributor

github-actions bot commented Dec 4, 2022

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 Dec 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug confirmed a Terraform Core team member has reproduced this issue explained a Terraform Core team member has described the root cause of this issue in code upstream
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants