feat(clientcache): Add sort parameters to CLI search command (#6362, #6366)

Co-authored-by: Sepehr <sepehr.foroughishafiei@hashicorp.com>

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

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

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
 	}