From 12ca81bc9b45807c5cf9d47fc8aee231f71a62e7 Mon Sep 17 00:00:00 2001 From: Tom Proctor Date: Thu, 29 Sep 2022 18:22:33 +0100 Subject: [PATCH] cli/api: Update plugin listing to always include version info in the response (#17347) --- api/sys_plugins.go | 71 ++++------ api/sys_plugins_test.go | 122 +++++++++++++++--- changelog/17347.txt | 6 + command/plugin_list.go | 30 +++-- command/plugin_list_test.go | 6 +- vault/logical_system.go | 2 +- website/content/docs/commands/plugin/list.mdx | 14 +- 7 files changed, 161 insertions(+), 90 deletions(-) create mode 100644 changelog/17347.txt diff --git a/api/sys_plugins.go b/api/sys_plugins.go index 389e66eb1..dc279a9dd 100644 --- a/api/sys_plugins.go +++ b/api/sys_plugins.go @@ -32,7 +32,7 @@ type ListPluginsResponse struct { } type PluginDetails struct { - Type string `json:"string"` + Type string `json:"type"` Name string `json:"name"` Version string `json:"version,omitempty"` Builtin bool `json:"builtin"` @@ -50,25 +50,7 @@ func (c *Sys) ListPluginsWithContext(ctx context.Context, i *ListPluginsInput) ( ctx, cancelFunc := c.c.withConfiguredTimeout(ctx) defer cancelFunc() - path := "" - method := "" - if i.Type == consts.PluginTypeUnknown { - path = "/v1/sys/plugins/catalog" - method = http.MethodGet - } else { - path = fmt.Sprintf("/v1/sys/plugins/catalog/%s", i.Type) - method = "LIST" - } - - req := c.c.NewRequest(method, path) - if method == "LIST" { - // Set this for broader compatibility, but we use LIST above to be able - // to handle the wrapping lookup function - req.Method = http.MethodGet - req.Params.Set("list", "true") - } - - resp, err := c.c.rawRequestWithContext(ctx, req) + resp, err := c.c.rawRequestWithContext(ctx, c.c.NewRequest(http.MethodGet, "/v1/sys/plugins/catalog")) if err != nil && resp == nil { return nil, err } @@ -77,27 +59,6 @@ func (c *Sys) ListPluginsWithContext(ctx context.Context, i *ListPluginsInput) ( } defer resp.Body.Close() - // We received an Unsupported Operation response from Vault, indicating - // Vault of an older version that doesn't support the GET method yet; - // switch it to a LIST. - if resp.StatusCode == 405 { - req.Params.Set("list", "true") - resp, err := c.c.rawRequestWithContext(ctx, req) - if err != nil { - return nil, err - } - defer resp.Body.Close() - var result struct { - Data struct { - Keys []string `json:"keys"` - } `json:"data"` - } - if err := resp.DecodeJSON(&result); err != nil { - return nil, err - } - return &ListPluginsResponse{Names: result.Data.Keys}, nil - } - secret, err := ParseSecret(resp.Body) if err != nil { return nil, err @@ -108,9 +69,9 @@ func (c *Sys) ListPluginsWithContext(ctx context.Context, i *ListPluginsInput) ( result := &ListPluginsResponse{ PluginsByType: make(map[consts.PluginType][]string), - Details: []PluginDetails{}, } - if i.Type == consts.PluginTypeUnknown { + switch i.Type { + case consts.PluginTypeUnknown: for _, pluginType := range consts.PluginTypes { pluginsRaw, ok := secret.Data[pluginType.String()] if !ok { @@ -132,18 +93,36 @@ func (c *Sys) ListPluginsWithContext(ctx context.Context, i *ListPluginsInput) ( } result.PluginsByType[pluginType] = plugins } - } else { + default: + pluginsRaw, ok := secret.Data[i.Type.String()] + if !ok { + return nil, fmt.Errorf("no %s entry in returned data", i.Type.String()) + } + var respKeys []string - if err := mapstructure.Decode(secret.Data["keys"], &respKeys); err != nil { + if err := mapstructure.Decode(pluginsRaw, &respKeys); err != nil { return nil, err } result.PluginsByType[i.Type] = respKeys } if detailed, ok := secret.Data["detailed"]; ok { - if err := mapstructure.Decode(detailed, &result.Details); err != nil { + var details []PluginDetails + if err := mapstructure.Decode(detailed, &details); err != nil { return nil, err } + + switch i.Type { + case consts.PluginTypeUnknown: + result.Details = details + default: + // Filter for just the queried type. + for _, entry := range details { + if entry.Type == i.Type.String() { + result.Details = append(result.Details, entry) + } + } + } } return result, nil diff --git a/api/sys_plugins_test.go b/api/sys_plugins_test.go index d4a577bac..14ba98b34 100644 --- a/api/sys_plugins_test.go +++ b/api/sys_plugins_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/hashicorp/vault/sdk/helper/consts" + "github.com/hashicorp/vault/sdk/helper/strutil" ) func TestRegisterPlugin(t *testing.T) { @@ -39,27 +40,78 @@ func TestListPlugins(t *testing.T) { t.Fatal(err) } - resp, err := client.Sys().ListPluginsWithContext(context.Background(), &ListPluginsInput{}) - if err != nil { - t.Fatal(err) - } - - expectedPlugins := map[consts.PluginType][]string{ - consts.PluginTypeCredential: {"alicloud"}, - consts.PluginTypeDatabase: {"cassandra-database-plugin"}, - consts.PluginTypeSecrets: {"ad", "alicloud"}, - } - - for pluginType, expected := range expectedPlugins { - actualPlugins := resp.PluginsByType[pluginType] - if len(expected) != len(actualPlugins) { - t.Fatal("Wrong number of plugins", expected, actualPlugins) - } - for i := range actualPlugins { - if expected[i] != actualPlugins[i] { - t.Fatalf("Expected %q but got %q", expected[i], actualPlugins[i]) + for name, tc := range map[string]struct { + input ListPluginsInput + expectedPlugins map[consts.PluginType][]string + }{ + "no type specified": { + input: ListPluginsInput{}, + expectedPlugins: map[consts.PluginType][]string{ + consts.PluginTypeCredential: {"alicloud"}, + consts.PluginTypeDatabase: {"cassandra-database-plugin"}, + consts.PluginTypeSecrets: {"ad", "alicloud"}, + }, + }, + "only auth plugins": { + input: ListPluginsInput{Type: consts.PluginTypeCredential}, + expectedPlugins: map[consts.PluginType][]string{ + consts.PluginTypeCredential: {"alicloud"}, + }, + }, + "only database plugins": { + input: ListPluginsInput{Type: consts.PluginTypeDatabase}, + expectedPlugins: map[consts.PluginType][]string{ + consts.PluginTypeDatabase: {"cassandra-database-plugin"}, + }, + }, + "only secret plugins": { + input: ListPluginsInput{Type: consts.PluginTypeSecrets}, + expectedPlugins: map[consts.PluginType][]string{ + consts.PluginTypeSecrets: {"ad", "alicloud"}, + }, + }, + } { + t.Run(name, func(t *testing.T) { + resp, err := client.Sys().ListPluginsWithContext(context.Background(), &tc.input) + if err != nil { + t.Fatal(err) } - } + + for pluginType, expected := range tc.expectedPlugins { + actualPlugins := resp.PluginsByType[pluginType] + if len(expected) != len(actualPlugins) { + t.Fatal("Wrong number of plugins", expected, actualPlugins) + } + for i := range actualPlugins { + if expected[i] != actualPlugins[i] { + t.Fatalf("Expected %q but got %q", expected[i], actualPlugins[i]) + } + } + + for _, expectedPlugin := range expected { + found := false + for _, plugin := range resp.Details { + if plugin.Type == pluginType.String() && plugin.Name == expectedPlugin { + found = true + break + } + } + if !found { + t.Errorf("Expected to find %s plugin %s but not found in details: %#v", pluginType.String(), expectedPlugin, resp.Details) + } + } + } + + for _, actual := range resp.Details { + pluginType, err := consts.ParsePluginType(actual.Type) + if err != nil { + t.Fatal(err) + } + if !strutil.StrListContains(tc.expectedPlugins[pluginType], actual.Name) { + t.Errorf("Did not expect to find %s in details", actual.Name) + } + } + }) } } @@ -90,6 +142,36 @@ const listUntypedResponse = `{ { "arbitraryData": 7 } + ], + "detailed": [ + { + "type": "auth", + "name": "alicloud", + "version": "v0.13.0+builtin", + "builtin": true, + "deprecation_status": "supported" + }, + { + "type": "database", + "name": "cassandra-database-plugin", + "version": "v1.13.0+builtin.vault", + "builtin": true, + "deprecation_status": "supported" + }, + { + "type": "secret", + "name": "ad", + "version": "v0.14.0+builtin", + "builtin": true, + "deprecation_status": "supported" + }, + { + "type": "secret", + "name": "alicloud", + "version": "v0.13.0+builtin", + "builtin": true, + "deprecation_status": "supported" + } ] }, "wrap_info": null, diff --git a/changelog/17347.txt b/changelog/17347.txt new file mode 100644 index 000000000..e74f4e3a3 --- /dev/null +++ b/changelog/17347.txt @@ -0,0 +1,6 @@ +```release-note:change +api: Exclusively use `GET /sys/plugins/catalog` endpoint for listing plugins, and add `details` field to list responses. +``` +```release-note:improvement +cli: `vault plugin list` now has a `details` field in JSON format, and version and type information in table format. +``` diff --git a/command/plugin_list.go b/command/plugin_list.go index d9651127b..641c5e2ba 100644 --- a/command/plugin_list.go +++ b/command/plugin_list.go @@ -2,7 +2,6 @@ package command import ( "fmt" - "sort" "strings" "github.com/hashicorp/vault/api" @@ -128,31 +127,34 @@ func (c *PluginListCommand) Run(args []string) int { c.UI.Output(tableOutput(c.detailedResponse(resp), nil)) return 0 } - c.UI.Output(tableOutput(c.simpleResponse(resp), nil)) + c.UI.Output(tableOutput(c.simpleResponse(resp, pluginType), nil)) return 0 default: res := make(map[string]interface{}) for k, v := range resp.PluginsByType { res[k.String()] = v } + res["details"] = resp.Details return OutputData(c.UI, res) } } -func (c *PluginListCommand) simpleResponse(plugins *api.ListPluginsResponse) []string { - var flattenedNames []string - namesAdded := make(map[string]bool) - for _, names := range plugins.PluginsByType { - for _, name := range names { - if ok := namesAdded[name]; !ok { - flattenedNames = append(flattenedNames, name) - namesAdded[name] = true - } +func (c *PluginListCommand) simpleResponse(plugins *api.ListPluginsResponse, pluginType consts.PluginType) []string { + var out []string + switch pluginType { + case consts.PluginTypeUnknown: + out = []string{"Name | Type | Version"} + for _, plugin := range plugins.Details { + out = append(out, fmt.Sprintf("%s | %s | %s", plugin.Name, plugin.Type, plugin.Version)) + } + default: + out = []string{"Name | Version"} + for _, plugin := range plugins.Details { + out = append(out, fmt.Sprintf("%s | %s", plugin.Name, plugin.Version)) } - sort.Strings(flattenedNames) } - list := append([]string{"Plugins"}, flattenedNames...) - return list + + return out } func (c *PluginListCommand) detailedResponse(plugins *api.ListPluginsResponse) []string { diff --git a/command/plugin_list_test.go b/command/plugin_list_test.go index ef0788c69..8e4bbbff8 100644 --- a/command/plugin_list_test.go +++ b/command/plugin_list_test.go @@ -1,6 +1,7 @@ package command import ( + "regexp" "strings" "testing" @@ -36,7 +37,7 @@ func TestPluginListCommand_Run(t *testing.T) { { "lists", nil, - "Plugins", + "Name\\s+Type\\s+Version", 0, }, } @@ -62,7 +63,8 @@ func TestPluginListCommand_Run(t *testing.T) { } combined := ui.OutputWriter.String() + ui.ErrorWriter.String() - if !strings.Contains(combined, tc.out) { + matcher := regexp.MustCompile(tc.out) + if !matcher.MatchString(combined) { t.Errorf("expected %q to contain %q", combined, tc.out) } }) diff --git a/vault/logical_system.go b/vault/logical_system.go index 539d2321e..f7ec198de 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -426,7 +426,7 @@ func (b *SystemBackend) handlePluginCatalogUntypedList(ctx context.Context, _ *l } // Sort for consistent ordering - sortVersionedPlugins(versionedPlugins) + sortVersionedPlugins(versioned) versionedPlugins = append(versionedPlugins, versioned...) } diff --git a/website/content/docs/commands/plugin/list.mdx b/website/content/docs/commands/plugin/list.mdx index df9e41960..45d77325c 100644 --- a/website/content/docs/commands/plugin/list.mdx +++ b/website/content/docs/commands/plugin/list.mdx @@ -21,16 +21,15 @@ List all available plugins in the catalog. ```shell-session $ vault plugin list - -Plugins -------- -my-custom-plugin +Name Type Version +---- ---- ------- +alicloud auth v0.13.0+builtin # ... $ vault plugin list database -Plugins -------- -cassandra-database-plugin +Name Version +---- ------- +cassandra-database-plugin v1.13.0+builtin.vault # ... ``` @@ -44,6 +43,7 @@ alicloud auth v0.12.0+builtin app-id auth v1.12.0+builtin.vault pending removal # ... ``` + ## Usage The following flags are available in addition to the [standard set of