CLI: fix prefix matching across multiple commands

.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

@@ -32,10 +32,10 @@ General Options:
   ` + generalOptionsUsage(usageOptsDefault) + `
 
 Status Specific Options:
-	
+
   -json
     Output the latest quota status information in a JSON format.
-	
+
   -t
     Format and display quota status information using a Go template.
 `
@@ -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))
-		return 1
-	}
-	if len(policies) > 1 {
-		out := formatScalingPolicies(policies, length)
+	if len(possible) > 0 {
+		out := formatScalingPolicies(possible, length)
 		s.Ui.Error(fmt.Sprintf("Prefix matched multiple scaling policies\n\n%s", out))
-		return 0
+		return 1
 	}
+	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
+		}
+	}
+
+}