Skip to content

Commit 0f21537

Browse files
authored
Merge pull request kubernetes-csi#1152 from manishym/snapshotter_panic_in_volumegroupsnapshot_processing_1123
Add nil check for groupSnapshotContent in deleteCSIGroupSnapshotOperaion and unit tests
2 parents 3f3b8b3 + a1074f5 commit 0f21537

File tree

2 files changed

+57
-0
lines changed

2 files changed

+57
-0
lines changed

pkg/sidecar-controller/groupsnapshot_helper.go

+4
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,9 @@ func (ctrl csiSnapshotSideCarController) removeGroupSnapshotContentFinalizer(gro
238238

239239
// Delete a groupsnapshot: Ask the backend to remove the groupsnapshot device
240240
func (ctrl *csiSnapshotSideCarController) deleteCSIGroupSnapshotOperation(groupSnapshotContent *crdv1beta1.VolumeGroupSnapshotContent) error {
241+
if groupSnapshotContent == nil {
242+
return fmt.Errorf("groupSnapshotContent is nil")
243+
}
241244
klog.V(5).Infof("deleteCSIGroupSnapshotOperation [%s] started", groupSnapshotContent.Name)
242245

243246
snapshotterCredentials, err := ctrl.GetCredentialsFromAnnotationForGroupSnapshot(groupSnapshotContent)
@@ -258,6 +261,7 @@ func (ctrl *csiSnapshotSideCarController) deleteCSIGroupSnapshotOperation(groupS
258261
} else if groupSnapshotContent.Spec.Source.GroupSnapshotHandles != nil {
259262
ids := groupSnapshotContent.Spec.Source.GroupSnapshotHandles.VolumeSnapshotHandles
260263
snapshotIDs = slices.Clone(ids)
264+
261265
}
262266
}
263267

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
package sidecar_controller
2+
3+
import (
4+
"testing"
5+
6+
crdv1beta1 "github.com/kubernetes-csi/external-snapshotter/client/v8/apis/volumegroupsnapshot/v1beta1"
7+
8+
v1 "github.com/kubernetes-csi/external-snapshotter/client/v8/apis/volumesnapshot/v1"
9+
"k8s.io/apimachinery/pkg/labels"
10+
"k8s.io/client-go/tools/record"
11+
)
12+
13+
type fakeContentLister struct {
14+
}
15+
16+
func (f *fakeContentLister) List(selector labels.Selector) (ret []*v1.VolumeSnapshotContent, err error) {
17+
return nil, nil
18+
}
19+
func (f *fakeContentLister) Get(name string) (*v1.VolumeSnapshotContent, error) {
20+
return &v1.VolumeSnapshotContent{}, nil
21+
}
22+
23+
func TestDeleteCSIGroupSnapshotOperation(t *testing.T) {
24+
ctrl := &csiSnapshotSideCarController{
25+
contentLister: &fakeContentLister{},
26+
handler: &csiHandler{},
27+
eventRecorder: &record.FakeRecorder{},
28+
}
29+
30+
defer func() {
31+
if r := recover(); r != nil {
32+
t.Errorf("deleteCSIGroupSnapshotOperation() panicked with: %v", r)
33+
}
34+
}()
35+
err := ctrl.deleteCSIGroupSnapshotOperation(nil)
36+
if err == nil {
37+
t.Errorf("expected deleteCSIGroupSnapshotOperation to return error when groupsnapshotContent is nil: %v", err)
38+
}
39+
gsc := crdv1beta1.VolumeGroupSnapshotContent{
40+
Status: &crdv1beta1.VolumeGroupSnapshotContentStatus{
41+
VolumeSnapshotHandlePairList: []crdv1beta1.VolumeSnapshotHandlePair{
42+
{
43+
VolumeHandle: "test-pv",
44+
SnapshotHandle: "test-vsc",
45+
},
46+
},
47+
},
48+
}
49+
err = ctrl.deleteCSIGroupSnapshotOperation(&gsc)
50+
if err == nil {
51+
t.Errorf("expected deleteCSIGroupSnapshotOperation to return error when groupsnapshotContent is empty: %v", err)
52+
}
53+
}

0 commit comments

Comments
 (0)