cloudposse / terraform-aws-security-group

Terraform module to provision an AWS Security Group
https://cloudposse.com/accelerate
Apache License 2.0
36 stars 35 forks source link

Can't pass self when other non-self rules specified #12

Closed mihaiplesa closed 3 years ago

mihaiplesa commented 3 years ago

Passing rules as below will fail with:

The given value is not suitable for child module variable "rules" defined at
.terraform/modules/search.search_ec2_security_group/variables.tf:33,1-17: all
list elements must have the same type.
rules = [
    {
      type        = "egress"
      from_port   = 0
      to_port     = 65535
      protocol    = "-1"
      cidr_blocks = ["0.0.0.0/0"]
    },
    {
      type        = "ingress"
      from_port   = 0
      to_port     = 65535
      protocol    = "tcp"
      self        = true
    }
  ]

Environment (please complete the following information):

Terraform 0.13.6 and module version 0.1.4 on macOS

mihaiplesa commented 3 years ago

This is a Terraform bug/feature after all https://github.com/hashicorp/terraform/issues/26265

naveenb30 commented 3 years ago

I'm also having same exact issue:


Terraform v0.12.29
provider.aws v3.31.0
nitrocode commented 3 years ago

What if you set the undefined key values explicitly to null so the keys for each element were the same.

rules = [
    {
      type        = "egress"
      from_port   = 0
      to_port     = 65535
      protocol    = "-1"
      cidr_blocks = ["0.0.0.0/0"]
      self        = null
    },
    {
      type        = "ingress"
      from_port   = 0
      to_port     = 65535
      protocol    = "tcp"
      cidr_blocks = null
      self        = true
    }
  ]
mihaiplesa commented 3 years ago

@nitrocode already tried and that crashes Terraform hard.

welderpb commented 3 years ago

I have found a work around with optional values. (experimental future)

terraform {
  experiments = [module_variable_optional_attrs]
}

define "rules" variable as following:

variable "rules" {
  type        = list(object({
      description = optional(string)
      type        = string
      from_port   = number
      to_port     = number
      protocol    = string
      cidr_blocks = optional(list(string))
      ipv6_cidr_blocks = optional(list(string))
      source_security_group_id = optional(string)
      self        = optional(bool)
  }))
}
mihaiplesa commented 3 years ago

Which Terraform version? It's their bug after all.

nitrocode commented 3 years ago

Terraform 14 with the experimental flag enabled. It's experimental so it won't be enabled in this module until it's a stable feature presumably in terraform 15.

https://www.terraform.io/docs/language/expressions/type-constraints.html#experimental-optional-object-type-attributes

The issue https://github.com/hashicorp/terraform/issues/19898 tracks the optional feature

Nuru commented 3 years ago

To the extent Hashicorp supports it, this is fixed in #15 (and maybe before).

Terraform requires that all the elements of the rules list be exactly the same type. This means you must supply all the same keys and, for each key, all the values for that key must be the same type. Any optional key, such as ipv6_cidr_blocks, can be omitted from all the rules without problem. However, if some rules have a key and other rules would omit the key if that were allowed (e.g one rule has cidr_blocks and another rule has self = true, and neither rule can include both cidr_blocks and self), instead of omitting the key, include the key with value of null, unless the value is a list type, in which case set the value to [] (an empty list). (Cannot set list type to null before Terraform 0.15 because of this Terraform bug)

So this will work going forward and probably works with earlier versions:

rules = [
    {
      type        = "egress"
      from_port   = 0
      to_port     = 65535
      protocol    = "all"
      cidr_blocks = ["0.0.0.0/0"]
      self        = null
    },
    {
      type        = "ingress"
      from_port   = 0
      to_port     = 65535
      protocol    = "tcp"
      cidr_blocks = []
      self        = true
    }
  ]