Skip to content

Commit e4c3759

Browse files
ChunyiLyuChunyiLyu
authored
Merge pull request #477 from rabbitmq/closenontlsport
Add spec.tls.disableNonTLSListeners property
2 parents 7ab7da0 + d1db350 commit e4c3759

14 files changed

+3756
-3645
lines changed

‎api/v1beta1/rabbitmqcluster_types.go

+7
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,9 @@ type TLSSpec struct {
241241
// The Secret must store this as ca.crt.
242242
// Used for mTLS, and TLS for rabbitmq_web_stomp and rabbitmq_web_mqtt.
243243
CaSecretName string `json:"caSecretName,omitempty"`
244+
// 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.
245+
// Only TLS-enabled clients will be able to connect.
246+
DisableNonTLSListeners bool `json:"disableNonTLSListeners,omitempty"`
244247
}
245248

246249
// kubebuilder validating tags 'Pattern' and 'MaxLength' must be specified on string type.
@@ -325,6 +328,10 @@ func (cluster *RabbitmqCluster) SingleTLSSecret() bool {
325328
return cluster.MutualTLSEnabled() && cluster.Spec.TLS.CaSecretName == cluster.Spec.TLS.SecretName
326329
}
327330

331+
func (cluster *RabbitmqCluster) DisableNonTLSListeners() bool {
332+
return cluster.Spec.TLS.DisableNonTLSListeners
333+
}
334+
328335
func (cluster *RabbitmqCluster) AdditionalPluginEnabled(plugin Plugin) bool {
329336
for _, p := range cluster.Spec.Rabbitmq.AdditionalPlugins {
330337
if p == plugin {

‎config/crd/bases/rabbitmq.com_rabbitmqclusters.yaml

+3,150-3,459
Large diffs are not rendered by default.

‎controllers/rabbitmqcluster_controller.go

+2-5
Original file line numberDiff line numberDiff line change
@@ -106,11 +106,8 @@ func (r *RabbitmqClusterReconciler) Reconcile(req ctrl.Request) (ctrl.Result, er
106106
return ctrl.Result{}, err
107107
}
108108

109-
// TLS: check if specified, and if secret exists
110-
if rabbitmqCluster.TLSEnabled() {
111-
if result, err := r.checkTLSSecrets(ctx, rabbitmqCluster); err != nil {
112-
return result, err
113-
}
109+
if err := r.reconcileTLS(ctx, rabbitmqCluster); err != nil {
110+
return ctrl.Result{}, err
114111
}
115112

116113
childResources, err := r.getChildResources(ctx, *rabbitmqCluster)

‎controllers/rabbitmqcluster_controller_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -1067,7 +1067,7 @@ var _ = Describe("RabbitmqClusterController", func() {
10671067
TargetPort: amqpTargetPort,
10681068
},
10691069
corev1.ServicePort{
1070-
Name: "http",
1070+
Name: "management",
10711071
Port: 15672,
10721072
Protocol: corev1.ProtocolTCP,
10731073
TargetPort: managementTargetPort,

‎controllers/reconcile_tls.go

+22-7
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,25 @@ import (
88
corev1 "k8s.io/api/core/v1"
99
"k8s.io/apimachinery/pkg/api/errors"
1010
"k8s.io/apimachinery/pkg/types"
11-
ctrl "sigs.k8s.io/controller-runtime"
1211
)
1312

14-
func (r *RabbitmqClusterReconciler) checkTLSSecrets(ctx context.Context, rabbitmqCluster *rabbitmqv1beta1.RabbitmqCluster) (ctrl.Result, error) {
13+
func (r *RabbitmqClusterReconciler) reconcileTLS(ctx context.Context, rabbitmqCluster *rabbitmqv1beta1.RabbitmqCluster) error {
14+
if rabbitmqCluster.DisableNonTLSListeners() && !rabbitmqCluster.TLSEnabled() {
15+
err := errors.NewBadRequest("TLS must be enabled if disableNonTLSListeners is set to true")
16+
r.Recorder.Event(rabbitmqCluster, corev1.EventTypeWarning, "TLSError", err.Error())
17+
r.Log.Error(err, "Error setting up TLS", "namespace", rabbitmqCluster.Namespace, "name", rabbitmqCluster.Name)
18+
return err
19+
}
20+
21+
if rabbitmqCluster.TLSEnabled() {
22+
if err := r.checkTLSSecrets(ctx, rabbitmqCluster); err != nil {
23+
return err
24+
}
25+
}
26+
return nil
27+
}
28+
29+
func (r *RabbitmqClusterReconciler) checkTLSSecrets(ctx context.Context, rabbitmqCluster *rabbitmqv1beta1.RabbitmqCluster) error {
1530
secretName := rabbitmqCluster.Spec.TLS.SecretName
1631
r.Log.Info("TLS enabled, looking for secret", "secret", secretName, "namespace", rabbitmqCluster.Namespace)
1732

@@ -21,7 +36,7 @@ func (r *RabbitmqClusterReconciler) checkTLSSecrets(ctx context.Context, rabbitm
2136
r.Recorder.Event(rabbitmqCluster, corev1.EventTypeWarning, "TLSError",
2237
fmt.Sprintf("Failed to get TLS secret %s in namespace %s: %v", secretName, rabbitmqCluster.Namespace, err.Error()))
2338
r.Log.Error(err, "Error setting up TLS", "namespace", rabbitmqCluster.Namespace, "name", rabbitmqCluster.Name)
24-
return ctrl.Result{}, err
39+
return err
2540
}
2641
// check if secret has the right keys
2742
_, hasTLSKey := secret.Data["tls.key"]
@@ -30,7 +45,7 @@ func (r *RabbitmqClusterReconciler) checkTLSSecrets(ctx context.Context, rabbitm
3045
err := errors.NewBadRequest(fmt.Sprintf("TLS secret %s in namespace %s does not have the fields tls.crt and tls.key", secretName, rabbitmqCluster.Namespace))
3146
r.Recorder.Event(rabbitmqCluster, corev1.EventTypeWarning, "TLSError", err.Error())
3247
r.Log.Error(err, "Error setting up TLS", "namespace", rabbitmqCluster.Namespace, "name", rabbitmqCluster.Name)
33-
return ctrl.Result{}, err
48+
return err
3449
}
3550

3651
// Mutual TLS: check if CA certificate is stored in a separate secret
@@ -45,7 +60,7 @@ func (r *RabbitmqClusterReconciler) checkTLSSecrets(ctx context.Context, rabbitm
4560
r.Recorder.Event(rabbitmqCluster, corev1.EventTypeWarning, "TLSError",
4661
fmt.Sprintf("Failed to get CA certificate secret %v in namespace %v: %v", secretName, rabbitmqCluster.Namespace, err.Error()))
4762
r.Log.Error(err, "Error setting up TLS", "namespace", rabbitmqCluster.Namespace, "name", rabbitmqCluster.Name)
48-
return ctrl.Result{}, err
63+
return err
4964
}
5065
}
5166

@@ -54,8 +69,8 @@ func (r *RabbitmqClusterReconciler) checkTLSSecrets(ctx context.Context, rabbitm
5469
err := errors.NewBadRequest(fmt.Sprintf("TLS secret %s in namespace %s does not have the field ca.crt", rabbitmqCluster.Spec.TLS.CaSecretName, rabbitmqCluster.Namespace))
5570
r.Recorder.Event(rabbitmqCluster, corev1.EventTypeWarning, "TLSError", err.Error())
5671
r.Log.Error(err, "Error setting up TLS", "namespace", rabbitmqCluster.Namespace, "name", rabbitmqCluster.Name)
57-
return ctrl.Result{}, err
72+
return err
5873
}
5974
}
60-
return ctrl.Result{}, nil
75+
return nil
6176
}

‎controllers/reconcile_tls_test.go

+14
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,20 @@ var _ = Describe("Reconcile TLS", func() {
201201
})
202202
})
203203
})
204+
205+
When("DiableNonTLSListeners set to true", func() {
206+
It("errors and logs TLSError when TLS is not enabled", func() {
207+
tlsSpec := rabbitmqv1beta1.TLSSpec{
208+
DisableNonTLSListeners: true,
209+
}
210+
cluster = rabbitmqClusterWithTLS(ctx, "rabbitmq-disablenontlslisteners", defaultNamespace, tlsSpec)
211+
212+
verifyTLSErrorEvents(ctx, cluster, "TLS must be enabled if disableNonTLSListeners is set to true")
213+
214+
_, err := clientSet.AppsV1().StatefulSets(cluster.Namespace).Get(ctx, cluster.ChildResourceName("server"), metav1.GetOptions{})
215+
Expect(err).To(HaveOccurred())
216+
})
217+
})
204218
})
205219

