Merge 6a55a361a3 into 3f8d4f7257

The cleanup routine test used time.Sleep() without actually verifying
the goroutine stopped. Added a done channel to provide deterministic
verification of goroutine termination.

Signed-off-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>

go.mod

@@ -16,13 +16,13 @@ require (
 	github.com/onsi/gomega v1.38.3
 	github.com/spf13/cobra v1.10.2
 	github.com/spf13/viper v1.21.0
-	google.golang.org/grpc v1.77.0
+	google.golang.org/grpc v1.78.0
 	gopkg.in/yaml.v3 v3.0.1
 	k8s.io/api v0.35.0
 	k8s.io/apiextensions-apiserver v0.35.0
 	k8s.io/apimachinery v0.35.0
 	k8s.io/client-go v0.35.0
-	k8s.io/utils v0.0.0-20251220205832-9d40a56c1308
+	k8s.io/utils v0.0.0-20251222233032-718f0e51e6d2
 	sigs.k8s.io/controller-runtime v0.22.4
 	sigs.k8s.io/kustomize/api v0.21.0
 	sigs.k8s.io/kustomize/kyaml v0.21.0
@@ -125,8 +125,8 @@ require (
 	golang.org/x/time v0.13.0 // indirect
 	golang.org/x/tools v0.38.0 // indirect
 	gomodules.xyz/jsonpatch/v2 v2.5.0 // indirect
-	google.golang.org/genproto/googleapis/api v0.0.0-20251022142026-3a174f9686a8 // indirect
-	google.golang.org/genproto/googleapis/rpc v0.0.0-20251022142026-3a174f9686a8 // indirect
+	google.golang.org/genproto/googleapis/api v0.0.0-20251029180050-ab9386a59fda // indirect
+	google.golang.org/genproto/googleapis/rpc v0.0.0-20251029180050-ab9386a59fda // indirect
 	google.golang.org/protobuf v1.36.10 // indirect
 	gopkg.in/evanphx/json-patch.v4 v4.13.0 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect

go.sum

@@ -289,12 +289,12 @@ gomodules.xyz/jsonpatch/v2 v2.5.0 h1:JELs8RLM12qJGXU4u/TO3V25KW8GreMKl9pdkk14RM0
 gomodules.xyz/jsonpatch/v2 v2.5.0/go.mod h1:AH3dM2RI6uoBZxn3LVrfvJ3E0/9dG4cSrbuBJT4moAY=
 gonum.org/v1/gonum v0.16.0 h1:5+ul4Swaf3ESvrOnidPp4GZbzf0mxVQpDCYUQE7OJfk=
 gonum.org/v1/gonum v0.16.0/go.mod h1:fef3am4MQ93R2HHpKnLk4/Tbh/s0+wqD5nfa6Pnwy4E=
-google.golang.org/genproto/googleapis/api v0.0.0-20251022142026-3a174f9686a8 h1:mepRgnBZa07I4TRuomDE4sTIYieg/osKmzIf4USdWS4=
-google.golang.org/genproto/googleapis/api v0.0.0-20251022142026-3a174f9686a8/go.mod h1:fDMmzKV90WSg1NbozdqrE64fkuTv6mlq2zxo9ad+3yo=
-google.golang.org/genproto/googleapis/rpc v0.0.0-20251022142026-3a174f9686a8 h1:M1rk8KBnUsBDg1oPGHNCxG4vc1f49epmTO7xscSajMk=
-google.golang.org/genproto/googleapis/rpc v0.0.0-20251022142026-3a174f9686a8/go.mod h1:7i2o+ce6H/6BluujYR+kqX3GKH+dChPTQU19wjRPiGk=
-google.golang.org/grpc v1.77.0 h1:wVVY6/8cGA6vvffn+wWK5ToddbgdU3d8MNENr4evgXM=
-google.golang.org/grpc v1.77.0/go.mod h1:z0BY1iVj0q8E1uSQCjL9cppRj+gnZjzDnzV0dHhrNig=
+google.golang.org/genproto/googleapis/api v0.0.0-20251029180050-ab9386a59fda h1:+2XxjfsAu6vqFxwGBRcHiMaDCuZiqXGDUDVWVtrFAnE=
+google.golang.org/genproto/googleapis/api v0.0.0-20251029180050-ab9386a59fda/go.mod h1:fDMmzKV90WSg1NbozdqrE64fkuTv6mlq2zxo9ad+3yo=
+google.golang.org/genproto/googleapis/rpc v0.0.0-20251029180050-ab9386a59fda h1:i/Q+bfisr7gq6feoJnS/DlpdwEL4ihp41fvRiM3Ork0=
+google.golang.org/genproto/googleapis/rpc v0.0.0-20251029180050-ab9386a59fda/go.mod h1:7i2o+ce6H/6BluujYR+kqX3GKH+dChPTQU19wjRPiGk=
+google.golang.org/grpc v1.78.0 h1:K1XZG/yGDJnzMdd/uZHAkVqJE+xIDOcmdSFZkBUicNc=
+google.golang.org/grpc v1.78.0/go.mod h1:I47qjTo4OKbMkjA/aOOwxDIiPSBofUtQUI5EfpWvW7U=
 google.golang.org/protobuf v1.36.10 h1:AYd7cD/uASjIL6Q9LiTjz8JLcrh/88q5UObnmY3aOOE=
 google.golang.org/protobuf v1.36.10/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
@@ -324,8 +324,8 @@ k8s.io/klog/v2 v2.130.1 h1:n9Xl7H1Xvksem4KFG4PYbdQCQxqc/tTUyrgXaOhHSzk=
 k8s.io/klog/v2 v2.130.1/go.mod h1:3Jpz1GvMt720eyJH1ckRHK1EDfpxISzJ7I9OYgaDtPE=
 k8s.io/kube-openapi v0.0.0-20250910181357-589584f1c912 h1:Y3gxNAuB0OBLImH611+UDZcmKS3g6CthxToOb37KgwE=
 k8s.io/kube-openapi v0.0.0-20250910181357-589584f1c912/go.mod h1:kdmbQkyfwUagLfXIad1y2TdrjPFWp2Q89B3qkRwf/pQ=
-k8s.io/utils v0.0.0-20251220205832-9d40a56c1308 h1:rk+D2uTO79bbNsICltOdVoA6mcJb0NpvBcts+ACymBQ=
-k8s.io/utils v0.0.0-20251220205832-9d40a56c1308/go.mod h1:xDxuJ0whA3d0I4mf/C4ppKHxXynQ+fxnkmQH0vTHnuk=
+k8s.io/utils v0.0.0-20251222233032-718f0e51e6d2 h1:OfgiEo21hGiwx1oJUU5MpEaeOEg6coWndBkZF/lkFuE=
+k8s.io/utils v0.0.0-20251222233032-718f0e51e6d2/go.mod h1:xDxuJ0whA3d0I4mf/C4ppKHxXynQ+fxnkmQH0vTHnuk=
 sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.33.0 h1:qPrZsv1cwQiFeieFlRqT627fVZ+tyfou/+S5S0H5ua0=
 sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.33.0/go.mod h1:Ve9uj1L+deCXFrPOk1LpFXqTg7LCFzFso6PA48q/XZw=
 sigs.k8s.io/controller-runtime v0.22.4 h1:GEjV7KV3TY8e+tJ2LCTxUTanW4z/FmNB7l327UfMq9A=

internal/cnpgi/instance/internal/client/client.go

@@ -36,6 +36,9 @@ import (
 // DefaultTTLSeconds is the default TTL in seconds of cache entries
 const DefaultTTLSeconds = 10
 
+// DefaultCleanupIntervalSeconds is the default interval in seconds for cache cleanup
+const DefaultCleanupIntervalSeconds = 30
+
 type cachedEntry struct {
 	entry         client.Object
 	fetchUnixTime int64
@@ -49,18 +52,30 @@ func (e *cachedEntry) isExpired() bool {
 // ExtendedClient is an extended client that is capable of caching multiple secrets without relying on informers
 type ExtendedClient struct {
 	client.Client
-	cachedObjects []cachedEntry
-	mux           *sync.Mutex
+	cachedObjects   []cachedEntry
+	mux             *sync.Mutex
+	cleanupInterval time.Duration
+	cleanupDone     chan struct{} // Signals when cleanup routine exits
 }
 
-// NewExtendedClient returns an extended client capable of caching secrets on the 'Get' operation
+// NewExtendedClient returns an extended client capable of caching secrets on the 'Get' operation.
+// It starts a background goroutine that periodically cleans up expired cache entries.
+// The cleanup routine will stop when the provided context is cancelled.
 func NewExtendedClient(
+	ctx context.Context,
 	baseClient client.Client,
 ) client.Client {
-	return &ExtendedClient{
-		Client: baseClient,
-		mux:    &sync.Mutex{},
+	ec := &ExtendedClient{
+		Client:          baseClient,
+		mux:             &sync.Mutex{},
+		cleanupInterval: DefaultCleanupIntervalSeconds * time.Second,
+		cleanupDone:     make(chan struct{}),
 	}
+
+	// Start the background cleanup routine
+	go ec.startCleanupRoutine(ctx)
+
+	return ec
 }
 
 func (e *ExtendedClient) isObjectCached(obj client.Object) bool {
@@ -208,3 +223,55 @@ func (e *ExtendedClient) Patch(
 
 	return e.Client.Patch(ctx, obj, patch, opts...)
 }
+
+// startCleanupRoutine periodically removes expired entries from the cache.
+// It runs until the context is cancelled.
+func (e *ExtendedClient) startCleanupRoutine(ctx context.Context) {
+	defer close(e.cleanupDone)
+	contextLogger := log.FromContext(ctx).WithName("extended_client_cleanup")
+	ticker := time.NewTicker(e.cleanupInterval)
+	defer ticker.Stop()
+
+	for {
+		select {
+		case <-ctx.Done():
+			contextLogger.Debug("stopping cache cleanup routine")
+			return
+		case <-ticker.C:
+			// Check context before cleanup to avoid unnecessary work during shutdown
+			if ctx.Err() != nil {
+				return
+			}
+			e.cleanupExpiredEntries(ctx)
+		}
+	}
+}
+
+// cleanupExpiredEntries removes all expired entries from the cache.
+func (e *ExtendedClient) cleanupExpiredEntries(ctx context.Context) {
+	contextLogger := log.FromContext(ctx).WithName("extended_client_cleanup")
+
+	e.mux.Lock()
+	defer e.mux.Unlock()
+
+	initialCount := len(e.cachedObjects)
+	if initialCount == 0 {
+		return
+	}
+
+	// Create a new slice with only non-expired entries
+	validEntries := make([]cachedEntry, 0, initialCount)
+	for _, entry := range e.cachedObjects {
+		if !entry.isExpired() {
+			validEntries = append(validEntries, entry)
+		}
+	}
+
+	removedCount := initialCount - len(validEntries)
+	if removedCount > 0 {
+		e.cachedObjects = validEntries
+		contextLogger.Debug("cleaned up expired cache entries",
+			"removedCount", removedCount,
+			"remainingCount", len(validEntries))
+	}
+}

internal/cnpgi/instance/internal/client/client_test.go

@@ -20,6 +20,7 @@ SPDX-License-Identifier: Apache-2.0
 package client
 
 import (
+	"context"
 	"time"
 
 	corev1 "k8s.io/api/core/v1"
@@ -59,6 +60,7 @@ var _ = Describe("ExtendedClient Get", func() {
 		extendedClient *ExtendedClient
 		secretInClient *corev1.Secret
 		objectStore    *barmancloudv1.ObjectStore
+		cancelCtx      context.CancelFunc
 	)
 
 	BeforeEach(func() {
@@ -79,7 +81,14 @@ var _ = Describe("ExtendedClient Get", func() {
 		baseClient := fake.NewClientBuilder().
 			WithScheme(scheme).
 			WithObjects(secretInClient, objectStore).Build()
-		extendedClient = NewExtendedClient(baseClient).(*ExtendedClient)
+		ctx, cancel := context.WithCancel(context.Background())
+		cancelCtx = cancel
+		extendedClient = NewExtendedClient(ctx, baseClient).(*ExtendedClient)
+	})
+
+	AfterEach(func() {
+		// Cancel the context to stop the cleanup routine
+		cancelCtx()
 	})
 
 	It("returns secret from cache if not expired", func(ctx SpecContext) {
@@ -164,3 +173,141 @@ var _ = Describe("ExtendedClient Get", func() {
 			Expect(objectStore.GetResourceVersion()).To(Equal("from cache"))
 		})
 })
+
+var _ = Describe("ExtendedClient Cache Cleanup", func() {
+	var (
+		extendedClient *ExtendedClient
+		cancelCtx      context.CancelFunc
+	)
+
+	BeforeEach(func() {
+		baseClient := fake.NewClientBuilder().
+			WithScheme(scheme).
+			Build()
+		ctx, cancel := context.WithCancel(context.Background())
+		cancelCtx = cancel
+		extendedClient = NewExtendedClient(ctx, baseClient).(*ExtendedClient)
+	})
+
+	AfterEach(func() {
+		cancelCtx()
+	})
+
+	It("cleans up expired entries", func(ctx SpecContext) {
+		// Add some expired entries
+		expiredSecret1 := &corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: "default",
+				Name:      "expired-secret-1",
+			},
+		}
+		expiredSecret2 := &corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: "default",
+				Name:      "expired-secret-2",
+			},
+		}
+		validSecret := &corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: "default",
+				Name:      "valid-secret",
+			},
+		}
+
+		// Add expired entries (2 minutes ago)
+		addToCache(extendedClient, expiredSecret1, time.Now().Add(-2*time.Minute).Unix())
+		addToCache(extendedClient, expiredSecret2, time.Now().Add(-2*time.Minute).Unix())
+		// Add valid entry (just now)
+		addToCache(extendedClient, validSecret, time.Now().Unix())
+
+		Expect(extendedClient.cachedObjects).To(HaveLen(3))
+
+		// Trigger cleanup
+		extendedClient.cleanupExpiredEntries(ctx)
+
+		// Only the valid entry should remain
+		Expect(extendedClient.cachedObjects).To(HaveLen(1))
+		Expect(extendedClient.cachedObjects[0].entry.GetName()).To(Equal("valid-secret"))
+	})
+
+	It("does nothing when all entries are valid", func(ctx SpecContext) {
+		validSecret1 := &corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: "default",
+				Name:      "valid-secret-1",
+			},
+		}
+		validSecret2 := &corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: "default",
+				Name:      "valid-secret-2",
+			},
+		}
+
+		addToCache(extendedClient, validSecret1, time.Now().Unix())
+		addToCache(extendedClient, validSecret2, time.Now().Unix())
+
+		Expect(extendedClient.cachedObjects).To(HaveLen(2))
+
+		// Trigger cleanup
+		extendedClient.cleanupExpiredEntries(ctx)
+
+		// Both entries should remain
+		Expect(extendedClient.cachedObjects).To(HaveLen(2))
+	})
+
+	It("does nothing when cache is empty", func(ctx SpecContext) {
+		Expect(extendedClient.cachedObjects).To(BeEmpty())
+
+		// Trigger cleanup
+		extendedClient.cleanupExpiredEntries(ctx)
+
+		Expect(extendedClient.cachedObjects).To(BeEmpty())
+	})
+
+	It("removes all entries when all are expired", func(ctx SpecContext) {
+		expiredSecret1 := &corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: "default",
+				Name:      "expired-secret-1",
+			},
+		}
+		expiredSecret2 := &corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: "default",
+				Name:      "expired-secret-2",
+			},
+		}
+
+		addToCache(extendedClient, expiredSecret1, time.Now().Add(-2*time.Minute).Unix())
+		addToCache(extendedClient, expiredSecret2, time.Now().Add(-2*time.Minute).Unix())
+
+		Expect(extendedClient.cachedObjects).To(HaveLen(2))
+
+		// Trigger cleanup
+		extendedClient.cleanupExpiredEntries(ctx)
+
+		Expect(extendedClient.cachedObjects).To(BeEmpty())
+	})
+
+	It("stops cleanup routine when context is cancelled", func() {
+		// Create a new client with a short cleanup interval for testing
+		baseClient := fake.NewClientBuilder().
+			WithScheme(scheme).
+			Build()
+		ctx, cancel := context.WithCancel(context.Background())
+		ec := NewExtendedClient(ctx, baseClient).(*ExtendedClient)
+		ec.cleanupInterval = 10 * time.Millisecond
+
+		// Cancel the context immediately
+		cancel()
+
+		// Verify the cleanup routine actually stops by waiting for the done channel
+		select {
+		case <-ec.cleanupDone:
+			// Success: cleanup routine exited as expected
+		case <-time.After(1 * time.Second):
+			Fail("cleanup routine did not stop within timeout")
+		}
+	})
+})

internal/cnpgi/instance/manager.go

@@ -84,7 +84,7 @@ func Start(ctx context.Context) error {
 		return err
 	}
 
-	customCacheClient := extendedclient.NewExtendedClient(mgr.GetClient())
+	customCacheClient := extendedclient.NewExtendedClient(ctx, mgr.GetClient())
 
 	if err := mgr.Add(&CNPGI{
 		Client:         customCacheClient,