Improve flexibility in defining the list of Load Balancers associated with an AWS Auto Scaling Group

[ketzacoatl]

As it is now, the ASG resource accepts one of two values for the load_balancers attribute, either: [ ] or [ "name_of_an_elb"], but you cannot provide the empty list as the value in a variable.

That might sound whacky, I know.. here is my reasoning.. first I don't want to have to maintain two modules if I want to optionally include an ELB with the ASG, but he lack of flexibility here forces one to do so. If support for conditional logic (via #1604) is not coming soon, I would like to propose a more immediate solution focused on improving flexibility in the ASG resource. Specifically, if the following code were to work correctly, we would be able to build modules which provide an ELB only if requested:

variable "elb_list" {
    default = "[ ]"
    description = "String literal as a list of AWS ELB to associate with the ASG, defaults to an empty string"
}

resource "aws_autoscaling_group" "app" {
    ...
    launch_configuration = "${aws_launch_configuration.v1.name}"
    load_balancers = "${var.elb_list}"
    ...
}

...

We could then provide [ "foo", "bar" ] as a value when calling the module, or leave the [ ] empty default, and the ASG would accept our requests. Maybe this sounds whacky, but it is true, without the basic constructs provided by conditional logic, we're getting desperate in an effort to keep the code compact and minimal (maintaining two ASG modules - one with ELB, one without - is not really a reasonable option).

[phinze]

but you cannot provide the empty list as the value in a variable.

After #2504 we can almost do this:

This does not yet work:

variable "elb_names" {
  description = "comma separated list of ELB names to associate with ASG"
  default = ""
}

module "ami" {
  source = "github.com/terraform-community-modules/tf_aws_ubuntu_ami/ebs"
  instance_type = "t2.micro"
  region = "us-west-2"
  distribution = "trusty"
}

resource "aws_launch_configuration" "app" {
  instance_type = "t2.micro"
  image_id = "${module.ami.ami_id}"
}

resource "aws_autoscaling_group" "app" {
  availability_zones   = [ "us-west-2a" ]
  launch_configuration = "${aws_launch_configuration.app.id}"
  max_size             = 1
  min_size             = 1
  name                 = "GH-2710"

  // results in [""], which AWS API treats as a validation error
  load_balancers = ["${split(",", var.elb_names)}"]
}

We need to either:

• add some sort of compact([...]) or without([...], "") function to remove the empty elements from the list
• add support into the ASG resource to ignore empty load balancers

[ketzacoatl]

My initial sense was that the ASG resource would simply accept, but ignore an empty list of LB.. but I know very little about go or the tf codebase.

[phinze]

@ketzacoatl yeah that's definitely the easier of the two options. seems to be a reasonable path to take for this particular issue.

[ketzacoatl]

I'm looking at the code in https://github.com/hashicorp/terraform/blob/master/builtin/providers/aws/resource_aws_autoscaling_group.go, but am not familiar enough with Go to pick up on the edits to make this work, otherwise I would submit the PR for it :(

[elblivion]

Hi, I've hit this same issue. I've got this working in https://github.com/elblivion/terraform/tree/ignore-empty-lb-list-as-string, but terraform plan always says it needs to modify the ASG to add an ELB, even though on an apply it works anyway. I still don't know the code base so well, is there some simple way of letting Terraform know that we want to skip a particular change? (sounds very kludgey :-/)

[elblivion]

@phinze is there a reason split() shouldn't return an empty list ([]) when the resulting list is a single empty string? There may be a use I cannot fathom but in my limited (AWS-related) viewpoint I can't really think of a reason for not simply filtering empty strings out of the result of any split().

[elblivion]

Ah, just noticed there is a PR open about this at #2973, will look at that

[skippy]

This would be great. I'm running into the same issue outlined by @ketzacoatl .

@elblivion I think this will remain an issue even after split() because the code is checking for nil, not nil || empty https://github.com/hashicorp/terraform/blob/master/builtin/providers/aws/resource_aws_ecs_service.go#L190

[skippy]

@ketzacoatl one workaround that may help you remove a duplicate module, is to do something like this:

resource "aws_autoscaling_group" "blue" {
  name = "${var.name}-blue"

# other stuff here

  provisioner "local-exec" {
    command = "[[ ! -z \"${var.load_balancers}\" ]] && aws autoscaling attach-load-balancers --auto-scaling-group-name \"${aws_autoscaling_group.blue.name}\" --load-balancer-names ${var.load_balancers} || true"
  }
}

then in your module, you default var.load_balancers to an empty string. If you need to pass in load balancers, just make sure they are a proper comma-delimited list (and make sure you don't have spaces or characters that need to be quoted)

[elblivion]

This should now be possible using compact() after #3239 was merged. :-)

[ketzacoatl]

This is true, compact() works very well here, thank you @elblivion!

[ketzacoatl]

Though I have to say, while I appreciate the power available through interpolation and built-in functions, I'm sometimes frustrated by the messy code that results:

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

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