206220
func tlsSecretWithCACert(ctx context.Context, secretName, namespace string) {

‎internal/resource/configmap.go

+25
Original file line numberDiff line numberDiff line change
@@ -89,15 +89,30 @@ func (builder *ServerConfigMapBuilder) Update(object runtime.Object) error {
8989
if err := cfg.Append([]byte(defaultTLSConf)); err != nil {
9090
return err
9191
}
92+
if builder.Instance.DisableNonTLSListeners() {
93+
if _, err := defaultSection.NewKey("listeners.tcp", "none"); err != nil {
94+
return err
95+
}
96+
}
9297
if builder.Instance.AdditionalPluginEnabled("rabbitmq_mqtt") {
9398
if _, err := defaultSection.NewKey("mqtt.listeners.ssl.default", "8883"); err != nil {
9499
return err
95100
}
101+
if builder.Instance.DisableNonTLSListeners() {
102+
if _, err := defaultSection.NewKey("mqtt.listeners.tcp", "none"); err != nil {
103+
return err
104+
}
105+
}
96106
}
97107
if builder.Instance.AdditionalPluginEnabled("rabbitmq_stomp") {
98108
if _, err := defaultSection.NewKey("stomp.listeners.ssl.1", "61614"); err != nil {
99109
return err
100110
}
111+
if builder.Instance.DisableNonTLSListeners() {
112+
if _, err := defaultSection.NewKey("stomp.listeners.tcp", "none"); err != nil {
113+
return err
114+
}
115+
}
101116
}
102117
}
103118

@@ -125,6 +140,11 @@ func (builder *ServerConfigMapBuilder) Update(object runtime.Object) error {
125140
if _, err := defaultSection.NewKey("web_mqtt.ssl.keyfile", tlsKeyPath); err != nil {
126141
return err
127142
}
143+
if builder.Instance.DisableNonTLSListeners() {
144+
if _, err := defaultSection.NewKey("web_mqtt.tcp.listener", "none"); err != nil {
145+
return err
146+
}
147+
}
128148
}
129149
if builder.Instance.AdditionalPluginEnabled("rabbitmq_web_stomp") {
130150
if _, err := defaultSection.NewKey("web_stomp.ssl.port", "15673"); err != nil {
@@ -139,6 +159,11 @@ func (builder *ServerConfigMapBuilder) Update(object runtime.Object) error {
139159
if _, err := defaultSection.NewKey("web_stomp.ssl.keyfile", tlsKeyPath); err != nil {
140160
return err
141161
}
162+
if builder.Instance.DisableNonTLSListeners() {
163+
if _, err := defaultSection.NewKey("web_stomp.tcp.listener", "none"); err != nil {
164+
return err
165+
}
166+
}
142167
}
143168
}
144169

‎internal/resource/configmap_test.go

+122
Original file line numberDiff line numberDiff line change
@@ -401,6 +401,128 @@ management.ssl.cacertfile = /etc/rabbitmq-tls/ca.crt
401401
})
402402
})
403403

