Enable SPIRE Agent LRU cache by default (spiffe#4773)

* Enable SPIRE Agent LRU cache by default Signed-off-by: Prasad Borole <[email protected]> Signed-off-by: Faisal Memon <[email protected]>
faisal-memon · Feb 7, 2024 · 48b1321 · 48b1321
1 parent d87b615
commit 48b1321
Show file tree

Hide file tree

Showing 7 changed files with 52 additions and 9 deletions.
diff --git a/cmd/spire-agent/cli/run/run.go b/cmd/spire-agent/cli/run/run.go
@@ -122,6 +122,7 @@ type experimentalConfig struct {
 
 	UnusedKeyPositions   map[string][]token.Pos `hcl:",unusedKeyPositions"`
 	X509SVIDCacheMaxSize int                    `hcl:"x509_svid_cache_max_size"`
+	DisableLRUCache      bool                   `hcl:"disable_lru_cache"`
 }
 
 type Command struct {
@@ -468,6 +469,11 @@ func NewAgentConfig(c *Config, logOptions []log.Option, allowUnknownConfig bool)
 	ac.UseSyncAuthorizedEntries = c.Agent.Experimental.UseSyncAuthorizedEntries
 	ac.X509SVIDCacheMaxSize = c.Agent.Experimental.X509SVIDCacheMaxSize
 
+	if c.Agent.Experimental.DisableLRUCache && ac.X509SVIDCacheMaxSize != 0 {
+		return nil, errors.New("x509_svid_cache_max_size should not be set when disable_lru_cache is set")
+	}
+	ac.DisableLRUCache = c.Agent.Experimental.DisableLRUCache
+
 	serverHostPort := net.JoinHostPort(c.Agent.ServerAddress, strconv.Itoa(c.Agent.ServerPort))
 	ac.ServerAddress = fmt.Sprintf("dns:///%s", serverHostPort)
 

diff --git a/cmd/spire-agent/cli/run/run_test.go b/cmd/spire-agent/cli/run/run_test.go
@@ -915,6 +915,34 @@ func TestNewAgentConfig(t *testing.T) {
 				require.Nil(t, c)
 			},
 		},
+		{
+			msg: "disable_lru_cache is set",
+			input: func(c *Config) {
+				c.Agent.Experimental.DisableLRUCache = true
+			},
+			test: func(t *testing.T, c *agent.Config) {
+				require.True(t, c.DisableLRUCache)
+			},
+		},
+		{
+			msg:         "both disable_lru_cache and x509_svid_cache_max_size are set",
+			expectError: true,
+			input: func(c *Config) {
+				c.Agent.Experimental.DisableLRUCache = true
+				c.Agent.Experimental.X509SVIDCacheMaxSize = 100
+			},
+			test: func(t *testing.T, c *agent.Config) {
+				require.Nil(t, c)
+			},
+		},
+		{
+			msg: "disable_lru_cache is not set",
+			input: func(c *Config) {
+			},
+			test: func(t *testing.T, c *agent.Config) {
+				require.False(t, c.DisableLRUCache)
+			},
+		},
 		{
 			msg: "allowed_foreign_jwt_claims provided",
 			input: func(c *Config) {

diff --git a/doc/spire_agent.md b/doc/spire_agent.md
@@ -70,10 +70,12 @@ This may be useful for templating configuration files, for example across differ
 | `workload_x509_svid_key_type`     | The workload X509 SVID key type &lt;rsa-2048&vert;ec-p256&gt;                                                                                                                                                                                     | ec-p256                          |
 | `availability_target`             | The minimum amount of time desired to gracefully handle SPIRE Server or Agent downtime. This configurable influences how aggressively X509 SVIDs should be rotated. If set, must be at least 24h. See [Availability Target](#availability-target) |                                  |
 
-| experimental      | Description                                                     | Default                 |
-|:------------------|-----------------------------------------------------------------|-------------------------|
-| `named_pipe_name` | Pipe name to bind the SPIRE Agent API named pipe (Windows only) | \spire-agent\public\api |
-| `sync_interval`   | Sync interval with SPIRE server with exponential backoff        | 5 sec                   |
+| experimental               | Description                                                           | Default                 |
+|:---------------------------|-----------------------------------------------------------------------|-------------------------|
+| `named_pipe_name`          | Pipe name to bind the SPIRE Agent API named pipe (Windows only)       | \spire-agent\public\api |
+| `sync_interval`            | Sync interval with SPIRE server with exponential backoff              | 5 sec                   |
+| `x509_svid_cache_max_size` | Soft limit of max number of SVIDs that would be stored in LRU cache   | 1000                    |
+| `disable_lru_cache`        | Reverts back to use the SPIRE Agent non-LRU cache for storing SVIDs   | false                   |
 
 ### Initial trust bundle configuration
 

diff --git a/pkg/agent/agent.go b/pkg/agent/agent.go
@@ -227,6 +227,7 @@ func (a *Agent) newManager(ctx context.Context, sto storage.Storage, cat catalog
 		SyncInterval:             a.c.SyncInterval,
 		UseSyncAuthorizedEntries: a.c.UseSyncAuthorizedEntries,
 		SVIDCacheMaxSize:         a.c.X509SVIDCacheMaxSize,
+		DisableLRUCache:          a.c.DisableLRUCache,
 		SVIDStoreCache:           cache,
 		NodeAttestor:             na,
 		RotationStrategy:         rotationutil.NewRotationStrategy(a.c.AvailabilityTarget),

diff --git a/pkg/agent/config.go b/pkg/agent/config.go
@@ -66,6 +66,9 @@ type Config struct {
 	// X509SVIDCacheMaxSize is a soft limit of max number of SVIDs that would be stored in cache
 	X509SVIDCacheMaxSize int
 
+	// DisableLRUCache disables the SPIRE Agent LRU cache used for storing SVIDs and fallback to original cache
+	DisableLRUCache bool
+
 	// Trust domain and associated CA bundle
 	TrustDomain spiffeid.TrustDomain
 	TrustBundle []*x509.Certificate

diff --git a/pkg/agent/manager/cache/lru_cache.go b/pkg/agent/manager/cache/lru_cache.go
@@ -17,8 +17,10 @@ import (
 )
 
 const (
+	// DefaultSVIDCacheMaxSize is set when svidCacheMaxSize is not provided
 	DefaultSVIDCacheMaxSize = 1000
-	SVIDSyncInterval        = 500 * time.Millisecond
+	// SVIDSyncInterval is the interval at which SVIDs are synced with subscribers
+	SVIDSyncInterval = 500 * time.Millisecond
 )
 
 // Cache caches each registration entry, bundles, and JWT SVIDs for the agent.

diff --git a/pkg/agent/manager/config.go b/pkg/agent/manager/config.go
@@ -39,6 +39,7 @@ type Config struct {
 	RotationInterval         time.Duration
 	SVIDStoreCache           *storecache.Cache
 	SVIDCacheMaxSize         int
+	DisableLRUCache          bool
 	NodeAttestor             nodeattestor.NodeAttestor
 	RotationStrategy         *rotationutil.RotationStrategy
 
@@ -65,13 +66,13 @@ func newManager(c *Config) *manager {
 	}
 
 	var cache Cache
-	if c.SVIDCacheMaxSize > 0 {
+	if c.DisableLRUCache {
+		cache = managerCache.New(c.Log.WithField(telemetry.SubsystemName, telemetry.CacheManager), c.TrustDomain, c.Bundle,
+			c.Metrics)
+	} else {
 		// use LRU cache implementation
 		cache = managerCache.NewLRUCache(c.Log.WithField(telemetry.SubsystemName, telemetry.CacheManager), c.TrustDomain, c.Bundle,
 			c.Metrics, c.SVIDCacheMaxSize, c.Clk)
-	} else {
-		cache = managerCache.New(c.Log.WithField(telemetry.SubsystemName, telemetry.CacheManager), c.TrustDomain, c.Bundle,
-			c.Metrics)
 	}
 
 	rotCfg := &svid.RotatorConfig{