Share TLS config with management, amqp 1.0, (web) mqtt and (web) stomp plugins (#451)

* Share TLS config with the management, amqp 1.0, (web) mqtt and (web) stomp plugins (#451)

This commit:
- Supports TLS for management 
- Supports TLS for rabbtmq_(web)_mqtt and rabbitmq_(web)_stomp
- Sets hardcoded path for CA certificate to 'ca.crt'
- Adds system tests to verify remote access via HTTPS to the MGMT dashboard, mqtt, and stomp plugins

Co-authored-by: Alex Blease <[email protected]>
Co-authored-by: Chunyi Lyu <[email protected]>

Author: 3 people

api/v1beta1/rabbitmqcluster_types.go

@@ -238,12 +238,9 @@ type TLSSpec struct {
 	// The Secret must store these as tls.key and tls.crt, respectively.
 	SecretName string `json:"secretName,omitempty"`
 	// Name of a Secret in the same Namespace as the RabbitmqCluster, containing the Certificate Authority's public certificate for TLS.
-	// This can be the same as SecretName.
-	// Used for mTLS.
+	// The Secret must store this as ca.crt.
+	// Used for mTLS, and TLS for rabbitmq_web_stomp and rabbitmq_web_mqtt.
 	CaSecretName string `json:"caSecretName,omitempty"`
-	// The Secret defined in CaSecretName must store the Certificate Authority's public certificate under the key specified in CaCertName.
-	// Used for mTLS.
-	CaCertName string `json:"caCertName,omitempty"`
 }
 
 // kubebuilder validating tags 'Pattern' and 'MaxLength' must be specified on string type.

config/crd/bases/rabbitmq.com_rabbitmqclusters.yaml

@@ -3734,15 +3734,11 @@ spec:
                 type: integer
               tls:
                 properties:
-                  caCertName:
-                    description: The Secret defined in CaSecretName must store the
-                      Certificate Authority's public certificate under the key specified
-                      in CaCertName. Used for mTLS.
-                    type: string
                   caSecretName:
                     description: Name of a Secret in the same Namespace as the RabbitmqCluster,
                       containing the Certificate Authority's public certificate for
-                      TLS. This can be the same as SecretName. Used for mTLS.
+                      TLS. The Secret must store this as ca.crt. Used for mTLS, and
+                      TLS for rabbitmq_web_stomp and rabbitmq_web_mqtt.
                     type: string
                   secretName:
                     description: Name of a Secret in the same Namespace as the RabbitmqCluster,

controllers/rabbitmqcluster_controller_test.go

@@ -13,9 +13,10 @@ package controllers_test
 import (
 	"context"
 	"fmt"
-	"k8s.io/utils/pointer"
 	"time"
 
+	"k8s.io/utils/pointer"
+
 	"k8s.io/apimachinery/pkg/util/intstr"
 
 	. "github.com/onsi/ginkgo"
@@ -1066,7 +1067,7 @@ var _ = Describe("RabbitmqClusterController", func() {
 					TargetPort: amqpTargetPort,
 				},
 				corev1.ServicePort{
-					Name:       "management",
+					Name:       "http",
 					Port:       15672,
 					Protocol:   corev1.ProtocolTCP,
 					TargetPort: managementTargetPort,
@@ -1190,7 +1191,7 @@ func waitForClusterDeletion(ctx context.Context, rabbitmqCluster *rabbitmqv1beta
 
 }
 
-func verifyError(ctx context.Context, rabbitmqCluster *rabbitmqv1beta1.RabbitmqCluster, expectedError string) {
+func verifyTLSErrorEvents(ctx context.Context, rabbitmqCluster *rabbitmqv1beta1.RabbitmqCluster, expectedError string) {
 	tlsEventTimeout := 5 * time.Second
 	tlsRetry := 1 * time.Second
 	Eventually(func() string { return aggregateEventMsgs(ctx, rabbitmqCluster, "TLSError") }, tlsEventTimeout, tlsRetry).Should(ContainSubstring(expectedError))

controllers/reconcile_tls.go

@@ -3,6 +3,7 @@ package controllers
 import (
 	"context"
 	"fmt"
+
 	rabbitmqv1beta1 "github.com/rabbitmq/cluster-operator/api/v1beta1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -11,48 +12,49 @@ import (
 )
 
 func (r *RabbitmqClusterReconciler) checkTLSSecrets(ctx context.Context, rabbitmqCluster *rabbitmqv1beta1.RabbitmqCluster) (ctrl.Result, error) {
-	logger := r.Log
 	secretName := rabbitmqCluster.Spec.TLS.SecretName
-	logger.Info("TLS set, looking for secret", "secret", secretName, "namespace", rabbitmqCluster.Namespace)
+	r.Log.Info("TLS enabled, looking for secret", "secret", secretName, "namespace", rabbitmqCluster.Namespace)
 
 	// check if secret exists
 	secret := &corev1.Secret{}
 	if err := r.Get(ctx, types.NamespacedName{Namespace: rabbitmqCluster.Namespace, Name: secretName}, secret); err != nil {
 		r.Recorder.Event(rabbitmqCluster, corev1.EventTypeWarning, "TLSError",
-			fmt.Sprintf("Failed to get TLS secret %v in namespace %v: %v", secretName, rabbitmqCluster.Namespace, err.Error()))
+			fmt.Sprintf("Failed to get TLS secret %s in namespace %s: %v", secretName, rabbitmqCluster.Namespace, err.Error()))
+		r.Log.Error(err, "Error setting up TLS", "namespace", rabbitmqCluster.Namespace, "name", rabbitmqCluster.Name)
 		return ctrl.Result{}, err
 	}
 	// check if secret has the right keys
 	_, hasTLSKey := secret.Data["tls.key"]
 	_, hasTLSCert := secret.Data["tls.crt"]
 	if !hasTLSCert || !hasTLSKey {
-		r.Recorder.Event(rabbitmqCluster, corev1.EventTypeWarning, "TLSError",
-			fmt.Sprintf("The TLS secret %v in namespace %v must have the fields tls.crt and tls.key", secretName, rabbitmqCluster.Namespace))
-
-		return ctrl.Result{}, errors.NewBadRequest("The TLS secret must have the fields tls.crt and tls.key")
+		err := errors.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())
+		r.Log.Error(err, "Error setting up TLS", "namespace", rabbitmqCluster.Namespace, "name", rabbitmqCluster.Name)
+		return ctrl.Result{}, err
 	}
 
 	// Mutual TLS: check if CA certificate is stored in a separate secret
 	if rabbitmqCluster.MutualTLSEnabled() {
 		if !rabbitmqCluster.SingleTLSSecret() {
 			secretName := rabbitmqCluster.Spec.TLS.CaSecretName
-			logger.Info("mutual TLS set, looking for CA certificate secret", "secret", secretName, "namespace", rabbitmqCluster.Namespace)
+			r.Log.Info("mutual TLS enabled, looking for CA certificate secret", "secret", secretName, "namespace", rabbitmqCluster.Namespace)
 
 			// check if secret exists
 			secret = &corev1.Secret{}
 			if err := r.Get(ctx, types.NamespacedName{Namespace: rabbitmqCluster.Namespace, Name: secretName}, secret); err != nil {
 				r.Recorder.Event(rabbitmqCluster, corev1.EventTypeWarning, "TLSError",
 					fmt.Sprintf("Failed to get CA certificate secret %v in namespace %v: %v", secretName, rabbitmqCluster.Namespace, err.Error()))
+				r.Log.Error(err, "Error setting up TLS", "namespace", rabbitmqCluster.Namespace, "name", rabbitmqCluster.Name)
 				return ctrl.Result{}, err
 			}
 		}
-		// Mutual TLS: verify that CA certificate is present in secret
-		_, hasCaCert := secret.Data[rabbitmqCluster.Spec.TLS.CaCertName]
-		if !hasCaCert {
-			r.Recorder.Event(rabbitmqCluster, corev1.EventTypeWarning, "TLSError",
-				fmt.Sprintf("The TLS secret %v in namespace %v must have the field %v", rabbitmqCluster.Spec.TLS.CaSecretName, rabbitmqCluster.Namespace, rabbitmqCluster.Spec.TLS.CaCertName))
 
-			return ctrl.Result{}, errors.NewBadRequest(fmt.Sprintf("The TLS secret must have the field %s", rabbitmqCluster.Spec.TLS.CaCertName))
+		// 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))
+			r.Recorder.Event(rabbitmqCluster, corev1.EventTypeWarning, "TLSError", err.Error())
+			r.Log.Error(err, "Error setting up TLS", "namespace", rabbitmqCluster.Namespace, "name", rabbitmqCluster.Name)
+			return ctrl.Result{}, err
 		}
 	}
 	return ctrl.Result{}, nil

controllers/reconcile_tls_test.go

@@ -3,6 +3,7 @@ package controllers_test
 import (
 	"context"
 	"fmt"
+
 	rabbitmqv1beta1 "github.com/rabbitmq/cluster-operator/api/v1beta1"
 
 	. "github.com/onsi/ginkgo"
@@ -32,11 +33,10 @@ var _ = Describe("Reconcile TLS", func() {
 
 		Context("Mutual TLS with single secret", func() {
 			It("Deploys successfully", func() {
-				tlsSecretWithCACert(ctx, "tls-secret", defaultNamespace, "caCERT")
+				tlsSecretWithCACert(ctx, "tls-secret", defaultNamespace)
 				tlsSpec := rabbitmqv1beta1.TLSSpec{
 					SecretName:   "tls-secret",
 					CaSecretName: "tls-secret",
-					CaCertName:   "caCERT",
 				}
 				cluster = rabbitmqClusterWithTLS(ctx, "mutual-tls-success", defaultNamespace, tlsSpec)
 				waitForClusterCreation(ctx, cluster, client)
@@ -45,23 +45,33 @@ var _ = Describe("Reconcile TLS", func() {
 				Expect(err).NotTo(HaveOccurred())
 				volumeMount := corev1.VolumeMount{
 					Name:      "rabbitmq-tls",
-					MountPath: "/etc/rabbitmq-tls/caCERT",
-					SubPath:   "caCERT",
+					MountPath: "/etc/rabbitmq-tls/ca.crt",
+					SubPath:   "ca.crt",
 					ReadOnly:  true,
 				}
 				Expect(sts.Spec.Template.Spec.Containers[0].VolumeMounts).To(ContainElement(volumeMount))
 			})
 
 			It("Does not deploy if the cert name does not match the contents of the secret", func() {
-				tlsSecretWithCACert(ctx, "tls-secret-missing", defaultNamespace, "ca.c")
+				tlsData := map[string]string{
+					"tls.crt":   "this is a tls cert",
+					"tls.key":   "this is a tls key",
+					"wrong-key": "certificate",
+				}
+
+				_, err := createSecret(ctx, "tls-secret-missing", defaultNamespace, tlsData)
+
+				if !apierrors.IsAlreadyExists(err) {
+					Expect(err).NotTo(HaveOccurred())
+				}
+
 				tlsSpec := rabbitmqv1beta1.TLSSpec{
 					SecretName:   "tls-secret-missing",
 					CaSecretName: "tls-secret-missing",
-					CaCertName:   "ca.crt",
 				}
 				cluster = rabbitmqClusterWithTLS(ctx, "tls-secret-missing", defaultNamespace, tlsSpec)
 
-				verifyError(ctx, cluster, fmt.Sprintf("The TLS secret tls-secret-missing in namespace %s must have the field ca.crt", defaultNamespace))
+				verifyTLSErrorEvents(ctx, cluster, fmt.Sprintf("TLS secret tls-secret-missing in namespace %s does not have the field ca.crt", defaultNamespace))
 			})
 		})
 
@@ -72,10 +82,9 @@ var _ = Describe("Reconcile TLS", func() {
 				tlsSpec := rabbitmqv1beta1.TLSSpec{
 					SecretName:   "rabbitmq-tls-secret-does-not-exist",
 					CaSecretName: "ca-cert-secret",
-					CaCertName:   "ca.crt",
 				}
 				cluster = rabbitmqClusterWithTLS(ctx, "rabbitmq-tls-secret-does-not-exist", defaultNamespace, tlsSpec)
-				verifyError(ctx, cluster, "Failed to get CA certificate secret")
+				verifyTLSErrorEvents(ctx, cluster, "Failed to get CA certificate secret")
 
 				_, err := clientSet.AppsV1().StatefulSets(cluster.Namespace).Get(ctx, cluster.ChildResourceName("server"), metav1.GetOptions{})
 				Expect(err).To(HaveOccurred())
@@ -105,10 +114,9 @@ var _ = Describe("Reconcile TLS", func() {
 				tlsSpec := rabbitmqv1beta1.TLSSpec{
 					SecretName:   "tls-secret",
 					CaSecretName: "ca-cert-secret-invalid",
-					CaCertName:   "ca.crt",
 				}
 				cluster = rabbitmqClusterWithTLS(ctx, "rabbitmq-mutual-tls-missing", defaultNamespace, tlsSpec)
-				verifyError(ctx, cluster, fmt.Sprintf("The TLS secret ca-cert-secret-invalid in namespace %s must have the field ca.crt", defaultNamespace))
+				verifyTLSErrorEvents(ctx, cluster, fmt.Sprintf("TLS secret ca-cert-secret-invalid in namespace %s does not have the field ca.crt", defaultNamespace))
 			})
 		})
 	})
@@ -163,7 +171,7 @@ var _ = Describe("Reconcile TLS", func() {
 			})
 
 			It("fails to deploy the RabbitmqCluster", func() {
-				verifyError(ctx, cluster, fmt.Sprintf("The TLS secret rabbitmq-tls-malformed in namespace %s must have the fields tls.crt and tls.key", defaultNamespace))
+				verifyTLSErrorEvents(ctx, cluster, fmt.Sprintf("TLS secret rabbitmq-tls-malformed in namespace %s does not have the fields tls.crt and tls.key", defaultNamespace))
 			})
 		})
 
@@ -175,7 +183,7 @@ var _ = Describe("Reconcile TLS", func() {
 				}
 				cluster = rabbitmqClusterWithTLS(ctx, "rabbitmq-tls-secret-does-not-exist", defaultNamespace, tlsSpec)
 
-				verifyError(ctx, cluster, "Failed to get TLS secret")
+				verifyTLSErrorEvents(ctx, cluster, "Failed to get TLS secret")
 
 				_, err := clientSet.AppsV1().StatefulSets(cluster.Namespace).Get(ctx, cluster.ChildResourceName("server"), metav1.GetOptions{})
 				Expect(err).To(HaveOccurred())
@@ -192,15 +200,14 @@ var _ = Describe("Reconcile TLS", func() {
 				statefulSet(ctx, cluster)
 			})
 		})
-
 	})
 })
 
-func tlsSecretWithCACert(ctx context.Context, secretName, namespace, caCertName string) {
+func tlsSecretWithCACert(ctx context.Context, secretName, namespace string) {
 	tlsData := map[string]string{
-		"tls.crt":  "this is a tls cert",
-		"tls.key":  "this is a tls key",
-		caCertName: "certificate",
+		"tls.crt": "this is a tls cert",
+		"tls.key": "this is a tls key",
+		"ca.crt":  "certificate",
 	}
 
 	_, err := createSecret(ctx, secretName, namespace, tlsData)

go.mod

@@ -3,7 +3,6 @@ module github.com/rabbitmq/cluster-operator
 go 1.15
 
 require (
-	cloud.google.com/go v0.47.0 // indirect
 	github.com/Azure/go-autorest/autorest v0.9.2 // indirect
 	github.com/Azure/go-autorest/autorest/adal v0.8.0 // indirect
 	github.com/cespare/xxhash/v2 v2.1.1 // indirect
@@ -13,9 +12,9 @@ require (
 	github.com/go-logr/logr v0.1.0
 	github.com/go-logr/zapr v0.1.1 // indirect
 	github.com/go-stomp/stomp v2.0.8+incompatible
-	github.com/golang/groupcache v0.0.0-20191027212112-611e8accdfc9 // indirect
 	github.com/gophercloud/gophercloud v0.5.0 // indirect
 	github.com/mattn/go-colorable v0.1.4 // indirect
+	github.com/michaelklishin/rabbit-hole/v2 v2.5.0
 	github.com/onsi/ginkgo v1.14.2
 	github.com/onsi/gomega v1.10.3
 	github.com/pelletier/go-toml v1.8.1 // indirect
@@ -26,7 +25,6 @@ require (
 	go.starlark.net v0.0.0-20200306205701-8dd3e2ee1dd5 // indirect
 	go.uber.org/multierr v1.2.0 // indirect
 	golang.org/x/net v0.0.0-20201010224723-4f7140c49acb
-	golang.org/x/oauth2 v0.0.0-20191122200657-5d9234df094c // indirect
 	golang.org/x/time v0.0.0-20191024005414-555d28b269f0 // indirect
 	gopkg.in/ini.v1 v1.62.0
 	k8s.io/api v0.18.6