From a1074f56a73f5b56531712d171ccc7a525cd4481 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                   |  4 ++
 .../groupsnapshot_helper_test.go              | 53 +++++++++++++++++++
 2 files changed, 57 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 bfd94b435..34a6ed67c 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 *crdv1beta1.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)
@@ -258,6 +261,7 @@ func (ctrl *csiSnapshotSideCarController) deleteCSIGroupSnapshotOperation(groupS
 		} else if groupSnapshotContent.Spec.Source.GroupSnapshotHandles != nil {
 			ids := groupSnapshotContent.Spec.Source.GroupSnapshotHandles.VolumeSnapshotHandles
 			snapshotIDs = slices.Clone(ids)
+
 		}
 	}
 
diff --git a/pkg/sidecar-controller/groupsnapshot_helper_test.go b/pkg/sidecar-controller/groupsnapshot_helper_test.go
new file mode 100644
index 000000000..86af6f7dc
--- /dev/null
+++ b/pkg/sidecar-controller/groupsnapshot_helper_test.go
@@ -0,0 +1,53 @@
+package sidecar_controller
+
+import (
+	"testing"
+
+	crdv1beta1 "github.com/kubernetes-csi/external-snapshotter/client/v8/apis/volumegroupsnapshot/v1beta1"
+
+	v1 "github.com/kubernetes-csi/external-snapshotter/client/v8/apis/volumesnapshot/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 := crdv1beta1.VolumeGroupSnapshotContent{
+		Status: &crdv1beta1.VolumeGroupSnapshotContentStatus{
+			VolumeSnapshotHandlePairList: []crdv1beta1.VolumeSnapshotHandlePair{
+				{
+					VolumeHandle:   "test-pv",
+					SnapshotHandle: "test-vsc",
+				},
+			},
+		},
+	}
+	err = ctrl.deleteCSIGroupSnapshotOperation(&gsc)
+	if err == nil {
+		t.Errorf("expected deleteCSIGroupSnapshotOperation to return error when groupsnapshotContent is empty: %v", err)
+	}
+}