add checks in securitygroup controller to avoid panic

Signed-off-by: hanenMizouni <[email protected]>
outscale · Dec 12, 2024 · eac07ee · eac07ee
1 parent 6f06eef
commit eac07ee
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 12 deletions.
diff --git a/controllers/osccluster_securitygroup_controller.go b/controllers/osccluster_securitygroup_controller.go
@@ -368,11 +368,18 @@ func reconcileDeleteSecurityGroupsRule(ctx context.Context, clusterScope *scope.
 	IpRange := securityGroupRuleSpec.IpRange
 	FromPortRange := securityGroupRuleSpec.FromPortRange
 	ToPortRange := securityGroupRuleSpec.ToPortRange
-	associateSecurityGroupId := securityGroupsRef.ResourceMap[securityGroupName]
-	targetSecurityGroupName := securityGroupRuleSpec.TargetSecurityGroupName
+	associateSecurityGroupId, exists := securityGroupsRef.ResourceMap[securityGroupName]
+	if !exists || associateSecurityGroupId == "" {
+		return reconcile.Result{}, fmt.Errorf("associateSecurityGroupId not found in ResourceMap for securityGroupName %s", securityGroupName)
+	}
 	targetSecurityGroupId := ""
-	if targetSecurityGroupName != "" {
-		targetSecurityGroupId = securityGroupsRef.ResourceMap[targetSecurityGroupName]
+	if targetSecurityGroupName := securityGroupRuleSpec.TargetSecurityGroupName; targetSecurityGroupName != "" {
+		if targetId, ok := securityGroupsRef.ResourceMap[targetSecurityGroupName]; ok && targetId != "" {
+			targetSecurityGroupId = targetId
+		} else {
+			clusterScope.V(2).Info("Target security group not found", "targetSecurityGroupName", targetSecurityGroupName)
+			return reconcile.Result{}, fmt.Errorf("target security group %s does not exist", targetSecurityGroupName)
+		}
 	}
 
 	clusterScope.V(4).Info("Check if the desired securityGroupRule exist", "securityGroupRuleName", securityGroupRuleName)
@@ -396,7 +403,9 @@ func reconcileDeleteSecurityGroupsRule(ctx context.Context, clusterScope *scope.
 // ReconcileRoute reconcile the RouteTable and the Route of the cluster.
 func reconcileDeleteSecurityGroup(ctx context.Context, clusterScope *scope.ClusterScope, securityGroupId string, securityGroupSvc security.OscSecurityGroupInterface) (reconcile.Result, error) {
 	securityGroupsRef := clusterScope.GetSecurityGroupsRef()
-
+	if securityGroupsRef == nil || securityGroupsRef.ResourceMap == nil {
+		return reconcile.Result{}, fmt.Errorf("securityGroupsRef or its ResourceMap is nil; ensure security groups are reconciled first")
+	}
 	clusterScope.V(4).Info("Check if the securityGroup exists", "securityGroupId", securityGroupId)
 	securityGroup, err := securityGroupSvc.GetSecurityGroup(securityGroupId)
 	if err != nil {
@@ -428,6 +437,9 @@ func reconcileDeleteSecurityGroups(ctx context.Context, clusterScope *scope.Clus
 		securityGroupsSpec = clusterScope.GetSecurityGroups()
 	}
 	securityGroupsRef := clusterScope.GetSecurityGroupsRef()
+	if securityGroupsRef == nil || securityGroupsRef.ResourceMap == nil {
+		return reconcile.Result{}, fmt.Errorf("securityGroupsRef or its ResourceMap is nil; ensure security groups are reconciled first")
+	}
 
 	netSpec := clusterScope.GetNet()
 	netSpec.SetDefaultValue()

diff --git a/controllers/osccluster_securitygroup_controller_unit_test.go b/controllers/osccluster_securitygroup_controller_unit_test.go
@@ -140,6 +140,12 @@ func defaultSecurityGroupSpec() infrastructurev1beta1.OscClusterSpec {
 	}
 }
 
+func missingTargetSecurityGroupSpec() infrastructurev1beta1.OscClusterSpec {
+	spec := defaultSecurityGroupSpec()
+	spec.Network.SecurityGroups[0].SecurityGroupRules[0].TargetSecurityGroupName = "test-securitygroup-target"
+	return spec
+}
+
 // SetupWithSecurityGroupMock set securityGroupMock with clusterScope and osccluster
 func SetupWithSecurityGroupMock(t *testing.T, name string, spec infrastructurev1beta1.OscClusterSpec) (clusterScope *scope.ClusterScope, ctx context.Context, mockOscSecurityGroupInterface *mock_security.MockOscSecurityGroupInterface, mockOscTagInterface *mock_tag.MockOscTagInterface) {
 	clusterScope = Setup(t, name, spec)

diff --git a/testenv/osccluster_controller_test.go b/testenv/osccluster_controller_test.go
@@ -869,12 +869,12 @@ var _ = Describe("Outscale Cluster Reconciler", func() {
 									ToPortRange:   6443,
 								},
 								{
-									Name:          "cluster-api-securitygrouprule-http",
-									Flow:          "Inbound",
-									IpProtocol:    "tcp",
-									IpRange:       "0.0.0.0/0",
-									FromPortRange: 80,
-									ToPortRange:   80,
+									Name:                    "cluster-api-securitygrouprule-http",
+									Flow:                    "Inbound",
+									IpProtocol:              "tcp",
+									IpRange:                 "0.0.0.0/0",
+									FromPortRange:           80,
+									ToPortRange:             80,
 									TargetSecurityGroupName: "cluster-api-securitygroups",
 								},
 								{
@@ -1156,4 +1156,4 @@ var _ = Describe("Outscale Cluster Reconciler", func() {
 			createCheckDeleteOscClusterMachine(ctx, infraClusterSpec, infraMachineSpec)
 		})
 	})
-})
+})