Enables providing multiple tags for worker node security group discovery

[carflo]

Issue

#2367

Description

• Adds a new flag --service-target-eni-security-group-tags to allow users to specify additional tags that should be used when the controller looks for the security group to use when adding ingress rules for NLB targets. This addresses the limitation called out in the documentation here.

Example: Using --service-target-eni-security-group-tags=Name=aws-lbc-nlb-sg will look for a security group that has the required kubernetes.io/cluster/${cluster-name} tag AND a Name=aws-lbc-nlb-sg tag.

Other:

• Fixes a couple typos in documentation.
• Removes trailing whitespace in documentation.

Checklist

• Added tests that cover your change (if possible)
• Added/modified documentation as required (such as the README.md, or the docs directory)
• Manually tested
• Made sure the title of the PR is a good description that can go into the release notes

BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯

• Backfilled missing tests for code in same general area 🎉
• Refactored something and made the world a better place 🌟

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: carflo / name: Carlos Flores (45d93b1, 5ad2025, 0d71626, 3e6e07c, a42369b, 8bf0c4f, 843e60f, d8a103e, 397753d, 582c1a7, ef54d46, 2adaeb6, 710ef1d, 9375a3c, fd59e21, 1c8a3cc, f4fb676, ad29cc0, 1f6503d, b0ec7ee, ca8a02f, f3a9742)

[k8s-ci-robot]

Welcome @carflo!

It looks like this is your first PR to kubernetes-sigs/aws-load-balancer-controller 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/aws-load-balancer-controller has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

Hi @carflo. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[armenr]

:) Really looking forward to this PR!

[Pacobart]

Looking forward to this much needed functionality!

[ricardorqr]

Any update on this PR?

[carflo]

@kishorj @johngmyers is there anything else I can do to get a review from you folks? Thanks in advance 🙇

[johngmyers]

It's in my queue

[johngmyers]

The TargetGroupBinding reconciler is not an area I'm overly familiar with, so I'm learning as I go.

The name of the flag doesn't say much about what it does:

• "endpoint" is ambiguous. Perhaps use something more like target-eni-security-group-tags
• This is documented in the section specific to Services and the description suggests that it is not relevant to Ingresses. I haven't yet found where in the code this gets bypassed for the ALB case. If it is specific to Service, then the flag name should start with service-.

[johngmyers]

/ok-to-test

[M00nF1sh]

left some minor comments.

this seems an alternative solution to #2367, but requires more manual intervention(extra tags on SGs).

I'm good with this change as a temporary solution and it works for customers that only want one SG to be modified.

[carflo]

/retest

[johngmyers]

/lgtm

[M00nF1sh]

/lgtm
/approve
Thanks for making this contribution :D

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: carflo, M00nF1sh

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [M00nF1sh]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[codecov-commenter]

Codecov Report

Patch coverage: 51.85% and project coverage change: +1.08% 🎉

Comparison is base (940efc7) 54.70% compared to head (f3a9742) 55.78%.
Report is 52 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3147      +/-   ##
==========================================
+ Coverage   54.70%   55.78%   +1.08%     
==========================================
  Files         148      149       +1     
  Lines        8590     8830     +240     
==========================================
+ Hits         4699     4926     +227     
- Misses       3559     3566       +7     
- Partials      332      338       +6     

Files Changed	Coverage Δ
pkg/config/controller_config.go	14.92% <0.00%> (-0.46%)	⬇️
pkg/targetgroupbinding/resource_manager.go	15.17% <0.00%> (ø)
pkg/targetgroupbinding/networking_manager.go	40.84% <58.33%> (+6.29%)	⬆️

... and 13 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.