diff --git a/agent/agent_test.go b/agent/agent_test.go index 75366fcf6b..149f3610bd 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -912,7 +912,7 @@ func TestCacheRateLimit(test *testing.T) { require.NoError(t, err) resp := httptest.NewRecorder() - a.srv.Handler.ServeHTTP(resp, req) + a.srv.handler(false).ServeHTTP(resp, req) // Key doesn't actually exist so we should get 404 if got, want := resp.Code, http.StatusOK; got != want { t.Fatalf("bad response code got %d want %d", got, want) diff --git a/agent/cache-types/catalog_list_services_test.go b/agent/cache-types/catalog_list_services_test.go index efb1ff33fd..a8f66ab25c 100644 --- a/agent/cache-types/catalog_list_services_test.go +++ b/agent/cache-types/catalog_list_services_test.go @@ -82,7 +82,7 @@ func TestCatalogListServices_IntegrationWithCache_NotModifiedResponse(t *testing reply.NotModified = true }) - c := cache.New(nil) + c := cache.New(cache.Options{}) c.RegisterType(CatalogListServicesName, typ) last := cache.FetchResult{ Value: &structs.IndexedServices{ diff --git a/agent/cache-types/connect_ca_leaf_test.go b/agent/cache-types/connect_ca_leaf_test.go index 90bd12082b..9dfa9763e1 100644 --- a/agent/cache-types/connect_ca_leaf_test.go +++ b/agent/cache-types/connect_ca_leaf_test.go @@ -1051,7 +1051,7 @@ func testCALeafType(t *testing.T, rpc RPC) (*ConnectCALeaf, chan structs.Indexed rootsRPC := &testGatedRootsRPC{ValueCh: rootsCh} // Create a cache - c := cache.TestCache(t) + c := cache.New(cache.Options{}) c.RegisterType(ConnectCARootName, &testConnectCaRoot{ ConnectCARoot: ConnectCARoot{RPC: rootsRPC}, }) diff --git a/agent/cache/cache.go b/agent/cache/cache.go index 281b76c57b..44aeb3a046 100644 --- a/agent/cache/cache.go +++ b/agent/cache/cache.go @@ -35,6 +35,17 @@ import ( const ( CacheRefreshBackoffMin = 3 // 3 attempts before backing off CacheRefreshMaxWait = 1 * time.Minute // maximum backoff wait time + + // The following constants are default values for the cache entry + // rate limiter settings. + + // DefaultEntryFetchRate is the default rate at which cache entries can + // be fetch. This defaults to not being unlimited + DefaultEntryFetchRate = rate.Inf + + // DefaultEntryFetchMaxBurst is the number of cache entry fetches that can + // occur in a burst. + DefaultEntryFetchMaxBurst = 2 ) // Cache is a agent-local cache of Consul data. Create a Cache using the @@ -136,6 +147,13 @@ type Options struct { // New creates a new cache with the given RPC client and reasonable defaults. // Further settings can be tweaked on the returned value. func New(options Options) *Cache { + if options.EntryFetchRate == 0.0 { + options.EntryFetchRate = DefaultEntryFetchRate + } + if options.EntryFetchMaxBurst == 0 { + options.EntryFetchMaxBurst = DefaultEntryFetchMaxBurst + } + // Initialize the heap. The buffer of 1 is really important because // its possible for the expiry loop to trigger the heap to update // itself and it'd block forever otherwise. diff --git a/agent/cache/cache_test.go b/agent/cache/cache_test.go index 72422fb258..0725f1a576 100644 --- a/agent/cache/cache_test.go +++ b/agent/cache/cache_test.go @@ -24,7 +24,7 @@ func TestCacheGet_noIndex(t *testing.T) { typ := TestType(t) defer typ.AssertExpectations(t) - c := TestCache(t) + c := New(Options{}) c.RegisterType("t", typ) // Configure the type @@ -57,7 +57,7 @@ func TestCacheGet_initError(t *testing.T) { typ := TestType(t) defer typ.AssertExpectations(t) - c := TestCache(t) + c := New(Options{}) c.RegisterType("t", typ) // Configure the type @@ -92,7 +92,7 @@ func TestCacheGet_cachedErrorsDontStick(t *testing.T) { typ := TestType(t) defer typ.AssertExpectations(t) - c := TestCache(t) + c := New(Options{}) c.RegisterType("t", typ) // Configure the type @@ -153,7 +153,7 @@ func TestCacheGet_blankCacheKey(t *testing.T) { typ := TestType(t) defer typ.AssertExpectations(t) - c := TestCache(t) + c := New(Options{}) c.RegisterType("t", typ) // Configure the type @@ -184,7 +184,7 @@ func TestCacheGet_blockingInitSameKey(t *testing.T) { typ := TestType(t) defer typ.AssertExpectations(t) - c := TestCache(t) + c := New(Options{}) c.RegisterType("t", typ) // Configure the type @@ -221,7 +221,7 @@ func TestCacheGet_blockingInitDiffKeys(t *testing.T) { typ := TestType(t) defer typ.AssertExpectations(t) - c := TestCache(t) + c := New(Options{}) c.RegisterType("t", typ) // Keep track of the keys @@ -271,7 +271,7 @@ func TestCacheGet_blockingIndex(t *testing.T) { typ := TestType(t) defer typ.AssertExpectations(t) - c := TestCache(t) + c := New(Options{}) c.RegisterType("t", typ) // Configure the type @@ -301,7 +301,7 @@ func TestCacheGet_blockingIndex(t *testing.T) { func TestCacheGet_cancellation(t *testing.T) { typ := TestType(t) defer typ.AssertExpectations(t) - c := TestCache(t) + c := New(Options{}) c.RegisterType("t", typ) typ.Static(FetchResult{Value: 1, Index: 4}, nil).Times(0).WaitUntil(time.After(1 * time.Millisecond)) @@ -325,7 +325,7 @@ func TestCacheGet_blockingIndexTimeout(t *testing.T) { typ := TestType(t) defer typ.AssertExpectations(t) - c := TestCache(t) + c := New(Options{}) c.RegisterType("t", typ) // Configure the type @@ -361,7 +361,7 @@ func TestCacheGet_blockingIndexError(t *testing.T) { typ := TestType(t) defer typ.AssertExpectations(t) - c := TestCache(t) + c := New(Options{}) c.RegisterType("t", typ) // Configure the type @@ -398,7 +398,7 @@ func TestCacheGet_emptyFetchResult(t *testing.T) { typ := TestType(t) defer typ.AssertExpectations(t) - c := TestCache(t) + c := New(Options{}) c.RegisterType("t", typ) stateCh := make(chan int, 1) @@ -469,7 +469,7 @@ func TestCacheGet_periodicRefresh(t *testing.T) { QueryTimeout: 5 * time.Minute, }) defer typ.AssertExpectations(t) - c := TestCache(t) + c := New(Options{}) c.RegisterType("t", typ) // This is a bit weird, but we do this to ensure that the final @@ -509,7 +509,7 @@ func TestCacheGet_periodicRefreshMultiple(t *testing.T) { QueryTimeout: 5 * time.Minute, }) defer typ.AssertExpectations(t) - c := TestCache(t) + c := New(Options{}) c.RegisterType("t", typ) // This is a bit weird, but we do this to ensure that the final @@ -558,7 +558,7 @@ func TestCacheGet_periodicRefreshErrorBackoff(t *testing.T) { QueryTimeout: 5 * time.Minute, }) defer typ.AssertExpectations(t) - c := TestCache(t) + c := New(Options{}) c.RegisterType("t", typ) // Configure the type @@ -600,7 +600,7 @@ func TestCacheGet_periodicRefreshBadRPCZeroIndexErrorBackoff(t *testing.T) { QueryTimeout: 5 * time.Minute, }) defer typ.AssertExpectations(t) - c := TestCache(t) + c := New(Options{}) c.RegisterType("t", typ) // Configure the type @@ -645,7 +645,7 @@ func TestCacheGet_noIndexSetsOne(t *testing.T) { QueryTimeout: 5 * time.Minute, }) defer typ.AssertExpectations(t) - c := TestCache(t) + c := New(Options{}) c.RegisterType("t", typ) // Simulate "well behaved" RPC with no data yet but returning 1 @@ -706,7 +706,7 @@ func TestCacheGet_fetchTimeout(t *testing.T) { SupportsBlocking: true, }) defer typ.AssertExpectations(t) - c := TestCache(t) + c := New(Options{}) // Register the type with a timeout c.RegisterType("t", typ) @@ -740,7 +740,7 @@ func TestCacheGet_expire(t *testing.T) { LastGetTTL: 400 * time.Millisecond, }) defer typ.AssertExpectations(t) - c := TestCache(t) + c := New(Options{}) // Register the type with a timeout c.RegisterType("t", typ) @@ -796,7 +796,7 @@ func TestCacheGet_expireResetGet(t *testing.T) { LastGetTTL: 150 * time.Millisecond, }) defer typ.AssertExpectations(t) - c := TestCache(t) + c := New(Options{}) // Register the type with a timeout c.RegisterType("t", typ) @@ -852,7 +852,7 @@ func TestCacheGet_duplicateKeyDifferentType(t *testing.T) { typ2 := TestType(t) defer typ2.AssertExpectations(t) - c := TestCache(t) + c := New(Options{}) c.RegisterType("t", typ) c.RegisterType("t2", typ2) @@ -894,7 +894,7 @@ func TestCacheGet_duplicateKeyDifferentType(t *testing.T) { func TestCacheGet_partitionDC(t *testing.T) { t.Parallel() - c := TestCache(t) + c := New(Options{}) c.RegisterType("t", &testPartitionType{}) // Perform multiple gets @@ -913,7 +913,7 @@ func TestCacheGet_partitionDC(t *testing.T) { func TestCacheGet_partitionToken(t *testing.T) { t.Parallel() - c := TestCache(t) + c := New(Options{}) c.RegisterType("t", &testPartitionType{}) // Perform multiple gets @@ -959,7 +959,7 @@ func TestCacheGet_refreshAge(t *testing.T) { QueryTimeout: 5 * time.Minute, }) defer typ.AssertExpectations(t) - c := TestCache(t) + c := New(Options{}) c.RegisterType("t", typ) // Configure the type @@ -1076,7 +1076,7 @@ func TestCacheGet_nonRefreshAge(t *testing.T) { LastGetTTL: 100 * time.Millisecond, }) defer typ.AssertExpectations(t) - c := TestCache(t) + c := New(Options{}) c.RegisterType("t", typ) // Configure the type @@ -1156,7 +1156,7 @@ func TestCacheGet_nonBlockingType(t *testing.T) { t.Parallel() typ := TestTypeNonBlocking(t) - c := TestCache(t) + c := New(Options{}) c.RegisterType("t", typ) // Configure the type diff --git a/agent/cache/testing.go b/agent/cache/testing.go index 3e03144dae..c96612d949 100644 --- a/agent/cache/testing.go +++ b/agent/cache/testing.go @@ -8,15 +8,8 @@ import ( "github.com/mitchellh/go-testing-interface" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" - "golang.org/x/time/rate" ) -// TestCache returns a Cache instance configuring for testing. -func TestCache(t testing.T) *Cache { - // Simple but lets us do some fine-tuning later if we want to. - return New(Options{EntryFetchRate: rate.Inf, EntryFetchMaxBurst: 2}) -} - // TestCacheGetCh returns a channel that returns the result of the Get call. // This is useful for testing timing and concurrency with Get calls. Any // error will be logged, so the result value should always be asserted. diff --git a/agent/cache/watch_test.go b/agent/cache/watch_test.go index bbf73503fd..771a783593 100644 --- a/agent/cache/watch_test.go +++ b/agent/cache/watch_test.go @@ -19,7 +19,7 @@ func TestCacheNotify(t *testing.T) { typ := TestType(t) typ.On("RegisterOptions").Return(RegisterOptions{}) defer typ.AssertExpectations(t) - c := TestCache(t) + c := New(Options{}) c.RegisterType("t", typ) // Setup triggers to control when "updates" should be delivered @@ -165,7 +165,7 @@ func TestCacheNotifyPolling(t *testing.T) { typ := TestTypeNonBlocking(t) defer typ.AssertExpectations(t) - c := TestCache(t) + c := New(Options{}) c.RegisterType("t", typ) // Configure the type @@ -279,7 +279,7 @@ func TestCacheWatch_ErrorBackoff(t *testing.T) { typ := TestType(t) typ.On("RegisterOptions").Return(RegisterOptions{}) defer typ.AssertExpectations(t) - c := TestCache(t) + c := New(Options{}) c.RegisterType("t", typ) // Configure the type @@ -340,7 +340,7 @@ func TestCacheWatch_ErrorBackoffNonBlocking(t *testing.T) { typ := TestTypeNonBlocking(t) defer typ.AssertExpectations(t) - c := TestCache(t) + c := New(Options{}) c.RegisterType("t", typ) // Configure the type diff --git a/agent/cert-monitor/cert_monitor_test.go b/agent/cert-monitor/cert_monitor_test.go index dbbd63b5b2..07a65b4a15 100644 --- a/agent/cert-monitor/cert_monitor_test.go +++ b/agent/cert-monitor/cert_monitor_test.go @@ -633,7 +633,7 @@ func TestCertMonitor_New_Errors(t *testing.T) { }, "no-tls-configurator": { cfg: Config{ - Cache: cache.New(nil), + Cache: cache.New(cache.Options{}), Fallback: fallback, Tokens: tokens, Datacenter: "foo", @@ -643,7 +643,7 @@ func TestCertMonitor_New_Errors(t *testing.T) { }, "no-fallback": { cfg: Config{ - Cache: cache.New(nil), + Cache: cache.New(cache.Options{}), TLSConfigurator: testTLSConfigurator(t), Tokens: tokens, Datacenter: "foo", @@ -653,7 +653,7 @@ func TestCertMonitor_New_Errors(t *testing.T) { }, "no-tokens": { cfg: Config{ - Cache: cache.New(nil), + Cache: cache.New(cache.Options{}), TLSConfigurator: testTLSConfigurator(t), Fallback: fallback, Datacenter: "foo", @@ -663,7 +663,7 @@ func TestCertMonitor_New_Errors(t *testing.T) { }, "no-datacenter": { cfg: Config{ - Cache: cache.New(nil), + Cache: cache.New(cache.Options{}), TLSConfigurator: testTLSConfigurator(t), Fallback: fallback, Tokens: tokens, @@ -673,7 +673,7 @@ func TestCertMonitor_New_Errors(t *testing.T) { }, "no-node-name": { cfg: Config{ - Cache: cache.New(nil), + Cache: cache.New(cache.Options{}), TLSConfigurator: testTLSConfigurator(t), Fallback: fallback, Tokens: tokens, diff --git a/agent/config/builder.go b/agent/config/builder.go index 2dbe98df87..b3c7620f71 100644 --- a/agent/config/builder.go +++ b/agent/config/builder.go @@ -33,15 +33,6 @@ import ( "golang.org/x/time/rate" ) -// The following constants are default values for some settings -// Ensure to update documentation if you modify those values -const ( - // DefaultEntryFetchMaxBurst is the default value for cache.entry_fetch_max_burst - DefaultEntryFetchMaxBurst = 2 - // DefaultEntryFetchRate is the default value for cache.entry_fetch_rate - DefaultEntryFetchRate = float64(rate.Inf) -) - // Builder constructs a valid runtime configuration from multiple // configuration sources. // @@ -899,9 +890,11 @@ func (b *Builder) Build() (rt RuntimeConfig, err error) { BootstrapExpect: b.intVal(c.BootstrapExpect), Cache: cache.Options{ EntryFetchRate: rate.Limit( - b.float64ValWithDefault(c.Cache.EntryFetchRate, DefaultEntryFetchRate)), + b.float64ValWithDefault(c.Cache.EntryFetchRate, float64(cache.DefaultEntryFetchRate)), + ), EntryFetchMaxBurst: b.intValWithDefault( - c.Cache.EntryFetchMaxBurst, DefaultEntryFetchMaxBurst), + c.Cache.EntryFetchMaxBurst, cache.DefaultEntryFetchMaxBurst, + ), }, CAFile: b.stringVal(c.CAFile), CAPath: b.stringVal(c.CAPath), diff --git a/agent/proxycfg/testing.go b/agent/proxycfg/testing.go index 910398c329..291c06762d 100644 --- a/agent/proxycfg/testing.go +++ b/agent/proxycfg/testing.go @@ -52,7 +52,7 @@ func NewTestCacheTypes(t testing.T) *TestCacheTypes { // TestCacheWithTypes registers ControllableCacheTypes for all types that // proxycfg will watch suitable for testing a proxycfg.State or Manager. func TestCacheWithTypes(t testing.T, types *TestCacheTypes) *cache.Cache { - c := cache.TestCache(t) + c := cache.New(cache.Options{}) c.RegisterType(cachetype.ConnectCARootName, types.roots) c.RegisterType(cachetype.ConnectCALeafName, types.leaf) c.RegisterType(cachetype.IntentionMatchName, types.intentions)