Skip to content

Commit

Permalink
Merge pull request #1747 from rabbitmq/fix/cluster-operator-1616
Browse files Browse the repository at this point in the history
Fix CA certs overriding server certs
  • Loading branch information
Zerpet authored Oct 14, 2024
2 parents 4ad3a5f + 77d8bf2 commit 766d03d
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 29 deletions.
6 changes: 3 additions & 3 deletions api/v1beta1/rabbitmqcluster_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions controllers/reconcile_tls.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
13 changes: 13 additions & 0 deletions controllers/reconcile_tls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"}},
},
},
},
Expand Down
6 changes: 3 additions & 3 deletions docs/api/rabbitmq.com.ref.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,7 @@ created from the StatefulSet VolumeClaimTemplates.
[id="{anchor_prefix}-github.aaakk.us.kg-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:
****
Expand All @@ -551,10 +551,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.
Expand Down
9 changes: 8 additions & 1 deletion internal/resource/statefulset.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
},
},
},
},
Expand All @@ -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)
Expand Down
33 changes: 11 additions & 22 deletions internal/resource/statefulset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
},
},
},
},
Expand Down Expand Up @@ -621,28 +625,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"
Expand All @@ -660,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"},
},
},
},
{
Expand All @@ -668,6 +654,9 @@ var _ = Describe("StatefulSet", func() {
Name: "mutual-tls-secret",
},
Optional: ptr.To(true),
Items: []corev1.KeyToPath{
{Key: "ca.crt", Path: "ca.crt"},
},
},
},
},
Expand Down

0 comments on commit 766d03d

Please sign in to comment.