CSI: Add secrets flag support for delete volume (#11245)

.changelog/11245.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+csi: add flag for providing secrets as a set of key/value pairs to delete a volume
+```

api/csi.go

@@ -99,6 +99,7 @@ func (v *CSIVolumes) Create(vol *CSIVolume, w *WriteOptions) ([]*CSIVolume, *Wri
 	return resp.Volumes, meta, err
 }
 
+// DEPRECATED: will be removed in Nomad 1.4.0
 // Delete deletes a CSI volume from an external storage provider. The ID
 // passed as an argument here is for the storage provider's ID, so a volume
 // that's already been deregistered can be deleted.
@@ -107,6 +108,19 @@ func (v *CSIVolumes) Delete(externalVolID string, w *WriteOptions) error {
 	return err
 }
 
+// DeleteOpts deletes a CSI volume from an external storage
+// provider. The ID passed in the request is for the storage
+// provider's ID, so a volume that's already been deregistered can be
+// deleted.
+func (v *CSIVolumes) DeleteOpts(req *CSIVolumeDeleteRequest, w *WriteOptions) error {
+	if w == nil {
+		w = &WriteOptions{}
+	}
+	w.SetHeadersFromCSISecrets(req.Secrets)
+	_, err := v.client.delete(fmt.Sprintf("/v1/volume/csi/%v/delete", url.PathEscape(req.ExternalVolumeID)), nil, w)
+	return err
+}
+
 // Detach causes Nomad to attempt to detach a CSI volume from a client
 // node. This is used in the case that the node is temporarily lost and the
 // allocations are unable to drop their claims automatically.
@@ -422,6 +436,12 @@ type CSIVolumeDeregisterRequest struct {
 	WriteRequest
 }
 
+type CSIVolumeDeleteRequest struct {
+	ExternalVolumeID string
+	Secrets          CSISecrets
+	WriteRequest
+}
+
 // CSISnapshot is the storage provider's view of a volume snapshot
 type CSISnapshot struct {
 	ID                     string // storage provider's ID

command/agent/csi_endpoint.go

@@ -223,8 +223,10 @@ func (s *HTTPServer) csiVolumeDelete(id string, resp http.ResponseWriter, req *h
 		return nil, CodedError(405, ErrInvalidMethod)
 	}
 
+	secrets := parseCSISecrets(req)
 	args := structs.CSIVolumeDeleteRequest{
 		VolumeIDs: []string{id},
+		Secrets:   secrets,
 	}
 	s.parseWriteRequest(req, &args.WriteRequest)
 
@@ -329,10 +331,8 @@ func (s *HTTPServer) csiSnapshotList(resp http.ResponseWriter, req *http.Request
 
 	query := req.URL.Query()
 	args.PluginID = query.Get("plugin_id")
-
 	secrets := parseCSISecrets(req)
 	args.Secrets = secrets
-
 	var out structs.CSISnapshotListResponse
 	if err := s.agent.RPC("CSIVolume.ListSnapshots", &args, &out); err != nil {
 		return nil, err

command/volume_delete.go

@@ -4,12 +4,15 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/hashicorp/nomad/api"
 	"github.com/hashicorp/nomad/api/contexts"
+	flaghelper "github.com/hashicorp/nomad/helper/flags"
 	"github.com/posener/complete"
 )
 
 type VolumeDeleteCommand struct {
 	Meta
+	Secrets string
 }
 
 func (c *VolumeDeleteCommand) Help() string {
@@ -30,6 +33,11 @@ General Options:
 
   ` + generalOptionsUsage(usageOptsDefault) + `
 
+Delete Options:
+
+  -secret
+    Secrets to pass to the plugin to delete the snapshot. Accepts multiple
+    flags in the form -secret key=value
 `
 	return strings.TrimSpace(helpText)
 }
