diff --git a/.changelog/12144.txt b/.changelog/12144.txt new file mode 100644 index 000000000000..433b4a11b744 --- /dev/null +++ b/.changelog/12144.txt @@ -0,0 +1,6 @@ +```release-note:improvement +api: CSI secrets for list and delete snapshots are now passed in HTTP headers +``` + +```release-note:improvement +cli: CSI secrets argument for `volume snapshot list` has been made consistent with `volume snapshot delete` diff --git a/api/api.go b/api/api.go index a92df23fa057..08521179b107 100644 --- a/api/api.go +++ b/api/api.go @@ -62,6 +62,9 @@ type QueryOptions struct { // Set HTTP parameters on the query. Params map[string]string + // Set HTTP headers on the query. + Headers map[string]string + // AuthToken is the secret ID of an ACL token AuthToken string @@ -101,6 +104,9 @@ type WriteOptions struct { // AuthToken is the secret ID of an ACL token AuthToken string + // Set HTTP headers on the query. + Headers map[string]string + // ctx is an optional context pass through to the underlying HTTP // request layer. Use Context() and WithContext() to manage this. ctx context.Context @@ -606,6 +612,10 @@ func (r *request) setQueryOptions(q *QueryOptions) { r.params.Set(k, v) } r.ctx = q.Context() + + for k, v := range q.Headers { + r.header.Set(k, v) + } } // durToMsec converts a duration to a millisecond specified string @@ -632,6 +642,10 @@ func (r *request) setWriteOptions(q *WriteOptions) { r.params.Set("idempotency_token", q.IdempotencyToken) } r.ctx = q.Context() + + for k, v := range q.Headers { + r.header.Set(k, v) + } } // toHTTP converts the request to an HTTP request diff --git a/api/csi.go b/api/csi.go index 120c239fde8f..9b5b016bf08e 100644 --- a/api/csi.go +++ b/api/csi.go @@ -4,6 +4,7 @@ import ( "fmt" "net/url" "sort" + "strings" "time" ) @@ -129,13 +130,37 @@ func (v *CSIVolumes) DeleteSnapshot(snap *CSISnapshot, w *WriteOptions) error { 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)) - } + w.SetHeadersFromCSISecrets(snap.Secrets) _, err := v.client.delete("/v1/volumes/snapshot?"+qp.Encode(), nil, w) return err } +// ListSnapshotsOpts lists external storage volume snapshots. +func (v *CSIVolumes) ListSnapshotsOpts(req *CSISnapshotListRequest) (*CSISnapshotListResponse, *QueryMeta, error) { + var resp *CSISnapshotListResponse + + qp := url.Values{} + if req.PluginID != "" { + qp.Set("plugin_id", req.PluginID) + } + if req.NextToken != "" { + qp.Set("next_token", req.NextToken) + } + if req.PerPage != 0 { + qp.Set("per_page", fmt.Sprint(req.PerPage)) + } + req.QueryOptions.SetHeadersFromCSISecrets(req.Secrets) + + qm, err := v.client.query("/v1/volumes/snapshot?"+qp.Encode(), &resp, &req.QueryOptions) + if err != nil { + return nil, nil, err + } + + sort.Sort(CSISnapshotSort(resp.Snapshots)) + return resp, qm, nil +} + +// DEPRECATED: will be removed in Nomad 1.4.0 // ListSnapshots lists external storage volume snapshots. func (v *CSIVolumes) ListSnapshots(pluginID string, secrets string, q *QueryOptions) (*CSISnapshotListResponse, *QueryMeta, error) { var resp *CSISnapshotListResponse @@ -150,9 +175,6 @@ func (v *CSIVolumes) ListSnapshots(pluginID string, secrets string, q *QueryOpti if q.PerPage != 0 { qp.Set("per_page", fmt.Sprint(q.PerPage)) } - if secrets != "" { - qp.Set("secrets", secrets) - } qm, err := v.client.query("/v1/volumes/snapshot?"+qp.Encode(), &resp, q) if err != nil { @@ -206,6 +228,28 @@ type CSIMountOptions struct { // API or in Nomad's logs. type CSISecrets map[string]string +func (q *QueryOptions) SetHeadersFromCSISecrets(secrets CSISecrets) { + pairs := []string{} + for k, v := range secrets { + pairs = append(pairs, fmt.Sprintf("%v=%v", k, v)) + } + if q.Headers == nil { + q.Headers = map[string]string{} + } + q.Headers["X-Nomad-CSI-Secrets"] = strings.Join(pairs, ",") +} + +func (q *WriteOptions) SetHeadersFromCSISecrets(secrets CSISecrets) { + pairs := []string{} + for k, v := range secrets { + pairs = append(pairs, fmt.Sprintf("%v=%v", k, v)) + } + if q.Headers == nil { + q.Headers = map[string]string{} + } + q.Headers["X-Nomad-CSI-Secrets"] = strings.Join(pairs, ",") +} + // CSIVolume is used for serialization, see also nomad/structs/csi.go type CSIVolume struct { ID string diff --git a/command/agent/csi_endpoint.go b/command/agent/csi_endpoint.go index 0a3bd0433781..ba4c9270e8c2 100644 --- a/command/agent/csi_endpoint.go +++ b/command/agent/csi_endpoint.go @@ -305,13 +305,9 @@ func (s *HTTPServer) csiSnapshotDelete(resp http.ResponseWriter, req *http.Reque 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] - } - } + + secrets := parseCSISecrets(req) + snap.Secrets = secrets args.Snapshots = []*structs.CSISnapshot{snap} @@ -333,19 +329,9 @@ func (s *HTTPServer) csiSnapshotList(resp http.ResponseWriter, req *http.Request query := req.URL.Query() args.PluginID = query.Get("plugin_id") - querySecrets := query["secrets"] - - // Parse comma separated secrets only when provided - if len(querySecrets) >= 1 { - secrets := strings.Split(querySecrets[0], ",") - args.Secrets = make(structs.CSISecrets) - for _, raw := range secrets { - secret := strings.Split(raw, "=") - if len(secret) == 2 { - args.Secrets[secret[0]] = secret[1] - } - } - } + + secrets := parseCSISecrets(req) + args.Secrets = secrets var out structs.CSISnapshotListResponse if err := s.agent.RPC("CSIVolume.ListSnapshots", &args, &out); err != nil { @@ -420,6 +406,28 @@ func (s *HTTPServer) CSIPluginSpecificRequest(resp http.ResponseWriter, req *htt return structsCSIPluginToApi(out.Plugin), nil } +// parseCSISecrets extracts a map of k/v pairs from the CSI secrets +// header. Silently ignores invalid secrets +func parseCSISecrets(req *http.Request) structs.CSISecrets { + secretsHeader := req.Header.Get("X-Nomad-CSI-Secrets") + if secretsHeader == "" { + return nil + } + + secrets := map[string]string{} + secretkvs := strings.Split(secretsHeader, ",") + for _, secretkv := range secretkvs { + kv := strings.Split(secretkv, "=") + if len(kv) == 2 { + secrets[kv[0]] = kv[1] + } + } + if len(secrets) == 0 { + return nil + } + return structs.CSISecrets(secrets) +} + // structsCSIPluginToApi converts CSIPlugin, setting Expected the count of known plugin // instances func structsCSIPluginToApi(plug *structs.CSIPlugin) *api.CSIPlugin { diff --git a/command/agent/csi_endpoint_test.go b/command/agent/csi_endpoint_test.go index d8de9cb88650..bbb997857099 100644 --- a/command/agent/csi_endpoint_test.go +++ b/command/agent/csi_endpoint_test.go @@ -44,6 +44,29 @@ func TestHTTP_CSIEndpointPlugin(t *testing.T) { }) } +func TestHTTP_CSIParseSecrets(t *testing.T) { + t.Parallel() + testCases := []struct { + val string + expect structs.CSISecrets + }{ + {"", nil}, + {"one", nil}, + {"one,two", nil}, + {"one,two=value_two", + structs.CSISecrets(map[string]string{"two": "value_two"})}, + {"one=value_one,one=overwrite", + structs.CSISecrets(map[string]string{"one": "overwrite"})}, + {"one=value_one,two=value_two", + structs.CSISecrets(map[string]string{"one": "value_one", "two": "value_two"})}, + } + for _, tc := range testCases { + req, _ := http.NewRequest("GET", "/v1/plugin/csi/foo", nil) + req.Header.Add("X-Nomad-CSI-Secrets", tc.val) + require.Equal(t, tc.expect, parseCSISecrets(req), tc.val) + } +} + func TestHTTP_CSIEndpointUtils(t *testing.T) { secrets := structsCSISecretsToApi(structs.CSISecrets{ "foo": "bar", diff --git a/command/volume_snapshot_delete.go b/command/volume_snapshot_delete.go index 4c11a4757b23..a2ac23c7b3cd 100644 --- a/command/volume_snapshot_delete.go +++ b/command/volume_snapshot_delete.go @@ -30,7 +30,7 @@ General Options: Snapshot Options: -secret - Secrets to pass to the plugin to create the snapshot. Accepts multiple + Secrets to pass to the plugin to delete the snapshot. Accepts multiple flags in the form -secret key=value ` diff --git a/command/volume_snapshot_list.go b/command/volume_snapshot_list.go index 2542b3d8c2a7..02c7f7ffac72 100644 --- a/command/volume_snapshot_list.go +++ b/command/volume_snapshot_list.go @@ -9,6 +9,7 @@ import ( humanize "github.com/dustin/go-humanize" "github.com/hashicorp/nomad/api" "github.com/hashicorp/nomad/api/contexts" + flaghelper "github.com/hashicorp/nomad/helper/flags" "github.com/pkg/errors" "github.com/posener/complete" ) @@ -36,7 +37,9 @@ List Options: -plugin: Display only snapshots managed by a particular plugin. By default this command will query all plugins for their snapshots. - -secrets: A set of key/value secrets to be used when listing snapshots. + -secret + Secrets to pass to the plugin to list snapshots. Accepts multiple + flags in the form -secret key=value ` return strings.TrimSpace(helpText) } @@ -70,13 +73,13 @@ func (c *VolumeSnapshotListCommand) Name() string { return "volume snapshot list func (c *VolumeSnapshotListCommand) Run(args []string) int { var pluginID string var verbose bool - var secrets string + var secretsArgs flaghelper.StringFlag flags := c.Meta.FlagSet(c.Name(), FlagSetClient) flags.Usage = func() { c.Ui.Output(c.Help()) } flags.StringVar(&pluginID, "plugin", "", "") flags.BoolVar(&verbose, "verbose", false, "") - flags.StringVar(&secrets, "secrets", "", "") + 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)) @@ -122,10 +125,22 @@ func (c *VolumeSnapshotListCommand) Run(args []string) int { pluginID = plugs[0].ID } - q := &api.QueryOptions{PerPage: 30} // TODO: tune page size + secrets := api.CSISecrets{} + for _, kv := range secretsArgs { + s := strings.Split(kv, "=") + if len(s) == 2 { + secrets[s[0]] = s[1] + } + } + + req := &api.CSISnapshotListRequest{ + PluginID: pluginID, + Secrets: secrets, + QueryOptions: api.QueryOptions{PerPage: 30}, + } for { - resp, _, err := client.CSIVolumes().ListSnapshots(pluginID, secrets, q) + resp, _, err := client.CSIVolumes().ListSnapshotsOpts(req) if err != nil && !errors.Is(err, io.EOF) { c.Ui.Error(fmt.Sprintf( "Error querying CSI external snapshots for plugin %q: %s", pluginID, err)) @@ -138,8 +153,8 @@ func (c *VolumeSnapshotListCommand) Run(args []string) int { } c.Ui.Output(csiFormatSnapshots(resp.Snapshots, verbose)) - q.NextToken = resp.NextToken - if q.NextToken == "" { + req.NextToken = resp.NextToken + if req.NextToken == "" { break } // we can't know the shape of arbitrarily-sized lists of snapshots, diff --git a/website/content/docs/commands/volume/snapshot-delete.mdx b/website/content/docs/commands/volume/snapshot-delete.mdx index 33cb6eab7a3a..d0e18ea042fa 100644 --- a/website/content/docs/commands/volume/snapshot-delete.mdx +++ b/website/content/docs/commands/volume/snapshot-delete.mdx @@ -29,6 +29,11 @@ volume` and `plugin:read` capabilities. @include 'general_options.mdx' +## Snapshot Delete Options + +- `-secret`: Secrets to pass to the plugin to delete the + snapshot. Accepts multiple flags in the form `-secret key=value` + ## Examples Delete a volume snapshot: diff --git a/website/content/docs/commands/volume/snapshot-list.mdx b/website/content/docs/commands/volume/snapshot-list.mdx index 078271db716e..bc9b86804ebf 100644 --- a/website/content/docs/commands/volume/snapshot-list.mdx +++ b/website/content/docs/commands/volume/snapshot-list.mdx @@ -27,7 +27,7 @@ Nomad. @include 'general_options.mdx' -## List Options +## Snapshot List Options - `-plugin`: Display only snapshots managed by a particular [CSI plugin][csi_plugin]. By default the `snapshot list` command will query all @@ -35,8 +35,8 @@ Nomad. there is an exact match based on the provided plugin, then that specific plugin will be queried. Otherwise, a list of matching plugins will be displayed. -- `-secrets`: A list of comma separated secret key/value pairs to be passed - to the CSI driver. +- `-secret`: Secrets to pass to the plugin to delete the + snapshot. Accepts multiple flags in the form `-secret key=value` When ACLs are enabled, this command requires a token with the `csi-list-volumes` capability for the plugin's namespace. @@ -54,7 +54,7 @@ snap-67890 vol-fedcba 50GiB 2021-01-04T15:45:00Z true List volume snapshots with two secret key/value pairs: ```shell-session -$ nomad volume snapshot list -secrets key1=value1,key2=val2 +$ nomad volume snapshot list -secret key1=value1 -secret key2=val2 Snapshot ID External ID Size Creation Time Ready? snap-12345 vol-abcdef 50GiB 2021-01-03T12:15:02Z true ``` @@ -62,4 +62,4 @@ snap-12345 vol-abcdef 50GiB 2021-01-03T12:15:02Z true [csi]: https://github.com/container-storage-interface/spec [csi_plugin]: /docs/job-specification/csi_plugin [registered]: /docs/commands/volume/register -[csi_plugins_internals]: /docs/internals/plugins/csi#csi-plugins \ No newline at end of file +[csi_plugins_internals]: /docs/internals/plugins/csi#csi-plugins