fix: add missing metrics and fundamentally fix the caching logic (#5894)

providers/v1/onepasswordsdk/client.go

@@ -18,6 +18,7 @@ limitations under the License.
 package onepasswordsdk
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
 	"errors"
@@ -29,7 +30,9 @@ import (
 	"k8s.io/kube-openapi/pkg/validation/strfmt"
 
 	esv1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1"
+	"github.com/external-secrets/external-secrets/runtime/constants"
 	"github.com/external-secrets/external-secrets/runtime/esutils/metadata"
+	"github.com/external-secrets/external-secrets/runtime/metrics"
 )
 
 const (
@@ -37,6 +40,8 @@ const (
 	filePrefix              = "file"
 	prefixSplitter          = "/"
 	errExpectedOneFieldMsgF = "found more than 1 fields with title '%s' in '%s', got %d"
+	itemCachePrefix         = "item:"
+	fileCachePrefix         = "file:"
 )
 
 // ErrKeyNotFound is returned when a key is not found in the 1Password Vaults.
@@ -49,7 +54,7 @@ type PushSecretMetadataSpec struct {
 
 // GetSecret returns a single secret from 1Password provider.
 // Follows syntax is used for the ref key: https://developer.1password.com/docs/cli/secret-reference-syntax/
-func (p *Provider) GetSecret(ctx context.Context, ref esv1.ExternalSecretDataRemoteRef) ([]byte, error) {
+func (p *SecretsClient) GetSecret(ctx context.Context, ref esv1.ExternalSecretDataRemoteRef) ([]byte, error) {
 	if ref.Version != "" {
 		return nil, errors.New(errVersionNotImplemented)
 	}
@@ -60,6 +65,7 @@ func (p *Provider) GetSecret(ctx context.Context, ref esv1.ExternalSecretDataRem
 	}
 
 	secret, err := p.client.Secrets().Resolve(ctx, key)
+	metrics.ObserveAPICall(constants.ProviderOnePasswordSDK, constants.CallOnePasswordSDKResolve, err)
 	if err != nil {
 		return nil, err
 	}
@@ -71,12 +77,12 @@ func (p *Provider) GetSecret(ctx context.Context, ref esv1.ExternalSecretDataRem
 }
 
 // Close closes the client connection.
-func (p *Provider) Close(_ context.Context) error {
+func (p *SecretsClient) Close(_ context.Context) error {
 	return nil
 }
 
 // DeleteSecret implements Secret Deletion on the provider when PushSecret.spec.DeletionPolicy=Delete.
-func (p *Provider) DeleteSecret(ctx context.Context, ref esv1.PushSecretRemoteRef) error {
+func (p *SecretsClient) DeleteSecret(ctx context.Context, ref esv1.PushSecretRemoteRef) error {
 	providerItem, err := p.findItem(ctx, ref.GetRemoteKey())
 	if errors.Is(err, ErrKeyNotFound) {
 		return nil
@@ -85,35 +91,50 @@ func (p *Provider) DeleteSecret(ctx context.Context, ref esv1.PushSecretRemoteRe
 		return err
 	}
 
-	providerItem.Fields, err = deleteField(providerItem.Fields, ref.GetProperty())
+	var deleted bool
+	providerItem.Fields, deleted, err = deleteField(providerItem.Fields, ref.GetProperty())
 	if err != nil {
 		return fmt.Errorf("failed to delete fields: %w", err)
 	}
 
+	if !deleted {
+		// also invalidate the cache here, as this field might have been deleted
+		// outside ESO.
+		p.invalidateCacheByPrefix(p.constructRefKey(ref.GetRemoteKey()))
+		p.invalidateItemCache(ref.GetRemoteKey())
+
+		return nil
+	}
+
 	// There is a chance that there is an empty item left in the section like this: [{ID: Title:}].
 	if len(providerItem.Sections) == 1 && providerItem.Sections[0].ID == "" && providerItem.Sections[0].Title == "" {
 		providerItem.Sections = nil
 	}
 
 	if len(providerItem.Fields) == 0 && len(providerItem.Files) == 0 && len(providerItem.Sections) == 0 {
-		// Delete the item if there are no fields, files or sections
-		if err = p.client.Items().Delete(ctx, providerItem.VaultID, providerItem.ID); err != nil {
+		err = p.client.Items().Delete(ctx, providerItem.VaultID, providerItem.ID)
+		metrics.ObserveAPICall(constants.ProviderOnePasswordSDK, constants.CallOnePasswordSDKItemsDelete, err)
+		if err != nil {
 			return fmt.Errorf("failed to delete item: %w", err)
 		}
 		p.invalidateCacheByPrefix(p.constructRefKey(ref.GetRemoteKey()))
+		p.invalidateItemCache(ref.GetRemoteKey())
 		return nil
 	}
 
-	if _, err = p.client.Items().Put(ctx, providerItem); err != nil {
+	_, err = p.client.Items().Put(ctx, providerItem)
+	metrics.ObserveAPICall(constants.ProviderOnePasswordSDK, constants.CallOnePasswordSDKItemsPut, err)
+	if err != nil {
 		return fmt.Errorf("failed to update item: %w", err)
 	}
 
 	p.invalidateCacheByPrefix(p.constructRefKey(ref.GetRemoteKey()))
+	p.invalidateItemCache(ref.GetRemoteKey())
 
 	return nil
 }
 
-func deleteField(fields []onepassword.ItemField, title string) ([]onepassword.ItemField, error) {
+func deleteField(fields []onepassword.ItemField, title string) ([]onepassword.ItemField, bool, error) {
 	// This will always iterate over all items,
 	// but it's done to ensure that two fields with the same label
 	// exist resulting in undefined behavior
@@ -124,23 +145,23 @@ func deleteField(fields []onepassword.ItemField, title string) ([]onepassword.It
 	for _, item := range fields {
 		if item.Title == title {
 			if found {
-				return nil, fmt.Errorf("found multiple labels on item %q", title)
+				return nil, false, fmt.Errorf("found multiple labels on item %q", title)
 			}
 			found = true
 			continue
 		}
 		fieldsF = append(fieldsF, item)
 	}
-	return fieldsF, nil
+	return fieldsF, found, nil
 }
 
 // GetAllSecrets Not Implemented.
-func (p *Provider) GetAllSecrets(_ context.Context, _ esv1.ExternalSecretFind) (map[string][]byte, error) {
+func (p *SecretsClient) GetAllSecrets(_ context.Context, _ esv1.ExternalSecretFind) (map[string][]byte, error) {
 	return nil, fmt.Errorf(errOnePasswordSdkStore, errors.New(errNotImplemented))
 }
 
 // GetSecretMap returns multiple k/v pairs from the provider, for dataFrom.extract.
-func (p *Provider) GetSecretMap(ctx context.Context, ref esv1.ExternalSecretDataRemoteRef) (map[string][]byte, error) {
+func (p *SecretsClient) GetSecretMap(ctx context.Context, ref esv1.ExternalSecretDataRemoteRef) (map[string][]byte, error) {
 	if ref.Version != "" {
 		return nil, errors.New(errVersionNotImplemented)
 	}
@@ -178,7 +199,7 @@ func (p *Provider) GetSecretMap(ctx context.Context, ref esv1.ExternalSecretData
 	return result, nil
 }
 
-func (p *Provider) getFields(item onepassword.Item, property string) (map[string][]byte, error) {
+func (p *SecretsClient) getFields(item onepassword.Item, property string) (map[string][]byte, error) {
 	secretData := make(map[string][]byte)
 	for _, field := range item.Fields {
 		if property != "" && field.Title != property {
@@ -195,18 +216,26 @@ func (p *Provider) getFields(item onepassword.Item, property string) (map[string
 	return secretData, nil
 }
 
-func (p *Provider) getFiles(ctx context.Context, item onepassword.Item, property string) (map[string][]byte, error) {
+func (p *SecretsClient) getFiles(ctx context.Context, item onepassword.Item, property string) (map[string][]byte, error) {
 	secretData := make(map[string][]byte)
 	for _, file := range item.Files {
 		if property != "" && file.Attributes.Name != property {
 			continue
 		}
 
+		cacheKey := fileCachePrefix + p.vaultID + ":" + item.ID + ":" + file.FieldID + ":" + file.Attributes.Name
+		if cached, ok := p.cacheGet(cacheKey); ok {
+			secretData[file.Attributes.Name] = cached
+			continue
+		}
+
 		contents, err := p.client.Items().Files().Read(ctx, p.vaultID, file.FieldID, file.Attributes)
+		metrics.ObserveAPICall(constants.ProviderOnePasswordSDK, constants.CallOnePasswordSDKFilesRead, err)
 		if err != nil {
 			return nil, fmt.Errorf("failed to read file: %w", err)
 		}
 
+		p.cacheAdd(cacheKey, contents)
 		secretData[file.Attributes.Name] = contents
 	}
 
@@ -241,7 +270,7 @@ func getObjType(documentType onepassword.ItemCategory, property string) (string,
 }
 
 // createItem creates a new item in the first vault. If no vaults exist, it returns an error.
-func (p *Provider) createItem(ctx context.Context, val []byte, ref esv1.PushSecretData) error {
+func (p *SecretsClient) createItem(ctx context.Context, val []byte, ref esv1.PushSecretData) error {
 	mdata, err := metadata.ParseMetadataParameters[PushSecretMetadataSpec](ref.GetMetadata())
 	if err != nil {
 		return fmt.Errorf("failed to parse push secret metadata: %w", err)
@@ -257,7 +286,6 @@ func (p *Provider) createItem(ctx context.Context, val []byte, ref esv1.PushSecr
 		tags = mdata.Spec.Tags
 	}
 
-	// Create the item
 	_, err = p.client.Items().Create(ctx, onepassword.ItemCreateParams{
 		Category: onepassword.ItemCategoryServer,
 		VaultID:  p.vaultID,
@@ -267,11 +295,13 @@ func (p *Provider) createItem(ctx context.Context, val []byte, ref esv1.PushSecr
 		},
 		Tags: tags,
 	})
+	metrics.ObserveAPICall(constants.ProviderOnePasswordSDK, constants.CallOnePasswordSDKItemsCreate, err)
 	if err != nil {
 		return fmt.Errorf("failed to create item: %w", err)
 	}
 
 	p.invalidateCacheByPrefix(p.constructRefKey(ref.GetRemoteKey()))
+	p.invalidateItemCache(ref.GetRemoteKey())
 
 	return nil
 }
@@ -320,7 +350,7 @@ func generateNewItemField(title, newVal string) onepassword.ItemField {
 }
 
 // PushSecret creates or updates a secret in 1Password.
-func (p *Provider) PushSecret(ctx context.Context, secret *corev1.Secret, ref esv1.PushSecretData) error {
+func (p *SecretsClient) PushSecret(ctx context.Context, secret *corev1.Secret, ref esv1.PushSecretData) error {
 	val, ok := secret.Data[ref.GetSecretKey()]
 	if !ok {
 		return fmt.Errorf("secret %s/%s does not contain a key", secret.Namespace, secret.Name)
@@ -356,29 +386,31 @@ func (p *Provider) PushSecret(ctx context.Context, secret *corev1.Secret, ref es
 
 	providerItem.Fields, err = updateFieldValue(providerItem.Fields, label, string(val))
 	if err != nil {
-		return fmt.Errorf("failed to update field with value %s: %w", string(val), err)
+		return fmt.Errorf("failed to update field with label: %s: %w", label, err)
 	}
 
-	if _, err = p.client.Items().Put(ctx, providerItem); err != nil {
+	_, err = p.client.Items().Put(ctx, providerItem)
+	metrics.ObserveAPICall(constants.ProviderOnePasswordSDK, constants.CallOnePasswordSDKItemsPut, err)
+	if err != nil {
 		return fmt.Errorf("failed to update item: %w", err)
 	}
 
 	p.invalidateCacheByPrefix(p.constructRefKey(title))
+	p.invalidateItemCache(title)
 
 	return nil
 }
 
 // GetVault retrieves a vault by its title or UUID from 1Password.
-func (p *Provider) GetVault(ctx context.Context, titleOrUUID string) (string, error) {
+func (p *SecretsClient) GetVault(ctx context.Context, titleOrUUID string) (string, error) {
 	vaults, err := p.client.VaultsAPI.List(ctx)
+	metrics.ObserveAPICall(constants.ProviderOnePasswordSDK, constants.CallOnePasswordSDKVaultsList, err)
 	if err != nil {
 		return "", fmt.Errorf("failed to list vaults: %w", err)
 	}
 
 	for _, v := range vaults {
 		if v.Title == titleOrUUID || v.ID == titleOrUUID {
-			// cache the ID so we don't have to repeat this lookup.
-			p.vaultID = v.ID
 			return v.ID, nil
 		}
 	}
@@ -386,60 +418,92 @@ func (p *Provider) GetVault(ctx context.Context, titleOrUUID string) (string, er
 	return "", fmt.Errorf("vault %s not found", titleOrUUID)
 }
 
-func (p *Provider) findItem(ctx context.Context, name string) (onepassword.Item, error) {
-	if strfmt.IsUUID(name) {
-		return p.client.Items().Get(ctx, p.vaultID, name)
+func (p *SecretsClient) findItem(ctx context.Context, name string) (onepassword.Item, error) {
+	cacheKey := itemCachePrefix + p.vaultID + ":" + name
+	if cached, ok := p.cacheGet(cacheKey); ok {
+		var item onepassword.Item
+		if err := json.Unmarshal(cached, &item); err == nil {
+			return item, nil
+		}
 	}
 
-	items, err := p.client.Items().List(ctx, p.vaultID)
-	if err != nil {
-		return onepassword.Item{}, fmt.Errorf("failed to list items: %w", err)
-	}
+	var item onepassword.Item
+	var err error
+
+	if strfmt.IsUUID(name) {
+		item, err = p.client.Items().Get(ctx, p.vaultID, name)
+		metrics.ObserveAPICall(constants.ProviderOnePasswordSDK, constants.CallOnePasswordSDKItemsGet, err)
+		if err != nil {
+			if isNotFoundError(err) {
+				return onepassword.Item{}, ErrKeyNotFound
+			}
+			return onepassword.Item{}, err
+		}
+	} else {
+		items, err := p.client.Items().List(ctx, p.vaultID)
+		metrics.ObserveAPICall(constants.ProviderOnePasswordSDK, constants.CallOnePasswordSDKItemsList, err)
+		if err != nil {
+			return onepassword.Item{}, fmt.Errorf("failed to list items: %w", err)
+		}
 
-	// We don't stop
-	var itemUUID string
-	for _, v := range items {
-		if v.Title == name {
-			if itemUUID != "" {
-				return onepassword.Item{}, fmt.Errorf("found multiple items with name %s", name)
+		var itemUUID string
+		for _, v := range items {
+			if v.Title == name {
+				if itemUUID != "" {
+					return onepassword.Item{}, fmt.Errorf("found multiple items with name %s", name)
+				}
+				itemUUID = v.ID
 			}
-			itemUUID = v.ID
+		}
+
+		if itemUUID == "" {
+			return onepassword.Item{}, ErrKeyNotFound
+		}
+
+		item, err = p.client.Items().Get(ctx, p.vaultID, itemUUID)
+		metrics.ObserveAPICall(constants.ProviderOnePasswordSDK, constants.CallOnePasswordSDKItemsGet, err)
+		if err != nil {
+			return onepassword.Item{}, err
 		}
 	}
 
-	if itemUUID == "" {
-		return onepassword.Item{}, ErrKeyNotFound
+	if serialized, err := json.Marshal(item); err == nil {
+		p.cacheAdd(cacheKey, serialized)
 	}
 
-	return p.client.Items().Get(ctx, p.vaultID, itemUUID)
+	return item, nil
 }
 
 // SecretExists checks if a secret exists in 1Password.
-func (p *Provider) SecretExists(_ context.Context, _ esv1.PushSecretRemoteRef) (bool, error) {
+func (p *SecretsClient) SecretExists(_ context.Context, _ esv1.PushSecretRemoteRef) (bool, error) {
 	return false, fmt.Errorf("not implemented")
 }
 
 // Validate does nothing here. It would be possible to ping the SDK to prove we're healthy, but
 // since the 1password SDK rate-limit is pretty aggressive, we prefer to do nothing.
-func (p *Provider) Validate() (esv1.ValidationResult, error) {
+func (p *SecretsClient) Validate() (esv1.ValidationResult, error) {
 	return esv1.ValidationResultReady, nil
 }
 
-func (p *Provider) constructRefKey(key string) string {
+func (p *SecretsClient) constructRefKey(key string) string {
 	// remove any possible leading slashes because the vaultPrefix already contains it.
 	return p.vaultPrefix + strings.TrimPrefix(key, "/")
 }
 
 // cacheGet retrieves a value from the cache. Returns false if cache is disabled or key not found.
-func (p *Provider) cacheGet(key string) ([]byte, bool) {
+func (p *SecretsClient) cacheGet(key string) ([]byte, bool) {
 	if p.cache == nil {
 		return nil, false
 	}
-	return p.cache.Get(key)
+	v, ok := p.cache.Get(key)
+	if !ok {
+		return nil, false
+	}
+	return bytes.Clone(v), true
 }
 
 // cacheAdd stores a value in the cache. No-op if cache is disabled.
-func (p *Provider) cacheAdd(key string, value []byte) {
+func (p *SecretsClient) cacheAdd(key string, value []byte) {
 	if p.cache == nil {
 		return
 	}
@@ -451,7 +515,7 @@ func (p *Provider) cacheAdd(key string, value []byte) {
 // No-op if cache is disabled.
 // Why are we using a Prefix? Because items and properties are stored via prefixes using 1Password SDK.
 // This means when an item is deleted we delete the fields and properties that belong to the item as well.
-func (p *Provider) invalidateCacheByPrefix(prefix string) {
+func (p *SecretsClient) invalidateCacheByPrefix(prefix string) {
 	if p.cache == nil {
 		return
 	}
@@ -459,11 +523,25 @@ func (p *Provider) invalidateCacheByPrefix(prefix string) {
 	keys := p.cache.Keys()
 	for _, key := range keys {
 		if strings.HasPrefix(key, prefix) {
-			// if exact match, or ends in `/` or `|` we can remove it.
-			// this will clear all fields and properties for this entry.
 			if len(key) == len(prefix) || key[len(prefix)] == '/' || key[len(prefix)] == '|' {
 				p.cache.Remove(key)
 			}
 		}
 	}
 }
+
+// invalidateItemCache removes cached item entries for the given item name.
+// No-op if cache is disabled.
+func (p *SecretsClient) invalidateItemCache(name string) {
+	if p.cache == nil {
+		return
+	}
+
+	cacheKey := itemCachePrefix + p.vaultID + ":" + name
+	p.cache.Remove(cacheKey)
+}
+
+func isNotFoundError(err error) bool {
+	msg := strings.ToLower(err.Error())
+	return strings.Contains(msg, "couldn't be found") || strings.Contains(msg, "resource not found")
+}

providers/v1/onepasswordsdk/client_test.go

@@ -101,7 +101,7 @@ func TestProviderGetSecret(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			p := &Provider{
+			p := &SecretsClient{
 				client:      tt.client(),
 				vaultPrefix: "op://vault/",
 			}
@@ -271,7 +271,7 @@ func TestProviderGetSecretMap(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			p := &Provider{
+			p := &SecretsClient{
 				client:      tt.client(),
 				vaultPrefix: "op://vault/",
 			}
@@ -316,7 +316,7 @@ func TestProviderValidate(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			p := &Provider{
+			p := &SecretsClient{
 				client:      tt.client(),
 				vaultPrefix: tt.vaultPrefix,
 			}
@@ -420,7 +420,7 @@ func TestPushSecret(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			ctx := t.Context()
 			lister := tt.lister()
-			p := &Provider{
+			p := &SecretsClient{
 				client: &onepassword.Client{
 					SecretsAPI: fc,
 					VaultsAPI:  fc,
@@ -546,7 +546,7 @@ func TestDeleteItemField(t *testing.T) {
 		t.Run(testCase.name, func(t *testing.T) {
 			ctx := t.Context()
 			lister := testCase.lister()
-			p := &Provider{
+			p := &SecretsClient{
 				client: &onepassword.Client{
 					SecretsAPI: fc,
 					VaultsAPI:  fc,
@@ -570,7 +570,7 @@ func TestGetVault(t *testing.T) {
 		},
 	}
 
-	p := &Provider{
+	p := &SecretsClient{
 		client: &onepassword.Client{
 			VaultsAPI: fc,
 		},
@@ -781,7 +781,7 @@ func TestDeleteMultipleFieldsFromSameItem(t *testing.T) {
 			},
 		}
 
-		p := &Provider{
+		p := &SecretsClient{
 			client: &onepassword.Client{
 				SecretsAPI: fc,
 				VaultsAPI:  fc,
@@ -826,7 +826,7 @@ func TestCachingGetSecret(t *testing.T) {
 			},
 		}
 
-		p := &Provider{
+		p := &SecretsClient{
 			client: &onepassword.Client{
 				SecretsAPI: fcWithCounter,
 				VaultsAPI:  fcWithCounter.fakeClient,
@@ -859,7 +859,7 @@ func TestCachingGetSecret(t *testing.T) {
 			},
 		}
 
-		p := &Provider{
+		p := &SecretsClient{
 			client: &onepassword.Client{
 				SecretsAPI: fcWithCounter,
 				VaultsAPI:  fcWithCounter.fakeClient,
@@ -907,7 +907,7 @@ func TestCachingGetSecretMap(t *testing.T) {
 			},
 		}
 
-		p := &Provider{
+		p := &SecretsClient{
 			client: &onepassword.Client{
 				SecretsAPI: fc,
 				VaultsAPI:  fc,
@@ -957,7 +957,7 @@ func TestCacheInvalidationPushSecret(t *testing.T) {
 			},
 		}
 
-		p := &Provider{
+		p := &SecretsClient{
 			client: &onepassword.Client{
 				SecretsAPI: fcWithCounter,
 				VaultsAPI:  fcWithCounter.fakeClient,
@@ -1023,7 +1023,7 @@ func TestCacheInvalidationDeleteSecret(t *testing.T) {
 			},
 		}
 
-		p := &Provider{
+		p := &SecretsClient{
 			client: &onepassword.Client{
 				SecretsAPI: fcWithCounter,
 				VaultsAPI:  fcWithCounter.fakeClient,
@@ -1058,7 +1058,7 @@ func TestCacheInvalidationDeleteSecret(t *testing.T) {
 
 func TestInvalidateCacheByPrefix(t *testing.T) {
 	t.Run("invalidates all entries with prefix", func(t *testing.T) {
-		p := &Provider{
+		p := &SecretsClient{
 			vaultPrefix: "op://vault/",
 			cache:       expirable.NewLRU[string, []byte](100, nil, time.Minute),
 		}
@@ -1084,7 +1084,7 @@ func TestInvalidateCacheByPrefix(t *testing.T) {
 	})
 
 	t.Run("handles nil cache gracefully", func(t *testing.T) {
-		p := &Provider{
+		p := &SecretsClient{
 			vaultPrefix: "op://vault/",
 			cache:       nil,
 		}
@@ -1094,7 +1094,7 @@ func TestInvalidateCacheByPrefix(t *testing.T) {
 	})
 
 	t.Run("does not invalidate entries with similar prefixes", func(t *testing.T) {
-		p := &Provider{
+		p := &SecretsClient{
 			vaultPrefix: "op://vault/",
 			cache:       expirable.NewLRU[string, []byte](100, nil, time.Minute),
 		}

providers/v1/onepasswordsdk/go.mod

@@ -50,6 +50,7 @@ require (
 	github.com/google/gnostic-models v0.7.1 // indirect
 	github.com/google/go-cmp v0.7.0 // indirect
 	github.com/google/uuid v1.6.0 // indirect
+	github.com/hashicorp/golang-lru v1.0.2 // indirect
 	github.com/huandu/xstrings v1.5.0 // indirect
 	github.com/ianlancetaylor/demangle v0.0.0-20250628045327-2d64ad6b7ec5 // indirect
 	github.com/json-iterator/go v1.1.12 // indirect

providers/v1/onepasswordsdk/go.sum

@@ -96,6 +96,8 @@ github.com/google/pprof v0.0.0-20250403155104-27863c87afa6 h1:BHT72Gu3keYf3ZEu2J
 github.com/google/pprof v0.0.0-20250403155104-27863c87afa6/go.mod h1:boTsfXsheKC2y+lKOCMpSfarhxDeIzfZG1jqGcPl3cA=
 github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=
 github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
+github.com/hashicorp/golang-lru v1.0.2 h1:dV3g9Z/unq5DpblPpw+Oqcv4dU/1omnb4Ok8iPY6p1c=
+github.com/hashicorp/golang-lru v1.0.2/go.mod h1:iADmTwqILo4mZ8BN3D2Q6+9jd8WM5uGBxy+E8yxSoD4=
 github.com/hashicorp/golang-lru/v2 v2.0.7 h1:a+bsQ5rvGLjzHuww6tVxozPZFVghXaHOwFs4luLUK2k=
 github.com/hashicorp/golang-lru/v2 v2.0.7/go.mod h1:QeFd9opnmA6QUJc5vARoKUSoFhyfM2/ZepoAG6RGpeM=
 github.com/huandu/xstrings v1.5.0 h1:2ag3IFq9ZDANvthTwTiqSSZLjDc+BedvHPAp5tJy2TI=

providers/v1/onepasswordsdk/provider.go

@@ -30,6 +30,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
 	esv1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1"
+	"github.com/external-secrets/external-secrets/runtime/cache"
 	"github.com/external-secrets/external-secrets/runtime/esutils"
 	"github.com/external-secrets/external-secrets/runtime/esutils/resolvers"
 )
@@ -46,16 +47,31 @@ const (
 	errNotImplemented                                   = "not implemented"
 )
 
-// Provider implements the External Secrets provider interface for 1Password SDK.
+// Provider contains the main cache for onepasswordsdk provider.
 type Provider struct {
+	clientCache *cache.Cache[esv1.SecretsClient]
+}
+
+// SecretsClient wraps a 1Password SDK client for a specific vault.
+type SecretsClient struct {
 	client      *onepassword.Client
 	vaultPrefix string
 	vaultID     string
-	cache       *expirable.LRU[string, []byte] // nil if caching is disabled
+	cache       *expirable.LRU[string, []byte]
 }
 
-// NewClient constructs a new secrets client based on the provided store.
+// NewClient will create a new client.
 func (p *Provider) NewClient(ctx context.Context, store esv1.GenericStore, kube client.Client, namespace string) (esv1.SecretsClient, error) {
+	key := cache.Key{
+		Name:      store.GetObjectMeta().Name,
+		Namespace: namespace,
+		Kind:      store.GetTypeMeta().Kind,
+	}
+
+	if cachedClient, ok := p.clientCache.Get(store.GetObjectMeta().ResourceVersion, key); ok {
+		return cachedClient, nil
+	}
+
 	config := store.GetSpec().Provider.OnePasswordSDK
 	serviceAccountToken, err := resolvers.SecretKeyRef(
 		ctx,
@@ -84,16 +100,16 @@ func (p *Provider) NewClient(ctx context.Context, store esv1.GenericStore, kube
 		return nil, err
 	}
 
-	provider := &Provider{
+	sc := &SecretsClient{
 		client:      c,
 		vaultPrefix: "op://" + config.Vault + "/",
 	}
 
-	vaultID, err := provider.GetVault(ctx, config.Vault)
+	vaultID, err := sc.GetVault(ctx, config.Vault)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get store ID: %w", err)
 	}
-	provider.vaultID = vaultID
+	sc.vaultID = vaultID
 
 	if config.Cache != nil {
 		ttl := 5 * time.Minute
@@ -106,10 +122,12 @@ func (p *Provider) NewClient(ctx context.Context, store esv1.GenericStore, kube
 			maxSize = config.Cache.MaxSize
 		}
 
-		provider.cache = expirable.NewLRU[string, []byte](maxSize, nil, ttl)
+		sc.cache = expirable.NewLRU[string, []byte](maxSize, nil, ttl)
 	}
 
-	return provider, nil
+	p.clientCache.Add(store.GetObjectMeta().ResourceVersion, key, sc)
+
+	return sc, nil
 }
 
 // ValidateStore validates the 1Password SDK SecretStore resource configuration.
@@ -152,7 +170,9 @@ func (p *Provider) Capabilities() esv1.SecretStoreCapabilities {
 
 // NewProvider creates a new Provider instance.
 func NewProvider() esv1.Provider {
-	return &Provider{}
+	return &Provider{
+		clientCache: cache.Must[esv1.SecretsClient](100, nil),
+	}
 }
 
 // ProviderSpec returns the provider specification for registration.

runtime/constants/constants.go

@@ -33,8 +33,8 @@ const (
 	CallAWSSMPutResourcePolicy    = "PutResourcePolicy"
 	CallAWSSMGetResourcePolicy    = "GetResourcePolicy"
 	CallAWSSMDeleteResourcePolicy = "DeleteResourcePolicy"
+	ProviderAWSPS                 = "AWS/ParameterStore"
 
-	ProviderAWSPS                = "AWS/ParameterStore"
 	CallAWSPSGetParameter        = "GetParameter"
 	CallAWSPSPutParameter        = "PutParameter"
 	CallAWSPSDeleteParameter     = "DeleteParameter"
@@ -111,6 +111,16 @@ const (
 	CallAKEYLESSSMUpdateSecretVal       = "UpdateSecretVal"
 	CallAKEYLESSSMDeleteItem            = "DeleteItem"
 
+	ProviderOnePasswordSDK        = "1Password/SDK"
+	CallOnePasswordSDKResolve     = "Resolve"
+	CallOnePasswordSDKItemsList   = "ItemsList"
+	CallOnePasswordSDKItemsGet    = "ItemsGet"
+	CallOnePasswordSDKItemsCreate = "ItemsCreate"
+	CallOnePasswordSDKItemsPut    = "ItemsPut"
+	CallOnePasswordSDKItemsDelete = "ItemsDelete"
+	CallOnePasswordSDKFilesRead   = "FilesRead"
+	CallOnePasswordSDKVaultsList  = "VaultsList"
+
 	StatusError   = "error"
 	StatusSuccess = "success"
 

runtime/testing/fake/fake.go

@@ -86,17 +86,21 @@ func (v *Client) RegisterAs(provider *esv1.SecretStoreProvider) {
 
 // GetAllSecrets implements the provider.Provider interface.
 func (v *Client) GetAllSecrets(ctx context.Context, ref esv1.ExternalSecretFind) (map[string][]byte, error) {
-	return v.GetAllSecretsFn(ctx, ref)
+	v.mu.RLock()
+	fn := v.GetAllSecretsFn
+	v.mu.RUnlock()
+	return fn(ctx, ref)
 }
 
 func (v *Client) PushSecret(_ context.Context, secret *corev1.Secret, data esv1.PushSecretData) error {
 	v.mu.Lock()
-	defer v.mu.Unlock()
 	v.pushSecretData[data.GetRemoteKey()] = SetSecretCallArgs{
 		Value:     secret.Data[data.GetSecretKey()],
 		RemoteRef: data,
 	}
-	return v.SetSecretFn()
+	fn := v.SetSecretFn
+	v.mu.Unlock()
+	return fn()
 }
 
 // GetPushSecretData safely retrieves the push secret data map for reading.
@@ -112,29 +116,43 @@ func (v *Client) GetPushSecretData() map[string]SetSecretCallArgs {
 }
 
 func (v *Client) DeleteSecret(_ context.Context, _ esv1.PushSecretRemoteRef) error {
-	return v.DeleteSecretFn()
+	v.mu.RLock()
+	fn := v.DeleteSecretFn
+	v.mu.RUnlock()
+	return fn()
 }
 
 func (v *Client) SecretExists(ctx context.Context, ref esv1.PushSecretRemoteRef) (bool, error) {
-	return v.SecretExistsFn(ctx, ref)
+	v.mu.RLock()
+	fn := v.SecretExistsFn
+	v.mu.RUnlock()
+	return fn(ctx, ref)
 }
 
 // GetSecret implements the provider.Provider interface.
 func (v *Client) GetSecret(ctx context.Context, ref esv1.ExternalSecretDataRemoteRef) ([]byte, error) {
-	return v.GetSecretFn(ctx, ref)
+	v.mu.RLock()
+	fn := v.GetSecretFn
+	v.mu.RUnlock()
+	return fn(ctx, ref)
 }
 
 // WithGetSecret wraps secret data returned by this provider.
 func (v *Client) WithGetSecret(secData []byte, err error) *Client {
+	v.mu.Lock()
 	v.GetSecretFn = func(context.Context, esv1.ExternalSecretDataRemoteRef) ([]byte, error) {
 		return secData, err
 	}
+	v.mu.Unlock()
 	return v
 }
 
 // GetSecretMap implements the provider.Provider interface.
 func (v *Client) GetSecretMap(ctx context.Context, ref esv1.ExternalSecretDataRemoteRef) (map[string][]byte, error) {
-	return v.GetSecretMapFn(ctx, ref)
+	v.mu.RLock()
+	fn := v.GetSecretMapFn
+	v.mu.RUnlock()
+	return fn(ctx, ref)
 }
 
 func (v *Client) Close(_ context.Context) error {
@@ -151,32 +169,40 @@ func (v *Client) ValidateStore(_ esv1.GenericStore) (admission.Warnings, error)
 
 // WithGetSecretMap wraps the secret data map returned by this fake provider.
 func (v *Client) WithGetSecretMap(secData map[string][]byte, err error) *Client {
+	v.mu.Lock()
 	v.GetSecretMapFn = func(context.Context, esv1.ExternalSecretDataRemoteRef) (map[string][]byte, error) {
 		return secData, err
 	}
+	v.mu.Unlock()
 	return v
 }
 
 // WithGetAllSecrets wraps the secret data map returned by this fake provider.
 func (v *Client) WithGetAllSecrets(secData map[string][]byte, err error) *Client {
+	v.mu.Lock()
 	v.GetAllSecretsFn = func(context.Context, esv1.ExternalSecretFind) (map[string][]byte, error) {
 		return secData, err
 	}
+	v.mu.Unlock()
 	return v
 }
 
 // WithSetSecret wraps the secret response to the fake provider.
 func (v *Client) WithSetSecret(err error) *Client {
+	v.mu.Lock()
 	v.SetSecretFn = func() error {
 		return err
 	}
+	v.mu.Unlock()
 	return v
 }
 
 // WithNew wraps the fake provider factory function.
 func (v *Client) WithNew(f func(context.Context, esv1.GenericStore, client.Client,
 	string) (esv1.SecretsClient, error)) *Client {
+	v.mu.Lock()
 	v.NewFn = f
+	v.mu.Unlock()
 	return v
 }
 
@@ -187,7 +213,10 @@ func (v *Client) Capabilities() esv1.SecretStoreCapabilities {
 
 // NewClient returns a new fake provider.
 func (v *Client) NewClient(ctx context.Context, store esv1.GenericStore, kube client.Client, namespace string) (esv1.SecretsClient, error) {
-	c, err := v.NewFn(ctx, store, kube, namespace)
+	v.mu.RLock()
+	fn := v.NewFn
+	v.mu.RUnlock()
+	c, err := fn(ctx, store, kube, namespace)
 	if err != nil {
 		return nil, err
 	}