kubernetes-sigs / aws-load-balancer-controller

A Kubernetes controller for Elastic Load Balancers
https://kubernetes-sigs.github.io/aws-load-balancer-controller/
Apache License 2.0
3.82k stars 1.41k forks source link

Missing "aws-load-balancer-extra-security-groups" annotation #3679

Open dcodix opened 1 month ago

dcodix commented 1 month ago

Is your feature request related to a problem? It is. We were trying to migrate from the standard controller to the aws-load-balancer-controller and we are hitting a wall right now. In the other controller there is this annotation "aws-load-balancer-extra-security-groups" that seems to have been dropped here (or I am failing to make it work).

We are using that annotation to attach to all our LBs a pre-created SG which would open the R53 healthchecks. We are doing it this way because we are using the aws managed prefix list, otherwise we could not list all the IPs needed for the r53 healthchecks to work.

With this controller we don't seem to be able to do so. We still want the "main" SG to be controller by the controller, but we need to add the extra SG managed outside of k8s.

Describe the solution you'd like Just add the functionality for the annotation aws-load-balancer-extra-security-groups

Describe alternatives you've considered If we cannot do this we might need either: 1) Not use this controller and go back to the standard controller which supports the annotation. 2) Write some mini controller that runs in paralel to this that will just discover and attach the SG to the LB given some annotation.

andreybutenko commented 1 month ago

Hi @dcodix, thanks for the question! At this time, the AWS Load Balancer Controller does not support this type of annotation.

We do have two related annotations to specify the security groups to attach. However, these replace the controller-managed security group, rather than attaching additional security groups.

We are open to a contribution for this annotation :)

/kind feature

omerap12 commented 1 month ago

Perhaps I can take this :) @andreybutenko

omerap12 commented 1 month ago

/assign

andreybutenko commented 1 month ago

@omerap12 Awesome, thanks for working on this :) Post here if you need anything!

dcodix commented 1 month ago

Thanks to both @omerap12 and @andreybutenko ! I was also writing something, but I had to stop because of work,... Just wanted to let you know tho, that once I had something written, and I was doing changes on the docs, I saw there is already an annotation aws-load-balancer-security-group-prefix-lists which might be exactly what I need! I did not try to use that one, so I am not sure, but the reason I was attaching an extra SG is just because I had one SG pre-created with a prefix-list.

I will test this soon, and let you know if it solves my particular problem, but even if it works, it may still be a good idea to be able able to add the extra SG for other use cases ?

dcodix commented 1 month ago

Sorry I clicked to create the PR by accident.

omerap12 commented 1 month ago

Hey @dcodix @andreybutenko , I think too that to be able to add an extra SG is a good idea.

omerap12 commented 3 weeks ago

Hey @andreybutenko , Will you be able to take a look at the PR?

andreybutenko commented 3 weeks ago

@omerap12 Thanks for the contribution! I left one minor comment, and shared your PR to get additional reviewers :)

omerap12 commented 3 weeks ago

@omerap12 Thanks for the contribution! I left one minor comment, and shared your PR to get additional reviewers :)

Cool! Ill take a look

omerap12 commented 5 days ago

@omerap12 Thanks for the contribution! I left one minor comment, and shared your PR to get additional reviewers :)

Friendly reminder ;)