diff --git a/changelogs/unreleased/10292-samay43 b/changelogs/unreleased/10292-samay43 new file mode 100644 index 000000000..9b688ea86 --- /dev/null +++ b/changelogs/unreleased/10292-samay43 @@ -0,0 +1 @@ +Fix nil pointer dereference in EnsureDeleteVS and EnsureDeleteVSC when the API call times out before the object is retrieved diff --git a/pkg/util/csi/volume_snapshot.go b/pkg/util/csi/volume_snapshot.go index bda9d2796..49152ffdc 100644 --- a/pkg/util/csi/volume_snapshot.go +++ b/pkg/util/csi/volume_snapshot.go @@ -187,6 +187,12 @@ func EnsureDeleteVS(ctx context.Context, snapshotClient snapshotter.SnapshotV1In if err != nil { if errors.Is(err, context.DeadlineExceeded) { + // updated is only set once the VS has been retrieved successfully, so it + // is still nil when the deadline is exceeded before that happens, e.g. + // when the first Get times out. No finalizers are available to report. + if updated == nil { + return errors.Errorf("timeout to assure VolumeSnapshot %s is deleted", vsName) + } return errors.Errorf("timeout to assure VolumeSnapshot %s is deleted, finalizers in VS %v", vsName, updated.Finalizers) } else { return errors.Wrapf(err, "error to assure VolumeSnapshot is deleted, %s", vsName) @@ -246,6 +252,12 @@ func EnsureDeleteVSC(ctx context.Context, snapshotClient snapshotter.SnapshotV1I if err != nil { if errors.Is(err, context.DeadlineExceeded) { + // updated is only set once the VSC has been retrieved successfully, so it + // is still nil when the deadline is exceeded before that happens, e.g. + // when the first Get times out. No finalizers are available to report. + if updated == nil { + return errors.Errorf("timeout to assure VolumeSnapshotContent %s is deleted", vscName) + } return errors.Errorf("timeout to assure VolumeSnapshotContent %s is deleted, finalizers in VSC %v", vscName, updated.Finalizers) } else { return errors.Wrapf(err, "error to assure VolumeSnapshotContent is deleted, %s", vscName) diff --git a/pkg/util/csi/volume_snapshot_test.go b/pkg/util/csi/volume_snapshot_test.go index 61e76302b..d25de47b1 100644 --- a/pkg/util/csi/volume_snapshot_test.go +++ b/pkg/util/csi/volume_snapshot_test.go @@ -377,6 +377,29 @@ func TestEnsureDeleteVS(t *testing.T) { }, err: "timeout to assure VolumeSnapshot fake-vs is deleted, finalizers in VS []", }, + { + name: "wait timeout before the VS is ever retrieved", + vsName: "fake-vs", + namespace: "fake-ns", + clientObj: []runtime.Object{vsObjWithFinalizer}, + reactors: []reactor{ + { + verb: "delete", + resource: "volumesnapshots", + reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, nil + }, + }, + { + verb: "get", + resource: "volumesnapshots", + reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, context.DeadlineExceeded + }, + }, + }, + err: "timeout to assure VolumeSnapshot fake-vs is deleted", + }, { name: "success", vsName: "fake-vs", @@ -488,6 +511,28 @@ func TestEnsureDeleteVSC(t *testing.T) { }, err: "timeout to assure VolumeSnapshotContent fake-vsc is deleted, finalizers in VSC []", }, + { + name: "wait timeout before the VSC is ever retrieved", + vscName: "fake-vsc", + clientObj: []runtime.Object{vscObjWithFinalizer}, + reactors: []reactor{ + { + verb: "delete", + resource: "volumesnapshotcontents", + reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, nil + }, + }, + { + verb: "get", + resource: "volumesnapshotcontents", + reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, context.DeadlineExceeded + }, + }, + }, + err: "timeout to assure VolumeSnapshotContent fake-vsc is deleted", + }, { name: "success", vscName: "fake-vsc",