CLI: fix prefix matching across multiple commands (#23502)

Several commands that inspect objects where the names are user-controlled share
a bug where the user cannot inspect the object if it has a name that is an exact
prefix of the name of another object (in the same namespace, where
applicable). For example, the object "test" can't be inspected if there's an
object with the name "testing".

Copy existing logic we have for jobs, node pools, etc. to the impacted commands:

* `plugin status`
* `quota inspect`
* `quota status`
* `scaling policy info`
* `service info`
* `volume deregister`
* `volume detach`
* `volume status`

If we get multiple objects for the prefix query, we check if any of them are an
exact match and use that object instead of returning an error. Where possible
because the prefix query signatures are the same, use a generic function that
can be shared across multiple commands.

Fixes: https://github.com/hashicorp/nomad/issues/13920
Fixes: https://github.com/hashicorp/nomad/issues/17132
Fixes: https://github.com/hashicorp/nomad/issues/23236
Ref: https://hashicorp.atlassian.net/browse/NET-10054
Ref: https://hashicorp.atlassian.net/browse/NET-10055

.changelog/23502.txt

@@ -0,0 +1,19 @@
+```release-note:bug
+cli: Fixed bug where the `plugin status` command would fail if the plugin ID was a prefix of another plugin ID
+```
+
+```release-note:bug
+cli: Fixed bug where the `quota status` and `quota inspect` commands would fail if the quota name was a prefix of another quota name
+```
+
+```release-note:bug
+cli: Fixed bug where the `scaling policy info` command would fail if the policy ID was a prefix of another policy ID
+```
+
+```release-note:bug
+cli: Fixed bug where the `service info` command would fail if the service name was a prefix of another service name in the same namespace
+```
+
+```release-note:bug
+cli: Fixed bug where the `volume deregister`, `volume detach`, and `volume status` commands would fail if the volume ID was a prefix of another volume ID in the same namespace
+```

command/helpers.go

@@ -765,3 +765,40 @@ func isTty() bool {
 	_, isStdoutTerminal := term.GetFdInfo(os.Stdout)
 	return isStdinTerminal && isStdoutTerminal
 }
+
+// getByPrefix makes a prefix list query and tries to find an exact match if
+// available, or returns a list of options if multiple objects match the prefix
+// and there's no exact match
+func getByPrefix[T any](
+	objName string,
+	queryFn func(*api.QueryOptions) ([]*T, *api.QueryMeta, error),
+	prefixCompareFn func(obj *T, prefix string) bool,
+	opts *api.QueryOptions,
+) (*T, []*T, error) {
+	objs, _, err := queryFn(opts)
+	if err != nil {
+		return nil, nil, fmt.Errorf("Error querying %s: %s", objName, err)
+	}
+	switch len(objs) {
+	case 0:
+		return nil, nil, fmt.Errorf("No %s with prefix or ID %q found", objName, opts.Prefix)
+	case 1:
+		return objs[0], nil, nil
+	default:
+		// List queries often sort by by CreateIndex, not by ID, so we need to
+		// search for exact matches but account for multiple exact ID matches
+		// across namespaces
+		var match *T
+		exactMatchesCount := 0
+		for _, obj := range objs {
+			if prefixCompareFn(obj, opts.Prefix) {
+				exactMatchesCount++
+				match = obj
+			}
+		}
+		if exactMatchesCount == 1 {
+			return match, nil, nil
+		}
+		return nil, objs, nil
+	}
+}

command/helpers_test.go

@@ -5,6 +5,7 @@ package command
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -731,3 +732,73 @@ func Test_extractJobSpecEnvVars(t *testing.T) {
 		}, result)
 	})
 }
