From 7f1a993122a144ff2b82531d2ece6c3ea242393f Mon Sep 17 00:00:00 2001 From: Emilia Grant Date: Thu, 22 Jan 2026 12:17:49 -0500 Subject: [PATCH] feat(clientcache): Add sort parameters to CLI search command (#6362, #6366) Co-authored-by: Sepehr --- internal/clientcache/cmd/search/search.go | 63 +++++++++- .../clientcache/cmd/search/search_test.go | 116 ++++++++++++++---- .../internal/daemon/search_handler.go | 15 ++- 3 files changed, 163 insertions(+), 31 deletions(-) diff --git a/internal/clientcache/cmd/search/search.go b/internal/clientcache/cmd/search/search.go index e159b6f95..c3cf65b8d 100644 --- a/internal/clientcache/cmd/search/search.go +++ b/internal/clientcache/cmd/search/search.go @@ -10,6 +10,7 @@ import ( "math" "net/url" "os" + "slices" "strings" "time" @@ -19,12 +20,12 @@ import ( "github.com/hashicorp/boundary/api/sessions" "github.com/hashicorp/boundary/api/targets" cachecmd "github.com/hashicorp/boundary/internal/clientcache/cmd/cache" + "github.com/hashicorp/boundary/internal/clientcache/internal/cache" "github.com/hashicorp/boundary/internal/clientcache/internal/client" "github.com/hashicorp/boundary/internal/clientcache/internal/daemon" "github.com/hashicorp/boundary/internal/cmd/base" "github.com/mitchellh/cli" "github.com/posener/complete" - "golang.org/x/exp/slices" ) var ( @@ -39,6 +40,7 @@ var ( } errCacheNotRunning = stderrors.New("The cache process is not running.") + sortDirections = []string{"asc", "desc"} ) type SearchCommand struct { @@ -47,6 +49,8 @@ type SearchCommand struct { flagResource string flagForceRefresh bool flagMaxResultSetSize int64 + flagSortBy string + flagSortDirection string } func (c *SearchCommand) Synopsis() string { @@ -128,6 +132,18 @@ func (c *SearchCommand) Flags() *base.FlagSets { Usage: `Forces a refresh to be attempted prior to performing the search`, Hidden: true, }) + f.StringVar(&base.StringVar{ + Name: "sort-by", + Target: &c.flagSortBy, + Usage: `Specifies which column to sort resources by. Targets may be sorted by 'name'. Sessions may be sorted by 'created_time'. Use sort-direction to control which direction to sort results by.`, + Completion: complete.PredictSet(getSortableResources()...), + }) + f.StringVar(&base.StringVar{ + Name: "sort-direction", + Target: &c.flagSortDirection, + Usage: `Specifies which direction, 'asc' or 'desc', to sort results by. Requires sort-by and defaults to 'asc'.`, + Completion: complete.PredictSet(sortDirections...), + }) return set } @@ -163,10 +179,33 @@ func (c *SearchCommand) Run(args []string) int { c.PrintCliError(stderrors.New("Max result set size must be greater than or equal to -1")) return base.CommandUserError case c.flagMaxResultSetSize > math.MaxInt: - c.PrintCliError(stderrors.New(fmt.Sprintf("Max result set size must be less than or equal to the %v", math.MaxInt))) + c.PrintCliError(fmt.Errorf("Max result set size must be less than or equal to the %v", math.MaxInt)) return base.CommandUserError } + if c.flagSortDirection != "" { + if c.flagSortBy == "" { + c.PrintCliError(stderrors.New("sort-direction requires sort-by")) + return base.CommandUserError + } + if c.flagSortDirection != "asc" && c.flagSortDirection != "desc" { + c.PrintCliError(stderrors.New("sort-direction must be one of 'asc' or 'desc'")) + return base.CommandUserError + } + } + + if c.flagSortBy != "" { + sortableBy, ok := daemon.SortableColumnsForResource[cache.ToSearchableResource(c.flagResource)] + if !ok { + c.PrintCliError(fmt.Errorf("resource %q is not sortable", c.flagResource)) + return base.CommandUserError + } + if !slices.Contains(sortableBy, cache.SortBy(c.flagSortBy)) { + c.PrintCliError(fmt.Errorf("resource %q is not sortable by %q. %q is sortable by %v", c.flagResource, c.flagSortBy, c.flagResource, sortableBy)) + return base.CommandUserError + } + } + resp, result, apiErr, err := c.Search(ctx) if err != nil { c.PrintCliError(err) @@ -238,10 +277,10 @@ func (c *SearchCommand) Search(ctx context.Context) (*api.Response, *daemon.Sear if err != nil { return nil, nil, nil, err } - return search(ctx, dotPath, tf, opts...) + return search(ctx, dotPath, tf, c.flagSortBy, c.flagSortDirection, opts...) } -func search(ctx context.Context, daemonPath string, fb filterBy, opt ...client.Option) (*api.Response, *daemon.SearchResult, *api.Error, error) { +func search(ctx context.Context, daemonPath string, fb filterBy, sortBy string, sortDirection string, opt ...client.Option) (*api.Response, *daemon.SearchResult, *api.Error, error) { addr := daemon.SocketAddress(daemonPath) _, err := os.Stat(addr.Path) if addr.Scheme == "unix" && err != nil { @@ -263,6 +302,14 @@ func search(ctx context.Context, daemonPath string, fb filterBy, opt ...client.O if fb.maxResultSetSize != 0 { q.Add("max_result_set_size", fmt.Sprintf("%d", fb.maxResultSetSize)) } + + if sortBy != "" { + q.Add(daemon.SortByKey, sortBy) + if sortDirection != "" { + q.Add(daemon.SortDirectionKey, sortDirection) + } + } + resp, err := c.Get(ctx, "/v1/search", q, opt...) if err != nil { return nil, nil, nil, fmt.Errorf("Error when sending request to the cache: %w.", err) @@ -491,3 +538,11 @@ type filterBy struct { forceRefresh bool maxResultSetSize int } + +func getSortableResources() []string { + ret := []string{} + for sr := range daemon.SortableColumnsForResource { + ret = append(ret, string(sr)) + } + return ret +} diff --git a/internal/clientcache/cmd/search/search_test.go b/internal/clientcache/cmd/search/search_test.go index 4e0a7fcde..68577e645 100644 --- a/internal/clientcache/cmd/search/search_test.go +++ b/internal/clientcache/cmd/search/search_test.go @@ -128,7 +128,7 @@ func TestSearch(t *testing.T) { for _, tc := range errorCases { t.Run(tc.name, func(t *testing.T) { - resp, r, apiErr, err := search(ctx, srv.BaseDotDir(), tc.fb) + resp, r, apiErr, err := search(ctx, srv.BaseDotDir(), tc.fb, "", "") require.NoError(t, err) assert.NotNil(t, apiErr) assert.Contains(t, apiErr.Message, tc.apiErrContains) @@ -141,7 +141,7 @@ func TestSearch(t *testing.T) { resp, r, apiErr, err := search(ctx, srv.BaseDotDir(), filterBy{ authTokenId: at.Id, resource: "targets", - }) + }, "", "") require.NoError(t, err) assert.NoError(t, err) assert.Nil(t, apiErr) @@ -157,7 +157,7 @@ func TestSearch(t *testing.T) { authTokenId: at.Id, flagQuery: "name='name'", resource: "targets", - }) + }, "", "") require.NoError(t, err) assert.NoError(t, err) assert.Nil(t, apiErr) @@ -173,7 +173,7 @@ func TestSearch(t *testing.T) { resp, r, apiErr, err := search(ctx, srv.BaseDotDir(), filterBy{ authTokenId: unsupportedAt.Id, resource: "targets", - }) + }, "", "") assert.NoError(t, err) require.NotNil(t, apiErr) assert.NotNil(t, resp) @@ -182,22 +182,27 @@ func TestSearch(t *testing.T) { assert.Contains(t, apiErr.Message, "doesn't support search") }) - srv.AddResources(t, at, []*aliases.Alias{ + expectedAliases := []*aliases.Alias{ {Id: "alt_1234567890", Value: "value1", DestinationId: "ttcp_1234567890"}, {Id: "alt_0987654321", Name: "value2", DestinationId: "ttcp_0987654321"}, - }, []*targets.Target{ + } + // These need to be sorted by name to preserve the sorting tests + expectedTargets := []*targets.Target{ {Id: "ttcp_1234567890", Name: "name1", Description: "description1"}, {Id: "ttcp_0987654321", Name: "name2", Description: "description2"}, - }, []*sessions.Session{ - {Id: "sess_1234567890", TargetId: "ttcp_1234567890", Status: "pending"}, - {Id: "sess_0987654321", TargetId: "ttcp_0987654321", Status: "pending"}, - }, boundaryTokenReaderFn) + } + expectedSessions := []*sessions.Session{ + {Id: "sess_0987654321", TargetId: "ttcp_0987654321", Status: "pending", CreatedTime: time.Now()}, + {Id: "sess_1234567890", TargetId: "ttcp_1234567890", Status: "pending", CreatedTime: time.Now().Add(-100 * time.Second)}, + } + + srv.AddResources(t, at, expectedAliases, expectedTargets, expectedSessions, boundaryTokenReaderFn) t.Run("target response from list", func(t *testing.T) { resp, r, apiErr, err := search(ctx, srv.BaseDotDir(), filterBy{ authTokenId: at.Id, resource: "targets", - }) + }, "", "") require.NoError(t, err) assert.Nil(t, apiErr) assert.NotNil(t, resp) @@ -208,7 +213,7 @@ func TestSearch(t *testing.T) { resp, r, apiErr, err := search(ctx, srv.BaseDotDir(), filterBy{ authTokenId: at.Id, resource: "targets", - }) + }, "", "") require.NoError(t, err) assert.NoError(t, err) assert.Nil(t, apiErr) @@ -221,7 +226,7 @@ func TestSearch(t *testing.T) { authTokenId: at.Id, flagQuery: "id % 'ttcp'", resource: "targets", - }) + }, "", "") require.NoError(t, err) assert.NoError(t, err) assert.Nil(t, apiErr) @@ -234,7 +239,7 @@ func TestSearch(t *testing.T) { authTokenId: at.Id, flagQuery: "id % 'ttcp_1234567890'", resource: "targets", - }) + }, "", "") require.NoError(t, err) assert.NoError(t, err) assert.Nil(t, apiErr) @@ -249,7 +254,7 @@ func TestSearch(t *testing.T) { authTokenId: at.Id, flagFilter: `"/item/id" matches "ttcp"`, resource: "targets", - }) + }, "", "") require.NoError(t, err) assert.NoError(t, err) assert.Nil(t, apiErr) @@ -262,7 +267,7 @@ func TestSearch(t *testing.T) { authTokenId: at.Id, flagFilter: `"/item/id" matches "ttcp_1234567890"`, resource: "targets", - }) + }, "", "") require.NoError(t, err) assert.NoError(t, err) assert.Nil(t, apiErr) @@ -276,7 +281,7 @@ func TestSearch(t *testing.T) { resp, r, apiErr, err := search(ctx, srv.BaseDotDir(), filterBy{ authTokenId: at.Id, resource: "sessions", - }) + }, "", "") require.NoError(t, err) assert.NoError(t, err) assert.Nil(t, apiErr) @@ -290,7 +295,7 @@ func TestSearch(t *testing.T) { authTokenId: at.Id, flagQuery: "id % 'sess'", resource: "sessions", - }) + }, "", "") require.NoError(t, err) assert.NoError(t, err) assert.Nil(t, apiErr) @@ -303,7 +308,7 @@ func TestSearch(t *testing.T) { authTokenId: at.Id, flagQuery: "id % 'sess_1234567890'", resource: "sessions", - }) + }, "", "") require.NoError(t, err) assert.NoError(t, err) assert.Nil(t, apiErr) @@ -316,7 +321,7 @@ func TestSearch(t *testing.T) { authTokenId: at.Id, flagFilter: `"/item/id" matches "sess"`, resource: "sessions", - }) + }, "", "") require.NoError(t, err) assert.NoError(t, err) assert.Nil(t, apiErr) @@ -329,7 +334,7 @@ func TestSearch(t *testing.T) { authTokenId: at.Id, flagFilter: `"/item/id" matches "sess_1234567890"`, resource: "sessions", - }) + }, "", "") require.NoError(t, err) assert.NoError(t, err) assert.Nil(t, apiErr) @@ -337,4 +342,73 @@ func TestSearch(t *testing.T) { assert.NotNil(t, r) assert.Len(t, r.Sessions, 1) }) + + t.Run("sorted targets from list", func(t *testing.T) { + resp, r, apiErr, err := search(ctx, srv.BaseDotDir(), filterBy{ + authTokenId: at.Id, + resource: "targets", + }, "name", "desc") + require.NoError(t, err) + assert.NoError(t, err) + assert.Nil(t, apiErr) + assert.NotNil(t, resp) + assert.NotNil(t, r) + assert.Equal(t, expectedTargets[1].Name, r.Targets[0].Name) + assert.Equal(t, expectedTargets[0].Name, r.Targets[1].Name) + }) + + t.Run("sorted targets ascending by default", func(t *testing.T) { + resp, r, apiErr, err := search(ctx, srv.BaseDotDir(), filterBy{ + authTokenId: at.Id, + resource: "targets", + }, "name", "") + require.NoError(t, err) + assert.NoError(t, err) + assert.Nil(t, apiErr) + assert.NotNil(t, resp) + assert.NotNil(t, r) + assert.Equal(t, expectedTargets[0].Name, r.Targets[0].Name) + assert.Equal(t, expectedTargets[1].Name, r.Targets[1].Name) + }) + + t.Run("sorted sessions from list", func(t *testing.T) { + resp, r, apiErr, err := search(ctx, srv.BaseDotDir(), filterBy{ + authTokenId: at.Id, + resource: "sessions", + }, "created_time", "desc") + require.NoError(t, err) + assert.NoError(t, err) + assert.Nil(t, apiErr) + assert.NotNil(t, resp) + assert.NotNil(t, r) + assert.Less(t, r.Sessions[1].CreatedTime, r.Sessions[0].CreatedTime) + }) + + t.Run("sorted sessions ascending by default", func(t *testing.T) { + resp, r, apiErr, err := search(ctx, srv.BaseDotDir(), filterBy{ + authTokenId: at.Id, + resource: "sessions", + }, "created_time", "") + require.NoError(t, err) + assert.NoError(t, err) + assert.Nil(t, apiErr) + assert.NotNil(t, resp) + assert.NotNil(t, r) + assert.Less(t, r.Sessions[0].CreatedTime, r.Sessions[1].CreatedTime) + }) + + t.Run("sorted and filtered", func(t *testing.T) { + resp, r, apiErr, err := search(ctx, srv.BaseDotDir(), filterBy{ + authTokenId: at.Id, + flagFilter: `"/item/id" matches "ttcp"`, + resource: "targets", + }, "name", "desc") + require.NoError(t, err) + assert.NoError(t, err) + assert.Nil(t, apiErr) + assert.NotNil(t, resp) + assert.NotNil(t, r) + assert.Equal(t, expectedTargets[1].Name, r.Targets[0].Name) + assert.Equal(t, expectedTargets[0].Name, r.Targets[1].Name) + }) } diff --git a/internal/clientcache/internal/daemon/search_handler.go b/internal/clientcache/internal/daemon/search_handler.go index 64668d113..a54babd2c 100644 --- a/internal/clientcache/internal/daemon/search_handler.go +++ b/internal/clientcache/internal/daemon/search_handler.go @@ -59,11 +59,14 @@ const ( forceRefreshKey = "force_refresh" authTokenIdKey = "auth_token_id" maxResultSetSizeKey = "max_result_set_size" - sortByKey = "sort_by" - sortDirectionKey = "sort_direction" + // SortByKey The key used as the query parameter for which column to sort resources by + SortByKey = "sort_by" + // SortDirectionKey The key used as the query parameter for which direction, asc or desc, to sort resources by. + SortDirectionKey = "sort_direction" ) -var sortableColumnsForResource = map[cache.SearchableResource][]cache.SortBy{ +// SortableColumnsForResource Defines which columns a given resource can be sorted by +var SortableColumnsForResource = map[cache.SearchableResource][]cache.SortBy{ cache.Targets: {cache.SortByName}, cache.Sessions: {cache.SortByCreatedTime}, } @@ -93,8 +96,8 @@ func newSearchHandlerFunc(ctx context.Context, repo *cache.Repository, refreshSe maxResultSetSizeInt, maxResultSetSizeIntErr := strconv.Atoi(maxResultSetSizeStr) query := q.Get(queryKey) filter := q.Get(filterKey) - sb := q.Get(sortByKey) - sd := q.Get(sortDirectionKey) + sb := q.Get(SortByKey) + sd := q.Get(SortDirectionKey) searchableResource := cache.ToSearchableResource(resource) switch { @@ -291,7 +294,7 @@ func parseSortBy(sb string, sr cache.SearchableResource) (cache.SortBy, bool) { return cache.SortByDefault, true } - sortableBys, ok := sortableColumnsForResource[sr] + sortableBys, ok := SortableColumnsForResource[sr] if !ok || !slices.Contains(sortableBys, by) { return cache.SortByDefault, false }