From 9752e2aabf0de835c71def7d3f666af59dbb5cb4 Mon Sep 17 00:00:00 2001
From: Manish <myathnal@redhat.com>
Date: Fri, 6 Sep 2024 12:51:02 +0530
Subject: [PATCH] Add nil check for groupSnapshotContent in
 deleteCSIGroupSnapshotOperation and unit tests

- Add a check to ensure `groupSnapshotContent` is not nil in `deleteCSIGroupSnapshotOperation` to prevent panics.
- Enhance error handling for missing `SnapshotHandle` in `VolumeSnapshotContent` objects during deletion.
- Create initial unit tests in `groupsnapshot_helper_test.go` to cover cases where `groupSnapshotContent` is nil or incomplete.

Signed-off-by: Manish <myathnal@redhat.com>
---
 .../groupsnapshot_helper.go                   |  7 +++
 .../groupsnapshot_helper_test.go              | 54 +++++++++++++++++++
 2 files changed, 61 insertions(+)
 create mode 100644 pkg/sidecar-controller/groupsnapshot_helper_test.go

diff --git a/pkg/sidecar-controller/groupsnapshot_helper.go b/pkg/sidecar-controller/groupsnapshot_helper.go
index 58c44f4ff..c0024f56b 100644
--- a/pkg/sidecar-controller/groupsnapshot_helper.go
+++ b/pkg/sidecar-controller/groupsnapshot_helper.go
@@ -238,6 +238,9 @@ func (ctrl csiSnapshotSideCarController) removeGroupSnapshotContentFinalizer(gro
 
 // Delete a groupsnapshot: Ask the backend to remove the groupsnapshot device
 func (ctrl *csiSnapshotSideCarController) deleteCSIGroupSnapshotOperation(groupSnapshotContent *crdv1alpha1.VolumeGroupSnapshotContent) error {
+	if groupSnapshotContent == nil {
+		return fmt.Errorf("groupSnapshotContent is nil")
+	}
 	klog.V(5).Infof("deleteCSIGroupSnapshotOperation [%s] started", groupSnapshotContent.Name)
 
 	snapshotterCredentials, err := ctrl.GetCredentialsFromAnnotationForGroupSnapshot(groupSnapshotContent)
@@ -253,6 +256,10 @@ func (ctrl *csiSnapshotSideCarController) deleteCSIGroupSnapshotOperation(groupS
 			if err != nil {
 				return fmt.Errorf("failed to get snapshot content %s from snapshot content store: %v", contentRef.VolumeSnapshotContentRef.Name, err)
 			}
+			if snapshotContent.Status == nil || snapshotContent.Status.SnapshotHandle == nil {
+				klog.V(5).Infof("snapshot content %s does not have snapshot handle", snapshotContent.Name)
+				continue
+			}
 			snapshotIDs = append(snapshotIDs, *snapshotContent.Status.SnapshotHandle)
 		}
 	}
diff --git a/pkg/sidecar-controller/groupsnapshot_helper_test.go b/pkg/sidecar-controller/groupsnapshot_helper_test.go
new file mode 100644
index 000000000..4eabeb1a0
--- /dev/null
+++ b/pkg/sidecar-controller/groupsnapshot_helper_test.go
@@ -0,0 +1,54 @@
+package sidecar_controller
+
+import (
+	"testing"
+
+	crdv1alpha1 "github.com/kubernetes-csi/external-snapshotter/client/v8/apis/volumegroupsnapshot/v1alpha1"
+
+	v1 "github.com/kubernetes-csi/external-snapshotter/client/v8/apis/volumesnapshot/v1"
+	core_v1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/client-go/tools/record"
+)
+
+type fakeContentLister struct {
+}
+
+func (f *fakeContentLister) List(selector labels.Selector) (ret []*v1.VolumeSnapshotContent, err error) {
+	return nil, nil
+}
+func (f *fakeContentLister) Get(name string) (*v1.VolumeSnapshotContent, error) {
+	return &v1.VolumeSnapshotContent{}, nil
+}
+
+func TestDeleteCSIGroupSnapshotOperation(t *testing.T) {
+	ctrl := &csiSnapshotSideCarController{
+		contentLister: &fakeContentLister{},
+		handler:       &csiHandler{},
+		eventRecorder: &record.FakeRecorder{},
+	}
+
+	defer func() {
+		if r := recover(); r != nil {
+			t.Errorf("deleteCSIGroupSnapshotOperation() panicked with: %v", r)
+		}
+	}()
+	err := ctrl.deleteCSIGroupSnapshotOperation(nil)
+	if err == nil {
+		t.Errorf("expected deleteCSIGroupSnapshotOperation to return error when groupsnapshotContent is nil: %v", err)
+	}
+	gsc := crdv1alpha1.VolumeGroupSnapshotContent{
+		Status: &crdv1alpha1.VolumeGroupSnapshotContentStatus{
+			PVVolumeSnapshotContentList: []crdv1alpha1.PVVolumeSnapshotContentPair{
+				{
+					PersistentVolumeRef:      core_v1.LocalObjectReference{Name: "test-pv"},
+					VolumeSnapshotContentRef: core_v1.LocalObjectReference{Name: "test-vsc"},
+				},
+			},
+		},
+	}
+	err = ctrl.deleteCSIGroupSnapshotOperation(&gsc)
+	if err == nil {
+		t.Errorf("expected deleteCSIGroupSnapshotOperation to return error when groupsnapshotContent is empty: %v", err)
+	}
+}