From 1bdfc3e61ab7fd4e7e7759f0007a09ad27efb057 Mon Sep 17 00:00:00 2001 From: Elliott Baron Date: Fri, 13 Sep 2024 13:48:33 -0400 Subject: [PATCH] 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 | 9 +- internal/controllers/common/builder.go | 81 ++++++ internal/controllers/common/common_utils.go | 11 + internal/controllers/constants/constants.go | 4 + internal/controllers/cryostat_controller.go | 2 +- internal/controllers/rbac.go | 14 +- internal/controllers/reconciler.go | 76 ++++- internal/controllers/reconciler_test.go | 300 +++++++++++++++++++- internal/main.go | 1 + internal/test/builder.go | 105 +++++++ internal/test/reconciler.go | 2 + internal/test/resources.go | 39 +++ 12 files changed, 616 insertions(+), 28 deletions(-) create mode 100644 internal/controllers/common/builder.go create mode 100644 internal/test/builder.go diff --git a/internal/controllers/certmanager.go b/internal/controllers/certmanager.go index 8bc09482b..2fb795889 100644 --- a/internal/controllers/certmanager.go +++ b/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{} } diff --git a/internal/controllers/common/builder.go b/internal/controllers/common/builder.go new file mode 100644 index 000000000..f16bf90c5 --- /dev/null +++ b/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...) +} diff --git a/internal/controllers/common/common_utils.go b/internal/controllers/common/common_utils.go index 2aa4a19d2..b1be06bc1 100644 --- a/internal/controllers/common/common_utils.go +++ b/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. diff --git a/internal/controllers/constants/constants.go b/internal/controllers/constants/constants.go index 862d742bd..678721895 100644 --- a/internal/controllers/constants/constants.go +++ b/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" ) diff --git a/internal/controllers/cryostat_controller.go b/internal/controllers/cryostat_controller.go index 8a4042bf2..2abdd6144 100644 --- a/internal/controllers/cryostat_controller.go +++ b/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 { diff --git a/internal/controllers/rbac.go b/internal/controllers/rbac.go index cd4cb4a6b..9c9bcb482 100644 --- a/internal/controllers/rbac.go +++ b/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 { diff --git a/internal/controllers/reconciler.go b/internal/controllers/reconciler.go index 371918d67..18759da44 100644 --- a/internal/controllers/reconciler.go +++ b/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 diff --git a/internal/controllers/reconciler_test.go b/internal/controllers/reconciler_test.go index 6aa975e66..c18262a46 100644 --- a/internal/controllers/reconciler_test.go +++ b/internal/controllers/reconciler_test.go @@ -23,6 +23,7 @@ import ( certv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + gomegatypes "github.com/onsi/gomega/types" configv1 "github.com/openshift/api/config/v1" consolev1 "github.com/openshift/api/console/v1" openshiftv1 "github.com/openshift/api/route/v1" @@ -39,10 +40,14 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/client-go/tools/record" + "sigs.k8s.io/controller-runtime/pkg/builder" ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/handler" logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" operatorv1beta2 "github.com/cryostatio/cryostat-operator/api/v1beta2" @@ -67,6 +72,7 @@ func (c *controllerTest) commonBeforeEach() *cryostatTestInput { t := &cryostatTestInput{ TestReconcilerConfig: test.TestReconcilerConfig{ GeneratedPasswords: []string{"auth_cookie_secret", "connection_key", "encryption_key", "object_storage", "keystore"}, + ControllerBuilder: &test.TestCtrlBuilder{}, }, TestResources: &test.TestResources{ Name: "cryostat", @@ -115,14 +121,16 @@ func (t *cryostatTestInput) newReconcilerConfig(scheme *runtime.Scheme, client c insightsURL = url } return &controllers.ReconcilerConfig{ - Client: test.NewClientWithTimestamp(test.NewTestClient(client, t.TestResources)), - Scheme: scheme, - IsOpenShift: t.OpenShift, - EventRecorder: record.NewFakeRecorder(1024), - RESTMapper: test.NewTESTRESTMapper(), - Log: logger, - ReconcilerTLS: test.NewTestReconcilerTLS(&t.TestReconcilerConfig), - InsightsProxy: insightsURL, + Client: test.NewClientWithTimestamp(test.NewTestClient(client, t.TestResources)), + Scheme: scheme, + IsOpenShift: t.OpenShift, + EventRecorder: record.NewFakeRecorder(1024), + RESTMapper: test.NewTESTRESTMapper(), + Log: logger, + ReconcilerTLS: test.NewTestReconcilerTLS(&t.TestReconcilerConfig), + InsightsProxy: insightsURL, + IsCertManagerInstalled: !t.CertManagerMissing, + NewControllerBuilder: test.NewControllerBuilder(&t.TestReconcilerConfig), } } @@ -424,8 +432,9 @@ func (c *controllerTest) commonTests() { err := t.Client.Get(context.Background(), types.NamespacedName{Name: expected.Name, Namespace: expected.Namespace}, binding) Expect(err).ToNot(HaveOccurred()) - // Labels are unaffected - Expect(binding.Labels).To(Equal(oldBinding.Labels)) + // Labels are merged with existing ones + metav1.SetMetaDataLabel(&expected.ObjectMeta, "test", "label") + Expect(binding.Labels).To(Equal(expected.Labels)) Expect(binding.Annotations).To(Equal(oldBinding.Annotations)) // Subjects and RoleRef should be fully replaced @@ -1851,7 +1860,7 @@ func (c *controllerTest) commonTests() { Expect(err).ToNot(BeNil()) Expect(kerrors.IsNotFound(err)).To(BeTrue()) }) - It("leave certficate secrets for the first namespace", func() { + It("leave certificate secrets for the first namespace", func() { t.expectCertificates() }) It("should remove CA cert secret from the second namespace", func() { @@ -2144,8 +2153,9 @@ func (c *controllerTest) commonTests() { err := t.Client.Get(context.Background(), types.NamespacedName{Name: expected.Name, Namespace: expected.Namespace}, binding) Expect(err).ToNot(HaveOccurred()) - // Labels are unaffected - Expect(binding.Labels).To(Equal(oldBinding.Labels)) + // Labels are merged with existing ones + metav1.SetMetaDataLabel(&expected.ObjectMeta, "test", "label") + Expect(binding.Labels).To(Equal(expected.Labels)) Expect(binding.Annotations).To(Equal(oldBinding.Annotations)) // Subjects and RoleRef should be fully replaced @@ -2194,6 +2204,245 @@ func (c *controllerTest) commonTests() { }) }) }) + + Describe("setting up with manager", func() { + BeforeEach(func() { + t = c.commonBeforeEach() + t.TargetNamespaces = []string{t.Namespace} + }) + + JustBeforeEach(func() { + c.commonJustBeforeEach(t) + // Create a default manager, not called + mgr, err := manager.New(cfg, manager.Options{}) + Expect(err).ToNot(HaveOccurred()) + t.controller.SetupWithManager(mgr) + }) + + JustAfterEach(func() { + c.commonJustAfterEach(t) + }) + + It("should watch Cryostat CRs", func() { + Expect(t.ControllerBuilder.ForCalls).To(HaveLen(1)) + args := t.ControllerBuilder.ForCalls[0] + Expect(args.Object).To(BeAssignableToTypeOf(t.NewCryostat().Object)) + Expect(args.Opts).To(BeEmpty()) + }) + + It("should call Complete", func() { + Expect(t.ControllerBuilder.CompleteCalled).To(BeTrue()) + }) + + Context("for owned resources", func() { + var ownsResources []ctrlclient.Object + + BeforeEach(func() { + ownsResources = []ctrlclient.Object{ + &appsv1.Deployment{}, + &corev1.Service{}, + &corev1.ConfigMap{}, + &corev1.Secret{}, + &corev1.PersistentVolumeClaim{}, + &corev1.ServiceAccount{}, + &rbacv1.Role{}, + &rbacv1.RoleBinding{}, + &netv1.Ingress{}, + } + }) + + expectOwnedResources := func() { + It("should watch objects owned by Cryostat CRs", func() { + Expect(t.ControllerBuilder.OwnsCalls).To(HaveLen(len(ownsResources))) + resources := []ctrlclient.Object{} + for _, call := range t.ControllerBuilder.OwnsCalls { + resources = append(resources, call.Object) + Expect(call.Opts).To(BeEmpty()) + } + Expect(resources).To(ConsistOf(ownsResources)) + }) + } + + Context("cert-manager installed", func() { + BeforeEach(func() { + ownsResources = append(ownsResources, &certv1.Certificate{}, &certv1.Issuer{}) + }) + Context("on OpenShift", func() { + BeforeEach(func() { + ownsResources = append(ownsResources, &openshiftv1.Route{}) + }) + expectOwnedResources() + }) + Context("on Kubernetes", func() { + BeforeEach(func() { + t.OpenShift = false + }) + expectOwnedResources() + }) + }) + + Context("cert-manager missing", func() { + BeforeEach(func() { + t.CertManagerMissing = true + }) + Context("on OpenShift", func() { + BeforeEach(func() { + ownsResources = append(ownsResources, &openshiftv1.Route{}) + }) + expectOwnedResources() + }) + Context("on Kubernetes", func() { + BeforeEach(func() { + t.OpenShift = false + }) + expectOwnedResources() + }) + }) + }) + + Context("watches in target namespaces", func() { + var expectedResources []ctrlclient.Object + + BeforeEach(func() { + expectedResources = []ctrlclient.Object{ + &rbacv1.RoleBinding{}, + &corev1.Secret{}, + } + }) + + It("should watch specified resources", func() { + Expect(t.ControllerBuilder.WatchesCalls).To(HaveLen(len(expectedResources))) + resources := []ctrlclient.Object{} + for _, watch := range t.ControllerBuilder.WatchesCalls { + resources = append(resources, watch.Object) + } + Expect(resources).To(ConsistOf(expectedResources)) + }) + + Context("filtering by labels", func() { + var pred predicate.Predicate + var obj ctrlclient.Object + + JustBeforeEach(func() { + Expect(t.ControllerBuilder.WatchesCalls).To(HaveLen(len(expectedResources))) + Expect(t.ControllerBuilder.Predicates).To(HaveLen(len(expectedResources))) + for _, watch := range t.ControllerBuilder.WatchesCalls { + Expect(watch.Opts).To(HaveLen(1)) + Expect(watch.Opts[0]).To(BeAssignableToTypeOf(builder.Predicates{})) + } + pred = t.ControllerBuilder.Predicates[0] + }) + + Context("with both labels present", func() { + BeforeEach(func() { + obj = t.NewAgentCertSecretCopy("foo") + }) + + It("should accept", func() { + t.expectPredicateToAccept(pred, obj) + }) + }) + + Context("with name label missing", func() { + BeforeEach(func() { + obj = t.NewAgentCertSecretCopy("foo") + delete(obj.GetLabels(), "operator.cryostat.io/name") + }) + + It("should reject", func() { + t.expectPredicateToReject(pred, obj) + }) + }) + + Context("with namespace label missing", func() { + BeforeEach(func() { + obj = t.NewAgentCertSecretCopy("foo") + delete(obj.GetLabels(), "operator.cryostat.io/namespace") + }) + + It("should reject", func() { + t.expectPredicateToReject(pred, obj) + }) + }) + + Context("both labels missing", func() { + BeforeEach(func() { + obj = t.NewAgentCertSecret("foo") + delete(obj.GetLabels(), "operator.cryostat.io/name") + }) + + It("should reject", func() { + t.expectPredicateToReject(pred, obj) + }) + }) + }) + + Context("handling events", func() { + var handlerFunc handler.MapFunc + var obj ctrlclient.Object + + JustBeforeEach(func() { + Expect(t.ControllerBuilder.WatchesCalls).To(HaveLen(len(expectedResources))) + Expect(t.ControllerBuilder.MapFuncs).To(HaveLen(len(expectedResources))) + for i, watch := range t.ControllerBuilder.WatchesCalls { + Expect(watch.EventHandler).ToNot(BeNil()) + // Check that the handler uses the expected underlying type + mapFunc := t.ControllerBuilder.MapFuncs[i] + expectedHandler := handler.EnqueueRequestsFromMapFunc(mapFunc) + Expect(watch.EventHandler).To(BeAssignableToTypeOf(expectedHandler)) + } + handlerFunc = t.ControllerBuilder.MapFuncs[0] + }) + + Context("with both labels present", func() { + BeforeEach(func() { + obj = t.NewAgentCertSecretCopy("foo") + }) + + It("should accept", func() { + result := handlerFunc(context.Background(), obj) + Expect(result).To(ConsistOf(newReconcileRequest(t.Namespace, t.Name))) + }) + }) + + Context("with name label missing", func() { + BeforeEach(func() { + obj = t.NewAgentCertSecretCopy("foo") + delete(obj.GetLabels(), "operator.cryostat.io/name") + }) + + It("should reject", func() { + result := handlerFunc(context.Background(), obj) + Expect(result).To(BeEmpty()) + }) + }) + + Context("with namespace label missing", func() { + BeforeEach(func() { + obj = t.NewAgentCertSecretCopy("foo") + delete(obj.GetLabels(), "operator.cryostat.io/namespace") + }) + + It("should reject", func() { + result := handlerFunc(context.Background(), obj) + Expect(result).To(BeEmpty()) + }) + }) + + Context("both labels missing", func() { + BeforeEach(func() { + obj = t.NewAgentCertSecret("foo") + delete(obj.GetLabels(), "operator.cryostat.io/name") + }) + + It("should reject", func() { + result := handlerFunc(context.Background(), obj) + Expect(result).To(BeEmpty()) + }) + }) + }) + }) + }) } func (t *cryostatTestInput) expectRoutes() { @@ -3058,11 +3307,16 @@ func (t *cryostatTestInput) reconcile() (reconcile.Result, error) { } func (t *cryostatTestInput) reconcileWithName(name string) (reconcile.Result, error) { - nsName := types.NamespacedName{Name: name, Namespace: t.Namespace} - req := reconcile.Request{NamespacedName: nsName} + req := newReconcileRequest(t.Namespace, name) return t.controller.Reconcile(context.Background(), req) } +func newReconcileRequest(namespace string, name string) reconcile.Request { + return reconcile.Request{ + NamespacedName: types.NamespacedName{Namespace: namespace, Name: name}, + } +} + func (t *cryostatTestInput) expectConsoleLink() { link := &consolev1.ConsoleLink{} expectedLink := t.NewConsoleLink() @@ -3075,3 +3329,19 @@ func (t *cryostatTestInput) expectTargetNamespaces() { cr := t.getCryostatInstance() Expect(*cr.TargetNamespaceStatus).To(ConsistOf(t.TargetNamespaces)) } + +func (t *cryostatTestInput) expectPredicateToAccept(pred predicate.Predicate, obj ctrlclient.Object) { + t.expectPredicate(pred, obj, BeTrue()) +} + +func (t *cryostatTestInput) expectPredicateToReject(pred predicate.Predicate, obj ctrlclient.Object) { + t.expectPredicate(pred, obj, BeFalse()) +} + +func (t *cryostatTestInput) expectPredicate(pred predicate.Predicate, obj ctrlclient.Object, + matcher gomegatypes.GomegaMatcher) { + Expect(pred.Create(t.NewCreateEvent(obj))).To(matcher) + Expect(pred.Update(t.NewUpdateEvent(obj))).To(matcher) + Expect(pred.Delete(t.NewDeleteEvent(obj))).To(matcher) + Expect(pred.Generic(t.NewGenericEvent(obj))).To(matcher) +} diff --git a/internal/main.go b/internal/main.go index 272f0a6b4..611b474af 100644 --- a/internal/main.go +++ b/internal/main.go @@ -229,6 +229,7 @@ func newReconcilerConfig(mgr ctrl.Manager, logName string, eventRecorderName str EventRecorder: mgr.GetEventRecorderFor(eventRecorderName), RESTMapper: mgr.GetRESTMapper(), InsightsProxy: insightsURL, + NewControllerBuilder: common.NewControllerBuilder, ReconcilerTLS: common.NewReconcilerTLS(&common.ReconcilerTLSConfig{ Client: mgr.GetClient(), }), diff --git a/internal/test/builder.go b/internal/test/builder.go new file mode 100644 index 000000000..a8b4c2c4c --- /dev/null +++ b/internal/test/builder.go @@ -0,0 +1,105 @@ +// 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 test + +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" + + "github.com/cryostatio/cryostat-operator/internal/controllers/common" +) + +// TestCtrlBuilder is a fake ControllerBuilder to aid testing of +// controller watches +type TestCtrlBuilder struct { + ForCalls []ForArgs + OwnsCalls []OwnsArgs + WatchesCalls []WatchesArgs + MapFuncs []handler.MapFunc + Predicates []predicate.Predicate + CompleteCalled bool +} + +// ForArgs contain arguments used for a call to For +type ForArgs struct { + Object client.Object + Opts []builder.ForOption +} + +// OwnsArgs contain arguments used for a call to Owns +type OwnsArgs struct { + Object client.Object + Opts []builder.OwnsOption +} + +// WatchesArgs contain arguments used for a call to Watches +type WatchesArgs struct { + Object client.Object + EventHandler handler.EventHandler + Opts []builder.WatchesOption +} + +var _ common.ControllerBuilder = (*TestCtrlBuilder)(nil) + +func NewControllerBuilder(config *TestReconcilerConfig) func(ctrl.Manager) common.ControllerBuilder { + return func(ctrl.Manager) common.ControllerBuilder { + return config.ControllerBuilder + } +} + +func (b *TestCtrlBuilder) For(object client.Object, opts ...builder.ForOption) common.ControllerBuilder { + b.ForCalls = append(b.ForCalls, ForArgs{ + Object: object, + Opts: opts, + }) + return b +} + +func (b *TestCtrlBuilder) Owns(object client.Object, opts ...builder.OwnsOption) common.ControllerBuilder { + b.OwnsCalls = append(b.OwnsCalls, OwnsArgs{ + Object: object, + Opts: opts, + }) + return b +} + +func (b *TestCtrlBuilder) Watches(object client.Object, eventHandler handler.EventHandler, + opts ...builder.WatchesOption) common.ControllerBuilder { + b.WatchesCalls = append(b.WatchesCalls, WatchesArgs{ + Object: object, + EventHandler: eventHandler, + Opts: opts, + }) + return b +} + +func (b *TestCtrlBuilder) Complete(r reconcile.Reconciler) error { + b.CompleteCalled = true + return nil +} + +func (b *TestCtrlBuilder) EnqueueRequestsFromMapFunc(fn handler.MapFunc) handler.EventHandler { + b.MapFuncs = append(b.MapFuncs, fn) + return handler.EnqueueRequestsFromMapFunc(fn) +} + +func (b *TestCtrlBuilder) WithPredicates(predicates ...predicate.Predicate) builder.Predicates { + b.Predicates = append(b.Predicates, predicates...) + return builder.WithPredicates(predicates...) +} diff --git a/internal/test/reconciler.go b/internal/test/reconciler.go index 6972373c8..008332935 100644 --- a/internal/test/reconciler.go +++ b/internal/test/reconciler.go @@ -36,6 +36,8 @@ type TestReconcilerConfig struct { EnvGrafanaImageTag *string EnvReportsImageTag *string GeneratedPasswords []string + ControllerBuilder *TestCtrlBuilder + CertManagerMissing bool } func NewTestReconcilerTLS(config *TestReconcilerConfig) common.ReconcilerTLS { diff --git a/internal/test/resources.go b/internal/test/resources.go index 6f7fdfeb2..53b9c25d9 100644 --- a/internal/test/resources.go +++ b/internal/test/resources.go @@ -41,6 +41,8 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/client-go/kubernetes/scheme" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/event" ) type TestResources struct { @@ -844,6 +846,10 @@ func (r *TestResources) NewCACertSecret(ns string) *corev1.Secret { ObjectMeta: metav1.ObjectMeta{ Name: r.getClusterUniqueNameForCA(), Namespace: ns, + Labels: map[string]string{ + "operator.cryostat.io/name": r.Name, + "operator.cryostat.io/namespace": r.Namespace, + }, }, Type: corev1.SecretTypeOpaque, Data: map[string][]byte{ @@ -868,6 +874,10 @@ func (r *TestResources) NewAgentCertSecret(ns string) *corev1.Secret { func (r *TestResources) NewAgentCertSecretCopy(ns string) *corev1.Secret { secret := r.NewAgentCertSecret(ns) + secret.Labels = map[string]string{ + "operator.cryostat.io/name": r.Name, + "operator.cryostat.io/namespace": r.Namespace, + } secret.Namespace = ns return secret } @@ -2707,6 +2717,10 @@ func (r *TestResources) NewRoleBinding(ns string) *rbacv1.RoleBinding { ObjectMeta: metav1.ObjectMeta{ Name: r.getClusterUniqueName(), Namespace: ns, + Labels: map[string]string{ + "operator.cryostat.io/name": r.Name, + "operator.cryostat.io/namespace": r.Namespace, + }, }, Subjects: []rbacv1.Subject{ { @@ -3101,3 +3115,28 @@ func (r *TestResources) getClusterUniqueNameForAgent(namespace string) string { func (r *TestResources) GetAgentCertPrefix() string { return "cryostat-agent-" } + +func (r *TestResources) NewCreateEvent(obj ctrlclient.Object) event.CreateEvent { + return event.CreateEvent{ + Object: obj, + } +} + +func (r *TestResources) NewUpdateEvent(obj ctrlclient.Object) event.UpdateEvent { + return event.UpdateEvent{ + ObjectOld: obj, + ObjectNew: obj, + } +} + +func (r *TestResources) NewDeleteEvent(obj ctrlclient.Object) event.DeleteEvent { + return event.DeleteEvent{ + Object: obj, + } +} + +func (r *TestResources) NewGenericEvent(obj ctrlclient.Object) event.GenericEvent { + return event.GenericEvent{ + Object: obj, + } +}