fix: handle some more nil checks and possible panics

Author: joekr

cloud/ociutil/ptr/from_ptr.go

@@ -13,6 +13,17 @@ func ToString(p *string) (v string) {
 	return *p
 }
 
+// ToBool returns bool value dereferenced if the passed
+// in pointer was not nil. Returns a bool zero value (false) if the
+// pointer was nil.
+func ToBool(p *bool) (v bool) {
+	if p == nil {
+		return v
+	}
+
+	return *p
+}
+
 // ToStringSlice returns a slice of string values, that are
 // dereferenced if the passed in pointer was not nil. Returns a string
 // zero value if the pointer was nil.

cloud/ociutil/ptr/from_ptr_test.go

@@ -33,6 +33,38 @@ func TestToString(t *testing.T) {
 	}
 }
 
+func TestToBool(t *testing.T) {
+	tests := []struct {
+		name  string
+		input *bool
+		want  bool
+	}{
+		{
+			name:  "nil pointer",
+			input: nil,
+			want:  false,
+		},
+		{
+			name:  "pointer to true",
+			input: func() *bool { b := true; return &b }(),
+			want:  true,
+		},
+		{
+			name:  "pointer to false",
+			input: func() *bool { b := false; return &b }(),
+			want:  false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := ToBool(tt.input); got != tt.want {
+				t.Errorf("ToBool() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
 func TestToStringSlice(t *testing.T) {
 	tests := []struct {
 		name  string

cloud/scope/load_balancer_reconciler.go

@@ -230,7 +230,7 @@ func (s *ClusterScope) getLoadbalancerIp(lb loadbalancer.LoadBalancer) (*string,
 	if len(lb.IpAddresses) < 1 {
 		return nil, errors.New("lb does not have valid ip addresses")
 	}
-	if *lb.IsPrivate {
+	if ptr.ToBool(lb.IsPrivate) {
 		lbIp = lb.IpAddresses[0].IpAddress
 	} else {
 		for _, ip := range lb.IpAddresses {

cloud/scope/machine.go

@@ -221,8 +221,14 @@ func (m *MachineScope) GetOrCreateMachine(ctx context.Context) (*core.Instance,
 			m.Logger.Error(err, "Failure domain should be a value between 1 and 3")
 			return nil, err
 		}
-		faultDomain = m.OCIClusterAccessor.GetFailureDomains()[*failureDomain].Attributes[FaultDomain]
-		availabilityDomain = m.OCIClusterAccessor.GetFailureDomains()[*failureDomain].Attributes[AvailabilityDomain]
+		fd, exists := m.OCIClusterAccessor.GetFailureDomains()[*failureDomain]
+		if !exists {
+			err = errors.New("failure domain not found in cluster failure domains")
+			m.Logger.Error(err, "Failure domain not found", "failure-domain", *failureDomain)
+			return nil, err
+		}
+		faultDomain = fd.Attributes[FaultDomain]
+		availabilityDomain = fd.Attributes[AvailabilityDomain]
 	} else {
 		randomFailureDomain, err := rand.Int(rand.Reader, big.NewInt(3))
 		if err != nil {
@@ -231,7 +237,13 @@ func (m *MachineScope) GetOrCreateMachine(ctx context.Context) (*core.Instance,
 		}
 		// the random number generated is between zero and two, whereas we need a number between one and three
 		failureDomain = common.String(strconv.Itoa(int(randomFailureDomain.Int64()) + 1))
-		availabilityDomain = m.OCIClusterAccessor.GetFailureDomains()[*failureDomain].Attributes[AvailabilityDomain]
+		fd, exists := m.OCIClusterAccessor.GetFailureDomains()[*failureDomain]
+		if !exists {
+			err = errors.New("failure domain not found in cluster failure domains")
+			m.Logger.Error(err, "Failure domain not found", "failure-domain", *failureDomain)
+			return nil, err
+		}
+		availabilityDomain = fd.Attributes[AvailabilityDomain]
 	}
 
 	metadata := m.OCIMachine.Spec.Metadata

cloud/scope/managed_machine_pool.go

@@ -345,7 +345,7 @@ func (m *ManagedMachinePoolScope) CreateNodePool(ctx context.Context) (*oke.Node
 	resources := wrResponse.Resources
 	var nodePoolId *string
 	for _, resource := range resources {
-		if *resource.EntityType == "nodepool" {
+		if ptr.ToString(resource.EntityType) == "nodepool" {
 			nodePoolId = resource.Identifier
 		}
 	}

cloud/scope/network_load_balancer_reconciler.go

@@ -246,7 +246,7 @@ func (s *ClusterScope) getNetworkLoadbalancerIp(nlb networkloadbalancer.NetworkL
 	if len(nlb.IpAddresses) < 1 {
 		return nil, errors.New("nlb does not have valid ip addresses")
 	}
-	if *nlb.IsPrivate {
+	if ptr.ToBool(nlb.IsPrivate) {
 		nlbIp = nlb.IpAddresses[0].IpAddress
 	} else {
 		for _, ip := range nlb.IpAddresses {

cloud/scope/nsg_reconciler.go

@@ -131,7 +131,7 @@ func (s *ClusterScope) GetNSG(ctx context.Context, spec infrastructurev1beta2.NS
 		return nil, errors.Wrap(err, "failed to list network security groups")
 	}
 	for _, nsg := range nsgs.Items {
-		if &nsg != nil && s.IsResourceCreatedByClusterAPI(nsg.FreeformTags) {
+		if s.IsResourceCreatedByClusterAPI(nsg.FreeformTags) {
 			return &nsg, nil
 		}
 	}

cloud/scope/route_table_reconciler.go

@@ -163,7 +163,8 @@ func (s *ClusterScope) CreateRouteTable(ctx context.Context, routeTableType stri
 		}
 		resp, err := s.VCNClient.GetVcn(ctx, core.GetVcnRequest{VcnId: s.getVcnId()})
 		if err != nil {
-			panic(err)
+			s.Logger.Error(err, "failed to get VCN")
+			return nil, errors.Wrap(err, "failed to get VCN for route table creation")
 		}
 		if resp.Vcn.Ipv6CidrBlocks != nil {
 			routeRules = append(routeRules, core.RouteRule{

cloud/scope/subnet_reconciler.go

@@ -119,15 +119,15 @@ func (s *ClusterScope) CreateSubnet(ctx context.Context, spec infrastructurev1be
 	var ipv6subnetCIDR_Ptr *string
 
 	// Constructing IPv6 Subnet CIDR
-	if resp.Vcn.Ipv6CidrBlocks != nil {
+	if len(resp.Vcn.Ipv6CidrBlocks) > 0 {
 
 		// VCNs can have multiple IPv6 CIDR Blocks, and the CIDR block with IPv6 GUA Allocated by Oracle is the first (index 0) in the list
 		vcnCIDR := resp.Vcn.Ipv6CidrBlocks[0]
 
 		// Split CIDR block into hextets
 		ip, _, err := net.ParseCIDR(vcnCIDR)
 		if err != nil {
-			panic(err)
+			return nil, errors.Wrap(err, "failed to parse IPv6 CIDR block")
 		}
 		hextets := strings.Split(ip.String(), ":")