From d61c267e4e7e92a3db325cadf3bd781d37e64354 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Wed, 7 Apr 2021 09:25:09 -0400 Subject: [PATCH] CSI: deletes with API don't have request body Our API client `delete` method doesn't include a request body, but accepts an interface for the response. We were accidentally putting the request body into the response, which doesn't get picked up in unit tests because we're not reading the (always empty) response body anyways. --- api/csi.go | 14 ++++++-------- command/agent/csi_endpoint.go | 18 +++++++++++++++--- vendor/github.com/hashicorp/nomad/api/csi.go | 14 ++++++-------- 3 files changed, 27 insertions(+), 19 deletions(-) diff --git a/api/csi.go b/api/csi.go index fd7c6679c..befadd797 100644 --- a/api/csi.go +++ b/api/csi.go @@ -126,10 +126,13 @@ func (v *CSIVolumes) CreateSnapshot(snap *CSISnapshot, w *WriteOptions) (*CSISna // DeleteSnapshot deletes an external storage volume snapshot. func (v *CSIVolumes) DeleteSnapshot(snap *CSISnapshot, w *WriteOptions) error { - req := &CSISnapshotDeleteRequest{ - Snapshots: []*CSISnapshot{snap}, + qp := url.Values{} + qp.Set("snapshot_id", snap.ID) + qp.Set("plugin_id", snap.PluginID) + for k, v := range snap.Secrets { + qp.Set("secret", fmt.Sprintf("%v=%v", k, v)) } - _, err := v.client.delete("/v1/volumes/snapshot", req, w) + _, err := v.client.delete("/v1/volumes/snapshot?"+qp.Encode(), nil, w) return err } @@ -397,11 +400,6 @@ type CSISnapshotCreateResponse struct { QueryMeta } -type CSISnapshotDeleteRequest struct { - Snapshots []*CSISnapshot - WriteRequest -} - // CSISnapshotListRequest is a request to a controller plugin to list all the // snapshot known to the the storage provider. This request is paginated by // the plugin and accepts the QueryOptions.PerPage and QueryOptions.NextToken diff --git a/command/agent/csi_endpoint.go b/command/agent/csi_endpoint.go index 7adf76818..c7375ee9a 100644 --- a/command/agent/csi_endpoint.go +++ b/command/agent/csi_endpoint.go @@ -298,11 +298,23 @@ func (s *HTTPServer) csiSnapshotCreate(resp http.ResponseWriter, req *http.Reque func (s *HTTPServer) csiSnapshotDelete(resp http.ResponseWriter, req *http.Request) (interface{}, error) { args := structs.CSISnapshotDeleteRequest{} - if err := decodeBody(req, &args); err != nil { - return err, CodedError(400, err.Error()) - } s.parseWriteRequest(req, &args.WriteRequest) + snap := &structs.CSISnapshot{Secrets: structs.CSISecrets{}} + + query := req.URL.Query() + snap.PluginID = query.Get("plugin_id") + snap.ID = query.Get("snapshot_id") + secrets := query["secret"] + for _, raw := range secrets { + secret := strings.Split(raw, "=") + if len(secret) == 2 { + snap.Secrets[secret[0]] = secret[1] + } + } + + args.Snapshots = []*structs.CSISnapshot{snap} + var out structs.CSISnapshotDeleteResponse if err := s.agent.RPC("CSIVolume.DeleteSnapshot", &args, &out); err != nil { return nil, err diff --git a/vendor/github.com/hashicorp/nomad/api/csi.go b/vendor/github.com/hashicorp/nomad/api/csi.go index fd7c6679c..befadd797 100644 --- a/vendor/github.com/hashicorp/nomad/api/csi.go +++ b/vendor/github.com/hashicorp/nomad/api/csi.go @@ -126,10 +126,13 @@ func (v *CSIVolumes) CreateSnapshot(snap *CSISnapshot, w *WriteOptions) (*CSISna // DeleteSnapshot deletes an external storage volume snapshot. func (v *CSIVolumes) DeleteSnapshot(snap *CSISnapshot, w *WriteOptions) error { - req := &CSISnapshotDeleteRequest{ - Snapshots: []*CSISnapshot{snap}, + qp := url.Values{} + qp.Set("snapshot_id", snap.ID) + qp.Set("plugin_id", snap.PluginID) + for k, v := range snap.Secrets { + qp.Set("secret", fmt.Sprintf("%v=%v", k, v)) } - _, err := v.client.delete("/v1/volumes/snapshot", req, w) + _, err := v.client.delete("/v1/volumes/snapshot?"+qp.Encode(), nil, w) return err } @@ -397,11 +400,6 @@ type CSISnapshotCreateResponse struct { QueryMeta } -type CSISnapshotDeleteRequest struct { - Snapshots []*CSISnapshot - WriteRequest -} - // CSISnapshotListRequest is a request to a controller plugin to list all the // snapshot known to the the storage provider. This request is paginated by // the plugin and accepts the QueryOptions.PerPage and QueryOptions.NextToken