@@ -68,8 +76,10 @@ func (c *VolumeDeleteCommand) Synopsis() string {
 func (c *VolumeDeleteCommand) Name() string { return "volume delete" }
 
 func (c *VolumeDeleteCommand) Run(args []string) int {
+	var secretsArgs flaghelper.StringFlag
 	flags := c.Meta.FlagSet(c.Name(), FlagSetClient)
 	flags.Usage = func() { c.Ui.Output(c.Help()) }
+	flags.Var(&secretsArgs, "secret", "secrets for snapshot, ex. -secret key=value")
 
 	if err := flags.Parse(args); err != nil {
 		c.Ui.Error(fmt.Sprintf("Error parsing arguments %s", err))
@@ -78,8 +88,8 @@ func (c *VolumeDeleteCommand) Run(args []string) int {
 
 	// Check that we get exactly two arguments
 	args = flags.Args()
-	if l := len(args); l != 1 {
-		c.Ui.Error("This command takes one argument: <vol id>")
+	if l := len(args); l < 1 {
+		c.Ui.Error("This command takes at least one argument: <vol id>")
 		c.Ui.Error(commandErrorText(c))
 		return 1
 	}
@@ -92,7 +102,21 @@ func (c *VolumeDeleteCommand) Run(args []string) int {
 		return 1
 	}
 
-	err = client.CSIVolumes().Delete(volID, nil)
+	secrets := api.CSISecrets{}
+	for _, kv := range secretsArgs {
+		s := strings.Split(kv, "=")
+		if len(s) == 2 {
+			secrets[s[0]] = s[1]
+		} else {
+			c.Ui.Error("Secret must be in the format: -secret key=value")
+			return 1
+		}
+	}
+
+	err = client.CSIVolumes().DeleteOpts(&api.CSIVolumeDeleteRequest{
+		ExternalVolumeID: volID,
+		Secrets:          secrets,
+	}, nil)
 	if err != nil {
 		c.Ui.Error(fmt.Sprintf("Error deleting volume: %s", err))
 		return 1

nomad/csi_endpoint.go

@@ -1048,7 +1048,7 @@ func (v *CSIVolume) Delete(args *structs.CSIVolumeDeleteRequest, reply *structs.
 		// NOTE: deleting the volume in the external storage provider can't be
 		// made atomic with deregistration. We can't delete a volume that's
 		// not registered because we need to be able to lookup its plugin.
-		err = v.deleteVolume(vol, plugin)
+		err = v.deleteVolume(vol, plugin, args.Secrets)
 		if err != nil {
 			return err
 		}
@@ -1072,12 +1072,18 @@ func (v *CSIVolume) Delete(args *structs.CSIVolumeDeleteRequest, reply *structs.
 	return nil
 }
 
-func (v *CSIVolume) deleteVolume(vol *structs.CSIVolume, plugin *structs.CSIPlugin) error {
+func (v *CSIVolume) deleteVolume(vol *structs.CSIVolume, plugin *structs.CSIPlugin, querySecrets structs.CSISecrets) error {
+	// Combine volume and query secrets into one map.
+	// Query secrets override any secrets stored with the volume.
+	combinedSecrets := vol.Secrets
+	for k, v := range querySecrets {
+		combinedSecrets[k] = v
+	}
 
 	method := "ClientCSI.ControllerDeleteVolume"
 	cReq := &cstructs.ClientCSIControllerDeleteVolumeRequest{
 		ExternalVolumeID: vol.ExternalID,
-		Secrets:          vol.Secrets,
+		Secrets:          combinedSecrets,
 	}
 	cReq.PluginID = plugin.ID
 	cResp := &cstructs.ClientCSIControllerDeleteVolumeResponse{}

nomad/csi_endpoint_test.go

@@ -1183,6 +1183,9 @@ func TestCSIVolumeEndpoint_Delete(t *testing.T) {
 			Region:    "global",
 			Namespace: ns,
 		},
+		Secrets: structs.CSISecrets{
+			"secret-key-1": "secret-val-1",
+		},
 	}
 	resp1 := &structs.CSIVolumeCreateResponse{}
 	err = msgpackrpc.CallWithCodec(codec, "CSIVolume.Delete", req1, resp1)

nomad/structs/csi.go

@@ -879,6 +879,7 @@ type CSIVolumeCreateResponse struct {
 
 type CSIVolumeDeleteRequest struct {
 	VolumeIDs []string
+	Secrets   CSISecrets
 	WriteRequest
 }
 

website/content/api-docs/volumes.mdx

@@ -468,22 +468,26 @@ The table below shows this endpoint's support for
 | ---------------- | ---------------------------- |
 | `NO`             | `namespace:csi-write-volume` |
 
+This endpoint accepts a `X-Nomad-CSI-Secrets` header to set secrets
+for deleting the volume as comma-separated key-value pairs (see the
+example below). These secrets will be merged with any secrets already
+stored when the CSI volume was created.
+
 ### Parameters
 
 - `:volume_id` `(string: <required>)` - Specifies the ID of the
   volume. This must be the full ID. This is specified as part of the
   path.
 
-
 ### Sample Request
 
 ```shell-session
 $ curl \
     --request DELETE \
+    -H "X-Nomad-CSI-Secrets: secret-key-1=value-1,secret-key-2=value-2" \
     https://localhost:4646/v1/volume/csi/volume-id1/delete
 ```
 
-
 ## Detach Volume
 
 This endpoint detaches an external volume from a Nomad client node. It is an
@@ -676,6 +680,11 @@ The table below shows this endpoint's support for
 | ---------------- | ---------------------------- |
 | `NO`             | `namespace:csi-write-volume` |
 
+This endpoint accepts a `X-Nomad-CSI-Secrets` header to set secrets
+for deleting the snapshot as comma-separated key-value pairs (see the
+example below). These secrets will be merged with any secrets already
+stored when the CSI snapshot was created.
+
 ### Parameters
 
 - `plugin_id` `(string: <required>)` - Specifies the prefix of a CSI plugin ID
@@ -691,6 +700,7 @@ The table below shows this endpoint's support for
 ```shell-session
 $ curl \
     --request DELETE \
+    -H "X-Nomad-CSI-Secrets: secret-key-1=value-1,secret-key-2=value-2" \
     https://localhost:4646/v1/volumes/snapshot
 ```
 
@@ -713,6 +723,11 @@ The table below shows this endpoint's support for
 | ---------------- | --------------------------- |
 | `YES`            | `namespace:csi-list-volume` |
 
+This endpoint accepts a `X-Nomad-CSI-Secrets` header to set secrets
+for deleting the snapshot as comma-separated key-value pairs (see the
+example below). These secrets will be merged with any secrets already
+stored when the CSI snapshot was created.
+
 ### Parameters
 
 - `plugin_id` `(string: <required>)` - Specifies the prefix of a CSI plugin ID
@@ -728,15 +743,12 @@ The table below shows this endpoint's support for
   return for this request. The response will include a `NextToken` field that
   can be passed to the next request to fetch additional pages.
 
-- `secrets` `(string: "")` - Specifies a list of key/value secrets for listing snapshots.
-  These key/value pairs are comma-separated and are passed directly to the CSI plugin.
-
 ### Sample Request
 
 ```shell-session
 $ curl \
-    https://localhost:4646/v1/volumes/snapshot?plugin_id=plugin-id1&per_page=2& \
-    secrets=secret-key-1=secret-value-1,secret-key-2=secret-value-2
+    -H "X-Nomad-CSI-Secrets: secret-key-1=value-1,secret-key-2=value-2" \
+    https://localhost:4646/v1/volumes/snapshot?plugin_id=plugin-id1&per_page=2
 ```
 
 ### Sample Response

website/content/docs/commands/volume/delete.mdx

@@ -36,3 +36,8 @@ When ACLs are enabled, this command requires a token with the
 [csi_plugins_internals]: /docs/internals/plugins/csi#csi-plugins
 [deregistered]: /docs/commands/volume/deregister
 [registered]: /docs/commands/volume/register
+
+## Delete Options
+
+- `-secret`: Secrets to pass to the plugin to delete the
+  snapshot. Accepts multiple flags in the form `-secret key=value`