Add alb_name variable to specify load balancer name

[adamantike]

What

• Add an alb_name variable to avoid using the default label id, if set.

Why

• According to AWS documentation, the Load Balancer name is limited to 32 characters.
• This adds a way to avoid issues like this one, while maintaining support for setting any labels or tags to the related resource:

Error: "name" cannot be longer than 32 characters

  on main.tf line 71, in resource "aws_lb" "default":
  71: resource "aws_lb" "default" {

References

• Slightly related to ALB target group name is limited to 32 characters #27, but for a different field.
• AWS documentation: https://docs.aws.amazon.com/elasticloadbalancing/2012-06-01/APIReference/API_CreateLoadBalancer.html

[adamantike]

Is there a chance for this MR to get reviewed? Thanks!

[Gowiem]

@adamantike If you wanted to provide just a blanket name to the ALB resource then you can just provide var.name and not provide the other label attributes (var.namespace, var.stage, etc). I haven't seen this pattern introduced elsewhere and I'm not sure we'd want to add this additional override unless it would be also useful to others.

@aknysh what are your thoughts here? I'm sure you've run into this type of request / PR before.

[adamantike]

Thanks for the feedback, @Gowiem!

var.name would indeed allow us to set the ALB name, but as you mentioned, it comes with a few drawbacks:

• No other null-label variables that are used to build the label's id can be set (which could be counter-intuitive, for people using multiple CloudPosse's modules).
• With the current implementation, the created default security group also needs to share this same name.

The main motivation for this PR, is that load balancer names are limited to 32 characters, which could be a reason for not having this pattern in other CloudPosse's modules, as the autogenerated label's id is fine when there are no length limitations.

Other alternatives could be to expose null-label's id_length_limit variable, or trim the name argument when instantiating the aws_lb resource. Both solutions would also help with the load balancer's name limitation, but would also reduce the flexibility of the user to provide a custom shorter name, though.

[Gowiem]

Ah @adamantike, you're just looking to limit the name length. Sorry, I didn't read your "Why" section it seems - my bad! I think introducing the id_length_limit variable would be the way to go here. While I get your point about flexibility, I think we'd prefer to go that route over providing a new variable.

Considering that, would you want to do a context.tf upgrade (like this) for this repository instead? That would add the id_length_limit var and get this module closer to our latest and greatest patterns. If that's too much work, then I would also accept just the addition of the id_length_limit var 😄

[jamengual]

mmmm this does not seem to be needed since is already possible to with the current module

 name                      = "nameoftheloadbalancer"

That will set the loadbalancer with whatever name you want, without using a label module

but I think in this case what @Gowiem recommend with the context.tf is a better and more complete way

[Gowiem]

@adamantike please check out the comments above. I'm going to close this out considering there are better ways of implementing the desired functionality. Thanks!