Add support for extra security groups annotation (NLB)

[omerap12]

Issue

#3679

Description

Add the annotation service.beta.kubernetes.io/aws-load-balancer-extra-security-groups to attach an existing security group to the NLB.

Add a loop in pkg/service/model_build_load_balancer.go to retrieve all security groups from the annotation service.beta.kubernetes.io/aws-load-balancer-extra-security-groups and append them to the existing security groups list.

Made local testing, for the following yaml fille:

apiVersion: v1
kind: Service
metadata:
  name: nlb-sample-service
  namespace: default
  annotations:
    service.beta.kubernetes.io/aws-load-balancer-name: "test-new-annotation"
    service.beta.kubernetes.io/aws-load-balancer-extra-security-groups: sg-0b036fd0124715a4c
    service.beta.kubernetes.io/aws-load-balancer-security-groups: sg-0c721856c63fba26b
spec:
  ports:
    - port: 80
      targetPort: 80
      protocol: TCP
  type: LoadBalancer
  selector:
    app: nginx

And this:

apiVersion: v1
kind: Service
metadata:
  name: nlb-sample-service
  namespace: default
  annotations:
    service.beta.kubernetes.io/aws-load-balancer-name: "test-new-annotation"
    service.beta.kubernetes.io/aws-load-balancer-extra-security-groups: test-for-alb-2
    service.beta.kubernetes.io/aws-load-balancer-security-groups: sg-0c721856c63fba26b
spec:
  ports:
    - port: 80
      targetPort: 80
      protocol: TCP
  type: LoadBalancer
  selector:
    app: nginx

The following nlb was created:

As you can see, the extra sg was added to the security groups list.

Checklist

• [ ] Added tests that cover your change (if possible)
• [x] Added/modified documentation as required (such as the README.md, or the docs directory)
• [x] Manually tested
• [x] 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?! :exploding_head:

• [ ] Backfilled missing tests for code in same general area :tada:
• [ ] Refactored something and made the world a better place :star2:

[omerap12]

I don't think the failures are due to my PR, right?

[omerap12]

Thanks for the contribution! Left one small comment, sharing your PR internally for additional reviewers

Hey, any update?

[andreybutenko]

Hi @oliviassss @wweiwei-li , could you please review this PR too? It is related to #3679 which we previously discussed. Thank you! :)

[wweiwei-li]

/lgtm

[wweiwei-li]

/retest

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: andreybutenko, omerap12 Once this PR has been reviewed and has the lgtm label, please assign oliviassss for approval. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

• OWNERS

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

[omerap12]

Friendly reminder :) @oliviassss

[shraddhabang]

/lgtm

[oliviassss]

@omerap12, sorry for the delayed reply. I'm a bit confused on the use-case. so if you're already using service.beta.kubernetes.io/aws-load-balancer-security-groups, which means the controller is not managing the SGs for you. why not just attach extra SGs in this annotation, like

service.beta.kubernetes.io/aws-load-balancer-security-groups: sg0, extraSG0, extraSG1

I'm just trying to avoid redundant annotations here. Thanks

[k8s-ci-robot]

PR needs rebase.

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-sigs/prow repository.