SPLAT-2113: spike NLB with SGs when annotation is present

Author: mtulio

pkg/providers/v1/aws.go

@@ -2255,6 +2255,22 @@ func (c *Cloud) EnsureLoadBalancer(ctx context.Context, clusterName string, apiS
 			instanceIDs = append(instanceIDs, string(id))
 		}
 
+		// TODO: get the list of SGs when it requires to add it (only if NLB was created with one)
+		// Check if SG annotation has been added and SG exists, then build the required permissions.
+		if _, present := annotations[ServiceAnnotationLoadBalancerSecurityGroups]; present {
+			securityGroups, _, err := c.buildELBSecurityGroupList(serviceName, loadBalancerName, annotations)
+			if err != nil {
+				return nil, err
+			}
+			if len(securityGroups) == 0 {
+				return nil, fmt.Errorf("NLB must be created with an Security Group to allow syncronization, please recrease the service with a security group")
+			}
+			permissions := buildSecuritySecurityGroupPermissions(apiService, sourceRanges)
+			if _, err = c.setSecurityGroupIngress(securityGroups[0], permissions); err != nil {
+				return nil, err
+			}
+		}
+
 		v2LoadBalancer, err := c.ensureLoadBalancerv2(
 			serviceName,
 			loadBalancerName,
@@ -2435,36 +2451,7 @@ func (c *Cloud) EnsureLoadBalancer(ctx context.Context, clusterName string, apiS
 	}
 
 	if setupSg {
-		ec2SourceRanges := []*ec2.IpRange{}
-		for _, sourceRange := range sourceRanges.StringSlice() {
-			ec2SourceRanges = append(ec2SourceRanges, &ec2.IpRange{CidrIp: aws.String(sourceRange)})
-		}
-
-		permissions := NewIPPermissionSet()
-		for _, port := range apiService.Spec.Ports {
-			portInt64 := int64(port.Port)
-			protocol := strings.ToLower(string(port.Protocol))
-
-			permission := &ec2.IpPermission{}
-			permission.FromPort = &portInt64
-			permission.ToPort = &portInt64
-			permission.IpRanges = ec2SourceRanges
-			permission.IpProtocol = &protocol
-
-			permissions.Insert(permission)
-		}
-
-		// Allow ICMP fragmentation packets, important for MTU discovery
-		{
-			permission := &ec2.IpPermission{
-				IpProtocol: aws.String("icmp"),
-				FromPort:   aws.Int64(3),
-				ToPort:     aws.Int64(4),
-				IpRanges:   ec2SourceRanges,
-			}
-
-			permissions.Insert(permission)
-		}
+		permissions := buildSecuritySecurityGroupPermissions(apiService, sourceRanges)
 		_, err = c.setSecurityGroupIngress(securityGroupIDs[0], permissions)
 		if err != nil {
 			return nil, err
@@ -2573,6 +2560,41 @@ func (c *Cloud) EnsureLoadBalancer(ctx context.Context, clusterName string, apiS
 	return status, nil
 }
 
+func buildSecuritySecurityGroupPermissions(apiService *v1.Service, sourceRanges netutils.IPNetSet) IPPermissionSet {
+	ec2SourceRanges := []*ec2.IpRange{}
+	for _, sourceRange := range sourceRanges.StringSlice() {
+		ec2SourceRanges = append(ec2SourceRanges, &ec2.IpRange{CidrIp: aws.String(sourceRange)})
+	}
+
+	permissions := NewIPPermissionSet()
+	for _, port := range apiService.Spec.Ports {
+		portInt64 := int64(port.Port)
+		protocol := strings.ToLower(string(port.Protocol))
+
+		permission := &ec2.IpPermission{}
+		permission.FromPort = &portInt64
+		permission.ToPort = &portInt64
+		permission.IpRanges = ec2SourceRanges
+		permission.IpProtocol = &protocol
+
+		permissions.Insert(permission)
+	}
+
+	// Allow ICMP fragmentation packets, important for MTU discovery
+	{
+		permission := &ec2.IpPermission{
+			IpProtocol: aws.String("icmp"),
+			FromPort:   aws.Int64(3),
+			ToPort:     aws.Int64(4),
+			IpRanges:   ec2SourceRanges,
+		}
+
+		permissions.Insert(permission)
+	}
+
+	return permissions
+}
+
 // GetLoadBalancer is an implementation of LoadBalancer.GetLoadBalancer
 func (c *Cloud) GetLoadBalancer(ctx context.Context, clusterName string, service *v1.Service) (*v1.LoadBalancerStatus, bool, error) {
 	if isLBExternal(service.Annotations) {
@@ -2898,6 +2920,8 @@ func (c *Cloud) EnsureLoadBalancerDeleted(ctx context.Context, clusterName strin
 			}
 		}
 
+		// TODO add delete SG
+
 		return c.updateInstanceSecurityGroupsForNLB(loadBalancerName, nil, nil, nil, nil)
 	}
 
@@ -3068,6 +3092,11 @@ func (c *Cloud) UpdateLoadBalancer(ctx context.Context, clusterName string, serv
 			return fmt.Errorf("Load balancer not found")
 		}
 		_, err = c.EnsureLoadBalancer(ctx, clusterName, service, nodes)
+		if err != nil {
+			return err
+		}
+
+		// TODO ensure security groups when defined
 		return err
 	}
 	lb, err := c.describeLoadBalancer(loadBalancerName)

pkg/providers/v1/aws_loadbalancer.go

@@ -177,6 +177,17 @@ func (c *Cloud) ensureLoadBalancerv2(namespacedName types.NamespacedName, loadBa
 		// TODO: What happens if we have more than one subnet per AZ?
 		createRequest.SubnetMappings = createSubnetMappings(discoveredSubnetIDs, allocationIDs)
 
+		if sgList, present := annotations[ServiceAnnotationLoadBalancerSecurityGroups]; present {
+			// assuming SGs already been verified (if exists or not)
+			klog.Infof("Setting up NLB with Security Groups: %v", sgList)
+			for _, sg := range strings.Split(sgList, ",") {
+				createRequest.SecurityGroups = append(createRequest.SecurityGroups, aws.String(sg))
+			}
+			// TODO(mtulio): check SGs exists, otherwise return fail
+			// TODO check if need to manage the backend rules
+			// createRequest.SecurityGroups =
+		}
+
 		for k, v := range tags {
 			createRequest.Tags = append(createRequest.Tags, &elbv2.Tag{
 				Key: aws.String(k), Value: aws.String(v),
@@ -381,6 +392,12 @@ func (c *Cloud) ensureLoadBalancerv2(namespacedName types.NamespacedName, loadBa
 			return nil, err
 		}
 
+		// TODO: reconcile Security Groups.
+		// Security Groups reconciliation is supported only when LB has been created with a SG.
+		// TODO#1: check Security Groups is enabled in the NLB
+		// TODO#1.1: reconcile SG from LB annotation
+		// TODO#1.2: reconcile extra SG from LB annotation
+
 		// Subnets cannot be modified on NLBs
 		if dirty {
 			loadBalancers, err := c.elbv2.DescribeLoadBalancers(
@@ -819,6 +836,31 @@ func (c *Cloud) updateInstanceSecurityGroupsForNLB(lbName string, instances map[
 		}
 	}
 
+	// This block of code is responsible for updating the security group rules for instances
+	// associated with a Network Load Balancer (NLB) in AWS. It ensures that the security group
+	// rules allow traffic from specified client CIDRs and health check ports, as well as
+	// handling MTU discovery rules for the NLB.
+	//
+	// Key steps in the process:
+	// 1. Initialize sets for client ports, health check ports, and determine the client protocol
+	//    (TCP or UDP) based on the provided port mappings.
+	// 2. Iterate through the port mappings to populate the client ports and health check ports.
+	//    If a custom health check port is specified, it is parsed and validated.
+	// 3. Construct annotations for client and health check rules, which are used to identify
+	//    the rules in the security group.
+	// 4. For each security group associated with the cluster:
+	//    - If the security group is desired (i.e., associated with an instance in the cluster):
+	//      - Add rules for health check traffic if the health check rules are not a subset of
+	//        the client rules.
+	//      - Add rules for client traffic.
+	//    - If the security group is not desired, remove any existing rules for health check
+	//      and client traffic.
+	//    - Update MTU discovery rules if necessary.
+	// 5. The `updateInstanceSecurityGroupForNLBTraffic` function is used to manage the specific
+	//    permissions for traffic, ensuring that the security group rules match the desired state.
+	//
+	// This ensures that the security group rules are correctly configured to allow traffic
+	// to and from the NLB, while also cleaning up any unnecessary rules.
 	{
 		clientPorts := sets.Int64{}
 		clientProtocol := "tcp"