From c74579cead26991707284604c03a87f26c7bf13d Mon Sep 17 00:00:00 2001 From: Todd Date: Wed, 6 Dec 2023 08:55:11 -0800 Subject: [PATCH] Allow forced refresh attempt when searching (#4077) * Allow forced refresh attempt when searching --- internal/clientcache/cmd/search/search.go | 28 +++-- .../clientcache/internal/cache/options.go | 10 ++ .../internal/cache/options_test.go | 7 ++ .../clientcache/internal/cache/refresh.go | 9 +- .../internal/cache/refresh_test.go | 109 ++++++++++++++++++ .../internal/daemon/search_handler.go | 16 ++- 6 files changed, 163 insertions(+), 16 deletions(-) diff --git a/internal/clientcache/cmd/search/search.go b/internal/clientcache/cmd/search/search.go index ec5db74bf..781120fca 100644 --- a/internal/clientcache/cmd/search/search.go +++ b/internal/clientcache/cmd/search/search.go @@ -38,8 +38,9 @@ var ( type SearchCommand struct { *base.Command - flagQuery string - flagResource string + flagQuery string + flagResource string + flagForceRefresh bool } func (c *SearchCommand) Synopsis() string { @@ -75,6 +76,12 @@ func (c *SearchCommand) Flags() *base.FlagSets { Usage: `Specifies the resource type to search over`, Completion: complete.PredictSet(supportedResourceTypes...), }) + f.BoolVar(&base.BoolVar{ + Name: "force-refresh", + Target: &c.flagForceRefresh, + Usage: `Forces a refresh to be attempted prior to performing the search`, + Hidden: true, + }) return set } @@ -148,9 +155,10 @@ func (c *SearchCommand) Search(ctx context.Context) (*api.Response, *daemon.Sear } tf := filterBy{ - flagQuery: c.flagQuery, - resource: c.flagResource, - authTokenId: strings.Join(tSlice[:2], "_"), + flagQuery: c.flagQuery, + resource: c.flagResource, + authTokenId: strings.Join(tSlice[:2], "_"), + forceRefresh: c.flagForceRefresh, } var opts []client.Option if c.FlagOutputCurlString { @@ -182,6 +190,9 @@ func search(ctx context.Context, daemonPath string, fb filterBy, opt ...client.O q.Add("auth_token_id", fb.authTokenId) q.Add("resource", fb.resource) q.Add("query", fb.flagQuery) + if fb.forceRefresh { + q.Add("force_refresh", "true") + } resp, err := c.Get(ctx, "/v1/search", q, opt...) if err != nil { return nil, nil, nil, fmt.Errorf("Error when sending request to the daemon: %w.", err) @@ -329,7 +340,8 @@ func printSessionListTable(items []*sessions.Session) string { } type filterBy struct { - flagQuery string - authTokenId string - resource string + flagQuery string + authTokenId string + resource string + forceRefresh bool } diff --git a/internal/clientcache/internal/cache/options.go b/internal/clientcache/internal/cache/options.go index c6780a7b1..574d39e09 100644 --- a/internal/clientcache/internal/cache/options.go +++ b/internal/clientcache/internal/cache/options.go @@ -14,6 +14,7 @@ type options struct { withUserId string withTargetRetrievalFunc TargetRetrievalFunc withSessionRetrievalFunc SessionRetrievalFunc + withIgnoreSearchStaleness bool } // Option - how options are passed as args @@ -75,3 +76,12 @@ func WithSessionRetrievalFunc(fn SessionRetrievalFunc) Option { return nil } } + +// WithIgnoreSearchStaleness provides an option for ignoring the resource +// staleness when performing a search. +func WithIgnoreSearchStaleness(b bool) Option { + return func(o *options) error { + o.withIgnoreSearchStaleness = b + return nil + } +} diff --git a/internal/clientcache/internal/cache/options_test.go b/internal/clientcache/internal/cache/options_test.go index 7f60342a4..a53d77276 100644 --- a/internal/clientcache/internal/cache/options_test.go +++ b/internal/clientcache/internal/cache/options_test.go @@ -72,4 +72,11 @@ func Test_GetOpts(t *testing.T) { testOpts := getDefaultOptions() assert.Equal(t, opts, testOpts) }) + t.Run("withIgnoreSearchStaleness", func(t *testing.T) { + opts, err := getOpts(WithIgnoreSearchStaleness(true)) + require.NoError(t, err) + testOpts := getDefaultOptions() + testOpts.withIgnoreSearchStaleness = true + assert.Equal(t, opts, testOpts) + }) } diff --git a/internal/clientcache/internal/cache/refresh.go b/internal/clientcache/internal/cache/refresh.go index 5c0cef14d..0a6b2b0c3 100644 --- a/internal/clientcache/internal/cache/refresh.go +++ b/internal/clientcache/internal/cache/refresh.go @@ -200,13 +200,18 @@ func (r *RefreshService) RefreshForSearch(ctx context.Context, authTokenid strin return errors.Wrap(ctx, err, op) } + opts, err := getOpts(opt...) + if err != nil { + return errors.Wrap(ctx, err, op) + } + switch resourceType { case Targets: rtv, err := r.repo.lookupRefreshToken(ctx, u, targetResourceType) if err != nil { return errors.Wrap(ctx, err, op) } - if rtv != nil && time.Since(rtv.UpdateTime) > r.maxSearchStaleness { + if opts.withIgnoreSearchStaleness || rtv != nil && time.Since(rtv.UpdateTime) > r.maxSearchStaleness { if err := r.repo.refreshTargets(ctx, u, tokens, opt...); err != nil { return errors.Wrap(ctx, err, op) } @@ -216,7 +221,7 @@ func (r *RefreshService) RefreshForSearch(ctx context.Context, authTokenid strin if err != nil { return errors.Wrap(ctx, err, op) } - if rtv != nil && time.Since(rtv.UpdateTime) > r.maxSearchStaleness { + if opts.withIgnoreSearchStaleness || rtv != nil && time.Since(rtv.UpdateTime) > r.maxSearchStaleness { if err := r.repo.refreshSessions(ctx, u, tokens, opt...); err != nil { return errors.Wrap(ctx, err, op) } diff --git a/internal/clientcache/internal/cache/refresh_test.go b/internal/clientcache/internal/cache/refresh_test.go index 9b8c69038..4539e5eb2 100644 --- a/internal/clientcache/internal/cache/refresh_test.go +++ b/internal/clientcache/internal/cache/refresh_test.go @@ -400,6 +400,61 @@ func TestRefreshForSearch(t *testing.T) { assert.ElementsMatch(t, retTargets[2:], cachedTargets) }) + t.Run("targets forced refreshed for searching", func(t *testing.T) { + s, err := db.Open(ctx) + require.NoError(t, err) + r, err := NewRepository(ctx, s, &sync.Map{}, mapBasedAuthTokenKeyringLookup(atMap), sliceBasedAuthTokenBoundaryReader(boundaryAuthTokens)) + require.NoError(t, err) + // Everything can stay stale for an hour + rs, err := NewRefreshService(ctx, r, time.Hour, 0) + require.NoError(t, err) + require.NoError(t, r.AddKeyringToken(ctx, boundaryAddr, KeyringToken{KeyringType: "k", TokenName: "t", AuthTokenId: at.Id})) + + retTargets := []*targets.Target{ + target("1"), + target("2"), + target("3"), + target("4"), + } + opts := []Option{ + WithSessionRetrievalFunc(testStaticResourceRetrievalFunc[*sessions.Session](t, nil, nil)), + WithTargetRetrievalFunc(testStaticResourceRetrievalFunc[*targets.Target](t, + [][]*targets.Target{ + retTargets[:3], + retTargets[3:], + }, + [][]string{ + nil, + {retTargets[0].Id, retTargets[1].Id}, + }, + )), + } + assert.NoError(t, rs.RefreshForSearch(ctx, at.Id, Targets, opts...)) + + cachedTargets, err := r.ListTargets(ctx, at.Id) + assert.NoError(t, err) + assert.Empty(t, cachedTargets) + + // Now load up a few resources and a token, and trying again should + // see the RefreshForSearch update more fields. + assert.NoError(t, rs.Refresh(ctx, opts...)) + cachedTargets, err = r.ListTargets(ctx, at.Id) + assert.NoError(t, err) + assert.ElementsMatch(t, retTargets[:3], cachedTargets) + + // No refresh happened because it is not considered stale + assert.NoError(t, rs.RefreshForSearch(ctx, at.Id, Targets, opts...)) + cachedTargets, err = r.ListTargets(ctx, at.Id) + assert.NoError(t, err) + assert.ElementsMatch(t, retTargets[:3], cachedTargets) + + // Now force refresh + assert.NoError(t, rs.RefreshForSearch(ctx, at.Id, Targets, append(opts, WithIgnoreSearchStaleness(true))...)) + cachedTargets, err = r.ListTargets(ctx, at.Id) + assert.NoError(t, err) + assert.ElementsMatch(t, retTargets[2:], cachedTargets) + }) + t.Run("no refresh token no refresh for search", func(t *testing.T) { s, err := db.Open(ctx) require.NoError(t, err) @@ -492,6 +547,60 @@ func TestRefreshForSearch(t *testing.T) { assert.NoError(t, err) assert.ElementsMatch(t, retSess[2:], cachedSessions) }) + + t.Run("sessions forced refreshed for searching", func(t *testing.T) { + s, err := db.Open(ctx) + require.NoError(t, err) + r, err := NewRepository(ctx, s, &sync.Map{}, mapBasedAuthTokenKeyringLookup(atMap), sliceBasedAuthTokenBoundaryReader(boundaryAuthTokens)) + require.NoError(t, err) + // Everything stays fresh for 1 hour + rs, err := NewRefreshService(ctx, r, time.Hour, 0) + require.NoError(t, err) + require.NoError(t, r.AddKeyringToken(ctx, boundaryAddr, KeyringToken{KeyringType: "k", TokenName: "t", AuthTokenId: at.Id})) + + retSess := []*sessions.Session{ + session("1"), + session("2"), + session("3"), + session("4"), + } + opts := []Option{ + WithTargetRetrievalFunc(testStaticResourceRetrievalFunc[*targets.Target](t, nil, nil)), + WithSessionRetrievalFunc(testStaticResourceRetrievalFunc[*sessions.Session](t, + [][]*sessions.Session{ + retSess[:3], + retSess[3:], + }, + [][]string{ + nil, + {retSess[0].Id, retSess[1].Id}, + }, + )), + } + + // First call doesn't sync anything because no sessions were already synced yet + assert.NoError(t, rs.RefreshForSearch(ctx, at.Id, Sessions, opts...)) + cachedSessions, err := r.ListSessions(ctx, at.Id) + assert.NoError(t, err) + assert.Empty(t, cachedSessions) + + assert.NoError(t, rs.Refresh(ctx, opts...)) + cachedSessions, err = r.ListSessions(ctx, at.Id) + assert.NoError(t, err) + assert.ElementsMatch(t, retSess[:3], cachedSessions) + + // Refresh for search doesn't refresh anything because it isn't stale + assert.NoError(t, rs.RefreshForSearch(ctx, at.Id, Sessions, opts...)) + cachedSessions, err = r.ListSessions(ctx, at.Id) + assert.NoError(t, err) + assert.ElementsMatch(t, retSess[:3], cachedSessions) + + // Now force the refresh and see things get updated + assert.NoError(t, rs.RefreshForSearch(ctx, at.Id, Sessions, append(opts, WithIgnoreSearchStaleness(true))...)) + cachedSessions, err = r.ListSessions(ctx, at.Id) + assert.NoError(t, err) + assert.ElementsMatch(t, retSess[2:], cachedSessions) + }) } func TestRefresh(t *testing.T) { diff --git a/internal/clientcache/internal/daemon/search_handler.go b/internal/clientcache/internal/daemon/search_handler.go index 819c36141..70b451675 100644 --- a/internal/clientcache/internal/daemon/search_handler.go +++ b/internal/clientcache/internal/daemon/search_handler.go @@ -8,6 +8,7 @@ import ( "encoding/json" "fmt" "net/http" + "strconv" "github.com/hashicorp/boundary/api/sessions" "github.com/hashicorp/boundary/api/targets" @@ -24,11 +25,10 @@ type SearchResult struct { } const ( - filterKey = "filter" - queryKey = "query" - resourceKey = "resource" - - boundaryAddrKey = "boundary_addr" + filterKey = "filter" + queryKey = "query" + resourceKey = "resource" + forceRefreshKey = "force_refresh" authTokenIdKey = "auth_token_id" ) @@ -70,10 +70,14 @@ func newSearchHandlerFunc(ctx context.Context, repo *cache.Repository, refreshSe return } + var opts []cache.Option + if b, err := strconv.ParseBool(r.URL.Query().Get(forceRefreshKey)); err == nil && b { + opts = append(opts, cache.WithIgnoreSearchStaleness(true)) + } // Refresh the resources for the provided user, if possible. This is best // effort, so if there is any problem refreshing, we just log the error // and move on to handling the search request. - if err := refreshService.RefreshForSearch(ctx, authTokenId, searchableResource); err != nil { + if err := refreshService.RefreshForSearch(ctx, authTokenId, searchableResource, opts...); err != nil { // we don't stop the search, we just log that the inline refresh failed event.WriteError(ctx, op, err, event.WithInfoMsg("when refreshing the resources inline for search", "auth_token_id", authTokenId, "resource", searchableResource)) }