Merge pull request #887 from rabbitmq/disable-non-tls-listener

Do not requeue error when disableNonTLSListen is misconfigured

Author: ChunyiLyu

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)

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

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,
 			}