Merge pull request kubernetes-sigs#2205 from kishorj/optimized-sg

Support optimized security group rules for ALB

controllers/ingress/group_controller.go

@@ -44,7 +44,7 @@ const (
 func NewGroupReconciler(cloud aws.Cloud, k8sClient client.Client, eventRecorder record.EventRecorder,
 	finalizerManager k8s.FinalizerManager, networkingSGManager networkingpkg.SecurityGroupManager,
 	networkingSGReconciler networkingpkg.SecurityGroupReconciler, subnetsResolver networkingpkg.SubnetsResolver,
-	config config.ControllerConfig, logger logr.Logger) *groupReconciler {
+	config config.ControllerConfig, backendSGProvider networkingpkg.BackendSGProvider, logger logr.Logger) *groupReconciler {
 
 	annotationParser := annotations.NewSuffixAnnotationParser(annotations.AnnotationPrefixIngress)
 	authConfigBuilder := ingress.NewDefaultAuthConfigBuilder(annotationParser)
@@ -57,7 +57,7 @@ func NewGroupReconciler(cloud aws.Cloud, k8sClient client.Client, eventRecorder
 		annotationParser, subnetsResolver,
 		authConfigBuilder, enhancedBackendBuilder, trackingProvider, elbv2TaggingManager,
 		cloud.VpcID(), config.ClusterName, config.DefaultTags, config.ExternalManagedTags,
-		config.DefaultSSLPolicy, logger)
+		config.DefaultSSLPolicy, backendSGProvider, config.EnableBackendSecurityGroup, logger)
 	stackMarshaller := deploy.NewDefaultStackMarshaller()
 	stackDeployer := deploy.NewDefaultStackDeployer(cloud, k8sClient, networkingSGManager, networkingSGReconciler,
 		config, ingressTagPrefix, logger)
@@ -68,12 +68,13 @@ func NewGroupReconciler(cloud aws.Cloud, k8sClient client.Client, eventRecorder
 	groupFinalizerManager := ingress.NewDefaultFinalizerManager(finalizerManager)
 
 	return &groupReconciler{
-		k8sClient:        k8sClient,
-		eventRecorder:    eventRecorder,
-		referenceIndexer: referenceIndexer,
-		modelBuilder:     modelBuilder,
-		stackMarshaller:  stackMarshaller,
-		stackDeployer:    stackDeployer,
+		k8sClient:         k8sClient,
+		eventRecorder:     eventRecorder,
+		referenceIndexer:  referenceIndexer,
+		modelBuilder:      modelBuilder,
+		stackMarshaller:   stackMarshaller,
+		stackDeployer:     stackDeployer,
+		backendSGProvider: backendSGProvider,
 
 		groupLoader:           groupLoader,
 		groupFinalizerManager: groupFinalizerManager,
@@ -85,12 +86,13 @@ func NewGroupReconciler(cloud aws.Cloud, k8sClient client.Client, eventRecorder
 
 // GroupReconciler reconciles a IngressGroup
 type groupReconciler struct {
-	k8sClient        client.Client
-	eventRecorder    record.EventRecorder
-	referenceIndexer ingress.ReferenceIndexer
-	modelBuilder     ingress.ModelBuilder
-	stackMarshaller  deploy.StackMarshaller
-	stackDeployer    deploy.StackDeployer
+	k8sClient         client.Client
+	eventRecorder     record.EventRecorder
+	referenceIndexer  ingress.ReferenceIndexer
+	modelBuilder      ingress.ModelBuilder
+	stackMarshaller   deploy.StackMarshaller
+	stackDeployer     deploy.StackDeployer
+	backendSGProvider networkingpkg.BackendSGProvider
 
 	groupLoader           ingress.GroupLoader
 	groupFinalizerManager ingress.FinalizerManager
@@ -124,7 +126,6 @@ func (r *groupReconciler) reconcile(ctx context.Context, req ctrl.Request) error
 		r.recordIngressGroupEvent(ctx, ingGroup, corev1.EventTypeWarning, k8s.IngressEventReasonFailedAddFinalizer, fmt.Sprintf("Failed add finalizer due to %v", err))
 		return err
 	}
-
 	_, lb, err := r.buildAndDeployModel(ctx, ingGroup)
 	if err != nil {
 		return err
@@ -141,6 +142,12 @@ func (r *groupReconciler) reconcile(ctx context.Context, req ctrl.Request) error
 		}
 	}
 
+	if len(ingGroup.Members) == 0 {
+		if err := r.backendSGProvider.Release(ctx); err != nil {
+			return err
+		}
+	}
+
 	if len(ingGroup.InactiveMembers) > 0 {
 		if err := r.groupFinalizerManager.RemoveGroupFinalizer(ctx, ingGroupID, ingGroup.InactiveMembers); err != nil {
 			r.recordIngressGroupEvent(ctx, ingGroup, corev1.EventTypeWarning, k8s.IngressEventReasonFailedRemoveFinalizer, fmt.Sprintf("Failed remove finalizer due to %v", err))

main.go

@@ -106,9 +106,11 @@ func main() {
 	vpcResolver := networking.NewDefaultVPCResolver(cloud.EC2(), cloud.VpcID(), ctrl.Log.WithName("vpc-resolver"))
 	tgbResManager := targetgroupbinding.NewDefaultResourceManager(mgr.GetClient(), cloud.ELBV2(), cloud.EC2(),
 		podInfoRepo, sgManager, sgReconciler, cloud.VpcID(), controllerCFG.ClusterName, mgr.GetEventRecorderFor("targetGroupBinding"), ctrl.Log)
+	backendSGProvider := networking.NewBackendSGProvider(controllerCFG.ClusterName, controllerCFG.BackendSecurityGroup,
+		cloud.VpcID(), cloud.EC2(), mgr.GetClient(), ctrl.Log.WithName("backend-sg-provider"))
 	ingGroupReconciler := ingress.NewGroupReconciler(cloud, mgr.GetClient(), mgr.GetEventRecorderFor("ingress"),
 		finalizerManager, sgManager, sgReconciler, subnetResolver,
-		controllerCFG, ctrl.Log.WithName("controllers").WithName("ingress"))
+		controllerCFG, backendSGProvider, ctrl.Log.WithName("controllers").WithName("ingress"))
 	svcReconciler := service.NewServiceReconciler(cloud, mgr.GetClient(), mgr.GetEventRecorderFor("service"),
 		finalizerManager, sgManager, sgReconciler, subnetResolver, vpcResolver,
 		controllerCFG, ctrl.Log.WithName("controllers").WithName("service"))

pkg/annotations/constants.go

@@ -45,6 +45,7 @@ const (
 	IngressSuffixAuthSessionCookie            = "auth-session-cookie"
 	IngressSuffixAuthSessionTimeout           = "auth-session-timeout"
 	IngressSuffixTargetNodeLabels             = "target-node-labels"
+	IngressSuffixManageSecurityGroupRules     = "manage-backend-security-group-rules"
 
 	// NLB annotation suffixes
 	// prefixes service.beta.kubernetes.io, service.kubernetes.io

pkg/config/controller_config.go

@@ -1,6 +1,7 @@
 package config
 
 import (
+	"strings"
 	"time"
 
 	"github.com/pkg/errors"
@@ -19,10 +20,13 @@ const (
 	flagTargetGroupBindingMaxConcurrentReconciles    = "targetgroupbinding-max-concurrent-reconciles"
 	flagTargetGroupBindingMaxExponentialBackoffDelay = "targetgroupbinding-max-exponential-backoff-delay"
 	flagDefaultSSLPolicy                             = "default-ssl-policy"
+	flagEnableBackendSG                              = "enable-backend-security-group"
+	flagBackendSecurityGroup                         = "backend-security-group"
 	defaultLogLevel                                  = "info"
 	defaultMaxConcurrentReconciles                   = 3
 	defaultMaxExponentialBackoffDelay                = time.Second * 1000
 	defaultSSLPolicy                                 = "ELBSecurityPolicy-2016-08"
+	defaultEnableBackendSG                           = true
 )
 
 var (
@@ -68,6 +72,13 @@ type ControllerConfig struct {
 	TargetGroupBindingMaxConcurrentReconciles int
 	// Max exponential backoff delay for reconcile failures of TargetGroupBinding
 	TargetGroupBindingMaxExponentialBackoffDelay time.Duration
+
+	// EnableBackendSecurityGroup specifies whether to use optimized security group rules
+	EnableBackendSecurityGroup bool
+
+	// BackendSecurityGroups specifies the configured backend security group to use
+	// for optimized security group rules
+	BackendSecurityGroup string
 }
 
 // BindFlags binds the command line flags to the fields in the config object
@@ -87,6 +98,10 @@ func (cfg *ControllerConfig) BindFlags(fs *pflag.FlagSet) {
 		"Maximum duration of exponential backoff for targetGroupBinding reconcile failures")
 	fs.StringVar(&cfg.DefaultSSLPolicy, flagDefaultSSLPolicy, defaultSSLPolicy,
 		"Default SSL policy for load balancers listeners")
+	fs.BoolVar(&cfg.EnableBackendSecurityGroup, flagEnableBackendSG, defaultEnableBackendSG,
+		"Enable sharing of security groups for backend traffic")
+	fs.StringVar(&cfg.BackendSecurityGroup, flagBackendSecurityGroup, "",
+		"Backend security group id to use for the ingress rules on the worker node SG")
 
 	cfg.AWSConfig.BindFlags(fs)
 	cfg.RuntimeConfig.BindFlags(fs)
@@ -111,6 +126,9 @@ func (cfg *ControllerConfig) Validate() error {
 	if err := cfg.validateExternalManagedTagsCollisionWithDefaultTags(); err != nil {
 		return err
 	}
+	if err := cfg.validateBackendSecurityGroupConfiguration(); err != nil {
+		return err
+	}
 	return nil
 }
 
@@ -141,3 +159,13 @@ func (cfg *ControllerConfig) validateExternalManagedTagsCollisionWithDefaultTags
 	}
 	return nil
 }
+
+func (cfg *ControllerConfig) validateBackendSecurityGroupConfiguration() error {
+	if len(cfg.BackendSecurityGroup) == 0 {
+		return nil
+	}
+	if !strings.HasPrefix(cfg.BackendSecurityGroup, "sg-") {
+		return errors.Errorf("invalid value %v for backend security group id", cfg.BackendSecurityGroup)
+	}
+	return nil
+}

pkg/ingress/model_build_load_balancer.go

@@ -242,6 +242,58 @@ func (t *defaultModelBuildTask) buildLoadBalancerSubnetMappings(ctx context.Cont
 }
 
 func (t *defaultModelBuildTask) buildLoadBalancerSecurityGroups(ctx context.Context, listenPortConfigByPort map[int64]listenPortConfig, ipAddressType elbv2model.IPAddressType) ([]core.StringToken, error) {
+	sgNameOrIDsViaAnnotation, err := t.buildFrontendSGNameOrIDsFromAnnotation(ctx)
+	if err != nil {
+		return nil, err
+	}
+	var lbSGTokens []core.StringToken
+	if len(sgNameOrIDsViaAnnotation) == 0 {
+		managedSG, err := t.buildManagedSecurityGroup(ctx, listenPortConfigByPort, ipAddressType)
+		if err != nil {
+			return nil, err
+		}
+		lbSGTokens = append(lbSGTokens, managedSG.GroupID())
+		if !t.enableBackendSG {
+			t.backendSGIDToken = managedSG.GroupID()
+		} else {
+			backendSGID, err := t.backendSGProvider.Get(ctx)
+			if err != nil {
+				return nil, err
+			}
+			t.backendSGIDToken = core.LiteralStringToken((backendSGID))
+			lbSGTokens = append(lbSGTokens, t.backendSGIDToken)
+		}
+		t.logger.Info("Auto Create SG", "LB SGs", lbSGTokens, "backend SG", t.backendSGIDToken)
+	} else {
+		manageBackendSGRules, err := t.buildManageSecurityGroupRulesFlag(ctx)
+		if err != nil {
+			return nil, err
+		}
+		frontendSGIDs, err := t.resolveSecurityGroupIDsViaNameOrIDSlice(ctx, sgNameOrIDsViaAnnotation)
+		if err != nil {
+			return nil, err
+		}
+		for _, sgID := range frontendSGIDs {
+			lbSGTokens = append(lbSGTokens, core.LiteralStringToken(sgID))
+		}
+
+		if manageBackendSGRules {
+			if !t.enableBackendSG {
+				return nil, errors.New("backendSG feature is required to manage worker node SG rules when frontendSG manually specified")
+			}
+			backendSGID, err := t.backendSGProvider.Get(ctx)
+			if err != nil {
+				return nil, err
+			}
+			t.backendSGIDToken = core.LiteralStringToken(backendSGID)
+			lbSGTokens = append(lbSGTokens, t.backendSGIDToken)
+		}
+		t.logger.Info("SG configured via annotation", "LB SGs", lbSGTokens, "backend SG", t.backendSGIDToken)
+	}
+	return lbSGTokens, nil
+}
+
+func (t *defaultModelBuildTask) buildFrontendSGNameOrIDsFromAnnotation(ctx context.Context) ([]string, error) {
 	var explicitSGNameOrIDsList [][]string
 	for _, member := range t.ingGroup.Members {
 		var rawSGNameOrIDs []string
@@ -251,29 +303,15 @@ func (t *defaultModelBuildTask) buildLoadBalancerSecurityGroups(ctx context.Cont
 		explicitSGNameOrIDsList = append(explicitSGNameOrIDsList, rawSGNameOrIDs)
 	}
 	if len(explicitSGNameOrIDsList) == 0 {
-		sg, err := t.buildManagedSecurityGroup(ctx, listenPortConfigByPort, ipAddressType)
-		if err != nil {
-			return nil, err
-		}
-		return []core.StringToken{sg.GroupID()}, nil
+		return nil, nil
 	}
-
 	chosenSGNameOrIDs := explicitSGNameOrIDsList[0]
 	for _, sgNameOrIDs := range explicitSGNameOrIDsList[1:] {
-		// securityGroups order might matters in the future(e.g. use the first securityGroup for traffic to nodeGroups)
 		if !cmp.Equal(chosenSGNameOrIDs, sgNameOrIDs) {
 			return nil, errors.Errorf("conflicting securityGroups: %v | %v", chosenSGNameOrIDs, sgNameOrIDs)
 		}
 	}
-	chosenSGIDs, err := t.resolveSecurityGroupIDsViaNameOrIDSlice(ctx, chosenSGNameOrIDs)
-	if err != nil {
-		return nil, err
-	}
-	sgIDTokens := make([]core.StringToken, 0, len(chosenSGIDs))
-	for _, sgID := range chosenSGIDs {
-		sgIDTokens = append(sgIDTokens, core.LiteralStringToken(sgID))
-	}
-	return sgIDTokens, nil
+	return chosenSGNameOrIDs, nil
 }
 
 func (t *defaultModelBuildTask) buildLoadBalancerCOIPv4Pool(_ context.Context) (*string, error) {
@@ -369,7 +407,7 @@ func (t *defaultModelBuildTask) resolveSecurityGroupIDsViaNameOrIDSlice(ctx cont
 		resolvedSGIDs = append(resolvedSGIDs, awssdk.StringValue(sg.GroupId))
 	}
 	if len(resolvedSGIDs) != len(sgNameOrIDs) {
-		return nil, errors.Errorf("couldn't found all securityGroups, nameOrIDs: %v, found: %v", sgNameOrIDs, resolvedSGIDs)
+		return nil, errors.Errorf("couldn't find all securityGroups, nameOrIDs: %v, found: %v", sgNameOrIDs, resolvedSGIDs)
 	}
 	return resolvedSGIDs, nil
 }

pkg/ingress/model_build_managed_sg.go

@@ -24,7 +24,6 @@ func (t *defaultModelBuildTask) buildManagedSecurityGroup(ctx context.Context, l
 	}
 
 	sg := ec2model.NewSecurityGroup(t.stack, resourceIDManagedSecurityGroup, sgSpec)
-	t.managedSG = sg
 	return sg, nil
 }
 

pkg/ingress/model_build_target_group.go

@@ -74,8 +74,8 @@ func (t *defaultModelBuildTask) buildTargetGroupBindingSpec(ctx context.Context,
 	}
 }
 
-func (t *defaultModelBuildTask) buildTargetGroupBindingNetworking(_ context.Context) *elbv2model.TargetGroupBindingNetworking {
-	if t.managedSG == nil {
+func (t *defaultModelBuildTask) buildTargetGroupBindingNetworking(ctx context.Context) *elbv2model.TargetGroupBindingNetworking {
+	if t.backendSGIDToken == nil {
 		return nil
 	}
 	protocolTCP := elbv2api.NetworkingProtocolTCP
@@ -85,7 +85,7 @@ func (t *defaultModelBuildTask) buildTargetGroupBindingNetworking(_ context.Cont
 				From: []elbv2model.NetworkingPeer{
 					{
 						SecurityGroup: &elbv2model.SecurityGroup{
-							GroupID: t.managedSG.GroupID(),
+							GroupID: t.backendSGIDToken,
 						},
 					},
 				},