From c39b8208f365268b64f796d5b0aae4a99cfabb9d Mon Sep 17 00:00:00 2001 From: Connor Rogers Date: Wed, 16 Sep 2020 16:15:23 +0000 Subject: [PATCH 1/4] Configure default user/password through conf.d This commit stops the operator configuring the default username and password for the cluster through environment variables. Instead, these credentials are configured through `/etc/rabbitmq/conf.d/`. This is for two reasons: * RabbitMQ is moving away from configuration via env vars * The operator should not be coupled to functionality in the entrypoint script in the docker-library/rabbitmq Docker image, as this prevents other RabbitMQ images being used This effectively removes support for RabbitMQ images pre 3.8.4, where the ability to configure RabbitMQ via sysctl-style `*.conf` files in the conf.d directory. --- .../rabbitmqcluster_controller_test.go | 10 +- internal/resource/admin_secret.go | 43 ++++++- internal/resource/admin_secret_test.go | 118 +++++++++++------- internal/resource/erlang_cookie.go | 12 +- internal/resource/statefulset.go | 58 +++++---- internal/resource/statefulset_test.go | 78 +++++------- 6 files changed, 182 insertions(+), 137 deletions(-) diff --git a/controllers/rabbitmqcluster_controller_test.go b/controllers/rabbitmqcluster_controller_test.go index c7ea44ff5..5a4e9c8c0 100644 --- a/controllers/rabbitmqcluster_controller_test.go +++ b/controllers/rabbitmqcluster_controller_test.go @@ -1218,19 +1218,15 @@ var _ = Describe("RabbitmqclusterController", func() { }, }, corev1.Volume{ - Name: "rabbitmq-admin", + Name: "rabbitmq-confd", VolumeSource: corev1.VolumeSource{ Secret: &corev1.SecretVolumeSource{ DefaultMode: &defaultMode, SecretName: "rabbitmq-sts-override-rabbitmq-admin", Items: []corev1.KeyToPath{ { - Key: "username", - Path: "username", - }, - { - Key: "password", - Path: "password", + Key: "default_user.conf", + Path: "default_user.conf", }, }, }, diff --git a/internal/resource/admin_secret.go b/internal/resource/admin_secret.go index 23b072197..e6fd2e853 100644 --- a/internal/resource/admin_secret.go +++ b/internal/resource/admin_secret.go @@ -10,8 +10,12 @@ package resource import ( + "bytes" + "encoding/base64" + rabbitmqv1beta1 "github.com/rabbitmq/cluster-operator/api/v1beta1" "github.com/rabbitmq/cluster-operator/internal/metadata" + "gopkg.in/ini.v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -31,6 +35,31 @@ func (builder *RabbitmqResourceBuilder) AdminSecret() *AdminSecretBuilder { } } +func generateDefaultUserConf(username, password []byte) ([]byte, error) { + + ini.PrettySection = false // Remove trailing new line because default_user.conf has only a default section. + cfg, err := ini.Load([]byte{}) + if err != nil { + return nil, err + } + defaultSection := cfg.Section("") + + if _, err := defaultSection.NewKey("default_user", string(username)); err != nil { + return nil, err + } + + if _, err := defaultSection.NewKey("default_pass", string(password)); err != nil { + return nil, err + } + + var userConfBuffer bytes.Buffer + if cfg.WriteTo(&userConfBuffer); err != nil { + return nil, err + } + + return userConfBuffer.Bytes(), nil +} + func (builder *AdminSecretBuilder) UpdateRequiresStsRestart() bool { return false } @@ -43,12 +72,17 @@ func (builder *AdminSecretBuilder) Update(object runtime.Object) error { } func (builder *AdminSecretBuilder) Build() (runtime.Object, error) { - username, err := randomEncodedString(24) + username, err := randomBytes(24) + if err != nil { + return nil, err + } + + password, err := randomBytes(24) if err != nil { return nil, err } - password, err := randomEncodedString(24) + defaultUserConf, err := generateDefaultUserConf(username, password) if err != nil { return nil, err } @@ -60,8 +94,9 @@ func (builder *AdminSecretBuilder) Build() (runtime.Object, error) { }, Type: corev1.SecretTypeOpaque, Data: map[string][]byte{ - "username": []byte(username), - "password": []byte(password), + "username": []byte(base64.URLEncoding.EncodeToString(username)), + "password": []byte(base64.URLEncoding.EncodeToString(password)), + "default_user.conf": []byte(base64.URLEncoding.EncodeToString(defaultUserConf)), }, }, nil } diff --git a/internal/resource/admin_secret_test.go b/internal/resource/admin_secret_test.go index 6adc0541e..de73c6531 100644 --- a/internal/resource/admin_secret_test.go +++ b/internal/resource/admin_secret_test.go @@ -12,6 +12,7 @@ package resource_test import ( b64 "encoding/base64" + "gopkg.in/ini.v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -44,41 +45,62 @@ var _ = Describe("AdminSecret", func() { }) Context("Build with defaults", func() { - BeforeEach(func() { + It("creates the necessary admin secret", func() { + var decodedUsername []byte + var decodedPassword []byte + var err error + obj, err := adminSecretBuilder.Build() Expect(err).NotTo(HaveOccurred()) secret = obj.(*corev1.Secret) - }) - It("creates the secret with correct name and namespace", func() { - Expect(secret.Name).To(Equal(instance.ChildResourceName("admin"))) - Expect(secret.Namespace).To(Equal("a namespace")) - }) + By("creating the secret with correct name and namespace", func() { + Expect(secret.Name).To(Equal(instance.ChildResourceName("admin"))) + Expect(secret.Namespace).To(Equal("a namespace")) + }) - It("creates a 'opaque' secret ", func() { - Expect(secret.Type).To(Equal(corev1.SecretTypeOpaque)) - }) + By("creating a 'opaque' secret ", func() { + Expect(secret.Type).To(Equal(corev1.SecretTypeOpaque)) + }) - It("creates a rabbitmq username that is base64 encoded and 24 characters in length", func() { - username, ok := secret.Data["username"] - Expect(ok).NotTo(BeFalse()) - decodedUsername, err := b64.URLEncoding.DecodeString(string(username)) - Expect(err).NotTo(HaveOccurred()) - Expect(len(decodedUsername)).To(Equal(24)) + By("creating a rabbitmq username that is base64 encoded and 24 characters in length", func() { + username, ok := secret.Data["username"] + Expect(ok).NotTo(BeFalse()) + decodedUsername, err = b64.URLEncoding.DecodeString(string(username)) + Expect(err).NotTo(HaveOccurred()) + Expect(len(decodedUsername)).To(Equal(24)) - }) + }) - It("creates a rabbitmq password that is base64 encoded and 24 characters in length", func() { - password, ok := secret.Data["password"] - Expect(ok).NotTo(BeFalse()) - decodedPassword, err := b64.URLEncoding.DecodeString(string(password)) - Expect(err).NotTo(HaveOccurred()) - Expect(len(decodedPassword)).To(Equal(24)) + By("creating a rabbitmq password that is base64 encoded and 24 characters in length", func() { + password, ok := secret.Data["password"] + Expect(ok).NotTo(BeFalse()) + decodedPassword, err = b64.URLEncoding.DecodeString(string(password)) + Expect(err).NotTo(HaveOccurred()) + Expect(len(decodedPassword)).To(Equal(24)) + }) + + By("creating a default_user.conf file that contains the correct sysctl config format to be parsed by RabbitMQ", func() { + defaultUserConf, ok := secret.Data["default_user.conf"] + Expect(ok).NotTo(BeFalse()) + + decodedDefaultUserConf, err := b64.URLEncoding.DecodeString(string(defaultUserConf)) + Expect(err).NotTo(HaveOccurred()) + + cfg, err := ini.Load(decodedDefaultUserConf) + Expect(err).NotTo(HaveOccurred()) + + Expect(cfg.Section("").HasKey("default_user")).To(BeTrue()) + Expect(cfg.Section("").HasKey("default_pass")).To(BeTrue()) + + Expect(cfg.Section("").Key("default_user").Value()).To(Equal(string(decodedUsername))) + Expect(cfg.Section("").Key("default_pass").Value()).To(Equal(string(decodedPassword))) + }) }) }) Context("Update with instance labels", func() { - BeforeEach(func() { + It("Updates the secret", func() { instance = rabbitmqv1beta1.RabbitmqCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "rabbit-labelled", @@ -102,26 +124,26 @@ var _ = Describe("AdminSecret", func() { } err := adminSecretBuilder.Update(secret) Expect(err).NotTo(HaveOccurred()) - }) - It("adds new labels from the CR", func() { - testLabels(secret.Labels) - }) + By("adding new labels from the CR", func() { + testLabels(secret.Labels) + }) - It("restores the default labels", func() { - labels := secret.Labels - Expect(labels["app.kubernetes.io/name"]).To(Equal(instance.Name)) - Expect(labels["app.kubernetes.io/component"]).To(Equal("rabbitmq")) - Expect(labels["app.kubernetes.io/part-of"]).To(Equal("rabbitmq")) - }) + By("restoring the default labels", func() { + labels := secret.Labels + Expect(labels["app.kubernetes.io/name"]).To(Equal(instance.Name)) + Expect(labels["app.kubernetes.io/component"]).To(Equal("rabbitmq")) + Expect(labels["app.kubernetes.io/part-of"]).To(Equal("rabbitmq")) + }) - It("deletes the labels that are removed from the CR", func() { - Expect(secret.Labels).NotTo(HaveKey("this-was-the-previous-label")) + By("deleting the labels that are removed from the CR", func() { + Expect(secret.Labels).NotTo(HaveKey("this-was-the-previous-label")) + }) }) }) Context("Update with instance annotations", func() { - BeforeEach(func() { + It("updates the secret with the annotations", func() { instance = rabbitmqv1beta1.RabbitmqCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "rabbit-labelled", @@ -150,19 +172,19 @@ var _ = Describe("AdminSecret", func() { } err := adminSecretBuilder.Update(secret) Expect(err).NotTo(HaveOccurred()) - }) - - It("updates secret annotations on admin secret", func() { - expectedAnnotations := map[string]string{ - "my-annotation": "i-like-this", - "i-was-here-already": "please-dont-delete-me", - "im-here-to-stay.kubernetes.io": "for-a-while", - "kubernetes.io/name": "should-stay", - "kubectl.kubernetes.io/name": "should-stay", - "k8s.io/name": "should-stay", - } - Expect(secret.Annotations).To(Equal(expectedAnnotations)) + By("updating secret annotations on admin secret", func() { + expectedAnnotations := map[string]string{ + "my-annotation": "i-like-this", + "i-was-here-already": "please-dont-delete-me", + "im-here-to-stay.kubernetes.io": "for-a-while", + "kubernetes.io/name": "should-stay", + "kubectl.kubernetes.io/name": "should-stay", + "k8s.io/name": "should-stay", + } + + Expect(secret.Annotations).To(Equal(expectedAnnotations)) + }) }) }) }) diff --git a/internal/resource/erlang_cookie.go b/internal/resource/erlang_cookie.go index 464bc4a7c..5de3cb95c 100644 --- a/internal/resource/erlang_cookie.go +++ b/internal/resource/erlang_cookie.go @@ -63,10 +63,18 @@ func (builder *ErlangCookieBuilder) Update(object runtime.Object) error { return nil } -func randomEncodedString(dataLen int) (string, error) { +func randomBytes(dataLen int) ([]byte, error) { randomBytes := make([]byte, dataLen) if _, err := rand.Read(randomBytes); err != nil { + return nil, err + } + return randomBytes, nil +} + +func randomEncodedString(dataLen int) (string, error) { + generatedBytes, err := randomBytes(dataLen) + if err != nil { return "", err } - return base64.URLEncoding.EncodeToString(randomBytes), nil + return base64.URLEncoding.EncodeToString(generatedBytes), nil } diff --git a/internal/resource/statefulset.go b/internal/resource/statefulset.go index 93b82e195..53e75dd5b 100644 --- a/internal/resource/statefulset.go +++ b/internal/resource/statefulset.go @@ -258,24 +258,6 @@ func (builder *StatefulSetBuilder) podTemplateSpec(annotations, labels map[strin terminationGracePeriod := defaultGracePeriodTimeoutSeconds volumes := []corev1.Volume{ - { - Name: "rabbitmq-admin", - VolumeSource: corev1.VolumeSource{ - Secret: &corev1.SecretVolumeSource{ - SecretName: builder.Instance.ChildResourceName(AdminSecretName), - Items: []corev1.KeyToPath{ - { - Key: "username", - Path: "username", - }, - { - Key: "password", - Path: "password", - }, - }, - }, - }, - }, { Name: "server-conf", VolumeSource: corev1.VolumeSource{ @@ -302,6 +284,20 @@ func (builder *StatefulSetBuilder) podTemplateSpec(annotations, labels map[strin EmptyDir: &corev1.EmptyDirVolumeSource{}, }, }, + { + Name: "rabbitmq-confd", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: builder.Instance.ChildResourceName(AdminSecretName), + Items: []corev1.KeyToPath{ + { + Key: "default_user.conf", + Path: "default_user.conf", + }, + }, + }, + }, + }, { Name: "rabbitmq-erlang-cookie", VolumeSource: corev1.VolumeSource{ @@ -378,10 +374,6 @@ func (builder *StatefulSetBuilder) podTemplateSpec(annotations, labels map[strin } rabbitmqContainerVolumeMounts := []corev1.VolumeMount{ - { - Name: "rabbitmq-admin", - MountPath: "/opt/rabbitmq-secret/", - }, { Name: "persistence", MountPath: "/var/lib/rabbitmq/mnesia/", @@ -390,6 +382,10 @@ func (builder *StatefulSetBuilder) podTemplateSpec(annotations, labels map[strin Name: "rabbitmq-etc", MountPath: "/etc/rabbitmq/", }, + { + Name: "rabbitmq-confd", + MountPath: "/etc/rabbitmq/conf.d/", + }, { Name: "rabbitmq-erlang-cookie", MountPath: "/var/lib/rabbitmq/", @@ -509,6 +505,8 @@ func (builder *StatefulSetBuilder) podTemplateSpec(annotations, labels map[strin "&& chown 999:999 /etc/rabbitmq/advanced.config ; " + "cp /tmp/rabbitmq/rabbitmq-env.conf /etc/rabbitmq/rabbitmq-env.conf " + "&& chown 999:999 /etc/rabbitmq/rabbitmq-env.conf ; " + + "cp /tmp/rabbitmq-admin/default_user.conf /etc/rabbitmq/conf.d/default_user.conf " + + "&& chown 999:999 /etc/rabbitmq/conf.d/*.conf ; " + "cp /tmp/erlang-cookie-secret/.erlang.cookie /var/lib/rabbitmq/.erlang.cookie " + "&& chown 999:999 /var/lib/rabbitmq/.erlang.cookie " + "&& chmod 600 /var/lib/rabbitmq/.erlang.cookie ; " + @@ -535,10 +533,18 @@ func (builder *StatefulSetBuilder) podTemplateSpec(annotations, labels map[strin Name: "plugins-conf", MountPath: "/tmp/rabbitmq-plugins/", }, + { + Name: "rabbitmq-admin", + MountPath: "/tmp/rabbitmq-admin/", + }, { Name: "rabbitmq-etc", MountPath: "/etc/rabbitmq/", }, + { + Name: "rabbitmq-confd", + MountPath: "/etc/rabbitmq/conf.d/", + }, { Name: "rabbitmq-erlang-cookie", MountPath: "/var/lib/rabbitmq/", @@ -561,14 +567,6 @@ func (builder *StatefulSetBuilder) podTemplateSpec(annotations, labels map[strin Resources: *builder.Instance.Spec.Resources, Image: builder.Instance.Spec.Image, Env: []corev1.EnvVar{ - { - Name: "RABBITMQ_DEFAULT_PASS_FILE", - Value: "/opt/rabbitmq-secret/password", - }, - { - Name: "RABBITMQ_DEFAULT_USER_FILE", - Value: "/opt/rabbitmq-secret/username", - }, { Name: "MY_POD_NAME", ValueFrom: &corev1.EnvVarSource{ diff --git a/internal/resource/statefulset_test.go b/internal/resource/statefulset_test.go index 6d1dab055..621c7afcc 100644 --- a/internal/resource/statefulset_test.go +++ b/internal/resource/statefulset_test.go @@ -741,14 +741,6 @@ var _ = Describe("StatefulSet", func() { Expect(stsBuilder.Update(statefulSet)).To(Succeed()) requiredEnvVariables := []corev1.EnvVar{ - { - Name: "RABBITMQ_DEFAULT_PASS_FILE", - Value: "/opt/rabbitmq-secret/password", - }, - { - Name: "RABBITMQ_DEFAULT_USER_FILE", - Value: "/opt/rabbitmq-secret/username", - }, { Name: "MY_POD_NAME", ValueFrom: &corev1.EnvVarSource{ @@ -795,10 +787,6 @@ var _ = Describe("StatefulSet", func() { container := extractContainer(statefulSet.Spec.Template.Spec.Containers, "rabbitmq") Expect(container.VolumeMounts).To(ConsistOf( - corev1.VolumeMount{ - Name: "rabbitmq-admin", - MountPath: "/opt/rabbitmq-secret/", - }, corev1.VolumeMount{ Name: "persistence", MountPath: "/var/lib/rabbitmq/mnesia/", @@ -807,6 +795,10 @@ var _ = Describe("StatefulSet", func() { Name: "rabbitmq-etc", MountPath: "/etc/rabbitmq/", }, + corev1.VolumeMount{ + Name: "rabbitmq-confd", + MountPath: "/etc/rabbitmq/conf.d/", + }, corev1.VolumeMount{ Name: "rabbitmq-erlang-cookie", MountPath: "/var/lib/rabbitmq/", @@ -823,24 +815,6 @@ var _ = Describe("StatefulSet", func() { Expect(stsBuilder.Update(statefulSet)).To(Succeed()) Expect(statefulSet.Spec.Template.Spec.Volumes).To(ConsistOf( - corev1.Volume{ - Name: "rabbitmq-admin", - VolumeSource: corev1.VolumeSource{ - Secret: &corev1.SecretVolumeSource{ - SecretName: instance.ChildResourceName("admin"), - Items: []corev1.KeyToPath{ - { - Key: "username", - Path: "username", - }, - { - Key: "password", - Path: "password", - }, - }, - }, - }, - }, corev1.Volume{ Name: "server-conf", VolumeSource: corev1.VolumeSource{ @@ -867,6 +841,20 @@ var _ = Describe("StatefulSet", func() { EmptyDir: &corev1.EmptyDirVolumeSource{}, }, }, + corev1.Volume{ + Name: "rabbitmq-confd", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: instance.ChildResourceName("admin"), + Items: []corev1.KeyToPath{ + { + Key: "default_user.conf", + Path: "default_user.conf", + }, + }, + }, + }, + }, corev1.Volume{ Name: "rabbitmq-erlang-cookie", VolumeSource: corev1.VolumeSource{ @@ -961,6 +949,8 @@ var _ = Describe("StatefulSet", func() { "&& chown 999:999 /etc/rabbitmq/advanced.config ; "+ "cp /tmp/rabbitmq/rabbitmq-env.conf /etc/rabbitmq/rabbitmq-env.conf "+ "&& chown 999:999 /etc/rabbitmq/rabbitmq-env.conf ; "+ + "cp /tmp/rabbitmq-admin/default_user.conf /etc/rabbitmq/conf.d/default_user.conf "+ + "&& chown 999:999 /etc/rabbitmq/conf.d/*.conf ; "+ "cp /tmp/erlang-cookie-secret/.erlang.cookie /var/lib/rabbitmq/.erlang.cookie "+ "&& chown 999:999 /var/lib/rabbitmq/.erlang.cookie "+ "&& chmod 600 /var/lib/rabbitmq/.erlang.cookie ; "+ @@ -977,10 +967,18 @@ var _ = Describe("StatefulSet", func() { Name: "plugins-conf", MountPath: "/tmp/rabbitmq-plugins/", }, + corev1.VolumeMount{ + Name: "rabbitmq-admin", + MountPath: "/tmp/rabbitmq-admin/", + }, corev1.VolumeMount{ Name: "rabbitmq-etc", MountPath: "/etc/rabbitmq/", }, + corev1.VolumeMount{ + Name: "rabbitmq-confd", + MountPath: "/etc/rabbitmq/conf.d/", + }, corev1.VolumeMount{ Name: "rabbitmq-erlang-cookie", MountPath: "/var/lib/rabbitmq/", @@ -1244,10 +1242,6 @@ var _ = Describe("StatefulSet", func() { { Name: "rabbitmq", Env: []corev1.EnvVar{ - { - Name: "RABBITMQ_DEFAULT_PASS_FILE", - Value: "my-password", - }, { Name: "test1", Value: "test1", @@ -1282,18 +1276,10 @@ var _ = Describe("StatefulSet", func() { Expect(stsBuilder.Update(statefulSet)).To(Succeed()) Expect(extractContainer(statefulSet.Spec.Template.Spec.Containers, "rabbitmq").Env).To(ConsistOf( - corev1.EnvVar{ - Name: "RABBITMQ_DEFAULT_PASS_FILE", - Value: "my-password", - }, corev1.EnvVar{ Name: "test1", Value: "test1", }, - corev1.EnvVar{ - Name: "RABBITMQ_DEFAULT_USER_FILE", - Value: "/opt/rabbitmq-secret/username", - }, corev1.EnvVar{ Name: "MY_POD_NAME", ValueFrom: &corev1.EnvVarSource{ @@ -1333,10 +1319,6 @@ var _ = Describe("StatefulSet", func() { Expect(extractContainer(statefulSet.Spec.Template.Spec.Containers, "new-container-1")).To(Equal( corev1.Container{Name: "new-container-1", Image: "my-image-1"})) Expect(extractContainer(statefulSet.Spec.Template.Spec.Containers, "rabbitmq").VolumeMounts).To(ConsistOf( - corev1.VolumeMount{ - Name: "rabbitmq-admin", - MountPath: "/opt/rabbitmq-secret/", - }, corev1.VolumeMount{ Name: "test", MountPath: "test-path", @@ -1349,6 +1331,10 @@ var _ = Describe("StatefulSet", func() { Name: "rabbitmq-etc", MountPath: "/etc/rabbitmq/", }, + corev1.VolumeMount{ + Name: "rabbitmq-confd", + MountPath: "/etc/rabbitmq/conf.d/", + }, corev1.VolumeMount{ Name: "rabbitmq-erlang-cookie", MountPath: "/var/lib/rabbitmq/", From 0bce98af719fe5d90b73c611c2733f9bc9946a6a Mon Sep 17 00:00:00 2001 From: Connor Rogers Date: Wed, 16 Sep 2020 16:58:39 +0000 Subject: [PATCH 2/4] Correct admin secret volumes rabbitmq-admin is the temporary volume for the initContainer, and contains the actual secret data. rabbitmq-confd is the empty volume that is populated by the initContainer from rabbitmq-admin. --- .../rabbitmqcluster_controller_test.go | 26 ++++++++++++------- internal/resource/statefulset.go | 24 ++++++++++------- internal/resource/statefulset_test.go | 24 ++++++++++------- 3 files changed, 46 insertions(+), 28 deletions(-) diff --git a/controllers/rabbitmqcluster_controller_test.go b/controllers/rabbitmqcluster_controller_test.go index 5a4e9c8c0..c12261b0d 100644 --- a/controllers/rabbitmqcluster_controller_test.go +++ b/controllers/rabbitmqcluster_controller_test.go @@ -1206,6 +1206,21 @@ var _ = Describe("RabbitmqclusterController", func() { Expect(sts.Spec.Template.Spec.HostNetwork).To(BeFalse()) Expect(sts.Spec.Template.Spec.Volumes).To(ConsistOf( + corev1.Volume{ + Name: "rabbitmq-admin", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + DefaultMode: &defaultMode, + SecretName: "rabbitmq-sts-override-rabbitmq-admin", + Items: []corev1.KeyToPath{ + { + Key: "default_user.conf", + Path: "default_user.conf", + }, + }, + }, + }, + }, corev1.Volume{ Name: "additional-config", VolumeSource: corev1.VolumeSource{ @@ -1220,16 +1235,7 @@ var _ = Describe("RabbitmqclusterController", func() { corev1.Volume{ Name: "rabbitmq-confd", VolumeSource: corev1.VolumeSource{ - Secret: &corev1.SecretVolumeSource{ - DefaultMode: &defaultMode, - SecretName: "rabbitmq-sts-override-rabbitmq-admin", - Items: []corev1.KeyToPath{ - { - Key: "default_user.conf", - Path: "default_user.conf", - }, - }, - }, + EmptyDir: &corev1.EmptyDirVolumeSource{}, }, }, corev1.Volume{ diff --git a/internal/resource/statefulset.go b/internal/resource/statefulset.go index 53e75dd5b..54cb03847 100644 --- a/internal/resource/statefulset.go +++ b/internal/resource/statefulset.go @@ -258,6 +258,20 @@ func (builder *StatefulSetBuilder) podTemplateSpec(annotations, labels map[strin terminationGracePeriod := defaultGracePeriodTimeoutSeconds volumes := []corev1.Volume{ + { + Name: "rabbitmq-admin", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: builder.Instance.ChildResourceName(AdminSecretName), + Items: []corev1.KeyToPath{ + { + Key: "default_user.conf", + Path: "default_user.conf", + }, + }, + }, + }, + }, { Name: "server-conf", VolumeSource: corev1.VolumeSource{ @@ -287,15 +301,7 @@ func (builder *StatefulSetBuilder) podTemplateSpec(annotations, labels map[strin { Name: "rabbitmq-confd", VolumeSource: corev1.VolumeSource{ - Secret: &corev1.SecretVolumeSource{ - SecretName: builder.Instance.ChildResourceName(AdminSecretName), - Items: []corev1.KeyToPath{ - { - Key: "default_user.conf", - Path: "default_user.conf", - }, - }, - }, + EmptyDir: &corev1.EmptyDirVolumeSource{}, }, }, { diff --git a/internal/resource/statefulset_test.go b/internal/resource/statefulset_test.go index 621c7afcc..5dc250013 100644 --- a/internal/resource/statefulset_test.go +++ b/internal/resource/statefulset_test.go @@ -815,6 +815,20 @@ var _ = Describe("StatefulSet", func() { Expect(stsBuilder.Update(statefulSet)).To(Succeed()) Expect(statefulSet.Spec.Template.Spec.Volumes).To(ConsistOf( + corev1.Volume{ + Name: "rabbitmq-admin", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: instance.ChildResourceName("admin"), + Items: []corev1.KeyToPath{ + { + Key: "default_user.conf", + Path: "default_user.conf", + }, + }, + }, + }, + }, corev1.Volume{ Name: "server-conf", VolumeSource: corev1.VolumeSource{ @@ -844,15 +858,7 @@ var _ = Describe("StatefulSet", func() { corev1.Volume{ Name: "rabbitmq-confd", VolumeSource: corev1.VolumeSource{ - Secret: &corev1.SecretVolumeSource{ - SecretName: instance.ChildResourceName("admin"), - Items: []corev1.KeyToPath{ - { - Key: "default_user.conf", - Path: "default_user.conf", - }, - }, - }, + EmptyDir: &corev1.EmptyDirVolumeSource{}, }, }, corev1.Volume{ From df0fab00e9e680e6153138bc78128812d8eb09b0 Mon Sep 17 00:00:00 2001 From: Connor Rogers Date: Thu, 17 Sep 2020 10:04:35 +0000 Subject: [PATCH 3/4] Fix double-encoding of config file and update README with min RMQ version --- README.md | 2 +- internal/resource/admin_secret.go | 17 ++++++++--------- internal/resource/admin_secret_test.go | 23 ++++++++++------------- internal/resource/erlang_cookie.go | 12 ++---------- 4 files changed, 21 insertions(+), 33 deletions(-) diff --git a/README.md b/README.md index 93b360259..c4c758df8 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ Manage [RabbitMQ](https://www.rabbitmq.com/) clusters deployed to [Kubernetes](h ## Supported Versions -The operator deploys RabbitMQ `3.8.8`, and requires a Kubernetes cluster of `1.16` or above. +The operator deploys RabbitMQ `3.8.8` by default, and supports versions from `3.8.4` upwards. The operator requires a Kubernetes cluster of `1.16` or above. ## Versioning diff --git a/internal/resource/admin_secret.go b/internal/resource/admin_secret.go index e6fd2e853..4ee8ef437 100644 --- a/internal/resource/admin_secret.go +++ b/internal/resource/admin_secret.go @@ -11,7 +11,6 @@ package resource import ( "bytes" - "encoding/base64" rabbitmqv1beta1 "github.com/rabbitmq/cluster-operator/api/v1beta1" "github.com/rabbitmq/cluster-operator/internal/metadata" @@ -35,7 +34,7 @@ func (builder *RabbitmqResourceBuilder) AdminSecret() *AdminSecretBuilder { } } -func generateDefaultUserConf(username, password []byte) ([]byte, error) { +func generateDefaultUserConf(username, password string) ([]byte, error) { ini.PrettySection = false // Remove trailing new line because default_user.conf has only a default section. cfg, err := ini.Load([]byte{}) @@ -44,11 +43,11 @@ func generateDefaultUserConf(username, password []byte) ([]byte, error) { } defaultSection := cfg.Section("") - if _, err := defaultSection.NewKey("default_user", string(username)); err != nil { + if _, err := defaultSection.NewKey("default_user", username); err != nil { return nil, err } - if _, err := defaultSection.NewKey("default_pass", string(password)); err != nil { + if _, err := defaultSection.NewKey("default_pass", password); err != nil { return nil, err } @@ -72,12 +71,12 @@ func (builder *AdminSecretBuilder) Update(object runtime.Object) error { } func (builder *AdminSecretBuilder) Build() (runtime.Object, error) { - username, err := randomBytes(24) + username, err := randomEncodedString(24) if err != nil { return nil, err } - password, err := randomBytes(24) + password, err := randomEncodedString(24) if err != nil { return nil, err } @@ -94,9 +93,9 @@ func (builder *AdminSecretBuilder) Build() (runtime.Object, error) { }, Type: corev1.SecretTypeOpaque, Data: map[string][]byte{ - "username": []byte(base64.URLEncoding.EncodeToString(username)), - "password": []byte(base64.URLEncoding.EncodeToString(password)), - "default_user.conf": []byte(base64.URLEncoding.EncodeToString(defaultUserConf)), + "username": []byte(username), + "password": []byte(password), + "default_user.conf": defaultUserConf, }, }, nil } diff --git a/internal/resource/admin_secret_test.go b/internal/resource/admin_secret_test.go index de73c6531..e73061242 100644 --- a/internal/resource/admin_secret_test.go +++ b/internal/resource/admin_secret_test.go @@ -46,9 +46,9 @@ var _ = Describe("AdminSecret", func() { Context("Build with defaults", func() { It("creates the necessary admin secret", func() { - var decodedUsername []byte - var decodedPassword []byte - var err error + var username []byte + var password []byte + var ok bool obj, err := adminSecretBuilder.Build() Expect(err).NotTo(HaveOccurred()) @@ -64,18 +64,18 @@ var _ = Describe("AdminSecret", func() { }) By("creating a rabbitmq username that is base64 encoded and 24 characters in length", func() { - username, ok := secret.Data["username"] + username, ok = secret.Data["username"] Expect(ok).NotTo(BeFalse()) - decodedUsername, err = b64.URLEncoding.DecodeString(string(username)) + decodedUsername, err := b64.URLEncoding.DecodeString(string(username)) Expect(err).NotTo(HaveOccurred()) Expect(len(decodedUsername)).To(Equal(24)) }) By("creating a rabbitmq password that is base64 encoded and 24 characters in length", func() { - password, ok := secret.Data["password"] + password, ok = secret.Data["password"] Expect(ok).NotTo(BeFalse()) - decodedPassword, err = b64.URLEncoding.DecodeString(string(password)) + decodedPassword, err := b64.URLEncoding.DecodeString(string(password)) Expect(err).NotTo(HaveOccurred()) Expect(len(decodedPassword)).To(Equal(24)) }) @@ -84,17 +84,14 @@ var _ = Describe("AdminSecret", func() { defaultUserConf, ok := secret.Data["default_user.conf"] Expect(ok).NotTo(BeFalse()) - decodedDefaultUserConf, err := b64.URLEncoding.DecodeString(string(defaultUserConf)) - Expect(err).NotTo(HaveOccurred()) - - cfg, err := ini.Load(decodedDefaultUserConf) + cfg, err := ini.Load(defaultUserConf) Expect(err).NotTo(HaveOccurred()) Expect(cfg.Section("").HasKey("default_user")).To(BeTrue()) Expect(cfg.Section("").HasKey("default_pass")).To(BeTrue()) - Expect(cfg.Section("").Key("default_user").Value()).To(Equal(string(decodedUsername))) - Expect(cfg.Section("").Key("default_pass").Value()).To(Equal(string(decodedPassword))) + Expect(cfg.Section("").Key("default_user").Value()).To(Equal(string(username))) + Expect(cfg.Section("").Key("default_pass").Value()).To(Equal(string(password))) }) }) }) diff --git a/internal/resource/erlang_cookie.go b/internal/resource/erlang_cookie.go index 5de3cb95c..464bc4a7c 100644 --- a/internal/resource/erlang_cookie.go +++ b/internal/resource/erlang_cookie.go @@ -63,18 +63,10 @@ func (builder *ErlangCookieBuilder) Update(object runtime.Object) error { return nil } -func randomBytes(dataLen int) ([]byte, error) { +func randomEncodedString(dataLen int) (string, error) { randomBytes := make([]byte, dataLen) if _, err := rand.Read(randomBytes); err != nil { - return nil, err - } - return randomBytes, nil -} - -func randomEncodedString(dataLen int) (string, error) { - generatedBytes, err := randomBytes(dataLen) - if err != nil { return "", err } - return base64.URLEncoding.EncodeToString(generatedBytes), nil + return base64.URLEncoding.EncodeToString(randomBytes), nil } From 706cf8fdecb085009f4fc2de56e2a381e3a400e6 Mon Sep 17 00:00:00 2001 From: Connor Rogers Date: Fri, 18 Sep 2020 09:15:37 +0000 Subject: [PATCH 4/4] Change admin secret to projected volume This removes the logic for copying over from the initContainer, which makes this author a very happy bunny. --- .../rabbitmqcluster_controller_test.go | 34 ++++++++------- internal/resource/admin_secret_test.go | 7 ++-- internal/resource/statefulset.go | 42 ++++++++----------- internal/resource/statefulset_test.go | 42 ++++++++----------- 4 files changed, 55 insertions(+), 70 deletions(-) diff --git a/controllers/rabbitmqcluster_controller_test.go b/controllers/rabbitmqcluster_controller_test.go index c12261b0d..621a19668 100644 --- a/controllers/rabbitmqcluster_controller_test.go +++ b/controllers/rabbitmqcluster_controller_test.go @@ -1206,21 +1206,6 @@ var _ = Describe("RabbitmqclusterController", func() { Expect(sts.Spec.Template.Spec.HostNetwork).To(BeFalse()) Expect(sts.Spec.Template.Spec.Volumes).To(ConsistOf( - corev1.Volume{ - Name: "rabbitmq-admin", - VolumeSource: corev1.VolumeSource{ - Secret: &corev1.SecretVolumeSource{ - DefaultMode: &defaultMode, - SecretName: "rabbitmq-sts-override-rabbitmq-admin", - Items: []corev1.KeyToPath{ - { - Key: "default_user.conf", - Path: "default_user.conf", - }, - }, - }, - }, - }, corev1.Volume{ Name: "additional-config", VolumeSource: corev1.VolumeSource{ @@ -1235,7 +1220,24 @@ var _ = Describe("RabbitmqclusterController", func() { corev1.Volume{ Name: "rabbitmq-confd", VolumeSource: corev1.VolumeSource{ - EmptyDir: &corev1.EmptyDirVolumeSource{}, + Projected: &corev1.ProjectedVolumeSource{ + Sources: []corev1.VolumeProjection{ + { + Secret: &corev1.SecretProjection{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "rabbitmq-sts-override-rabbitmq-admin", + }, + Items: []corev1.KeyToPath{ + { + Key: "default_user.conf", + Path: "default_user.conf", + }, + }, + }, + }, + }, + DefaultMode: &defaultMode, + }, }, }, corev1.Volume{ diff --git a/internal/resource/admin_secret_test.go b/internal/resource/admin_secret_test.go index e73061242..aa1ae6b24 100644 --- a/internal/resource/admin_secret_test.go +++ b/internal/resource/admin_secret_test.go @@ -65,16 +65,15 @@ var _ = Describe("AdminSecret", func() { By("creating a rabbitmq username that is base64 encoded and 24 characters in length", func() { username, ok = secret.Data["username"] - Expect(ok).NotTo(BeFalse()) + Expect(ok).NotTo(BeFalse(), "Failed to find a key \"username\" in the generated Secret") decodedUsername, err := b64.URLEncoding.DecodeString(string(username)) Expect(err).NotTo(HaveOccurred()) Expect(len(decodedUsername)).To(Equal(24)) - }) By("creating a rabbitmq password that is base64 encoded and 24 characters in length", func() { password, ok = secret.Data["password"] - Expect(ok).NotTo(BeFalse()) + Expect(ok).NotTo(BeFalse(), "Failed to find a key \"password\" in the generated Secret") decodedPassword, err := b64.URLEncoding.DecodeString(string(password)) Expect(err).NotTo(HaveOccurred()) Expect(len(decodedPassword)).To(Equal(24)) @@ -82,7 +81,7 @@ var _ = Describe("AdminSecret", func() { By("creating a default_user.conf file that contains the correct sysctl config format to be parsed by RabbitMQ", func() { defaultUserConf, ok := secret.Data["default_user.conf"] - Expect(ok).NotTo(BeFalse()) + Expect(ok).NotTo(BeFalse(), "Failed to find a key \"default_user.conf\" in the generated Secret") cfg, err := ini.Load(defaultUserConf) Expect(err).NotTo(HaveOccurred()) diff --git a/internal/resource/statefulset.go b/internal/resource/statefulset.go index 54cb03847..0879e61dc 100644 --- a/internal/resource/statefulset.go +++ b/internal/resource/statefulset.go @@ -258,20 +258,6 @@ func (builder *StatefulSetBuilder) podTemplateSpec(annotations, labels map[strin terminationGracePeriod := defaultGracePeriodTimeoutSeconds volumes := []corev1.Volume{ - { - Name: "rabbitmq-admin", - VolumeSource: corev1.VolumeSource{ - Secret: &corev1.SecretVolumeSource{ - SecretName: builder.Instance.ChildResourceName(AdminSecretName), - Items: []corev1.KeyToPath{ - { - Key: "default_user.conf", - Path: "default_user.conf", - }, - }, - }, - }, - }, { Name: "server-conf", VolumeSource: corev1.VolumeSource{ @@ -301,7 +287,23 @@ func (builder *StatefulSetBuilder) podTemplateSpec(annotations, labels map[strin { Name: "rabbitmq-confd", VolumeSource: corev1.VolumeSource{ - EmptyDir: &corev1.EmptyDirVolumeSource{}, + Projected: &corev1.ProjectedVolumeSource{ + Sources: []corev1.VolumeProjection{ + { + Secret: &corev1.SecretProjection{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: builder.Instance.ChildResourceName(AdminSecretName), + }, + Items: []corev1.KeyToPath{ + { + Key: "default_user.conf", + Path: "default_user.conf", + }, + }, + }, + }, + }, + }, }, }, { @@ -511,8 +513,6 @@ func (builder *StatefulSetBuilder) podTemplateSpec(annotations, labels map[strin "&& chown 999:999 /etc/rabbitmq/advanced.config ; " + "cp /tmp/rabbitmq/rabbitmq-env.conf /etc/rabbitmq/rabbitmq-env.conf " + "&& chown 999:999 /etc/rabbitmq/rabbitmq-env.conf ; " + - "cp /tmp/rabbitmq-admin/default_user.conf /etc/rabbitmq/conf.d/default_user.conf " + - "&& chown 999:999 /etc/rabbitmq/conf.d/*.conf ; " + "cp /tmp/erlang-cookie-secret/.erlang.cookie /var/lib/rabbitmq/.erlang.cookie " + "&& chown 999:999 /var/lib/rabbitmq/.erlang.cookie " + "&& chmod 600 /var/lib/rabbitmq/.erlang.cookie ; " + @@ -539,18 +539,10 @@ func (builder *StatefulSetBuilder) podTemplateSpec(annotations, labels map[strin Name: "plugins-conf", MountPath: "/tmp/rabbitmq-plugins/", }, - { - Name: "rabbitmq-admin", - MountPath: "/tmp/rabbitmq-admin/", - }, { Name: "rabbitmq-etc", MountPath: "/etc/rabbitmq/", }, - { - Name: "rabbitmq-confd", - MountPath: "/etc/rabbitmq/conf.d/", - }, { Name: "rabbitmq-erlang-cookie", MountPath: "/var/lib/rabbitmq/", diff --git a/internal/resource/statefulset_test.go b/internal/resource/statefulset_test.go index 5dc250013..d411bb869 100644 --- a/internal/resource/statefulset_test.go +++ b/internal/resource/statefulset_test.go @@ -815,20 +815,6 @@ var _ = Describe("StatefulSet", func() { Expect(stsBuilder.Update(statefulSet)).To(Succeed()) Expect(statefulSet.Spec.Template.Spec.Volumes).To(ConsistOf( - corev1.Volume{ - Name: "rabbitmq-admin", - VolumeSource: corev1.VolumeSource{ - Secret: &corev1.SecretVolumeSource{ - SecretName: instance.ChildResourceName("admin"), - Items: []corev1.KeyToPath{ - { - Key: "default_user.conf", - Path: "default_user.conf", - }, - }, - }, - }, - }, corev1.Volume{ Name: "server-conf", VolumeSource: corev1.VolumeSource{ @@ -858,7 +844,23 @@ var _ = Describe("StatefulSet", func() { corev1.Volume{ Name: "rabbitmq-confd", VolumeSource: corev1.VolumeSource{ - EmptyDir: &corev1.EmptyDirVolumeSource{}, + Projected: &corev1.ProjectedVolumeSource{ + Sources: []corev1.VolumeProjection{ + { + Secret: &corev1.SecretProjection{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: builder.Instance.ChildResourceName("admin"), + }, + Items: []corev1.KeyToPath{ + { + Key: "default_user.conf", + Path: "default_user.conf", + }, + }, + }, + }, + }, + }, }, }, corev1.Volume{ @@ -955,8 +957,6 @@ var _ = Describe("StatefulSet", func() { "&& chown 999:999 /etc/rabbitmq/advanced.config ; "+ "cp /tmp/rabbitmq/rabbitmq-env.conf /etc/rabbitmq/rabbitmq-env.conf "+ "&& chown 999:999 /etc/rabbitmq/rabbitmq-env.conf ; "+ - "cp /tmp/rabbitmq-admin/default_user.conf /etc/rabbitmq/conf.d/default_user.conf "+ - "&& chown 999:999 /etc/rabbitmq/conf.d/*.conf ; "+ "cp /tmp/erlang-cookie-secret/.erlang.cookie /var/lib/rabbitmq/.erlang.cookie "+ "&& chown 999:999 /var/lib/rabbitmq/.erlang.cookie "+ "&& chmod 600 /var/lib/rabbitmq/.erlang.cookie ; "+ @@ -973,18 +973,10 @@ var _ = Describe("StatefulSet", func() { Name: "plugins-conf", MountPath: "/tmp/rabbitmq-plugins/", }, - corev1.VolumeMount{ - Name: "rabbitmq-admin", - MountPath: "/tmp/rabbitmq-admin/", - }, corev1.VolumeMount{ Name: "rabbitmq-etc", MountPath: "/etc/rabbitmq/", }, - corev1.VolumeMount{ - Name: "rabbitmq-confd", - MountPath: "/etc/rabbitmq/conf.d/", - }, corev1.VolumeMount{ Name: "rabbitmq-erlang-cookie", MountPath: "/var/lib/rabbitmq/",