Merge pull request #3329 from oliviassss/nlb-sg

Add support for NLB security group

controllers/service/service_controller.go

@@ -7,6 +7,7 @@ import (
 	"github.com/go-logr/logr"
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/tools/record"
 	"sigs.k8s.io/aws-load-balancer-controller/controllers/service/eventhandlers"
 	"sigs.k8s.io/aws-load-balancer-controller/pkg/annotations"
@@ -37,14 +38,17 @@ const (
 func NewServiceReconciler(cloud aws.Cloud, k8sClient client.Client, eventRecorder record.EventRecorder,
 	finalizerManager k8s.FinalizerManager, networkingSGManager networking.SecurityGroupManager,
 	networkingSGReconciler networking.SecurityGroupReconciler, subnetsResolver networking.SubnetsResolver,
-	vpcInfoProvider networking.VPCInfoProvider, controllerConfig config.ControllerConfig, logger logr.Logger) *serviceReconciler {
+	vpcInfoProvider networking.VPCInfoProvider, controllerConfig config.ControllerConfig,
+	backendSGProvider networking.BackendSGProvider, sgResolver networking.SecurityGroupResolver, logger logr.Logger) *serviceReconciler {
 
 	annotationParser := annotations.NewSuffixAnnotationParser(serviceAnnotationPrefix)
 	trackingProvider := tracking.NewDefaultProvider(serviceTagPrefix, controllerConfig.ClusterName)
 	elbv2TaggingManager := elbv2.NewDefaultTaggingManager(cloud.ELBV2(), cloud.VpcID(), controllerConfig.FeatureGates, cloud.RGT(), logger)
 	serviceUtils := service.NewServiceUtils(annotationParser, serviceFinalizer, controllerConfig.ServiceConfig.LoadBalancerClass, controllerConfig.FeatureGates)
 	modelBuilder := service.NewDefaultModelBuilder(annotationParser, subnetsResolver, vpcInfoProvider, cloud.VpcID(), trackingProvider,
-		elbv2TaggingManager, controllerConfig.FeatureGates, controllerConfig.ClusterName, controllerConfig.DefaultTags, controllerConfig.ExternalManagedTags, controllerConfig.DefaultSSLPolicy, controllerConfig.DefaultTargetType, controllerConfig.FeatureGates.Enabled(config.EnableIPTargetType), serviceUtils)
+		elbv2TaggingManager, cloud.EC2(), controllerConfig.FeatureGates, controllerConfig.ClusterName, controllerConfig.DefaultTags, controllerConfig.ExternalManagedTags,
+		controllerConfig.DefaultSSLPolicy, controllerConfig.DefaultTargetType, controllerConfig.FeatureGates.Enabled(config.EnableIPTargetType), serviceUtils,
+		backendSGProvider, sgResolver, controllerConfig.EnableBackendSecurityGroup, controllerConfig.DisableRestrictedSGRules)
 	stackMarshaller := deploy.NewDefaultStackMarshaller()
 	stackDeployer := deploy.NewDefaultStackDeployer(cloud, k8sClient, networkingSGManager, networkingSGReconciler, controllerConfig, serviceTagPrefix, logger)
 	return &serviceReconciler{
@@ -54,6 +58,7 @@ func NewServiceReconciler(cloud aws.Cloud, k8sClient client.Client, eventRecorde
 		annotationParser:  annotationParser,
 		loadBalancerClass: controllerConfig.ServiceConfig.LoadBalancerClass,
 		serviceUtils:      serviceUtils,
+		backendSGProvider: backendSGProvider,
 
 		modelBuilder:    modelBuilder,
 		stackMarshaller: stackMarshaller,
@@ -71,6 +76,7 @@ type serviceReconciler struct {
 	annotationParser  annotations.Parser
 	loadBalancerClass string
 	serviceUtils      service.ServiceUtils
+	backendSGProvider networking.BackendSGProvider
 
 	modelBuilder    service.ModelBuilder
 	stackMarshaller deploy.StackMarshaller
@@ -93,29 +99,29 @@ func (r *serviceReconciler) reconcile(ctx context.Context, req ctrl.Request) err
 	if err := r.k8sClient.Get(ctx, req.NamespacedName, svc); err != nil {
 		return client.IgnoreNotFound(err)
 	}
-	stack, lb, err := r.buildModel(ctx, svc)
+	stack, lb, backendSGRequired, err := r.buildModel(ctx, svc)
 	if err != nil {
 		return err
 	}
 	if lb == nil {
 		return r.cleanupLoadBalancerResources(ctx, svc, stack)
 	}
-	return r.reconcileLoadBalancerResources(ctx, svc, stack, lb)
+	return r.reconcileLoadBalancerResources(ctx, svc, stack, lb, backendSGRequired)
 }
 
-func (r *serviceReconciler) buildModel(ctx context.Context, svc *corev1.Service) (core.Stack, *elbv2model.LoadBalancer, error) {
-	stack, lb, err := r.modelBuilder.Build(ctx, svc)
+func (r *serviceReconciler) buildModel(ctx context.Context, svc *corev1.Service) (core.Stack, *elbv2model.LoadBalancer, bool, error) {
+	stack, lb, backendSGRequired, err := r.modelBuilder.Build(ctx, svc)
 	if err != nil {
 		r.eventRecorder.Event(svc, corev1.EventTypeWarning, k8s.ServiceEventReasonFailedBuildModel, fmt.Sprintf("Failed build model due to %v", err))
-		return nil, nil, err
+		return nil, nil, false, err
 	}
 	stackJSON, err := r.stackMarshaller.Marshal(stack)
 	if err != nil {
 		r.eventRecorder.Event(svc, corev1.EventTypeWarning, k8s.ServiceEventReasonFailedBuildModel, fmt.Sprintf("Failed build model due to %v", err))
-		return nil, nil, err
+		return nil, nil, false, err
 	}
 	r.logger.Info("successfully built model", "model", stackJSON)
-	return stack, lb, nil
+	return stack, lb, backendSGRequired, nil
 }
 
 func (r *serviceReconciler) deployModel(ctx context.Context, svc *corev1.Service, stack core.Stack) error {
@@ -128,7 +134,8 @@ func (r *serviceReconciler) deployModel(ctx context.Context, svc *corev1.Service
 	return nil
 }
 
-func (r *serviceReconciler) reconcileLoadBalancerResources(ctx context.Context, svc *corev1.Service, stack core.Stack, lb *elbv2model.LoadBalancer) error {
+func (r *serviceReconciler) reconcileLoadBalancerResources(ctx context.Context, svc *corev1.Service, stack core.Stack,
+	lb *elbv2model.LoadBalancer, backendSGRequired bool) error {
 	if err := r.finalizerManager.AddFinalizers(ctx, svc, serviceFinalizer); err != nil {
 		r.eventRecorder.Event(svc, corev1.EventTypeWarning, k8s.ServiceEventReasonFailedAddFinalizer, fmt.Sprintf("Failed add finalizer due to %v", err))
 		return err
@@ -142,6 +149,12 @@ func (r *serviceReconciler) reconcileLoadBalancerResources(ctx context.Context,
 		return err
 	}
 
+	if !backendSGRequired {
+		if err := r.backendSGProvider.Release(ctx, networking.ResourceTypeService, []types.NamespacedName{k8s.NamespacedName(svc)}); err != nil {
+			return err
+		}
+	}
+
 	if err = r.updateServiceStatus(ctx, lbDNS, svc); err != nil {
 		r.eventRecorder.Event(svc, corev1.EventTypeWarning, k8s.ServiceEventReasonFailedUpdateStatus, fmt.Sprintf("Failed update status due to %v", err))
 		return err
@@ -156,6 +169,9 @@ func (r *serviceReconciler) cleanupLoadBalancerResources(ctx context.Context, sv
 		if err != nil {
 			return err
 		}
+		if err := r.backendSGProvider.Release(ctx, networking.ResourceTypeService, []types.NamespacedName{k8s.NamespacedName(svc)}); err != nil {
+			return err
+		}
 		if err = r.cleanupServiceStatus(ctx, svc); err != nil {
 			r.eventRecorder.Event(svc, corev1.EventTypeWarning, k8s.ServiceEventReasonFailedCleanupStatus, fmt.Sprintf("Failed update status due to %v", err))
 			return err

main.go

@@ -117,7 +117,7 @@ func main() {
 		controllerCFG, backendSGProvider, sgResolver, ctrl.Log.WithName("controllers").WithName("ingress"))
 	svcReconciler := service.NewServiceReconciler(cloud, mgr.GetClient(), mgr.GetEventRecorderFor("service"),
 		finalizerManager, sgManager, sgReconciler, subnetResolver, vpcInfoProvider,
-		controllerCFG, ctrl.Log.WithName("controllers").WithName("service"))
+		controllerCFG, backendSGProvider, sgResolver, ctrl.Log.WithName("controllers").WithName("service"))
 	tgbReconciler := elbv2controller.NewTargetGroupBindingReconciler(mgr.GetClient(), mgr.GetEventRecorderFor("targetGroupBinding"),
 		finalizerManager, tgbResManager,
 		controllerCFG, ctrl.Log.WithName("controllers").WithName("targetGroupBinding"))

pkg/annotations/constants.go

@@ -82,5 +82,6 @@ const (
 	SvcLBSuffixALPNPolicy                    = "aws-load-balancer-alpn-policy"
 	SvcLBSuffixTargetNodeLabels              = "aws-load-balancer-target-node-labels"
 	SvcLBSuffixLoadBalancerAttributes        = "aws-load-balancer-attributes"
+	SvcLBSuffixLoadBalancerSecurityGroups    = "aws-load-balancer-security-groups"
 	SvcLBSuffixManageSGRules                 = "aws-load-balancer-manage-backend-security-group-rules"
 )

pkg/config/feature_gates.go

@@ -20,6 +20,7 @@ const (
 	EnableRGTAPI                 Feature = "EnableRGTAPI"
 	SubnetsClusterTagCheck       Feature = "SubnetsClusterTagCheck"
 	NLBHealthCheckAdvancedConfig Feature = "NLBHealthCheckAdvancedConfig"
+	NLBSecurityGroup             Feature = "NLBSecurityGroup"
 )
 
 type FeatureGates interface {
@@ -56,6 +57,7 @@ func NewFeatureGates() FeatureGates {
 			EnableRGTAPI:                 false,
 			SubnetsClusterTagCheck:       true,
 			NLBHealthCheckAdvancedConfig: true,
+			NLBSecurityGroup:             true,
 		},
 	}
 }

pkg/networking/backend_sg_provider.go

@@ -16,6 +16,7 @@ import (
 	ec2sdk "github.com/aws/aws-sdk-go/service/ec2"
 	"github.com/go-logr/logr"
 	"github.com/pkg/errors"
+	corev1 "k8s.io/api/core/v1"
 	networking "k8s.io/api/networking/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/aws-load-balancer-controller/pkg/aws/services"
@@ -35,6 +36,7 @@ const (
 
 	explicitGroupFinalizerPrefix = "group.ingress.k8s.aws/"
 	implicitGroupFinalizer       = "ingress.k8s.aws/resources"
+	serviceFinalizer             = "service.k8s.aws/resources"
 
 	sgDescription = "[k8s] Shared Backend SecurityGroup for LoadBalancer"
 )
@@ -76,6 +78,15 @@ func NewBackendSGProvider(clusterName string, backendSG string, vpcID string,
 			return false
 		},
 
+		checkServiceFinalizersFunc: func(finalizers []string) bool {
+			for _, fin := range finalizers {
+				if fin == serviceFinalizer {
+					return true
+				}
+			}
+			return false
+		},
+
 		defaultDeletionPollInterval: defaultSGDeletionPollInterval,
 		defaultDeletionTimeout:      defaultSGDeletionTimeout,
 	}
@@ -101,6 +112,7 @@ type defaultBackendSGProvider struct {
 	// controller deletes the backend SG.
 	objectsMap sync.Map
 
+	checkServiceFinalizersFunc func([]string) bool
 	checkIngressFinalizersFunc func([]string) bool
 
 	defaultDeletionPollInterval time.Duration
@@ -126,7 +138,7 @@ func (p *defaultBackendSGProvider) Release(ctx context.Context, resourceType Res
 	}
 	defer func() {
 		for _, res := range inactiveResources {
-			p.objectsMap.Delete(getObjectKey(resourceType, res))
+			p.objectsMap.CompareAndDelete(getObjectKey(resourceType, res), false)
 		}
 	}()
 	p.updateObjectsMap(ctx, resourceType, inactiveResources, false)
@@ -159,6 +171,9 @@ func (p *defaultBackendSGProvider) isBackendSGRequired(ctx context.Context) (boo
 	if required, err := p.checkIngressListForUnmapped(ctx); required || err != nil {
 		return required, err
 	}
+	if required, err := p.checkServiceListForUnmapped(ctx); required || err != nil {
+		return required, err
+	}
 	return false, nil
 }
 
@@ -178,6 +193,22 @@ func (p *defaultBackendSGProvider) checkIngressListForUnmapped(ctx context.Conte
 	return false, nil
 }
 
+func (p *defaultBackendSGProvider) checkServiceListForUnmapped(ctx context.Context) (bool, error) {
+	svcList := &corev1.ServiceList{}
+	if err := p.k8sClient.List(ctx, svcList); err != nil {
+		return true, errors.Wrapf(err, "unable to list services")
+	}
+	for _, svc := range svcList.Items {
+		if !p.checkServiceFinalizersFunc(svc.GetFinalizers()) {
+			continue
+		}
+		if !p.existsInObjectMap(ResourceTypeService, k8s.NamespacedName(&svc)) {
+			return true, nil
+		}
+	}
+	return false, nil
+}
+
 func (p *defaultBackendSGProvider) existsInObjectMap(resourceType ResourceType, resource types.NamespacedName) bool {
 	if _, exists := p.objectsMap.Load(getObjectKey(resourceType, resource)); exists {
 		return true

pkg/networking/backend_sg_provider_test.go

@@ -542,14 +542,6 @@ func Test_defaultBackendSGProvider_Release(t *testing.T) {
 					},
 				},
 				inactiveIngresses: []*networking.Ingress{ing},
-				deleteSGCalls: []deleteSecurityGroupWithContextCall{
-					{
-						req: &ec2sdk.DeleteSecurityGroupInput{
-							GroupId: awssdk.String("sg-autogen"),
-						},
-						resp: &ec2sdk.DeleteSecurityGroupOutput{},
-					},
-				},
 			},
 		},
 		{
@@ -727,15 +719,8 @@ func Test_defaultBackendSGProvider_Release(t *testing.T) {
 					},
 				},
 				inactiveIngresses: []*networking.Ingress{ing},
-				deleteSGCalls: []deleteSecurityGroupWithContextCall{
-					{
-						req: &ec2sdk.DeleteSecurityGroupInput{
-							GroupId: awssdk.String("sg-autogen"),
-						},
-						resp: &ec2sdk.DeleteSecurityGroupOutput{},
-					},
-				},
 			},
+			wantErr: errors.New("unable to list services: failed"),
 		},
 	}
 	for _, tt := range tests {
@@ -753,7 +738,7 @@ func Test_defaultBackendSGProvider_Release(t *testing.T) {
 			}
 			for _, item := range tt.fields.resourceMapItems {
 				var resourceType ResourceType = ResourceTypeIngress
-				if reflect.TypeOf(item).String() == "service" {
+				if reflect.TypeOf(item.key).String() == "*v1.Service" {
 					resourceType = ResourceTypeService
 				}
 				sgProvider.objectsMap.Store(getObjectKey(resourceType, k8s.NamespacedName(item.key)), item.value)