Fix nil ptr exceptions in ontap rest layer

Author: vivakeram

storage_drivers/ontap/api/abstraction_rest.go

@@ -1341,14 +1341,19 @@ func (d OntapAPIREST) QtreeListByPrefix(ctx context.Context, prefix, volumePrefi
 	}
 	qtrees := Qtrees{}
 	for _, qtree := range qtreeList.GetPayload().QtreeResponseInlineRecords {
-		newQtree := d.convertQtree(qtree)
-		qtrees = append(qtrees, newQtree)
+		if newQtree := d.convertQtree(qtree); newQtree != nil {
+			qtrees = append(qtrees, newQtree)
+		}
 	}
 
 	return qtrees, nil
 }
 
 func (d OntapAPIREST) convertQtree(qtree *models.Qtree) *Qtree {
+	if qtree == nil {
+		return nil
+	}
+
 	newQtree := &Qtree{}
 
 	if qtree.Name != nil {
@@ -1379,7 +1384,11 @@ func (d OntapAPIREST) QtreeGetByName(ctx context.Context, name, volumePrefix str
 		Logc(ctx).WithError(err).Error(msg)
 		return nil, errors.New(msg)
 	}
-	return d.convertQtree(qtree), nil
+	converted := d.convertQtree(qtree)
+	if converted == nil {
+		return nil, errors.NotFoundError("qtree %q not found", name)
+	}
+	return converted, nil
 }
 
 func (d OntapAPIREST) QuotaEntryList(ctx context.Context, volumeName string) (QuotaEntries, error) {
@@ -1390,7 +1399,9 @@ func (d OntapAPIREST) QuotaEntryList(ctx context.Context, volumeName string) (Qu
 	entries := QuotaEntries{}
 	if response != nil && response.Payload != nil {
 		for _, entry := range response.Payload.QuotaRuleResponseInlineRecords {
-			entries = append(entries, d.convertQuota(entry))
+			if converted := d.convertQuota(entry); converted != nil {
+				entries = append(entries, converted)
+			}
 		}
 	}
 	return entries, nil
@@ -1458,20 +1469,27 @@ func (d OntapAPIREST) QuotaGetEntry(ctx context.Context, volumeName, qtreeName,
 	quota, err := d.api.QuotaGetEntry(ctx, volumeName, qtreeName, quotaType)
 	if err != nil {
 		Logc(ctx).WithError(err).Error("error getting quota rule")
+		return nil, err
 	}
 	quotaEntry := d.convertQuota(quota)
+	if quotaEntry == nil {
+		return nil, errors.NotFoundError("quota rule for volume %q qtree %q not found", volumeName, qtreeName)
+	}
 	return quotaEntry, nil
 }
 
 func (d OntapAPIREST) convertQuota(quota *models.QuotaRule) *QuotaEntry {
+	if quota == nil {
+		return nil
+	}
 	diskLimit := int64(-1)
 	if quota.Space != nil && quota.Space.HardLimit != nil {
 		diskLimit = *quota.Space.HardLimit
 	}
 	quotaEntry := &QuotaEntry{
 		DiskLimitBytes: diskLimit,
 	}
-	if quota != nil && quota.Volume != nil && quota.Volume.Name != nil && *quota.Volume.Name != "" {
+	if quota.Volume != nil && quota.Volume.Name != nil && *quota.Volume.Name != "" {
 		quotaEntry.Target = fmt.Sprintf("/vol/%s", *quota.Volume.Name)
 
 		if quota.Qtree != nil && quota.Qtree.Name != nil && *quota.Qtree.Name != "" {
@@ -1558,10 +1576,15 @@ func (d OntapAPIREST) VolumeWaitForStates(ctx context.Context, volumeName string
 		}
 
 		if vol == nil {
+			volumeState = ""
 			return fmt.Errorf("volume %v not found", volumeName)
 		}
 
-		Logc(ctx).Debugf("Volume %v is in state:%v", volumeName, *vol.State)
+		if vol.State == nil {
+			volumeState = ""
+			return fmt.Errorf("volume %v state not yet populated", volumeName)
+		}
+
 		volumeState = *vol.State
 
 		if collection.ContainsString(desiredStates, volumeState) {
@@ -1966,7 +1989,7 @@ func (d OntapAPIREST) SnapmirrorGet(
 		snapmirror.IsHealthy = *snapmirrorResponse.Healthy
 		if !snapmirror.IsHealthy {
 			unhealthyReason := snapmirrorResponse.SnapmirrorRelationshipInlineUnhealthyReason
-			if len(unhealthyReason) > 0 {
+			if len(unhealthyReason) > 0 && unhealthyReason[0] != nil && unhealthyReason[0].Message != nil {
 				snapmirror.UnhealthyReason = *unhealthyReason[0].Message
 			}
 		}
@@ -2138,8 +2161,10 @@ func (d OntapAPIREST) SnapmirrorBreak(
 func (d OntapAPIREST) SnapmirrorUpdate(ctx context.Context, localInternalVolumeName, snapshotName string) error {
 	err := d.api.SnapmirrorUpdate(ctx, localInternalVolumeName, snapshotName)
 	if err != nil {
-		if restErr, err := ExtractErrorResponse(ctx, err); err == nil {
-			return errors.New(*restErr.Error.Message)
+		if restErr, extractErr := ExtractErrorResponse(ctx, err); extractErr == nil {
+			if restErr != nil && restErr.Error != nil && restErr.Error.Message != nil {
+				return errors.New(*restErr.Error.Message)
+			}
 		}
 		return err
 	}
@@ -3508,6 +3533,9 @@ func (d OntapAPIREST) NVMeAddHostToSubsystem(ctx context.Context, hostNQN, subsy
 	}
 
 	for _, host := range hosts {
+		if host == nil || host.Nqn == nil {
+			continue
+		}
 		if *host.Nqn == hostNQN {
 			// Host already part of the subsystem, no need to add it again
 			return nil
@@ -3537,7 +3565,7 @@ func (d OntapAPIREST) NVMeRemoveHostFromSubsystem(ctx context.Context, hostNQN,
 
 	hostFound := false
 	for _, host := range hosts {
-		if host != nil && *host.Nqn == hostNQN {
+		if host != nil && host.Nqn != nil && *host.Nqn == hostNQN {
 			hostFound = true
 			break
 		}
@@ -3589,6 +3617,9 @@ func (d OntapAPIREST) NVMeSubsystemCreate(ctx context.Context, subsystemName, co
 						return nil, err
 					}
 					if subsystem != nil {
+						if subsystem.UUID == nil || subsystem.Name == nil || subsystem.TargetNqn == nil {
+							return nil, fmt.Errorf("subsystem %v returned with missing fields", subsystemName)
+						}
 						return &NVMeSubsystem{UUID: *subsystem.UUID, Name: *subsystem.Name, NQN: *subsystem.TargetNqn}, nil
 					}
 					return nil, fmt.Errorf("unable to create subsystem %v", subsystemName)
@@ -3603,6 +3634,10 @@ func (d OntapAPIREST) NVMeSubsystemCreate(ctx context.Context, subsystemName, co
 		}
 	}
 
+	if subsystem.UUID == nil || subsystem.Name == nil || subsystem.TargetNqn == nil {
+		return nil, fmt.Errorf("subsystem %v returned with missing fields", subsystemName)
+	}
+
 	Logc(ctx).Debugf("Found subsystem %v and target nqns are %v", *subsystem.Name, *subsystem.TargetNqn)
 
 	return &NVMeSubsystem{UUID: *subsystem.UUID, Name: *subsystem.Name, NQN: *subsystem.TargetNqn}, nil

storage_drivers/ontap/api/abstraction_rest_test.go

@@ -1041,6 +1041,31 @@ func TestVolumeWaitForStates(t *testing.T) {
 	assert.Equal(t, currentState, *volume.State, "volume state does not match")
 
 	ctrl.Finish()
+
+	// Test8: Error - Volume returned with nil State (transient ONTAP response during clone init);
+	// must retry and eventually time out without panicking (TRID-19789).
+	ctrl = gomock.NewController(t)
+	mock = mockapi.NewMockRestClientInterface(ctrl)
+	oapi, err = api.NewOntapAPIRESTFromRestClientInterface(mock)
+	assert.NoError(t, err)
+	nilStateDesiredStates := []string{"online"}
+	nilStateAbortStates := []string{}
+	nilStateVolume := &models.Volume{State: nil}
+	nilStateMaxElapsedTime := 2 * time.Second
+
+	mock.EXPECT().ClientConfig().Return(clientConfig).AnyTimes()
+	// MinTimes(2) asserts the nil-State error is treated as transient: the closure
+	// must be retried at least once before the backoff window elapses. This guards
+	// against a future change accidentally wrapping the error in backoff.Permanent,
+	// which would silently bypass the retry semantic the TRID-19789 fix relies on.
+	mock.EXPECT().VolumeGetByName(ctx, volName, gomock.Any()).Return(nilStateVolume, nil).MinTimes(2)
+
+	currentState, err = oapi.VolumeWaitForStates(ctx, "fakeVolName", nilStateDesiredStates, nilStateAbortStates, nilStateMaxElapsedTime)
+
+	assert.Error(t, err, "expected timeout error when volume state is perpetually nil")
+	assert.Equal(t, "", currentState, "volume state should be empty when state was never populated")
+
+	ctrl.Finish()
 }
 
 func newMockOntapAPIREST(t *testing.T) (api.OntapAPIREST, *mockapi.MockRestClientInterface) {
@@ -2946,11 +2971,19 @@ func TestQuotaGetEntry(t *testing.T) {
 	assert.Equal(t, "/vol/quotaVolume/quotaQtree", quotaEntries.Target)
 	assert.Equal(t, int64(1073741810), quotaEntries.DiskLimitBytes)
 
-	// case 2: Quota get entry failed. Backend returned an error
+	// case 2: Quota get entry failed. Backend returned an error - it must be propagated.
 	rsi.EXPECT().QuotaGetEntry(ctx, quotaVolumeName, quotaQtreeName, quotaType).Return(
-		&quotaRule, errors.New("error getting quota rule"))
-	_, err = oapi.QuotaGetEntry(ctx, quotaVolumeName, quotaQtreeName, quotaType)
-	assert.NoError(t, err, "no error returned while getting a quota")
+		nil, errors.New("error getting quota rule"))
+	quotaEntries, err = oapi.QuotaGetEntry(ctx, quotaVolumeName, quotaQtreeName, quotaType)
+	assert.Error(t, err, "expected error to be propagated when backend returns one")
+	assert.Nil(t, quotaEntries, "expected nil quota entry on backend error")
+
+	// case 3: Backend returned (nil, nil) - the quota rule was not found.
+	rsi.EXPECT().QuotaGetEntry(ctx, quotaVolumeName, quotaQtreeName, quotaType).Return(nil, nil)
+	quotaEntries, err = oapi.QuotaGetEntry(ctx, quotaVolumeName, quotaQtreeName, quotaType)
+	assert.Error(t, err, "expected NotFoundError when underlying call returns no quota rule")
+	assert.True(t, errors.IsNotFoundError(err), "expected error to be a NotFoundError")
+	assert.Nil(t, quotaEntries, "expected nil quota entry when not found")
 }
 
 func getSnapshot() *models.Snapshot {

storage_drivers/ontap/api/ontap_rest.go

@@ -1302,7 +1302,7 @@ func (c *RestClient) listAllVolumeNamesBackedBySnapshot(ctx context.Context, vol
 		}
 		if vol.Clone != nil {
 			if vol.Clone.ParentSnapshot != nil && vol.Clone.ParentSnapshot.Name != nil && *vol.Clone.ParentSnapshot.Name == snapshotName &&
-				vol.Clone.ParentVolume != nil && *vol.Clone.ParentVolume.Name == volumeName {
+				vol.Clone.ParentVolume != nil && vol.Clone.ParentVolume.Name != nil && *vol.Clone.ParentVolume.Name == volumeName {
 				volumeNames = append(volumeNames, *vol.Name)
 			}
 		}
@@ -5429,7 +5429,7 @@ func (c *RestClient) QuotaSetEntry(ctx context.Context, qtreeName, volumeName, q
 		return c.QuotaAddEntry(ctx, volumeName, qtreeName, quotaType, diskLimit)
 	}
 
-	if quotaRule.UUID == nil {
+	if quotaRule == nil || quotaRule.UUID == nil {
 		return fmt.Errorf("unexpected response from quota entry lookup")
 	}
 
@@ -6169,7 +6169,10 @@ func (c *RestClient) SnapmirrorDeleteViaDestination(
 	if err != nil {
 		if restErr, extractErr := ExtractErrorResponse(ctx, err); extractErr == nil {
 			if restErr.Error != nil && restErr.Error.Code != nil && *restErr.Error.Code != ENTRY_DOESNT_EXIST {
-				return errors.New(*restErr.Error.Message)
+				if restErr.Error.Message != nil {
+					return errors.New(*restErr.Error.Message)
+				}
+				return fmt.Errorf("snapmirror destination delete failed with error code %v", *restErr.Error.Code)
 			}
 		} else {
 			return err