+
+// TestHelperGetByPrefix exercises the generic getByPrefix function used by
+// commands to find a single match by prefix or return matching results if there
+// are multiple
+func TestHelperGetByPrefix(t *testing.T) {
+
+	type testStub struct{ ID string }
+
+	testCases := []struct {
+		name        string
+		queryObjs   []*testStub
+		queryErr    error
+		queryPrefix string
+
+		expectMatch    *testStub
+		expectPossible []*testStub
+		expectErr      string
+	}{
+		{
+			name:      "query error",
+			queryErr:  errors.New("foo"),
+			expectErr: "Error querying stubs: foo",
+		},
+		{
+			name: "multiple prefix matches with exact match",
+			queryObjs: []*testStub{
+				{ID: "testing"},
+				{ID: "testing123"},
+			},
+			queryPrefix: "testing",
+			expectMatch: &testStub{ID: "testing"},
+		},
+		{
+			name: "multiple prefix matches no exact match",
+			queryObjs: []*testStub{
+				{ID: "testing"},
+				{ID: "testing123"},
+			},
+			queryPrefix:    "test",
+			expectPossible: []*testStub{{ID: "testing"}, {ID: "testing123"}},
+		},
+		{
+			name:        "no matches",
+			queryObjs:   []*testStub{},
+			queryPrefix: "test",
+			expectErr:   "No stubs with prefix or ID \"test\" found",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+
+			match, possible, err := getByPrefix[testStub]("stubs",
+				func(*api.QueryOptions) ([]*testStub, *api.QueryMeta, error) {
+					return tc.queryObjs, nil, tc.queryErr
+				},
+				func(stub *testStub, prefix string) bool { return stub.ID == prefix },
+				&api.QueryOptions{Prefix: tc.queryPrefix})
+
+			if tc.expectErr != "" {
+				must.EqError(t, err, tc.expectErr)
+			} else {
+				must.NoError(t, err)
+				must.Eq(t, tc.expectMatch, match, must.Sprint("expected exact match"))
+				must.Eq(t, tc.expectPossible, possible, must.Sprint("expected prefix matches"))
+			}
+		})
+	}
+
+}

command/plugin_status_csi.go

@@ -40,28 +40,28 @@ func (c *PluginStatusCommand) csiStatus(client *api.Client, id string) int {
 		return 0
 	}
 
-	// filter by plugin if a plugin ID was passed
-	plugs, _, err := client.CSIPlugins().List(&api.QueryOptions{Prefix: id})
+	// get a CSI plugin that matches the given prefix or a list of all matches if an
+	// exact match is not found.
+	pluginStub, possible, err := getByPrefix[api.CSIPluginListStub]("plugins", client.CSIPlugins().List,
+		func(plugin *api.CSIPluginListStub, prefix string) bool { return plugin.ID == prefix },
+		&api.QueryOptions{
+			Prefix:    id,
+			Namespace: c.namespace,
+		})
 	if err != nil {
-		c.Ui.Error(fmt.Sprintf("Error querying CSI plugins: %s", err))
+		c.Ui.Error(fmt.Sprintf("Error listing plugins: %s", err))
 		return 1
 	}
