feat: Introduce annotation for setting explicit egress CIDR ranges on Service LB frontend security group

Author: mtweten

docs/guide/service/annotations.md

@@ -65,7 +65,8 @@
 | [service.beta.kubernetes.io/aws-load-balancer-minimum-load-balancer-capacity](#load-balancer-capacity-reservation)   | stringMap                  |                          |
 | [service.beta.kubernetes.io/aws-load-balancer-enable-icmp-for-path-mtu-discovery](#icmp-path-mtu-discovery)          | string                  |                          | If specified, a security group rule is added to the managed security group to allow explicit ICMP traffic for [Path MTU discovery](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/network_mtu.html#path_mtu_discovery) for IPv4 and dual-stack VPCs. Creates a rule for each source range if `service.beta.kubernetes.io/load-balancer-source-ranges` is present.                                               |
 | [service.beta.kubernetes.io/aws-load-balancer-enable-tcp-udp-listener](#tcp-udp-listener)                            | boolean                  | false                    | If specified, the controller will attempt to try TCP_UDP Listeners when the service defines a TCP and UDP port on the same port number.                                                                                                                                                                                                                                                                              |
-| [service.beta.kubernetes.io/aws-load-balancer-disable-nlb-sg](#nlb-sg-disable)                            | boolean                  | false                    | If specified, the controller will not create or manage Security Groups for the service.                                                                                                                                                                                                                                                                                                                              |
+| [service.beta.kubernetes.io/aws-load-balancer-disable-nlb-sg](#nlb-sg-disable)                                       | boolean                  | false                    | If specified, the controller will not create or manage Security Groups for the service.                                                                                                                                                                                                                                                                                                                              |
+| [service.beta.kubernetes.io/aws-load-balancer-outbound-cidrs](#outbound-cidrs)                                       | stringList               |                          | If specified, the controller will add the CIDR ranges as egress rules to the managed frontend security group, instead of relying on the default AWS `0.0.0.0/0` egress rule. If not set, aws-load-balancer-controller will maintain previous behavior and not manage egress rules at all.                                                                                                                            |
 
 ## Traffic Routing
 Traffic Routing can be controlled with following annotations:
@@ -354,7 +355,6 @@ for proxy protocol v2 configuration.
     service.beta.kubernetes.io/aws-load-balancer-disable-nlb-sg: "true"
     ```
 
-
 - <a name="deprecated-attributes"></a>the following annotations are deprecated in v2.3.0 release in favor of [service.beta.kubernetes.io/aws-load-balancer-attributes](#load-balancer-attributes)
 
     !!!note ""
@@ -621,6 +621,21 @@ Load balancer access can be controlled via following annotations:
         service.beta.kubernetes.io/aws-load-balancer-inbound-sg-rules-on-private-link-traffic: "off"
         ```
 
+- <a name="outbound-cidrs">`service.beta.kubernetes.io/aws-load-balancer-outbound-cidrs`</a> allows specifying a comma-delimited list of CIDR ranges to be added as egress rules to the frontend security group.
+
+    !!!note ""
+        - Historically, `aws-load-balancer-controller` hasn't explicitly added any egress rules to managed frontend security groups - instead, it relies on the fact that AWS will add a default `0.0.0.0/0` outbound egress rule for all SGs created without an explicit egress rule list. This is required for the load balancer to be able to talk to the target group and potentially other services (e.g. CloudWatch).
+        
+        - However, some organizations may have issues with the default `0.0.0.0/0` egress rule (e.g. security scanners may flag them) and would rather be able to further limit the rule to a specific set of CIDR range(s). This annotation allows that.
+    
+    !!!warning "Note"
+        - If this annotation is not present, `aws-load-balancer-controller` will effectively not manage egress rules at all, maintaining the behavior before the annotation was added. This means that if the annotation is added to a service to set the egress security group rules and then subsequently removed, the egress security group rule will not be removed automatically.
+
+    !!!example
+    ```
+    service.beta.kubernetes.io/aws-load-balancer-outbound-cidrs: "172.18.0.0/16"
+    ```
+
 ## Capacity Unit Reservation
 Load balancer capacity unit reservation can be configured via following annotations:
 

pkg/annotations/constants.go

@@ -124,4 +124,5 @@ const (
 	SvcLBSuffixEnableIcmpForPathMtuDiscovery             = "aws-load-balancer-enable-icmp-for-path-mtu-discovery"
 	SvcLBSuffixEnableTCPUDPListener                      = "aws-load-balancer-enable-tcp-udp-listener"
 	SvcLBSuffixDisableNLBSG                              = "aws-load-balancer-disable-nlb-sg"
+	SvcLBSuffixOutboundCIDRs                             = "aws-load-balancer-outbound-cidrs"
 )

pkg/aws/services/ec2.go

@@ -33,7 +33,9 @@ type EC2 interface {
 	CreateSecurityGroupWithContext(ctx context.Context, input *ec2.CreateSecurityGroupInput) (*ec2.CreateSecurityGroupOutput, error)
 	DeleteSecurityGroupWithContext(ctx context.Context, input *ec2.DeleteSecurityGroupInput) (*ec2.DeleteSecurityGroupOutput, error)
 	AuthorizeSecurityGroupIngressWithContext(ctx context.Context, input *ec2.AuthorizeSecurityGroupIngressInput) (*ec2.AuthorizeSecurityGroupIngressOutput, error)
+	AuthorizeSecurityGroupEgressWithContext(ctx context.Context, input *ec2.AuthorizeSecurityGroupEgressInput) (*ec2.AuthorizeSecurityGroupEgressOutput, error)
 	RevokeSecurityGroupIngressWithContext(ctx context.Context, input *ec2.RevokeSecurityGroupIngressInput) (*ec2.RevokeSecurityGroupIngressOutput, error)
+	RevokeSecurityGroupEgressWithContext(ctx context.Context, input *ec2.RevokeSecurityGroupEgressInput) (*ec2.RevokeSecurityGroupEgressOutput, error)
 	DescribeAvailabilityZonesWithContext(ctx context.Context, input *ec2.DescribeAvailabilityZonesInput) (*ec2.DescribeAvailabilityZonesOutput, error)
 	DescribeVpcsWithContext(ctx context.Context, input *ec2.DescribeVpcsInput) (*ec2.DescribeVpcsOutput, error)
 	DescribeInstancesWithContext(ctx context.Context, input *ec2.DescribeInstancesInput) (*ec2.DescribeInstancesOutput, error)
@@ -202,6 +204,14 @@ func (c *ec2Client) AuthorizeSecurityGroupIngressWithContext(ctx context.Context
 	return client.AuthorizeSecurityGroupIngress(ctx, input)
 }
 
+func (c *ec2Client) AuthorizeSecurityGroupEgressWithContext(ctx context.Context, input *ec2.AuthorizeSecurityGroupEgressInput) (*ec2.AuthorizeSecurityGroupEgressOutput, error) {
+	client, err := c.awsClientsProvider.GetEC2Client(ctx, "AuthorizeSecurityGroupIngress")
+	if err != nil {
+		return nil, err
+	}
+	return client.AuthorizeSecurityGroupEgress(ctx, input)
+}
+
 func (c *ec2Client) RevokeSecurityGroupIngressWithContext(ctx context.Context, input *ec2.RevokeSecurityGroupIngressInput) (*ec2.RevokeSecurityGroupIngressOutput, error) {
 	client, err := c.awsClientsProvider.GetEC2Client(ctx, "RevokeSecurityGroupIngress")
 	if err != nil {
@@ -210,6 +220,14 @@ func (c *ec2Client) RevokeSecurityGroupIngressWithContext(ctx context.Context, i
 	return client.RevokeSecurityGroupIngress(ctx, input)
 }
 
+func (c *ec2Client) RevokeSecurityGroupEgressWithContext(ctx context.Context, input *ec2.RevokeSecurityGroupEgressInput) (*ec2.RevokeSecurityGroupEgressOutput, error) {
+	client, err := c.awsClientsProvider.GetEC2Client(ctx, "RevokeSecurityGroupEgress")
+	if err != nil {
+		return nil, err
+	}
+	return client.RevokeSecurityGroupEgress(ctx, input)
+}
+
 func (c *ec2Client) DescribeAvailabilityZonesWithContext(ctx context.Context, input *ec2.DescribeAvailabilityZonesInput) (*ec2.DescribeAvailabilityZonesOutput, error) {
 	client, err := c.awsClientsProvider.GetEC2Client(ctx, "DescribeAvailabilityZones")
 	if err != nil {

pkg/aws/services/ec2_mocks.go

@@ -98,22 +98,50 @@ func (m *defaultSecurityGroupManager) Create(ctx context.Context, resSG *ec2mode
 		return ec2model.SecurityGroupStatus{}, err
 	}
 
+	// Only reconcile egress rules when explicitly set by the service.beta.kubernetes.io/aws-load-balancer-outbound-cidrs annotation. Otherwise, preserve the previous behavior of not touching egress rules.
+	if resSG.Spec.Egress != nil {
+		permissionInfosEgress, err := buildIPPermissionInfos(resSG.Spec.Egress)
+		if err != nil {
+			return ec2model.SecurityGroupStatus{}, err
+		}
+
+		if err := m.networkingSGReconciler.ReconcileEgress(ctx, sgID, permissionInfosEgress); err != nil {
+			return ec2model.SecurityGroupStatus{}, err
+		}
+	}
+
 	return ec2model.SecurityGroupStatus{
 		GroupID: sgID,
 	}, nil
 }
 
 func (m *defaultSecurityGroupManager) Update(ctx context.Context, resSG *ec2model.SecurityGroup, sdkSG networking.SecurityGroupInfo) (ec2model.SecurityGroupStatus, error) {
 	permissionInfos, err := buildIPPermissionInfos(resSG.Spec.Ingress)
+
 	if err != nil {
 		return ec2model.SecurityGroupStatus{}, err
 	}
+
 	if err := m.updateSDKSecurityGroupGroupWithTags(ctx, resSG, sdkSG); err != nil {
 		return ec2model.SecurityGroupStatus{}, err
 	}
 	if err := m.networkingSGReconciler.ReconcileIngress(ctx, sdkSG.SecurityGroupID, permissionInfos); err != nil {
 		return ec2model.SecurityGroupStatus{}, err
 	}
+
+	// Only reconcile egress rules when explicitly set by the service.beta.kubernetes.io/aws-load-balancer-outbound-cidrs annotation. Otherwise, preserve the previous behavior of not touching egress rules.
+	if resSG.Spec.Egress != nil {
+		permissionInfosEgress, err := buildIPPermissionInfos(resSG.Spec.Egress)
+
+		if err != nil {
+			return ec2model.SecurityGroupStatus{}, err
+		}
+
+		if err := m.networkingSGReconciler.ReconcileEgress(ctx, sdkSG.SecurityGroupID, permissionInfosEgress); err != nil {
+			return ec2model.SecurityGroupStatus{}, err
+		}
+	}
+
 	return ec2model.SecurityGroupStatus{
 		GroupID: sdkSG.SecurityGroupID,
 	}, nil

pkg/deploy/ec2/security_group_manager.go

@@ -61,6 +61,9 @@ type SecurityGroupSpec struct {
 
 	// +optional
 	Ingress []IPPermission `json:"ingress,omitempty"`
+
+	// +optional
+	Egress []IPPermission `json:"egress,omitempty"`
 }
 
 // SecurityGroupStatus defines the observed state of SecurityGroup

pkg/model/ec2/security_group.go

@@ -2069,8 +2069,8 @@ func Test_AttemptGarbageCollection(t *testing.T) {
 				assert.Error(t, err)
 			} else {
 				assert.NoError(t, err)
-				assert.Equal(t, len(tt.expectedSgReconciles), len(mockReconciler.calls))
-				for _, call := range mockReconciler.calls {
+				assert.Equal(t, len(tt.expectedSgReconciles), len(mockReconciler.ingressCalls))
+				for _, call := range mockReconciler.ingressCalls {
 					assert.True(t, tt.expectedSgReconciles.Has(call.sgID), fmt.Sprintf("expected sgID: %s to be in calls", call.sgID))
 				}
 			}
@@ -2083,12 +2083,29 @@ type reconcileIngressCall struct {
 	desiredPermissions []IPPermissionInfo
 	opts               []SecurityGroupReconcileOption
 }
+
+type reconcileEgressCall struct {
+	sgID               string
+	desiredPermissions []IPPermissionInfo
+	opts               []SecurityGroupReconcileOption
+}
+
 type mockSGReconciler struct {
-	calls []reconcileIngressCall
+	ingressCalls []reconcileIngressCall
+	egressCalls  []reconcileEgressCall
 }
 
 func (m *mockSGReconciler) ReconcileIngress(ctx context.Context, sgID string, desiredPermissions []IPPermissionInfo, opts ...SecurityGroupReconcileOption) error {
-	m.calls = append(m.calls, reconcileIngressCall{
+	m.ingressCalls = append(m.ingressCalls, reconcileIngressCall{
+		sgID:               sgID,
+		desiredPermissions: desiredPermissions,
+		opts:               opts,
+	})
+	return nil
+}
+
+func (m *mockSGReconciler) ReconcileEgress(ctx context.Context, sgID string, desiredPermissions []IPPermissionInfo, opts ...SecurityGroupReconcileOption) error {
+	m.egressCalls = append(m.egressCalls, reconcileEgressCall{
 		sgID:               sgID,
 		desiredPermissions: desiredPermissions,
 		opts:               opts,

pkg/networking/networking_manager_test.go

@@ -23,6 +23,9 @@ type SecurityGroupInfo struct {
 	// Ingress permission for securityGroup.
 	Ingress []IPPermissionInfo
 
+	// Egress permission for securityGroup
+	Egress []IPPermissionInfo
+
 	// Tags for securityGroup.
 	Tags map[string]string
 }
@@ -74,10 +77,17 @@ func NewRawSecurityGroupInfo(sdkSG ec2types.SecurityGroup) SecurityGroupInfo {
 			ingress = append(ingress, NewRawIPPermission(expandedPermission))
 		}
 	}
+	var egress []IPPermissionInfo
+	for _, sdkPermission := range sdkSG.IpPermissionsEgress {
+		for _, expandedPermission := range expandSDKIPPermission(sdkPermission) {
+			egress = append(egress, NewRawIPPermission(expandedPermission))
+		}
+	}
 	tags := buildSecurityGroupTags(sdkSG)
 	return SecurityGroupInfo{
 		SecurityGroupID: sgID,
 		Ingress:         ingress,
+		Egress:          egress,
 		Tags:            tags,
 	}
 }

pkg/networking/security_group_info.go

@@ -51,8 +51,14 @@ type SecurityGroupManager interface {
 	// AuthorizeSGIngress will authorize Ingress permissions to SecurityGroup.
 	AuthorizeSGIngress(ctx context.Context, sgID string, permissions []IPPermissionInfo) error
 
+	// AuthorizeSGEgress will authorize Ingress permissions to SecurityGroup.
+	AuthorizeSGEgress(ctx context.Context, sgID string, permissions []IPPermissionInfo) error
+
 	// RevokeSGIngress will revoke Ingress permissions from SecurityGroup.
 	RevokeSGIngress(ctx context.Context, sgID string, permissions []IPPermissionInfo) error
+
+	// RevokeSGEgress will revoke Ingress permissions from SecurityGroup.
+	RevokeSGEgress(ctx context.Context, sgID string, permissions []IPPermissionInfo) error
 }
 
 // NewDefaultSecurityGroupManager constructs new defaultSecurityGroupManager.
@@ -146,6 +152,26 @@ func (m *defaultSecurityGroupManager) AuthorizeSGIngress(ctx context.Context, sg
 	return nil
 }
 
+func (m *defaultSecurityGroupManager) AuthorizeSGEgress(ctx context.Context, sgID string, permissions []IPPermissionInfo) error {
+	sdkIPPermissions := buildSDKIPPermissions(permissions)
+	req := &ec2sdk.AuthorizeSecurityGroupEgressInput{
+		GroupId:       awssdk.String(sgID),
+		IpPermissions: sdkIPPermissions,
+	}
+	m.logger.Info("authorizing securityGroup egress",
+		"securityGroupID", sgID,
+		"permission", sdkIPPermissions)
+	if _, err := m.ec2Client.AuthorizeSecurityGroupEgressWithContext(ctx, req); err != nil {
+		return err
+	}
+	m.logger.Info("authorized securityGroup egress",
+		"securityGroupID", sgID)
+
+	// TODO: ideally we can remember the permissions we granted to save DescribeSecurityGroup API calls.
+	m.clearSGInfosFromCache(sgID)
+	return nil
+}
+
 func (m *defaultSecurityGroupManager) RevokeSGIngress(ctx context.Context, sgID string, permissions []IPPermissionInfo) error {
 	sdkIPPermissions := buildSDKIPPermissions(permissions)
 	req := &ec2sdk.RevokeSecurityGroupIngressInput{
@@ -166,6 +192,26 @@ func (m *defaultSecurityGroupManager) RevokeSGIngress(ctx context.Context, sgID
 	return nil
 }
 
+func (m *defaultSecurityGroupManager) RevokeSGEgress(ctx context.Context, sgID string, permissions []IPPermissionInfo) error {
+	sdkIPPermissions := buildSDKIPPermissions(permissions)
+	req := &ec2sdk.RevokeSecurityGroupEgressInput{
+		GroupId:       awssdk.String(sgID),
+		IpPermissions: sdkIPPermissions,
+	}
+	m.logger.Info("revoking securityGroup egress",
+		"securityGroupID", sgID,
+		"permission", sdkIPPermissions)
+	if _, err := m.ec2Client.RevokeSecurityGroupEgressWithContext(ctx, req); err != nil {
+		return err
+	}
+	m.logger.Info("revoked securityGroup egress",
+		"securityGroupID", sgID)
+
+	// TODO: ideally we can remember the permissions we revoked to save DescribeSecurityGroup API calls.
+	m.clearSGInfosFromCache(sgID)
+	return nil
+}
+
 func (m *defaultSecurityGroupManager) fetchSGInfosFromCache(sgIDs []string) map[string]SecurityGroupInfo {
 	m.sgInfoCacheMutex.RLock()
 	defer m.sgInfoCacheMutex.RUnlock()