From cdbf58fedf92a3536f9efecc6999a997d53b7185 Mon Sep 17 00:00:00 2001
From: Andreas Auernhammer <github@aead.dev>
Date: Fri, 12 Apr 2024 15:50:00 +0200
Subject: [PATCH] set cache default values as documented (#460)

This commit fixes a two bugs in the cache configuration logic.

  1. KES now applies a default cache configuration as documented here:
     https://github.com/minio/kes/blob/master/server-config.yaml#L147

  2. KES now removes entry from the offline cache if no offline cache
     expiry is specified. Before, KES used to not clear the cache if
     no offline cache configuration has been specified (docs claim that
     KES disables offline caching without explicit configuration).

Signed-off-by: Andreas Auernhammer <github@aead.dev>
---
 cmd/kes/server.go | 25 +++++++++++++++++++++++--
 keystore.go       |  7 ++++---
 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/cmd/kes/server.go b/cmd/kes/server.go
index 626fd3ad..1e640766 100644
--- a/cmd/kes/server.go
+++ b/cmd/kes/server.go
@@ -181,6 +181,7 @@ func startServer(addrFlag, configFlag string) error {
 	defer conf.Keys.Close()
 
 	srv := &kes.Server{}
+	conf.Cache = configureCache(conf.Cache)
 	if rawConfig.Log != nil {
 		srv.ErrLevel.Set(rawConfig.Log.ErrLevel)
 		srv.AuditLevel.Set(rawConfig.Log.AuditLevel)
@@ -242,6 +243,7 @@ func startServer(addrFlag, configFlag string) error {
 					fmt.Fprintf(os.Stderr, "Failed to reload server config: %v\n", err)
 					continue
 				}
+				config.Cache = configureCache(config.Cache)
 
 				closer, err := srv.Update(config)
 				if err != nil {
@@ -345,8 +347,12 @@ func startDevServer(addr string) error {
 	conf := &kes.Config{
 		Admin: apiKey.Identity(),
 		TLS:   tlsConf,
-		Cache: &kes.CacheConfig{},
-		Keys:  &kes.MemKeyStore{},
+		Cache: &kes.CacheConfig{
+			Expiry:        5 * time.Minute,
+			ExpiryUnused:  30 * time.Second,
+			ExpiryOffline: 0,
+		},
+		Keys: &kes.MemKeyStore{},
 	}
 	srv := &kes.Server{}
 
@@ -382,6 +388,21 @@ func startDevServer(addr string) error {
 	return nil
 }
 
+// configureCache sets default values for each cache config option
+// as documented in: https://github.com/minio/kes/blob/master/server-config.yaml
+func configureCache(c *kes.CacheConfig) *kes.CacheConfig {
+	if c == nil {
+		c = &kes.CacheConfig{}
+	}
+	if c.Expiry == 0 {
+		c.Expiry = 5 * time.Minute
+	}
+	if c.ExpiryUnused == 0 {
+		c.Expiry = 30 * time.Second
+	}
+	return c
+}
+
 // lookupInterfaceIPs returns a list of IP addrs for which a listener
 // listening on listenerIP is reachable. If listenerIP is not
 // unspecified (0.0.0.0) it returns []net.IP{listenerIP}.
diff --git a/keystore.go b/keystore.go
index 95ac7b5f..6dd0aa6d 100644
--- a/keystore.go
+++ b/keystore.go
@@ -172,13 +172,14 @@ func newCache(store KeyStore, conf *CacheConfig) *keyCache {
 		stop:  stop,
 	}
 
+	expiryOffline := conf.ExpiryOffline
 	go c.gc(ctx, conf.Expiry, func() {
-		if offline := c.offline.Load(); !offline {
+		if offline := c.offline.Load(); !offline || expiryOffline <= 0 {
 			c.cache.DeleteAll()
 		}
 	})
 	go c.gc(ctx, conf.ExpiryUnused/2, func() {
-		if offline := c.offline.Load(); !offline {
+		if offline := c.offline.Load(); !offline || conf.ExpiryOffline <= 0 {
 			c.cache.DeleteFunc(func(_ string, e *cacheEntry) bool {
 				// We remove an entry if it isn't marked as used.
 				// We also change all other entries to unused such
@@ -195,7 +196,7 @@ func newCache(store KeyStore, conf *CacheConfig) *keyCache {
 		}
 	})
 	go c.gc(ctx, conf.ExpiryOffline, func() {
-		if offline := c.offline.Load(); offline {
+		if offline := c.offline.Load(); offline && expiryOffline > 0 {
 			c.cache.DeleteAll()
 		}
 	})