refactor security group handling to reuse worker node security group instead of creating new one

Author: M00nF1sh

internal/alb/lb/loadbalancer.go

@@ -62,11 +62,10 @@ type loadBalancerConfig struct {
 	Name string
 	Tags map[string]string
 
-	Type           *string
-	Scheme         *string
-	IpAddressType  *string
-	SecurityGroups []string
-	Subnets        []string
+	Type          *string
+	Scheme        *string
+	IpAddressType *string
+	Subnets       []string
 }
 
 type defaultController struct {
@@ -88,6 +87,7 @@ func (controller *defaultController) Reconcile(ctx context.Context, ingress *ext
 	if err != nil {
 		return nil, err
 	}
+
 	lbConfig, err := controller.buildLBConfig(ctx, ingress, ingressAnnos)
 	if err != nil {
 		return nil, fmt.Errorf("failed to build LoadBalancer configuration due to %v", err)
@@ -96,7 +96,12 @@ func (controller *defaultController) Reconcile(ctx context.Context, ingress *ext
 		return nil, err
 	}
 
-	instance, err := controller.ensureLBInstance(ctx, lbConfig)
+	ingKey := k8s.NamespacedName(ingress)
+	sgAttachment, err := controller.sgAssociationController.Setup(ctx, ingKey)
+	if err != nil {
+		return nil, err
+	}
+	instance, err := controller.ensureLBInstance(ctx, lbConfig, sgAttachment)
 	if err != nil {
 		return nil, err
 	}
@@ -122,7 +127,7 @@ func (controller *defaultController) Reconcile(ctx context.Context, ingress *ext
 		return nil, fmt.Errorf("failed to GC targetGroups due to %v", err)
 	}
 
-	if err := controller.sgAssociationController.Reconcile(ctx, ingress, instance, tgGroup); err != nil {
+	if err := controller.sgAssociationController.Reconcile(ctx, ingKey, sgAttachment, instance, tgGroup); err != nil {
 		return nil, fmt.Errorf("failed to reconcile securityGroup associations due to %v", err)
 	}
 	return &LoadBalancer{
@@ -138,9 +143,6 @@ func (controller *defaultController) Delete(ctx context.Context, ingressKey type
 		return fmt.Errorf("failed to find existing LoadBalancer due to %v", err)
 	}
 	if instance != nil {
-		if err = controller.sgAssociationController.Delete(ctx, ingressKey, instance); err != nil {
-			return fmt.Errorf("failed to clean up securityGroups due to %v", err)
-		}
 		if err = controller.lsGroupController.Delete(ctx, aws.StringValue(instance.LoadBalancerArn)); err != nil {
 			return fmt.Errorf("failed to delete listeners due to %v", err)
 		}
@@ -153,24 +155,27 @@ func (controller *defaultController) Delete(ctx context.Context, ingressKey type
 			return err
 		}
 	}
+	if err = controller.sgAssociationController.Delete(ctx, ingressKey); err != nil {
+		return fmt.Errorf("failed to clean up securityGroups due to %v", err)
+	}
 
 	return nil
 }
 
-func (controller *defaultController) ensureLBInstance(ctx context.Context, lbConfig *loadBalancerConfig) (*elbv2.LoadBalancer, error) {
+func (controller *defaultController) ensureLBInstance(ctx context.Context, lbConfig *loadBalancerConfig, sgAttachment sg.LbAttachmentInfo) (*elbv2.LoadBalancer, error) {
 	instance, err := controller.cloud.GetLoadBalancerByName(ctx, lbConfig.Name)
 	if err != nil {
 		return nil, fmt.Errorf("failed to find existing LoadBalancer due to %v", err)
 	}
 	if instance == nil {
-		instance, err = controller.newLBInstance(ctx, lbConfig)
+		instance, err = controller.newLBInstance(ctx, lbConfig, sgAttachment)
 		if err != nil {
 			return nil, fmt.Errorf("failed to create LoadBalancer due to %v", err)
 		}
 		return instance, nil
 	}
 	if controller.isLBInstanceNeedRecreation(ctx, instance, lbConfig) {
-		instance, err = controller.recreateLBInstance(ctx, instance, lbConfig)
+		instance, err = controller.recreateLBInstance(ctx, instance, lbConfig, sgAttachment)
 		if err != nil {
 			return nil, fmt.Errorf("failed to recreate LoadBalancer due to %v", err)
 		}
@@ -182,14 +187,14 @@ func (controller *defaultController) ensureLBInstance(ctx context.Context, lbCon
 	return instance, nil
 }
 
-func (controller *defaultController) newLBInstance(ctx context.Context, lbConfig *loadBalancerConfig) (*elbv2.LoadBalancer, error) {
+func (controller *defaultController) newLBInstance(ctx context.Context, lbConfig *loadBalancerConfig, sgAttachment sg.LbAttachmentInfo) (*elbv2.LoadBalancer, error) {
 	albctx.GetLogger(ctx).Infof("creating LoadBalancer %v", lbConfig.Name)
 	resp, err := controller.cloud.CreateLoadBalancerWithContext(ctx, &elbv2.CreateLoadBalancerInput{
 		Name:           aws.String(lbConfig.Name),
 		Type:           lbConfig.Type,
 		Scheme:         lbConfig.Scheme,
 		IpAddressType:  lbConfig.IpAddressType,
-		SecurityGroups: aws.StringSlice(lbConfig.SecurityGroups),
+		SecurityGroups: aws.StringSlice(sgAttachment.SGIDs()),
 		Subnets:        aws.StringSlice(lbConfig.Subnets),
 		Tags:           tags.ConvertToELBV2(lbConfig.Tags),
 	})
@@ -205,13 +210,13 @@ func (controller *defaultController) newLBInstance(ctx context.Context, lbConfig
 	return instance, nil
 }
 
-func (controller *defaultController) recreateLBInstance(ctx context.Context, existingInstance *elbv2.LoadBalancer, lbConfig *loadBalancerConfig) (*elbv2.LoadBalancer, error) {
+func (controller *defaultController) recreateLBInstance(ctx context.Context, existingInstance *elbv2.LoadBalancer, lbConfig *loadBalancerConfig, sgAttachment sg.LbAttachmentInfo) (*elbv2.LoadBalancer, error) {
 	existingLBArn := aws.StringValue(existingInstance.LoadBalancerArn)
 	albctx.GetLogger(ctx).Infof("deleting LoadBalancer %v for recreation", existingLBArn)
 	if err := controller.cloud.DeleteLoadBalancerByArn(ctx, existingLBArn); err != nil {
 		return nil, err
 	}
-	return controller.newLBInstance(ctx, lbConfig)
+	return controller.newLBInstance(ctx, lbConfig, sgAttachment)
 }
 
 func (controller *defaultController) reconcileLBInstance(ctx context.Context, instance *elbv2.LoadBalancer, lbConfig *loadBalancerConfig) error {
@@ -307,15 +312,15 @@ func (controller *defaultController) buildLBConfig(ctx context.Context, ingress
 	if err != nil {
 		return nil, err
 	}
+
 	return &loadBalancerConfig{
 		Name: controller.nameTagGen.NameLB(ingress.Namespace, ingress.Name),
 		Tags: lbTags,
 
-		Type:           aws.String(elbv2.LoadBalancerTypeEnumApplication),
-		Scheme:         ingressAnnos.LoadBalancer.Scheme,
-		IpAddressType:  ingressAnnos.LoadBalancer.IPAddressType,
-		SecurityGroups: ingressAnnos.LoadBalancer.SecurityGroups,
-		Subnets:        subnets,
+		Type:          aws.String(elbv2.LoadBalancerTypeEnumApplication),
+		Scheme:        ingressAnnos.LoadBalancer.Scheme,
+		IpAddressType: ingressAnnos.LoadBalancer.IPAddressType,
+		Subnets:       subnets,
 	}, nil
 }