Stop disabling management tcp listener when tls enabled (#487)

- fix a bug where management tcp listener no longer
works when tls is enabled and disableNonTLSListeners is set to false
- management tcp listener can be disabled by setting
management.ssl.port without setting management.tcp.port
- ensure that management.tcp.port is set to 15672 when tls is enabled
but disableNonTLSListeners is false
- do not set management.tcp.port when tls is enabled and
disableNonTLSListeners is true

Author: ChunyiLyu

api/v1beta1/rabbitmqcluster_types.go

@@ -241,7 +241,7 @@ type TLSSpec struct {
 	// 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"`
-	// When set to true, the RabbitmqCluster disables non-TLS listeners for RabbitMQ and for any enabled plugins in the following list: stomp, mqtt, web_stomp, web_mqtt.
+	// When set to true, the RabbitmqCluster disables non-TLS listeners for RabbitMQ, management plugin and for any enabled plugins in the following list: stomp, mqtt, web_stomp, web_mqtt.
 	// Only TLS-enabled clients will be able to connect.
 	DisableNonTLSListeners bool `json:"disableNonTLSListeners,omitempty"`
 }

config/crd/bases/rabbitmq.com_rabbitmqclusters.yaml

@@ -3456,7 +3456,7 @@ spec:
                       description: Name of a Secret in the same Namespace as the RabbitmqCluster, containing the Certificate Authority's public certificate for TLS. The Secret must store this as ca.crt. Used for mTLS, and TLS for rabbitmq_web_stomp and rabbitmq_web_mqtt.
                       type: string
                     disableNonTLSListeners:
-                      description: 'When set to true, the RabbitmqCluster disables non-TLS listeners for RabbitMQ and for any enabled plugins in the following list: stomp, mqtt, web_stomp, web_mqtt. Only TLS-enabled clients will be able to connect.'
+                      description: 'When set to true, the RabbitmqCluster disables non-TLS listeners for RabbitMQ, management plugin and for any enabled plugins in the following list: stomp, mqtt, web_stomp, web_mqtt. Only TLS-enabled clients will be able to connect.'
                       type: boolean
                     secretName:
                       description: Name of a Secret in the same Namespace as the RabbitmqCluster, containing the server's private key & public certificate for TLS. The Secret must store these as tls.key and tls.crt, respectively.

controllers/rabbitmqcluster_controller_test.go

@@ -641,7 +641,7 @@ var _ = Describe("RabbitmqClusterController", func() {
 					return false
 				}
 				return string(sts.UID) != string(oldSts.UID)
-			}, 5).Should(BeTrue())
+			}, 10).Should(BeTrue())
 
 			Eventually(func() bool {
 				clientSvc, err := clientSet.CoreV1().Services(defaultNamespace).Get(ctx, svcName, metav1.GetOptions{})

controllers/reconcile_tls_test.go

@@ -75,7 +75,7 @@ var _ = Describe("Reconcile TLS", func() {
 			})
 		})
 
-		Context("Mutual TLS with a seperate CA certificate secret", func() {
+		Context("Mutual TLS with a separate CA certificate secret", func() {
 			It("Does not deploy the RabbitmqCluster, and retries every 10 seconds", func() {
 				tlsSecretWithoutCACert(ctx, "rabbitmq-tls-secret-does-not-exist", defaultNamespace)
 

internal/resource/configmap.go

@@ -93,6 +93,13 @@ func (builder *ServerConfigMapBuilder) Update(object runtime.Object) error {
 			if _, err := defaultSection.NewKey("listeners.tcp", "none"); err != nil {
 				return err
 			}
+		} else {
+			// management plugin does not have a *.listeners.tcp settings like other plugins
+			// management tcp listener can be disabled by setting management.ssl.port without setting management.tcp.port
+			// we set management tcp listener only if tls is enabled and disableNonTLSListeners is false
+			if _, err := defaultSection.NewKey("management.tcp.port", "15672"); err != nil {
+				return err
+			}
 		}
 		if builder.Instance.AdditionalPluginEnabled("rabbitmq_mqtt") {
 			if _, err := defaultSection.NewKey("mqtt.listeners.ssl.default", "8883"); err != nil {

internal/resource/configmap_test.go

@@ -272,6 +272,7 @@ listeners.ssl.default = 5671
 management.ssl.certfile   = /etc/rabbitmq-tls/tls.crt
 management.ssl.keyfile    = /etc/rabbitmq-tls/tls.key
 management.ssl.port       = 15671
+management.tcp.port       = 15672
 `)
 
 				Expect(configMapBuilder.Update(configMap)).To(Succeed())
@@ -306,6 +307,7 @@ listeners.ssl.default = 5671
 management.ssl.certfile   = /etc/rabbitmq-tls/tls.crt
 management.ssl.keyfile    = /etc/rabbitmq-tls/tls.key
 management.ssl.port       = 15671
+management.tcp.port       = 15672
 
 mqtt.listeners.ssl.default = 8883
 
@@ -340,6 +342,7 @@ listeners.ssl.default  = 5671
 management.ssl.certfile   = /etc/rabbitmq-tls/tls.crt
 management.ssl.keyfile    = /etc/rabbitmq-tls/tls.key
 management.ssl.port       = 15671
+management.tcp.port       = 15672
 
 ssl_options.cacertfile = /etc/rabbitmq-tls/ca.crt
 ssl_options.verify     = verify_peer
@@ -378,6 +381,7 @@ management.ssl.cacertfile = /etc/rabbitmq-tls/ca.crt
 			management.ssl.certfile   = /etc/rabbitmq-tls/tls.crt
 			management.ssl.keyfile    = /etc/rabbitmq-tls/tls.key
 			management.ssl.port       = 15671
+			management.tcp.port       = 15672
 
 			ssl_options.cacertfile = /etc/rabbitmq-tls/ca.crt
 			ssl_options.verify     = verify_peer
@@ -402,7 +406,7 @@ management.ssl.cacertfile = /etc/rabbitmq-tls/ca.crt
 		})
 
 		When("DisableNonTLSListeners is set to true", func() {
-			It("disables non tls listeners in rabbitmq.conf", func() {
+			It("disables non tls listeners for rabbitmq and management plugin", func() {
 				instance = rabbitmqv1beta1.RabbitmqCluster{
 					ObjectMeta: metav1.ObjectMeta{
 						Name: "rabbit-tls",

system_tests/system_tests.go

@@ -409,9 +409,12 @@ CONSOLE_LOG=new`
 				By("disabling non TLS listeners", func() {
 					// verify that rabbitmq.conf contains listeners.tcp = none
 					cfgMap := getConfigFileFromPod(namespace, cluster, "/etc/rabbitmq/rabbitmq.conf")
-					Expect(cfgMap).To(HaveKeyWithValue("listeners.tcp", "none"))
-					Expect(cfgMap).To(HaveKeyWithValue("stomp.listeners.tcp", "none"))
-					Expect(cfgMap).To(HaveKeyWithValue("mqtt.listeners.tcp", "none"))
+					Expect(cfgMap).To(SatisfyAll(
+						HaveKeyWithValue("listeners.tcp", "none"),
+						HaveKeyWithValue("stomp.listeners.tcp", "none"),
+						HaveKeyWithValue("mqtt.listeners.tcp", "none"),
+						HaveKeyWithValue("management.ssl.port", "15671"),
+						Not(HaveKey("management.tcp.port"))))
 
 					// verify that only tls ports are exposed in service
 					service, err := clientSet.CoreV1().Services(cluster.Namespace).Get(ctx, cluster.ChildResourceName(""), metav1.GetOptions{})