From cf86b9a528d2e31762ac8c14a92846d8e2b3ca46 Mon Sep 17 00:00:00 2001
From: Pascal Fischer <32096965+pascal-fischer@users.noreply.github.com>
Date: Fri, 10 Apr 2026 17:07:27 +0200
Subject: [PATCH] [management] enable access log cleanup by default (#5842)

---
 combined/cmd/config.go                             | 12 ++++++++----
 .../reverseproxy/accesslogs/manager/manager.go     | 14 ++++++++++++--
 .../accesslogs/manager/manager_test.go             |  4 ++--
 management/internals/server/config/config.go       |  2 +-
 4 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/combined/cmd/config.go b/combined/cmd/config.go
index 85664d0d2..ce4df8394 100644
--- a/combined/cmd/config.go
+++ b/combined/cmd/config.go
@@ -179,9 +179,11 @@ type StoreConfig struct {
 
 // ReverseProxyConfig contains reverse proxy settings
 type ReverseProxyConfig struct {
-	TrustedHTTPProxies      []string `yaml:"trustedHTTPProxies"`
-	TrustedHTTPProxiesCount uint     `yaml:"trustedHTTPProxiesCount"`
-	TrustedPeers            []string `yaml:"trustedPeers"`
+	TrustedHTTPProxies            []string `yaml:"trustedHTTPProxies"`
+	TrustedHTTPProxiesCount       uint     `yaml:"trustedHTTPProxiesCount"`
+	TrustedPeers                  []string `yaml:"trustedPeers"`
+	AccessLogRetentionDays        int      `yaml:"accessLogRetentionDays"`
+	AccessLogCleanupIntervalHours int      `yaml:"accessLogCleanupIntervalHours"`
 }
 
 // DefaultConfig returns a CombinedConfig with default values
@@ -645,7 +647,9 @@ func (c *CombinedConfig) ToManagementConfig() (*nbconfig.Config, error) {
 
 	// Build reverse proxy config
 	reverseProxy := nbconfig.ReverseProxy{
-		TrustedHTTPProxiesCount: mgmt.ReverseProxy.TrustedHTTPProxiesCount,
+		TrustedHTTPProxiesCount:       mgmt.ReverseProxy.TrustedHTTPProxiesCount,
+		AccessLogRetentionDays:        mgmt.ReverseProxy.AccessLogRetentionDays,
+		AccessLogCleanupIntervalHours: mgmt.ReverseProxy.AccessLogCleanupIntervalHours,
 	}
 	for _, p := range mgmt.ReverseProxy.TrustedHTTPProxies {
 		if prefix, err := netip.ParsePrefix(p); err == nil {
diff --git a/management/internals/modules/reverseproxy/accesslogs/manager/manager.go b/management/internals/modules/reverseproxy/accesslogs/manager/manager.go
index e8d0ce763..59d7704eb 100644
--- a/management/internals/modules/reverseproxy/accesslogs/manager/manager.go
+++ b/management/internals/modules/reverseproxy/accesslogs/manager/manager.go
@@ -106,13 +106,23 @@ func (m *managerImpl) CleanupOldAccessLogs(ctx context.Context, retentionDays in
 
 // StartPeriodicCleanup starts a background goroutine that periodically cleans up old access logs
 func (m *managerImpl) StartPeriodicCleanup(ctx context.Context, retentionDays, cleanupIntervalHours int) {
-	if retentionDays <= 0 {
-		log.WithContext(ctx).Debug("periodic access log cleanup disabled: retention days is 0 or negative")
+	if retentionDays < 0 {
+		log.WithContext(ctx).Debug("periodic access log cleanup disabled: retention days is negative")
 		return
 	}
 
+	if retentionDays == 0 {
+		retentionDays = 7
+		log.WithContext(ctx).Debugf("no retention days specified for access log cleanup, defaulting to %d days", retentionDays)
+	} else {
+		log.WithContext(ctx).Debugf("access log retention period set to %d days", retentionDays)
+	}
+
 	if cleanupIntervalHours <= 0 {
 		cleanupIntervalHours = 24
+		log.WithContext(ctx).Debugf("no cleanup interval specified for access log cleanup, defaulting to %d hours", cleanupIntervalHours)
+	} else {
+		log.WithContext(ctx).Debugf("access log cleanup interval set to %d hours", cleanupIntervalHours)
 	}
 
 	cleanupCtx, cancel := context.WithCancel(ctx)
diff --git a/management/internals/modules/reverseproxy/accesslogs/manager/manager_test.go b/management/internals/modules/reverseproxy/accesslogs/manager/manager_test.go
index 8fadef85f..11bf60829 100644
--- a/management/internals/modules/reverseproxy/accesslogs/manager/manager_test.go
+++ b/management/internals/modules/reverseproxy/accesslogs/manager/manager_test.go
@@ -121,7 +121,7 @@ func TestCleanupWithExactBoundary(t *testing.T) {
 }
 
 func TestStartPeriodicCleanup(t *testing.T) {
-	t.Run("periodic cleanup disabled with zero retention", func(t *testing.T) {
+	t.Run("periodic cleanup disabled with negative retention", func(t *testing.T) {
 		ctrl := gomock.NewController(t)
 		defer ctrl.Finish()
 
@@ -135,7 +135,7 @@ func TestStartPeriodicCleanup(t *testing.T) {
 		ctx, cancel := context.WithCancel(context.Background())
 		defer cancel()
 
-		manager.StartPeriodicCleanup(ctx, 0, 1)
+		manager.StartPeriodicCleanup(ctx, -1, 1)
 
 		time.Sleep(100 * time.Millisecond)
 
diff --git a/management/internals/server/config/config.go b/management/internals/server/config/config.go
index 0ba393263..fb9c842b7 100644
--- a/management/internals/server/config/config.go
+++ b/management/internals/server/config/config.go
@@ -203,7 +203,7 @@ type ReverseProxy struct {
 
 	// AccessLogRetentionDays specifies the number of days to retain access logs.
 	// Logs older than this duration will be automatically deleted during cleanup.
-	// A value of 0 or negative means logs are kept indefinitely (no cleanup).
+	// A value of 0 will default to 7 days. Negative means logs are kept indefinitely (no cleanup).
 	AccessLogRetentionDays int
 
 	// AccessLogCleanupIntervalHours specifies how often (in hours) to run the cleanup routine.