fix: add more nil checks and tests (#464)

Author: joekr

api/v1beta2/ocicluster_webhook.go

@@ -833,7 +833,7 @@ func (c *OCICluster) GetControlPlaneEndpointDefaultEgressRules() []EgressSecurit
 
 func (c *OCICluster) GetControlPlaneEndpointSubnet() *Subnet {
 	for _, subnet := range c.Spec.NetworkSpec.Vcn.Subnets {
-		if subnet.Role == ControlPlaneEndpointRole {
+		if subnet != nil && subnet.Role == ControlPlaneEndpointRole {
 			return subnet
 		}
 	}
@@ -842,7 +842,7 @@ func (c *OCICluster) GetControlPlaneEndpointSubnet() *Subnet {
 
 func (c *OCICluster) GetControlPlaneMachineSubnet() *Subnet {
 	for _, subnet := range c.Spec.NetworkSpec.Vcn.Subnets {
-		if subnet.Role == ControlPlaneRole {
+		if subnet != nil && subnet.Role == ControlPlaneRole {
 			return subnet
 		}
 	}
@@ -851,7 +851,7 @@ func (c *OCICluster) GetControlPlaneMachineSubnet() *Subnet {
 
 func (c *OCICluster) GetServiceLoadBalancerSubnet() *Subnet {
 	for _, subnet := range c.Spec.NetworkSpec.Vcn.Subnets {
-		if subnet.Role == ServiceLoadBalancerRole {
+		if subnet != nil && subnet.Role == ServiceLoadBalancerRole {
 			return subnet
 		}
 	}
@@ -861,7 +861,7 @@ func (c *OCICluster) GetServiceLoadBalancerSubnet() *Subnet {
 func (c *OCICluster) GetNodeSubnet() []*Subnet {
 	var nodeSubnets []*Subnet
 	for _, subnet := range c.Spec.NetworkSpec.Vcn.Subnets {
-		if subnet.Role == WorkerRole {
+		if subnet != nil && subnet.Role == WorkerRole {
 			nodeSubnets = append(nodeSubnets, subnet)
 		}
 	}
@@ -870,7 +870,7 @@ func (c *OCICluster) GetNodeSubnet() []*Subnet {
 
 func (c *OCICluster) IsNSGExitsByRole(role Role) bool {
 	for _, nsg := range c.Spec.NetworkSpec.Vcn.NetworkSecurityGroup.List {
-		if role == nsg.Role {
+		if nsg != nil && role == nsg.Role {
 			return true
 		}
 	}
@@ -879,7 +879,7 @@ func (c *OCICluster) IsNSGExitsByRole(role Role) bool {
 
 func (c *OCICluster) IsSecurityListExitsByRole(role Role) bool {
 	for _, subnet := range c.Spec.NetworkSpec.Vcn.Subnets {
-		if role == subnet.Role {
+		if subnet != nil && role == subnet.Role {
 			if subnet.SecurityList != nil {
 				return true
 			}

cloud/scope/drg_vcn_attachment_reconciler.go

@@ -32,6 +32,10 @@ func (s *ClusterScope) ReconcileDRGVCNAttachment(ctx context.Context) error {
 		return nil
 	}
 
+	if s.getDRG() == nil {
+		return errors.New("DRG is nil")
+	}
+
 	attachment, err := s.GetDRGAttachment(ctx)
 	if err != nil {
 		return err
@@ -65,6 +69,9 @@ func (s *ClusterScope) ReconcileDRGVCNAttachment(ctx context.Context) error {
 
 // nolint:nilnil
 func (s *ClusterScope) GetDRGAttachment(ctx context.Context) (*core.DrgAttachment, error) {
+	if s.getDRG() == nil {
+		return nil, errors.New("DRG is nil")
+	}
 	if s.getDRG().VcnAttachmentId != nil {
 		response, err := s.VCNClient.GetDrgAttachment(ctx, core.GetDrgAttachmentRequest{
 			DrgAttachmentId: s.getDRG().VcnAttachmentId,

cloud/scope/nsg_reconciler.go

@@ -37,16 +37,21 @@ func (s *ClusterScope) ReconcileNSG(ctx context.Context) error {
 	}
 	desiredNSGs := s.OCIClusterAccessor.GetNetworkSpec().Vcn.NetworkSecurityGroup
 	for _, desiredNSG := range desiredNSGs.List {
-		nsg, err := s.GetNSG(ctx, *desiredNSG)
+		if desiredNSG == nil {
+			s.Logger.Info("Skipping nil NSG pointer in spec")
+			continue
+		}
+		desiredNSGPtr := *desiredNSG
+		nsg, err := s.GetNSG(ctx, desiredNSGPtr)
 		if err != nil {
 			s.Logger.Error(err, "error to get nsg")
 			return err
 		}
 		if nsg != nil {
 			nsgOCID := nsg.Id
 			desiredNSG.ID = nsgOCID
-			if !s.IsNSGEqual(nsg, *desiredNSG) {
-				err = s.UpdateNSG(ctx, *desiredNSG)
+			if !s.IsNSGEqual(nsg, desiredNSGPtr) {
+				err = s.UpdateNSG(ctx, desiredNSGPtr)
 				if err != nil {
 					return err
 				}
@@ -55,7 +60,7 @@ func (s *ClusterScope) ReconcileNSG(ctx context.Context) error {
 			continue
 		}
 		s.Logger.Info("Creating the network security list")
-		nsgID, err := s.CreateNSG(ctx, *desiredNSG)
+		nsgID, err := s.CreateNSG(ctx, desiredNSGPtr)
 		if err != nil {
 			return err
 		}
@@ -64,6 +69,10 @@ func (s *ClusterScope) ReconcileNSG(ctx context.Context) error {
 
 	}
 	for _, desiredNSG := range desiredNSGs.List {
+		if desiredNSG == nil {
+			s.Logger.Info("Skipping nil NSG pointer in spec")
+			continue
+		}
 		s.adjustNSGRulesSpec(desiredNSG, desiredNSGs.List)
 		isNSGUpdated, err := s.UpdateNSGSecurityRulesIfNeeded(ctx, *desiredNSG, desiredNSG.ID)
 		if err != nil {

cloud/scope/nsg_reconciler_test.go

@@ -827,6 +827,23 @@ func TestClusterScope_ReconcileNSG(t *testing.T) {
 			wantErr:       true,
 			expectedError: "failed add nsg security rules: some error",
 		},
+		{
+			name: "nil NSG in list",
+			spec: infrastructurev1beta2.OCIClusterSpec{
+				NetworkSpec: infrastructurev1beta2.NetworkSpec{
+					Vcn: infrastructurev1beta2.VCN{
+						ID: common.String("vcn"),
+						NetworkSecurityGroup: infrastructurev1beta2.NetworkSecurityGroup{
+							List: []*infrastructurev1beta2.NSG{nil},
+						},
+					},
+				},
+			},
+			wantErr: false,
+			testSpecificSetup: func(clusterScope *ClusterScope, nlbClient *mock_vcn.MockClient) {
+				// nothing should be called
+			},
+		},
 	}
 	l := log.FromContext(context.Background())
 	for _, tt := range tests {

cloud/scope/vcn_reconciler.go

@@ -180,5 +180,10 @@ func (s *ClusterScope) DeleteVCN(ctx context.Context) error {
 }
 
 func (s *ClusterScope) getVcnId() *string {
-	return s.OCIClusterAccessor.GetNetworkSpec().Vcn.ID
+	id := s.OCIClusterAccessor.GetNetworkSpec().Vcn.ID
+	if id == nil {
+		s.Logger.Info("VCN ID is nil")
+
+	}
+	return id
 }