From f1bdc3a96ded6c35ecae91b19f9b04dd6e2fc786 Mon Sep 17 00:00:00 2001
From: Luiz Aoqui <luiz@hashicorp.com>
Date: Thu, 17 Aug 2023 09:47:40 -0400
Subject: [PATCH] csi: prevent panic on volume delete (#18234)

When a CSI volume is deleted while its plugin is not running, the
function `volAndPluginLookup` returns a `nil` plugin value resulting in a
panic in the request handler.
---
 .changelog/18234.txt       |  3 +++
 nomad/csi_endpoint.go      |  3 +++
 nomad/csi_endpoint_test.go | 54 +++++++++++++++++++++++++++-----------
 3 files changed, 45 insertions(+), 15 deletions(-)
 create mode 100644 .changelog/18234.txt

diff --git a/.changelog/18234.txt b/.changelog/18234.txt
new file mode 100644
index 000000000000..21e0824b3565
--- /dev/null
+++ b/.changelog/18234.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+csi: fixed a bug that could case a panic when deleting volumes
+```
diff --git a/nomad/csi_endpoint.go b/nomad/csi_endpoint.go
index d94b21c71bad..671d47232825 100644
--- a/nomad/csi_endpoint.go
+++ b/nomad/csi_endpoint.go
@@ -1152,6 +1152,9 @@ func (v *CSIVolume) Delete(args *structs.CSIVolumeDeleteRequest, reply *structs.
 				return err
 			}
 		}
+		if plugin == nil {
+			return fmt.Errorf("plugin %q for volume %q not found", vol.PluginID, volID)
+		}
 
 		// NOTE: deleting the volume in the external storage provider can't be
 		// made atomic with deregistration. We can't delete a volume that's
diff --git a/nomad/csi_endpoint_test.go b/nomad/csi_endpoint_test.go
index 9d106a5e2259..0a1a0d3201f3 100644
--- a/nomad/csi_endpoint_test.go
+++ b/nomad/csi_endpoint_test.go
@@ -1198,7 +1198,7 @@ func TestCSIVolumeEndpoint_Delete(t *testing.T) {
 	}
 	var resp0 structs.NodeUpdateResponse
 	err = client.RPC("Node.Register", req0, &resp0)
-	require.NoError(t, err)
+	must.NoError(t, err)
 
 	testutil.WaitForResult(func() (bool, error) {
 		nodes := srv.connectedNodes()
@@ -1233,18 +1233,27 @@ func TestCSIVolumeEndpoint_Delete(t *testing.T) {
 		}
 	}).Node
 	index++
-	require.NoError(t, state.UpsertNode(structs.MsgTypeTestSetup, index, node))
+	must.NoError(t, state.UpsertNode(structs.MsgTypeTestSetup, index, node))
 
 	volID := uuid.Generate()
-	vols := []*structs.CSIVolume{{
-		ID:        volID,
-		Namespace: structs.DefaultNamespace,
-		PluginID:  "minnie",
-		Secrets:   structs.CSISecrets{"mysecret": "secretvalue"},
-	}}
+	noPluginVolID := uuid.Generate()
+	vols := []*structs.CSIVolume{
+		{
+			ID:        volID,
+			Namespace: structs.DefaultNamespace,
+			PluginID:  "minnie",
+			Secrets:   structs.CSISecrets{"mysecret": "secretvalue"},
+		},
+		{
+			ID:        noPluginVolID,
+			Namespace: structs.DefaultNamespace,
+			PluginID:  "doesnt-exist",
+			Secrets:   structs.CSISecrets{"mysecret": "secretvalue"},
+		},
+	}
 	index++
 	err = state.UpsertCSIVolume(index, vols)
-	require.NoError(t, err)
+	must.NoError(t, err)
 
 	// Delete volumes
 
@@ -1261,7 +1270,7 @@ func TestCSIVolumeEndpoint_Delete(t *testing.T) {
 	}
 	resp1 := &structs.CSIVolumeCreateResponse{}
 	err = msgpackrpc.CallWithCodec(codec, "CSIVolume.Delete", req1, resp1)
-	require.EqualError(t, err, "volume not found: bad")
+	must.EqError(t, err, "volume not found: bad")
 
 	// Make sure the valid volume wasn't deleted
 	req2 := &structs.CSIVolumeGetRequest{
@@ -1272,19 +1281,34 @@ func TestCSIVolumeEndpoint_Delete(t *testing.T) {
 	}
 	resp2 := &structs.CSIVolumeGetResponse{}
 	err = msgpackrpc.CallWithCodec(codec, "CSIVolume.Get", req2, resp2)
-	require.NoError(t, err)
-	require.NotNil(t, resp2.Volume)
+	must.NoError(t, err)
+	must.NotNil(t, resp2.Volume)
 
 	// Fix the delete request
 	fake.NextDeleteError = nil
 	req1.VolumeIDs = []string{volID}
 	err = msgpackrpc.CallWithCodec(codec, "CSIVolume.Delete", req1, resp1)
-	require.NoError(t, err)
+	must.NoError(t, err)
 
 	// Make sure it was deregistered
 	err = msgpackrpc.CallWithCodec(codec, "CSIVolume.Get", req2, resp2)
-	require.NoError(t, err)
-	require.Nil(t, resp2.Volume)
+	must.NoError(t, err)
+	must.Nil(t, resp2.Volume)
+
+	// Create a delete request for a volume without plugin.
+	req3 := &structs.CSIVolumeDeleteRequest{
+		VolumeIDs: []string{noPluginVolID},
+		WriteRequest: structs.WriteRequest{
+			Region:    "global",
+			Namespace: ns,
+		},
+		Secrets: structs.CSISecrets{
+			"secret-key-1": "secret-val-1",
+		},
+	}
+	resp3 := &structs.CSIVolumeCreateResponse{}
+	err = msgpackrpc.CallWithCodec(codec, "CSIVolume.Delete", req3, resp3)
+	must.EqError(t, err, fmt.Sprintf(`plugin "doesnt-exist" for volume "%s" not found`, noPluginVolID))
 }
 
 func TestCSIVolumeEndpoint_ListExternal(t *testing.T) {