Skip to content

Commit

Permalink
Use NLB Subnet CIDRs instead of VPC CIDRs in updateInstanceSecurityGr…
Browse files Browse the repository at this point in the history
…oupsForNLB

Signed-off-by: Pedro Tôrres <t0rr3sp3dr0@gmail.com>
  • Loading branch information
t0rr3sp3dr0 committed Jul 28, 2020
1 parent 498b1e2 commit a2bd59b
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 29 deletions.
31 changes: 29 additions & 2 deletions staging/src/k8s.io/legacy-cloud-providers/aws/aws.go
Original file line number Diff line number Diff line change
Expand Up @@ -3675,6 +3675,27 @@ func buildListener(port v1.ServicePort, annotations map[string]string, sslPorts
return listener, nil
}

func (c *Cloud) getSubnetCidrs(subnetIDs []string) ([]string, error) {
request := &ec2.DescribeSubnetsInput{}
for _, subnetID := range subnetIDs {
request.SubnetIds = append(request.SubnetIds, aws.String(subnetID))
}

subnets, err := c.ec2.DescribeSubnets(request)
if err != nil {
return nil, fmt.Errorf("error querying Subnet for ELB: %q", err)
}
if len(subnets) != len(subnetIDs) {
return nil, fmt.Errorf("error querying Subnet for ELB, got %d subnets for %v", len(subnets), subnetIDs)
}

cidrs := make([]string, 0, len(subnets))
for _, subnet := range subnets {
cidrs = append(cidrs, aws.StringValue(subnet.CidrBlock))
}
return cidrs, nil
}

// EnsureLoadBalancer implements LoadBalancer.EnsureLoadBalancer
func (c *Cloud) EnsureLoadBalancer(ctx context.Context, clusterName string, apiService *v1.Service, nodes []*v1.Node) (*v1.LoadBalancerStatus, error) {
annotations := apiService.Annotations
Expand Down Expand Up @@ -3804,6 +3825,12 @@ func (c *Cloud) EnsureLoadBalancer(ctx context.Context, clusterName string, apiS
return nil, err
}

subnetCidrs, err := c.getSubnetCidrs(subnetIDs)
if err != nil {
klog.Errorf("Error getting subnet cidrs: %q", err)
return nil, err
}

sourceRangeCidrs := []string{}
for cidr := range sourceRanges {
sourceRangeCidrs = append(sourceRangeCidrs, cidr)
Expand All @@ -3812,7 +3839,7 @@ func (c *Cloud) EnsureLoadBalancer(ctx context.Context, clusterName string, apiS
sourceRangeCidrs = append(sourceRangeCidrs, "0.0.0.0/0")
}

err = c.updateInstanceSecurityGroupsForNLB(loadBalancerName, instances, sourceRangeCidrs, v2Mappings)
err = c.updateInstanceSecurityGroupsForNLB(loadBalancerName, instances, subnetCidrs, sourceRangeCidrs, v2Mappings)
if err != nil {
klog.Warningf("Error opening ingress rules for the load balancer to the instances: %q", err)
return nil, err
Expand Down Expand Up @@ -4383,7 +4410,7 @@ func (c *Cloud) EnsureLoadBalancerDeleted(ctx context.Context, clusterName strin
}
}

return c.updateInstanceSecurityGroupsForNLB(loadBalancerName, nil, nil, nil)
return c.updateInstanceSecurityGroupsForNLB(loadBalancerName, nil, nil, nil, nil)
}

lb, err := c.describeLoadBalancer(loadBalancerName)
Expand Down
29 changes: 2 additions & 27 deletions staging/src/k8s.io/legacy-cloud-providers/aws/aws_loadbalancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -716,30 +716,9 @@ func (c *Cloud) ensureTargetGroup(targetGroup *elbv2.TargetGroup, serviceName ty
return targetGroup, nil
}

func (c *Cloud) getVpcCidrBlocks() ([]string, error) {
vpcs, err := c.ec2.DescribeVpcs(&ec2.DescribeVpcsInput{
VpcIds: []*string{aws.String(c.vpcID)},
})
if err != nil {
return nil, fmt.Errorf("error querying VPC for ELB: %q", err)
}
if len(vpcs.Vpcs) != 1 {
return nil, fmt.Errorf("error querying VPC for ELB, got %d vpcs for %s", len(vpcs.Vpcs), c.vpcID)
}

cidrBlocks := make([]string, 0, len(vpcs.Vpcs[0].CidrBlockAssociationSet))
for _, cidr := range vpcs.Vpcs[0].CidrBlockAssociationSet {
if aws.StringValue(cidr.CidrBlockState.State) != ec2.VpcCidrBlockStateCodeAssociated {
continue
}
cidrBlocks = append(cidrBlocks, aws.StringValue(cidr.CidrBlock))
}
return cidrBlocks, nil
}

// updateInstanceSecurityGroupsForNLB will adjust securityGroup's settings to allow inbound traffic into instances from clientCIDRs and portMappings.
// TIP: if either instances or clientCIDRs or portMappings are nil, then the securityGroup rules for lbName are cleared.
func (c *Cloud) updateInstanceSecurityGroupsForNLB(lbName string, instances map[InstanceID]*ec2.Instance, clientCIDRs []string, portMappings []nlbPortMapping) error {
func (c *Cloud) updateInstanceSecurityGroupsForNLB(lbName string, instances map[InstanceID]*ec2.Instance, subnetCIDRs []string, clientCIDRs []string, portMappings []nlbPortMapping) error {
if c.cfg.Global.DisableSecurityGroupIngress {
return nil
}
Expand Down Expand Up @@ -787,14 +766,10 @@ func (c *Cloud) updateInstanceSecurityGroupsForNLB(lbName string, instances map[
}
clientRuleAnnotation := fmt.Sprintf("%s=%s", NLBClientRuleDescription, lbName)
healthRuleAnnotation := fmt.Sprintf("%s=%s", NLBHealthCheckRuleDescription, lbName)
vpcCIDRs, err := c.getVpcCidrBlocks()
if err != nil {
return err
}
for sgID, sg := range clusterSGs {
sgPerms := NewIPPermissionSet(sg.IpPermissions...).Ungroup()
if desiredSGIDs.Has(sgID) {
if err := c.updateInstanceSecurityGroupForNLBTraffic(sgID, sgPerms, healthRuleAnnotation, "tcp", healthCheckPorts, vpcCIDRs); err != nil {
if err := c.updateInstanceSecurityGroupForNLBTraffic(sgID, sgPerms, healthRuleAnnotation, "tcp", healthCheckPorts, subnetCIDRs); err != nil {
return err
}
if err := c.updateInstanceSecurityGroupForNLBTraffic(sgID, sgPerms, clientRuleAnnotation, clientProtocol, clientPorts, clientCIDRs); err != nil {
Expand Down

0 comments on commit a2bd59b

Please sign in to comment.