Merge pull request csi-addons#207 from Rakshith-R/bp-SCName

BUG 2314404: controller/pvc: add checks for nil/empty pvc.Spec.StorageClassName
rewantsoni · Sep 24, 2024 · 9a83108 · 9a83108
2 parents 22ad64f + b51cd65
commit 9a83108
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 6 deletions.
diff --git a/internal/controller/csiaddons/persistentvolumeclaim_controller.go b/internal/controller/csiaddons/persistentvolumeclaim_controller.go
@@ -264,22 +264,23 @@ func (r *PersistentVolumeClaimReconciler) determineScheduleAndRequeue(
 		}
 	}
 
-	// For static provisioned PVs, StorageClassName is empty.
-	if len(*pvc.Spec.StorageClassName) == 0 {
+	// For static provisioned PVs, StorageClassName is nil or empty.
+	if pvc.Spec.StorageClassName == nil || len(*pvc.Spec.StorageClassName) == 0 {
 		logger.Info("StorageClassName is empty")
 		return "", ErrScheduleNotFound
 	}
+	storageClassName := *pvc.Spec.StorageClassName
 
 	// check for storageclass schedule annotation.
 	sc := &storagev1.StorageClass{}
-	err = r.Client.Get(ctx, types.NamespacedName{Name: *pvc.Spec.StorageClassName}, sc)
+	err = r.Client.Get(ctx, types.NamespacedName{Name: storageClassName}, sc)
 	if err != nil {
 		if apierrors.IsNotFound(err) {
-			logger.Error(err, "StorageClass not found", "StorageClass", *pvc.Spec.StorageClassName)
+			logger.Error(err, "StorageClass not found", "StorageClass", storageClassName)
 			return "", ErrScheduleNotFound
 		}
 
-		logger.Error(err, "Failed to get StorageClass", "StorageClass", *pvc.Spec.StorageClassName)
+		logger.Error(err, "Failed to get StorageClass", "StorageClass", storageClassName)
 		return "", err
 	}
 	schedule, scheduleFound = getScheduleFromAnnotation(annotationKey, logger, sc.Annotations)
@@ -367,7 +368,7 @@ func (r *PersistentVolumeClaimReconciler) SetupWithManager(mgr ctrl.Manager, ctr
 		"spec.storageClassName",
 		func(rawObj client.Object) []string {
 			pvc, ok := rawObj.(*corev1.PersistentVolumeClaim)
-			if !ok {
+			if !ok || pvc.Spec.StorageClassName == nil || len(*pvc.Spec.StorageClassName) == 0 {
 				return nil
 			}
 			return []string{*pvc.Spec.StorageClassName}

diff --git a/internal/controller/csiaddons/persistentvolumeclaim_controller_test.go b/internal/controller/csiaddons/persistentvolumeclaim_controller_test.go
@@ -364,4 +364,12 @@ func TestDetermineScheduleAndRequeue(t *testing.T) {
 		assert.Equal(t, "", schedule)
 	})
 
+	// test for StorageClassName is nil
+	t.Run("StorageClassName is nil", func(t *testing.T) {
+		pvc.Spec.StorageClassName = nil
+		pvc.Annotations = nil
+		schedule, error := r.determineScheduleAndRequeue(ctx, &logger, pvc, driverName, rsCronJobScheduleTimeAnnotation)
+		assert.ErrorIs(t, error, ErrScheduleNotFound)
+		assert.Equal(t, "", schedule)
+	})
 }