cli/api: Update plugin listing to always include version info in the response (#17347)

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

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,

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.
+```

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 {

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)
 				}
 			})

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...)
 	}

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