Pass label selector predicate to the Secret Watch and handler (#443)

* Pass label selector predicate to the Secret Watch and handler

* Simplify watcher predicate setup

* Fix typo

Author: kurlov

pkg/reconciler/reconciler.go

@@ -74,7 +74,7 @@ type Reconciler struct {
 	log                              logr.Logger
 	gvk                              *schema.GroupVersionKind
 	chrt                             *chart.Chart
-	selectorPredicate                predicate.Predicate
+	labelSelector                    metav1.LabelSelector
 	overrideValues                   map[string]string
 	skipDependentWatches             bool
 	maxConcurrentReconciles          int
@@ -523,15 +523,11 @@ func WithValueMapper(m values.Mapper) Option {
 	}
 }
 
-// WithSelector is an Option that configures the reconciler to creates a
-// predicate that is used to filter resources based on the specified selector
+// WithSelector is an Option that configures label selector for the reconciler.
+// The label selector is used to filter watch resources.
 func WithSelector(s metav1.LabelSelector) Option {
 	return func(r *Reconciler) error {
-		p, err := predicate.LabelSelectorPredicate(s)
-		if err != nil {
-			return err
-		}
-		r.selectorPredicate = p
+		r.labelSelector = s
 		return nil
 	}
 }
@@ -1028,8 +1024,13 @@ func (r *Reconciler) setupWatches(mgr ctrl.Manager, c controller.Controller) err
 	obj.SetGroupVersionKind(*r.gvk)
 
 	var preds []predicate.Predicate
-	if r.selectorPredicate != nil {
-		preds = append(preds, r.selectorPredicate)
+
+	if r.labelSelector.MatchLabels != nil {
+		selectorPredicate, err := predicate.LabelSelectorPredicate(r.labelSelector)
+		if err != nil {
+			return err
+		}
+		preds = append(preds, selectorPredicate)
 	}
 
 	if err := c.Watch(
@@ -1053,13 +1054,14 @@ func (r *Reconciler) setupWatches(mgr ctrl.Manager, c controller.Controller) err
 	if err := c.Watch(
 		source.Kind(
 			mgr.GetCache(),
-			secret,
-			handler.TypedEnqueueRequestForOwner[*corev1.Secret](
+			client.Object(secret),
+			handler.TypedEnqueueRequestForOwner[client.Object](
 				mgr.GetScheme(),
 				mgr.GetRESTMapper(),
 				obj,
 				handler.OnlyControllerOwner(),
 			),
+			preds...,
 		),
 	); err != nil {
 		return err

pkg/reconciler/reconciler_test.go

@@ -53,6 +53,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
+	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	"sigs.k8s.io/controller-runtime/pkg/source"
 	"sigs.k8s.io/yaml"
@@ -449,18 +450,21 @@ var _ = Describe("Reconciler", func() {
 
 				selector := metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}}
 				Expect(WithSelector(selector)(r)).To(Succeed())
-				Expect(r.selectorPredicate).NotTo(BeNil())
-
-				Expect(r.selectorPredicate.Create(event.CreateEvent{Object: objLabeled})).To(BeTrue())
-				Expect(r.selectorPredicate.Update(event.UpdateEvent{ObjectOld: objUnlabeled, ObjectNew: objLabeled})).To(BeTrue())
-				Expect(r.selectorPredicate.Delete(event.DeleteEvent{Object: objLabeled})).To(BeTrue())
-				Expect(r.selectorPredicate.Generic(event.GenericEvent{Object: objLabeled})).To(BeTrue())
-
-				Expect(r.selectorPredicate.Create(event.CreateEvent{Object: objUnlabeled})).To(BeFalse())
-				Expect(r.selectorPredicate.Update(event.UpdateEvent{ObjectOld: objLabeled, ObjectNew: objUnlabeled})).To(BeFalse())
-				Expect(r.selectorPredicate.Update(event.UpdateEvent{ObjectOld: objUnlabeled, ObjectNew: objUnlabeled})).To(BeFalse())
-				Expect(r.selectorPredicate.Delete(event.DeleteEvent{Object: objUnlabeled})).To(BeFalse())
-				Expect(r.selectorPredicate.Generic(event.GenericEvent{Object: objUnlabeled})).To(BeFalse())
+				Expect(r.labelSelector).NotTo(BeNil())
+
+				selectorPredicate, err := predicate.LabelSelectorPredicate(r.labelSelector)
+				Expect(err).ToNot(HaveOccurred())
+
+				Expect(selectorPredicate.Create(event.CreateEvent{Object: objLabeled})).To(BeTrue())
+				Expect(selectorPredicate.Update(event.UpdateEvent{ObjectOld: objUnlabeled, ObjectNew: objLabeled})).To(BeTrue())
+				Expect(selectorPredicate.Delete(event.DeleteEvent{Object: objLabeled})).To(BeTrue())
+				Expect(selectorPredicate.Generic(event.GenericEvent{Object: objLabeled})).To(BeTrue())
+
+				Expect(selectorPredicate.Create(event.CreateEvent{Object: objUnlabeled})).To(BeFalse())
+				Expect(selectorPredicate.Update(event.UpdateEvent{ObjectOld: objLabeled, ObjectNew: objUnlabeled})).To(BeFalse())
+				Expect(selectorPredicate.Update(event.UpdateEvent{ObjectOld: objUnlabeled, ObjectNew: objUnlabeled})).To(BeFalse())
+				Expect(selectorPredicate.Delete(event.DeleteEvent{Object: objUnlabeled})).To(BeFalse())
+				Expect(selectorPredicate.Generic(event.GenericEvent{Object: objUnlabeled})).To(BeFalse())
 			})
 		})
 	})
@@ -1488,6 +1492,45 @@ var _ = Describe("Reconciler", func() {
 								})
 							})
 						})
+						When("label selector set", func() {
+							It("reconcile only matching CR", func() {
+								By("adding selector to the reconciler", func() {
+									selectorFoo := metav1.LabelSelector{MatchLabels: map[string]string{"app": "foo"}}
+									Expect(WithSelector(selectorFoo)(r)).To(Succeed())
+								})
+
+								By("adding not matching label to the CR", func() {
+									Expect(mgr.GetClient().Get(ctx, objKey, obj)).To(Succeed())
+									obj.SetLabels(map[string]string{"app": "bar"})
+									Expect(mgr.GetClient().Update(ctx, obj)).To(Succeed())
+								})
+
+								By("reconciling skipped and no actions for the release", func() {
+									res, err := r.Reconcile(ctx, req)
+									Expect(res).To(Equal(reconcile.Result{}))
+									Expect(err).ToNot(HaveOccurred())
+								})
+
+								By("verifying the release has not changed", func() {
+									rel, err := ac.Get(obj.GetName())
+									Expect(err).ToNot(HaveOccurred())
+									Expect(rel).NotTo(BeNil())
+									Expect(*rel).To(Equal(*currentRelease))
+								})
+
+								By("adding matching label to the CR", func() {
+									Expect(mgr.GetClient().Get(ctx, objKey, obj)).To(Succeed())
+									obj.SetLabels(map[string]string{"app": "foo"})
+									Expect(mgr.GetClient().Update(ctx, obj)).To(Succeed())
+								})
+
+								By("successfully reconciling with correct labels", func() {
+									res, err := r.Reconcile(ctx, req)
+									Expect(res).To(Equal(reconcile.Result{}))
+									Expect(err).ToNot(HaveOccurred())
+								})
+							})
+						})
 					})
 				})
 			})