fix nil pointer dereference in EnsureDeletePVC, EnsureDeletePV and EnsureDeletePod timeout paths (#10293)

* fix nil pointer dereference in kube EnsureDelete timeout paths

Signed-off-by: samay43 <samayrbhat43@gmail.com>

* add changelog entry

Signed-off-by: samay43 <samayrbhat43@gmail.com>

---------

Signed-off-by: samay43 <samayrbhat43@gmail.com>

changelogs/unreleased/10293-samay43

@@ -0,0 +1 @@
+Fix nil pointer dereference in EnsureDeletePVC, EnsureDeletePV and EnsureDeletePod when the API call times out before the object is retrieved

pkg/util/kube/pod.go

@@ -129,6 +129,12 @@ func EnsureDeletePod(ctx context.Context, podGetter corev1client.CoreV1Interface
 
 	if err != nil {
 		if errors.Is(err, context.DeadlineExceeded) {
+			// updated is only set once the pod 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 pod %s is deleted", pod)
+			}
 			return errors.Errorf("timeout to assure pod %s is deleted, finalizers in pod %v", pod, updated.Finalizers)
 		} else {
 			return errors.Wrapf(err, "error to assure pod is deleted, %s", pod)

pkg/util/kube/pod_test.go

@@ -106,6 +106,29 @@ func TestEnsureDeletePod(t *testing.T) {
 			},
 			err: "timeout to assure pod fake-pod is deleted, finalizers in pod []",
 		},
+		{
+			name:      "wait timeout before the pod is ever retrieved",
+			podName:   "fake-pod",
+			namespace: "fake-ns",
+			clientObj: []runtime.Object{podObjectWithFinalizer},
+			reactors: []reactor{
+				{
+					verb:     "delete",
+					resource: "pods",
+					reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) {
+						return true, nil, nil
+					},
+				},
+				{
+					verb:     "get",
+					resource: "pods",
+					reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) {
+						return true, nil, context.DeadlineExceeded
+					},
+				},
+			},
+			err: "timeout to assure pod fake-pod is deleted",
+		},
 		{
 			name:      "wait fail",
 			podName:   "fake-pod",

pkg/util/kube/pvc_pv.go

@@ -153,6 +153,12 @@ func EnsureDeletePVC(ctx context.Context, pvcGetter corev1client.CoreV1Interface
 
 	if err != nil {
 		if errors.Is(err, context.DeadlineExceeded) {
+			// updated is only set once the PVC 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 pvc %s is deleted", pvcName)
+			}
 			return errors.Errorf("timeout to assure pvc %s is deleted, finalizers in pvc %v", pvcName, updated.Finalizers)
 		} else {
 			return errors.Wrapf(err, "error to ensure pvc deleted for %s", pvcName)
@@ -189,6 +195,12 @@ func EnsureDeletePV(ctx context.Context, pvGetter corev1client.CoreV1Interface,
 
 	if err != nil {
 		if errors.Is(err, context.DeadlineExceeded) {
+			// updated is only set once the PV 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 pv %s is deleted", pvName)
+			}
 			return errors.Errorf("timeout to assure pv %s is deleted, finalizers in pv %v", pvName, updated.Finalizers)
 		} else {
 			return errors.Wrapf(err, "error to ensure pv deleted for %s", pvName)

pkg/util/kube/pvc_pv_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package kube
 
 import (
+	"context"
 	"testing"
 	"time"
 
@@ -687,6 +688,30 @@ func TestEnsureDeletePVC(t *testing.T) {
 			},
 			err: "timeout to assure pvc fake-pvc is deleted, finalizers in pvc []",
 		},
+		{
+			name:      "wait timeout before the pvc is ever retrieved",
+			pvcName:   "fake-pvc",
+			namespace: "fake-ns",
+			clientObj: []runtime.Object{pvcObjectWithFinalizer},
+			timeout:   time.Millisecond,
+			reactors: []reactor{
+				{
+					verb:     "delete",
+					resource: "persistentvolumeclaims",
+					reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) {
+						return true, pvcObject, nil
+					},
+				},
+				{
+					verb:     "get",
+					resource: "persistentvolumeclaims",
+					reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) {
+						return true, nil, context.DeadlineExceeded
+					},
+				},
+			},
+			err: "timeout to assure pvc fake-pvc is deleted",
+		},
 	}
 
 	for _, test := range tests {
@@ -2059,6 +2084,13 @@ func TestEnsureDeletePV(t *testing.T) {
 		},
 	}
 
+	pvObjWithFinalizer := &corev1api.PersistentVolume{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:       "fake-pv",
+			Finalizers: []string{"fake-finalizer-1", "fake-finalizer-2"},
+		},
+	}
+
 	tests := []struct {
 		name          string
 		pvName        string
@@ -2134,6 +2166,29 @@ func TestEnsureDeletePV(t *testing.T) {
 			},
 			expectedErr: "timeout to assure pv fake-pv is deleted, finalizers in pv []",
 		},
+		{
+			name:          "wait timeout before the pv is ever retrieved",
+			pvName:        "fake-pv",
+			timeout:       time.Millisecond,
+			kubeClientObj: []runtime.Object{pvObjWithFinalizer},
+			kubeReactors: []reactor{
+				{
+					verb:     "delete",
+					resource: "persistentvolumes",
+					reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) {
+						return true, nil, nil
+					},
+				},
+				{
+					verb:     "get",
+					resource: "persistentvolumes",
+					reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) {
+						return true, nil, context.DeadlineExceeded
+					},
+				},
+			},
+			expectedErr: "timeout to assure pv fake-pv is deleted",
+		},
 	}
 
 	for _, test := range tests {