404+
When("DisableNonTLSListeners is set to true", func() {
405+
It("disables non tls listeners in rabbitmq.conf", func() {
406+
instance = rabbitmqv1beta1.RabbitmqCluster{
407+
ObjectMeta: metav1.ObjectMeta{
408+
Name: "rabbit-tls",
409+
},
410+
Spec: rabbitmqv1beta1.RabbitmqClusterSpec{
411+
TLS: rabbitmqv1beta1.TLSSpec{
412+
SecretName: "some-secret",
413+
DisableNonTLSListeners: true,
414+
},
415+
},
416+
}
417+
418+
expectedRabbitmqConf := iniString(defaultRabbitmqConf(builder.Instance.Name) + `
419+
ssl_options.certfile = /etc/rabbitmq-tls/tls.crt
420+
ssl_options.keyfile = /etc/rabbitmq-tls/tls.key
421+
listeners.ssl.default = 5671
422+
423+
management.ssl.certfile = /etc/rabbitmq-tls/tls.crt
424+
management.ssl.keyfile = /etc/rabbitmq-tls/tls.key
425+
management.ssl.port = 15671
426+
427+
listeners.tcp = none
428+
`)
429+
430+
Expect(configMapBuilder.Update(configMap)).To(Succeed())
431+
Expect(configMap.Data).To(HaveKeyWithValue("rabbitmq.conf", expectedRabbitmqConf))
432+
})
433+
434+
It("disables non tls listeners for mqtt and stomp when enabled", func() {
435+
instance = rabbitmqv1beta1.RabbitmqCluster{
436+
ObjectMeta: metav1.ObjectMeta{
437+
Name: "rabbit-tls",
438+
},
439+
Spec: rabbitmqv1beta1.RabbitmqClusterSpec{
440+
TLS: rabbitmqv1beta1.TLSSpec{
441+
SecretName: "some-secret",
442+
DisableNonTLSListeners: true,
443+
},
444+
Rabbitmq: rabbitmqv1beta1.RabbitmqClusterConfigurationSpec{
445+
AdditionalPlugins: []rabbitmqv1beta1.Plugin{
446+
"rabbitmq_mqtt",
447+
"rabbitmq_stomp",
448+
},
449+
},
450+
},
451+
}
452+
453+
expectedRabbitmqConf := iniString(defaultRabbitmqConf(builder.Instance.Name) + `
454+
ssl_options.certfile = /etc/rabbitmq-tls/tls.crt
455+
ssl_options.keyfile = /etc/rabbitmq-tls/tls.key
456+
listeners.ssl.default = 5671
457+
458+
management.ssl.certfile = /etc/rabbitmq-tls/tls.crt
459+
management.ssl.keyfile = /etc/rabbitmq-tls/tls.key
460+
management.ssl.port = 15671
461+
listeners.tcp = none
462+
463+
mqtt.listeners.ssl.default = 8883
464+
mqtt.listeners.tcp = none
465+
466+
stomp.listeners.ssl.1 = 61614
467+
stomp.listeners.tcp = none
468+
`)
469+
470+
Expect(configMapBuilder.Update(configMap)).To(Succeed())
471+
Expect(configMap.Data).To(HaveKeyWithValue("rabbitmq.conf", expectedRabbitmqConf))
472+
})
473+
474+
It("disables non tls listeners for web mqtt and web stomp when enabled", func() {
475+
instance = rabbitmqv1beta1.RabbitmqCluster{
476+
ObjectMeta: metav1.ObjectMeta{
477+
Name: "rabbit-tls",
478+
},
479+
Spec: rabbitmqv1beta1.RabbitmqClusterSpec{
480+
TLS: rabbitmqv1beta1.TLSSpec{
481+
SecretName: "some-secret",
482+
CaSecretName: "some-mutual-secret",
483+
DisableNonTLSListeners: true,
484+
},
485+
Rabbitmq: rabbitmqv1beta1.RabbitmqClusterConfigurationSpec{
486+
AdditionalPlugins: []rabbitmqv1beta1.Plugin{
487+
"rabbitmq_web_mqtt",
488+
"rabbitmq_web_stomp",
489+
},
490+
},
491+
},
492+
}
493+
494+
expectedRabbitmqConf := iniString(defaultRabbitmqConf(builder.Instance.Name) + `
495+
ssl_options.certfile = /etc/rabbitmq-tls/tls.crt
496+
ssl_options.keyfile = /etc/rabbitmq-tls/tls.key
497+
listeners.ssl.default = 5671
498+
499+
management.ssl.certfile = /etc/rabbitmq-tls/tls.crt
500+
management.ssl.keyfile = /etc/rabbitmq-tls/tls.key
501+
management.ssl.port = 15671
502+
listeners.tcp = none
503+
504+
ssl_options.cacertfile = /etc/rabbitmq-tls/ca.crt
505+
ssl_options.verify = verify_peer
506+
management.ssl.cacertfile = /etc/rabbitmq-tls/ca.crt
507+
508+
web_mqtt.ssl.port = 15676
509+
web_mqtt.ssl.cacertfile = /etc/rabbitmq-tls/ca.crt
510+
web_mqtt.ssl.certfile = /etc/rabbitmq-tls/tls.crt
511+
web_mqtt.ssl.keyfile = /etc/rabbitmq-tls/tls.key
512+
web_mqtt.tcp.listener = none
513+
514+
web_stomp.ssl.port = 15673
515+
web_stomp.ssl.cacertfile = /etc/rabbitmq-tls/ca.crt
516+
web_stomp.ssl.certfile = /etc/rabbitmq-tls/tls.crt
517+
web_stomp.ssl.keyfile = /etc/rabbitmq-tls/tls.key
518+
web_stomp.tcp.listener = none
519+
`)
520+
521+
Expect(configMapBuilder.Update(configMap)).To(Succeed())
522+
Expect(configMap.Data).To(HaveKeyWithValue("rabbitmq.conf", expectedRabbitmqConf))
523+
})
524+
})
525+
404526
Context("Memory Limits", func() {
405527
It("sets a RabbitMQ memory limit with headroom when memory limits are specified", func() {
406528
const GiB int64 = 1073741824

0 commit comments

Comments
 (0)