Add support for shared subnets

[rifelpet]

This allows the aws-alb-ingress-controller to create ALBs in subnets that are shared across accounts via AWS Resource Access Manager. See #901 for more details.

This currently requires that security groups be defined in the ingress annotation. I couldn't figure out an easy way to get the desired security group ID from the security group AssociationController into newLBInstance, so I'm settling for the annotation requirement for now.

Closes #901
Closes #865

[rifelpet]

/test pull-aws-alb-ingress-controller-unit-test

[M00nF1sh]

Thanks for making this change :D. I'll take the time to fix the unit test issues(seems coverall is down).

I have make changes around securityGroup handling in my ingress group implementation, which will make managed LB securityGroup work as well(not ready to release out yet).

BTW,
Another change I did is removing the need for auto-generate node securityGroups. Instead, I directly adding an rule to existing node securityGroup that allow ingress traffic from LB securityGroup(similar to what we did for CLB/NLB).
This offers some benefits:

1. do not need to deal with attach/detach node securityGroup from ENIs based on pod IPs.
2. solves Deregistration delay doesn't seem to have any effect, since the security group is detached too early from the ENI #824 and Only 5 SGs per ENI allowed #682 without extra work.
3. supports IP modes for kubenet without extra work.

It do have some "drawbacks" from the current solution(create an separate node sg per ALB, and attach/detach from ENIs as needed). But I don't consider it as less secure since ALB should be inside trust boundary. Also, securityGroup can only be attached to ENI instead of pod IP, there is no fundamental difference from just attach it to all ENIs.

Is this fine to you?

[M00nF1sh]

/test pull-aws-alb-ingress-controller-unit-test

[rifelpet]

Interesting idea regarding security groups. If a node has multiple security groups attached, how will the controller choose which security group to update? We define one of our nodes' security groups in terraform and would prefer it not be changed by anything but terraform. If we can choose which security group the controller will update, or have the controller create one new security group, attach it to nodes, and use it for all updates, that would be great.

[M00nF1sh]

For each node, the securityGroup will be choose as follows(follow same logic as kubernetes support for CLB/NLB):
If there are multiple SG tagged with cluster tag, an error will be rised.
Else If there are one SG tagged with cluster tag, it will be choosen.
Else if there only one un-tagged securityGroup, it will be choosen.

The rules will be updated for all choose securityGroup to allow ingress traffic from auto-generated LB securityGroup.
Will this be good enough to not have an auto-created node securityGroup for nodes? or there are some use case i missed that requires it.

BTW, for We define one of our nodes' security groups in terraform and would prefer it not be changed by anything but terraform, if you use CLB/NLB(serviceType=LoadBalancer), kubenets will update that securityGroup using rules i described above.

[rifelpet]

That makes sense and is sufficient for our situation. thanks!

[rifelpet]

/test pull-aws-alb-ingress-controller-unit-test

[rifelpet]

/test pull-aws-alb-ingress-controller-unit-test

[rifelpet]

@M00nF1sh any idea why the unit tests are failing?

[M00nF1sh]

@M00nF1sh any idea why the unit tests are failing?

#915 will fix it 😄

[rifelpet]

@M00nF1sh I rebased and all tests now pass. Thanks for fixing that.

[M00nF1sh]

an caveat: you'll need to manually clean up the LB when you delete the loadbalancer.(otherwise, this will fail deletion in shared subnets case)

/lgtm

[M00nF1sh]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

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

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