Extend this module to support NLB

[antonbabenko]

Is your feature request related to a problem? Please describe.

ALB and NLB are rather close in terms of API in AWS and Terraform AWS providers, so maybe extend this module and allow users to specify load_balancer_type here:

terraform-aws-alb/alb_no_logs.tf

Line 2 in 22b380c

load_balancer_type = "application"

Describe alternatives you've considered

Alternatively, this module can be renamed to terraform-aws-lb and support both ALB and NLB.

Additional context

I have not heard requests about NLB so often to consider this as an urgent feature. What do you think @brandoconnor ?

[RIMikeC]

I'd like to add my support for NLB inclusion. I use NLBs for PrivateLink services
https://docs.aws.amazon.com/AmazonVPC/latest/UserGuide/endpoint-service.html
There's no suggestion that classic load-balancers will ever be able to present such services

Anton's suggestion of load_balancer_type = "network" seems like a good way forward to me, as it mirrors the syntax we're used to seeing in aws_lb resources.

[brandonjbjelland]

Hey @antonbabenko and @RIMikeC 🖖

Thanks for chiming in with a feature request. I've actually put some serious thought into this before and started down the path of implementation a while back but it wasn't successful. What I found is that the provider maintainers aren't actually so happy with how NLB has been implemented under the aws_lb resource type and are back to considering breaking the load balancers out into separate resources. Namely, some of the arguments that purport to work across both load balancer types, don't actually work on the NLB side and instead produce error or silently fail.

A PR is still open with some details and fixes. This issue shows some of the problems I describe have been resolved.

Perhaps the state of the upstream provider has improved enough. If that's the case, feel free to package up an NLB test case and add the functionality. Given how hairy the addition of optional was, I would be surprised (but also delighted) if the change were as simple as making the network_load_balancer_type a variable. Feel free to take a stab at the PR. If I have some spare cycles over the next week or so, I may give it a go.

Thanks again on the feature request. If the NLB remains overly specific and can't be added here, it may be worth splitting out into its own repo and module.

[antonbabenko]

Looking at the implementation of current ALB module it seems that if changing of load_balancer_type is not enough then we can create another aws_lb resource with NLB-specific arguments and make it transparent for the user.

I'll leave this to @RIMikeC to submit a PR.

[sa-shukla]

In terms of implementation, what kind of code needs to be written? Who decides on the variables naming convention, etc?

[brandonjbjelland]

What sort of code needs to be written?

Terraform code ;)

Who decides on naming convention?

The rest of the codebase should give you a pretty good idea but I usually provide that feedback of any is needed. It's not too painful a process (I hope!).

[snemetz]

I added support for NLB in my fork, but I've also done a number of changes making it incompatible for a PR
https://github.com/devops-workflow/terraform-aws-lb

Looks like I'll probably be updating it to add more features soon

[antonbabenko]

v5.0.0 has been just released with NLB support (and more).

Check UPGRADE-5.0.md for upgrade instructions.

[github-actions]

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.