ClusterResourceSet: use generic predicates for secrets and check secr…

…et type in controller

exp/addons/controllers/clusterresourceset_controller.go

@@ -86,7 +86,7 @@ func (r *ClusterResourceSetReconciler) SetupWithManager(ctx context.Context, mgr
 			handler.EnqueueRequestsFromMapFunc(r.resourceToClusterResourceSet),
 			builder.OnlyMetadata,
 			builder.WithPredicates(
-				resourcepredicates.AddonsSecretCreate(ctrl.LoggerFrom(ctx)),
+				resourcepredicates.ResourceCreate(ctrl.LoggerFrom(ctx)),
 			),
 		).
 		WithOptions(options).

exp/addons/controllers/clusterresourceset_controller_test.go

@@ -34,7 +34,7 @@ import (
 )
 
 const (
-	timeout              = time.Second * 20
+	timeout              = time.Second * 5
 	defaultNamespaceName = "default"
 )
 
@@ -255,7 +255,7 @@ metadata:
 		}, timeout).Should(BeTrue())
 	})
 
-	t.Run("Should reconcile a ClusterResourceSet when a resource is created that is part of ClusterResourceSet resources", func(t *testing.T) {
+	t.Run("Should reconcile a ClusterResourceSet when a ConfigMap resource is created that is part of ClusterResourceSet resources", func(t *testing.T) {
 		g := NewWithT(t)
 		setup(t, g)
 		defer teardown(t, g)
@@ -306,6 +306,10 @@ metadata:
 			return false
 		}, timeout).Should(BeTrue())
 
+		// Must sleep here to make sure resource is created after the previous reconcile.
+		// If the resource is created in between, predicates are not used as intended in this test.
+		time.Sleep(2 * time.Second)
+
 		newConfigmap := &corev1.ConfigMap{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      newCMName,
@@ -350,6 +354,106 @@ metadata:
 		}, timeout).Should(BeTrue())
 	})
 
