Skip to content

Add support for advanced.config #221

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jul 29, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions api/v1beta1/rabbitmqcluster_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,9 @@ type RabbitmqClusterConfigurationSpec struct {
// Modify to add to the rabbitmq.conf file in addition to default configurations set by the operator. Modify this property on an existing RabbitmqCluster will trigger a StatefulSet rolling restart and will cause rabbitmq downtime.
// +kubebuilder:validation:MaxLength:=2000
AdditionalConfig string `json:"additionalConfig,omitempty"`
// Specify any rabbitmq advanced.config configurations
// +kubebuilder:validation:MaxLength:=2000
AdvancedConfig string `json:"advancedConfig,omitempty"`
}

// The settings for the persistent storage desired for each Pod in the RabbitmqCluster.
Expand Down
12 changes: 6 additions & 6 deletions config/crd/bases/rabbitmq.com_rabbitmqclusters.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,10 @@ spec:
description: RabbitmqCluster is the Schema for the rabbitmqclusters API
properties:
apiVersion:
description: 'APIVersion defines the versioned schema of this representation
of an object. Servers should convert recognized schemas to the latest
internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
description: 'APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
type: string
kind:
description: 'Kind is a string value representing the REST resource this
object represents. Servers may infer this from the endpoint the client
submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
description: 'Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
type: string
metadata:
type: object
Expand Down Expand Up @@ -4476,6 +4472,10 @@ spec:
type: string
maxItems: 100
type: array
advancedConfig:
description: Specify any rabbitmq advanced.config configurations
maxLength: 2000
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this limit required? 2000 characters can be exceeded in environments with extensive LDAP queries and OAuth 2 settings, for example.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not required. There is a hard limit in kubernetes that all objects are stored in etcd, and etcd has a limit of 1MB per object which translate to 1048576 characters. I will update the limit to 100000. Thanks for pointing it out Michael!

type: string
type: object
replicas:
description: Replicas is the number of nodes in the RabbitMQ cluster.
Expand Down
4 changes: 4 additions & 0 deletions internal/resource/configmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ func (builder *ServerConfigMapBuilder) Update(object runtime.Object) error {
if builder.Instance.Spec.Rabbitmq.AdditionalConfig != "" {
configMap.Data["rabbitmq.conf"] = configMap.Data["rabbitmq.conf"] + builder.Instance.Spec.Rabbitmq.AdditionalConfig
}

if builder.Instance.Spec.Rabbitmq.AdvancedConfig != "" {
configMap.Data["advanced.config"] = builder.Instance.Spec.Rabbitmq.AdvancedConfig
}
return nil
}

Expand Down
12 changes: 12 additions & 0 deletions internal/resource/configmap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,18 @@ my-config-property-1 = better-value`
Expect(rabbitmqConf).To(Equal(expectedRabbitmqConf))
})

It("sets data.advancedConfig when provided", func() {
instance.Spec.Rabbitmq.AdvancedConfig = `
[
{rabbit, [{auth_backends, [rabbit_auth_backend_ldap]}]}
].`
Expect(configMapBuilder.Update(configMap)).To(Succeed())
advancedConfig, ok := configMap.Data["advanced.config"]
Expect(ok).To(BeTrue())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cosmetic change request 🙃 Let's annotate the assertion so that if this ever fails, we get a message more descriptive than "expected false to be true"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated in the new commit ;) good call Aitor

Expect(advancedConfig).To(Equal("\n[\n {rabbit, [{auth_backends, [rabbit_auth_backend_ldap]}]}\n]."))

})

Context("TLS", func() {
It("adds TLS config when TLS is enabled", func() {
instance = rabbitmqv1beta1.RabbitmqCluster{
Expand Down
1 change: 1 addition & 0 deletions internal/resource/statefulset.go
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,7 @@ func (builder *StatefulSetBuilder) podTemplateSpec(annotations, labels map[strin
Image: builder.Instance.Spec.Image,
Command: []string{
"sh", "-c", "cp /tmp/rabbitmq/rabbitmq.conf /etc/rabbitmq/rabbitmq.conf && echo '' >> /etc/rabbitmq/rabbitmq.conf ; " +
"cp /tmp/rabbitmq/advanced.config /etc/rabbitmq/advanced.config ; " +
"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 ; " +
Expand Down
1 change: 1 addition & 0 deletions internal/resource/statefulset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -918,6 +918,7 @@ var _ = Describe("StatefulSet", func() {
container := extractContainer(initContainers, "copy-config")
Expect(container.Command).To(Equal([]string{
"sh", "-c", "cp /tmp/rabbitmq/rabbitmq.conf /etc/rabbitmq/rabbitmq.conf && echo '' >> /etc/rabbitmq/rabbitmq.conf ; " +
"cp /tmp/rabbitmq/advanced.config /etc/rabbitmq/advanced.config ; " +
"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 ; " +
Expand Down
19 changes: 19 additions & 0 deletions system_tests/system_tests.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,25 @@ cluster_keepalive_interval = 10000`
Expect(string(output)).Should(ContainSubstring("cluster_keepalive_interval = 10000"))
Expect(string(output)).Should(ContainSubstring("cluster_partition_handling = ignore"))
})

By("updating the advanced.config file when advancedConfig are modifed", func() {
Expect(updateRabbitmqCluster(rmqClusterClient, cluster.Name, cluster.Namespace, func(cluster *rabbitmqv1beta1.RabbitmqCluster) {
cluster.Spec.Rabbitmq.AdvancedConfig = `[
{rabbit, [{auth_backends, [rabbit_auth_backend_ldap]}]}
].`
})).To(Succeed())

// wait for statefulSet to be restarted
waitForRabbitmqUpdate(cluster)

output, err := kubectlExec(namespace,
statefulSetPodName(cluster, 0),
"cat",
"/etc/rabbitmq/advanced.config",
)
Expect(err).NotTo(HaveOccurred())
Expect(string(output)).Should(ContainSubstring("[\n {rabbit, [{auth_backends, [rabbit_auth_backend_ldap]}]}\n]."))
})
})
})

Expand Down