config: compact interpolate function

[elblivion]

Hi,

After reading #2973 and #2710 I decided to try my hand at writing a compact() function to return a StringList devoid of empty strings. This seems to work and passes the unit tests I wrote (caveat: total Go and unit testing n00b), yet when I try to use it on my real world case terraform plan still thinks it has to add a load_balancers field with an empty string:

Module config:

resource "aws_autoscaling_group" "fixed_asg" {
  ...
  load_balancers = [ "${compact(split(",", var.elb_names))}" ]
  ...
}

main config:

module "my_asg" {
  ...
  elb_names             = ""
  ...

Output of terraform plan:

~ module.my_asg.aws_autoscaling_group.fixed_asg
    launch_configuration: "terraform-swff3ybv3jb6jcwbsci2guehu4" => "${aws_launch_configuration.fixed_asg.name}"
    load_balancers.#:     "0" => "1"
    load_balancers.0:     "" => ""

[apparentlymart]

If you write this signature as func (sl Stringlist) Compact() StringList then it can be callable as a method, like stringList.Compact(). Looks like the other StringList functions further up in this file were defined in this way, so I think it'd be nice for consistency.

(Also looks like those StringList methods are sorted in alphabetical order, so maybe push this up to its rightful spot in that ordering.)

[elblivion]

Thanks, fixed that. The functions in interpolate_funcs.go were also ordered alphabetically, I had not noticed - fixed that too.

[apparentlymart]

Looks good to me! Thanks!

I just noted one minor style thing inline.

[elblivion]

@apparentlymart fixed those things; still no idea why the end result isn't what I expected (i.e. 0 load balancers in that example). Something is still making GetOk return a set of values with a single empty string here, I guess: https://github.com/hashicorp/terraform/blob/master/builtin/providers/aws/resource_aws_autoscaling_group.go#L163 😭

[apparentlymart]

Sorry I didn't look more closely at your cover message when I read this the first time, and missed your question. Nothing obvious is jumping out at me right now but if nobody else figures it out first I'll try to debug a bit later in the week and see where that extra empty item is coming from.

[svend]

The compact() function removes empty strings if the list has more than one string. If the list has a single empty string, it does not return an empty list. I think this is because StringList cannot represent empty lists. See #3437

[svend]

@elblivion After applying #3438, I get an empty list

output length {
  value = "${length(compact(split(",", "")))}"
}

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

Outputs:

  length = 0

[elblivion]

@svend awesome! But when I merged your change it broke the tests :-/ I notice the Travis build for your PR also failed, how did you get it to compile?

[elblivion]

@radeksimko please take another look. I've removed one failing test which you introduced in #2157 about a splat with two zero-length lists, this now fails because of the change in the StringList Slice function but I don't know enough about what it's doing to fix it. :-( I've incorporated the changes from @svend's PR #3438.

$ cat test.tf
variable "test" {
  default = ""
}
output length {
  value = "${length(compact(split(",", "${var.test}")))}"
}
output zeroth {
  value = "${element(compact(split(",", "${var.test}")), 0)}"
}

$ terraform apply

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

Outputs:

  length = 0
  zeroth =
$ TF_VAR_test=foo, terraform apply

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

Outputs:

  length = 1
  zeroth = foo
$ TF_VAR_test=,bar terraform apply

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

Outputs:

  length = 1
  zeroth = bar
$ TF_VAR_test=foo,bar terraform apply

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

Outputs:

  length = 2
  zeroth = foo

[elblivion]

This is still not working anyway for my use case of passing an empty list of load_balancers to my AWS ASG module. So it's something else, e.g. what I said here #3239 (comment). 😞

[elblivion]

Actually it does, I was testing it wrong! 🎉 This is good to go from my side /cc @radeksimko @apparentlymart - hopefully could still make it into 0.6.4. :-)

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