fix(watch): enqueue CR when target namespace is deleted (#950)

* fix(watch): enqueue CR when target namespace is deleted

* Add tests for controller watches

* Fix rebase

internal/controllers/certmanager.go

@@ -118,7 +118,8 @@ func (r *Reconciler) setupTLS(ctx context.Context, cr *model.CryostatInstance) (
 				},
 				Type: corev1.SecretTypeOpaque,
 			}
-			err = r.createOrUpdateCertSecret(ctx, namespaceSecret, caBytes)
+			err = r.createOrUpdateCertSecret(ctx, namespaceSecret, caBytes,
+				common.LabelsForTargetNamespaceObject(cr))
 			if err != nil {
 				return nil, err
 			}
@@ -377,6 +378,8 @@ func (r *Reconciler) reconcileAgentCertificate(ctx context.Context, cert *certv1
 			},
 		}
 		err = r.createOrUpdateSecret(ctx, targetSecret, nil, func() error {
+			common.MergeLabelsAndAnnotations(&targetSecret.ObjectMeta,
+				common.LabelsForTargetNamespaceObject(cr), map[string]string{})
 			targetSecret.Data = secret.Data
 			return nil
 		})
@@ -436,8 +439,10 @@ func (r *Reconciler) createOrUpdateKeystoreSecret(ctx context.Context, secret *c
 	return nil
 }
 
