Browse Source

feat(doppler): add ETag-based caching (#5997)

Co-authored-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
Co-authored-by: Gergely Bräutigam <gergely.brautigam@sap.com>
Mike Sellitto 2 weeks ago
parent
commit
8d8f9d7062

+ 128 - 0
providers/v1/doppler/cache.go

@@ -0,0 +1,128 @@
+/*
+Copyright © The ESO Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    https://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package doppler
+
+import (
+	"sync"
+
+	dclient "github.com/external-secrets/external-secrets/providers/v1/doppler/client"
+	"github.com/external-secrets/external-secrets/runtime/cache"
+)
+
+type cacheEntry struct {
+	etag    string
+	secrets dclient.Secrets
+	body    []byte
+}
+
+// Constant version because entries are invalidated explicitly on mutations.
+const etagCacheVersion = ""
+
+type secretsCache struct {
+	cache  *cache.Cache[*cacheEntry]
+	keys   map[string]map[cache.Key]struct{}
+	keysMu sync.RWMutex
+}
+
+func newSecretsCache(size int) *secretsCache {
+	if size <= 0 {
+		return nil
+	}
+	return &secretsCache{
+		cache: cache.Must(size, func(_ *cacheEntry) {
+			// No cleanup needed on eviction — entries are plain data with no external resources.
+		}),
+		keys: make(map[string]map[cache.Key]struct{}),
+	}
+}
+
+type storeIdentity struct {
+	namespace string
+	name      string
+	kind      string
+}
+
+func cacheKey(store storeIdentity, secretName string) cache.Key {
+	name := store.name
+	if secretName != "" {
+		name = store.name + "|" + secretName
+	}
+	return cache.Key{
+		Name:      name,
+		Namespace: store.namespace,
+		Kind:      store.kind,
+	}
+}
+
+func storeKey(store storeIdentity) string {
+	return store.namespace + "|" + store.name + "|" + store.kind
+}
+
+func (c *secretsCache) get(store storeIdentity, secretName string) (*cacheEntry, bool) {
+	if c == nil || c.cache == nil {
+		return nil, false
+	}
+	key := cacheKey(store, secretName)
+	return c.cache.Get(etagCacheVersion, key)
+}
+
+func (c *secretsCache) set(store storeIdentity, secretName string, entry *cacheEntry) {
+	if c == nil || c.cache == nil {
+		return
+	}
+	key := cacheKey(store, secretName)
+
+	c.keysMu.Lock()
+	c.cache.Add(etagCacheVersion, key, entry)
+	prefix := storeKey(store)
+	if c.keys[prefix] == nil {
+		c.keys[prefix] = make(map[cache.Key]struct{})
+	}
+	// Prune keys that have been evicted from the underlying cache to prevent unbounded growth.
+	for k := range c.keys[prefix] {
+		if !c.cache.Contains(k) {
+			delete(c.keys[prefix], k)
+		}
+	}
+	c.keys[prefix][key] = struct{}{}
+	c.keysMu.Unlock()
+}
+
+func (c *secretsCache) invalidate(store storeIdentity) {
+	if c == nil || c.cache == nil {
+		return
+	}
+
+	c.keysMu.Lock()
+	defer c.keysMu.Unlock()
+
+	prefix := storeKey(store)
+	keys, exists := c.keys[prefix]
+	if !exists {
+		return
+	}
+
+	for key := range keys {
+		if c.cache.Contains(key) {
+			c.cache.Get("__invalidate__", key) // wrong version triggers eviction
+		}
+	}
+
+	delete(c.keys, prefix)
+}
+
+var etagCache *secretsCache

+ 664 - 0
providers/v1/doppler/cache_test.go

@@ -0,0 +1,664 @@
+/*
+Copyright © The ESO Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    https://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package doppler
+
+import (
+	"bytes"
+	"context"
+	"sync"
+	"sync/atomic"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	corev1 "k8s.io/api/core/v1"
+
+	esv1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1"
+	esv1alpha1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1alpha1"
+	"github.com/external-secrets/external-secrets/providers/v1/doppler/client"
+	"github.com/external-secrets/external-secrets/providers/v1/doppler/fake"
+	"github.com/external-secrets/external-secrets/runtime/cache"
+)
+
+const testETagValue = "etag-123"
+
+// testCacheSize is used in tests to create caches with sufficient capacity.
+const testCacheSize = 100
+
+const testAPIKeyValue = "secret-value"
+const testDBPassValue = "password"
+
+// testStore is a default store identity used in tests.
+var testStore = storeIdentity{
+	namespace: "test-namespace",
+	name:      "test-store",
+	kind:      "SecretStore",
+}
+
+func TestCacheKey(t *testing.T) {
+	store := storeIdentity{namespace: "ns", name: "store", kind: "SecretStore"}
+
+	tests := []struct {
+		store      storeIdentity
+		secretName string
+		expected   cache.Key
+	}{
+		{store, "", cache.Key{Name: "store", Namespace: "ns", Kind: "SecretStore"}},
+		{store, "API_KEY", cache.Key{Name: "store|API_KEY", Namespace: "ns", Kind: "SecretStore"}},
+		{store, "DB_PASS", cache.Key{Name: "store|DB_PASS", Namespace: "ns", Kind: "SecretStore"}},
+	}
+
+	for _, tt := range tests {
+		result := cacheKey(tt.store, tt.secretName)
+		if result != tt.expected {
+			t.Errorf("cacheKey(%v, %q) = %v, want %v", tt.store, tt.secretName, result, tt.expected)
+		}
+	}
+}
+
+func TestSecretsCacheGetSet(t *testing.T) {
+	c := newSecretsCache(testCacheSize)
+
+	entry, found := c.get(testStore, "")
+	if found || entry != nil {
+		t.Error("expected empty cache to return nil, false")
+	}
+
+	testEntry := &cacheEntry{
+		etag:    "test-etag",
+		secrets: client.Secrets{"KEY": "value"},
+		body:    []byte("test body"),
+	}
+	c.set(testStore, "", testEntry)
+
+	entry, found = c.get(testStore, "")
+	if !found {
+		t.Error("expected cache hit after set")
+	}
+	if entry.etag != testEntry.etag {
+		t.Errorf("expected etag %q, got %q", testEntry.etag, entry.etag)
+	}
+	if !cmp.Equal(entry.secrets, testEntry.secrets) {
+		t.Errorf("expected secrets %v, got %v", testEntry.secrets, entry.secrets)
+	}
+
+	// Different secret name should miss
+	entry, found = c.get(testStore, "API_KEY")
+	if found || entry != nil {
+		t.Error("expected cache miss for different secret name")
+	}
+
+	// Different store should not see the entry
+	otherStore := storeIdentity{namespace: "other-ns", name: "other-store", kind: "SecretStore"}
+	entry, found = c.get(otherStore, "")
+	if found || entry != nil {
+		t.Error("expected cache miss for different store")
+	}
+}
+
+func TestSecretsCacheInvalidate(t *testing.T) {
+	c := newSecretsCache(testCacheSize)
+
+	testEntry := &cacheEntry{
+		etag:    "test-etag",
+		secrets: client.Secrets{"KEY": "value"},
+	}
+	c.set(testStore, "", testEntry)
+	c.set(testStore, "API_KEY", testEntry)
+	c.set(testStore, "DB_PASS", testEntry)
+
+	_, found := c.get(testStore, "")
+	if !found {
+		t.Error("expected cache hit before invalidate")
+	}
+	_, found = c.get(testStore, "API_KEY")
+	if !found {
+		t.Error("expected cache hit for API_KEY before invalidate")
+	}
+
+	c.invalidate(testStore)
+
+	_, found = c.get(testStore, "")
+	if found {
+		t.Error("expected cache miss after invalidate")
+	}
+	_, found = c.get(testStore, "API_KEY")
+	if found {
+		t.Error("expected cache miss for API_KEY after invalidate")
+	}
+	_, found = c.get(testStore, "DB_PASS")
+	if found {
+		t.Error("expected cache miss for DB_PASS after invalidate")
+	}
+}
+
+func TestSecretsCacheConcurrency(t *testing.T) {
+	c := newSecretsCache(testCacheSize)
+	const numGoroutines = 100
+	const numIterations = 100
+
+	var wg sync.WaitGroup
+	wg.Add(numGoroutines)
+
+	for i := range numGoroutines {
+		go func(id int) {
+			defer wg.Done()
+			for j := range numIterations {
+				entry := &cacheEntry{
+					etag:    "etag",
+					secrets: client.Secrets{"KEY": "value"},
+				}
+				c.set(testStore, "", entry)
+				c.get(testStore, "")
+				if j%10 == 0 {
+					c.invalidate(testStore)
+				}
+			}
+		}(i)
+	}
+
+	wg.Wait()
+}
+
+func TestGetAllSecretsUsesCache(t *testing.T) {
+	etagCache = newSecretsCache(testCacheSize)
+
+	fakeClient := &fake.DopplerClient{}
+
+	var callCount atomic.Int32
+	testSecrets := client.Secrets{"API_KEY": testAPIKeyValue, "DB_PASS": testDBPassValue}
+	testETag := testETagValue
+
+	fakeClient.WithSecretsFunc(func(request client.SecretsRequest) (*client.SecretsResponse, error) {
+		count := callCount.Add(1)
+
+		if request.ETag == "" {
+			return &client.SecretsResponse{
+				Modified: true,
+				Secrets:  testSecrets,
+				ETag:     testETag,
+			}, nil
+		}
+
+		if request.ETag == testETag {
+			return &client.SecretsResponse{
+				Modified: false,
+				Secrets:  nil,
+				ETag:     testETag,
+			}, nil
+		}
+
+		t.Errorf("unexpected call %d with ETag %q", count, request.ETag)
+		return nil, nil
+	})
+
+	c := &Client{
+		doppler:   fakeClient,
+		project:   "test-project",
+		config:    "test-config",
+		namespace: "test-namespace",
+		storeName: "test-store",
+		storeKind: "SecretStore",
+	}
+
+	secrets, err := c.secrets(context.Background())
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if len(secrets) != 2 {
+		t.Errorf("expected 2 secrets, got %d", len(secrets))
+	}
+	if string(secrets["API_KEY"]) != testAPIKeyValue {
+		t.Errorf("expected API_KEY=%s, got %s", testAPIKeyValue, secrets["API_KEY"])
+	}
+
+	secrets, err = c.secrets(context.Background())
+	if err != nil {
+		t.Fatalf("unexpected error on second call: %v", err)
+	}
+	if len(secrets) != 2 {
+		t.Errorf("expected 2 secrets on second call, got %d", len(secrets))
+	}
+
+	if callCount.Load() != 2 {
+		t.Errorf("expected 2 API calls, got %d", callCount.Load())
+	}
+}
+
+func TestGetSecretUsesCache(t *testing.T) {
+	etagCache = newSecretsCache(testCacheSize)
+
+	fakeClient := &fake.DopplerClient{}
+
+	var callCount atomic.Int32
+	apiKeyETag := "etag-api-key"
+	dbPassETag := "etag-db-pass"
+
+	fakeClient.WithSecretFunc(func(request client.SecretRequest) (*client.SecretResponse, error) {
+		callCount.Add(1)
+
+		secretName := request.Name
+		var expectedETag string
+		var secretValue string
+
+		switch secretName {
+		case "API_KEY":
+			expectedETag = apiKeyETag
+			secretValue = testAPIKeyValue
+		case "DB_PASS":
+			expectedETag = dbPassETag
+			secretValue = testDBPassValue
+		default:
+			t.Errorf("unexpected secret requested: %s", secretName)
+			return nil, nil
+		}
+
+		if request.ETag == expectedETag {
+			return &client.SecretResponse{Modified: false, ETag: expectedETag}, nil
+		}
+
+		return &client.SecretResponse{
+			Name:     secretName,
+			Value:    secretValue,
+			Modified: true,
+			ETag:     expectedETag,
+		}, nil
+	})
+
+	c := &Client{
+		doppler:   fakeClient,
+		project:   "test-project",
+		config:    "test-config",
+		namespace: "test-namespace",
+		storeName: "test-store",
+		storeKind: "SecretStore",
+	}
+
+	secret, err := c.GetSecret(context.Background(), esv1.ExternalSecretDataRemoteRef{Key: "API_KEY"})
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if string(secret) != testAPIKeyValue {
+		t.Errorf("expected %s, got %s", testAPIKeyValue, secret)
+	}
+
+	secret, err = c.GetSecret(context.Background(), esv1.ExternalSecretDataRemoteRef{Key: "API_KEY"})
+	if err != nil {
+		t.Fatalf("unexpected error on second call: %v", err)
+	}
+	if string(secret) != testAPIKeyValue {
+		t.Errorf("expected %s on second call, got %s", testAPIKeyValue, secret)
+	}
+
+	secret, err = c.GetSecret(context.Background(), esv1.ExternalSecretDataRemoteRef{Key: "DB_PASS"})
+	if err != nil {
+		t.Fatalf("unexpected error for DB_PASS: %v", err)
+	}
+	if string(secret) != testDBPassValue {
+		t.Errorf("expected %s, got %s", testDBPassValue, secret)
+	}
+
+	secret, err = c.GetSecret(context.Background(), esv1.ExternalSecretDataRemoteRef{Key: "DB_PASS"})
+	if err != nil {
+		t.Fatalf("unexpected error on second DB_PASS call: %v", err)
+	}
+	if string(secret) != testDBPassValue {
+		t.Errorf("expected %s on second call, got %s", testDBPassValue, secret)
+	}
+
+	if callCount.Load() != 4 {
+		t.Errorf("expected 4 API calls, got %d", callCount.Load())
+	}
+}
+
+func TestCacheInvalidationOnPushSecret(t *testing.T) {
+	etagCache = newSecretsCache(testCacheSize)
+
+	fakeClient := &fake.DopplerClient{}
+
+	var secretsCallCount atomic.Int32
+	testSecrets := client.Secrets{"API_KEY": "original-value"}
+	updatedSecrets := client.Secrets{"API_KEY": "updated-value"}
+	testETag := testETagValue
+	newETag := "etag-456"
+
+	fakeClient.WithSecretsFunc(func(request client.SecretsRequest) (*client.SecretsResponse, error) {
+		count := secretsCallCount.Add(1)
+
+		switch count {
+		case 1:
+			return &client.SecretsResponse{
+				Modified: true,
+				Secrets:  testSecrets,
+				ETag:     testETag,
+			}, nil
+		case 2:
+			if request.ETag != "" {
+				t.Errorf("expected no ETag after cache invalidation, got %q", request.ETag)
+			}
+			return &client.SecretsResponse{
+				Modified: true,
+				Secrets:  updatedSecrets,
+				ETag:     newETag,
+			}, nil
+		default:
+			t.Errorf("unexpected call %d", count)
+			return nil, nil
+		}
+	})
+
+	fakeClient.WithUpdateValue(client.UpdateSecretsRequest{
+		Secrets: client.Secrets{validRemoteKey: validSecretValue},
+		Project: "test-project",
+		Config:  "test-config",
+	}, nil)
+
+	c := &Client{
+		doppler:   fakeClient,
+		project:   "test-project",
+		config:    "test-config",
+		namespace: "test-namespace",
+		storeName: "test-store",
+		storeKind: "SecretStore",
+	}
+
+	_, err := c.secrets(context.Background())
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	storeID := c.storeIdentity()
+	entry, found := etagCache.get(storeID, "")
+	if !found {
+		t.Error("expected cache to be populated after first call")
+	}
+	if entry.etag != testETag {
+		t.Errorf("expected ETag %q, got %q", testETag, entry.etag)
+	}
+
+	secret := &corev1.Secret{
+		Data: map[string][]byte{
+			validSecretName: []byte(validSecretValue),
+		},
+	}
+	secretData := esv1alpha1.PushSecretData{
+		Match: esv1alpha1.PushSecretMatch{
+			SecretKey: validSecretName,
+			RemoteRef: esv1alpha1.PushSecretRemoteRef{
+				RemoteKey: validRemoteKey,
+			},
+		},
+	}
+	err = c.PushSecret(context.Background(), secret, secretData)
+	if err != nil {
+		t.Fatalf("unexpected error pushing secret: %v", err)
+	}
+
+	_, found = etagCache.get(storeID, "")
+	if found {
+		t.Error("expected cache to be invalidated after push")
+	}
+
+	_, err = c.secrets(context.Background())
+	if err != nil {
+		t.Fatalf("unexpected error after push: %v", err)
+	}
+
+	if secretsCallCount.Load() != 2 {
+		t.Errorf("expected 2 secrets API calls, got %d", secretsCallCount.Load())
+	}
+}
+
+func TestCacheInvalidationOnDeleteSecret(t *testing.T) {
+	etagCache = newSecretsCache(testCacheSize)
+
+	fakeClient := &fake.DopplerClient{}
+
+	testSecrets := client.Secrets{"API_KEY": "value"}
+	testETag := testETagValue
+
+	fakeClient.WithSecretsFunc(func(_ client.SecretsRequest) (*client.SecretsResponse, error) {
+		return &client.SecretsResponse{
+			Modified: true,
+			Secrets:  testSecrets,
+			ETag:     testETag,
+		}, nil
+	})
+
+	fakeClient.WithUpdateValue(client.UpdateSecretsRequest{
+		ChangeRequests: []client.Change{
+			{
+				Name:         validRemoteKey,
+				OriginalName: validRemoteKey,
+				ShouldDelete: true,
+			},
+		},
+		Project: "test-project",
+		Config:  "test-config",
+	}, nil)
+
+	c := &Client{
+		doppler:   fakeClient,
+		project:   "test-project",
+		config:    "test-config",
+		namespace: "test-namespace",
+		storeName: "test-store",
+		storeKind: "SecretStore",
+	}
+
+	_, err := c.secrets(context.Background())
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	storeID := c.storeIdentity()
+	_, found := etagCache.get(storeID, "")
+	if !found {
+		t.Error("expected cache to be populated")
+	}
+
+	remoteRef := &esv1alpha1.PushSecretRemoteRef{RemoteKey: validRemoteKey}
+	err = c.DeleteSecret(context.Background(), remoteRef)
+	if err != nil {
+		t.Fatalf("unexpected error deleting secret: %v", err)
+	}
+
+	_, found = etagCache.get(storeID, "")
+	if found {
+		t.Error("expected cache to be invalidated after delete")
+	}
+}
+
+func TestCacheWithFormat(t *testing.T) {
+	etagCache = newSecretsCache(testCacheSize)
+
+	fakeClient := &fake.DopplerClient{}
+
+	var callCount atomic.Int32
+	testBody := []byte("KEY=value\nDB_PASS=password")
+	testETag := "etag-format-123"
+
+	fakeClient.WithSecretsFunc(func(request client.SecretsRequest) (*client.SecretsResponse, error) {
+		count := callCount.Add(1)
+
+		if request.ETag == "" {
+			return &client.SecretsResponse{
+				Modified: true,
+				Body:     testBody,
+				ETag:     testETag,
+			}, nil
+		}
+
+		if request.ETag == testETag {
+			return &client.SecretsResponse{
+				Modified: false,
+				Body:     nil,
+				ETag:     testETag,
+			}, nil
+		}
+
+		t.Errorf("unexpected call %d with ETag %q", count, request.ETag)
+		return nil, nil
+	})
+
+	c := &Client{
+		doppler:   fakeClient,
+		project:   "test-project",
+		config:    "test-config",
+		format:    "env",
+		namespace: "test-namespace",
+		storeName: "test-store",
+		storeKind: "SecretStore",
+	}
+
+	secrets, err := c.secrets(context.Background())
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if !bytes.Equal(secrets["DOPPLER_SECRETS_FILE"], testBody) {
+		t.Errorf("expected body %q, got %q", testBody, secrets["DOPPLER_SECRETS_FILE"])
+	}
+
+	secrets, err = c.secrets(context.Background())
+	if err != nil {
+		t.Fatalf("unexpected error on second call: %v", err)
+	}
+	if !bytes.Equal(secrets["DOPPLER_SECRETS_FILE"], testBody) {
+		t.Errorf("expected cached body %q, got %q", testBody, secrets["DOPPLER_SECRETS_FILE"])
+	}
+
+	if callCount.Load() != 2 {
+		t.Errorf("expected 2 API calls, got %d", callCount.Load())
+	}
+}
+
+func TestSecretsCacheDisabled(t *testing.T) {
+	// When cache size is 0, caching should be disabled
+	c := newSecretsCache(0)
+	if c != nil {
+		t.Error("expected nil cache when size is 0")
+	}
+
+	// Operations on nil cache should be no-ops (not panic)
+	var nilCache *secretsCache
+	entry, found := nilCache.get(testStore, "")
+	if found || entry != nil {
+		t.Error("expected nil cache get to return nil, false")
+	}
+
+	// set should be a no-op
+	nilCache.set(testStore, "", &cacheEntry{etag: "test"})
+
+	// invalidate should be a no-op
+	nilCache.invalidate(testStore)
+}
+
+func TestDisabledCacheDoesNotCacheSecrets(t *testing.T) {
+	// Test that when cache is disabled, secrets are fetched on every call
+	etagCache = nil // Disabled cache
+
+	fakeClient := &fake.DopplerClient{}
+
+	var callCount atomic.Int32
+	testSecrets := client.Secrets{"API_KEY": testAPIKeyValue}
+
+	fakeClient.WithSecretsFunc(func(_ client.SecretsRequest) (*client.SecretsResponse, error) {
+		callCount.Add(1)
+		return &client.SecretsResponse{
+			Modified: true,
+			Secrets:  testSecrets,
+			ETag:     "etag-123",
+		}, nil
+	})
+
+	c := &Client{
+		doppler:   fakeClient,
+		project:   "test-project",
+		config:    "test-config",
+		namespace: "test-namespace",
+		storeName: "test-store",
+		storeKind: "SecretStore",
+	}
+
+	// First call
+	_, err := c.secrets(context.Background())
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	// Second call - should still fetch because cache is disabled
+	_, err = c.secrets(context.Background())
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	// Third call
+	_, err = c.secrets(context.Background())
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	// All three calls should have been made to the API
+	if callCount.Load() != 3 {
+		t.Errorf("expected 3 API calls with disabled cache, got %d", callCount.Load())
+	}
+}
+
+func TestCacheIsolationBetweenStores(t *testing.T) {
+	// Test that different stores don't share cache entries even with same project/config
+	c := newSecretsCache(testCacheSize)
+
+	storeA := storeIdentity{namespace: "ns-a", name: "store-a", kind: "SecretStore"}
+	storeB := storeIdentity{namespace: "ns-b", name: "store-b", kind: "SecretStore"}
+
+	entryA := &cacheEntry{etag: "etag-a", secrets: client.Secrets{"KEY": "value-a"}}
+	entryB := &cacheEntry{etag: "etag-b", secrets: client.Secrets{"KEY": "value-b"}}
+
+	// Both stores use same project/config but should have separate cache entries
+	c.set(storeA, "", entryA)
+	c.set(storeB, "", entryB)
+
+	// Each store should get its own entry
+	gotA, foundA := c.get(storeA, "")
+	gotB, foundB := c.get(storeB, "")
+
+	if !foundA {
+		t.Error("expected cache hit for store A")
+	}
+	if !foundB {
+		t.Error("expected cache hit for store B")
+	}
+
+	if gotA.etag != "etag-a" {
+		t.Errorf("store A got wrong etag: %q, want %q", gotA.etag, "etag-a")
+	}
+	if gotB.etag != "etag-b" {
+		t.Errorf("store B got wrong etag: %q, want %q", gotB.etag, "etag-b")
+	}
+
+	// Invalidating store A should not affect store B
+	c.invalidate(storeA)
+
+	_, foundA = c.get(storeA, "")
+	_, foundB = c.get(storeB, "")
+
+	if foundA {
+		t.Error("expected cache miss for store A after invalidation")
+	}
+	if !foundB {
+		t.Error("expected cache hit for store B after store A invalidation")
+	}
+}

+ 55 - 2
providers/v1/doppler/client.go

@@ -64,6 +64,7 @@ type Client struct {
 	store       *esv1.DopplerProvider
 	store       *esv1.DopplerProvider
 	namespace   string
 	namespace   string
 	storeKind   string
 	storeKind   string
+	storeName   string
 	oidcManager *OIDCTokenManager
 	oidcManager *OIDCTokenManager
 }
 }
 
 
@@ -129,6 +130,14 @@ func (c *Client) Validate() (esv1.ValidationResult, error) {
 	return esv1.ValidationResultReady, nil
 	return esv1.ValidationResultReady, nil
 }
 }
 
 
+func (c *Client) storeIdentity() storeIdentity {
+	return storeIdentity{
+		namespace: c.namespace,
+		name:      c.storeName,
+		kind:      c.storeKind,
+	}
+}
+
 // DeleteSecret removes a secret from Doppler.
 // DeleteSecret removes a secret from Doppler.
 func (c *Client) DeleteSecret(ctx context.Context, ref esv1.PushSecretRemoteRef) error {
 func (c *Client) DeleteSecret(ctx context.Context, ref esv1.PushSecretRemoteRef) error {
 	if err := c.refreshAuthIfNeeded(ctx); err != nil {
 	if err := c.refreshAuthIfNeeded(ctx); err != nil {
@@ -151,6 +160,8 @@ func (c *Client) DeleteSecret(ctx context.Context, ref esv1.PushSecretRemoteRef)
 		return fmt.Errorf(errDeleteSecrets, ref.GetRemoteKey(), err)
 		return fmt.Errorf(errDeleteSecrets, ref.GetRemoteKey(), err)
 	}
 	}
 
 
+	etagCache.invalidate(c.storeIdentity())
+
 	return nil
 	return nil
 }
 }
 
 
@@ -177,6 +188,8 @@ func (c *Client) PushSecret(ctx context.Context, secret *corev1.Secret, data esv
 		return fmt.Errorf(errPushSecrets, data.GetRemoteKey(), err)
 		return fmt.Errorf(errPushSecrets, data.GetRemoteKey(), err)
 	}
 	}
 
 
+	etagCache.invalidate(c.storeIdentity())
+
 	return nil
 	return nil
 }
 }
 
 
@@ -185,18 +198,35 @@ func (c *Client) GetSecret(ctx context.Context, ref esv1.ExternalSecretDataRemot
 	if err := c.refreshAuthIfNeeded(ctx); err != nil {
 	if err := c.refreshAuthIfNeeded(ctx); err != nil {
 		return nil, err
 		return nil, err
 	}
 	}
+
+	var etag string
+	cached, hasCached := etagCache.get(c.storeIdentity(), ref.Key)
+	if hasCached {
+		etag = cached.etag
+	}
+
 	request := dclient.SecretRequest{
 	request := dclient.SecretRequest{
 		Name:    ref.Key,
 		Name:    ref.Key,
 		Project: c.project,
 		Project: c.project,
 		Config:  c.config,
 		Config:  c.config,
+		ETag:    etag,
 	}
 	}
 
 
-	secret, err := c.doppler.GetSecret(request)
+	response, err := c.doppler.GetSecret(request)
 	if err != nil {
 	if err != nil {
 		return nil, fmt.Errorf(errGetSecret, ref.Key, err)
 		return nil, fmt.Errorf(errGetSecret, ref.Key, err)
 	}
 	}
 
 
-	return []byte(secret.Value), nil
+	if !response.Modified && hasCached {
+		return []byte(cached.secrets[ref.Key]), nil
+	}
+
+	etagCache.set(c.storeIdentity(), ref.Key, &cacheEntry{
+		etag:    response.ETag,
+		secrets: dclient.Secrets{response.Name: response.Value},
+	})
+
+	return []byte(response.Value), nil
 }
 }
 
 
 // GetSecretMap retrieves a secret from Doppler and returns it as a map.
 // GetSecretMap retrieves a secret from Doppler and returns it as a map.
@@ -266,11 +296,19 @@ func (c *Client) secrets(ctx context.Context) (map[string][]byte, error) {
 	if err := c.refreshAuthIfNeeded(ctx); err != nil {
 	if err := c.refreshAuthIfNeeded(ctx); err != nil {
 		return nil, err
 		return nil, err
 	}
 	}
+
+	var etag string
+	cached, hasCached := etagCache.get(c.storeIdentity(), "")
+	if hasCached {
+		etag = cached.etag
+	}
+
 	request := dclient.SecretsRequest{
 	request := dclient.SecretsRequest{
 		Project:         c.project,
 		Project:         c.project,
 		Config:          c.config,
 		Config:          c.config,
 		NameTransformer: c.nameTransformer,
 		NameTransformer: c.nameTransformer,
 		Format:          c.format,
 		Format:          c.format,
+		ETag:            etag,
 	}
 	}
 
 
 	response, err := c.doppler.GetSecrets(request)
 	response, err := c.doppler.GetSecrets(request)
@@ -278,6 +316,21 @@ func (c *Client) secrets(ctx context.Context) (map[string][]byte, error) {
 		return nil, fmt.Errorf(errGetSecrets, err)
 		return nil, fmt.Errorf(errGetSecrets, err)
 	}
 	}
 
 
+	if !response.Modified && hasCached {
+		if c.format != "" {
+			return map[string][]byte{
+				secretsDownloadFileKey: cached.body,
+			}, nil
+		}
+		return externalSecretsFormat(cached.secrets), nil
+	}
+
+	etagCache.set(c.storeIdentity(), "", &cacheEntry{
+		etag:    response.ETag,
+		secrets: response.Secrets,
+		body:    response.Body,
+	})
+
 	if c.format != "" {
 	if c.format != "" {
 		return map[string][]byte{
 		return map[string][]byte{
 			secretsDownloadFileKey: response.Body,
 			secretsDownloadFileKey: response.Body,

+ 32 - 34
providers/v1/doppler/client/client.go

@@ -76,6 +76,7 @@ type SecretRequest struct {
 	Name    string
 	Name    string
 	Project string
 	Project string
 	Config  string
 	Config  string
+	ETag    string
 }
 }
 
 
 // SecretsRequest represents a request to retrieve multiple secrets.
 // SecretsRequest represents a request to retrieve multiple secrets.
@@ -84,7 +85,7 @@ type SecretsRequest struct {
 	Config          string
 	Config          string
 	NameTransformer string
 	NameTransformer string
 	Format          string
 	Format          string
-	ETag            string // Specifying an ETag implies that the caller has implemented response caching
+	ETag            string
 }
 }
 
 
 // UpdateSecretsRequest represents a request to update secrets in Doppler.
 // UpdateSecretsRequest represents a request to update secrets in Doppler.
@@ -95,20 +96,12 @@ type UpdateSecretsRequest struct {
 	Config         string   `json:"config,omitempty"`
 	Config         string   `json:"config,omitempty"`
 }
 }
 
 
-type secretResponseBody struct {
-	Name  string `json:"name,omitempty"`
-	Value struct {
-		Raw      *string `json:"raw"`
-		Computed *string `json:"computed"`
-	} `json:"value,omitempty"`
-	Messages *[]string `json:"messages,omitempty"`
-	Success  bool      `json:"success"`
-}
-
 // SecretResponse represents the response from retrieving a secret.
 // SecretResponse represents the response from retrieving a secret.
 type SecretResponse struct {
 type SecretResponse struct {
-	Name  string
-	Value string
+	Name     string
+	Value    string
+	Modified bool
+	ETag     string
 }
 }
 
 
 // SecretsResponse represents the response from retrieving multiple secrets.
 // SecretsResponse represents the response from retrieving multiple secrets.
@@ -168,22 +161,42 @@ func (c *DopplerClient) Authenticate() error {
 
 
 // GetSecret retrieves a secret from Doppler.
 // GetSecret retrieves a secret from Doppler.
 func (c *DopplerClient) GetSecret(request SecretRequest) (*SecretResponse, error) {
 func (c *DopplerClient) GetSecret(request SecretRequest) (*SecretResponse, error) {
-	params := request.buildQueryParams(request.Name)
-	response, err := c.performRequest("/v3/configs/config/secret", "GET", headers{}, params, httpRequestBody{})
+	hdrs := headers{}
+	if request.ETag != "" {
+		hdrs["if-none-match"] = request.ETag
+	}
+
+	params := queryParams{}
+	if request.Project != "" {
+		params["project"] = request.Project
+	}
+	if request.Config != "" {
+		params["config"] = request.Config
+	}
+	params["secrets"] = request.Name
+
+	response, err := c.performRequest("/v3/configs/config/secrets/download", "GET", hdrs, params, httpRequestBody{})
 	if err != nil {
 	if err != nil {
 		return nil, err
 		return nil, err
 	}
 	}
 
 
-	var data secretResponseBody
-	if err := json.Unmarshal(response.Body, &data); err != nil {
+	if response.HTTPResponse.StatusCode == 304 {
+		return &SecretResponse{Modified: false, ETag: request.ETag}, nil
+	}
+
+	eTag := response.HTTPResponse.Header.Get("etag")
+
+	var secrets Secrets
+	if err := json.Unmarshal(response.Body, &secrets); err != nil {
 		return nil, &APIError{Err: err, Message: "unable to unmarshal secret payload", Data: string(response.Body)}
 		return nil, &APIError{Err: err, Message: "unable to unmarshal secret payload", Data: string(response.Body)}
 	}
 	}
 
 
-	if data.Value.Computed == nil {
+	value, ok := secrets[request.Name]
+	if !ok {
 		return nil, &APIError{Message: fmt.Sprintf("secret '%s' not found", request.Name)}
 		return nil, &APIError{Message: fmt.Sprintf("secret '%s' not found", request.Name)}
 	}
 	}
 
 
-	return &SecretResponse{Name: data.Name, Value: *data.Value.Computed}, nil
+	return &SecretResponse{Name: request.Name, Value: value, Modified: true, ETag: eTag}, nil
 }
 }
 
 
 // GetSecrets should only have an ETag supplied if Secrets are cached as SecretsResponse.Secrets will be nil if 304 (not modified) returned.
 // GetSecrets should only have an ETag supplied if Secrets are cached as SecretsResponse.Secrets will be nil if 304 (not modified) returned.
@@ -233,21 +246,6 @@ func (c *DopplerClient) UpdateSecrets(request UpdateSecretsRequest) error {
 	return nil
 	return nil
 }
 }
 
 
-func (r *SecretRequest) buildQueryParams(name string) queryParams {
-	params := queryParams{}
-	params["name"] = name
-
-	if r.Project != "" {
-		params["project"] = r.Project
-	}
-
-	if r.Config != "" {
-		params["config"] = r.Config
-	}
-
-	return params
-}
-
 func (r *SecretsRequest) buildQueryParams() queryParams {
 func (r *SecretsRequest) buildQueryParams() queryParams {
 	params := queryParams{}
 	params := queryParams{}
 
 

+ 13 - 1
providers/v1/doppler/doppler_test.go

@@ -164,7 +164,19 @@ func makeValidDopplerTestCaseCustom(tweaks ...func(pstc *dopplerTestCase)) *dopp
 	for _, fn := range tweaks {
 	for _, fn := range tweaks {
 		fn(pstc)
 		fn(pstc)
 	}
 	}
-	pstc.fakeClient.WithValue(pstc.request, pstc.response, pstc.apiErr)
+	pstc.fakeClient.WithSecretFunc(func(req client.SecretRequest) (*client.SecretResponse, error) {
+		if pstc.apiErr != nil {
+			return nil, pstc.apiErr
+		}
+		if pstc.response == nil {
+			return nil, &client.APIError{Message: "secret not found"}
+		}
+		return &client.SecretResponse{
+			Name:     pstc.response.Name,
+			Value:    pstc.response.Value,
+			Modified: true,
+		}, nil
+	})
 	return pstc
 	return pstc
 }
 }
 
 

+ 27 - 3
providers/v1/doppler/fake/fake.go

@@ -27,6 +27,7 @@ import (
 
 
 type DopplerClient struct {
 type DopplerClient struct {
 	getSecret     func(request client.SecretRequest) (*client.SecretResponse, error)
 	getSecret     func(request client.SecretRequest) (*client.SecretResponse, error)
+	getSecrets    func(request client.SecretsRequest) (*client.SecretsResponse, error)
 	updateSecrets func(request client.UpdateSecretsRequest) error
 	updateSecrets func(request client.UpdateSecretsRequest) error
 }
 }
 
 
@@ -42,9 +43,12 @@ func (dc *DopplerClient) GetSecret(request client.SecretRequest) (*client.Secret
 	return dc.getSecret(request)
 	return dc.getSecret(request)
 }
 }
 
 
-func (dc *DopplerClient) GetSecrets(_ client.SecretsRequest) (*client.SecretsResponse, error) {
-	// Not implemented
-	return &client.SecretsResponse{}, nil
+func (dc *DopplerClient) GetSecrets(request client.SecretsRequest) (*client.SecretsResponse, error) {
+	if dc.getSecrets != nil {
+		return dc.getSecrets(request)
+	}
+	// Default: return empty response with Modified=true (simulates fresh fetch)
+	return &client.SecretsResponse{Modified: true}, nil
 }
 }
 
 
 func (dc *DopplerClient) UpdateSecrets(request client.UpdateSecretsRequest) error {
 func (dc *DopplerClient) UpdateSecrets(request client.UpdateSecretsRequest) error {
@@ -72,3 +76,23 @@ func (dc *DopplerClient) WithUpdateValue(request client.UpdateSecretsRequest, er
 		}
 		}
 	}
 	}
 }
 }
+
+func (dc *DopplerClient) WithSecretsValue(response *client.SecretsResponse, err error) {
+	if dc != nil {
+		dc.getSecrets = func(_ client.SecretsRequest) (*client.SecretsResponse, error) {
+			return response, err
+		}
+	}
+}
+
+func (dc *DopplerClient) WithSecretsFunc(fn func(request client.SecretsRequest) (*client.SecretsResponse, error)) {
+	if dc != nil {
+		dc.getSecrets = fn
+	}
+}
+
+func (dc *DopplerClient) WithSecretFunc(fn func(request client.SecretRequest) (*client.SecretResponse, error)) {
+	if dc != nil {
+		dc.getSecret = fn
+	}
+}

+ 24 - 7
providers/v1/doppler/provider.go

@@ -50,13 +50,15 @@ var _ esv1.SecretsClient = &Client{}
 var _ esv1.Provider = &Provider{}
 var _ esv1.Provider = &Provider{}
 
 
 var (
 var (
-	enableCache      bool
-	oidcClientCache  *cache.Cache[esv1.SecretsClient]
-	defaultCacheSize = 2 << 17
+	enableCache          bool
+	oidcClientCache      *cache.Cache[esv1.SecretsClient]
+	defaultOIDCCacheSize = 2 << 17
+	defaultETagCacheSize = 1 << 14
 )
 )
 
 
 func init() {
 func init() {
 	var dopplerOIDCCacheSize int
 	var dopplerOIDCCacheSize int
+	var dopplerETagCacheSize int
 	fs := pflag.NewFlagSet("doppler", pflag.ExitOnError)
 	fs := pflag.NewFlagSet("doppler", pflag.ExitOnError)
 	fs.BoolVar(
 	fs.BoolVar(
 		&enableCache,
 		&enableCache,
@@ -67,17 +69,25 @@ func init() {
 	fs.IntVar(
 	fs.IntVar(
 		&dopplerOIDCCacheSize,
 		&dopplerOIDCCacheSize,
 		"experimental-doppler-oidc-cache-size",
 		"experimental-doppler-oidc-cache-size",
-		defaultCacheSize,
+		defaultOIDCCacheSize,
 		"Maximum size of Doppler OIDC provider cache. Set to 0 to disable caching. Only used if --experimental-enable-doppler-oidc-cache is set.")
 		"Maximum size of Doppler OIDC provider cache. Set to 0 to disable caching. Only used if --experimental-enable-doppler-oidc-cache is set.")
+	fs.IntVar(
+		&dopplerETagCacheSize,
+		"doppler-etag-cache-size",
+		defaultETagCacheSize,
+		"Maximum size of Doppler ETag-based secrets cache. Set to 0 to disable caching.")
 
 
 	feature.Register(feature.Feature{
 	feature.Register(feature.Feature{
-		Flags:      fs,
-		Initialize: func() { initCache(dopplerOIDCCacheSize) },
+		Flags: fs,
+		Initialize: func() {
+			initOIDCCache(dopplerOIDCCacheSize)
+			initETagCache(dopplerETagCacheSize)
+		},
 	})
 	})
 }
 }
 
 
 // Gating on enableCache to not enable cache out of the blue for new releases.
 // Gating on enableCache to not enable cache out of the blue for new releases.
-func initCache(cacheSize int) {
+func initOIDCCache(cacheSize int) {
 	if oidcClientCache == nil && cacheSize > 0 && enableCache {
 	if oidcClientCache == nil && cacheSize > 0 && enableCache {
 		oidcClientCache = cache.Must(cacheSize, func(_ esv1.SecretsClient) {
 		oidcClientCache = cache.Must(cacheSize, func(_ esv1.SecretsClient) {
 			// No cleanup is needed when evicting OIDC clients from cache
 			// No cleanup is needed when evicting OIDC clients from cache
@@ -85,6 +95,12 @@ func initCache(cacheSize int) {
 	}
 	}
 }
 }
 
 
+func initETagCache(cacheSize int) {
+	if etagCache == nil {
+		etagCache = newSecretsCache(cacheSize)
+	}
+}
+
 // Capabilities returns the provider's supported capabilities.
 // Capabilities returns the provider's supported capabilities.
 func (p *Provider) Capabilities() esv1.SecretStoreCapabilities {
 func (p *Provider) Capabilities() esv1.SecretStoreCapabilities {
 	return esv1.SecretStoreReadOnly
 	return esv1.SecretStoreReadOnly
@@ -119,6 +135,7 @@ func (p *Provider) NewClient(ctx context.Context, store esv1.GenericStore, kube
 		store:     dopplerStoreSpec,
 		store:     dopplerStoreSpec,
 		namespace: namespace,
 		namespace: namespace,
 		storeKind: store.GetObjectKind().GroupVersionKind().Kind,
 		storeKind: store.GetObjectKind().GroupVersionKind().Kind,
+		storeName: store.GetObjectMeta().Name,
 	}
 	}
 
 
 	if err := p.setupClientAuth(ctx, client, dopplerStoreSpec, store, namespace); err != nil {
 	if err := p.setupClientAuth(ctx, client, dopplerStoreSpec, store, namespace); err != nil {