diff --git a/.changelog/23502.txt b/.changelog/23502.txt new file mode 100644 index 00000000000..d676cc88d37 --- /dev/null +++ b/.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 +``` diff --git a/command/helpers.go b/command/helpers.go index d89c21e71e8..fbc5fadca10 100644 --- a/command/helpers.go +++ b/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 + } +} diff --git a/command/helpers_test.go b/command/helpers_test.go index 25402b92432..5d5645b54d2 100644 --- a/command/helpers_test.go +++ b/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")) + } + }) + } + +} diff --git a/command/plugin_status_csi.go b/command/plugin_status_csi.go index 2e5231d13e7..bbc69a0857a 100644 --- a/command/plugin_status_csi.go +++ b/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) diff --git a/command/quota_inspect.go b/command/quota_inspect.go index a7a077a47f6..f6a510186b5 100644 --- a/command/quota_inspect.go +++ b/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 diff --git a/command/quota_status.go b/command/quota_status.go index a396de87217..b3f9e257c33 100644 --- a/command/quota_status.go +++ b/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 } } diff --git a/command/scaling_policy_info.go b/command/scaling_policy_info.go index 67d415ce475..d8b62251f43 100644 --- a/command/scaling_policy_info.go +++ b/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 diff --git a/command/scaling_policy_info_test.go b/command/scaling_policy_info_test.go index 7346c7834bf..2150911d212 100644 --- a/command/scaling_policy_info_test.go +++ b/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. diff --git a/command/service_info.go b/command/service_info.go index c4880dc80a4..a91126499d4 100644 --- a/command/service_info.go +++ b/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 + } + } + +} diff --git a/command/volume_deregister.go b/command/volume_deregister.go index ab625a1b108..c4c78cfcc2d 100644 --- a/command/volume_deregister.go +++ b/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)) - return 1 - } - if len(vols) == 0 { - c.Ui.Error(fmt.Sprintf("No volumes(s) with prefix or ID %q found", volID)) + c.Ui.Error(fmt.Sprintf("Error listing volumes: %s", err)) 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 { diff --git a/command/volume_detach.go b/command/volume_detach.go index a6b8b72e42c..851d804a2f6 100644 --- a/command/volume_detach.go +++ b/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 { diff --git a/command/volume_status_csi.go b/command/volume_status_csi.go index 60df37aa08c..85d302743c2 100644 --- a/command/volume_status_csi.go +++ b/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)) - return 1 - } - if len(vols) == 0 { - c.Ui.Error(fmt.Sprintf("No volumes(s) with prefix or ID %q found", id)) + c.Ui.Error(fmt.Sprintf("Error listing volumes: %s", err)) 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) }