fix: do not set tags if undefined (#6103)

* fix: predicate for push secret and do not set tags if undefined

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>

* remove the predicate

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>

* add a narrower predicate

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>

* tried to add some tests for this

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>

---------

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>

pkg/controllers/pushsecret/predicate_test.go

@@ -0,0 +1,103 @@
+/*
+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 pushsecret
+
+import (
+	"testing"
+	"time"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/controller-runtime/pkg/event"
+
+	esapi "github.com/external-secrets/external-secrets/apis/externalsecrets/v1alpha1"
+)
+
+func TestPushSecretWatchPredicate_Update(t *testing.T) {
+	now := metav1.NewTime(time.Now())
+
+	base := func() *esapi.PushSecret {
+		return &esapi.PushSecret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:        "ps",
+				Namespace:   "ns",
+				Generation:  1,
+				Labels:      map[string]string{"app": "demo"},
+				Annotations: map[string]string{"owner": "guppi"},
+			},
+		}
+	}
+
+	tests := []struct {
+		name   string
+		mutate func(newObj *esapi.PushSecret)
+		want   bool
+	}{
+		{
+			name: "status-only change is filtered out",
+			mutate: func(newObj *esapi.PushSecret) {
+				newObj.Status.SyncedResourceVersion = "rv-2"
+				newObj.Status.RefreshTime = now
+			},
+			want: false,
+		},
+		{
+			name: "generation bump triggers reconcile",
+			mutate: func(newObj *esapi.PushSecret) {
+				newObj.Generation = 2
+			},
+			want: true,
+		},
+		{
+			name: "label change triggers reconcile",
+			mutate: func(newObj *esapi.PushSecret) {
+				newObj.Labels["app"] = "demo2"
+			},
+			want: true,
+		},
+		{
+			name: "annotation change triggers reconcile",
+			mutate: func(newObj *esapi.PushSecret) {
+				newObj.Annotations["owner"] = "captain"
+			},
+			want: true,
+		},
+		{
+			name: "deletion timestamp appearance triggers reconcile",
+			mutate: func(newObj *esapi.PushSecret) {
+				newObj.DeletionTimestamp = &now
+			},
+			want: true,
+		},
+	}
+
+	pred := pushSecretWatchPredicate()
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			oldObj := base()
+			newObj := base()
+			tt.mutate(newObj)
+
+			got := pred.Update(event.UpdateEvent{
+				ObjectOld: oldObj,
+				ObjectNew: newObj,
+			})
+			if got != tt.want {
+				t.Errorf("predicate.Update = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

pkg/controllers/pushsecret/pushsecret_controller.go

@@ -39,9 +39,12 @@ import (
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/record"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"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/predicate"
 
 	esv1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1"
 	esapi "github.com/external-secrets/external-secrets/apis/externalsecrets/v1alpha1"
@@ -129,10 +132,44 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt
 
 	return ctrl.NewControllerManagedBy(mgr).
 		WithOptions(opts).
-		For(&esapi.PushSecret{}).
+		For(&esapi.PushSecret{}, builder.WithPredicates(pushSecretWatchPredicate())).
 		Complete(r)
 }
 
+func pushSecretWatchPredicate() predicate.Predicate {
+	return predicate.Funcs{
+		CreateFunc: func(event.CreateEvent) bool {
+			return true
+		},
+		DeleteFunc: func(event.DeleteEvent) bool {
+			return true
+		},
+		UpdateFunc: func(e event.UpdateEvent) bool {
+			if e.ObjectOld == nil || e.ObjectNew == nil {
+				return true
+			}
+
+			return shouldReconcilePushSecretUpdate(e.ObjectOld, e.ObjectNew)
+		},
+	}
+}
+
+func shouldReconcilePushSecretUpdate(oldObj, newObj client.Object) bool {
+	if oldObj.GetGeneration() != newObj.GetGeneration() {
+		return true
+	}
+	if !maps.Equal(oldObj.GetLabels(), newObj.GetLabels()) {
+		return true
+	}
+	if !maps.Equal(oldObj.GetAnnotations(), newObj.GetAnnotations()) {
+		return true
+	}
+
+	oldDeleting := oldObj.GetDeletionTimestamp() != nil
+	newDeleting := newObj.GetDeletionTimestamp() != nil
+	return oldDeleting != newDeleting
+}
+
 // Reconcile is part of the main kubernetes reconciliation loop which aims to
 // move the current state of the cluster closer to the desired state.
 // For more details, check Reconcile and its Result here:

providers/v1/aws/secretsmanager/secretsmanager.go

@@ -658,8 +658,16 @@ func (sm *SecretsManager) putSecretValueWithContext(ctx context.Context, secretA
 	return err
 }
 
-func (sm *SecretsManager) patchTags(ctx context.Context, metadata *apiextensionsv1.JSON, secretID *string, tags map[string]string) error {
-	meta, err := sm.constructMetadataWithDefaults(metadata)
+func (sm *SecretsManager) patchTags(ctx context.Context, rawMetadata *apiextensionsv1.JSON, secretID *string, tags map[string]string) error {
+	rawMeta, err := metadata.ParseMetadataParameters[PushSecretMetadataSpec](rawMetadata)
+	if err != nil {
+		return err
+	}
+	if rawMeta == nil || len(rawMeta.Spec.Tags) == 0 {
+		return nil
+	}
+
+	meta, err := sm.constructMetadataWithDefaults(rawMetadata)
 	if err != nil {
 		return err
 	}