Hi guys,
When you include an empty element in an array of elements, terraform crashes.
I think it would be useful/helpful if terraform recognised and ignored the empty element in the array?
Sounds odd, so I'll try and explain my use case, maybe someone else will find it useful; I've written a module that controls a "whitelist" of IP addresses that are allowed to access my AWS ELB's, this module creates a security group rule that adds a list of IP's to the security group id I pass into the module. This saves me having the whitelist in several different places (its never in sync otherwise ;)).
I wanted to expand upon this module by adding an optional variable to include additional IPs for some services which only have slight whitelist changes so I can use the module for the rest of my ELBs. The problem is when the "optional" variable is empty it splits to a new element that is empty and therefore invalid. (Note count 4, but only 3 rules. :()
$ terraform plan -var="port=22" -var="security_group=sg-1234abcd"
+ aws_security_group_rule.my-ip-whitelist
cidr_blocks.#: "" => "4"
cidr_blocks.0: "" => "1.2.3.4/32"
cidr_blocks.1: "" => "4.5.6.7/32"
cidr_blocks.2: "" => "8.8.8.8/32"
from_port: "" => "22"
protocol: "" => "tcp"
*snip*
Obviously, if I declare an optional IP it works perfectly.
$ terraform plan -var="port=22" -var="security_group=sg-1234abcd" -var="optional=5.5.5.5/32"
+ aws_security_group_rule.my-ip-whitelist
cidr_blocks.#: "" => "4"
cidr_blocks.0: "" => "1.2.3.4/32"
cidr_blocks.1: "" => "4.5.6.7/32"
cidr_blocks.2: "" => "8.8.8.8/32"
cidr_blocks.3: "" => "5.5.5.5/32"
from_port: "" => "22"
protocol: "" => "tcp"
*snip*
I think it would be useful if terraform handled the empty array element better and if it was to "drop" it and move forward without it that would be useful for scenarios such as the one I've described.
You can find the "module" and crashlog here if you wanted to repro; https://gist.github.com/jedineeper/041e5ea6b8b2e0624bd0
Hey @jedineeper! I've come up with a minimal test case of this based on your code, and can reproduce the error you are seeing. The current crashing behaviour is clearly undesirable. Our options seem to me to be:
Option 1 may be better from a usability perspective in this case, but I can imagine there may be a case where empty values are desired. @phinze, are you aware of any situations where option 1 would not work?
I believe optional list variables is one of the main use cases for the compact function.
My five cents: option 1 seems to make terraform a little too opinionated. Who knows what plugins people may create that desire empty elements. My vote would be for option 2, but perhaps really call out (in the docs) the compact function being useful in this context. Also, fixing https://github.com/hashicorp/terraform/issues/57 would likely make this a non-issue.
@thegedge You're correct that this is the use case for compact and that in this case that would be the best solution (cc @jedineeper).
I'm going to work on ensuring that we present an error in this case instead of crashing though.
For anyone who has stumbled upon this ticket and is wondering what the full compact() solution looks like mentioned in https://github.com/hashicorp/terraform/issues/3786#issuecomment-158798304, this is working for me:
variable "optional" {
default = ""
}
resource "aws_security_group_rule" "my-ip-whitelist" {
type = "ingress"
from_port = 22
to_port = 22
protocol = "tcp"
cidr_blocks = [
"1.2.3.4/32", # Static address of PersonA
"4.5.6.7/32", # Static address of PersonB
"8.8.8.8/32", # Static address of PersonC
"${compact(split(",", var.optional))}"
]
security_group_id = "sg-1234567"
}
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.
Most helpful comment
For anyone who has stumbled upon this ticket and is wondering what the full
compact()solution looks like mentioned in https://github.com/hashicorp/terraform/issues/3786#issuecomment-158798304, this is working for me: