fix: prevent feedback loop (#6021)

* fix: prevent feedback loop

Signed-off-by: Moritz Johner <beller.moritz@googlemail.com>

* chore: add regression test for feedback loop

Signed-off-by: Moritz Johner <beller.moritz@googlemail.com>

* fix: address comments

Signed-off-by: Moritz Johner <beller.moritz@googlemail.com>

* chore: refactor get-es-conditions func

Signed-off-by: Moritz Johner <beller.moritz@googlemail.com>

* fix: prevent feedback loop in ExternalSecret controller (#6034)

* Initial plan

* fix: address review comments - pointer-to-loop-var, finalizers predicate, nil dereference

Co-authored-by: moolen <1709030+moolen@users.noreply.github.com>

* fix: prevent feedback loop in ExternalSecret controller

Co-authored-by: moolen <1709030+moolen@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: moolen <1709030+moolen@users.noreply.github.com>

* fix: wire cli flag to conditionally enable the direct-read fallback

Signed-off-by: Moritz Johner <beller.moritz@googlemail.com>

* chore: fix lint and license headers

* test: correct cmp diff labels in conditions test

---------

Signed-off-by: Moritz Johner <beller.moritz@googlemail.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: moolen <1709030+moolen@users.noreply.github.com>
Co-authored-by: Gergely Bräutigam <gergely.brautigam@sap.com>

apis/externalsecrets/v1/conditions.go

@@ -0,0 +1,28 @@
+/*
+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 v1
+
+// GetExternalSecretCondition returns the condition with the provided type.
+func GetExternalSecretCondition(status ExternalSecretStatus, condType ExternalSecretConditionType) *ExternalSecretStatusCondition {
+	for i := range status.Conditions {
+		if status.Conditions[i].Type == condType {
+			return &status.Conditions[i]
+		}
+	}
+
+	return nil
+}

apis/externalsecrets/v1/conditions_test.go

@@ -0,0 +1,69 @@
+/*
+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 v1
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	corev1 "k8s.io/api/core/v1"
+)
+
+func TestGetExternalSecretCondition(t *testing.T) {
+	status := ExternalSecretStatus{
+		Conditions: []ExternalSecretStatusCondition{
+			{
+				Type:   ExternalSecretReady,
+				Status: corev1.ConditionFalse,
+			},
+			{
+				Type:   ExternalSecretReady,
+				Status: corev1.ConditionTrue,
+			},
+		},
+	}
+
+	tests := []struct {
+		name     string
+		condType ExternalSecretConditionType
+		expected *ExternalSecretStatusCondition
+	}{
+		{
+			name:     "Status has a condition of the specified type",
+			condType: ExternalSecretReady,
+			expected: &ExternalSecretStatusCondition{
+				Type:   ExternalSecretReady,
+				Status: corev1.ConditionFalse,
+			},
+		},
+		{
+			name:     "Status does not have a condition of the specified type",
+			condType: ExternalSecretDeleted,
+			expected: nil,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := GetExternalSecretCondition(status, tt.condType)
+
+			if diff := cmp.Diff(tt.expected, got); diff != "" {
+				t.Errorf("(-want, +got)\n%s", diff)
+			}
+		})
+	}
+}

apis/go.mod

@@ -3,6 +3,7 @@ module github.com/external-secrets/external-secrets/apis
 go 1.26.1
 
 require (
+	github.com/google/go-cmp v0.7.0
 	github.com/stretchr/testify v1.11.1
 	k8s.io/api v0.35.0
 	k8s.io/apiextensions-apiserver v0.35.0
@@ -35,7 +36,6 @@ require (
 	github.com/go-openapi/swag/yamlutils v0.25.4 // indirect
 	github.com/google/btree v1.1.3 // indirect
 	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/json-iterator/go v1.1.12 // indirect
 	github.com/kr/text v0.2.0 // indirect

cmd/controller/root.go

@@ -74,6 +74,7 @@ var (
 	enableSecretsCache                    bool
 	enableConfigMapsCache                 bool
 	enableManagedSecretsCache             bool
+	enableSecretAPIReadOnCacheMismatch    bool
 	enablePartialCache                    bool
 	concurrent                            int
 	port                                  int
@@ -236,17 +237,18 @@ var rootCmd = &cobra.Command{
 			os.Exit(1)
 		}
 		if err = (&externalsecret.Reconciler{
-			Client:                    mgr.GetClient(),
-			SecretClient:              secretClient,
-			Log:                       ctrl.Log.WithName("controllers").WithName("ExternalSecret"),
-			Scheme:                    mgr.GetScheme(),
-			RestConfig:                mgr.GetConfig(),
-			ControllerClass:           controllerClass,
-			RequeueInterval:           time.Hour,
-			ClusterSecretStoreEnabled: enableClusterStoreReconciler,
-			EnableFloodGate:           enableFloodGate,
-			EnableGeneratorState:      enableGeneratorState,
-			AllowGenericTargets:       allowGenericTargets,
+			Client:                             mgr.GetClient(),
+			SecretClient:                       secretClient,
+			EnableSecretAPIReadOnCacheMismatch: enableSecretAPIReadOnCacheMismatch,
+			Log:                                ctrl.Log.WithName("controllers").WithName("ExternalSecret"),
+			Scheme:                             mgr.GetScheme(),
+			RestConfig:                         mgr.GetConfig(),
+			ControllerClass:                    controllerClass,
+			RequeueInterval:                    time.Hour,
+			ClusterSecretStoreEnabled:          enableClusterStoreReconciler,
+			EnableFloodGate:                    enableFloodGate,
+			EnableGeneratorState:               enableGeneratorState,
+			AllowGenericTargets:                allowGenericTargets,
 		}).SetupWithManager(cmd.Context(), mgr, ctrlcommon.BuildControllerOptions(concurrent)); err != nil {
 			setupLog.Error(err, errCreateController, "controller", "ExternalSecret")
 			os.Exit(1)
@@ -349,6 +351,12 @@ func init() {
 	rootCmd.Flags().BoolVar(&enableSecretsCache, "enable-secrets-caching", false, "Enable secrets caching for ALL secrets in the cluster (WARNING: can increase memory usage).")
 	rootCmd.Flags().BoolVar(&enableConfigMapsCache, "enable-configmaps-caching", false, "Enable configmaps caching for ALL configmaps in the cluster (WARNING: can increase memory usage).")
 	rootCmd.Flags().BoolVar(&enableManagedSecretsCache, "enable-managed-secrets-caching", true, "Enable secrets caching for secrets managed by an ExternalSecret")
+	rootCmd.Flags().BoolVar(
+		&enableSecretAPIReadOnCacheMismatch,
+		"enable-secret-api-read-on-cache-mismatch",
+		true,
+		"Enable a direct API read when the partial Secret cache and managed Secret cache disagree. Disable to rely on cache retry only.",
+	)
 	rootCmd.Flags().DurationVar(&storeRequeueInterval, "store-requeue-interval", time.Minute*5, "Default Time duration between reconciling (Cluster)SecretStores")
 	rootCmd.Flags().BoolVar(&enableFloodGate, "enable-flood-gate", true, "Enable flood gate. External secret will be reconciled only if the ClusterStore or Store have an healthy or unknown state.")
 	rootCmd.Flags().BoolVar(&enableGeneratorState, "enable-generator-state", true, "Whether the Controller should manage GeneratorState")

e2e/suites/provider/cases/common/regressions.go

@@ -0,0 +1,107 @@
+/*
+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 common
+
+import (
+	"time"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
+
+	"github.com/external-secrets/external-secrets-e2e/framework"
+	esv1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1"
+)
+
+// StatusNotUpdatedAfterSuccessfulSync validates that a successful sync does not trigger
+// continuous status updates when the refresh interval is long.
+func StatusNotUpdatedAfterSuccessfulSync(f *framework.Framework) (string, func(*framework.TestCase)) {
+	return "[regression] should not continuously update status after a successful sync", func(tc *framework.TestCase) {
+		remoteKey := f.MakeRemoteRefKey(f.Namespace.Name + "-feedback-loop-key")
+		remoteValue := "feedback-loop-value"
+
+		tc.Secrets = map[string]framework.SecretEntry{
+			remoteKey: {Value: remoteValue},
+		}
+		tc.ExpectedSecret = &v1.Secret{
+			Type: v1.SecretTypeOpaque,
+			Data: map[string][]byte{
+				"value": []byte(remoteValue),
+			},
+		}
+		tc.ExternalSecret.ObjectMeta.Name = "feedback-loop-es"
+		tc.ExternalSecret.Spec.RefreshInterval = &metav1.Duration{Duration: time.Hour}
+		tc.ExternalSecret.Spec.Target.Name = framework.TargetSecretName
+		tc.ExternalSecret.Spec.Data = []esv1.ExternalSecretData{
+			{
+				SecretKey: "value",
+				RemoteRef: esv1.ExternalSecretDataRemoteRef{
+					Key: remoteKey,
+				},
+			},
+		}
+		tc.AfterSync = func(_ framework.SecretStoreProvider, _ *v1.Secret) {
+			key := types.NamespacedName{Name: tc.ExternalSecret.Name, Namespace: tc.ExternalSecret.Namespace}
+			var baseline *esv1.ExternalSecret
+
+			Eventually(func() bool {
+				current := &esv1.ExternalSecret{}
+				if err := tc.Framework.CRClient.Get(GinkgoT().Context(), key, current); err != nil {
+					return false
+				}
+
+				ready := esv1.GetExternalSecretCondition(current.Status, esv1.ExternalSecretReady)
+				if ready == nil || ready.Status != v1.ConditionTrue {
+					return false
+				}
+				if current.Status.RefreshTime.IsZero() || current.Status.SyncedResourceVersion == "" {
+					return false
+				}
+
+				if baseline == nil {
+					baseline = current.DeepCopy()
+					return false
+				}
+
+				if current.ResourceVersion != baseline.ResourceVersion {
+					baseline = current.DeepCopy()
+					return false
+				}
+				if !current.Status.RefreshTime.Equal(&baseline.Status.RefreshTime) {
+					baseline = current.DeepCopy()
+					return false
+				}
+				if current.Status.SyncedResourceVersion != baseline.Status.SyncedResourceVersion {
+					baseline = current.DeepCopy()
+					return false
+				}
+
+				return true
+			}).WithTimeout(30 * time.Second).WithPolling(500 * time.Millisecond).Should(BeTrue())
+
+			Consistently(func(g Gomega) {
+				current := &esv1.ExternalSecret{}
+				g.Expect(tc.Framework.CRClient.Get(GinkgoT().Context(), key, current)).To(Succeed())
+				g.Expect(current.ResourceVersion).To(Equal(baseline.ResourceVersion))
+				g.Expect(current.Status.RefreshTime.Equal(&baseline.Status.RefreshTime)).To(BeTrue())
+				g.Expect(current.Status.SyncedResourceVersion).To(Equal(baseline.Status.SyncedResourceVersion))
+			}).WithTimeout(10 * time.Second).WithPolling(500 * time.Millisecond).Should(Succeed())
+		}
+	}
+}

e2e/suites/provider/cases/fake/regressions.go

@@ -0,0 +1,34 @@
+/*
+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 fake
+
+import (
+	"github.com/external-secrets/external-secrets-e2e/framework"
+	"github.com/external-secrets/external-secrets-e2e/suites/provider/cases/common"
+	. "github.com/onsi/ginkgo/v2"
+)
+
+var _ = Describe("[fake] ", Label("fake"), func() {
+	f := framework.New("eso-fake")
+	prov := NewProvider(f)
+
+	// we use the fake provider to test regressions
+	DescribeTable("controller regressions",
+		framework.TableFuncWithExternalSecret(f, prov),
+		Entry(common.StatusNotUpdatedAfterSuccessfulSync(f)),
+	)
+})

e2e/suites/provider/cases/import.go

@@ -24,6 +24,7 @@ import (
 	_ "github.com/external-secrets/external-secrets-e2e/suites/provider/cases/azure"
 	_ "github.com/external-secrets/external-secrets-e2e/suites/provider/cases/conjur"
 	_ "github.com/external-secrets/external-secrets-e2e/suites/provider/cases/delinea"
+	_ "github.com/external-secrets/external-secrets-e2e/suites/provider/cases/fake"
 	_ "github.com/external-secrets/external-secrets-e2e/suites/provider/cases/gcp"
 	_ "github.com/external-secrets/external-secrets-e2e/suites/provider/cases/kubernetes"
 	_ "github.com/external-secrets/external-secrets-e2e/suites/provider/cases/scaleway"

pkg/controllers/externalsecret/externalsecret_controller.go

@@ -47,6 +47,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -119,6 +120,9 @@ const (
 	eventDeletedOrphaned          = "secret deleted because it was orphaned"
 	eventMissingProviderSecret    = "secret does not exist at provider using spec.dataFrom[%d]"
 	eventMissingProviderSecretKey = "secret does not exist at provider using spec.dataFrom[%d] (key=%s)"
+
+	// cacheSyncRetryDelay is used when partial and full secret caches are temporarily out of sync.
+	cacheSyncRetryDelay = 200 * time.Millisecond
 )
 
 // these errors are explicitly defined so we can detect them with `errors.Is()`.
@@ -137,17 +141,19 @@ const (
 // Reconciler reconciles a ExternalSecret object.
 type Reconciler struct {
 	client.Client
-	SecretClient              client.Client
-	Log                       logr.Logger
-	Scheme                    *runtime.Scheme
-	RestConfig                *rest.Config
-	ControllerClass           string
-	RequeueInterval           time.Duration
-	ClusterSecretStoreEnabled bool
-	EnableFloodGate           bool
-	EnableGeneratorState      bool
-	AllowGenericTargets       bool
-	recorder                  record.EventRecorder
+	SecretClient                       client.Client
+	APIReader                          client.Reader
+	EnableSecretAPIReadOnCacheMismatch bool
+	Log                                logr.Logger
+	Scheme                             *runtime.Scheme
+	RestConfig                         *rest.Config
+	ControllerClass                    string
+	RequeueInterval                    time.Duration
+	ClusterSecretStoreEnabled          bool
+	EnableFloodGate                    bool
+	EnableGeneratorState               bool
+	AllowGenericTargets                bool
+	recorder                           record.EventRecorder
 
 	// informerManager manages dynamic informers for generic targets
 	informerManager InformerManager
@@ -334,13 +340,17 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ct
 
 	// ensure the full cache is up-to-date
 	// NOTE: this prevents race conditions between the partial and full cache.
-	//       we return an error so we get an exponential backoff if we end up looping,
-	//       for example, during high cluster load and frequent updates to the target secret by other controllers.
-	if secretPartial.UID != existingSecret.UID || secretPartial.ResourceVersion != existingSecret.ResourceVersion {
-		err = fmt.Errorf(errSecretCachesNotSynced, secretName)
-		log.Error(err, logErrorSecretCacheNotSynced, "secretName", secretName, "secretNamespace", externalSecret.Namespace)
+	//       if enabled, we verify against the API server before retrying to avoid unnecessary error backoff
+	//       when the cache is temporarily stale.
+	existingSecret, cacheNotSynced, getErr := r.resolveSecretCacheMismatch(ctx, client.ObjectKey{Name: secretName, Namespace: externalSecret.Namespace}, secretPartial, existingSecret)
+	if getErr != nil && !apierrors.IsNotFound(getErr) {
+		log.Error(getErr, logErrorGetSecret, "secretName", secretName, "secretNamespace", externalSecret.Namespace)
 		syncCallsError.With(resourceLabels).Inc()
-		return ctrl.Result{}, err
+		return ctrl.Result{}, getErr
+	}
+	if cacheNotSynced {
+		log.V(1).Info(logErrorSecretCacheNotSynced, "secretName", secretName, "secretNamespace", externalSecret.Namespace)
+		return ctrl.Result{RequeueAfter: cacheSyncRetryDelay}, nil
 	}
 
 	// refresh will be skipped if ALL the following conditions are met:
@@ -780,7 +790,7 @@ func (r *Reconciler) getRequeueResult(externalSecret *esv1.ExternalSecret) ctrl.
 }
 
 func (r *Reconciler) markAsDone(externalSecret *esv1.ExternalSecret, start time.Time, log logr.Logger, reason, msg string) {
-	oldReadyCondition := GetExternalSecretCondition(externalSecret.Status, esv1.ExternalSecretReady)
+	oldReadyCondition := esv1.GetExternalSecretCondition(externalSecret.Status, esv1.ExternalSecretReady)
 	newReadyCondition := NewExternalSecretCondition(esv1.ExternalSecretReady, v1.ConditionTrue, reason, msg)
 	SetExternalSecretCondition(externalSecret, *newReadyCondition)
 
@@ -1220,6 +1230,9 @@ func genericTargetContentHash(obj *unstructured.Unstructured) (string, error) {
 // SetupWithManager returns a new controller builder that will be started by the provided Manager.
 func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opts controller.Options) error {
 	r.recorder = mgr.GetEventRecorderFor("external-secrets")
+	if r.APIReader == nil {
+		r.APIReader = mgr.GetAPIReader()
+	}
 	// Initialize informer manager only if generic targets are allowed
 	if r.AllowGenericTargets && r.informerManager == nil {
 		r.informerManager = NewInformerManager(ctx, mgr.GetCache(), r.Client, r.Log.WithName("informer-manager"))
@@ -1265,10 +1278,17 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt
 		return hasLabel && value == esv1.LabelManagedValue
 	})
 
+	// filter ExternalSecret updates to avoid requeueing on status-only changes.
+	externalSecretPredicate := predicate.Funcs{
+		UpdateFunc: func(e event.UpdateEvent) bool {
+			return shouldEnqueueExternalSecretUpdate(e.ObjectOld, e.ObjectNew)
+		},
+	}
+
 	// Build the controller
 	builder := ctrl.NewControllerManagedBy(mgr).
 		WithOptions(opts).
-		For(&esv1.ExternalSecret{}).
+		For(&esv1.ExternalSecret{}, builder.WithPredicates(externalSecretPredicate)).
 		// we cant use Owns(), as we don't set ownerReferences when the creationPolicy is not Owner.
 		// we use WatchesMetadata() to reduce memory usage, as otherwise we have to process full secret objects.
 		WatchesMetadata(
@@ -1286,6 +1306,73 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt
 	return builder.Complete(r)
 }
 
+// shouldEnqueueExternalSecretUpdate returns true for spec/metadata updates that can affect reconciliation behavior,
+// while ignoring status-only updates.
+func shouldEnqueueExternalSecretUpdate(oldObj, newObj client.Object) bool {
+	oldES, oldOK := oldObj.(*esv1.ExternalSecret)
+	newES, newOK := newObj.(*esv1.ExternalSecret)
+	if !oldOK || !newOK {
+		return true
+	}
+
+	if oldES.GetGeneration() != newES.GetGeneration() {
+		return true
+	}
+
+	if !equality.Semantic.DeepEqual(oldES.GetLabels(), newES.GetLabels()) {
+		return true
+	}
+
+	if !equality.Semantic.DeepEqual(oldES.GetAnnotations(), newES.GetAnnotations()) {
+		return true
+	}
+
+	if !equality.Semantic.DeepEqual(oldES.GetFinalizers(), newES.GetFinalizers()) {
+		return true
+	}
+
+	oldDeletion := oldES.GetDeletionTimestamp()
+	newDeletion := newES.GetDeletionTimestamp()
+	if oldDeletion == nil && newDeletion == nil {
+		return false
+	}
+	if oldDeletion == nil || newDeletion == nil {
+		return true
+	}
+
+	return !oldDeletion.Equal(newDeletion)
+}
+
+// resolveSecretCacheMismatch optionally uses a direct API read when the partial
+// and full secret caches disagree. It returns the secret to continue with,
+// whether the caches should still be treated as out of sync, and any read error.
+func (r *Reconciler) resolveSecretCacheMismatch(ctx context.Context, key client.ObjectKey, secretPartial *metav1.PartialObjectMetadata, existingSecret *v1.Secret) (*v1.Secret, bool, error) {
+	if secretPartial.UID == existingSecret.UID && secretPartial.ResourceVersion == existingSecret.ResourceVersion {
+		return existingSecret, false, nil
+	}
+
+	if !r.EnableSecretAPIReadOnCacheMismatch {
+		return nil, true, nil
+	}
+
+	authoritativeSecret := &v1.Secret{}
+	secretReader := r.APIReader
+	if secretReader == nil {
+		secretReader = r.SecretClient
+	}
+
+	err := secretReader.Get(ctx, key, authoritativeSecret)
+	if err != nil && !apierrors.IsNotFound(err) {
+		return nil, false, err
+	}
+
+	if secretPartial.UID != authoritativeSecret.UID || secretPartial.ResourceVersion != authoritativeSecret.ResourceVersion {
+		return nil, true, nil
+	}
+
+	return authoritativeSecret, false, nil
+}
+
 func (r *Reconciler) findObjectsForSecret(ctx context.Context, secret client.Object) []reconcile.Request {
 	externalSecretsList := &esv1.ExternalSecretList{}
 	listOps := &client.ListOptions{

pkg/controllers/externalsecret/externalsecret_controller_test.go

@@ -280,7 +280,7 @@ var _ = Describe("ExternalSecret controller", Serial, func() {
 			// default condition: es should be ready
 			targetSecretName: ExternalSecretTargetSecretName,
 			checkCondition: func(es *esv1.ExternalSecret) bool {
-				cond := GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
+				cond := esv1.GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
 				if cond == nil || cond.Status != v1.ConditionTrue {
 					return false
 				}
@@ -1659,14 +1659,14 @@ var _ = Describe("ExternalSecret controller", Serial, func() {
 			"bar/foo": []byte(BarValue),
 		}, nil)
 		tc.checkCondition = func(es *esv1.ExternalSecret) bool {
-			cond := GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
+			cond := esv1.GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
 			if cond == nil || cond.Status != v1.ConditionFalse || cond.Reason != esv1.ConditionReasonSecretSyncedError {
 				return false
 			}
 			return true
 		}
 		tc.checkCondition = func(es *esv1.ExternalSecret) bool {
-			cond := GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
+			cond := esv1.GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
 			if cond == nil || cond.Status != v1.ConditionFalse || cond.Reason != esv1.ConditionReasonSecretSyncedError {
 				return false
 			}
@@ -1708,14 +1708,14 @@ var _ = Describe("ExternalSecret controller", Serial, func() {
 			"bar/foo": []byte(BarValue),
 		}, nil)
 		tc.checkCondition = func(es *esv1.ExternalSecret) bool {
-			cond := GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
+			cond := esv1.GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
 			if cond == nil || cond.Status != v1.ConditionFalse || cond.Reason != esv1.ConditionReasonSecretSyncedError {
 				return false
 			}
 			return true
 		}
 		tc.checkCondition = func(es *esv1.ExternalSecret) bool {
-			cond := GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
+			cond := esv1.GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
 			if cond == nil || cond.Status != v1.ConditionFalse || cond.Reason != esv1.ConditionReasonSecretSyncedError {
 				return false
 			}
@@ -1847,7 +1847,7 @@ var _ = Describe("ExternalSecret controller", Serial, func() {
 		fakeProvider.WithGetSecret(nil, errors.New("boom"))
 		tc.externalSecret.Spec.RefreshInterval = &metav1.Duration{Duration: time.Millisecond * 100}
 		tc.checkCondition = func(es *esv1.ExternalSecret) bool {
-			cond := GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
+			cond := esv1.GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
 			if cond == nil || cond.Status != v1.ConditionFalse || cond.Reason != esv1.ConditionReasonSecretSyncedError {
 				return false
 			}
@@ -1871,8 +1871,8 @@ var _ = Describe("ExternalSecret controller", Serial, func() {
 					return false
 				}
 				// condition must now be true!
-				cond := GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
-				if cond == nil && cond.Status != v1.ConditionTrue {
+				cond := esv1.GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
+				if cond == nil || cond.Status != v1.ConditionTrue {
 					return false
 				}
 				return true
@@ -1885,7 +1885,7 @@ var _ = Describe("ExternalSecret controller", Serial, func() {
 	storeMissingErrCondition := func(tc *testCase) {
 		tc.externalSecret.Spec.SecretStoreRef.Name = "nonexistent"
 		tc.checkCondition = func(es *esv1.ExternalSecret) bool {
-			cond := GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
+			cond := esv1.GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
 			if cond == nil || cond.Status != v1.ConditionFalse || cond.Reason != esv1.ConditionReasonSecretSyncedError {
 				return false
 			}
@@ -1911,7 +1911,7 @@ var _ = Describe("ExternalSecret controller", Serial, func() {
 		})
 		tc.checkCondition = func(es *esv1.ExternalSecret) bool {
 			// condition must be false
-			cond := GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
+			cond := esv1.GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
 			if cond == nil || cond.Status != v1.ConditionFalse || cond.Reason != esv1.ConditionReasonSecretSyncedError {
 				return false
 			}
@@ -1933,7 +1933,7 @@ var _ = Describe("ExternalSecret controller", Serial, func() {
 	ignoreMismatchController := func(tc *testCase) {
 		tc.secretStore.GetSpec().Controller = "nop"
 		tc.checkCondition = func(es *esv1.ExternalSecret) bool {
-			cond := GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
+			cond := esv1.GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
 			return cond == nil
 		}
 		tc.checkExternalSecret = func(_ *esv1.ExternalSecret) {
@@ -1968,7 +1968,7 @@ var _ = Describe("ExternalSecret controller", Serial, func() {
 		)).To(BeTrue())
 
 		tc.checkCondition = func(es *esv1.ExternalSecret) bool {
-			cond := GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
+			cond := esv1.GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
 			return cond == nil
 		}
 	}
@@ -2158,7 +2158,7 @@ var _ = Describe("ExternalSecret controller", Serial, func() {
 		}
 
 		tc.checkCondition = func(es *esv1.ExternalSecret) bool {
-			cond := GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
+			cond := esv1.GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
 			if cond == nil || cond.Status != v1.ConditionFalse || cond.Reason != esv1.ConditionReasonSecretSyncedError {
 				return false
 			}
@@ -2175,7 +2175,7 @@ var _ = Describe("ExternalSecret controller", Serial, func() {
 		}
 
 		tc.checkCondition = func(es *esv1.ExternalSecret) bool {
-			cond := GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
+			cond := esv1.GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
 			if cond == nil || cond.Status != v1.ConditionFalse || cond.Reason != esv1.ConditionReasonSecretSyncedError {
 				return false
 			}
@@ -2195,7 +2195,7 @@ var _ = Describe("ExternalSecret controller", Serial, func() {
 		}
 
 		tc.checkCondition = func(es *esv1.ExternalSecret) bool {
-			cond := GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
+			cond := esv1.GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
 			if cond == nil || cond.Status != v1.ConditionFalse || cond.Reason != esv1.ConditionReasonSecretSyncedError {
 				return false
 			}
@@ -2237,7 +2237,7 @@ var _ = Describe("ExternalSecret controller", Serial, func() {
 		}
 
 		tc.checkCondition = func(es *esv1.ExternalSecret) bool {
-			cond := GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
+			cond := esv1.GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
 			if cond == nil || cond.Status != v1.ConditionFalse || cond.Reason != esv1.ConditionReasonSecretSyncedError {
 				return false
 			}
@@ -2267,7 +2267,7 @@ var _ = Describe("ExternalSecret controller", Serial, func() {
 		}
 
 		tc.checkCondition = func(es *esv1.ExternalSecret) bool {
-			cond := GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
+			cond := esv1.GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
 			if cond == nil || cond.Status != v1.ConditionFalse || cond.Reason != esv1.ConditionReasonSecretSyncedError {
 				return false
 			}
@@ -2319,7 +2319,7 @@ var _ = Describe("ExternalSecret controller", Serial, func() {
 		}
 
 		tc.checkCondition = func(es *esv1.ExternalSecret) bool {
-			cond := GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
+			cond := esv1.GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
 			if cond == nil || cond.Status != v1.ConditionFalse || cond.Reason != esv1.ConditionReasonSecretSyncedError {
 				return false
 			}
@@ -2662,6 +2662,103 @@ var _ = Describe("ExternalSecret refresh logic", func() {
 	})
 })
 
+var _ = Describe("ExternalSecret update predicate", func() {
+	It("should ignore status-only updates", func() {
+		oldES := &esv1.ExternalSecret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:       "foo",
+				Namespace:  "default",
+				Generation: 1,
+			},
+			Status: esv1.ExternalSecretStatus{
+				RefreshTime: metav1.Now(),
+			},
+		}
+		newES := oldES.DeepCopy()
+		newES.Status.RefreshTime = metav1.NewTime(time.Now().Add(time.Minute))
+
+		Expect(shouldEnqueueExternalSecretUpdate(oldES, newES)).To(BeFalse())
+	})
+
+	It("should enqueue when generation changes", func() {
+		oldES := &esv1.ExternalSecret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:       "foo",
+				Namespace:  "default",
+				Generation: 1,
+			},
+		}
+		newES := oldES.DeepCopy()
+		newES.Generation = 2
+
+		Expect(shouldEnqueueExternalSecretUpdate(oldES, newES)).To(BeTrue())
+	})
+
+	It("should enqueue when labels change", func() {
+		oldES := &esv1.ExternalSecret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:       "foo",
+				Namespace:  "default",
+				Generation: 1,
+				Labels: map[string]string{
+					"app": "a",
+				},
+			},
+		}
+		newES := oldES.DeepCopy()
+		newES.Labels["app"] = "b"
+
+		Expect(shouldEnqueueExternalSecretUpdate(oldES, newES)).To(BeTrue())
+	})
+
+	It("should enqueue when annotations change", func() {
+		oldES := &esv1.ExternalSecret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:       "foo",
+				Namespace:  "default",
+				Generation: 1,
+				Annotations: map[string]string{
+					"note": "a",
+				},
+			},
+		}
+		newES := oldES.DeepCopy()
+		newES.Annotations["note"] = "b"
+
+		Expect(shouldEnqueueExternalSecretUpdate(oldES, newES)).To(BeTrue())
+	})
+
+	It("should enqueue when deletion timestamp changes", func() {
+		oldES := &esv1.ExternalSecret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:       "foo",
+				Namespace:  "default",
+				Generation: 1,
+			},
+		}
+		newES := oldES.DeepCopy()
+		now := metav1.Now()
+		newES.DeletionTimestamp = &now
+
+		Expect(shouldEnqueueExternalSecretUpdate(oldES, newES)).To(BeTrue())
+	})
+
+	It("should enqueue when finalizers change", func() {
+		oldES := &esv1.ExternalSecret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:       "foo",
+				Namespace:  "default",
+				Generation: 1,
+				Finalizers: []string{"external-secrets.io/finalizer"},
+			},
+		}
+		newES := oldES.DeepCopy()
+		newES.Finalizers = nil
+
+		Expect(shouldEnqueueExternalSecretUpdate(oldES, newES)).To(BeTrue())
+	})
+})
+
 var _ = Describe("ExternalSecret refresh policy", func() {
 	Context("RefreshPolicy=CreatedOnce", func() {
 		It("should refresh when SyncedResourceVersion is empty", func() {

pkg/controllers/externalsecret/suite_test.go

@@ -110,13 +110,14 @@ var _ = BeforeSuite(func() {
 	Expect(err).ToNot(HaveOccurred())
 
 	err = (&Reconciler{
-		Client:                    k8sManager.GetClient(),
-		SecretClient:              secretClient,
-		RestConfig:                cfg,
-		Scheme:                    k8sManager.GetScheme(),
-		Log:                       ctrl.Log.WithName("controllers").WithName("ExternalSecrets"),
-		RequeueInterval:           time.Second,
-		ClusterSecretStoreEnabled: true,
+		Client:                             k8sManager.GetClient(),
+		SecretClient:                       secretClient,
+		EnableSecretAPIReadOnCacheMismatch: true,
+		RestConfig:                         cfg,
+		Scheme:                             k8sManager.GetScheme(),
+		Log:                                ctrl.Log.WithName("controllers").WithName("ExternalSecrets"),
+		RequeueInterval:                    time.Second,
+		ClusterSecretStoreEnabled:          true,
 	}).SetupWithManager(ctx, k8sManager, controller.Options{
 		MaxConcurrentReconciles: 1,
 		RateLimiter:             ctrlcommon.BuildRateLimiter(),

pkg/controllers/externalsecret/util.go

@@ -38,20 +38,10 @@ func NewExternalSecretCondition(condType esv1.ExternalSecretConditionType, statu
 	}
 }
 
-// GetExternalSecretCondition returns the condition with the provided type.
-func GetExternalSecretCondition(status esv1.ExternalSecretStatus, condType esv1.ExternalSecretConditionType) *esv1.ExternalSecretStatusCondition {
-	for _, c := range status.Conditions {
-		if c.Type == condType {
-			return &c
-		}
-	}
-	return nil
-}
-
 // SetExternalSecretCondition updates the external secret to include the provided
 // condition.
 func SetExternalSecretCondition(es *esv1.ExternalSecret, condition esv1.ExternalSecretStatusCondition) {
-	currentCond := GetExternalSecretCondition(es.Status, condition.Type)
+	currentCond := esv1.GetExternalSecretCondition(es.Status, condition.Type)
 
 	if currentCond != nil && currentCond.Status == condition.Status &&
 		currentCond.Reason == condition.Reason && currentCond.Message == condition.Message {

pkg/controllers/externalsecret/util_test.go

@@ -27,51 +27,6 @@ import (
 	esv1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1"
 )
 
-func TestGetExternalSecretCondition(t *testing.T) {
-	status := esv1.ExternalSecretStatus{
-		Conditions: []esv1.ExternalSecretStatusCondition{
-			{
-				Type:   esv1.ExternalSecretReady,
-				Status: corev1.ConditionFalse,
-			},
-			{
-				Type:   esv1.ExternalSecretReady,
-				Status: corev1.ConditionTrue,
-			},
-		},
-	}
-
-	tests := []struct {
-		name     string
-		condType esv1.ExternalSecretConditionType
-		expected *esv1.ExternalSecretStatusCondition
-	}{
-		{
-			name:     "Status has a condition of the specified type",
-			condType: esv1.ExternalSecretReady,
-			expected: &esv1.ExternalSecretStatusCondition{
-				Type:   esv1.ExternalSecretReady,
-				Status: corev1.ConditionFalse,
-			},
-		},
-		{
-			name:     "Status does not have a condition of the specified type",
-			condType: esv1.ExternalSecretDeleted,
-			expected: nil,
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			got := GetExternalSecretCondition(status, tt.condType)
-
-			if diff := cmp.Diff(tt.expected, got); diff != "" {
-				t.Errorf("(-got, +want)\n%s", diff)
-			}
-		})
-	}
-}
-
 func TestSetExternalSecretCondition(t *testing.T) {
 	now := time.Now()