Backport #8389 (#8392)

# Conflicts:
#	agent/cache-types/catalog_list_services_test.go

agent/agent_test.go

@@ -911,7 +911,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)

agent/cache-types/connect_ca_leaf_test.go

@@ -1040,7 +1040,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},
 	})

agent/cache/cache.go

@@ -34,6 +34,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
@@ -135,6 +146,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.

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

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.

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

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,

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),

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)