+	t.Run("Should reconcile a ClusterResourceSet when a Secret resource is created that is part of ClusterResourceSet resources", func(t *testing.T) {
+		g := NewWithT(t)
+		setup(t, g)
+		defer teardown(t, g)
+
+		newSecretName := fmt.Sprintf("test-secret-%s", util.RandomString(6))
+
+		crsInstance := &addonsv1.ClusterResourceSet{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      clusterResourceSetName,
+				Namespace: defaultNamespaceName,
+			},
+			Spec: addonsv1.ClusterResourceSetSpec{
+				ClusterSelector: metav1.LabelSelector{
+					MatchLabels: labels,
+				},
+				Resources: []addonsv1.ResourceRef{{Name: newSecretName, Kind: "Secret"}},
+			},
+		}
+		// Create the ClusterResourceSet.
+		g.Expect(env.Create(ctx, crsInstance)).To(Succeed())
+
+		testCluster.SetLabels(labels)
+		g.Expect(env.Update(ctx, testCluster)).To(Succeed())
+
+		// Must sleep here to make sure resource is created after the previous reconcile.
+		// If the resource is created in between, predicates are not used as intended in this test.
+		time.Sleep(2 * time.Second)
+
+		t.Log("Verifying ClusterResourceSetBinding is created with cluster owner reference")
+		// Wait until ClusterResourceSetBinding is created for the Cluster
+		clusterResourceSetBindingKey := client.ObjectKey{
+			Namespace: testCluster.Namespace,
+			Name:      testCluster.Name,
+		}
+		g.Eventually(func() bool {
+			binding := &addonsv1.ClusterResourceSetBinding{}
+
+			err := env.Get(ctx, clusterResourceSetBindingKey, binding)
+			return err == nil
+		}, timeout).Should(BeTrue())
+
+		// Initially Secret is missing, so no resources in the binding.
+		g.Eventually(func() bool {
+			binding := &addonsv1.ClusterResourceSetBinding{}
+
+			err := env.Get(ctx, clusterResourceSetBindingKey, binding)
+			if err == nil {
+				if len(binding.Spec.Bindings) > 0 && len(binding.Spec.Bindings[0].Resources) == 0 {
+					return true
+				}
+			}
+			return false
+		}, timeout).Should(BeTrue())
+
+		newSecret := &corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      newSecretName,
+				Namespace: defaultNamespaceName,
+			},
+			Type: addonsv1.ClusterResourceSetSecretType,
+			Data: map[string][]byte{},
+		}
+		g.Expect(env.Create(ctx, newSecret)).To(Succeed())
+		defer func() {
+			g.Expect(env.Delete(ctx, newSecret)).To(Succeed())
+		}()
+
+		cmKey := client.ObjectKey{
+			Namespace: defaultNamespaceName,
+			Name:      newSecretName,
+		}
+		g.Eventually(func() bool {
+			m := &corev1.Secret{}
+			err := env.Get(ctx, cmKey, m)
+			return err == nil
+		}, timeout).Should(BeTrue())
+
+		// When the Secret resource is created, CRS should get reconciled immediately.
+		g.Eventually(func() error {
+			binding := &addonsv1.ClusterResourceSetBinding{}
+			if err := env.Get(ctx, clusterResourceSetBindingKey, binding); err != nil {
+				return err
+			}
+			if len(binding.Spec.Bindings[0].Resources) > 0 && binding.Spec.Bindings[0].Resources[0].Name == newSecretName {
+				return nil
+			}
+			return errors.Errorf("ClusterResourceSet binding does not have any resources matching %q: %v", newSecretName, binding.Spec.Bindings)
+		}, timeout).Should(Succeed())
+
+		t.Log("Verifying ClusterResourceSetBinding is deleted when its cluster owner reference is deleted")
+		g.Expect(env.Delete(ctx, testCluster)).To(Succeed())
+
+		g.Eventually(func() bool {
+			binding := &addonsv1.ClusterResourceSetBinding{}
+			err := env.Get(ctx, clusterResourceSetBindingKey, binding)
+			return apierrors.IsNotFound(err)
+		}, timeout).Should(BeTrue())
+	})
+
 	t.Run("Should delete ClusterResourceSet from the bindings list when ClusterResourceSet is deleted", func(t *testing.T) {
 		g := NewWithT(t)
 		setup(t, g)

exp/addons/controllers/predicates/resource_predicates.go

@@ -19,8 +19,6 @@ package predicates
 
 import (
 	"github.com/go-logr/logr"
-	corev1 "k8s.io/api/core/v1"
-	addonsv1 "sigs.k8s.io/cluster-api/exp/addons/api/v1alpha4"
 	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 )
@@ -34,28 +32,3 @@ func ResourceCreate(logger logr.Logger) predicate.Funcs {
 		GenericFunc: func(e event.GenericEvent) bool { return false },
 	}
 }
-
-// AddonsSecretCreate returns a predicate that returns true for a Secret create event if in addons Secret type.
-func AddonsSecretCreate(logger logr.Logger) predicate.Funcs {
-	log := logger.WithValues("predicate", "SecretCreateOrUpdate")
-
-	return predicate.Funcs{
-		CreateFunc: func(e event.CreateEvent) bool {
-			log = log.WithValues("eventType", "create")
-			s, ok := e.Object.(*corev1.Secret)
-			if !ok {
-				log.V(4).Info("Expected Secret", "secret", e.Object.GetObjectKind().GroupVersionKind().String())
-				return false
-			}
-			if string(s.Type) != string(addonsv1.ClusterResourceSetSecretType) {
-				log.V(4).Info("Expected Secret Type", "type", addonsv1.SecretClusterResourceSetResourceKind,
-					"got", string(s.Type))
-				return false
-			}
-			return true
-		},
-		UpdateFunc:  func(e event.UpdateEvent) bool { return false },
-		DeleteFunc:  func(e event.DeleteEvent) bool { return false },
-		GenericFunc: func(e event.GenericEvent) bool { return false },
-	}
-}