-func (r *Reconciler) createOrUpdateCertSecret(ctx context.Context, secret *corev1.Secret, cert []byte) error {
+func (r *Reconciler) createOrUpdateCertSecret(ctx context.Context, secret *corev1.Secret, cert []byte,
+	labels map[string]string) error {
 	op, err := controllerutil.CreateOrUpdate(ctx, r.Client, secret, func() error {
+		common.MergeLabelsAndAnnotations(&secret.ObjectMeta, labels, map[string]string{})
 		if secret.Data == nil {
 			secret.Data = map[string][]byte{}
 		}

internal/controllers/common/builder.go

@@ -0,0 +1,81 @@
+// Copyright The Cryostat 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
+//
+// http://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 (
+	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/handler"
+	"sigs.k8s.io/controller-runtime/pkg/predicate"
+	"sigs.k8s.io/controller-runtime/pkg/reconcile"
+)
+
+// ControllerBuilder wraps controller-runtime's builder.Builder
+// as an interface to aid testing.
+type ControllerBuilder interface {
+	For(object client.Object, opts ...builder.ForOption) ControllerBuilder
+	Owns(object client.Object, opts ...builder.OwnsOption) ControllerBuilder
+	Watches(object client.Object, eventHandler handler.EventHandler, opts ...builder.WatchesOption) ControllerBuilder
+	Complete(r reconcile.Reconciler) error
+	EnqueueRequestsFromMapFunc(fn handler.MapFunc) handler.EventHandler
+	WithPredicates(predicates ...predicate.Predicate) builder.Predicates
+}
+
+type ctrlBuilder struct {
+	impl *builder.Builder
+}
+
+var _ ControllerBuilder = (*ctrlBuilder)(nil)
+
+// NewControllerBuilder returns a new ControllerBuilder for the provided manager.
+func NewControllerBuilder(mgr ctrl.Manager) ControllerBuilder {
+	return &ctrlBuilder{
+		impl: ctrl.NewControllerManagedBy(mgr),
+	}
+}
+
+// For wraps the [builder.Builder.For] method
+func (b *ctrlBuilder) For(object client.Object, opts ...builder.ForOption) ControllerBuilder {
+	b.impl = b.impl.For(object, opts...)
+	return b
+}
+
+// Owns wraps the [builder.Builder.Owns] method
+func (b *ctrlBuilder) Owns(object client.Object, opts ...builder.OwnsOption) ControllerBuilder {
+	b.impl = b.impl.Owns(object, opts...)
+	return b
+}
+
+// Watches wraps the [builder.Builder.Watches] method
+func (b *ctrlBuilder) Watches(object client.Object, eventHandler handler.EventHandler, opts ...builder.WatchesOption) ControllerBuilder {
+	b.impl = b.impl.Watches(object, eventHandler, opts...)
+	return b
+}
+
+// Complete wraps the [builder.Builder.Complete] method
+func (b *ctrlBuilder) Complete(r reconcile.Reconciler) error {
+	return b.impl.Complete(r)
+}
+
+// EnqueueRequestsFromMapFunc wraps the [handler.EnqueueRequestsFromMapFunc] function
+func (b *ctrlBuilder) EnqueueRequestsFromMapFunc(fn handler.MapFunc) handler.EventHandler {
+	return handler.EnqueueRequestsFromMapFunc(fn)
+}
+
+// WithPredicates wraps the [builder.WithPredicates] function
+func (b *ctrlBuilder) WithPredicates(predicates ...predicate.Predicate) builder.Predicates {
+	return builder.WithPredicates(predicates...)
+}

internal/controllers/common/common_utils.go

@@ -23,6 +23,8 @@ import (
 	"strings"
 	"time"
 
+	"github.com/cryostatio/cryostat-operator/internal/controllers/constants"
+	"github.com/cryostatio/cryostat-operator/internal/controllers/model"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -114,6 +116,15 @@ func MergeLabelsAndAnnotations(dest *metav1.ObjectMeta, srcLabels, srcAnnotation
 	}
 }
 
+// LabelsForTargetNamespaceObject returns a set of labels for an object in a
+// target namespace that refer back to the CR associated with the object.
+func LabelsForTargetNamespaceObject(cr *model.CryostatInstance) map[string]string {
+	return map[string]string{
+		constants.TargetNamespaceCRNameLabel:      cr.Name,
+		constants.TargetNamespaceCRNamespaceLabel: cr.InstallNamespace,
+	}
+}
+
 // SeccompProfile returns a SeccompProfile for the restricted
 // Pod Security Standard that, on OpenShift, is backwards-compatible
 // with OpenShift < 4.11.

internal/controllers/constants/constants.go

@@ -41,4 +41,8 @@ const (
 	DatabaseSecretConnectionKey = "CONNECTION_KEY"
 	// DatabaseSecretEncryptionKey indexes the database encryption key within the Cryostat database Secret
 	DatabaseSecretEncryptionKey = "ENCRYPTION_KEY"
+
+	targetNamespaceCRLabelPrefix    = "operator.cryostat.io/"
+	TargetNamespaceCRNameLabel      = targetNamespaceCRLabelPrefix + "name"
+	TargetNamespaceCRNamespaceLabel = targetNamespaceCRLabelPrefix + "namespace"
 )

internal/controllers/cryostat_controller.go

@@ -104,7 +104,7 @@ func (r *CryostatReconciler) Reconcile(ctx context.Context, request ctrl.Request
 
 // SetupWithManager sets up the controller with the Manager.
 func (r *CryostatReconciler) SetupWithManager(mgr ctrl.Manager) error {
-	return r.delegate.setupWithManager(mgr, r)
+	return r.delegate.setupWithManager(r.NewControllerBuilder(mgr), r)
 }
 
 func (r *CryostatReconciler) GetConfig() *ReconcilerConfig {

internal/controllers/rbac.go

@@ -137,7 +137,9 @@ func (r *Reconciler) reconcileRoleBinding(ctx context.Context, cr *model.Cryosta
 			Kind:     "ClusterRole",
 			Name:     "cryostat-operator-cryostat-namespaced",
 		}
-		err := r.createOrUpdateRoleBinding(ctx, binding, cr.Object, subjects, roleRef)
+
+		err := r.createOrUpdateRoleBinding(ctx, binding, cr.Object, subjects, roleRef,
+			common.LabelsForTargetNamespaceObject(cr))
 		if err != nil {
 			return err
 		}
@@ -241,9 +243,11 @@ func (r *Reconciler) cleanUpRole(ctx context.Context, cr *model.CryostatInstance
 }
 
 func (r *Reconciler) createOrUpdateRoleBinding(ctx context.Context, binding *rbacv1.RoleBinding,
-	owner metav1.Object, subjects []rbacv1.Subject, roleRef *rbacv1.RoleRef) error {
+	owner metav1.Object, subjects []rbacv1.Subject, roleRef *rbacv1.RoleRef,
+	labels map[string]string) error {
 	bindingCopy := binding.DeepCopy()
 	op, err := controllerutil.CreateOrUpdate(ctx, r.Client, binding, func() error {
+		common.MergeLabelsAndAnnotations(&binding.ObjectMeta, labels, map[string]string{})
 		// Update the list of Subjects
 		binding.Subjects = subjects
 		// Update the Role reference
@@ -256,7 +260,7 @@ func (r *Reconciler) createOrUpdateRoleBinding(ctx context.Context, binding *rba
 	})
 	if err != nil {
 		if err == errRoleRefModified {
-			return r.recreateRoleBinding(ctx, bindingCopy, owner, subjects, roleRef)
+			return r.recreateRoleBinding(ctx, bindingCopy, owner, subjects, roleRef, labels)
 		}
 		return err
 	}
@@ -281,13 +285,13 @@ func getRoleRef(binding metav1.Object, oldRef *rbacv1.RoleRef, newRef *rbacv1.Ro
 }
 
 func (r *Reconciler) recreateRoleBinding(ctx context.Context, binding *rbacv1.RoleBinding, owner metav1.Object,
-	subjects []rbacv1.Subject, roleRef *rbacv1.RoleRef) error {
+	subjects []rbacv1.Subject, roleRef *rbacv1.RoleRef, labels map[string]string) error {
 	// Delete and recreate role binding
 	err := r.deleteRoleBinding(ctx, binding)
 	if err != nil {
 		return err
 	}
-	return r.createOrUpdateRoleBinding(ctx, binding, owner, subjects, roleRef)
+	return r.createOrUpdateRoleBinding(ctx, binding, owner, subjects, roleRef, labels)
 }
 
 func (r *Reconciler) deleteRoleBinding(ctx context.Context, binding *rbacv1.RoleBinding) error {

internal/controllers/reconciler.go

@@ -27,6 +27,7 @@ import (
 	operatorv1beta2 "github.com/cryostatio/cryostat-operator/api/v1beta2"
 	common "github.com/cryostatio/cryostat-operator/internal/controllers/common"
 	resources "github.com/cryostatio/cryostat-operator/internal/controllers/common/resource_definitions"
+	"github.com/cryostatio/cryostat-operator/internal/controllers/constants"
 	"github.com/cryostatio/cryostat-operator/internal/controllers/model"
 	"github.com/go-logr/logr"
 	"github.com/google/go-cmp/cmp"
@@ -47,6 +48,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 )
 
@@ -61,6 +63,7 @@ type ReconcilerConfig struct {
 	EventRecorder          record.EventRecorder
 	RESTMapper             meta.RESTMapper
 	InsightsProxy          *url.URL // Only defined if Insights is enabled
+	NewControllerBuilder   func(ctrl.Manager) common.ControllerBuilder
 	common.ReconcilerTLS
 }
 
@@ -307,13 +310,12 @@ func (r *Reconciler) reconcileCryostat(ctx context.Context, cr *model.CryostatIn
 	return reconcile.Result{}, nil
 }
 
-func (r *Reconciler) setupWithManager(mgr ctrl.Manager, impl reconcile.Reconciler) error {
-	c := ctrl.NewControllerManagedBy(mgr).
-		For(r.objectType)
+func (r *Reconciler) setupWithManager(c common.ControllerBuilder, impl reconcile.Reconciler) error {
+	c = c.For(r.objectType)
 
 	// Watch for changes to secondary resources and requeue the owner Cryostat
-	resources := []client.Object{&appsv1.Deployment{}, &corev1.Service{}, &corev1.Secret{}, &corev1.PersistentVolumeClaim{},
-		&corev1.ServiceAccount{}, &rbacv1.Role{}, &rbacv1.RoleBinding{}, &netv1.Ingress{}}
+	resources := []client.Object{&appsv1.Deployment{}, &corev1.Service{}, &corev1.ConfigMap{}, &corev1.Secret{},
+		&corev1.PersistentVolumeClaim{}, &corev1.ServiceAccount{}, &rbacv1.Role{}, &rbacv1.RoleBinding{}, &netv1.Ingress{}}
 	if r.IsOpenShift {
 		resources = append(resources, &openshiftv1.Route{})
 	}
@@ -326,6 +328,12 @@ func (r *Reconciler) setupWithManager(mgr ctrl.Manager, impl reconcile.Reconcile
 		c = c.Owns(resource)
 	}
 
+	// Watch objects that we create in target namespaces
+	c, err := r.watchTargetNamespaces(c, &rbacv1.RoleBinding{}, &corev1.Secret{})
+	if err != nil {
+		return err
+	}
+
 	return c.Complete(impl)
 }
 
@@ -536,6 +544,64 @@ func (r *Reconciler) deleteDeployment(ctx context.Context, deploy *appsv1.Deploy
 	return nil
 }
 
+func (r *Reconciler) watchTargetNamespaces(c common.ControllerBuilder,
+	resources ...client.Object) (common.ControllerBuilder, error) {
+	// Create a controller watch for resources we create in target namespaces.
+	// The watch filters objects for those that have our labels that identify their CR,
+	// and then enqueues that CR.
+	pred, err := r.targetNamespacePredicate()
+	if err != nil {
+		return nil, err
+	}
+	for _, resource := range resources {
+		c = c.Watches(resource, c.EnqueueRequestsFromMapFunc(r.mapFromTargetNamespace()),
+			c.WithPredicates(pred))
+	}
+	return c, nil
+}
+
+func (r *Reconciler) targetNamespacePredicate() (predicate.Predicate, error) {
+	// Use a label selector that matches the existence of the
+	// target namespace labels
+	selector := metav1.LabelSelector{}
+	labels := []string{
+		constants.TargetNamespaceCRNameLabel,
+		constants.TargetNamespaceCRNamespaceLabel,
+	}
+	for _, label := range labels {
+		selector.MatchExpressions = append(selector.MatchExpressions, metav1.LabelSelectorRequirement{
+			Key:      label,
+			Operator: metav1.LabelSelectorOpExists,
+		})
+	}
+	return predicate.LabelSelectorPredicate(selector)
+}
+
+func (r *Reconciler) mapFromTargetNamespace() func(ctx context.Context, obj client.Object) []reconcile.Request {
+	return func(ctx context.Context, obj client.Object) []reconcile.Request {
+		// Get the namespace/name of the CR from the object's labels,
+		// and return a reconcile request for that namespaced name
+		labels := obj.GetLabels()
+		name, ok := labels[constants.TargetNamespaceCRNameLabel]
+		if !ok {
+			r.Log.Info("Object is missing label", "label", constants.TargetNamespaceCRNameLabel,
+				"name", obj.GetName(), "namespace", obj.GetNamespace())
+			return nil
+		}
+		namespace, ok := labels[constants.TargetNamespaceCRNamespaceLabel]
+		if !ok {
+			r.Log.Info("Object is missing label", "label", constants.TargetNamespaceCRNamespaceLabel,
+				"name", obj.GetName(), "namespace", obj.GetNamespace())
+			return nil
+		}
+		return []reconcile.Request{
+			{
+				NamespacedName: types.NamespacedName{Namespace: namespace, Name: name},
+			},
+		}
+	}
+}
+
 func requeueIfIngressNotReady(log logr.Logger, err error) (reconcile.Result, error) {
 	if err == ErrIngressNotReady {
 		return reconcile.Result{RequeueAfter: 5 * time.Second}, nil