From ba021593f3fe9ce069f77f8f356b02b6a942d810 Mon Sep 17 00:00:00 2001 From: Aitor Perez Cedres <1515757+Zerpet@users.noreply.github.com> Date: Fri, 11 Oct 2024 17:36:10 +0100 Subject: [PATCH 1/3] Remove ineffective test This test became ineffective after #1735. We want to enable Web MQTT/STOMP TLS regardless of whether a CA bundle certificate is provided. This test was asserting that we enable web stomp/mqqt tls when the CA bundle is set; however, by not setting the CA bundle, the test passes anyway. This is not correct, making the test ineffective. --- internal/resource/statefulset_test.go | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/internal/resource/statefulset_test.go b/internal/resource/statefulset_test.go index dcc98cb28..3dc543b3d 100644 --- a/internal/resource/statefulset_test.go +++ b/internal/resource/statefulset_test.go @@ -621,28 +621,6 @@ var _ = Describe("StatefulSet", func() { })) }) - When("Mutual TLS (same secret) is enabled", func() { - It("opens tls ports when rabbitmq_web_mqtt and rabbitmq_web_stomp are configured", func() { - instance.Spec.TLS.SecretName = "tls-secret" - instance.Spec.TLS.CaSecretName = "tls-secret" - instance.Spec.Rabbitmq.AdditionalPlugins = []rabbitmqv1beta1.Plugin{"rabbitmq_web_mqtt", "rabbitmq_web_stomp"} - Expect(stsBuilder.Update(statefulSet)).To(Succeed()) - - rabbitmqContainerSpec := extractContainer(statefulSet.Spec.Template.Spec.Containers, "rabbitmq") - - Expect(rabbitmqContainerSpec.Ports).To(ContainElements([]corev1.ContainerPort{ - { - Name: "web-mqtt-tls", - ContainerPort: 15676, - }, - { - Name: "web-stomp-tls", - ContainerPort: 15673, - }, - })) - }) - }) - When("Mutual TLS (different secret) is enabled", func() { It("adds the CA cert secret to tls project volume", func() { instance.Spec.TLS.SecretName = "tls-secret" From d4938b11260dbb2bdbf29f7cf43356cec99d571b Mon Sep 17 00:00:00 2001 From: Aitor Perez Cedres <1515757+Zerpet@users.noreply.github.com> Date: Fri, 11 Oct 2024 18:29:12 +0100 Subject: [PATCH 2/3] Fix CA certs overriding server certs Related to #1616 --- api/v1beta1/rabbitmqcluster_types.go | 6 +++--- controllers/reconcile_tls.go | 1 + controllers/reconcile_tls_test.go | 13 +++++++++++++ internal/resource/statefulset.go | 9 ++++++++- internal/resource/statefulset_test.go | 11 +++++++++++ 5 files changed, 36 insertions(+), 4 deletions(-) diff --git a/api/v1beta1/rabbitmqcluster_types.go b/api/v1beta1/rabbitmqcluster_types.go index 6865a27a7..855935293 100644 --- a/api/v1beta1/rabbitmqcluster_types.go +++ b/api/v1beta1/rabbitmqcluster_types.go @@ -350,15 +350,15 @@ type PersistentVolumeClaim struct { Spec corev1.PersistentVolumeClaimSpec `json:"spec,omitempty" protobuf:"bytes,2,opt,name=spec"` } -// Allows for the configuration of TLS certificates to be used by RabbitMQ. Also allows for non-TLS traffic to be disabled. +// TLSSpec allows for the configuration of TLS certificates to be used by RabbitMQ. Also allows for non-TLS traffic to be disabled. type TLSSpec struct { // 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. - // This Secret can be created by running `kubectl create secret tls tls-secret --cert=path/to/tls.cert --key=path/to/tls.key` + // This Secret can be created by running `kubectl create secret tls tls-secret --cert=path/to/tls.crt --key=path/to/tls.key` 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. // The Secret must store this as ca.crt. - // This Secret can be created by running `kubectl create secret generic ca-secret --from-file=ca.crt=path/to/ca.cert` + // This Secret can be created by running `kubectl create secret generic ca-secret --from-file=ca.crt=path/to/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, management plugin and for any enabled plugins in the following list: stomp, mqtt, web_stomp, web_mqtt. diff --git a/controllers/reconcile_tls.go b/controllers/reconcile_tls.go index 13c4e5184..dc5bf28ea 100644 --- a/controllers/reconcile_tls.go +++ b/controllers/reconcile_tls.go @@ -60,6 +60,7 @@ func (r *RabbitmqClusterReconciler) checkTLSSecrets(ctx context.Context, rabbitm // Mutual TLS: check if CA certificate is stored in a separate secret if rabbitmqCluster.MutualTLSEnabled() { + // This is an optimisation to avoid reading the same secret twice if !rabbitmqCluster.SingleTLSSecret() { secretName := rabbitmqCluster.Spec.TLS.CaSecretName logger.V(1).Info("mutual TLS enabled, looking for CA certificate secret", "secret", secretName) diff --git a/controllers/reconcile_tls_test.go b/controllers/reconcile_tls_test.go index 4a60b7cdc..89debd782 100644 --- a/controllers/reconcile_tls_test.go +++ b/controllers/reconcile_tls_test.go @@ -58,6 +58,19 @@ var _ = Describe("Reconcile TLS", func() { Name: "tls-secret", }, Optional: ptr.To(true), + Items: []corev1.KeyToPath{ + {Key: "tls.crt", Path: "tls.crt"}, + {Key: "tls.key", Path: "tls.key"}, + }, + }, + }, + { + Secret: &corev1.SecretProjection{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "tls-secret", + }, + Optional: ptr.To(true), + Items: []corev1.KeyToPath{{Key: "ca.crt", Path: "ca.crt"}}, }, }, }, diff --git a/internal/resource/statefulset.go b/internal/resource/statefulset.go index e177eeb7e..c75bba541 100644 --- a/internal/resource/statefulset.go +++ b/internal/resource/statefulset.go @@ -523,6 +523,10 @@ func (builder *StatefulSetBuilder) podTemplateSpec(previousPodAnnotations map[st Name: tlsSpec.SecretName, }, Optional: &secretEnforced, + Items: []corev1.KeyToPath{ + {Key: "tls.crt", Path: "tls.crt"}, + {Key: "tls.key", Path: "tls.key"}, + }, }, }, }, @@ -531,11 +535,14 @@ func (builder *StatefulSetBuilder) podTemplateSpec(previousPodAnnotations map[st }, } - if builder.Instance.MutualTLSEnabled() && !builder.Instance.SingleTLSSecret() { + if builder.Instance.MutualTLSEnabled() { caSecretProjection := corev1.VolumeProjection{ Secret: &corev1.SecretProjection{ LocalObjectReference: corev1.LocalObjectReference{Name: tlsSpec.CaSecretName}, Optional: &secretEnforced, + Items: []corev1.KeyToPath{ + {Key: "ca.crt", Path: "ca.crt"}, + }, }, } tlsProjectedVolume.VolumeSource.Projected.Sources = append(tlsProjectedVolume.VolumeSource.Projected.Sources, caSecretProjection) diff --git a/internal/resource/statefulset_test.go b/internal/resource/statefulset_test.go index 3dc543b3d..403cc73c7 100644 --- a/internal/resource/statefulset_test.go +++ b/internal/resource/statefulset_test.go @@ -530,6 +530,10 @@ var _ = Describe("StatefulSet", func() { Name: "tls-secret", }, Optional: ptr.To(true), + Items: []corev1.KeyToPath{ + {Key: "tls.crt", Path: "tls.crt"}, + {Key: "tls.key", Path: "tls.key"}, + }, }, }, }, @@ -638,6 +642,10 @@ var _ = Describe("StatefulSet", func() { Name: "tls-secret", }, Optional: ptr.To(true), + Items: []corev1.KeyToPath{ + {Key: "tls.crt", Path: "tls.crt"}, + {Key: "tls.key", Path: "tls.key"}, + }, }, }, { @@ -646,6 +654,9 @@ var _ = Describe("StatefulSet", func() { Name: "mutual-tls-secret", }, Optional: ptr.To(true), + Items: []corev1.KeyToPath{ + {Key: "ca.crt", Path: "ca.crt"}, + }, }, }, }, From 77d8bf2a5d5d2ecd9d989fe3f9986fe66a2d5424 Mon Sep 17 00:00:00 2001 From: Aitor Perez Cedres <1515757+Zerpet@users.noreply.github.com> Date: Fri, 11 Oct 2024 18:30:05 +0100 Subject: [PATCH 3/3] Regenerate API docs Due to changes in the spec code comments. --- docs/api/rabbitmq.com.ref.asciidoc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/api/rabbitmq.com.ref.asciidoc b/docs/api/rabbitmq.com.ref.asciidoc index 02257c042..26882d3a0 100644 --- a/docs/api/rabbitmq.com.ref.asciidoc +++ b/docs/api/rabbitmq.com.ref.asciidoc @@ -287,7 +287,7 @@ The settings for the persistent storage desired for each Pod in the RabbitmqClus |=== | Field | Description | *`storageClassName`* __string__ | The name of the StorageClass to claim a PersistentVolume from. -| *`storage`* __xref:{anchor_prefix}-k8s-io-apimachinery-pkg-api-resource-quantity[$$Quantity$$]__ | The requested size of the persistent volume attached to each Pod in the RabbitmqCluster. +| *`storage`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.23/#quantity-resource-api[$$Quantity$$]__ | The requested size of the persistent volume attached to each Pod in the RabbitmqCluster. The format of this field matches that defined by kubernetes/apimachinery. See https://pkg.go.dev/k8s.io/apimachinery/pkg/api/resource#Quantity for more info on the format of this field. |=== @@ -540,7 +540,7 @@ created from the StatefulSet VolumeClaimTemplates. [id="{anchor_prefix}-github-com-rabbitmq-cluster-operator-v2-api-v1beta1-tlsspec"] ==== TLSSpec -Allows for the configuration of TLS certificates to be used by RabbitMQ. Also allows for non-TLS traffic to be disabled. +TLSSpec allows for the configuration of TLS certificates to be used by RabbitMQ. Also allows for non-TLS traffic to be disabled. .Appears In: **** @@ -552,10 +552,10 @@ Allows for the configuration of TLS certificates to be used by RabbitMQ. Also al | Field | Description | *`secretName`* __string__ | 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. -This Secret can be created by running `kubectl create secret tls tls-secret --cert=path/to/tls.cert --key=path/to/tls.key` +This Secret can be created by running `kubectl create secret tls tls-secret --cert=path/to/tls.crt --key=path/to/tls.key` | *`caSecretName`* __string__ | 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. -This Secret can be created by running `kubectl create secret generic ca-secret --from-file=ca.crt=path/to/ca.cert` +This Secret can be created by running `kubectl create secret generic ca-secret --from-file=ca.crt=path/to/ca.crt` Used for mTLS, and TLS for rabbitmq_web_stomp and rabbitmq_web_mqtt. | *`disableNonTLSListeners`* __boolean__ | 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.