aws-load-balancer-security-groups annotation is updating neighbour VPC SG if its in same AWS account

[saiteja313]

What happened:
While creating service with LoadBalance type, when a Security group from different VPC (non VPC of Kubernetes Cluster deployed, same AWS account) is referenced. Security Group update is done successfully without checking its VPC and is left behind by failling ELB creation.

Error:

Warning  CreatingLoadBalancerFailed  39s  service-controller  Error creating load balancer (will retry): failed to ensure load balancer for service appmesh-system/nginx: InvalidConfigurationRequest: Security group "sg-04cb4d54" does not belong to VPC "vpc-0121ce8563b0fcb56"

What you expected to happen:

There should be a guard rail that checks if,

• SG ID belongs to same VPC,
• SG should have cluster tag 'kubernetes.io/cluster/'
  and fail updating Security group.

How to reproduce it (as minimally and precisely as possible):

1. Create a deployment

   $ kubectl run nginx --image nginx --dry-run -o yaml
   

2. Select a Security group Id from a different VPC (same AWS account) and describe it.

   $ aws ec2 describe-security-groups --group-ids sg-xxxxxx > sg-describe.json
   

   Note: Make sure there are some dummy Ingress rules on the SG. We will describe SG again after step 3 and will compare results.
3. Expose 'nginx' deployment as Service with type LoadBalancer, ref annotation service.beta.kubernetes.io/aws-load-balancer-security-groups. Update 'sg-xxxx' with security group from step 2 in below file.

   apiVersion: v1
   kind: Service
   metadata:
   creationTimestamp: null
   labels:
       run: nginx
   name: nginx
   annotations:
       service.beta.kubernetes.io/aws-load-balancer-additional-resource-tags: "auto-delete=no"
       service.beta.kubernetes.io/aws-load-balancer-security-groups: sg-xxxx
   spec:
   ports:
   - port: 80
       name: status-port1
       protocol: TCP
       targetPort: 80
   - port: 81
       name: status-port2
       protocol: TCP
       targetPort: 80
   selector:
       run: nginx
   type: LoadBalancer
   

4. Service will be in <pending> state, it should show warnings

   $ kubectl get svc -l run=nginx
   $ kubectl describe svc nginx
   
       Events:
       Type     Reason                      Age                 From                Message
       ----     ------                      ----                ----                -------
       Warning  CreatingLoadBalancerFailed  49m (x10 over 94m)  service-controller  (combined from similar events): Error creating load balancer (will retry): failed to ensure load balancer for service appmesh-system/nginx: InvalidConfigurationRequest: Security group "sg-04cb4d54" does not belong to VPC "vpc-0121ce8563b0fcb56"
               status code: 409, request id: 2801210a-6d2e-4ea7-aaaf-bd54fe84780f
       Normal   EnsuringLoadBalancer  4m28s (x28 over 115m)  service-controller  Ensuring load balancer
   

5. Describe security group again and it should have overrided ingress rules port 80 and 81

   $ aws ec2 describe-security-groups --group-ids sg-xxxxxx > sg-describe2.json
   

6. Compare sg-describe.json and sg-describe2.json should show overrided rules.

Anything else we need to know?:

Environment:

• Kubernetes version (use kubectl version):

  $ kubectl version
      Client Version: version.Info{Major:"1", Minor:"17", GitVersion:"v1.17.3", GitCommit:"06ad960bfd03b39c8310aaf92d1e7c12ce618213", GitTreeState:"clean", BuildDate:"2020-02-13T18:06:54Z", GoVersion:"go1.13.8", Compiler:"gc", Platform:"darwin/amd64"}
      Server Version: version.Info{Major:"1", Minor:"14+", GitVersion:"v1.14.9-eks-c0eccc", GitCommit:"c0eccca51d7500bb03b2f163dd8d534ffeb2f7a2", GitTreeState:"clean", BuildDate:"2019-12-22T23:14:11Z", GoVersion:"go1.12.12", Compiler:"gc", Platform:"linux/amd64"}
  

• Cloud provider or hardware configuration: Cluster created using AWS EKS service.

/kind bug

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

[fejta-bot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

[k8s-ci-robot]

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

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.

[M00nF1sh]

It's a misconfiguration, but we can do improve it by add more client-side validation.