From 4881d468eb9bc41773c642c55b8d680df4dd2409 Mon Sep 17 00:00:00 2001 From: Chunyi Lyu Date: Wed, 3 Nov 2021 12:58:10 +0000 Subject: [PATCH] Do not requeue error when disableNonTLSListen is set to - true but TLS is not enabled; this is a user confguration error and need to be fixed by user updating the rabbitmqcluster - reconcileTLS returns a special error so the controller will exit --- controllers/rabbitmqcluster_controller.go | 8 ++++++-- controllers/reconcile_tls.go | 20 ++++++++++++-------- controllers/reconcile_tls_test.go | 2 +- 3 files changed, 19 insertions(+), 11 deletions(-) diff --git a/controllers/rabbitmqcluster_controller.go b/controllers/rabbitmqcluster_controller.go index d8bf34bcc..fb7821dec 100644 --- a/controllers/rabbitmqcluster_controller.go +++ b/controllers/rabbitmqcluster_controller.go @@ -13,6 +13,7 @@ package controllers import ( "context" "encoding/json" + "errors" "fmt" "reflect" "strings" @@ -150,8 +151,11 @@ func (r *RabbitmqClusterReconciler) Reconcile(ctx context.Context, req ctrl.Requ return ctrl.Result{RequeueAfter: requeueAfter}, err } - if err := r.reconcileTLS(ctx, rabbitmqCluster); err != nil { - return ctrl.Result{}, err + tlsErr := r.reconcileTLS(ctx, rabbitmqCluster) + if errors.Is(tlsErr, disableNonTLSConfigErr) { + return ctrl.Result{}, nil + } else if tlsErr != nil { + return ctrl.Result{}, tlsErr } sts, err := r.statefulSet(ctx, rabbitmqCluster) diff --git a/controllers/reconcile_tls.go b/controllers/reconcile_tls.go index d7020345e..b37bc89bc 100644 --- a/controllers/reconcile_tls.go +++ b/controllers/reconcile_tls.go @@ -2,23 +2,27 @@ package controllers import ( "context" + "errors" "fmt" ctrl "sigs.k8s.io/controller-runtime" rabbitmqv1beta1 "github.com/rabbitmq/cluster-operator/api/v1beta1" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" + k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" ) +var disableNonTLSConfigErr = errors.New("TLS must be enabled if disableNonTLSListeners is set to true") + func (r *RabbitmqClusterReconciler) reconcileTLS(ctx context.Context, rabbitmqCluster *rabbitmqv1beta1.RabbitmqCluster) error { + // if tls.disableNonTLSListeners set to true and TLS is not enabled, it's a configuration error + // reconcileTLS() will return a special error so the operator won't requeue if rabbitmqCluster.DisableNonTLSListeners() && !rabbitmqCluster.TLSEnabled() { - err := errors.NewBadRequest("TLS must be enabled if disableNonTLSListeners is set to true") - r.Recorder.Event(rabbitmqCluster, corev1.EventTypeWarning, "TLSError", err.Error()) - ctrl.LoggerFrom(ctx).Error(err, "Error setting up TLS") - r.setReconcileSuccess(ctx, rabbitmqCluster, corev1.ConditionFalse, "TLSError", err.Error()) - return err + r.Recorder.Event(rabbitmqCluster, corev1.EventTypeWarning, "TLSError", disableNonTLSConfigErr.Error()) + ctrl.LoggerFrom(ctx).Error(disableNonTLSConfigErr, "Error setting up TLS") + r.setReconcileSuccess(ctx, rabbitmqCluster, corev1.ConditionFalse, "TLSError", disableNonTLSConfigErr.Error()) + return disableNonTLSConfigErr } if rabbitmqCluster.SecretTLSEnabled() { @@ -47,7 +51,7 @@ func (r *RabbitmqClusterReconciler) checkTLSSecrets(ctx context.Context, rabbitm _, hasTLSKey := secret.Data["tls.key"] _, hasTLSCert := secret.Data["tls.crt"] if !hasTLSCert || !hasTLSKey { - err := errors.NewBadRequest(fmt.Sprintf("TLS secret %s in namespace %s does not have the fields tls.crt and tls.key", secretName, rabbitmqCluster.Namespace)) + err := k8serrors.NewBadRequest(fmt.Sprintf("TLS secret %s in namespace %s does not have the fields tls.crt and tls.key", secretName, rabbitmqCluster.Namespace)) r.Recorder.Event(rabbitmqCluster, corev1.EventTypeWarning, "TLSError", err.Error()) logger.Error(err, "Error setting up TLS") return err @@ -71,7 +75,7 @@ func (r *RabbitmqClusterReconciler) checkTLSSecrets(ctx context.Context, rabbitm // Mutual TLS: verify that CA certificate is present in secret if _, hasCaCert := secret.Data["ca.crt"]; !hasCaCert { - err := errors.NewBadRequest(fmt.Sprintf("TLS secret %s in namespace %s does not have the field ca.crt", rabbitmqCluster.Spec.TLS.CaSecretName, rabbitmqCluster.Namespace)) + err := k8serrors.NewBadRequest(fmt.Sprintf("TLS secret %s in namespace %s does not have the field ca.crt", rabbitmqCluster.Spec.TLS.CaSecretName, rabbitmqCluster.Namespace)) r.Recorder.Event(rabbitmqCluster, corev1.EventTypeWarning, "TLSError", err.Error()) logger.Error(err, "Error setting up TLS") return err diff --git a/controllers/reconcile_tls_test.go b/controllers/reconcile_tls_test.go index ac28140a8..d6db8d002 100644 --- a/controllers/reconcile_tls_test.go +++ b/controllers/reconcile_tls_test.go @@ -228,7 +228,7 @@ var _ = Describe("Reconcile TLS", func() { }) When("DiableNonTLSListeners set to true", func() { - It("returns an error, logs TLSError and set ReconcileSuccess to false when TLS is not enabled", func() { + It("logs TLSError and set ReconcileSuccess to false when TLS is not enabled", func() { tlsSpec := rabbitmqv1beta1.TLSSpec{ DisableNonTLSListeners: true, }