Delete non-LRU cache in SPIRE Agent (#5383)

* Delete non-LRU cache in SPIRE Agent

Signed-off-by: amoore877 <[email protected]>

cmd/spire-agent/cli/run/run.go

@@ -120,10 +120,6 @@ type experimentalConfig struct {
 	UseSyncAuthorizedEntries bool   `hcl:"use_sync_authorized_entries"`
 
 	Flags fflag.RawConfig `hcl:"feature_flags"`
-
-	UnusedKeyPositions   map[string][]token.Pos `hcl:",unusedKeyPositions"`
-	X509SVIDCacheMaxSize int                    `hcl:"x509_svid_cache_max_size"`
-	DisableLRUCache      bool                   `hcl:"disable_lru_cache"`
 }
 
 type Command struct {
@@ -498,19 +494,6 @@ func NewAgentConfig(c *Config, logOptions []log.Option, allowUnknownConfig bool)
 		ac.LogReopener = log.ReopenOnSignal(logger, reopenableFile)
 	}
 
-	if c.Agent.Experimental.X509SVIDCacheMaxSize < 0 {
-		return nil, errors.New("x509_svid_cache_max_size should not be negative")
-	}
-	if c.Agent.Experimental.X509SVIDCacheMaxSize > 0 || c.Agent.Experimental.DisableLRUCache {
-		logger.Warn("The `x509_svid_cache_max_size` and `disable_lru_cache` configurations are deprecated. They will be removed in a future release.")
-	}
-	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
-
 	td, err := common_cli.ParseTrustDomain(c.Agent.TrustDomain, logger)
 	if err != nil {
 		return nil, err

cmd/spire-agent/cli/run/run_test.go

@@ -897,106 +897,6 @@ func TestNewAgentConfig(t *testing.T) {
 				require.Nil(t, c)
 			},
 		},
-		{
-			msg: "x509_svid_cache_max_size is set",
-			input: func(c *Config) {
-				c.Agent.Experimental.X509SVIDCacheMaxSize = 100
-			},
-			logOptions: func(t *testing.T) []log.Option {
-				return []log.Option{
-					func(logger *log.Logger) error {
-						logger.SetOutput(io.Discard)
-						hook := test.NewLocal(logger.Logger)
-						t.Cleanup(func() {
-							spiretest.AssertLogsContainEntries(t, hook.AllEntries(), []spiretest.LogEntry{
-								{
-									Level: logrus.WarnLevel,
-									Message: "The `x509_svid_cache_max_size` and `disable_lru_cache` " +
-										"configurations are deprecated. They will be removed in a future release.",
-								},
-							})
-						})
-						return nil
-					},
-				}
-			},
-			test: func(t *testing.T, c *agent.Config) {
-				require.EqualValues(t, 100, c.X509SVIDCacheMaxSize)
-			},
-		},
-		{
-			msg: "x509_svid_cache_max_size is not set",
-			input: func(c *Config) {
-			},
-			test: func(t *testing.T, c *agent.Config) {
-				require.EqualValues(t, 0, c.X509SVIDCacheMaxSize)
-			},
-		},
-		{
-			msg: "x509_svid_cache_max_size is zero",
-			input: func(c *Config) {
-				c.Agent.Experimental.X509SVIDCacheMaxSize = 0
-			},
-			test: func(t *testing.T, c *agent.Config) {
-				require.EqualValues(t, 0, c.X509SVIDCacheMaxSize)
-			},
-		},
-		{
-			msg:         "x509_svid_cache_max_size is negative",
-			expectError: true,
-			input: func(c *Config) {
-				c.Agent.Experimental.X509SVIDCacheMaxSize = -10
-			},
-			test: func(t *testing.T, c *agent.Config) {
-				require.Nil(t, c)
-			},
-		},
-		{
-			msg: "disable_lru_cache is set",
-			input: func(c *Config) {
-				c.Agent.Experimental.DisableLRUCache = true
-			},
-			logOptions: func(t *testing.T) []log.Option {
-				return []log.Option{
-					func(logger *log.Logger) error {
-						logger.SetOutput(io.Discard)
-						hook := test.NewLocal(logger.Logger)
-						t.Cleanup(func() {
-							spiretest.AssertLogsContainEntries(t, hook.AllEntries(), []spiretest.LogEntry{
-								{
-									Level: logrus.WarnLevel,
-									Message: "The `x509_svid_cache_max_size` and `disable_lru_cache` " +
-										"configurations are deprecated. They will be removed in a future release.",
-								},
-							})
-						})
-						return nil
-					},
-				}
-			},
-			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) {

doc/spire_agent.md

@@ -75,8 +75,6 @@ This may be useful for templating configuration files, for example across differ
 |:------------------------------|--------------------------------------------------------------------------------------|-------------------------|
 | `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 (deprecated)     | 1000                    |
-| `disable_lru_cache`           | Reverts back to use the SPIRE Agent non-LRU cache for storing SVIDs (deprecated)     | false                   |
 | `use_sync_authorized_entries` | Use SyncAuthorizedEntries API for periodically synchronization of authorized entries | false                   |
 
 ### Initial trust bundle configuration
@@ -387,7 +385,7 @@ There are two ways the trusted delegate workload can request SVIDs for other wor
   In this approach, the trusted delegate workload is entirely responsible for attesting the other workload and building the attested selectors.
   When those selectors are presented to the SPIRE Agent, the SPIRE Agent will simply return SVIDs for any workload registration entries that match the provided selectors.
   No other checks or attestations will be performed by the SPIRE Agent.
-  
+
 1. By obtaining a PID for the other workload, and providing that PID to the SPIRE Agent over the Delegated Identity API.
    In this approach, the SPIRE Agent will do attestation for the provided PID, build the attested selectors, and return SVIDs for any workload registration entries that match the selectors the SPIRE Agent attested from that PID.
    This differs from the previous approach in that the SPIRE Agent itself (not the trusted delegate) handles the attestation of the other workload.

pkg/agent/agent.go

@@ -275,8 +275,6 @@ func (a *Agent) newManager(ctx context.Context, sto storage.Storage, cat catalog
 		Storage:                  sto,
 		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),

pkg/agent/api/delegatedidentity/v1/service_test.go

@@ -9,6 +9,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/andres-erbsen/clock"
 	"github.com/sirupsen/logrus"
 	"github.com/sirupsen/logrus/hooks/test"
 	"github.com/spiffe/go-spiffe/v2/bundle/spiffebundle"
@@ -993,9 +994,9 @@ func (m *FakeManager) SubscribeToBundleChanges() *cache.BundleStream {
 	return myCache.BundleCache.SubscribeToBundleChanges()
 }
 
-func newTestCache() *cache.Cache {
+func newTestCache() *cache.LRUCache {
 	log, _ := test.NewNullLogger()
-	return cache.New(log, trustDomain1, bundle1, telemetry.Blackhole{})
+	return cache.NewLRUCache(log, trustDomain1, bundle1, telemetry.Blackhole{}, clock.New())
 }
 
 func generateSubscribeToX509SVIDMetrics() []fakemetrics.MetricItem {

pkg/agent/config.go

@@ -66,12 +66,6 @@ type Config struct {
 	// is used to sync entries from the server.
 	UseSyncAuthorizedEntries bool
 
-	// 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

pkg/agent/manager/cache/bundle_cache.go

@@ -2,9 +2,12 @@ package cache
 
 import (
 	"github.com/imkira/go-observer"
+	"github.com/spiffe/go-spiffe/v2/bundle/spiffebundle"
 	"github.com/spiffe/go-spiffe/v2/spiffeid"
 )
 
+type Bundle = spiffebundle.Bundle
+
 type BundleCache struct {
 	trustDomain spiffeid.TrustDomain
 	bundles     observer.Property