From e1cc7e6b71b5e4063944ca76c507a21204e2438b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20Valais?= Date: Fri, 15 Dec 2023 11:37:30 +0100 Subject: [PATCH] add tests around whether or not a sync should happen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maël Valais --- internal/controller/controller.go | 45 +++++++++----- internal/controller/controller_test.go | 85 ++++++++++++++++++++++++++ 2 files changed, 114 insertions(+), 16 deletions(-) create mode 100644 internal/controller/controller_test.go diff --git a/internal/controller/controller.go b/internal/controller/controller.go index 66b56e6..005a80b 100644 --- a/internal/controller/controller.go +++ b/internal/controller/controller.go @@ -43,6 +43,30 @@ type Route struct { log logr.Logger } +func shouldSync(log logr.Logger, route *routev1.Route) bool { + if len(route.ObjectMeta.OwnerReferences) > 0 { + for _, o := range route.ObjectMeta.OwnerReferences { + if o.Kind == "Ingress" { + log.V(5).Info("Route is owned by an Ingress") + return false + } + } + } + + if metav1.HasAnnotation(route.ObjectMeta, cmapi.IssuerNameAnnotationKey) { + log.V(5).Info("Route has the annotation %s=%s", cmapi.IssuerNameAnnotationKey, route.Annotations[cmapi.IssuerNameAnnotationKey]) + return true + } + + if metav1.HasAnnotation(route.ObjectMeta, cmapi.IngressIssuerNameAnnotationKey) { + log.V(5).Info("Route has the annotation %s=%s", cmapi.IngressIssuerNameAnnotationKey, route.Annotations[cmapi.IngressIssuerNameAnnotationKey]) + return true + } + + log.V(5).Info("Route does not have the cert-manager issuer annotation") + return false +} + func (r *Route) Reconcile(ctx context.Context, req reconcile.Request) (reconcile.Result, error) { log := r.log.WithValues("object", req.NamespacedName) log.V(5).Info("started reconciling") @@ -53,24 +77,13 @@ func (r *Route) Reconcile(ctx context.Context, req reconcile.Request) (reconcile if err != nil { return reconcile.Result{}, err } - if len(route.ObjectMeta.OwnerReferences) > 0 { - for _, o := range route.ObjectMeta.OwnerReferences { - if o.Kind == "Ingress" { - log.V(5).Info("ignoring route owned by ingress", o.Name) - return reconcile.Result{}, nil - } - } - } log.V(5).Info("retrieved route") - if metav1.HasAnnotation(route.ObjectMeta, cmapi.IssuerNameAnnotationKey) { - log.V(5).Info("route has cert-manager annotation, reconciling", cmapi.IssuerNameAnnotationKey, route.Annotations[cmapi.IssuerNameAnnotationKey]) - return r.sync(ctx, req, route.DeepCopy()) - } else if metav1.HasAnnotation(route.ObjectMeta, cmapi.IngressIssuerNameAnnotationKey) { - log.V(5).Info("route has cert-manager annotation, reconciling", cmapi.IngressIssuerNameAnnotationKey, route.Annotations[cmapi.IngressIssuerNameAnnotationKey]) - return r.sync(ctx, req, route.DeepCopy()) + + if !shouldSync(log, route) { + return reconcile.Result{}, nil } - log.V(5).Info("ignoring route without cert-manager issuer name annotation") - return reconcile.Result{}, nil + + return r.sync(ctx, req, route.DeepCopy()) } func New(base logr.Logger, config *rest.Config, recorder record.EventRecorder) (*Route, error) { diff --git a/internal/controller/controller_test.go b/internal/controller/controller_test.go new file mode 100644 index 0000000..0e709a2 --- /dev/null +++ b/internal/controller/controller_test.go @@ -0,0 +1,85 @@ +/* +Copyright 2022 The cert-manager 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 controller + +import ( + "testing" + + "github.com/go-logr/logr" + routev1 "github.com/openshift/api/route/v1" + "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func Test_shouldReconcile(t *testing.T) { + tests := []struct { + name string + given *routev1.Route + want bool + }{ + { + name: "should reconcile with cert-manager.io/issuer-name annotation", + given: &routev1.Route{ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "cert-manager.io/issuer-name": "test", + }}, + }, + want: true, + }, + { + name: "should sync with cert-manager.io/issuer annotation", + given: &routev1.Route{ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "cert-manager.io/issuer": "test", + }}, + }, + want: true, + }, + { + name: "should not sync when Route owned by Ingress", + given: &routev1.Route{ObjectMeta: metav1.ObjectMeta{ + OwnerReferences: []metav1.OwnerReference{ + { + Kind: "Ingress", + }, + }}, + }, + want: false, + }, + { + name: "should not sync when Route owned by Ingress", + given: &routev1.Route{ObjectMeta: metav1.ObjectMeta{ + OwnerReferences: []metav1.OwnerReference{ + { + Kind: "Ingress", + }, + }}, + }, + want: false, + }, + { + name: "should not sync when no annotation is found", + given: &routev1.Route{ObjectMeta: metav1.ObjectMeta{}}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := shouldSync(logr.Discard(), tt.given) + assert.Equal(t, tt.want, got) + }) + } +}