Mark LRU Cache disablement and sizing as deprecated (#5150)

* Mark LRU Cache disablement and sizing as deprecated Signed-off-by: amoore877 <andrew.s.moore@uber.com> * correct placement of accidentally moved line Signed-off-by: amoore877 <andrew.s.moore@uber.com> --------- Signed-off-by: amoore877 <andrew.s.moore@uber.com> Co-authored-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
spiffe · May 31, 2024 · 70fe7a7 · 70fe7a7
1 parent 4e7f835
commit 70fe7a7
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 15 deletions.
diff --git a/cmd/spire-agent/cli/run/run.go b/cmd/spire-agent/cli/run/run.go
@@ -470,16 +470,7 @@ func NewAgentConfig(c *Config, logOptions []log.Option, allowUnknownConfig bool)
 		}
 	}
 
-	if c.Agent.Experimental.X509SVIDCacheMaxSize < 0 {
-		return nil, errors.New("x509_svid_cache_max_size should not be negative")
-	}
 	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)
@@ -510,6 +501,19 @@ 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

diff --git a/cmd/spire-agent/cli/run/run_test.go b/cmd/spire-agent/cli/run/run_test.go
@@ -902,6 +902,24 @@ func TestNewAgentConfig(t *testing.T) {
 			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)
 			},
@@ -938,6 +956,24 @@ func TestNewAgentConfig(t *testing.T) {
 			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)
 			},

diff --git a/doc/spire_agent.md b/doc/spire_agent.md
@@ -72,12 +72,12 @@ This may be useful for templating configuration files, for example across differ
 | `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) |                                  |
 | `disable_reattest_to_renew`       | Allow agent to renew certificate when it expires rather than reattest                                                                                                                                                                             | false                            |
 
-| 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                   |
+| 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 (deprecated)   | 1000                    |
+| `disable_lru_cache`        | Reverts back to use the SPIRE Agent non-LRU cache for storing SVIDs (deprecated)   | false                   |
 
 ### Initial trust bundle configuration