-	if len(plugs) == 0 {
-		c.Ui.Error(fmt.Sprintf("No plugins(s) with prefix or ID %q found", id))
-		return 1
-	}
-	if len(plugs) > 1 {
-		if id != plugs[0].ID {
-			out, err := c.csiFormatPlugins(plugs)
-			if err != nil {
-				c.Ui.Error(fmt.Sprintf("Error formatting: %s", err))
-				return 1
-			}
-			c.Ui.Error(fmt.Sprintf("Prefix matched multiple plugins\n\n%s", out))
+	if len(possible) > 0 {
+		out, err := c.csiFormatPlugins(possible)
+		if err != nil {
+			c.Ui.Error(fmt.Sprintf("Error formatting: %s", err))
 			return 1
 		}
+		c.Ui.Error(fmt.Sprintf("Prefix matched multiple plugins\n\n%s", out))
+		return 1
 	}
-	id = plugs[0].ID
+	id = pluginStub.ID
 
 	// Lookup matched a single plugin
 	plug, _, err := client.CSIPlugins().Info(id, nil)

command/quota_inspect.go

@@ -93,9 +93,8 @@ func (c *QuotaInspectCommand) Run(args []string) int {
 		return 1
 	}
 
-	// Do a prefix lookup
 	quotas := client.Quotas()
-	spec, possible, err := getQuota(quotas, name)
+	spec, possible, err := getQuotaByPrefix(quotas, name)
 	if err != nil {
 		c.Ui.Error(fmt.Sprintf("Error retrieving quota: %s", err))
 		return 1

command/quota_status.go

@@ -91,14 +91,12 @@ func (c *QuotaStatusCommand) Run(args []string) int {
 		return 1
 	}
 
-	// Do a prefix lookup
 	quotas := client.Quotas()
-	spec, possible, err := getQuota(quotas, name)
+	spec, possible, err := getQuotaByPrefix(quotas, name)
 	if err != nil {
 		c.Ui.Error(fmt.Sprintf("Error retrieving quota: %s", err))
 		return 1
 	}
-
 	if len(possible) != 0 {
 		c.Ui.Error(fmt.Sprintf("Prefix matched multiple quotas\n\n%s", formatQuotaSpecs(possible)))
 		return 1
@@ -252,20 +250,25 @@ func formatQuotaLimitInt(value *int) string {
 	return strconv.Itoa(v)
 }
 
-func getQuota(client *api.Quotas, quota string) (match *api.QuotaSpec, possible []*api.QuotaSpec, err error) {
+func getQuotaByPrefix(client *api.Quotas, quota string) (match *api.QuotaSpec, possible []*api.QuotaSpec, err error) {
 	// Do a prefix lookup
 	quotas, _, err := client.PrefixList(quota, nil)
 	if err != nil {
 		return nil, nil, err
 	}
 
-	l := len(quotas)
-	switch {
-	case l == 0:
+	switch len(quotas) {
+	case 0:
 		return nil, nil, fmt.Errorf("Quota %q matched no quotas", quota)
-	case l == 1:
+	case 1:
 		return quotas[0], nil, nil
 	default:
+		// find exact match if possible
+		for _, q := range quotas {
+			if q.Name == quota {
+				return q, nil, nil
+			}
+		}
 		return nil, quotas, nil
 	}
 }

command/scaling_policy_info.go

@@ -141,24 +141,27 @@ func (s *ScalingPolicyInfoCommand) Run(args []string) int {
 	}
 
 	policyID = sanitizeUUIDPrefix(policyID)
-	policies, _, err := client.Scaling().ListPolicies(&api.QueryOptions{
-		Prefix: policyID,
-	})
+
+	// get a policy that matches the given prefix or a list of all matches if an
+	// exact match is not found.
+	policyStub, possible, err := getByPrefix[api.ScalingPolicyListStub]("scaling policies",
+		client.Scaling().ListPolicies,
+		func(policy *api.ScalingPolicyListStub, prefix string) bool { return policy.ID == prefix },
+		&api.QueryOptions{
+			Prefix: policyID,
+		})
 	if err != nil {
 		s.Ui.Error(fmt.Sprintf("Error querying scaling policy: %v", err))
 		return 1
 	}
-	if len(policies) == 0 {
-		s.Ui.Error(fmt.Sprintf("No scaling policies with prefix or id %q found", policyID))
+	if len(possible) > 0 {
+		out := formatScalingPolicies(possible, length)
+		s.Ui.Error(fmt.Sprintf("Prefix matched multiple scaling policies\n\n%s", out))
 		return 1
 	}
-	if len(policies) > 1 {
-		out := formatScalingPolicies(policies, length)
-		s.Ui.Error(fmt.Sprintf("Prefix matched multiple scaling policies\n\n%s", out))
-		return 0
-	}
+	policyID = policyStub.ID
 
-	policy, _, err := client.Scaling().GetPolicy(policies[0].ID, nil)
+	policy, _, err := client.Scaling().GetPolicy(policyID, nil)
 	if err != nil {
 		s.Ui.Error(fmt.Sprintf("Error querying scaling policy: %s", err))
 		return 1

command/scaling_policy_info_test.go

@@ -58,8 +58,8 @@ func TestScalingPolicyInfoCommand_Run(t *testing.T) {
 	if code := cmd.Run([]string{"-address=" + url, "scaling_policy_info"}); code != 1 {
 		t.Fatalf("expected cmd run exit code 1, got: %d", code)
 	}
-	if out := ui.ErrorWriter.String(); !strings.Contains(out, `No scaling policies with prefix or id "scaling_policy_inf" found`) {
-		t.Fatalf("expected 'no policies found' within output: %v", out)
+	if out := ui.ErrorWriter.String(); !strings.Contains(out, `No scaling policies with prefix or ID "scaling_policy_inf" found`) {
+		t.Fatalf("expected 'No scaling policies with prefix' within output: %v", out)
 	}
 
 	// Generate a test job.

command/service_info.go

@@ -123,23 +123,15 @@ func (s *ServiceInfoCommand) Run(args []string) int {
 		Prefix:    serviceID,
 		Namespace: ns,
 	}
-	services, _, err := client.Services().List(&opts)
+
+	ns, serviceID, possible, err := getServiceByPrefix(client.Services(), &opts)
 	if err != nil {
 		s.Ui.Error(fmt.Sprintf("Error listing service registrations: %s", err))
 		return 1
 	}
-	switch len(services) {
-	case 0:
-		s.Ui.Error(fmt.Sprintf("No service registrations with prefix %q found", serviceID))
-		return 1
-	case 1:
-		ns = services[0].Namespace
-		if len(services[0].Services) > 0 { // should always be valid
-			serviceID = services[0].Services[0].ServiceName
-		}
-	default:
+	if len(possible) > 0 {
 		s.Ui.Error(fmt.Sprintf("Prefix matched multiple services\n\n%s",
-			formatServiceListOutput(s.Meta.namespace, services)))
+			formatServiceListOutput(s.Meta.namespace, possible)))
 		return 1
 	}
 
@@ -294,3 +286,61 @@ func argsWithNewPageToken(osArgs []string, nextToken string) string {
 	}
 	return strings.Join(newArgs, " ")
 }
+
+func getServiceByPrefix(client *api.Services, opts *api.QueryOptions) (ns, id string, possible []*api.ServiceRegistrationListStub, err error) {
+	possible, _, err = client.List(opts)
+	if err != nil {
+		return
+	}
+
+	switch len(possible) {
+	case 0:
+		err = fmt.Errorf("No service registrations with prefix %q found", opts.Prefix)
+		return
+	case 1: // single namespace
+		ns = possible[0].Namespace
+		services := possible[0].Services
+		switch len(services) {
+		case 0:
+			// should never happen because we should never get an empty stub
+			err = fmt.Errorf("No service registrations with prefix %q found", opts.Prefix)
+			return
+		case 1:
+			id = services[0].ServiceName
+			possible = nil
+			return
+		default:
+			for _, service := range services {
+				if service.ServiceName == opts.Prefix { // exact match
+					id = service.ServiceName
+					possible = nil
+					return
+				}
+			}
+			return
+		}
+	default: // multiple namespaces, so we passed '*' namespace arg
+		exactMatchesCount := 0
+		for _, stub := range possible {
+			for _, service := range stub.Services {
+				if service.ServiceName == opts.Prefix {
+					id = service.ServiceName
+					exactMatchesCount++
+					continue
+				}
+			}
+		}
+		switch exactMatchesCount {
+		case 0:
+			// should never happen because we should never get an empty stub
+			err = fmt.Errorf("No service registrations with prefix %q found", opts.Prefix)
+			return
+		case 1:
+			possible = nil
+			return
+		default:
+			return
+		}
+	}
+
+}

command/volume_deregister.go

@@ -5,7 +5,6 @@ package command
 
 import (
 	"fmt"
-	"sort"
 	"strings"
 
 	"github.com/hashicorp/nomad/api"
@@ -96,29 +95,28 @@ func (c *VolumeDeregisterCommand) Run(args []string) int {
 		return 1
 	}
 
-	// Prefix search for the volume
-	vols, _, err := client.CSIVolumes().List(&api.QueryOptions{Prefix: volID})
+	// get a CSI volume that matches the given prefix or a list of all matches if an
+	// exact match is not found.
+	volStub, possible, err := getByPrefix[api.CSIVolumeListStub]("volumes", client.CSIVolumes().List,
+		func(vol *api.CSIVolumeListStub, prefix string) bool { return vol.ID == prefix },
+		&api.QueryOptions{
+			Prefix:    volID,
+			Namespace: c.namespace,
+		})
 	if err != nil {
-		c.Ui.Error(fmt.Sprintf("Error querying volumes: %s", err))
+		c.Ui.Error(fmt.Sprintf("Error listing volumes: %s", err))
 		return 1
 	}
-	if len(vols) == 0 {
-		c.Ui.Error(fmt.Sprintf("No volumes(s) with prefix or ID %q found", volID))
-		return 1
-	}
-	if len(vols) > 1 {
-		if (volID != vols[0].ID) || (c.allNamespaces() && vols[0].ID == vols[1].ID) {
-			sort.Slice(vols, func(i, j int) bool { return vols[i].ID < vols[j].ID })
-			out, err := csiFormatSortedVolumes(vols)
-			if err != nil {
-				c.Ui.Error(fmt.Sprintf("Error formatting: %s", err))
-				return 1
-			}
-			c.Ui.Error(fmt.Sprintf("Prefix matched multiple volumes\n\n%s", out))
+	if len(possible) > 0 {
+		out, err := csiFormatVolumes(possible, false, "")
+		if err != nil {
+			c.Ui.Error(fmt.Sprintf("Error formatting: %s", err))
 			return 1
 		}
+		c.Ui.Error(fmt.Sprintf("Prefix matched multiple volumes\n\n%s", out))
+		return 1
 	}
-	volID = vols[0].ID
+	volID = volStub.ID
 
 	// Confirm the -force flag
 	if force {

command/volume_detach.go

@@ -5,7 +5,6 @@ package command
 
 import (
 	"fmt"
-	"sort"
 	"strings"
 
 	"github.com/hashicorp/nomad/api"
@@ -118,29 +117,28 @@ func (c *VolumeDetachCommand) Run(args []string) int {
 	// given.
 	if len(nodes) == 0 {
 
-		// Prefix search for the volume
-		vols, _, err := client.CSIVolumes().List(&api.QueryOptions{Prefix: volID})
+		// get a CSI volume that matches the given prefix or a list of all
+		// matches if an exact match is not found.
+		volStub, possible, err := getByPrefix[api.CSIVolumeListStub]("volumes", client.CSIVolumes().List,
+			func(vol *api.CSIVolumeListStub, prefix string) bool { return vol.ID == prefix },
+			&api.QueryOptions{
+				Prefix:    volID,
+				Namespace: c.namespace,
+			})
 		if err != nil {
-			c.Ui.Error(fmt.Sprintf("Error querying volumes: %s", err))
+			c.Ui.Error(fmt.Sprintf("Error listing volumes: %s", err))
 			return 1
 		}
-		if len(vols) == 0 {
-			c.Ui.Error(fmt.Sprintf("No volumes(s) with prefix or ID %q found", volID))
-			return 1
-		}
-		if len(vols) > 1 {
-			if (volID != vols[0].ID) || (c.allNamespaces() && vols[0].ID == vols[1].ID) {
-				sort.Slice(vols, func(i, j int) bool { return vols[i].ID < vols[j].ID })
-				out, err := csiFormatSortedVolumes(vols)
-				if err != nil {
-					c.Ui.Error(fmt.Sprintf("Error formatting: %s", err))
-					return 1
-				}
-				c.Ui.Error(fmt.Sprintf("Prefix matched multiple volumes\n\n%s", out))
+		if len(possible) > 0 {
+			out, err := csiFormatVolumes(possible, false, "")
+			if err != nil {
+				c.Ui.Error(fmt.Sprintf("Error formatting: %s", err))
 				return 1
 			}
+			c.Ui.Error(fmt.Sprintf("Prefix matched multiple volumes\n\n%s", out))
+			return 1
 		}
-		volID = vols[0].ID
+		volID = volStub.ID
 
 		vol, _, err := client.CSIVolumes().Info(volID, nil)
 		if err != nil {

command/volume_status_csi.go

@@ -26,50 +26,30 @@ func (c *VolumeStatusCommand) csiStatus(client *api.Client, id string) int {
 		return c.listVolumes(client)
 	}
 
-	// Prefix search for the volume
-	vols, _, err := client.CSIVolumes().List(&api.QueryOptions{Prefix: id})
+	// get a CSI volume that matches the given prefix or a list of all matches if an
+	// exact match is not found.
+	volStub, possible, err := getByPrefix[api.CSIVolumeListStub]("volumes", client.CSIVolumes().List,
+		func(vol *api.CSIVolumeListStub, prefix string) bool { return vol.ID == prefix },
+		&api.QueryOptions{
+			Prefix:    id,
+			Namespace: c.namespace,
+		})
 	if err != nil {
-		c.Ui.Error(fmt.Sprintf("Error querying volumes: %s", err))
+		c.Ui.Error(fmt.Sprintf("Error listing volumes: %s", err))
 		return 1
 	}
-	if len(vols) == 0 {
-		c.Ui.Error(fmt.Sprintf("No volumes(s) with prefix or ID %q found", id))
-		return 1
-	}
-
-	var ns string
-
-	if len(vols) == 1 {
-		// need to set id from the actual ID because it might be a prefix
-		id = vols[0].ID
-		ns = vols[0].Namespace
-	}
-
-	// List sorts by CreateIndex, not by ID, so we need to search for
-	// exact matches but account for multiple exact ID matches across
-	// namespaces
-	if len(vols) > 1 {
-		exactMatchesCount := 0
-		for _, vol := range vols {
-			if vol.ID == id {
-				exactMatchesCount++
-				ns = vol.Namespace
-			}
-		}
-		if exactMatchesCount != 1 {
-			out, err := c.csiFormatVolumes(vols)
-			if err != nil {
-				c.Ui.Error(fmt.Sprintf("Error formatting: %s", err))
-				return 1
-			}
-			c.Ui.Error(fmt.Sprintf("Prefix matched multiple volumes\n\n%s", out))
+	if len(possible) > 0 {
+		out, err := csiFormatVolumes(possible, c.json, c.template)
+		if err != nil {
+			c.Ui.Error(fmt.Sprintf("Error formatting: %s", err))
 			return 1
 		}
+		c.Ui.Error(fmt.Sprintf("Prefix matched multiple volumes\n\n%s", out))
+		return 1
 	}
 
 	// Try querying the volume
-	client.SetNamespace(ns)
-	vol, _, err := client.CSIVolumes().Info(id, nil)
+	vol, _, err := client.CSIVolumes().Info(volStub.ID, nil)
 	if err != nil {
 		c.Ui.Error(fmt.Sprintf("Error querying volume: %s", err))
 		return 1
@@ -98,7 +78,7 @@ func (c *VolumeStatusCommand) listVolumes(client *api.Client) int {
 		// No output if we have no volumes
 		c.Ui.Error("No CSI volumes")
 	} else {
-		str, err := c.csiFormatVolumes(vols)
+		str, err := csiFormatVolumes(vols, c.json, c.template)
 		if err != nil {
 			c.Ui.Error(fmt.Sprintf("Error formatting: %s", err))
 			return 1
@@ -170,12 +150,12 @@ NEXT_PLUGIN:
 	return code
 }
 
-func (c *VolumeStatusCommand) csiFormatVolumes(vols []*api.CSIVolumeListStub) (string, error) {
+func csiFormatVolumes(vols []*api.CSIVolumeListStub, json bool, template string) (string, error) {
 	// Sort the output by volume id
 	sort.Slice(vols, func(i, j int) bool { return vols[i].ID < vols[j].ID })
 
-	if c.json || len(c.template) > 0 {
-		out, err := Format(c.json, c.template, vols)
+	if json || len(template) > 0 {
+		out, err := Format(json, template, vols)
 		if err != nil {
 			return "", fmt.Errorf("format error: %v", err)
 		}