From ecb5c5dbc4942e433b84cffbde3a5f134b3aba95 Mon Sep 17 00:00:00 2001 From: Todd Date: Mon, 18 Dec 2023 14:52:23 -0800 Subject: [PATCH] Reviewer comments --- internal/clientcache/cmd/daemon/start.go | 24 +++------ internal/clientcache/internal/cache/status.go | 50 +++++++++---------- .../internal/daemon/search_handler.go | 2 +- .../internal/daemon/status_handler.go | 2 +- 4 files changed, 33 insertions(+), 45 deletions(-) diff --git a/internal/clientcache/cmd/daemon/start.go b/internal/clientcache/cmd/daemon/start.go index 178a2e1971..117c890ea7 100644 --- a/internal/clientcache/cmd/daemon/start.go +++ b/internal/clientcache/cmd/daemon/start.go @@ -104,26 +104,26 @@ func (c *StartCommand) Flags() *base.FlagSets { f.DurationVar(&base.DurationVar{ Name: "refresh-interval", Target: &c.flagRefreshInterval, - Usage: `Specifies the interval between refresh token supported cache refreshes. Default: 5 minutes`, + Usage: `Specifies the interval between refresh token supported cache refreshes.`, Default: daemon.DefaultRefreshInterval, }) f.DurationVar(&base.DurationVar{ Name: "recheck-support-interval", Target: &c.flagRecheckSupportInterval, - Usage: `Specifies the interval between checking if a boundary instances is supported when it previously was not. Default: 1 hour`, + Usage: `Specifies the interval between checking if a boundary instances is supported when it previously was not.`, Default: daemon.DefaultRecheckSupportInterval, Hidden: true, }) f.DurationVar(&base.DurationVar{ Name: "max-search-staleness", Target: &c.flagMaxSearchStaleness, - Usage: `Specifies the duration of time that can pass since the resource was last updated before performing a search waits for the resources being refreshed first. Default: 1 minute`, + Usage: `Specifies the duration of time that can pass since the resource was last updated before performing a search waits for the resources being refreshed first.`, Default: daemon.DefaultSearchStaleness, }) f.DurationVar(&base.DurationVar{ Name: "max-search-refresh-timeout", Target: &c.flagMaxSearchRefreshTimeout, - Usage: `If a search request triggers a best effort refresh, this specifies how long the refresh should run before timing out. Default: 1 second`, + Usage: `If a search request triggers a best effort refresh, this specifies how long the refresh should run before timing out.`, Default: daemon.DefaultSearchRefreshTimeout, }) f.BoolVar(&base.BoolVar{ @@ -308,6 +308,10 @@ func (c *StartCommand) makeBackground(ctx context.Context, dotDir string) (bool, env := os.Environ() env = append(env, fmt.Sprintf("%s=%s", backgroundEnvName, backgroundEnvVal)) args := []string{"daemon", "start"} + args = append(args, "-refresh-interval", c.flagRefreshInterval.String()) + args = append(args, "-max-search-staleness", c.flagMaxSearchStaleness.String()) + args = append(args, "-max-search-refresh-timeout", c.flagMaxSearchRefreshTimeout.String()) + args = append(args, "-recheck-support-interval", c.flagRecheckSupportInterval.String()) if c.flagLogLevel != "" { args = append(args, "-log-level", c.flagLogLevel) } @@ -317,18 +321,6 @@ func (c *StartCommand) makeBackground(ctx context.Context, dotDir string) (bool, if c.flagDatabaseUrl != "" { args = append(args, "-database-url", c.flagDatabaseUrl) } - if c.flagRefreshInterval > 0 && c.flagRefreshInterval != daemon.DefaultRefreshInterval { - args = append(args, "-refresh-interval", c.flagRefreshInterval.String()) - } - if c.flagRecheckSupportInterval > 0 && c.flagRecheckSupportInterval != daemon.DefaultRecheckSupportInterval { - args = append(args, "-full-fetch-interval", c.flagRecheckSupportInterval.String()) - } - if c.flagMaxSearchStaleness > 0 && c.flagMaxSearchStaleness != daemon.DefaultSearchStaleness { - args = append(args, "-max-search-staleness", c.flagMaxSearchStaleness.String()) - } - if c.flagMaxSearchRefreshTimeout > 0 && c.flagMaxSearchRefreshTimeout != daemon.DefaultSearchRefreshTimeout { - args = append(args, "-max-search-refresh-timeout", c.flagMaxSearchRefreshTimeout.String()) - } if c.flagStoreDebug { args = append(args, "-store-debug") } diff --git a/internal/clientcache/internal/cache/status.go b/internal/clientcache/internal/cache/status.go index cb6ef3f9b5..b423832194 100644 --- a/internal/clientcache/internal/cache/status.go +++ b/internal/clientcache/internal/cache/status.go @@ -93,40 +93,36 @@ func (s *StatusService) Status(ctx context.Context) (*Status, error) { Id: u.Id, } - { - cacheSupported, err := s.repo.cacheSupportState(ctx, u) - if err != nil { - return nil, errors.Wrap(ctx, err, op) - } + cacheSupported, err := s.repo.cacheSupportState(ctx, u) + if err != nil { + return nil, errors.Wrap(ctx, err, op) + } - us.BoundaryStatus = BoundaryStatus{ - Address: u.Address, - CachingSupported: cacheSupported.supported, - } - if cacheSupported.lastChecked != nil && !cacheSupported.lastChecked.IsZero() { - us.BoundaryStatus.LastSupportCheck = time.Since(*cacheSupported.lastChecked) - } + us.BoundaryStatus = BoundaryStatus{ + Address: u.Address, + CachingSupported: cacheSupported.supported, + } + if cacheSupported.lastChecked != nil && !cacheSupported.lastChecked.IsZero() { + us.BoundaryStatus.LastSupportCheck = time.Since(*cacheSupported.lastChecked) } - { - toks, err := s.repo.listTokens(ctx, u) + toks, err := s.repo.listTokens(ctx, u) + if err != nil { + return nil, errors.Wrap(ctx, err, op) + } + for _, t := range toks { + ts := &AuthTokenStatus{ + Id: t.Id, + } + kt, err := s.repo.listKeyringTokens(ctx, t) if err != nil { return nil, errors.Wrap(ctx, err, op) } - for _, t := range toks { - ts := &AuthTokenStatus{ - Id: t.Id, - } - kt, err := s.repo.listKeyringTokens(ctx, t) - if err != nil { - return nil, errors.Wrap(ctx, err, op) - } - ts.KeyringReferences = len(kt) - if _, ok := s.repo.idToKeyringlessAuthToken.Load(t.Id); ok { - ts.KeyringlessReferences = 1 - } - us.AuthTokens = append(us.AuthTokens, *ts) + ts.KeyringReferences = len(kt) + if _, ok := s.repo.idToKeyringlessAuthToken.Load(t.Id); ok { + ts.KeyringlessReferences = 1 } + us.AuthTokens = append(us.AuthTokens, *ts) } for _, rt := range []resourceType{targetResourceType, sessionResourceType} { diff --git a/internal/clientcache/internal/daemon/search_handler.go b/internal/clientcache/internal/daemon/search_handler.go index 3f0f637c73..f4727a7790 100644 --- a/internal/clientcache/internal/daemon/search_handler.go +++ b/internal/clientcache/internal/daemon/search_handler.go @@ -139,7 +139,7 @@ func toApiResult(sr *cache.SearchResult) *SearchResult { var errSearchNotSupported = &api.Error{ Kind: "Unsupported Search Request", - Message: "The requesting user is for a Boundary instance that doesn't support search. The Boundary instance must support refresh tokens for search to be supported.", + Message: "The request is for a Boundary instance that doesn't support search. The Boundary instance must support refresh tokens for search to be supported.", } func writeUnsupportedError(w http.ResponseWriter) { diff --git a/internal/clientcache/internal/daemon/status_handler.go b/internal/clientcache/internal/daemon/status_handler.go index 169025dd1c..0526650037 100644 --- a/internal/clientcache/internal/daemon/status_handler.go +++ b/internal/clientcache/internal/daemon/status_handler.go @@ -47,7 +47,7 @@ type BoundaryStatus struct { // The boundary address for this user Address string `json:"address,omitempty"` // Whether the controller responses are supported by the cache - CacheSupport string `json:"cache_response,omitempty"` + CacheSupport string `json:"cache_support,omitempty"` // How long ago the Boundary instance was checked for cache compatability LastSupportCheck time.Duration `json:"last_support_check,omitempty"` }