fix security list rule clean up flow for OCI loadbalancer delete calls

Author: pranavsriram8

pkg/cloudprovider/providers/oci/load_balancer.go

@@ -1421,6 +1421,7 @@ func (cp *CloudProvider) cleanupSecurityRulesForLoadBalancerDelete(lb *client.Ge
 			return errors.Wrapf(err, "delete security rules for listener %q on load balancer %q", listenerName, name)
 		}
 
+		logger.Infof("Security rule management mode %s", securityRuleManagerMode)
 		if securityRuleManagerMode == ManagementModeAll || securityRuleManagerMode == ManagementModeFrontend {
 			if err = securityListManager.Delete(ctx, lbSubnets, nodeSubnets, ports, sourceCIDRs, isPreserveSource); err != nil {
 				logger.With(zap.Error(err)).Errorf("Failed to delete security rules for listener %q on load balancer %q", listenerName, name)

pkg/cloudprovider/providers/oci/load_balancer_security_lists.go

@@ -382,7 +382,7 @@ func getNodeIngressRules(
 					"source", *rule.Source,
 					"destinationPortRangeMin", *rule.TcpOptions.DestinationPortRange.Min,
 					"destinationPortRangeMax", *rule.TcpOptions.DestinationPortRange.Max,
-				).Debug("Deleting load balancer ingres security rule")
+				).Debug("Deleting node ingress security rule")
 				continue
 			}
 		}
@@ -512,7 +512,7 @@ func getLoadBalancerIngressRules(
 			"source", *rule.Source,
 			"destinationPortRangeMin", *rule.TcpOptions.DestinationPortRange.Min,
 			"destinationPortRangeMax", *rule.TcpOptions.DestinationPortRange.Max,
-		).Debug("Deleting load balancer ingres security rule")
+		).Debug("Deleting load balancer ingress security rule")
 	}
 
 	if desired.Len() == 0 {
@@ -558,7 +558,7 @@ func getLoadBalancerEgressRules(
 				"destination", *rule.Destination,
 				"destinationPortRangeMin", *rule.TcpOptions.DestinationPortRange.Min,
 				"destinationPortRangeMax", *rule.TcpOptions.DestinationPortRange.Max,
-			).Debug("Deleting load balancer ingres security rule")
+			).Debug("Deleting load balancer egress security rule")
 			continue
 		}
 
@@ -655,15 +655,20 @@ func makeIngressSecurityRule(cidrBlock string, port int) core.IngressSecurityRul
 
 func portInUse(serviceLister listersv1.ServiceLister, port int32) (bool, error) {
 	serviceList, err := serviceLister.List(labels.Everything())
+
 	if err != nil {
 		return false, err
 	}
 	for _, service := range serviceList {
-		if service.Spec.Type == api.ServiceTypeLoadBalancer {
-			for _, p := range service.Spec.Ports {
-				if p.Port == port {
-					return true, nil
-				}
+		if service.DeletionTimestamp != nil {
+			continue
+		}
+		if service.Spec.Type != api.ServiceTypeLoadBalancer {
+			continue
+		}
+		for _, p := range service.Spec.Ports {
+			if p.Port == port {
+				return true, nil
 			}
 		}
 	}

pkg/cloudprovider/providers/oci/load_balancer_spec.go

@@ -421,17 +421,17 @@ func NewLBSpec(logger *zap.SugaredLogger, svc *v1.Service, nodes []*v1.Node, sub
 
 func getSecurityListManagementMode(svc *v1.Service) (string, error) {
 	lbType := getLoadBalancerType(svc)
+	logger := *zap.L().Sugar()
 	knownSecListModes := map[string]struct{}{
 		ManagementModeAll:      struct{}{},
 		ManagementModeNone:     struct{}{},
 		ManagementModeFrontend: struct{}{},
 	}
-
+	annotationExists := false
+	var annotationValue string
 	switch lbType {
 	case NLB:
 		{
-			annotationExists := false
-			var annotationValue string
 			annotationValue, annotationExists = svc.Annotations[ServiceAnnotationNetworkLoadBalancerSecurityListManagementMode]
 			if !annotationExists {
 				return ManagementModeNone, nil
@@ -442,6 +442,14 @@ func getSecurityListManagementMode(svc *v1.Service) (string, error) {
 			return svc.Annotations[ServiceAnnotationNetworkLoadBalancerSecurityListManagementMode], nil
 		}
 	default:
+		annotationValue, annotationExists = svc.Annotations[ServiceAnnotationLoadBalancerSecurityListManagementMode]
+		if !annotationExists {
+			return ManagementModeAll, nil
+		}
+		if _, ok := knownSecListModes[annotationValue]; !ok {
+			logger.Infof("invalid value: %s provided for annotation: %s; using default All", annotationValue, ServiceAnnotationLoadBalancerSecurityListManagementMode)
+			return ManagementModeAll, nil
+		}
 		return svc.Annotations[ServiceAnnotationLoadBalancerSecurityListManagementMode], nil
 	}
 }

pkg/cloudprovider/providers/oci/load_balancer_spec_test.go

@@ -4007,13 +4007,23 @@ func Test_getSecurityListManagementMode(t *testing.T) {
 		service  *v1.Service
 		expected string
 	}{
-		"defaults": {
+		"defaults - lb": {
 			service: &v1.Service{
 				ObjectMeta: metav1.ObjectMeta{
 					Annotations: map[string]string{},
 				},
 			},
-			expected: "",
+			expected: "All",
+		},
+		"defaults - nlb": {
+			service: &v1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						ServiceAnnotationLoadBalancerType: "nlb",
+					},
+				},
+			},
+			expected: "None",
 		},
 		"lb mode None": {
 			service: &v1.Service{
@@ -4115,7 +4125,22 @@ func Test_getRuleManagementMode(t *testing.T) {
 					Annotations: map[string]string{},
 				},
 			},
-			expected: "",
+			expected: "All",
+			nsg: &ManagedNetworkSecurityGroup{
+				nsgRuleManagementMode: ManagementModeNone,
+				frontendNsgId:         "",
+				backendNsgId:          []string{},
+			},
+		},
+		"defaults - nlb": {
+			service: &v1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						ServiceAnnotationLoadBalancerType: "nlb",
+					},
+				},
+			},
+			expected: "None",
 			nsg: &ManagedNetworkSecurityGroup{
 				nsgRuleManagementMode: ManagementModeNone,
 				frontendNsgId:         "",