diff --git a/api/v1alpha1/superstream_webhook.go b/api/v1alpha1/superstream_webhook.go index af4cd6e6..1edd9293 100644 --- a/api/v1alpha1/superstream_webhook.go +++ b/api/v1alpha1/superstream_webhook.go @@ -28,9 +28,10 @@ func (s *SuperStream) SetupWebhookWithManager(mgr ctrl.Manager) error { var _ webhook.Validator = &SuperStream{} -// no validation on create +// ValidateCreate implements webhook.Validator so a webhook will be registered for the type +// either rabbitmqClusterReference.name or rabbitmqClusterReference.connectionSecret must be provided but not both func (s *SuperStream) ValidateCreate() error { - return nil + return s.Spec.RabbitmqClusterReference.ValidateOnCreate(s.GroupResource(), s.Name) } // returns error type 'forbidden' for updates on superstream name, vhost and rabbitmqClusterReference @@ -50,7 +51,7 @@ func (s *SuperStream) ValidateUpdate(old runtime.Object) error { field.Forbidden(field.NewPath("spec", "vhost"), detailMsg)) } - if s.Spec.RabbitmqClusterReference != oldSuperStream.Spec.RabbitmqClusterReference { + if !oldSuperStream.Spec.RabbitmqClusterReference.Matches(&s.Spec.RabbitmqClusterReference) { return apierrors.NewForbidden(s.GroupResource(), s.Name, field.Forbidden(field.NewPath("spec", "rabbitmqClusterReference"), detailMsg)) } diff --git a/api/v1alpha1/superstream_webhook_test.go b/api/v1alpha1/superstream_webhook_test.go index 09969890..7a06183f 100644 --- a/api/v1alpha1/superstream_webhook_test.go +++ b/api/v1alpha1/superstream_webhook_test.go @@ -4,6 +4,7 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" topologyv1beta1 "github.com/rabbitmq/messaging-topology-operator/api/v1beta1" + corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -26,54 +27,77 @@ var _ = Describe("superstream webhook", func() { } }) - It("does not allow updates on superstream name", func() { - newSuperStream := superstream.DeepCopy() - newSuperStream.Spec.Name = "new-name" - Expect(apierrors.IsForbidden(newSuperStream.ValidateUpdate(&superstream))).To(BeTrue()) - }) + Context("ValidateCreate", func() { + It("does not allow both spec.rabbitmqClusterReference.name and spec.rabbitmqClusterReference.connectionSecret be configured", func() { + notAllowed := superstream.DeepCopy() + notAllowed.Spec.RabbitmqClusterReference.ConnectionSecret = &corev1.LocalObjectReference{Name: "some-secret"} + Expect(apierrors.IsForbidden(notAllowed.ValidateCreate())).To(BeTrue()) + }) - It("does not allow updates on superstream vhost", func() { - newSuperStream := superstream.DeepCopy() - newSuperStream.Spec.Vhost = "new-vhost" - Expect(apierrors.IsForbidden(newSuperStream.ValidateUpdate(&superstream))).To(BeTrue()) + It("spec.rabbitmqClusterReference.name and spec.rabbitmqClusterReference.connectionSecret cannot both be empty", func() { + notAllowed := superstream.DeepCopy() + notAllowed.Spec.RabbitmqClusterReference.Name = "" + notAllowed.Spec.RabbitmqClusterReference.ConnectionSecret = nil + Expect(apierrors.IsForbidden(notAllowed.ValidateCreate())).To(BeTrue()) + }) }) - It("does not allow updates on RabbitmqClusterReference", func() { - newSuperStream := superstream.DeepCopy() - newSuperStream.Spec.RabbitmqClusterReference = topologyv1beta1.RabbitmqClusterReference{ - Name: "new-cluster", - } - Expect(apierrors.IsForbidden(newSuperStream.ValidateUpdate(&superstream))).To(BeTrue()) - }) + Context("ValidateUpdate", func() { + It("does not allow updates on superstream name", func() { + newSuperStream := superstream.DeepCopy() + newSuperStream.Spec.Name = "new-name" + Expect(apierrors.IsForbidden(newSuperStream.ValidateUpdate(&superstream))).To(BeTrue()) + }) - It("does not allow updates on superstream.spec.routingKeys", func() { - newSuperStream := superstream.DeepCopy() - newSuperStream.Spec.RoutingKeys = []string{"a1", "d6"} - Expect(apierrors.IsForbidden(newSuperStream.ValidateUpdate(&superstream))).To(BeTrue()) - }) + It("does not allow updates on superstream vhost", func() { + newSuperStream := superstream.DeepCopy() + newSuperStream.Spec.Vhost = "new-vhost" + Expect(apierrors.IsForbidden(newSuperStream.ValidateUpdate(&superstream))).To(BeTrue()) + }) - It("if the superstream previously had routing keys and the update only appends, the update succeeds", func() { - newSuperStream := superstream.DeepCopy() - newSuperStream.Spec.RoutingKeys = []string{"a1", "b2", "f17", "z66"} - Expect(newSuperStream.ValidateUpdate(&superstream)).To(Succeed()) - }) + It("does not allow updates on RabbitmqClusterReference", func() { + newSuperStream := superstream.DeepCopy() + newSuperStream.Spec.RabbitmqClusterReference = topologyv1beta1.RabbitmqClusterReference{ + Name: "new-cluster", + } + Expect(apierrors.IsForbidden(newSuperStream.ValidateUpdate(&superstream))).To(BeTrue()) + }) - It("if the superstream previously had no routing keys but now does, the update fails", func() { - superstream.Spec.RoutingKeys = nil - newSuperStream := superstream.DeepCopy() - newSuperStream.Spec.RoutingKeys = []string{"a1", "b2", "f17"} - Expect(apierrors.IsForbidden(newSuperStream.ValidateUpdate(&superstream))).To(BeTrue()) - }) + It("does not allow updates on rabbitmqClusterReference.connectionSecret", func() { + newSuperStream := superstream.DeepCopy() + newSuperStream.Spec.RabbitmqClusterReference = topologyv1beta1.RabbitmqClusterReference{ConnectionSecret: &corev1.LocalObjectReference{Name: "a-secret"}} + Expect(apierrors.IsForbidden(newSuperStream.ValidateUpdate(&superstream))).To(BeTrue()) + }) - It("allows superstream.spec.partitions to be increased", func() { - newSuperStream := superstream.DeepCopy() - newSuperStream.Spec.Partitions = 1000 - Expect(newSuperStream.ValidateUpdate(&superstream)).To(Succeed()) - }) - It("does not allow superstream.spec.partitions to be decreased", func() { - newSuperStream := superstream.DeepCopy() - newSuperStream.Spec.Partitions = 1 - Expect(apierrors.IsForbidden(newSuperStream.ValidateUpdate(&superstream))).To(BeTrue()) - }) + It("does not allow updates on superstream.spec.routingKeys", func() { + newSuperStream := superstream.DeepCopy() + newSuperStream.Spec.RoutingKeys = []string{"a1", "d6"} + Expect(apierrors.IsForbidden(newSuperStream.ValidateUpdate(&superstream))).To(BeTrue()) + }) + + It("if the superstream previously had routing keys and the update only appends, the update succeeds", func() { + newSuperStream := superstream.DeepCopy() + newSuperStream.Spec.RoutingKeys = []string{"a1", "b2", "f17", "z66"} + Expect(newSuperStream.ValidateUpdate(&superstream)).To(Succeed()) + }) + It("if the superstream previously had no routing keys but now does, the update fails", func() { + superstream.Spec.RoutingKeys = nil + newSuperStream := superstream.DeepCopy() + newSuperStream.Spec.RoutingKeys = []string{"a1", "b2", "f17"} + Expect(apierrors.IsForbidden(newSuperStream.ValidateUpdate(&superstream))).To(BeTrue()) + }) + + It("allows superstream.spec.partitions to be increased", func() { + newSuperStream := superstream.DeepCopy() + newSuperStream.Spec.Partitions = 1000 + Expect(newSuperStream.ValidateUpdate(&superstream)).To(Succeed()) + }) + + It("does not allow superstream.spec.partitions to be decreased", func() { + newSuperStream := superstream.DeepCopy() + newSuperStream.Spec.Partitions = 1 + Expect(apierrors.IsForbidden(newSuperStream.ValidateUpdate(&superstream))).To(BeTrue()) + }) + }) }) diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 91adbe8a..d368cfb4 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -86,7 +86,7 @@ func (in *SuperStreamSpec) DeepCopyInto(out *SuperStreamSpec) { *out = make([]string, len(*in)) copy(*out, *in) } - out.RabbitmqClusterReference = in.RabbitmqClusterReference + in.RabbitmqClusterReference.DeepCopyInto(&out.RabbitmqClusterReference) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SuperStreamSpec. diff --git a/api/v1beta1/binding_webhook.go b/api/v1beta1/binding_webhook.go index 67ce1fbc..04824195 100644 --- a/api/v1beta1/binding_webhook.go +++ b/api/v1beta1/binding_webhook.go @@ -20,12 +20,13 @@ func (b *Binding) SetupWebhookWithManager(mgr ctrl.Manager) error { var _ webhook.Validator = &Binding{} -// no validation logic on create +// ValidateCreate implements webhook.Validator so a webhook will be registered for the type +// either rabbitmqClusterReference.name or rabbitmqClusterReference.connectionSecret must be provided but not both func (b *Binding) ValidateCreate() error { - return nil + return b.Spec.RabbitmqClusterReference.ValidateOnCreate(b.GroupResource(), b.Name) } -// updates on bindings.rabbitmq.com is forbidden +// ValidateUpdate updates on vhost and rabbitmqClusterReference are forbidden func (b *Binding) ValidateUpdate(old runtime.Object) error { oldBinding, ok := old.(*Binding) if !ok { @@ -40,7 +41,7 @@ func (b *Binding) ValidateUpdate(old runtime.Object) error { field.Forbidden(field.NewPath("spec", "vhost"), detailMsg)) } - if b.Spec.RabbitmqClusterReference != oldBinding.Spec.RabbitmqClusterReference { + if !oldBinding.Spec.RabbitmqClusterReference.Matches(&b.Spec.RabbitmqClusterReference) { return apierrors.NewForbidden(b.GroupResource(), b.Name, field.Forbidden(field.NewPath("spec", "rabbitmqClusterReference"), detailMsg)) } diff --git a/api/v1beta1/binding_webhook_test.go b/api/v1beta1/binding_webhook_test.go index d81104c9..ee503502 100644 --- a/api/v1beta1/binding_webhook_test.go +++ b/api/v1beta1/binding_webhook_test.go @@ -3,6 +3,7 @@ package v1beta1 import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -25,47 +26,83 @@ var _ = Describe("Binding webhook", func() { }, } - It("does not allow updates on vhost", func() { - newBinding := oldBinding.DeepCopy() - newBinding.Spec.Vhost = "/new-vhost" - Expect(apierrors.IsForbidden(newBinding.ValidateUpdate(&oldBinding))).To(BeTrue()) - }) + Context("ValidateCreate", func() { + It("does not allow both spec.rabbitmqClusterReference.name and spec.rabbitmqClusterReference.connectionSecret be configured", func() { + notAllowed := oldBinding.DeepCopy() + notAllowed.Spec.RabbitmqClusterReference.ConnectionSecret = &corev1.LocalObjectReference{Name: "some-secret"} + Expect(apierrors.IsForbidden(notAllowed.ValidateCreate())).To(BeTrue()) + }) - It("does not allow updates on RabbitmqClusterReference", func() { - newBinding := oldBinding.DeepCopy() - newBinding.Spec.RabbitmqClusterReference = RabbitmqClusterReference{ - Name: "new-cluster", - } - Expect(apierrors.IsForbidden(newBinding.ValidateUpdate(&oldBinding))).To(BeTrue()) + It("spec.rabbitmqClusterReference.name and spec.rabbitmqClusterReference.connectionSecret cannot both be empty", func() { + notAllowed := oldBinding.DeepCopy() + notAllowed.Spec.RabbitmqClusterReference.Name = "" + notAllowed.Spec.RabbitmqClusterReference.ConnectionSecret = nil + Expect(apierrors.IsForbidden(notAllowed.ValidateCreate())).To(BeTrue()) + }) }) - It("does not allow updates on source", func() { - newBinding := oldBinding.DeepCopy() - newBinding.Spec.Source = "updated-source" - Expect(apierrors.IsInvalid(newBinding.ValidateUpdate(&oldBinding))).To(BeTrue()) - }) + Context("ValidateUpdate", func() { + It("does not allow updates on vhost", func() { + newBinding := oldBinding.DeepCopy() + newBinding.Spec.Vhost = "/new-vhost" + Expect(apierrors.IsForbidden(newBinding.ValidateUpdate(&oldBinding))).To(BeTrue()) + }) - It("does not allow updates on destination", func() { - newBinding := oldBinding.DeepCopy() - newBinding.Spec.Destination = "updated-des" - Expect(apierrors.IsInvalid(newBinding.ValidateUpdate(&oldBinding))).To(BeTrue()) - }) + It("does not allow updates on RabbitmqClusterReference", func() { + newBinding := oldBinding.DeepCopy() + newBinding.Spec.RabbitmqClusterReference = RabbitmqClusterReference{ + Name: "new-cluster", + } + Expect(apierrors.IsForbidden(newBinding.ValidateUpdate(&oldBinding))).To(BeTrue()) + }) - It("does not allow updates on destination type", func() { - newBinding := oldBinding.DeepCopy() - newBinding.Spec.DestinationType = "exchange" - Expect(apierrors.IsInvalid(newBinding.ValidateUpdate(&oldBinding))).To(BeTrue()) - }) + It("does not allow updates on rabbitmqClusterReference.connectionSecret", func() { + connectionScr := Binding{ + ObjectMeta: metav1.ObjectMeta{ + Name: "connect-test-queue", + }, + Spec: BindingSpec{ + RabbitmqClusterReference: RabbitmqClusterReference{ + ConnectionSecret: &corev1.LocalObjectReference{ + Name: "a-secret", + }, + }, + }, + } + new := connectionScr.DeepCopy() + new.Spec.RabbitmqClusterReference.ConnectionSecret.Name = "new-secret" + Expect(apierrors.IsForbidden(new.ValidateUpdate(&connectionScr))).To(BeTrue()) + }) - It("does not allow updates on routing key", func() { - newBinding := oldBinding.DeepCopy() - newBinding.Spec.RoutingKey = "not-allowed" - Expect(apierrors.IsInvalid(newBinding.ValidateUpdate(&oldBinding))).To(BeTrue()) - }) + It("does not allow updates on source", func() { + newBinding := oldBinding.DeepCopy() + newBinding.Spec.Source = "updated-source" + Expect(apierrors.IsInvalid(newBinding.ValidateUpdate(&oldBinding))).To(BeTrue()) + }) - It("does not allow updates on binding arguments", func() { - newBinding := oldBinding.DeepCopy() - newBinding.Spec.Arguments = &runtime.RawExtension{Raw: []byte(`{"new":"new-value"}`)} - Expect(apierrors.IsInvalid(newBinding.ValidateUpdate(&oldBinding))).To(BeTrue()) + It("does not allow updates on destination", func() { + newBinding := oldBinding.DeepCopy() + newBinding.Spec.Destination = "updated-des" + Expect(apierrors.IsInvalid(newBinding.ValidateUpdate(&oldBinding))).To(BeTrue()) + }) + + It("does not allow updates on destination type", func() { + newBinding := oldBinding.DeepCopy() + newBinding.Spec.DestinationType = "exchange" + Expect(apierrors.IsInvalid(newBinding.ValidateUpdate(&oldBinding))).To(BeTrue()) + }) + + It("does not allow updates on routing key", func() { + newBinding := oldBinding.DeepCopy() + newBinding.Spec.RoutingKey = "not-allowed" + Expect(apierrors.IsInvalid(newBinding.ValidateUpdate(&oldBinding))).To(BeTrue()) + }) + + It("does not allow updates on binding arguments", func() { + newBinding := oldBinding.DeepCopy() + newBinding.Spec.Arguments = &runtime.RawExtension{Raw: []byte(`{"new":"new-value"}`)} + Expect(apierrors.IsInvalid(newBinding.ValidateUpdate(&oldBinding))).To(BeTrue()) + }) }) + }) diff --git a/api/v1beta1/exchange_webhook.go b/api/v1beta1/exchange_webhook.go index c07a3e3d..e7618c26 100644 --- a/api/v1beta1/exchange_webhook.go +++ b/api/v1beta1/exchange_webhook.go @@ -19,9 +19,10 @@ func (r *Exchange) SetupWebhookWithManager(mgr ctrl.Manager) error { var _ webhook.Validator = &Exchange{} -// no validation on create +// ValidateCreate implements webhook.Validator so a webhook will be registered for the type +// either rabbitmqClusterReference.name or rabbitmqClusterReference.connectionSecret must be provided but not both func (e *Exchange) ValidateCreate() error { - return nil + return e.Spec.RabbitmqClusterReference.ValidateOnCreate(e.GroupResource(), e.Name) } // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type @@ -46,7 +47,7 @@ func (e *Exchange) ValidateUpdate(old runtime.Object) error { field.Forbidden(field.NewPath("spec", "vhost"), detailMsg)) } - if e.Spec.RabbitmqClusterReference != oldExchange.Spec.RabbitmqClusterReference { + if !oldExchange.Spec.RabbitmqClusterReference.Matches(&e.Spec.RabbitmqClusterReference) { return apierrors.NewForbidden(e.GroupResource(), e.Name, field.Forbidden(field.NewPath("spec", "rabbitmqClusterReference"), detailMsg)) } diff --git a/api/v1beta1/exchange_webhook_test.go b/api/v1beta1/exchange_webhook_test.go index 19ac45a6..8d32f496 100644 --- a/api/v1beta1/exchange_webhook_test.go +++ b/api/v1beta1/exchange_webhook_test.go @@ -3,6 +3,7 @@ package v1beta1 import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -26,47 +27,85 @@ var _ = Describe("exchange webhook", func() { }, } - It("does not allow updates on exchange name", func() { - newExchange := exchange.DeepCopy() - newExchange.Spec.Name = "new-name" - Expect(apierrors.IsForbidden(newExchange.ValidateUpdate(&exchange))).To(BeTrue()) - }) + Context("ValidateCreate", func() { + It("does not allow both spec.rabbitmqClusterReference.name and spec.rabbitmqClusterReference.connectionSecret be configured", func() { + notAllowed := exchange.DeepCopy() + notAllowed.Spec.RabbitmqClusterReference.ConnectionSecret = &corev1.LocalObjectReference{Name: "some-secret"} + Expect(apierrors.IsForbidden(notAllowed.ValidateCreate())).To(BeTrue()) + }) - It("does not allow updates on vhost", func() { - newExchange := exchange.DeepCopy() - newExchange.Spec.Vhost = "/a-new-vhost" - Expect(apierrors.IsForbidden(newExchange.ValidateUpdate(&exchange))).To(BeTrue()) + It("spec.rabbitmqClusterReference.name and spec.rabbitmqClusterReference.connectionSecret cannot both be empty", func() { + notAllowed := exchange.DeepCopy() + notAllowed.Spec.RabbitmqClusterReference.Name = "" + notAllowed.Spec.RabbitmqClusterReference.ConnectionSecret = nil + Expect(apierrors.IsForbidden(notAllowed.ValidateCreate())).To(BeTrue()) + }) }) - It("does not allow updates on RabbitmqClusterReference", func() { - newExchange := exchange.DeepCopy() - newExchange.Spec.RabbitmqClusterReference = RabbitmqClusterReference{ - Name: "new-cluster", - } - Expect(apierrors.IsForbidden(newExchange.ValidateUpdate(&exchange))).To(BeTrue()) - }) + Context("ValidateUpdate", func() { + It("does not allow updates on exchange name", func() { + newExchange := exchange.DeepCopy() + newExchange.Spec.Name = "new-name" + Expect(apierrors.IsForbidden(newExchange.ValidateUpdate(&exchange))).To(BeTrue()) + }) - It("does not allow updates on exchange type", func() { - newExchange := exchange.DeepCopy() - newExchange.Spec.Type = "direct" - Expect(apierrors.IsInvalid(newExchange.ValidateUpdate(&exchange))).To(BeTrue()) - }) + It("does not allow updates on vhost", func() { + newExchange := exchange.DeepCopy() + newExchange.Spec.Vhost = "/a-new-vhost" + Expect(apierrors.IsForbidden(newExchange.ValidateUpdate(&exchange))).To(BeTrue()) + }) - It("does not allow updates on durable", func() { - newExchange := exchange.DeepCopy() - newExchange.Spec.Durable = true - Expect(apierrors.IsInvalid(newExchange.ValidateUpdate(&exchange))).To(BeTrue()) - }) + It("does not allow updates on RabbitmqClusterReference", func() { + newExchange := exchange.DeepCopy() + newExchange.Spec.RabbitmqClusterReference = RabbitmqClusterReference{ + Name: "new-cluster", + } + Expect(apierrors.IsForbidden(newExchange.ValidateUpdate(&exchange))).To(BeTrue()) + }) - It("does not allow updates on autoDelete", func() { - newExchange := exchange.DeepCopy() - newExchange.Spec.AutoDelete = false - Expect(apierrors.IsInvalid(newExchange.ValidateUpdate(&exchange))).To(BeTrue()) - }) + It("does not allow updates on rabbitmqClusterReference.connectionSecret", func() { + connectionScr := Exchange{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-exchange", + }, + Spec: ExchangeSpec{ + Name: "test", + Vhost: "/test", + Type: "fanout", + RabbitmqClusterReference: RabbitmqClusterReference{ + ConnectionSecret: &corev1.LocalObjectReference{ + Name: "a-secret", + }, + }, + }, + } + new := connectionScr.DeepCopy() + new.Spec.RabbitmqClusterReference.ConnectionSecret.Name = "new-secret" + Expect(apierrors.IsForbidden(new.ValidateUpdate(&connectionScr))).To(BeTrue()) + }) + + It("does not allow updates on exchange type", func() { + newExchange := exchange.DeepCopy() + newExchange.Spec.Type = "direct" + Expect(apierrors.IsInvalid(newExchange.ValidateUpdate(&exchange))).To(BeTrue()) + }) + + It("does not allow updates on durable", func() { + newExchange := exchange.DeepCopy() + newExchange.Spec.Durable = true + Expect(apierrors.IsInvalid(newExchange.ValidateUpdate(&exchange))).To(BeTrue()) + }) + + It("does not allow updates on autoDelete", func() { + newExchange := exchange.DeepCopy() + newExchange.Spec.AutoDelete = false + Expect(apierrors.IsInvalid(newExchange.ValidateUpdate(&exchange))).To(BeTrue()) + }) - It("allows updates on arguments", func() { - newExchange := exchange.DeepCopy() - newExchange.Spec.Arguments = &runtime.RawExtension{Raw: []byte(`{"new":"new-value"}`)} - Expect(newExchange.ValidateUpdate(&exchange)).To(Succeed()) + It("allows updates on arguments", func() { + newExchange := exchange.DeepCopy() + newExchange.Spec.Arguments = &runtime.RawExtension{Raw: []byte(`{"new":"new-value"}`)} + Expect(newExchange.ValidateUpdate(&exchange)).To(Succeed()) + }) }) }) diff --git a/api/v1beta1/federation_webhook.go b/api/v1beta1/federation_webhook.go index de35f8cf..ced3d70c 100644 --- a/api/v1beta1/federation_webhook.go +++ b/api/v1beta1/federation_webhook.go @@ -16,9 +16,10 @@ func (f *Federation) SetupWebhookWithManager(mgr ctrl.Manager) error { // +kubebuilder:webhook:verbs=create;update,path=/validate-rabbitmq-com-v1beta1-federation,mutating=false,failurePolicy=fail,groups=rabbitmq.com,resources=federations,versions=v1beta1,name=vfederation.kb.io,sideEffects=none,admissionReviewVersions=v1 -// no validation for create +// ValidateCreate implements webhook.Validator so a webhook will be registered for the type +// either rabbitmqClusterReference.name or rabbitmqClusterReference.connectionSecret must be provided but not both func (f *Federation) ValidateCreate() error { - return nil + return f.Spec.RabbitmqClusterReference.ValidateOnCreate(f.GroupResource(), f.Name) } // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type @@ -39,7 +40,7 @@ func (f *Federation) ValidateUpdate(old runtime.Object) error { field.Forbidden(field.NewPath("spec", "vhost"), detailMsg)) } - if f.Spec.RabbitmqClusterReference != oldFederation.Spec.RabbitmqClusterReference { + if !oldFederation.Spec.RabbitmqClusterReference.Matches(&f.Spec.RabbitmqClusterReference) { return apierrors.NewForbidden(f.GroupResource(), f.Name, field.Forbidden(field.NewPath("spec", "rabbitmqClusterReference"), detailMsg)) } diff --git a/api/v1beta1/federation_webhook_test.go b/api/v1beta1/federation_webhook_test.go index c9fc4a3e..161ce48a 100644 --- a/api/v1beta1/federation_webhook_test.go +++ b/api/v1beta1/federation_webhook_test.go @@ -32,78 +32,117 @@ var _ = Describe("federation webhook", func() { }, }, } + Context("ValidateCreate", func() { + It("does not allow both spec.rabbitmqClusterReference.name and spec.rabbitmqClusterReference.connectionSecret be configured", func() { + notAllowed := federation.DeepCopy() + notAllowed.Spec.RabbitmqClusterReference.ConnectionSecret = &corev1.LocalObjectReference{Name: "some-secret"} + Expect(apierrors.IsForbidden(notAllowed.ValidateCreate())).To(BeTrue()) + }) - It("does not allow updates on name", func() { - newFederation := federation.DeepCopy() - newFederation.Spec.Name = "new-upstream" - Expect(apierrors.IsForbidden(newFederation.ValidateUpdate(&federation))).To(BeTrue()) + It("spec.rabbitmqClusterReference.name and spec.rabbitmqClusterReference.connectionSecret cannot both be empty", func() { + notAllowed := federation.DeepCopy() + notAllowed.Spec.RabbitmqClusterReference.Name = "" + notAllowed.Spec.RabbitmqClusterReference.ConnectionSecret = nil + Expect(apierrors.IsForbidden(notAllowed.ValidateCreate())).To(BeTrue()) + }) }) - It("does not allow updates on vhost", func() { - newFederation := federation.DeepCopy() - newFederation.Spec.Vhost = "new-vhost" - Expect(apierrors.IsForbidden(newFederation.ValidateUpdate(&federation))).To(BeTrue()) - }) + Context("ValidateUpdate", func() { + It("does not allow updates on name", func() { + newFederation := federation.DeepCopy() + newFederation.Spec.Name = "new-upstream" + Expect(apierrors.IsForbidden(newFederation.ValidateUpdate(&federation))).To(BeTrue()) + }) - It("does not allow updates on RabbitmqClusterReference", func() { - newFederation := federation.DeepCopy() - newFederation.Spec.RabbitmqClusterReference = RabbitmqClusterReference{ - Name: "new-cluster", - } - Expect(apierrors.IsForbidden(newFederation.ValidateUpdate(&federation))).To(BeTrue()) - }) + It("does not allow updates on vhost", func() { + newFederation := federation.DeepCopy() + newFederation.Spec.Vhost = "new-vhost" + Expect(apierrors.IsForbidden(newFederation.ValidateUpdate(&federation))).To(BeTrue()) + }) - It("allows updates on federation.spec.uriSecret", func() { - newFederation := federation.DeepCopy() - newFederation.Spec.UriSecret = &corev1.LocalObjectReference{Name: "a-new-secret"} - Expect(newFederation.ValidateUpdate(&federation)).To(Succeed()) - }) + It("does not allow updates on RabbitmqClusterReference", func() { + newFederation := federation.DeepCopy() + newFederation.Spec.RabbitmqClusterReference = RabbitmqClusterReference{ + Name: "new-cluster", + } + Expect(apierrors.IsForbidden(newFederation.ValidateUpdate(&federation))).To(BeTrue()) + }) - It("allows updates on federation.spec.expires", func() { - newFederation := federation.DeepCopy() - newFederation.Spec.Expires = 10 - Expect(newFederation.ValidateUpdate(&federation)).To(Succeed()) - }) + It("does not allow updates on rabbitmqClusterReference.connectionSecret", func() { + connectionScr := Federation{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + Spec: FederationSpec{ + Name: "test-upstream", + Vhost: "/a-vhost", + UriSecret: &corev1.LocalObjectReference{ + Name: "a-secret", + }, + RabbitmqClusterReference: RabbitmqClusterReference{ + ConnectionSecret: &corev1.LocalObjectReference{ + Name: "a-secret", + }, + }, + }, + } + new := connectionScr.DeepCopy() + new.Spec.RabbitmqClusterReference.ConnectionSecret.Name = "new-secret" + Expect(apierrors.IsForbidden(new.ValidateUpdate(&connectionScr))).To(BeTrue()) + }) - It("allows updates on federation.spec.messageTTL", func() { - newFederation := federation.DeepCopy() - newFederation.Spec.MessageTTL = 10 - Expect(newFederation.ValidateUpdate(&federation)).To(Succeed()) - }) + It("allows updates on federation.spec.uriSecret", func() { + newFederation := federation.DeepCopy() + newFederation.Spec.UriSecret = &corev1.LocalObjectReference{Name: "a-new-secret"} + Expect(newFederation.ValidateUpdate(&federation)).To(Succeed()) + }) - It("allows updates on federation.spec.maxHops", func() { - newFederation := federation.DeepCopy() - newFederation.Spec.MaxHops = 10 - Expect(newFederation.ValidateUpdate(&federation)).To(Succeed()) - }) + It("allows updates on federation.spec.expires", func() { + newFederation := federation.DeepCopy() + newFederation.Spec.Expires = 10 + Expect(newFederation.ValidateUpdate(&federation)).To(Succeed()) + }) - It("allows updates on federation.spec.prefetchCount", func() { - newFederation := federation.DeepCopy() - newFederation.Spec.PrefetchCount = 10 - Expect(newFederation.ValidateUpdate(&federation)).To(Succeed()) - }) + It("allows updates on federation.spec.messageTTL", func() { + newFederation := federation.DeepCopy() + newFederation.Spec.MessageTTL = 10 + Expect(newFederation.ValidateUpdate(&federation)).To(Succeed()) + }) - It("allows updates on federation.spec.reconnectDelay", func() { - newFederation := federation.DeepCopy() - newFederation.Spec.PrefetchCount = 10000 - Expect(newFederation.ValidateUpdate(&federation)).To(Succeed()) - }) + It("allows updates on federation.spec.maxHops", func() { + newFederation := federation.DeepCopy() + newFederation.Spec.MaxHops = 10 + Expect(newFederation.ValidateUpdate(&federation)).To(Succeed()) + }) - It("allows updates on federation.spec.trustUserId", func() { - newFederation := federation.DeepCopy() - newFederation.Spec.TrustUserId = false - Expect(newFederation.ValidateUpdate(&federation)).To(Succeed()) - }) + It("allows updates on federation.spec.prefetchCount", func() { + newFederation := federation.DeepCopy() + newFederation.Spec.PrefetchCount = 10 + Expect(newFederation.ValidateUpdate(&federation)).To(Succeed()) + }) - It("allows updates on federation.spec.exchange", func() { - newFederation := federation.DeepCopy() - newFederation.Spec.Exchange = "new-exchange" - Expect(newFederation.ValidateUpdate(&federation)).To(Succeed()) - }) + It("allows updates on federation.spec.reconnectDelay", func() { + newFederation := federation.DeepCopy() + newFederation.Spec.PrefetchCount = 10000 + Expect(newFederation.ValidateUpdate(&federation)).To(Succeed()) + }) + + It("allows updates on federation.spec.trustUserId", func() { + newFederation := federation.DeepCopy() + newFederation.Spec.TrustUserId = false + Expect(newFederation.ValidateUpdate(&federation)).To(Succeed()) + }) + + It("allows updates on federation.spec.exchange", func() { + newFederation := federation.DeepCopy() + newFederation.Spec.Exchange = "new-exchange" + Expect(newFederation.ValidateUpdate(&federation)).To(Succeed()) + }) - It("allows updates on federation.spec.ackMode", func() { - newFederation := federation.DeepCopy() - newFederation.Spec.AckMode = "no-ack" - Expect(newFederation.ValidateUpdate(&federation)).To(Succeed()) + It("allows updates on federation.spec.ackMode", func() { + newFederation := federation.DeepCopy() + newFederation.Spec.AckMode = "no-ack" + Expect(newFederation.ValidateUpdate(&federation)).To(Succeed()) + }) }) }) diff --git a/api/v1beta1/permission_webhook.go b/api/v1beta1/permission_webhook.go index 2cbf6a0c..9df03b28 100644 --- a/api/v1beta1/permission_webhook.go +++ b/api/v1beta1/permission_webhook.go @@ -21,6 +21,7 @@ func (p *Permission) SetupWebhookWithManager(mgr ctrl.Manager) error { var _ webhook.Validator = &Permission{} // ValidateCreate checks if only one of spec.user and spec.userReference is specified +// either rabbitmqClusterReference.name or rabbitmqClusterReference.connectionSecret must be provided but not both func (p *Permission) ValidateCreate() error { var errorList field.ErrorList if p.Spec.User == "" && p.Spec.UserReference == nil { @@ -35,7 +36,7 @@ func (p *Permission) ValidateCreate() error { return apierrors.NewInvalid(GroupVersion.WithKind("Permission").GroupKind(), p.Name, errorList) } - return nil + return p.Spec.RabbitmqClusterReference.ValidateOnCreate(p.GroupResource(), p.Name) } // ValidateUpdate do not allow updates on spec.vhost, spec.user, spec.userReference, and spec.rabbitmqClusterReference @@ -76,7 +77,7 @@ func (p *Permission) ValidateUpdate(old runtime.Object) error { field.Forbidden(field.NewPath("spec", "vhost"), detailMsg)) } - if p.Spec.RabbitmqClusterReference != oldPermission.Spec.RabbitmqClusterReference { + if !oldPermission.Spec.RabbitmqClusterReference.Matches(&p.Spec.RabbitmqClusterReference) { return apierrors.NewForbidden(p.GroupResource(), p.Name, field.Forbidden(field.NewPath("spec", "rabbitmqClusterReference"), detailMsg)) } diff --git a/api/v1beta1/permission_webhook_test.go b/api/v1beta1/permission_webhook_test.go index 9c25bda9..1369b0d5 100644 --- a/api/v1beta1/permission_webhook_test.go +++ b/api/v1beta1/permission_webhook_test.go @@ -27,6 +27,34 @@ var _ = Describe("permission webhook", func() { }, } + Context("ValidateCreate", func() { + It("does not allow user and userReference to be specified at the same time", func() { + invalidPermission := permission.DeepCopy() + invalidPermission.Spec.UserReference = &corev1.LocalObjectReference{Name: "invalid"} + invalidPermission.Spec.User = "test-user" + Expect(apierrors.IsInvalid(invalidPermission.ValidateCreate())).To(BeTrue()) + }) + It("does not allow both user and userReference to be unset", func() { + invalidPermission := permission.DeepCopy() + invalidPermission.Spec.UserReference = nil + invalidPermission.Spec.User = "" + Expect(apierrors.IsInvalid(invalidPermission.ValidateCreate())).To(BeTrue()) + }) + + It("does not allow both spec.rabbitmqClusterReference.name and spec.rabbitmqClusterReference.connectionSecret be configured", func() { + notAllowed := permission.DeepCopy() + notAllowed.Spec.RabbitmqClusterReference.ConnectionSecret = &corev1.LocalObjectReference{Name: "some-secret"} + Expect(apierrors.IsForbidden(notAllowed.ValidateCreate())).To(BeTrue()) + }) + + It("spec.rabbitmqClusterReference.name and spec.rabbitmqClusterReference.connectionSecret cannot both be empty", func() { + notAllowed := permission.DeepCopy() + notAllowed.Spec.RabbitmqClusterReference.Name = "" + notAllowed.Spec.RabbitmqClusterReference.ConnectionSecret = nil + Expect(apierrors.IsForbidden(notAllowed.ValidateCreate())).To(BeTrue()) + }) + }) + Context("ValidateUpdate", func() { It("does not allow updates on user", func() { newPermission := permission.DeepCopy() @@ -57,6 +85,31 @@ var _ = Describe("permission webhook", func() { Expect(apierrors.IsForbidden(newPermission.ValidateUpdate(&permission))).To(BeTrue()) }) + It("does not allow updates on rabbitmqClusterReference.connectionSecret", func() { + connectionScr := Permission{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + Spec: PermissionSpec{ + User: "test-user", + Vhost: "/a-vhost", + Permissions: VhostPermissions{ + Configure: ".*", + Read: ".*", + Write: ".*", + }, + RabbitmqClusterReference: RabbitmqClusterReference{ + ConnectionSecret: &corev1.LocalObjectReference{ + Name: "a-secret", + }, + }, + }, + } + new := connectionScr.DeepCopy() + new.Spec.RabbitmqClusterReference.ConnectionSecret.Name = "new-secret" + Expect(apierrors.IsForbidden(new.ValidateUpdate(&connectionScr))).To(BeTrue()) + }) + It("allows updates on permission.spec.permissions.configure", func() { newPermission := permission.DeepCopy() newPermission.Spec.Permissions.Configure = "?" @@ -88,19 +141,4 @@ var _ = Describe("permission webhook", func() { Expect(apierrors.IsInvalid(newPermission.ValidateUpdate(&permission))).To(BeTrue()) }) }) - - Context("ValidateCreate", func() { - It("does not allow user and userReference to be specified at the same time", func() { - invalidPermission := permission.DeepCopy() - invalidPermission.Spec.UserReference = &corev1.LocalObjectReference{Name: "invalid"} - invalidPermission.Spec.User = "test-user" - Expect(apierrors.IsInvalid(invalidPermission.ValidateCreate())).To(BeTrue()) - }) - It("does not allow both user and userReference to be unset", func() { - invalidPermission := permission.DeepCopy() - invalidPermission.Spec.UserReference = nil - invalidPermission.Spec.User = "" - Expect(apierrors.IsInvalid(invalidPermission.ValidateCreate())).To(BeTrue()) - }) - }) }) diff --git a/api/v1beta1/policy_webhook.go b/api/v1beta1/policy_webhook.go index 865deb35..180c00a2 100644 --- a/api/v1beta1/policy_webhook.go +++ b/api/v1beta1/policy_webhook.go @@ -19,12 +19,13 @@ func (p *Policy) SetupWebhookWithManager(mgr ctrl.Manager) error { var _ webhook.Validator = &Policy{} -// no validation on create +// ValidateCreate implements webhook.Validator so a webhook will be registered for the type +// either rabbitmqClusterReference.name or rabbitmqClusterReference.connectionSecret must be provided but not both func (p *Policy) ValidateCreate() error { - return nil + return p.Spec.RabbitmqClusterReference.ValidateOnCreate(p.GroupResource(), p.Name) } -// returns error type 'forbidden' for updates on policy name, vhost and rabbitmqClusterReference +// ValidateUpdate returns error type 'forbidden' for updates on policy name, vhost and rabbitmqClusterReference func (p *Policy) ValidateUpdate(old runtime.Object) error { oldPolicy, ok := old.(*Policy) if !ok { @@ -42,7 +43,7 @@ func (p *Policy) ValidateUpdate(old runtime.Object) error { field.Forbidden(field.NewPath("spec", "vhost"), detailMsg)) } - if p.Spec.RabbitmqClusterReference != oldPolicy.Spec.RabbitmqClusterReference { + if !oldPolicy.Spec.RabbitmqClusterReference.Matches(&p.Spec.RabbitmqClusterReference) { return apierrors.NewForbidden(p.GroupResource(), p.Name, field.Forbidden(field.NewPath("spec", "rabbitmqClusterReference"), detailMsg)) } diff --git a/api/v1beta1/policy_webhook_test.go b/api/v1beta1/policy_webhook_test.go index c206cb42..db492377 100644 --- a/api/v1beta1/policy_webhook_test.go +++ b/api/v1beta1/policy_webhook_test.go @@ -3,6 +3,7 @@ package v1beta1 import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -25,47 +26,87 @@ var _ = Describe("policy webhook", func() { }, } - It("does not allow updates on policy name", func() { - newPolicy := policy.DeepCopy() - newPolicy.Spec.Name = "new-name" - Expect(apierrors.IsForbidden(newPolicy.ValidateUpdate(&policy))).To(BeTrue()) - }) + Context("ValidateCreate", func() { + It("does not allow both spec.rabbitmqClusterReference.name and spec.rabbitmqClusterReference.connectionSecret be configured", func() { + notAllowed := policy.DeepCopy() + notAllowed.Spec.RabbitmqClusterReference.ConnectionSecret = &corev1.LocalObjectReference{Name: "some-secret"} + Expect(apierrors.IsForbidden(notAllowed.ValidateCreate())).To(BeTrue()) + }) - It("does not allow updates on vhost", func() { - newPolicy := policy.DeepCopy() - newPolicy.Spec.Vhost = "new-vhost" - Expect(apierrors.IsForbidden(newPolicy.ValidateUpdate(&policy))).To(BeTrue()) + It("spec.rabbitmqClusterReference.name and spec.rabbitmqClusterReference.connectionSecret cannot both be empty", func() { + notAllowed := policy.DeepCopy() + notAllowed.Spec.RabbitmqClusterReference.Name = "" + notAllowed.Spec.RabbitmqClusterReference.ConnectionSecret = nil + Expect(apierrors.IsForbidden(notAllowed.ValidateCreate())).To(BeTrue()) + }) }) - It("does not allow updates on RabbitmqClusterReference", func() { - newPolicy := policy.DeepCopy() - newPolicy.Spec.RabbitmqClusterReference = RabbitmqClusterReference{ - Name: "new-cluster", - } - Expect(apierrors.IsForbidden(newPolicy.ValidateUpdate(&policy))).To(BeTrue()) - }) + Context("ValidateUpdate", func() { + It("does not allow updates on policy name", func() { + newPolicy := policy.DeepCopy() + newPolicy.Spec.Name = "new-name" + Expect(apierrors.IsForbidden(newPolicy.ValidateUpdate(&policy))).To(BeTrue()) + }) - It("allows updates on policy.spec.pattern", func() { - newPolicy := policy.DeepCopy() - newPolicy.Spec.Pattern = "new-pattern" - Expect(newPolicy.ValidateUpdate(&policy)).To(Succeed()) - }) + It("does not allow updates on vhost", func() { + newPolicy := policy.DeepCopy() + newPolicy.Spec.Vhost = "new-vhost" + Expect(apierrors.IsForbidden(newPolicy.ValidateUpdate(&policy))).To(BeTrue()) + }) - It("allows updates on policy.spec.applyTo", func() { - newPolicy := policy.DeepCopy() - newPolicy.Spec.ApplyTo = "queues" - Expect(newPolicy.ValidateUpdate(&policy)).To(Succeed()) - }) + It("does not allow updates on RabbitmqClusterReference", func() { + newPolicy := policy.DeepCopy() + newPolicy.Spec.RabbitmqClusterReference = RabbitmqClusterReference{ + Name: "new-cluster", + } + Expect(apierrors.IsForbidden(newPolicy.ValidateUpdate(&policy))).To(BeTrue()) + }) - It("allows updates on policy.spec.priority", func() { - newPolicy := policy.DeepCopy() - newPolicy.Spec.Priority = 1000 - Expect(newPolicy.ValidateUpdate(&policy)).To(Succeed()) - }) + It("does not allow updates on rabbitmqClusterReference.connectionSecret", func() { + connectionScr := Policy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + Spec: PolicySpec{ + Name: "test", + Vhost: "/test", + Pattern: "a-pattern", + ApplyTo: "all", + Priority: 0, + RabbitmqClusterReference: RabbitmqClusterReference{ + ConnectionSecret: &corev1.LocalObjectReference{ + Name: "a-secret", + }, + }, + }, + } + new := connectionScr.DeepCopy() + new.Spec.RabbitmqClusterReference.ConnectionSecret.Name = "new-secret" + Expect(apierrors.IsForbidden(new.ValidateUpdate(&connectionScr))).To(BeTrue()) + }) + + It("allows updates on policy.spec.pattern", func() { + newPolicy := policy.DeepCopy() + newPolicy.Spec.Pattern = "new-pattern" + Expect(newPolicy.ValidateUpdate(&policy)).To(Succeed()) + }) + + It("allows updates on policy.spec.applyTo", func() { + newPolicy := policy.DeepCopy() + newPolicy.Spec.ApplyTo = "queues" + Expect(newPolicy.ValidateUpdate(&policy)).To(Succeed()) + }) + + It("allows updates on policy.spec.priority", func() { + newPolicy := policy.DeepCopy() + newPolicy.Spec.Priority = 1000 + Expect(newPolicy.ValidateUpdate(&policy)).To(Succeed()) + }) - It("allows updates on policy.spec.definition", func() { - newPolicy := policy.DeepCopy() - newPolicy.Spec.Definition = &runtime.RawExtension{Raw: []byte(`{"key":"new-definition-value"}`)} - Expect(newPolicy.ValidateUpdate(&policy)).To(Succeed()) + It("allows updates on policy.spec.definition", func() { + newPolicy := policy.DeepCopy() + newPolicy.Spec.Definition = &runtime.RawExtension{Raw: []byte(`{"key":"new-definition-value"}`)} + Expect(newPolicy.ValidateUpdate(&policy)).To(Succeed()) + }) }) }) diff --git a/api/v1beta1/queue_types.go b/api/v1beta1/queue_types.go index 81532857..2ad09b68 100644 --- a/api/v1beta1/queue_types.go +++ b/api/v1beta1/queue_types.go @@ -43,16 +43,6 @@ type QueueSpec struct { RabbitmqClusterReference RabbitmqClusterReference `json:"rabbitmqClusterReference"` } -type RabbitmqClusterReference struct { - // The name of the RabbitMQ cluster to reference. - // +kubebuilder:validation:Required - Name string `json:"name"` - // The namespace of the RabbitMQ cluster to reference. - // Defaults to the namespace of the requested resource if omitted. - // +kubebuilder:validation:Optional - Namespace string `json:"namespace"` -} - // QueueStatus defines the observed state of Queue type QueueStatus struct { // observedGeneration is the most recent successful generation observed for this Queue. It corresponds to the diff --git a/api/v1beta1/queue_webhook.go b/api/v1beta1/queue_webhook.go index 4da07ba0..6aec4915 100644 --- a/api/v1beta1/queue_webhook.go +++ b/api/v1beta1/queue_webhook.go @@ -9,9 +9,9 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook" ) -func (r *Queue) SetupWebhookWithManager(mgr ctrl.Manager) error { +func (q *Queue) SetupWebhookWithManager(mgr ctrl.Manager) error { return ctrl.NewWebhookManagedBy(mgr). - For(r). + For(q). Complete() } @@ -19,9 +19,10 @@ func (r *Queue) SetupWebhookWithManager(mgr ctrl.Manager) error { var _ webhook.Validator = &Queue{} -// no validation on create +// ValidateCreate implements webhook.Validator so a webhook will be registered for the type +// either rabbitmqClusterReference.name or rabbitmqClusterReference.connectionSecret must be provided but not both func (q *Queue) ValidateCreate() error { - return nil + return q.Spec.RabbitmqClusterReference.ValidateOnCreate(q.GroupResource(), q.Name) } // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type @@ -46,7 +47,7 @@ func (q *Queue) ValidateUpdate(old runtime.Object) error { field.Forbidden(field.NewPath("spec", "vhost"), detailMsg)) } - if q.Spec.RabbitmqClusterReference != oldQueue.Spec.RabbitmqClusterReference { + if !oldQueue.Spec.RabbitmqClusterReference.Matches(&q.Spec.RabbitmqClusterReference) { return apierrors.NewForbidden(q.GroupResource(), q.Name, field.Forbidden(field.NewPath("spec", "rabbitmqClusterReference"), detailMsg)) } diff --git a/api/v1beta1/queue_webhook_test.go b/api/v1beta1/queue_webhook_test.go index ca58b7b8..d22e75c6 100644 --- a/api/v1beta1/queue_webhook_test.go +++ b/api/v1beta1/queue_webhook_test.go @@ -3,6 +3,7 @@ package v1beta1 import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -11,7 +12,7 @@ var _ = Describe("queue webhook", func() { var queue = Queue{ ObjectMeta: metav1.ObjectMeta{ - Name: "update-binding", + Name: "test-queue", }, Spec: QueueSpec{ Name: "test", @@ -25,41 +26,85 @@ var _ = Describe("queue webhook", func() { }, } - It("does not allow updates on queue name", func() { - newQueue := queue.DeepCopy() - newQueue.Spec.Name = "new-name" - Expect(apierrors.IsForbidden(newQueue.ValidateUpdate(&queue))).To(BeTrue()) - }) + Context("ValidateCreate", func() { + It("does not allow both spec.rabbitmqClusterReference.name and spec.rabbitmqClusterReference.connectionSecret be configured", func() { + notAllowedQ := queue.DeepCopy() + notAllowedQ.Spec.RabbitmqClusterReference.ConnectionSecret = &corev1.LocalObjectReference{Name: "some-secret"} + Expect(apierrors.IsForbidden(notAllowedQ.ValidateCreate())).To(BeTrue()) + }) - It("does not allow updates on vhost", func() { - newQueue := queue.DeepCopy() - newQueue.Spec.Vhost = "/new-vhost" - Expect(apierrors.IsForbidden(newQueue.ValidateUpdate(&queue))).To(BeTrue()) + It("spec.rabbitmqClusterReference.name and spec.rabbitmqClusterReference.connectionSecret cannot both be empty", func() { + notAllowedQ := queue.DeepCopy() + notAllowedQ.Spec.RabbitmqClusterReference.Name = "" + notAllowedQ.Spec.RabbitmqClusterReference.ConnectionSecret = nil + Expect(apierrors.IsForbidden(notAllowedQ.ValidateCreate())).To(BeTrue()) + }) }) - It("does not allow updates on RabbitmqClusterReference", func() { - newQueue := queue.DeepCopy() - newQueue.Spec.RabbitmqClusterReference = RabbitmqClusterReference{ - Name: "new-cluster", - } - Expect(apierrors.IsForbidden(newQueue.ValidateUpdate(&queue))).To(BeTrue()) - }) + Context("ValidateUpdate", func() { + It("does not allow updates on queue name", func() { + newQueue := queue.DeepCopy() + newQueue.Spec.Name = "new-name" + Expect(apierrors.IsForbidden(newQueue.ValidateUpdate(&queue))).To(BeTrue()) + }) - It("does not allow updates on queue type", func() { - newQueue := queue.DeepCopy() - newQueue.Spec.Type = "classic" - Expect(apierrors.IsInvalid(newQueue.ValidateUpdate(&queue))).To(BeTrue()) - }) + It("does not allow updates on vhost", func() { + newQueue := queue.DeepCopy() + newQueue.Spec.Vhost = "/new-vhost" + Expect(apierrors.IsForbidden(newQueue.ValidateUpdate(&queue))).To(BeTrue()) + }) - It("does not allow updates on durable", func() { - newQueue := queue.DeepCopy() - newQueue.Spec.Durable = true - Expect(apierrors.IsInvalid(newQueue.ValidateUpdate(&queue))).To(BeTrue()) - }) + It("does not allow updates on rabbitmqClusterReference.name", func() { + newQueue := queue.DeepCopy() + newQueue.Spec.RabbitmqClusterReference = RabbitmqClusterReference{ + Name: "new-cluster", + } + Expect(apierrors.IsForbidden(newQueue.ValidateUpdate(&queue))).To(BeTrue()) + }) + + It("does not allow updates on rabbitmqClusterReference.namespace", func() { + newQueue := queue.DeepCopy() + newQueue.Spec.RabbitmqClusterReference = RabbitmqClusterReference{ + Namespace: "new-ns", + } + Expect(apierrors.IsForbidden(newQueue.ValidateUpdate(&queue))).To(BeTrue()) + }) + + It("does not allow updates on rabbitmqClusterReference.connectionSecret", func() { + connectionScrQ := Queue{ + ObjectMeta: metav1.ObjectMeta{ + Name: "connect-test-queue", + }, + Spec: QueueSpec{ + Name: "test", + RabbitmqClusterReference: RabbitmqClusterReference{ + ConnectionSecret: &corev1.LocalObjectReference{ + Name: "a-secret", + }, + }, + }, + } + newQueue := connectionScrQ.DeepCopy() + newQueue.Spec.RabbitmqClusterReference.ConnectionSecret.Name = "new-secret" + Expect(apierrors.IsForbidden(newQueue.ValidateUpdate(&connectionScrQ))).To(BeTrue()) + }) + + It("does not allow updates on queue type", func() { + newQueue := queue.DeepCopy() + newQueue.Spec.Type = "classic" + Expect(apierrors.IsInvalid(newQueue.ValidateUpdate(&queue))).To(BeTrue()) + }) + + It("does not allow updates on durable", func() { + newQueue := queue.DeepCopy() + newQueue.Spec.Durable = true + Expect(apierrors.IsInvalid(newQueue.ValidateUpdate(&queue))).To(BeTrue()) + }) - It("does not allow updates on autoDelete", func() { - newQueue := queue.DeepCopy() - newQueue.Spec.AutoDelete = false - Expect(apierrors.IsInvalid(newQueue.ValidateUpdate(&queue))).To(BeTrue()) + It("does not allow updates on autoDelete", func() { + newQueue := queue.DeepCopy() + newQueue.Spec.AutoDelete = false + Expect(apierrors.IsInvalid(newQueue.ValidateUpdate(&queue))).To(BeTrue()) + }) }) }) diff --git a/api/v1beta1/rabbitmq_cluster_reference.go b/api/v1beta1/rabbitmq_cluster_reference.go new file mode 100644 index 00000000..129b4070 --- /dev/null +++ b/api/v1beta1/rabbitmq_cluster_reference.go @@ -0,0 +1,64 @@ +package v1beta1 + +import ( + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/validation/field" +) + +type RabbitmqClusterReference struct { + // The name of the RabbitMQ cluster to reference. + // Have to set either name or connectionSecret, but not both. + // +kubebuilder:validation:Optional + Name string `json:"name,omitempty"` + // The namespace of the RabbitMQ cluster to reference. + // Defaults to the namespace of the requested resource if omitted. + // +kubebuilder:validation:Optional + Namespace string `json:"namespace,omitempty"` + // Secret contains the http management uri for the RabbitMQ cluster. + // The Secret must contain the key `uri`, `username` and `password` or operator will error. + // Have to set either name or connectionSecret, but not both. + // +kubebuilder:validation:Optional + ConnectionSecret *corev1.LocalObjectReference `json:"connectionSecret,omitempty"` +} + +func (r *RabbitmqClusterReference) Matches(new *RabbitmqClusterReference) bool { + if new.Name != r.Name || new.Namespace != r.Namespace { + return false + } + + // when connectionSecret has been updated + if new.ConnectionSecret != nil && r.ConnectionSecret != nil && *new.ConnectionSecret != *r.ConnectionSecret { + return false + } + + // when connectionSecret is removed + if new.ConnectionSecret == nil && r.ConnectionSecret != nil { + return false + } + + // when connectionSecret is added + if new.ConnectionSecret != nil && r.ConnectionSecret == nil { + return false + } + + return true +} + +// ValidateOnCreate validates RabbitmqClusterReference on resources create +// either rabbitmqClusterReference.name or rabbitmqClusterReference.connectionSecret must be provided but not both; else it errors +func (ref *RabbitmqClusterReference) ValidateOnCreate(groupResource schema.GroupResource, name string) error { + if ref.Name != "" && ref.ConnectionSecret != nil { + return apierrors.NewForbidden(groupResource, name, + field.Forbidden(field.NewPath("spec", "rabbitmqClusterReference"), + "do not provide both spec.rabbitmqClusterReference.name and spec.rabbitmqClusterReference.connectionSecret")) + } + + if ref.Name == "" && ref.ConnectionSecret == nil { + return apierrors.NewForbidden(groupResource, name, + field.Forbidden(field.NewPath("spec", "rabbitmqClusterReference"), + "must provide either spec.rabbitmqClusterReference.name or spec.rabbitmqClusterReference.connectionSecret")) + } + return nil +} diff --git a/api/v1beta1/rabbitmq_cluster_reference_test.go b/api/v1beta1/rabbitmq_cluster_reference_test.go new file mode 100644 index 00000000..b08a2b5c --- /dev/null +++ b/api/v1beta1/rabbitmq_cluster_reference_test.go @@ -0,0 +1,110 @@ +package v1beta1 + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + v1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +var _ = Describe("RabbitmqClusterReference", func() { + var reference *RabbitmqClusterReference + + BeforeEach(func() { + reference = &RabbitmqClusterReference{ + Name: "a-name", + Namespace: "a-ns", + ConnectionSecret: &v1.LocalObjectReference{ + Name: "a-secret-name", + }, + } + }) + + Context("Matches", func() { + When("name is different", func() { + It("returns false", func() { + new := reference.DeepCopy() + new.Name = "new-name" + Expect(reference.Matches(new)).To(BeFalse()) + }) + }) + + When("namespace is different", func() { + It("returns false", func() { + new := reference.DeepCopy() + new.Namespace = "new-ns" + Expect(reference.Matches(new)).To(BeFalse()) + }) + }) + + When("connectionSecret.name is different", func() { + It("returns false", func() { + new := reference.DeepCopy() + new.ConnectionSecret.Name = "new-secret-name" + Expect(reference.Matches(new)).To(BeFalse()) + }) + }) + + When("connectionSecret is removed", func() { + It("returns false", func() { + new := reference.DeepCopy() + new.ConnectionSecret = nil + Expect(reference.Matches(new)).To(BeFalse()) + }) + }) + + When("connectionSecret is added", func() { + It("returns false", func() { + reference.ConnectionSecret = nil + new := reference.DeepCopy() + new.ConnectionSecret = &v1.LocalObjectReference{ + Name: "a-secret-name", + } + Expect(reference.Matches(new)).To(BeFalse()) + }) + }) + + When("RabbitmqClusterReference stayed the same", func() { + It("returns true", func() { + new := reference.DeepCopy() + Expect(reference.Matches(new)).To(BeTrue()) + }) + }) + }) + + Context("ValidateOnCreate", func() { + When("name is provided", func() { + It("returns no error", func() { + reference.ConnectionSecret = nil + reference.Name = "a-name" + Expect(reference.ValidateOnCreate(schema.GroupResource{}, "a-resource")).To(Succeed()) + }) + }) + + When("connectionSecret is provided", func() { + It("returns no error", func() { + reference.ConnectionSecret = &v1.LocalObjectReference{Name: "a-secret-name"} + reference.Name = "" + Expect(reference.ValidateOnCreate(schema.GroupResource{}, "a-resource")).To(Succeed()) + }) + }) + + When("name and connectionSecrets are both provided", func() { + It("returns a forbidden api error", func() { + reference.Name = "a-cluster" + reference.ConnectionSecret = &v1.LocalObjectReference{Name: "a-secret-name"} + Expect(apierrors.IsForbidden(reference.ValidateOnCreate(schema.GroupResource{}, "a-resource"))).To(BeTrue()) + }) + }) + + When("name and connectionSecrets are both empty", func() { + It("returns a forbidden api error", func() { + reference.ConnectionSecret = nil + reference.Name = "" + Expect(apierrors.IsForbidden(reference.ValidateOnCreate(schema.GroupResource{}, "a-resource"))).To(BeTrue()) + }) + }) + }) + +}) diff --git a/api/v1beta1/schemareplication_webhook.go b/api/v1beta1/schemareplication_webhook.go index a9073575..26c7e2d6 100644 --- a/api/v1beta1/schemareplication_webhook.go +++ b/api/v1beta1/schemareplication_webhook.go @@ -19,9 +19,10 @@ func (s *SchemaReplication) SetupWebhookWithManager(mgr ctrl.Manager) error { var _ webhook.Validator = &SchemaReplication{} -// no validation on create +// ValidateCreate implements webhook.Validator so a webhook will be registered for the type +// either rabbitmqClusterReference.name or rabbitmqClusterReference.connectionSecret must be provided but not both func (s *SchemaReplication) ValidateCreate() error { - return nil + return s.Spec.RabbitmqClusterReference.ValidateOnCreate(s.GroupResource(), s.Name) } // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type @@ -31,7 +32,7 @@ func (s *SchemaReplication) ValidateUpdate(old runtime.Object) error { return apierrors.NewBadRequest(fmt.Sprintf("expected a schema replication type but got a %T", old)) } - if s.Spec.RabbitmqClusterReference != oldReplication.Spec.RabbitmqClusterReference { + if !oldReplication.Spec.RabbitmqClusterReference.Matches(&s.Spec.RabbitmqClusterReference) { return apierrors.NewForbidden(s.GroupResource(), s.Name, field.Forbidden(field.NewPath("spec", "rabbitmqClusterReference"), "update on rabbitmqClusterReference is forbidden")) } diff --git a/api/v1beta1/schemareplication_webhook_test.go b/api/v1beta1/schemareplication_webhook_test.go index 7781fe65..be0ee35e 100644 --- a/api/v1beta1/schemareplication_webhook_test.go +++ b/api/v1beta1/schemareplication_webhook_test.go @@ -24,25 +24,64 @@ var _ = Describe("schema-replication webhook", func() { }, } - It("does not allow updates on RabbitmqClusterReference", func() { - updated := replication.DeepCopy() - updated.Spec.RabbitmqClusterReference = RabbitmqClusterReference{ - Name: "different-cluster", - } - Expect(apierrors.IsForbidden(updated.ValidateUpdate(&replication))).To(BeTrue()) - }) + Context("ValidateCreate", func() { + It("does not allow both spec.rabbitmqClusterReference.name and spec.rabbitmqClusterReference.connectionSecret be configured", func() { + notAllowed := replication.DeepCopy() + notAllowed.Spec.RabbitmqClusterReference.ConnectionSecret = &corev1.LocalObjectReference{Name: "some-secret"} + Expect(apierrors.IsForbidden(notAllowed.ValidateCreate())).To(BeTrue()) + }) - It("allows updates on spec.upstreamSecret", func() { - updated := replication.DeepCopy() - updated.Spec.UpstreamSecret = &corev1.LocalObjectReference{ - Name: "a-different-secret", - } - Expect(updated.ValidateUpdate(&replication)).To(Succeed()) + It("spec.rabbitmqClusterReference.name and spec.rabbitmqClusterReference.connectionSecret cannot both be empty", func() { + notAllowed := replication.DeepCopy() + notAllowed.Spec.RabbitmqClusterReference.Name = "" + notAllowed.Spec.RabbitmqClusterReference.ConnectionSecret = nil + Expect(apierrors.IsForbidden(notAllowed.ValidateCreate())).To(BeTrue()) + }) }) - It("allows updates on spec.endpoints", func() { - updated := replication.DeepCopy() - updated.Spec.Endpoints = "abc.new-rmq:1111" - Expect(updated.ValidateUpdate(&replication)).To(Succeed()) + Context("ValidateUpdate", func() { + It("does not allow updates on RabbitmqClusterReference", func() { + updated := replication.DeepCopy() + updated.Spec.RabbitmqClusterReference = RabbitmqClusterReference{ + Name: "different-cluster", + } + Expect(apierrors.IsForbidden(updated.ValidateUpdate(&replication))).To(BeTrue()) + }) + + It("does not allow updates on rabbitmqClusterReference.connectionSecret", func() { + connectionScr := SchemaReplication{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-replication", + }, + Spec: SchemaReplicationSpec{ + UpstreamSecret: &corev1.LocalObjectReference{ + Name: "a-secret", + }, + Endpoints: "abc.rmq.com:1234", + RabbitmqClusterReference: RabbitmqClusterReference{ + ConnectionSecret: &corev1.LocalObjectReference{ + Name: "a-secret", + }, + }, + }, + } + new := connectionScr.DeepCopy() + new.Spec.RabbitmqClusterReference.ConnectionSecret.Name = "new-secret" + Expect(apierrors.IsForbidden(new.ValidateUpdate(&connectionScr))).To(BeTrue()) + }) + + It("allows updates on spec.upstreamSecret", func() { + updated := replication.DeepCopy() + updated.Spec.UpstreamSecret = &corev1.LocalObjectReference{ + Name: "a-different-secret", + } + Expect(updated.ValidateUpdate(&replication)).To(Succeed()) + }) + + It("allows updates on spec.endpoints", func() { + updated := replication.DeepCopy() + updated.Spec.Endpoints = "abc.new-rmq:1111" + Expect(updated.ValidateUpdate(&replication)).To(Succeed()) + }) }) }) diff --git a/api/v1beta1/shovel_webhook.go b/api/v1beta1/shovel_webhook.go index cf9abcff..ed166613 100644 --- a/api/v1beta1/shovel_webhook.go +++ b/api/v1beta1/shovel_webhook.go @@ -19,9 +19,10 @@ func (s *Shovel) SetupWebhookWithManager(mgr ctrl.Manager) error { var _ webhook.Validator = &Shovel{} -// no validation on create +// ValidateCreate implements webhook.Validator so a webhook will be registered for the type +// either rabbitmqClusterReference.name or rabbitmqClusterReference.connectionSecret must be provided but not both func (s *Shovel) ValidateCreate() error { - return nil + return s.Spec.RabbitmqClusterReference.ValidateOnCreate(s.GroupResource(), s.Name) } // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type @@ -42,7 +43,7 @@ func (s *Shovel) ValidateUpdate(old runtime.Object) error { field.Forbidden(field.NewPath("spec", "vhost"), detailMsg)) } - if s.Spec.RabbitmqClusterReference != oldShovel.Spec.RabbitmqClusterReference { + if !oldShovel.Spec.RabbitmqClusterReference.Matches(&s.Spec.RabbitmqClusterReference) { return apierrors.NewForbidden(s.GroupResource(), s.Name, field.Forbidden(field.NewPath("spec", "rabbitmqClusterReference"), detailMsg)) } diff --git a/api/v1beta1/shovel_webhook_test.go b/api/v1beta1/shovel_webhook_test.go index 8d2b33b0..f3a7d4b8 100644 --- a/api/v1beta1/shovel_webhook_test.go +++ b/api/v1beta1/shovel_webhook_test.go @@ -47,166 +47,206 @@ var _ = Describe("shovel webhook", func() { }, } - It("does not allow updates on name", func() { - newShovel := shovel.DeepCopy() - newShovel.Spec.Name = "another-shovel" - Expect(apierrors.IsForbidden(newShovel.ValidateUpdate(&shovel))).To(BeTrue()) - }) - - It("does not allow updates on vhost", func() { - newShovel := shovel.DeepCopy() - newShovel.Spec.Vhost = "another-vhost" - Expect(apierrors.IsForbidden(newShovel.ValidateUpdate(&shovel))).To(BeTrue()) - }) - - It("does not allow updates on RabbitmqClusterReference", func() { - newShovel := shovel.DeepCopy() - newShovel.Spec.RabbitmqClusterReference = RabbitmqClusterReference{ - Name: "another-cluster", - } - Expect(apierrors.IsForbidden(newShovel.ValidateUpdate(&shovel))).To(BeTrue()) - }) - - It("allows updates on shovel.spec.uriSecret", func() { - newShovel := shovel.DeepCopy() - newShovel.Spec.UriSecret = &corev1.LocalObjectReference{Name: "another-secret"} - Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) - }) - - It("allows updates on AckMode", func() { - newShovel := shovel.DeepCopy() - newShovel.Spec.AckMode = "on-confirm" - Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) - }) - - It("allows updates on AddForwardHeaders", func() { - newShovel := shovel.DeepCopy() - newShovel.Spec.AddForwardHeaders = false - Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) - }) - - It("allows updates on DeleteAfter", func() { - newShovel := shovel.DeepCopy() - newShovel.Spec.DeleteAfter = "100" - Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) - }) - - It("allows updates on AddForwardHeaders", func() { - newShovel := shovel.DeepCopy() - newShovel.Spec.AddForwardHeaders = false - Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) - }) - - It("allows updates on DestinationAddForwardHeaders", func() { - newShovel := shovel.DeepCopy() - newShovel.Spec.DestinationAddForwardHeaders = false - Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) - }) - - It("allows updates on DestinationAddTimestampHeader", func() { - newShovel := shovel.DeepCopy() - newShovel.Spec.DestinationAddTimestampHeader = false - Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) - }) - - It("allows updates on DestinationAddress", func() { - newShovel := shovel.DeepCopy() - newShovel.Spec.DeleteAfter = "another" - Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) - }) - - It("allows updates on DestinationApplicationProperties", func() { - newShovel := shovel.DeepCopy() - newShovel.Spec.DestinationApplicationProperties = "new-property" - Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) - }) - - It("allows updates on DestinationExchange", func() { - newShovel := shovel.DeepCopy() - newShovel.Spec.DestinationExchange = "new-exchange" - Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) - }) - - It("allows updates on DestinationExchangeKey", func() { - newShovel := shovel.DeepCopy() - newShovel.Spec.DestinationExchangeKey = "new-key" - Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) - }) - - It("allows updates on DestinationProperties", func() { - newShovel := shovel.DeepCopy() - newShovel.Spec.DestinationProperties = "new" - Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) - }) - It("allows updates on DestinationProtocol", func() { - newShovel := shovel.DeepCopy() - newShovel.Spec.DestinationProtocol = "new" - Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) - }) - - It("allows updates on DestinationPublishProperties", func() { - newShovel := shovel.DeepCopy() - newShovel.Spec.DestinationPublishProperties = "new-property" - Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) - }) - - It("allows updates on DestinationQueue", func() { - newShovel := shovel.DeepCopy() - newShovel.Spec.DestinationQueue = "another-queue" - Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) - }) - - It("allows updates on shovel.spec.PrefetchCount", func() { - newShovel := shovel.DeepCopy() - newShovel.Spec.PrefetchCount = 20 - Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) - }) - - It("allows updates on shovel.spec.reconnectDelay", func() { - newShovel := shovel.DeepCopy() - newShovel.Spec.PrefetchCount = 10000 - Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) - }) - - It("allows updates on SourceAddress", func() { - newShovel := shovel.DeepCopy() - newShovel.Spec.SourceAddress = "another-queue" - Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) - }) - - It("allows updates on SourceDeleteAfter", func() { - newShovel := shovel.DeepCopy() - newShovel.Spec.SourceDeleteAfter = "100" - Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) - }) - - It("allows updates on SourceExchange", func() { - newShovel := shovel.DeepCopy() - newShovel.Spec.SourceExchange = "another-exchange" - Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) - }) - - It("allows updates on SourceExchangeKey", func() { - newShovel := shovel.DeepCopy() - newShovel.Spec.SourceExchangeKey = "another-key" - Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) - }) - - It("allows updates on SourcePrefetchCount", func() { - newShovel := shovel.DeepCopy() - newShovel.Spec.SourcePrefetchCount = 50 - Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) - }) - - It("allows updates on SourceProtocol", func() { - newShovel := shovel.DeepCopy() - newShovel.Spec.SourceProtocol = "another-protocol" - Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) - }) - - It("allows updates on SourceQueue", func() { - newShovel := shovel.DeepCopy() - newShovel.Spec.SourceQueue = "another-queue" - Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) + Context("ValidateCreate", func() { + It("does not allow both spec.rabbitmqClusterReference.name and spec.rabbitmqClusterReference.connectionSecret be configured", func() { + notAllowed := shovel.DeepCopy() + notAllowed.Spec.RabbitmqClusterReference.ConnectionSecret = &corev1.LocalObjectReference{Name: "some-secret"} + Expect(apierrors.IsForbidden(notAllowed.ValidateCreate())).To(BeTrue()) + }) + + It("spec.rabbitmqClusterReference.name and spec.rabbitmqClusterReference.connectionSecret cannot both be empty", func() { + notAllowed := shovel.DeepCopy() + notAllowed.Spec.RabbitmqClusterReference.Name = "" + notAllowed.Spec.RabbitmqClusterReference.ConnectionSecret = nil + Expect(apierrors.IsForbidden(notAllowed.ValidateCreate())).To(BeTrue()) + }) + }) + + Context("ValidateUpdate", func() { + It("does not allow updates on name", func() { + newShovel := shovel.DeepCopy() + newShovel.Spec.Name = "another-shovel" + Expect(apierrors.IsForbidden(newShovel.ValidateUpdate(&shovel))).To(BeTrue()) + }) + + It("does not allow updates on vhost", func() { + newShovel := shovel.DeepCopy() + newShovel.Spec.Vhost = "another-vhost" + Expect(apierrors.IsForbidden(newShovel.ValidateUpdate(&shovel))).To(BeTrue()) + }) + + It("does not allow updates on RabbitmqClusterReference", func() { + newShovel := shovel.DeepCopy() + newShovel.Spec.RabbitmqClusterReference = RabbitmqClusterReference{ + Name: "another-cluster", + } + Expect(apierrors.IsForbidden(newShovel.ValidateUpdate(&shovel))).To(BeTrue()) + }) + + It("does not allow updates on rabbitmqClusterReference.connectionSecret", func() { + connectionScr := Shovel{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + Spec: ShovelSpec{ + Name: "test-upstream", + Vhost: "/a-vhost", + UriSecret: &corev1.LocalObjectReference{ + Name: "a-secret", + }, + RabbitmqClusterReference: RabbitmqClusterReference{ + ConnectionSecret: &corev1.LocalObjectReference{ + Name: "a-secret", + }, + }, + }, + } + new := connectionScr.DeepCopy() + new.Spec.RabbitmqClusterReference.ConnectionSecret.Name = "new-secret" + Expect(apierrors.IsForbidden(new.ValidateUpdate(&connectionScr))).To(BeTrue()) + }) + + It("allows updates on shovel.spec.uriSecret", func() { + newShovel := shovel.DeepCopy() + newShovel.Spec.UriSecret = &corev1.LocalObjectReference{Name: "another-secret"} + Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) + }) + + It("allows updates on AckMode", func() { + newShovel := shovel.DeepCopy() + newShovel.Spec.AckMode = "on-confirm" + Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) + }) + + It("allows updates on AddForwardHeaders", func() { + newShovel := shovel.DeepCopy() + newShovel.Spec.AddForwardHeaders = false + Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) + }) + + It("allows updates on DeleteAfter", func() { + newShovel := shovel.DeepCopy() + newShovel.Spec.DeleteAfter = "100" + Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) + }) + + It("allows updates on AddForwardHeaders", func() { + newShovel := shovel.DeepCopy() + newShovel.Spec.AddForwardHeaders = false + Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) + }) + + It("allows updates on DestinationAddForwardHeaders", func() { + newShovel := shovel.DeepCopy() + newShovel.Spec.DestinationAddForwardHeaders = false + Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) + }) + + It("allows updates on DestinationAddTimestampHeader", func() { + newShovel := shovel.DeepCopy() + newShovel.Spec.DestinationAddTimestampHeader = false + Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) + }) + + It("allows updates on DestinationAddress", func() { + newShovel := shovel.DeepCopy() + newShovel.Spec.DeleteAfter = "another" + Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) + }) + + It("allows updates on DestinationApplicationProperties", func() { + newShovel := shovel.DeepCopy() + newShovel.Spec.DestinationApplicationProperties = "new-property" + Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) + }) + + It("allows updates on DestinationExchange", func() { + newShovel := shovel.DeepCopy() + newShovel.Spec.DestinationExchange = "new-exchange" + Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) + }) + + It("allows updates on DestinationExchangeKey", func() { + newShovel := shovel.DeepCopy() + newShovel.Spec.DestinationExchangeKey = "new-key" + Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) + }) + + It("allows updates on DestinationProperties", func() { + newShovel := shovel.DeepCopy() + newShovel.Spec.DestinationProperties = "new" + Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) + }) + It("allows updates on DestinationProtocol", func() { + newShovel := shovel.DeepCopy() + newShovel.Spec.DestinationProtocol = "new" + Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) + }) + + It("allows updates on DestinationPublishProperties", func() { + newShovel := shovel.DeepCopy() + newShovel.Spec.DestinationPublishProperties = "new-property" + Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) + }) + + It("allows updates on DestinationQueue", func() { + newShovel := shovel.DeepCopy() + newShovel.Spec.DestinationQueue = "another-queue" + Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) + }) + + It("allows updates on shovel.spec.PrefetchCount", func() { + newShovel := shovel.DeepCopy() + newShovel.Spec.PrefetchCount = 20 + Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) + }) + + It("allows updates on shovel.spec.reconnectDelay", func() { + newShovel := shovel.DeepCopy() + newShovel.Spec.PrefetchCount = 10000 + Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) + }) + + It("allows updates on SourceAddress", func() { + newShovel := shovel.DeepCopy() + newShovel.Spec.SourceAddress = "another-queue" + Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) + }) + + It("allows updates on SourceDeleteAfter", func() { + newShovel := shovel.DeepCopy() + newShovel.Spec.SourceDeleteAfter = "100" + Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) + }) + + It("allows updates on SourceExchange", func() { + newShovel := shovel.DeepCopy() + newShovel.Spec.SourceExchange = "another-exchange" + Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) + }) + + It("allows updates on SourceExchangeKey", func() { + newShovel := shovel.DeepCopy() + newShovel.Spec.SourceExchangeKey = "another-key" + Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) + }) + + It("allows updates on SourcePrefetchCount", func() { + newShovel := shovel.DeepCopy() + newShovel.Spec.SourcePrefetchCount = 50 + Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) + }) + + It("allows updates on SourceProtocol", func() { + newShovel := shovel.DeepCopy() + newShovel.Spec.SourceProtocol = "another-protocol" + Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) + }) + + It("allows updates on SourceQueue", func() { + newShovel := shovel.DeepCopy() + newShovel.Spec.SourceQueue = "another-queue" + Expect(newShovel.ValidateUpdate(&shovel)).To(Succeed()) + }) }) }) diff --git a/api/v1beta1/user_webhook.go b/api/v1beta1/user_webhook.go index f6eecada..f1cdc9ca 100644 --- a/api/v1beta1/user_webhook.go +++ b/api/v1beta1/user_webhook.go @@ -19,12 +19,13 @@ func (u *User) SetupWebhookWithManager(mgr ctrl.Manager) error { var _ webhook.Validator = &User{} -// no validation on create +// ValidateCreate implements webhook.Validator so a webhook will be registered for the type +// either rabbitmqClusterReference.name or rabbitmqClusterReference.connectionSecret must be provided but not both func (u *User) ValidateCreate() error { - return nil + return u.Spec.RabbitmqClusterReference.ValidateOnCreate(u.GroupResource(), u.Name) } -// returns error type 'forbidden' for updates on rabbitmqClusterReference +// ValidateUpdate returns error type 'forbidden' for updates on rabbitmqClusterReference // user.spec.tags can be updated func (u *User) ValidateUpdate(old runtime.Object) error { oldUser, ok := old.(*User) @@ -32,7 +33,7 @@ func (u *User) ValidateUpdate(old runtime.Object) error { return apierrors.NewBadRequest(fmt.Sprintf("expected a user but got a %T", old)) } - if u.Spec.RabbitmqClusterReference != oldUser.Spec.RabbitmqClusterReference { + if !oldUser.Spec.RabbitmqClusterReference.Matches(&u.Spec.RabbitmqClusterReference) { return apierrors.NewForbidden(u.GroupResource(), u.Name, field.Forbidden(field.NewPath("spec", "rabbitmqClusterReference"), "update on rabbitmqClusterReference is forbidden")) } diff --git a/api/v1beta1/user_webhook_test.go b/api/v1beta1/user_webhook_test.go index 79e09178..64734902 100644 --- a/api/v1beta1/user_webhook_test.go +++ b/api/v1beta1/user_webhook_test.go @@ -3,6 +3,7 @@ package v1beta1 import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -20,17 +21,54 @@ var _ = Describe("user webhook", func() { }, } - It("does not allow updates on RabbitmqClusterReference", func() { - newUser := user.DeepCopy() - newUser.Spec.RabbitmqClusterReference = RabbitmqClusterReference{ - Name: "newUser-cluster", - } - Expect(apierrors.IsForbidden(newUser.ValidateUpdate(&user))).To(BeTrue()) + Context("ValidateCreate", func() { + It("does not allow both spec.rabbitmqClusterReference.name and spec.rabbitmqClusterReference.connectionSecret be configured", func() { + notAllowed := user.DeepCopy() + notAllowed.Spec.RabbitmqClusterReference.ConnectionSecret = &corev1.LocalObjectReference{Name: "some-secret"} + Expect(apierrors.IsForbidden(notAllowed.ValidateCreate())).To(BeTrue()) + }) + + It("spec.rabbitmqClusterReference.name and spec.rabbitmqClusterReference.connectionSecret cannot both be empty", func() { + notAllowed := user.DeepCopy() + notAllowed.Spec.RabbitmqClusterReference.Name = "" + notAllowed.Spec.RabbitmqClusterReference.ConnectionSecret = nil + Expect(apierrors.IsForbidden(notAllowed.ValidateCreate())).To(BeTrue()) + }) }) - It("allows update on tags", func() { - newUser := user.DeepCopy() - newUser.Spec.Tags = []UserTag{"monitoring"} - Expect(newUser.ValidateUpdate(&user)).To(Succeed()) + Context("ValidateUpdate", func() { + It("does not allow updates on RabbitmqClusterReference", func() { + newUser := user.DeepCopy() + newUser.Spec.RabbitmqClusterReference = RabbitmqClusterReference{ + Name: "newUser-cluster", + } + Expect(apierrors.IsForbidden(newUser.ValidateUpdate(&user))).To(BeTrue()) + }) + + It("does not allow updates on rabbitmqClusterReference.connectionSecret", func() { + connectionScr := User{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + Spec: UserSpec{ + Tags: []UserTag{"policymaker"}, + RabbitmqClusterReference: RabbitmqClusterReference{ + ConnectionSecret: &corev1.LocalObjectReference{ + Name: "a-secret", + }, + }, + }, + } + new := connectionScr.DeepCopy() + new.Spec.RabbitmqClusterReference.Name = "a-name" + new.Spec.RabbitmqClusterReference.ConnectionSecret = nil + Expect(apierrors.IsForbidden(new.ValidateUpdate(&connectionScr))).To(BeTrue()) + }) + + It("allows update on tags", func() { + newUser := user.DeepCopy() + newUser.Spec.Tags = []UserTag{"monitoring"} + Expect(newUser.ValidateUpdate(&user)).To(Succeed()) + }) }) }) diff --git a/api/v1beta1/vhost_webhook.go b/api/v1beta1/vhost_webhook.go index ad8d1786..4d34436a 100644 --- a/api/v1beta1/vhost_webhook.go +++ b/api/v1beta1/vhost_webhook.go @@ -19,12 +19,13 @@ func (r *Vhost) SetupWebhookWithManager(mgr ctrl.Manager) error { var _ webhook.Validator = &Vhost{} -// no validation on create +// ValidateCreate implements webhook.Validator so a webhook will be registered for the type +// either rabbitmqClusterReference.name or rabbitmqClusterReference.connectionSecret must be provided but not both func (v *Vhost) ValidateCreate() error { - return nil + return v.Spec.RabbitmqClusterReference.ValidateOnCreate(v.GroupResource(), v.Name) } -// returns error type 'forbidden' for updates on vhost name and rabbitmqClusterReference +// ValidateUpdate returns error type 'forbidden' for updates on vhost name and rabbitmqClusterReference // vhost.spec.tracing can be updated func (v *Vhost) ValidateUpdate(old runtime.Object) error { oldVhost, ok := old.(*Vhost) @@ -38,7 +39,7 @@ func (v *Vhost) ValidateUpdate(old runtime.Object) error { field.Forbidden(field.NewPath("spec", "name"), detailMsg)) } - if v.Spec.RabbitmqClusterReference != oldVhost.Spec.RabbitmqClusterReference { + if !oldVhost.Spec.RabbitmqClusterReference.Matches(&v.Spec.RabbitmqClusterReference) { return apierrors.NewForbidden(v.GroupResource(), v.Name, field.Forbidden(field.NewPath("spec", "rabbitmqClusterReference"), detailMsg)) } diff --git a/api/v1beta1/vhost_webhook_test.go b/api/v1beta1/vhost_webhook_test.go index 89d2ea05..ae2f8558 100644 --- a/api/v1beta1/vhost_webhook_test.go +++ b/api/v1beta1/vhost_webhook_test.go @@ -3,6 +3,7 @@ package v1beta1 import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -22,29 +23,66 @@ var _ = Describe("vhost webhook", func() { }, } - It("does not allow updates on vhost name", func() { - newVhost := vhost.DeepCopy() - newVhost.Spec.Name = "new-name" - Expect(apierrors.IsForbidden(newVhost.ValidateUpdate(&vhost))).To(BeTrue()) - }) + Context("ValidateCreate", func() { + It("does not allow both spec.rabbitmqClusterReference.name and spec.rabbitmqClusterReference.connectionSecret be configured", func() { + notAllowed := vhost.DeepCopy() + notAllowed.Spec.RabbitmqClusterReference.ConnectionSecret = &corev1.LocalObjectReference{Name: "some-secret"} + Expect(apierrors.IsForbidden(notAllowed.ValidateCreate())).To(BeTrue()) + }) - It("does not allow updates on RabbitmqClusterReference", func() { - newVhost := vhost.DeepCopy() - newVhost.Spec.RabbitmqClusterReference = RabbitmqClusterReference{ - Name: "new-cluster", - } - Expect(apierrors.IsForbidden(newVhost.ValidateUpdate(&vhost))).To(BeTrue()) + It("spec.rabbitmqClusterReference.name and spec.rabbitmqClusterReference.connectionSecret cannot both be empty", func() { + notAllowed := vhost.DeepCopy() + notAllowed.Spec.RabbitmqClusterReference.Name = "" + notAllowed.Spec.RabbitmqClusterReference.ConnectionSecret = nil + Expect(apierrors.IsForbidden(notAllowed.ValidateCreate())).To(BeTrue()) + }) }) - It("allows updates on vhost.spec.tracing", func() { - newVhost := vhost.DeepCopy() - newVhost.Spec.Tracing = true - Expect(newVhost.ValidateUpdate(&vhost)).To(Succeed()) - }) + Context("ValidateUpdate", func() { + It("does not allow updates on vhost name", func() { + newVhost := vhost.DeepCopy() + newVhost.Spec.Name = "new-name" + Expect(apierrors.IsForbidden(newVhost.ValidateUpdate(&vhost))).To(BeTrue()) + }) + + It("does not allow updates on RabbitmqClusterReference", func() { + newVhost := vhost.DeepCopy() + newVhost.Spec.RabbitmqClusterReference = RabbitmqClusterReference{ + Name: "new-cluster", + } + Expect(apierrors.IsForbidden(newVhost.ValidateUpdate(&vhost))).To(BeTrue()) + }) + + It("does not allow updates on rabbitmqClusterReference.connectionSecret", func() { + connectionScr := Vhost{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-vhost", + }, + Spec: VhostSpec{ + Name: "test", + RabbitmqClusterReference: RabbitmqClusterReference{ + ConnectionSecret: &corev1.LocalObjectReference{ + Name: "a-secret", + }, + }, + }, + } + new := connectionScr.DeepCopy() + new.Spec.RabbitmqClusterReference.Name = "a-name" + new.Spec.RabbitmqClusterReference.ConnectionSecret = nil + Expect(apierrors.IsForbidden(new.ValidateUpdate(&connectionScr))).To(BeTrue()) + }) + + It("allows updates on vhost.spec.tracing", func() { + newVhost := vhost.DeepCopy() + newVhost.Spec.Tracing = true + Expect(newVhost.ValidateUpdate(&vhost)).To(Succeed()) + }) - It("allows updates on vhost.spec.tags", func() { - newVhost := vhost.DeepCopy() - newVhost.Spec.Tags = []string{"new-tag"} - Expect(newVhost.ValidateUpdate(&vhost)).To(Succeed()) + It("allows updates on vhost.spec.tags", func() { + newVhost := vhost.DeepCopy() + newVhost.Spec.Tags = []string{"new-tag"} + Expect(newVhost.ValidateUpdate(&vhost)).To(Succeed()) + }) }) }) diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 48e76c23..f9b0c70f 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -86,7 +86,7 @@ func (in *BindingSpec) DeepCopyInto(out *BindingSpec) { *out = new(runtime.RawExtension) (*in).DeepCopyInto(*out) } - out.RabbitmqClusterReference = in.RabbitmqClusterReference + in.RabbitmqClusterReference.DeepCopyInto(&out.RabbitmqClusterReference) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BindingSpec. @@ -204,7 +204,7 @@ func (in *ExchangeSpec) DeepCopyInto(out *ExchangeSpec) { *out = new(runtime.RawExtension) (*in).DeepCopyInto(*out) } - out.RabbitmqClusterReference = in.RabbitmqClusterReference + in.RabbitmqClusterReference.DeepCopyInto(&out.RabbitmqClusterReference) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ExchangeSpec. @@ -301,7 +301,7 @@ func (in *FederationList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *FederationSpec) DeepCopyInto(out *FederationSpec) { *out = *in - out.RabbitmqClusterReference = in.RabbitmqClusterReference + in.RabbitmqClusterReference.DeepCopyInto(&out.RabbitmqClusterReference) if in.UriSecret != nil { in, out := &in.UriSecret, &out.UriSecret *out = new(v1.LocalObjectReference) @@ -409,7 +409,7 @@ func (in *PermissionSpec) DeepCopyInto(out *PermissionSpec) { **out = **in } out.Permissions = in.Permissions - out.RabbitmqClusterReference = in.RabbitmqClusterReference + in.RabbitmqClusterReference.DeepCopyInto(&out.RabbitmqClusterReference) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PermissionSpec. @@ -511,7 +511,7 @@ func (in *PolicySpec) DeepCopyInto(out *PolicySpec) { *out = new(runtime.RawExtension) (*in).DeepCopyInto(*out) } - out.RabbitmqClusterReference = in.RabbitmqClusterReference + in.RabbitmqClusterReference.DeepCopyInto(&out.RabbitmqClusterReference) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PolicySpec. @@ -613,7 +613,7 @@ func (in *QueueSpec) DeepCopyInto(out *QueueSpec) { *out = new(runtime.RawExtension) (*in).DeepCopyInto(*out) } - out.RabbitmqClusterReference = in.RabbitmqClusterReference + in.RabbitmqClusterReference.DeepCopyInto(&out.RabbitmqClusterReference) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new QueueSpec. @@ -651,6 +651,11 @@ func (in *QueueStatus) DeepCopy() *QueueStatus { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RabbitmqClusterReference) DeepCopyInto(out *RabbitmqClusterReference) { *out = *in + if in.ConnectionSecret != nil { + in, out := &in.ConnectionSecret, &out.ConnectionSecret + *out = new(v1.LocalObjectReference) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RabbitmqClusterReference. @@ -725,7 +730,7 @@ func (in *SchemaReplicationList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SchemaReplicationSpec) DeepCopyInto(out *SchemaReplicationSpec) { *out = *in - out.RabbitmqClusterReference = in.RabbitmqClusterReference + in.RabbitmqClusterReference.DeepCopyInto(&out.RabbitmqClusterReference) if in.UpstreamSecret != nil { in, out := &in.UpstreamSecret, &out.UpstreamSecret *out = new(v1.LocalObjectReference) @@ -827,7 +832,7 @@ func (in *ShovelList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ShovelSpec) DeepCopyInto(out *ShovelSpec) { *out = *in - out.RabbitmqClusterReference = in.RabbitmqClusterReference + in.RabbitmqClusterReference.DeepCopyInto(&out.RabbitmqClusterReference) if in.UriSecret != nil { in, out := &in.UriSecret, &out.UriSecret *out = new(v1.LocalObjectReference) @@ -934,7 +939,7 @@ func (in *UserSpec) DeepCopyInto(out *UserSpec) { *out = make([]UserTag, len(*in)) copy(*out, *in) } - out.RabbitmqClusterReference = in.RabbitmqClusterReference + in.RabbitmqClusterReference.DeepCopyInto(&out.RabbitmqClusterReference) if in.ImportCredentialsSecret != nil { in, out := &in.ImportCredentialsSecret, &out.ImportCredentialsSecret *out = new(v1.LocalObjectReference) @@ -1061,7 +1066,7 @@ func (in *VhostSpec) DeepCopyInto(out *VhostSpec) { *out = make([]string, len(*in)) copy(*out, *in) } - out.RabbitmqClusterReference = in.RabbitmqClusterReference + in.RabbitmqClusterReference.DeepCopyInto(&out.RabbitmqClusterReference) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VhostSpec. diff --git a/config/crd/bases/rabbitmq.com_bindings.yaml b/config/crd/bases/rabbitmq.com_bindings.yaml index 7549ba09..40fbd00b 100644 --- a/config/crd/bases/rabbitmq.com_bindings.yaml +++ b/config/crd/bases/rabbitmq.com_bindings.yaml @@ -55,15 +55,25 @@ spec: description: Reference to the RabbitmqCluster that the binding will be created in. Required property. properties: + connectionSecret: + description: Secret contains the http management uri for the RabbitMQ + cluster. The Secret must contain the key `uri`, `username` and + `password` or operator will error. Have to set either name or + connectionSecret, but not both. + properties: + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + type: object name: - description: The name of the RabbitMQ cluster to reference. + description: The name of the RabbitMQ cluster to reference. Have + to set either name or connectionSecret, but not both. type: string namespace: description: The namespace of the RabbitMQ cluster to reference. Defaults to the namespace of the requested resource if omitted. type: string - required: - - name type: object routingKey: description: Cannot be updated diff --git a/config/crd/bases/rabbitmq.com_exchanges.yaml b/config/crd/bases/rabbitmq.com_exchanges.yaml index 01e59fe8..29d86383 100644 --- a/config/crd/bases/rabbitmq.com_exchanges.yaml +++ b/config/crd/bases/rabbitmq.com_exchanges.yaml @@ -54,15 +54,25 @@ spec: description: Reference to the RabbitmqCluster that the exchange will be created in. Required property. properties: + connectionSecret: + description: Secret contains the http management uri for the RabbitMQ + cluster. The Secret must contain the key `uri`, `username` and + `password` or operator will error. Have to set either name or + connectionSecret, but not both. + properties: + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + type: object name: - description: The name of the RabbitMQ cluster to reference. + description: The name of the RabbitMQ cluster to reference. Have + to set either name or connectionSecret, but not both. type: string namespace: description: The namespace of the RabbitMQ cluster to reference. Defaults to the namespace of the requested resource if omitted. type: string - required: - - name type: object type: default: direct diff --git a/config/crd/bases/rabbitmq.com_federations.yaml b/config/crd/bases/rabbitmq.com_federations.yaml index 92cdb5ee..f795595f 100644 --- a/config/crd/bases/rabbitmq.com_federations.yaml +++ b/config/crd/bases/rabbitmq.com_federations.yaml @@ -64,15 +64,25 @@ spec: description: Reference to the RabbitmqCluster that this federation upstream will be created in. Required property. properties: + connectionSecret: + description: Secret contains the http management uri for the RabbitMQ + cluster. The Secret must contain the key `uri`, `username` and + `password` or operator will error. Have to set either name or + connectionSecret, but not both. + properties: + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + type: object name: - description: The name of the RabbitMQ cluster to reference. + description: The name of the RabbitMQ cluster to reference. Have + to set either name or connectionSecret, but not both. type: string namespace: description: The namespace of the RabbitMQ cluster to reference. Defaults to the namespace of the requested resource if omitted. type: string - required: - - name type: object reconnectDelay: type: integer diff --git a/config/crd/bases/rabbitmq.com_permissions.yaml b/config/crd/bases/rabbitmq.com_permissions.yaml index 297a060b..3b489eb4 100644 --- a/config/crd/bases/rabbitmq.com_permissions.yaml +++ b/config/crd/bases/rabbitmq.com_permissions.yaml @@ -53,15 +53,25 @@ spec: description: Reference to the RabbitmqCluster that both the provided user and vhost are. Required property. properties: + connectionSecret: + description: Secret contains the http management uri for the RabbitMQ + cluster. The Secret must contain the key `uri`, `username` and + `password` or operator will error. Have to set either name or + connectionSecret, but not both. + properties: + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + type: object name: - description: The name of the RabbitMQ cluster to reference. + description: The name of the RabbitMQ cluster to reference. Have + to set either name or connectionSecret, but not both. type: string namespace: description: The namespace of the RabbitMQ cluster to reference. Defaults to the namespace of the requested resource if omitted. type: string - required: - - name type: object user: description: Name of an existing user; must provide user or userReference, diff --git a/config/crd/bases/rabbitmq.com_policies.yaml b/config/crd/bases/rabbitmq.com_policies.yaml index ced9a371..b94504ed 100644 --- a/config/crd/bases/rabbitmq.com_policies.yaml +++ b/config/crd/bases/rabbitmq.com_policies.yaml @@ -68,15 +68,25 @@ spec: description: Reference to the RabbitmqCluster that the exchange will be created in. Required property. properties: + connectionSecret: + description: Secret contains the http management uri for the RabbitMQ + cluster. The Secret must contain the key `uri`, `username` and + `password` or operator will error. Have to set either name or + connectionSecret, but not both. + properties: + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + type: object name: - description: The name of the RabbitMQ cluster to reference. + description: The name of the RabbitMQ cluster to reference. Have + to set either name or connectionSecret, but not both. type: string namespace: description: The namespace of the RabbitMQ cluster to reference. Defaults to the namespace of the requested resource if omitted. type: string - required: - - name type: object vhost: default: / diff --git a/config/crd/bases/rabbitmq.com_queues.yaml b/config/crd/bases/rabbitmq.com_queues.yaml index ab92dc1a..505563f7 100644 --- a/config/crd/bases/rabbitmq.com_queues.yaml +++ b/config/crd/bases/rabbitmq.com_queues.yaml @@ -59,15 +59,25 @@ spec: description: Reference to the RabbitmqCluster that the queue will be created in. Required property. properties: + connectionSecret: + description: Secret contains the http management uri for the RabbitMQ + cluster. The Secret must contain the key `uri`, `username` and + `password` or operator will error. Have to set either name or + connectionSecret, but not both. + properties: + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + type: object name: - description: The name of the RabbitMQ cluster to reference. + description: The name of the RabbitMQ cluster to reference. Have + to set either name or connectionSecret, but not both. type: string namespace: description: The namespace of the RabbitMQ cluster to reference. Defaults to the namespace of the requested resource if omitted. type: string - required: - - name type: object type: type: string diff --git a/config/crd/bases/rabbitmq.com_schemareplications.yaml b/config/crd/bases/rabbitmq.com_schemareplications.yaml index 45221a07..1326f5fa 100644 --- a/config/crd/bases/rabbitmq.com_schemareplications.yaml +++ b/config/crd/bases/rabbitmq.com_schemareplications.yaml @@ -48,15 +48,25 @@ spec: description: Reference to the RabbitmqCluster that schema replication would be set for. Must be an existing cluster. properties: + connectionSecret: + description: Secret contains the http management uri for the RabbitMQ + cluster. The Secret must contain the key `uri`, `username` and + `password` or operator will error. Have to set either name or + connectionSecret, but not both. + properties: + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + type: object name: - description: The name of the RabbitMQ cluster to reference. + description: The name of the RabbitMQ cluster to reference. Have + to set either name or connectionSecret, but not both. type: string namespace: description: The namespace of the RabbitMQ cluster to reference. Defaults to the namespace of the requested resource if omitted. type: string - required: - - name type: object upstreamSecret: description: Defines a Secret which contains credentials to be used diff --git a/config/crd/bases/rabbitmq.com_shovels.yaml b/config/crd/bases/rabbitmq.com_shovels.yaml index 169b2efe..9a309a75 100644 --- a/config/crd/bases/rabbitmq.com_shovels.yaml +++ b/config/crd/bases/rabbitmq.com_shovels.yaml @@ -78,15 +78,25 @@ spec: description: Reference to the RabbitmqCluster that this Shovel will be created in. Required property. properties: + connectionSecret: + description: Secret contains the http management uri for the RabbitMQ + cluster. The Secret must contain the key `uri`, `username` and + `password` or operator will error. Have to set either name or + connectionSecret, but not both. + properties: + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + type: object name: - description: The name of the RabbitMQ cluster to reference. + description: The name of the RabbitMQ cluster to reference. Have + to set either name or connectionSecret, but not both. type: string namespace: description: The namespace of the RabbitMQ cluster to reference. Defaults to the namespace of the requested resource if omitted. type: string - required: - - name type: object reconnectDelay: type: integer diff --git a/config/crd/bases/rabbitmq.com_superstreams.yaml b/config/crd/bases/rabbitmq.com_superstreams.yaml index 6b3f9bb8..9655a6dc 100644 --- a/config/crd/bases/rabbitmq.com_superstreams.yaml +++ b/config/crd/bases/rabbitmq.com_superstreams.yaml @@ -50,15 +50,25 @@ spec: description: Reference to the RabbitmqCluster that the SuperStream will be created in. Required property. properties: + connectionSecret: + description: Secret contains the http management uri for the RabbitMQ + cluster. The Secret must contain the key `uri`, `username` and + `password` or operator will error. Have to set either name or + connectionSecret, but not both. + properties: + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + type: object name: - description: The name of the RabbitMQ cluster to reference. + description: The name of the RabbitMQ cluster to reference. Have + to set either name or connectionSecret, but not both. type: string namespace: description: The namespace of the RabbitMQ cluster to reference. Defaults to the namespace of the requested resource if omitted. type: string - required: - - name type: object routingKeys: description: Routing keys to use for each of the partitions in the diff --git a/config/crd/bases/rabbitmq.com_users.yaml b/config/crd/bases/rabbitmq.com_users.yaml index 63dcbfdf..a9bd5230 100644 --- a/config/crd/bases/rabbitmq.com_users.yaml +++ b/config/crd/bases/rabbitmq.com_users.yaml @@ -56,15 +56,25 @@ spec: description: Reference to the RabbitmqCluster that the user will be created for. This cluster must exist for the User object to be created. properties: + connectionSecret: + description: Secret contains the http management uri for the RabbitMQ + cluster. The Secret must contain the key `uri`, `username` and + `password` or operator will error. Have to set either name or + connectionSecret, but not both. + properties: + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + type: object name: - description: The name of the RabbitMQ cluster to reference. + description: The name of the RabbitMQ cluster to reference. Have + to set either name or connectionSecret, but not both. type: string namespace: description: The namespace of the RabbitMQ cluster to reference. Defaults to the namespace of the requested resource if omitted. type: string - required: - - name type: object tags: description: List of permissions tags to associate with the user. diff --git a/config/crd/bases/rabbitmq.com_vhosts.yaml b/config/crd/bases/rabbitmq.com_vhosts.yaml index e97d90c1..a498af41 100644 --- a/config/crd/bases/rabbitmq.com_vhosts.yaml +++ b/config/crd/bases/rabbitmq.com_vhosts.yaml @@ -45,15 +45,25 @@ spec: description: Reference to the RabbitmqCluster that the vhost will be created in. Required property. properties: + connectionSecret: + description: Secret contains the http management uri for the RabbitMQ + cluster. The Secret must contain the key `uri`, `username` and + `password` or operator will error. Have to set either name or + connectionSecret, but not both. + properties: + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + type: object name: - description: The name of the RabbitMQ cluster to reference. + description: The name of the RabbitMQ cluster to reference. Have + to set either name or connectionSecret, but not both. type: string namespace: description: The namespace of the RabbitMQ cluster to reference. Defaults to the namespace of the requested resource if omitted. type: string - required: - - name type: object tags: items: diff --git a/controllers/binding_controller.go b/controllers/binding_controller.go index 9a7b0a85..96ba393c 100644 --- a/controllers/binding_controller.go +++ b/controllers/binding_controller.go @@ -54,12 +54,12 @@ func (r *BindingReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct return ctrl.Result{}, err } - rmq, svc, credsProvider, err := internal.ParseRabbitmqClusterReference(ctx, r.Client, binding.Spec.RabbitmqClusterReference, binding.Namespace) + credsProvider, tlsEnabled, err := internal.ParseRabbitmqClusterReference(ctx, r.Client, binding.Spec.RabbitmqClusterReference, binding.Namespace) if err != nil { return handleRMQReferenceParseError(ctx, r.Client, r.Recorder, binding, &binding.Status.Conditions, err) } - rabbitClient, err := r.RabbitmqClientFactory(rmq, svc, credsProvider, serviceDNSAddress(svc), systemCertPool) + rabbitClient, err := r.RabbitmqClientFactory(credsProvider, tlsEnabled, systemCertPool) if err != nil { logger.Error(err, failedGenerateRabbitClient) return reconcile.Result{}, err diff --git a/controllers/exchange_controller.go b/controllers/exchange_controller.go index e0565e56..e137c223 100644 --- a/controllers/exchange_controller.go +++ b/controllers/exchange_controller.go @@ -52,12 +52,12 @@ func (r *ExchangeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c return ctrl.Result{}, err } - rmq, svc, credsProvider, err := internal.ParseRabbitmqClusterReference(ctx, r.Client, exchange.Spec.RabbitmqClusterReference, exchange.Namespace) + credsProvider, tlsEnabled, err := internal.ParseRabbitmqClusterReference(ctx, r.Client, exchange.Spec.RabbitmqClusterReference, exchange.Namespace) if err != nil { return handleRMQReferenceParseError(ctx, r.Client, r.Recorder, exchange, &exchange.Status.Conditions, err) } - rabbitClient, err := r.RabbitmqClientFactory(rmq, svc, credsProvider, serviceDNSAddress(svc), systemCertPool) + rabbitClient, err := r.RabbitmqClientFactory(credsProvider, tlsEnabled, systemCertPool) if err != nil { logger.Error(err, failedGenerateRabbitClient) return reconcile.Result{}, err diff --git a/controllers/federation_controller.go b/controllers/federation_controller.go index aacbc33d..9f457227 100644 --- a/controllers/federation_controller.go +++ b/controllers/federation_controller.go @@ -45,12 +45,12 @@ func (r *FederationReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, err } - rmq, svc, credsProvider, err := internal.ParseRabbitmqClusterReference(ctx, r.Client, federation.Spec.RabbitmqClusterReference, federation.Namespace) + credsProvider, tlsEnabled, err := internal.ParseRabbitmqClusterReference(ctx, r.Client, federation.Spec.RabbitmqClusterReference, federation.Namespace) if err != nil { return handleRMQReferenceParseError(ctx, r.Client, r.Recorder, federation, &federation.Status.Conditions, err) } - rabbitClient, err := r.RabbitmqClientFactory(rmq, svc, credsProvider, serviceDNSAddress(svc), systemCertPool) + rabbitClient, err := r.RabbitmqClientFactory(credsProvider, tlsEnabled, systemCertPool) if err != nil { logger.Error(err, failedGenerateRabbitClient) return reconcile.Result{}, err diff --git a/controllers/permission_controller.go b/controllers/permission_controller.go index cdf6ec18..0daee98b 100644 --- a/controllers/permission_controller.go +++ b/controllers/permission_controller.go @@ -46,12 +46,12 @@ func (r *PermissionReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, err } - rmq, svc, credsProvider, err := internal.ParseRabbitmqClusterReference(ctx, r.Client, permission.Spec.RabbitmqClusterReference, permission.Namespace) + credsProvider, tlsEnabled, err := internal.ParseRabbitmqClusterReference(ctx, r.Client, permission.Spec.RabbitmqClusterReference, permission.Namespace) if err != nil { return handleRMQReferenceParseError(ctx, r.Client, r.Recorder, permission, &permission.Status.Conditions, err) } - rabbitClient, err := r.RabbitmqClientFactory(rmq, svc, credsProvider, serviceDNSAddress(svc), systemCertPool) + rabbitClient, err := r.RabbitmqClientFactory(credsProvider, tlsEnabled, systemCertPool) if err != nil { logger.Error(err, failedGenerateRabbitClient) return reconcile.Result{}, err diff --git a/controllers/policy_controller.go b/controllers/policy_controller.go index c8ba6d38..e545c0f9 100644 --- a/controllers/policy_controller.go +++ b/controllers/policy_controller.go @@ -53,12 +53,12 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr return ctrl.Result{}, err } - rmq, svc, credsProvider, err := internal.ParseRabbitmqClusterReference(ctx, r.Client, policy.Spec.RabbitmqClusterReference, policy.Namespace) + credsProvider, tlsEnabled, err := internal.ParseRabbitmqClusterReference(ctx, r.Client, policy.Spec.RabbitmqClusterReference, policy.Namespace) if err != nil { return handleRMQReferenceParseError(ctx, r.Client, r.Recorder, policy, &policy.Status.Conditions, err) } - rabbitClient, err := r.RabbitmqClientFactory(rmq, svc, credsProvider, serviceDNSAddress(svc), systemCertPool) + rabbitClient, err := r.RabbitmqClientFactory(credsProvider, tlsEnabled, systemCertPool) if err != nil { logger.Error(err, failedGenerateRabbitClient) return reconcile.Result{}, err diff --git a/controllers/queue_controller.go b/controllers/queue_controller.go index 85c91076..d07ebd9d 100644 --- a/controllers/queue_controller.go +++ b/controllers/queue_controller.go @@ -55,12 +55,12 @@ func (r *QueueReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl return ctrl.Result{}, err } - rmq, svc, credsProvider, err := internal.ParseRabbitmqClusterReference(ctx, r.Client, queue.Spec.RabbitmqClusterReference, queue.Namespace) + credsProvider, tlsEnabled, err := internal.ParseRabbitmqClusterReference(ctx, r.Client, queue.Spec.RabbitmqClusterReference, queue.Namespace) if err != nil { return handleRMQReferenceParseError(ctx, r.Client, r.Recorder, queue, &queue.Status.Conditions, err) } - rabbitClient, err := r.RabbitmqClientFactory(rmq, svc, credsProvider, serviceDNSAddress(svc), systemCertPool) + rabbitClient, err := r.RabbitmqClientFactory(credsProvider, tlsEnabled, systemCertPool) if err != nil { logger.Error(err, failedGenerateRabbitClient) return reconcile.Result{}, err diff --git a/controllers/schemareplication_controller.go b/controllers/schemareplication_controller.go index 7a6282c5..1cd42032 100644 --- a/controllers/schemareplication_controller.go +++ b/controllers/schemareplication_controller.go @@ -47,12 +47,12 @@ func (r *SchemaReplicationReconciler) Reconcile(ctx context.Context, req ctrl.Re return ctrl.Result{}, err } - rmq, svc, credsProvider, err := internal.ParseRabbitmqClusterReference(ctx, r.Client, replication.Spec.RabbitmqClusterReference, replication.Namespace) + credsProvider, tlsEnabled, err := internal.ParseRabbitmqClusterReference(ctx, r.Client, replication.Spec.RabbitmqClusterReference, replication.Namespace) if err != nil { return handleRMQReferenceParseError(ctx, r.Client, r.Recorder, replication, &replication.Status.Conditions, err) } - rabbitClient, err := r.RabbitmqClientFactory(rmq, svc, credsProvider, serviceDNSAddress(svc), systemCertPool) + rabbitClient, err := r.RabbitmqClientFactory(credsProvider, tlsEnabled, systemCertPool) if err != nil { logger.Error(err, failedGenerateRabbitClient) return reconcile.Result{}, err diff --git a/controllers/shovel_controller.go b/controllers/shovel_controller.go index 3c3ef408..62a2889e 100644 --- a/controllers/shovel_controller.go +++ b/controllers/shovel_controller.go @@ -45,12 +45,12 @@ func (r *ShovelReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr return ctrl.Result{}, err } - rmq, svc, credsProvider, err := internal.ParseRabbitmqClusterReference(ctx, r.Client, shovel.Spec.RabbitmqClusterReference, shovel.Namespace) + credsProvider, tlsEnabled, err := internal.ParseRabbitmqClusterReference(ctx, r.Client, shovel.Spec.RabbitmqClusterReference, shovel.Namespace) if err != nil { return handleRMQReferenceParseError(ctx, r.Client, r.Recorder, shovel, &shovel.Status.Conditions, err) } - rabbitClient, err := r.RabbitmqClientFactory(rmq, svc, credsProvider, serviceDNSAddress(svc), systemCertPool) + rabbitClient, err := r.RabbitmqClientFactory(credsProvider, tlsEnabled, systemCertPool) if err != nil { logger.Error(err, failedGenerateRabbitClient) return reconcile.Result{}, err diff --git a/controllers/suite_test.go b/controllers/suite_test.go index 5cc14e3e..617e892f 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -51,13 +51,13 @@ var ( ctx = context.Background() fakeRabbitMQClient *internalfakes.FakeRabbitMQClient fakeRabbitMQClientError error - fakeRabbitMQClientFactory = func(rmq *rabbitmqv1beta1.RabbitmqCluster, svc *corev1.Service, credsProvider internal.CredentialsProvider, hostname string, certPool *x509.CertPool) (internal.RabbitMQClient, error) { + fakeRabbitMQClientFactory = func(connectionCreds internal.ConnectionCredentials, tlsEnabled bool, certPool *x509.CertPool) (internal.RabbitMQClient, error) { return fakeRabbitMQClient, fakeRabbitMQClientError } - existingRabbitMQUsername = "abc123" - existingRabbitMQPassword = "foo1234" - fakeCredentialsProvider *internalfakes.FakeCredentialsProvider - fakeRecorder *record.FakeRecorder + existingRabbitMQUsername = "abc123" + existingRabbitMQPassword = "foo1234" + fakeConnectionCredentials *internalfakes.FakeConnectionCredentials + fakeRecorder *record.FakeRecorder ) var _ = BeforeSuite(func() { @@ -87,10 +87,11 @@ var _ = BeforeSuite(func() { }) Expect(err).ToNot(HaveOccurred()) - fakeCredentialsProvider = &internalfakes.FakeCredentialsProvider{} + fakeConnectionCredentials = &internalfakes.FakeConnectionCredentials{} - fakeCredentialsProvider.DataReturnsOnCall(0, []byte(existingRabbitMQUsername), true) - fakeCredentialsProvider.DataReturnsOnCall(1, []byte(existingRabbitMQPassword), true) + fakeConnectionCredentials.DataReturnsOnCall(0, []byte(existingRabbitMQUsername), true) + fakeConnectionCredentials.DataReturnsOnCall(1, []byte(existingRabbitMQPassword), true) + fakeConnectionCredentials.DataReturnsOnCall(2, []byte("example.com"), true) fakeRecorder = record.NewFakeRecorder(128) err = (&controllers.BindingReconciler{ @@ -194,8 +195,13 @@ var _ = BeforeSuite(func() { }, Spec: corev1.ServiceSpec{ Ports: []corev1.ServicePort{ + { + Port: 15672, + Name: "management", + }, { Port: 15671, + Name: "management-tls", }, }, }, @@ -242,8 +248,13 @@ var _ = BeforeSuite(func() { }, Spec: corev1.ServiceSpec{ Ports: []corev1.ServicePort{ + { + Port: 15672, + Name: "management", + }, { Port: 15671, + Name: "management-tls", }, }, }, diff --git a/controllers/super_stream_controller.go b/controllers/super_stream_controller.go index 8a1429f4..783ce7ce 100644 --- a/controllers/super_stream_controller.go +++ b/controllers/super_stream_controller.go @@ -13,12 +13,14 @@ import ( "context" "fmt" "github.com/go-logr/logr" + rabbitmqv1beta1 "github.com/rabbitmq/cluster-operator/api/v1beta1" topologyv1alpha1 "github.com/rabbitmq/messaging-topology-operator/api/v1alpha1" topology "github.com/rabbitmq/messaging-topology-operator/api/v1beta1" "github.com/rabbitmq/messaging-topology-operator/internal" "github.com/rabbitmq/messaging-topology-operator/internal/managedresource" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/record" clientretry "k8s.io/client-go/util/retry" ctrl "sigs.k8s.io/controller-runtime" @@ -55,7 +57,7 @@ func (r *SuperStreamReconciler) Reconcile(ctx context.Context, req ctrl.Request) return reconcile.Result{}, client.IgnoreNotFound(err) } - rmq, _, _, err := internal.ParseRabbitmqClusterReference(ctx, r.Client, superStream.Spec.RabbitmqClusterReference, superStream.Namespace) + rmqClusterRef, err := r.getRabbitmqClusterReference(ctx, superStream.Spec.RabbitmqClusterReference, superStream.Namespace) if err != nil { return handleRMQReferenceParseError(ctx, r.Client, r.Recorder, superStream, &superStream.Status.Conditions, err) } @@ -103,10 +105,6 @@ func (r *SuperStreamReconciler) Reconcile(ctx context.Context, req ctrl.Request) Scheme: r.Scheme, } - rmqClusterRef := &topology.RabbitmqClusterReference{ - Name: rmq.Name, - Namespace: rmq.Namespace, - } builders := []managedresource.ResourceBuilder{managedResourceBuilder.SuperStreamExchange(superStream.Spec.Vhost, rmqClusterRef)} for index, routingKey := range routingKeys { builders = append( @@ -160,6 +158,29 @@ func (r *SuperStreamReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, nil } +func (r *SuperStreamReconciler) getRabbitmqClusterReference(ctx context.Context, rmq topology.RabbitmqClusterReference, requestNamespace string) (*topology.RabbitmqClusterReference, error) { + var namespace string + if rmq.Namespace == "" { + namespace = requestNamespace + } else { + namespace = rmq.Namespace + } + + cluster := &rabbitmqv1beta1.RabbitmqCluster{} + if err := r.Get(ctx, types.NamespacedName{Name: rmq.Name, Namespace: namespace}, cluster); err != nil { + return nil, fmt.Errorf("failed to get cluster from reference: %s Error: %w", err, internal.NoSuchRabbitmqClusterError) + } + + if !internal.AllowedNamespace(rmq, requestNamespace, cluster) { + return nil, internal.ResourceNotAllowedError + } + + return &topology.RabbitmqClusterReference{ + Name: rmq.Name, + Namespace: namespace, + }, nil +} + func (r *SuperStreamReconciler) generateRoutingKeys(superStream *topologyv1alpha1.SuperStream) (routingKeys []string) { for i := 0; i < superStream.Spec.Partitions; i++ { routingKeys = append(routingKeys, strconv.Itoa(i)) diff --git a/controllers/super_stream_controller_test.go b/controllers/super_stream_controller_test.go index e32071c0..94953bed 100644 --- a/controllers/super_stream_controller_test.go +++ b/controllers/super_stream_controller_test.go @@ -85,8 +85,9 @@ var _ = Describe("super-stream-controller", func() { "Type": Equal("direct"), "Durable": BeTrue(), "RabbitmqClusterReference": MatchAllFields(Fields{ - "Name": Equal("example-rabbit"), - "Namespace": Equal("default"), + "Name": Equal("example-rabbit"), + "Namespace": Equal("default"), + "ConnectionSecret": BeNil(), }), })) }) @@ -111,8 +112,9 @@ var _ = Describe("super-stream-controller", func() { "Type": Equal("stream"), "Durable": BeTrue(), "RabbitmqClusterReference": MatchAllFields(Fields{ - "Name": Equal("example-rabbit"), - "Namespace": Equal("default"), + "Name": Equal("example-rabbit"), + "Namespace": Equal("default"), + "ConnectionSecret": BeNil(), }), })) } @@ -150,8 +152,9 @@ var _ = Describe("super-stream-controller", func() { })), "RoutingKey": Equal(strconv.Itoa(i)), "RabbitmqClusterReference": MatchAllFields(Fields{ - "Name": Equal("example-rabbit"), - "Namespace": Equal("default"), + "Name": Equal("example-rabbit"), + "Namespace": Equal("default"), + "ConnectionSecret": BeNil(), }), })) } @@ -381,8 +384,9 @@ var _ = Describe("super-stream-controller", func() { "Type": Equal("stream"), "Durable": BeTrue(), "RabbitmqClusterReference": MatchAllFields(Fields{ - "Name": Equal("example-rabbit"), - "Namespace": Equal("default"), + "Name": Equal("example-rabbit"), + "Namespace": Equal("default"), + "ConnectionSecret": BeNil(), }), })) } @@ -420,8 +424,9 @@ var _ = Describe("super-stream-controller", func() { })), "RoutingKey": Equal(strconv.Itoa(i)), "RabbitmqClusterReference": MatchAllFields(Fields{ - "Name": Equal("example-rabbit"), - "Namespace": Equal("default"), + "Name": Equal("example-rabbit"), + "Namespace": Equal("default"), + "ConnectionSecret": BeNil(), }), })) } @@ -484,8 +489,9 @@ var _ = Describe("super-stream-controller", func() { "Type": Equal("stream"), "Durable": BeTrue(), "RabbitmqClusterReference": MatchAllFields(Fields{ - "Name": Equal("example-rabbit"), - "Namespace": Equal("default"), + "Name": Equal("example-rabbit"), + "Namespace": Equal("default"), + "ConnectionSecret": BeNil(), }), })) } @@ -523,8 +529,9 @@ var _ = Describe("super-stream-controller", func() { })), "RoutingKey": Equal(strconv.Itoa(i)), "RabbitmqClusterReference": MatchAllFields(Fields{ - "Name": Equal("example-rabbit"), - "Namespace": Equal("default"), + "Name": Equal("example-rabbit"), + "Namespace": Equal("default"), + "ConnectionSecret": BeNil(), }), })) } @@ -589,8 +596,9 @@ var _ = Describe("super-stream-controller", func() { "Type": Equal("direct"), "Durable": BeTrue(), "RabbitmqClusterReference": MatchAllFields(Fields{ - "Name": Equal("example-rabbit"), - "Namespace": Equal("default"), + "Name": Equal("example-rabbit"), + "Namespace": Equal("default"), + "ConnectionSecret": BeNil(), }), })) }) @@ -614,8 +622,9 @@ var _ = Describe("super-stream-controller", func() { "Type": Equal("stream"), "Durable": BeTrue(), "RabbitmqClusterReference": MatchAllFields(Fields{ - "Name": Equal("example-rabbit"), - "Namespace": Equal("default"), + "Name": Equal("example-rabbit"), + "Namespace": Equal("default"), + "ConnectionSecret": BeNil(), }), })) } @@ -652,8 +661,9 @@ var _ = Describe("super-stream-controller", func() { })), "RoutingKey": Equal(superStream.Spec.RoutingKeys[i]), "RabbitmqClusterReference": MatchAllFields(Fields{ - "Name": Equal("example-rabbit"), - "Namespace": Equal("default"), + "Name": Equal("example-rabbit"), + "Namespace": Equal("default"), + "ConnectionSecret": BeNil(), }), })) } diff --git a/controllers/user_controller.go b/controllers/user_controller.go index 4b9d49c5..854b6314 100644 --- a/controllers/user_controller.go +++ b/controllers/user_controller.go @@ -63,12 +63,12 @@ func (r *UserReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl. return ctrl.Result{}, err } - rmq, svc, credsProvider, err := internal.ParseRabbitmqClusterReference(ctx, r.Client, user.Spec.RabbitmqClusterReference, user.Namespace) + credsProvider, tlsEnabled, err := internal.ParseRabbitmqClusterReference(ctx, r.Client, user.Spec.RabbitmqClusterReference, user.Namespace) if err != nil { return handleRMQReferenceParseError(ctx, r.Client, r.Recorder, user, &user.Status.Conditions, err) } - rabbitClient, err := r.RabbitmqClientFactory(rmq, svc, credsProvider, serviceDNSAddress(svc), systemCertPool) + rabbitClient, err := r.RabbitmqClientFactory(credsProvider, tlsEnabled, systemCertPool) if err != nil { logger.Error(err, failedGenerateRabbitClient) return reconcile.Result{}, err diff --git a/controllers/utils.go b/controllers/utils.go index 9c803917..216fe87a 100644 --- a/controllers/utils.go +++ b/controllers/utils.go @@ -58,15 +58,6 @@ func validateResponseForDeletion(res *http.Response, err error) error { return validateResponse(res, err) } -// serviceDNSAddress returns the cluster-local DNS entry associated -// with the provided Service -func serviceDNSAddress(svc *corev1.Service) string { - // NOTE: this does not use the `cluster.local` suffix, because that is not - // uniform across clusters. See the `clusterDomain` KubeletConfiguration - // value for how this can be changed for a cluster. - return fmt.Sprintf("%s.%s.svc", svc.Name, svc.Namespace) -} - func addFinalizerIfNeeded(ctx context.Context, client client.Client, obj client.Object) error { finalizer := deletionFinalizer(obj.GetObjectKind().GroupVersionKind().Kind) if obj.GetDeletionTimestamp().IsZero() && !controllerutil.ContainsFinalizer(obj, finalizer) { diff --git a/controllers/vhost_controller.go b/controllers/vhost_controller.go index 846c1189..74e7696e 100644 --- a/controllers/vhost_controller.go +++ b/controllers/vhost_controller.go @@ -43,12 +43,12 @@ func (r *VhostReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl return ctrl.Result{}, err } - rmq, svc, credsProvider, err := internal.ParseRabbitmqClusterReference(ctx, r.Client, vhost.Spec.RabbitmqClusterReference, vhost.Namespace) + credsProvider, tlsEnabled, err := internal.ParseRabbitmqClusterReference(ctx, r.Client, vhost.Spec.RabbitmqClusterReference, vhost.Namespace) if err != nil { return handleRMQReferenceParseError(ctx, r.Client, r.Recorder, vhost, &vhost.Status.Conditions, err) } - rabbitClient, err := r.RabbitmqClientFactory(rmq, svc, credsProvider, serviceDNSAddress(svc), systemCertPool) + rabbitClient, err := r.RabbitmqClientFactory(credsProvider, tlsEnabled, systemCertPool) if err != nil { logger.Error(err, failedGenerateRabbitClient) return reconcile.Result{}, err diff --git a/docs/.DS_Store b/docs/.DS_Store new file mode 100644 index 00000000..6c3736f3 Binary files /dev/null and b/docs/.DS_Store differ diff --git a/docs/api/rabbitmq.com.ref.asciidoc b/docs/api/rabbitmq.com.ref.asciidoc index 3d6ea6d2..734159ee 100644 --- a/docs/api/rabbitmq.com.ref.asciidoc +++ b/docs/api/rabbitmq.com.ref.asciidoc @@ -702,8 +702,9 @@ QueueStatus defines the observed state of Queue [cols="25a,75a", options="header"] |=== | Field | Description -| *`name`* __string__ | The name of the RabbitMQ cluster to reference. +| *`name`* __string__ | The name of the RabbitMQ cluster to reference. Have to set either name or connectionSecret, but not both. | *`namespace`* __string__ | The namespace of the RabbitMQ cluster to reference. Defaults to the namespace of the requested resource if omitted. +| *`connectionSecret`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.22/#localobjectreference-v1-core[$$LocalObjectReference$$]__ | Secret contains the http management uri for the RabbitMQ cluster. The Secret must contain the key `uri`, `username` and `password` or operator will error. Have to set either name or connectionSecret, but not both. |=== diff --git a/docs/examples/non-operator-managed-rabbitmq/README.md b/docs/examples/non-operator-managed-rabbitmq/README.md new file mode 100644 index 00000000..6ad96bd8 --- /dev/null +++ b/docs/examples/non-operator-managed-rabbitmq/README.md @@ -0,0 +1,7 @@ +## Supporting to any RabbitMQ clusters + +This is an example on how to create RabbitMQ topology objects in RabbitMQ clusters that's not Cluster Operator managed. +The example first creates a Kubernetes secret object that contains keys 'username', 'password' and 'uri'. +When creating a `queue.rabbitmq.com` resource, the secret is provided instead of a name reference +to a RabbitmqCluster. + diff --git a/docs/examples/non-operator-managed-rabbitmq/queue.yaml b/docs/examples/non-operator-managed-rabbitmq/queue.yaml new file mode 100644 index 00000000..aeb215c5 --- /dev/null +++ b/docs/examples/non-operator-managed-rabbitmq/queue.yaml @@ -0,0 +1,23 @@ +--- +apiVersion: v1 +kind: Secret +metadata: + name: my-rabbitmq-creds +type: Opaque +stringData: + username: a-user # an existing user + password: a-secure-password + uri: my.rabbit:15672 # uri for the management api +--- +apiVersion: rabbitmq.com/v1beta1 +kind: Queue +metadata: + name: qq-example +spec: + name: qq + type: quorum + autoDelete: false + durable: true + rabbitmqClusterReference: + connectionSecret: + name: my-rabbitmq-creds # provided instead of a rabbitmqcluster name; secret must contain keys username, password and uri, and be in the same namespace as the object \ No newline at end of file diff --git a/internal/cluster_reference.go b/internal/cluster_reference.go index c2b644fc..e3a7e842 100644 --- a/internal/cluster_reference.go +++ b/internal/cluster_reference.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - ctrl "sigs.k8s.io/controller-runtime" "strings" rabbitmqv1beta1 "github.com/rabbitmq/cluster-operator/api/v1beta1" @@ -14,8 +13,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . CredentialsProvider -type CredentialsProvider interface { +//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . ConnectionCredentials +type ConnectionCredentials interface { Data(key string) ([]byte, bool) } @@ -35,98 +34,156 @@ var ( ResourceNotAllowedError = errors.New("Resource is not allowed to reference defined cluster reference. Check the namespace of the resource is allowed as part of the cluster's `rabbitmq.com/topology-allowed-namespaces` annotation") ) -func ParseRabbitmqClusterReference(ctx context.Context, c client.Client, rmq topology.RabbitmqClusterReference, requestNamespace string) (*rabbitmqv1beta1.RabbitmqCluster, *corev1.Service, CredentialsProvider, error) { +func ParseRabbitmqClusterReference(ctx context.Context, c client.Client, rmq topology.RabbitmqClusterReference, requestNamespace string) (ConnectionCredentials, bool, error) { + if rmq.ConnectionSecret != nil { + secret := &corev1.Secret{} + if err := c.Get(ctx, types.NamespacedName{Namespace: requestNamespace, Name: rmq.ConnectionSecret.Name}, secret); err != nil { + return nil, false, err + } + creds, err := readCredentialsFromKubernetesSecret(secret) + if err != nil { + return nil, false, fmt.Errorf("unable to retrieve information from Kubernetes secret %s: %w", secret.Name, err) + } + return creds, false, nil + } + var namespace string if rmq.Namespace == "" { namespace = requestNamespace } else { namespace = rmq.Namespace } + cluster := &rabbitmqv1beta1.RabbitmqCluster{} if err := c.Get(ctx, types.NamespacedName{Name: rmq.Name, Namespace: namespace}, cluster); err != nil { - return nil, nil, nil, fmt.Errorf("failed to get cluster from reference: %s Error: %w", err, NoSuchRabbitmqClusterError) + return nil, false, fmt.Errorf("failed to get cluster from reference: %s Error: %w", err, NoSuchRabbitmqClusterError) } - if rmq.Namespace != "" && rmq.Namespace != requestNamespace { - var isAllowed bool - if allowedNamespaces, ok := cluster.Annotations["rabbitmq.com/topology-allowed-namespaces"]; ok { - for _, allowedNamespace := range strings.Split(allowedNamespaces, ",") { - if requestNamespace == allowedNamespace || allowedNamespace == "*" { - isAllowed = true - break - } - } - } - if !isAllowed { - return nil, nil, nil, ResourceNotAllowedError - } + if !AllowedNamespace(rmq, requestNamespace, cluster) { + return nil, false, ResourceNotAllowedError } - var credentialsProvider CredentialsProvider - svc := &corev1.Service{} + var user, pass string if cluster.Spec.SecretBackend.Vault != nil && cluster.Spec.SecretBackend.Vault.DefaultUserPath != "" { // ask the configured secure store for the credentials available at the path retrieved from the cluster resource secretStoreClient, err := SecretStoreClientProvider() if err != nil { - return nil, nil, nil, fmt.Errorf("unable to create a client connection to secret store: %w", err) + return nil, false, fmt.Errorf("unable to create a client connection to secret store: %w", err) } - credsProv, err := secretStoreClient.ReadCredentials(cluster.Spec.SecretBackend.Vault.DefaultUserPath) + user, pass, err = secretStoreClient.ReadCredentials(cluster.Spec.SecretBackend.Vault.DefaultUserPath) if err != nil { - return nil, nil, nil, fmt.Errorf("unable to retrieve credentials from secret store: %w", err) - } - - credentialsProvider = credsProv - - if err := c.Get(ctx, types.NamespacedName{Namespace: namespace, Name: cluster.ObjectMeta.Name}, svc); err != nil { - return nil, nil, nil, err + return nil, false, fmt.Errorf("unable to retrieve credentials from secret store: %w", err) } } else { // use credentials in namespace Kubernetes Secret if cluster.Status.Binding == nil { - return nil, nil, nil, errors.New("no status.binding set") + return nil, false, errors.New("no status.binding set") } if cluster.Status.DefaultUser == nil { - return nil, nil, nil, errors.New("no status.defaultUser set") + return nil, false, errors.New("no status.defaultUser set") } secret := &corev1.Secret{} if err := c.Get(ctx, types.NamespacedName{Namespace: namespace, Name: cluster.Status.Binding.Name}, secret); err != nil { - return nil, nil, nil, err + return nil, false, err } - credsProv, err := readCredentialsFromKubernetesSecret(secret) + var err error + user, pass, err = readUsernamePassword(secret) if err != nil { - return nil, nil, nil, fmt.Errorf("unable to retrieve credentials from Kubernetes secret %s: %w", secret.Name, err) + return nil, false, fmt.Errorf("unable to retrieve credentials from Kubernetes secret %s: %w", secret.Name, err) } - credentialsProvider = credsProv + } - if err := c.Get(ctx, types.NamespacedName{Namespace: namespace, Name: cluster.Status.DefaultUser.ServiceReference.Name}, svc); err != nil { - return nil, nil, nil, err - } + svc := &corev1.Service{} + if err := c.Get(ctx, types.NamespacedName{Namespace: namespace, Name: cluster.Status.DefaultUser.ServiceReference.Name}, svc); err != nil { + return nil, false, err } - return cluster, svc, credentialsProvider, nil -} -func readCredentialsFromKubernetesSecret(secret *corev1.Secret) (CredentialsProvider, error) { - if secret == nil { - return nil, errors.New("unable to extract data from nil secret") + endpoint, err := managementURI(svc) + if err != nil { + return nil, false, fmt.Errorf("failed to get endpoint from specified rabbitmqcluster: %w", err) } - logger := ctrl.LoggerFrom(nil) + return ClusterCredentials{ + data: map[string][]byte{ + "username": []byte(user), + "password": []byte(pass), + "uri": []byte(endpoint), + }, + }, cluster.TLSEnabled(), nil +} - if secret.Data["username"] == nil { - logger.Info("Kubernetes secret data contains no username value") +func AllowedNamespace(rmq topology.RabbitmqClusterReference, requestNamespace string, cluster *rabbitmqv1beta1.RabbitmqCluster) bool { + if rmq.Namespace != "" && rmq.Namespace != requestNamespace { + var isAllowed bool + if allowedNamespaces, ok := cluster.Annotations["rabbitmq.com/topology-allowed-namespaces"]; ok { + for _, allowedNamespace := range strings.Split(allowedNamespaces, ",") { + if requestNamespace == allowedNamespace || allowedNamespace == "*" { + isAllowed = true + break + } + } + } + if !isAllowed { + return false + } } + return true +} - if secret.Data["password"] == nil { - logger.Info("Kubernetes secret data contains no password value") +func readCredentialsFromKubernetesSecret(secret *corev1.Secret) (ConnectionCredentials, error) { + if secret == nil { + return nil, errors.New("unable to extract data from nil secret") } return ClusterCredentials{ data: map[string][]byte{ "username": secret.Data["username"], "password": secret.Data["password"], + "uri": secret.Data["uri"], }, }, nil } + +func readUsernamePassword(secret *corev1.Secret) (string, string, error) { + if secret == nil { + return "", "", errors.New("unable to extract data from nil secret") + } + + return string(secret.Data["username"]), string(secret.Data["password"]), nil +} + +func managementURI(svc *corev1.Service) (string, error) { + port := managementPort(svc) + if port == 0 { + return "", fmt.Errorf("failed to find 'management' or 'management-tls' from service %s", svc.Name) + } + + return fmt.Sprintf("%s:%d", serviceDNSAddress(svc), port), nil +} + +// serviceDNSAddress returns the cluster-local DNS entry associated +// with the provided Service +func serviceDNSAddress(svc *corev1.Service) string { + // NOTE: this does not use the `cluster.local` suffix, because that is not + // uniform across clusters. See the `clusterDomain` KubeletConfiguration + // value for how this can be changed for a cluster. + return fmt.Sprintf("%s.%s.svc", svc.Name, svc.Namespace) +} + +// returns RabbitMQ management port from given service +// if both "management-tls" and "management" ports are present, returns the "management-tls" port +func managementPort(svc *corev1.Service) int { + var httpPort int + for _, port := range svc.Spec.Ports { + if port.Name == "management-tls" { + return int(port.Port) + } + if port.Name == "management" { + httpPort = int(port.Port) + } + } + return httpPort +} diff --git a/internal/cluster_reference_test.go b/internal/cluster_reference_test.go index 721ebf4b..57fd63c0 100644 --- a/internal/cluster_reference_test.go +++ b/internal/cluster_reference_test.go @@ -27,7 +27,7 @@ var _ = Describe("ParseRabbitmqClusterReference", func() { existingCredentialSecret *corev1.Secret existingService *corev1.Service ctx = context.Background() - fakeCredentialsProvider *internalfakes.FakeCredentialsProvider + namespace = "rabbitmq-system" ) JustBeforeEach(func() { s := scheme.Scheme @@ -40,7 +40,7 @@ var _ = Describe("ParseRabbitmqClusterReference", func() { existingRabbitMQCluster = &rabbitmqv1beta1.RabbitmqCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "rmq", - Namespace: "rabbitmq-system", + Namespace: namespace, }, Status: rabbitmqv1beta1.RabbitmqClusterStatus{ Binding: &corev1.LocalObjectReference{ @@ -49,7 +49,7 @@ var _ = Describe("ParseRabbitmqClusterReference", func() { DefaultUser: &rabbitmqv1beta1.RabbitmqClusterDefaultUser{ ServiceReference: &rabbitmqv1beta1.RabbitmqClusterServiceReference{ Name: "rmq", - Namespace: "rabbitmq-system", + Namespace: namespace, }, }, }, @@ -57,7 +57,7 @@ var _ = Describe("ParseRabbitmqClusterReference", func() { existingCredentialSecret = &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "rmq-default-user-credentials", - Namespace: "rabbitmq-system", + Namespace: namespace, }, Data: map[string][]byte{ "username": []byte(existingRabbitMQUsername), @@ -67,7 +67,7 @@ var _ = Describe("ParseRabbitmqClusterReference", func() { existingService = &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: "rmq", - Namespace: "rabbitmq-system", + Namespace: namespace, }, Spec: corev1.ServiceSpec{ ClusterIP: "1.2.3.4", @@ -83,13 +83,10 @@ var _ = Describe("ParseRabbitmqClusterReference", func() { }) It("generates a rabbithole client which makes successful requests to the RabbitMQ Server", func() { - rmq, svc, credsProvider, err := internal.ParseRabbitmqClusterReference(ctx, fakeClient, topology.RabbitmqClusterReference{Name: existingRabbitMQCluster.Name}, existingRabbitMQCluster.Namespace) + credsProvider, tlsEnabled, err := internal.ParseRabbitmqClusterReference(ctx, fakeClient, topology.RabbitmqClusterReference{Name: existingRabbitMQCluster.Name}, existingRabbitMQCluster.Namespace) Expect(err).NotTo(HaveOccurred()) - Expect(rmq.ObjectMeta).To(Equal(existingRabbitMQCluster.ObjectMeta)) - Expect(rmq.Status).To(Equal(existingRabbitMQCluster.Status)) - Expect(svc.ObjectMeta).To(Equal(existingService.ObjectMeta)) - Expect(svc.Spec).To(Equal(existingService.Spec)) + Expect(tlsEnabled).To(BeFalse()) usernameBytes, _ := credsProvider.Data("username") passwordBytes, _ := credsProvider.Data("password") Expect(usernameBytes).To(Equal([]byte(existingRabbitMQUsername))) @@ -101,7 +98,7 @@ var _ = Describe("ParseRabbitmqClusterReference", func() { *existingRabbitMQCluster = rabbitmqv1beta1.RabbitmqCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "rmq-incomplete", - Namespace: "rabbitmq-system", + Namespace: namespace, }, Status: rabbitmqv1beta1.RabbitmqClusterStatus{ Binding: &corev1.LocalObjectReference{ @@ -112,7 +109,7 @@ var _ = Describe("ParseRabbitmqClusterReference", func() { }) It("errors", func() { - _, _, _, err := internal.ParseRabbitmqClusterReference(ctx, fakeClient, topology.RabbitmqClusterReference{Name: existingRabbitMQCluster.Name}, existingRabbitMQCluster.Namespace) + _, _, err := internal.ParseRabbitmqClusterReference(ctx, fakeClient, topology.RabbitmqClusterReference{Name: existingRabbitMQCluster.Name}, existingRabbitMQCluster.Namespace) Expect(err).To(MatchError("no status.defaultUser set")) }) }) @@ -121,14 +118,15 @@ var _ = Describe("ParseRabbitmqClusterReference", func() { var ( err error fakeSecretStoreClient *internalfakes.FakeSecretStoreClient - credsProv internal.CredentialsProvider + credsProv internal.ConnectionCredentials + tlsEnabled bool ) BeforeEach(func() { *existingRabbitMQCluster = rabbitmqv1beta1.RabbitmqCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "rmq", - Namespace: "rabbitmq-system", + Namespace: namespace, }, Status: rabbitmqv1beta1.RabbitmqClusterStatus{ Binding: &corev1.LocalObjectReference{ @@ -137,7 +135,7 @@ var _ = Describe("ParseRabbitmqClusterReference", func() { DefaultUser: &rabbitmqv1beta1.RabbitmqClusterDefaultUser{ ServiceReference: &rabbitmqv1beta1.RabbitmqClusterServiceReference{ Name: "rmq", - Namespace: "rabbitmq-system", + Namespace: namespace, }, }, }, @@ -152,12 +150,7 @@ var _ = Describe("ParseRabbitmqClusterReference", func() { } fakeSecretStoreClient = &internalfakes.FakeSecretStoreClient{} - - fakeCredentialsProvider = &internalfakes.FakeCredentialsProvider{} - fakeCredentialsProvider.DataReturnsOnCall(0, []byte(existingRabbitMQUsername), true) - fakeCredentialsProvider.DataReturnsOnCall(1, []byte(existingRabbitMQPassword), true) - - fakeSecretStoreClient.ReadCredentialsReturns(fakeCredentialsProvider, nil) + fakeSecretStoreClient.ReadCredentialsReturns(existingRabbitMQUsername, existingRabbitMQPassword, nil) internal.SecretStoreClientProvider = func() (internal.SecretStoreClient, error) { return fakeSecretStoreClient, nil } @@ -168,10 +161,11 @@ var _ = Describe("ParseRabbitmqClusterReference", func() { }) JustBeforeEach(func() { - _, _, credsProv, err = internal.ParseRabbitmqClusterReference(ctx, fakeClient, topology.RabbitmqClusterReference{Name: existingRabbitMQCluster.Name}, existingRabbitMQCluster.Namespace) + credsProv, tlsEnabled, err = internal.ParseRabbitmqClusterReference(ctx, fakeClient, topology.RabbitmqClusterReference{Name: existingRabbitMQCluster.Name}, existingRabbitMQCluster.Namespace) }) It("should not return an error", func() { + Expect(tlsEnabled).To(BeFalse()) Expect(err).ToNot(HaveOccurred()) }) @@ -183,4 +177,99 @@ var _ = Describe("ParseRabbitmqClusterReference", func() { }) }) }) + When("spec.rabbitmqClusterReference.connectionSecret is set instead of cluster name", func() { + BeforeEach(func() { + connectionSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "rmq-connection-info", + Namespace: namespace, + }, + Data: map[string][]byte{ + "uri": []byte("10.0.0.0:15671"), + "username": []byte("test-user"), + "password": []byte("test-password"), + }, + } + objs = []runtime.Object{connectionSecret} + }) + + It("returns the expected connection information", func() { + credsProvider, tlsEnabled, err := internal.ParseRabbitmqClusterReference(ctx, fakeClient, + topology.RabbitmqClusterReference{ + ConnectionSecret: &corev1.LocalObjectReference{ + Name: "rmq-connection-info", + }, + }, + namespace) + Expect(err).NotTo(HaveOccurred()) + + Expect(tlsEnabled).To(BeFalse()) + returnedUser, _ := credsProvider.Data("username") + returnedPass, _ := credsProvider.Data("password") + returnedURI, _ := credsProvider.Data("uri") + Expect(string(returnedUser)).To(Equal("test-user")) + Expect(string(returnedPass)).To(Equal("test-password")) + Expect(string(returnedURI)).To(Equal("10.0.0.0:15671")) + }) + }) +}) + +var _ = Describe("AllowedNamespace", func() { + When("rabbitmqcluster reference namespace is an empty string", func() { + It("returns true", func() { + Expect(internal.AllowedNamespace(topology.RabbitmqClusterReference{Name: "a-name"}, "", nil)).To(BeTrue()) + }) + }) + + When("rabbitmqcluster reference namespace matches requested namespace", func() { + It("returns true", func() { + Expect(internal.AllowedNamespace(topology.RabbitmqClusterReference{Name: "a-name", Namespace: "a-ns"}, "a-ns", nil)).To(BeTrue()) + }) + }) + + When("requested namespace matches topology-allowed-namespaces annotation", func() { + It("returns true", func() { + cluster := &rabbitmqv1beta1.RabbitmqCluster{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "rabbitmq.com/topology-allowed-namespaces": "test,test0,test1", + }, + }, + } + ref := topology.RabbitmqClusterReference{Name: "a-name"} + Expect(internal.AllowedNamespace(ref, "test", cluster)).To(BeTrue()) + Expect(internal.AllowedNamespace(ref, "test0", cluster)).To(BeTrue()) + Expect(internal.AllowedNamespace(ref, "test1", cluster)).To(BeTrue()) + }) + }) + + When("request namespace is not listed in topology-allowed-namespaces annotations", func() { + It("returns false", func() { + cluster := &rabbitmqv1beta1.RabbitmqCluster{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "rabbitmq.com/topology-allowed-namespaces": "test,test0,test1", + }, + }, + } + ref := topology.RabbitmqClusterReference{Name: "a-name"} + Expect(internal.AllowedNamespace(ref, "notThere", cluster)).To(BeTrue()) + }) + }) + + When("topology-allowed-namespaces is set to *", func() { + It("returns true", func() { + cluster := &rabbitmqv1beta1.RabbitmqCluster{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "rabbitmq.com/topology-allowed-namespaces": "*", + }, + }, + } + ref := topology.RabbitmqClusterReference{Name: "a-name"} + Expect(internal.AllowedNamespace(ref, "anything", cluster)).To(BeTrue()) + Expect(internal.AllowedNamespace(ref, "whatever", cluster)).To(BeTrue()) + }) + }) + }) diff --git a/internal/internalfakes/fake_credentials_provider.go b/internal/internalfakes/fake_connection_credentials.go similarity index 75% rename from internal/internalfakes/fake_credentials_provider.go rename to internal/internalfakes/fake_connection_credentials.go index 6faca2a6..8f8f7b77 100644 --- a/internal/internalfakes/fake_credentials_provider.go +++ b/internal/internalfakes/fake_connection_credentials.go @@ -7,7 +7,7 @@ import ( "github.com/rabbitmq/messaging-topology-operator/internal" ) -type FakeCredentialsProvider struct { +type FakeConnectionCredentials struct { DataStub func(string) ([]byte, bool) dataMutex sync.RWMutex dataArgsForCall []struct { @@ -25,7 +25,7 @@ type FakeCredentialsProvider struct { invocationsMutex sync.RWMutex } -func (fake *FakeCredentialsProvider) Data(arg1 string) ([]byte, bool) { +func (fake *FakeConnectionCredentials) Data(arg1 string) ([]byte, bool) { fake.dataMutex.Lock() ret, specificReturn := fake.dataReturnsOnCall[len(fake.dataArgsForCall)] fake.dataArgsForCall = append(fake.dataArgsForCall, struct { @@ -44,26 +44,26 @@ func (fake *FakeCredentialsProvider) Data(arg1 string) ([]byte, bool) { return fakeReturns.result1, fakeReturns.result2 } -func (fake *FakeCredentialsProvider) DataCallCount() int { +func (fake *FakeConnectionCredentials) DataCallCount() int { fake.dataMutex.RLock() defer fake.dataMutex.RUnlock() return len(fake.dataArgsForCall) } -func (fake *FakeCredentialsProvider) DataCalls(stub func(string) ([]byte, bool)) { +func (fake *FakeConnectionCredentials) DataCalls(stub func(string) ([]byte, bool)) { fake.dataMutex.Lock() defer fake.dataMutex.Unlock() fake.DataStub = stub } -func (fake *FakeCredentialsProvider) DataArgsForCall(i int) string { +func (fake *FakeConnectionCredentials) DataArgsForCall(i int) string { fake.dataMutex.RLock() defer fake.dataMutex.RUnlock() argsForCall := fake.dataArgsForCall[i] return argsForCall.arg1 } -func (fake *FakeCredentialsProvider) DataReturns(result1 []byte, result2 bool) { +func (fake *FakeConnectionCredentials) DataReturns(result1 []byte, result2 bool) { fake.dataMutex.Lock() defer fake.dataMutex.Unlock() fake.DataStub = nil @@ -73,7 +73,7 @@ func (fake *FakeCredentialsProvider) DataReturns(result1 []byte, result2 bool) { }{result1, result2} } -func (fake *FakeCredentialsProvider) DataReturnsOnCall(i int, result1 []byte, result2 bool) { +func (fake *FakeConnectionCredentials) DataReturnsOnCall(i int, result1 []byte, result2 bool) { fake.dataMutex.Lock() defer fake.dataMutex.Unlock() fake.DataStub = nil @@ -89,7 +89,7 @@ func (fake *FakeCredentialsProvider) DataReturnsOnCall(i int, result1 []byte, re }{result1, result2} } -func (fake *FakeCredentialsProvider) Invocations() map[string][][]interface{} { +func (fake *FakeConnectionCredentials) Invocations() map[string][][]interface{} { fake.invocationsMutex.RLock() defer fake.invocationsMutex.RUnlock() fake.dataMutex.RLock() @@ -101,7 +101,7 @@ func (fake *FakeCredentialsProvider) Invocations() map[string][][]interface{} { return copiedInvocations } -func (fake *FakeCredentialsProvider) recordInvocation(key string, args []interface{}) { +func (fake *FakeConnectionCredentials) recordInvocation(key string, args []interface{}) { fake.invocationsMutex.Lock() defer fake.invocationsMutex.Unlock() if fake.invocations == nil { @@ -113,4 +113,4 @@ func (fake *FakeCredentialsProvider) recordInvocation(key string, args []interfa fake.invocations[key] = append(fake.invocations[key], args) } -var _ internal.CredentialsProvider = new(FakeCredentialsProvider) +var _ internal.ConnectionCredentials = new(FakeConnectionCredentials) diff --git a/internal/internalfakes/fake_secret_store_client.go b/internal/internalfakes/fake_secret_store_client.go index cb403cb8..f797b0ba 100644 --- a/internal/internalfakes/fake_secret_store_client.go +++ b/internal/internalfakes/fake_secret_store_client.go @@ -8,24 +8,26 @@ import ( ) type FakeSecretStoreClient struct { - ReadCredentialsStub func(string) (internal.CredentialsProvider, error) + ReadCredentialsStub func(string) (string, string, error) readCredentialsMutex sync.RWMutex readCredentialsArgsForCall []struct { arg1 string } readCredentialsReturns struct { - result1 internal.CredentialsProvider - result2 error + result1 string + result2 string + result3 error } readCredentialsReturnsOnCall map[int]struct { - result1 internal.CredentialsProvider - result2 error + result1 string + result2 string + result3 error } invocations map[string][][]interface{} invocationsMutex sync.RWMutex } -func (fake *FakeSecretStoreClient) ReadCredentials(arg1 string) (internal.CredentialsProvider, error) { +func (fake *FakeSecretStoreClient) ReadCredentials(arg1 string) (string, string, error) { fake.readCredentialsMutex.Lock() ret, specificReturn := fake.readCredentialsReturnsOnCall[len(fake.readCredentialsArgsForCall)] fake.readCredentialsArgsForCall = append(fake.readCredentialsArgsForCall, struct { @@ -39,9 +41,9 @@ func (fake *FakeSecretStoreClient) ReadCredentials(arg1 string) (internal.Creden return stub(arg1) } if specificReturn { - return ret.result1, ret.result2 + return ret.result1, ret.result2, ret.result3 } - return fakeReturns.result1, fakeReturns.result2 + return fakeReturns.result1, fakeReturns.result2, fakeReturns.result3 } func (fake *FakeSecretStoreClient) ReadCredentialsCallCount() int { @@ -50,7 +52,7 @@ func (fake *FakeSecretStoreClient) ReadCredentialsCallCount() int { return len(fake.readCredentialsArgsForCall) } -func (fake *FakeSecretStoreClient) ReadCredentialsCalls(stub func(string) (internal.CredentialsProvider, error)) { +func (fake *FakeSecretStoreClient) ReadCredentialsCalls(stub func(string) (string, string, error)) { fake.readCredentialsMutex.Lock() defer fake.readCredentialsMutex.Unlock() fake.ReadCredentialsStub = stub @@ -63,30 +65,33 @@ func (fake *FakeSecretStoreClient) ReadCredentialsArgsForCall(i int) string { return argsForCall.arg1 } -func (fake *FakeSecretStoreClient) ReadCredentialsReturns(result1 internal.CredentialsProvider, result2 error) { +func (fake *FakeSecretStoreClient) ReadCredentialsReturns(result1 string, result2 string, result3 error) { fake.readCredentialsMutex.Lock() defer fake.readCredentialsMutex.Unlock() fake.ReadCredentialsStub = nil fake.readCredentialsReturns = struct { - result1 internal.CredentialsProvider - result2 error - }{result1, result2} + result1 string + result2 string + result3 error + }{result1, result2, result3} } -func (fake *FakeSecretStoreClient) ReadCredentialsReturnsOnCall(i int, result1 internal.CredentialsProvider, result2 error) { +func (fake *FakeSecretStoreClient) ReadCredentialsReturnsOnCall(i int, result1 string, result2 string, result3 error) { fake.readCredentialsMutex.Lock() defer fake.readCredentialsMutex.Unlock() fake.ReadCredentialsStub = nil if fake.readCredentialsReturnsOnCall == nil { fake.readCredentialsReturnsOnCall = make(map[int]struct { - result1 internal.CredentialsProvider - result2 error + result1 string + result2 string + result3 error }) } fake.readCredentialsReturnsOnCall[i] = struct { - result1 internal.CredentialsProvider - result2 error - }{result1, result2} + result1 string + result2 string + result3 error + }{result1, result2, result3} } func (fake *FakeSecretStoreClient) Invocations() map[string][][]interface{} { diff --git a/internal/rabbitmq_client_factory.go b/internal/rabbitmq_client_factory.go index 6d578b9c..5632478a 100644 --- a/internal/rabbitmq_client_factory.go +++ b/internal/rabbitmq_client_factory.go @@ -16,10 +16,7 @@ import ( "fmt" "net/http" - rabbitmqv1beta1 "github.com/rabbitmq/cluster-operator/api/v1beta1" - rabbithole "github.com/michaelklishin/rabbit-hole/v2" - corev1 "k8s.io/api/core/v1" ) //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . RabbitMQClient @@ -48,41 +45,43 @@ type RabbitMQClient interface { DeleteShovel(vhost, shovel string) (res *http.Response, err error) } -type RabbitMQClientFactory func(rmq *rabbitmqv1beta1.RabbitmqCluster, svc *corev1.Service, credsProvider CredentialsProvider, hostname string, certPool *x509.CertPool) (RabbitMQClient, error) +type RabbitMQClientFactory func(connectionCreds ConnectionCredentials, tlsEnabled bool, certPool *x509.CertPool) (RabbitMQClient, error) -var RabbitholeClientFactory RabbitMQClientFactory = func(rmq *rabbitmqv1beta1.RabbitmqCluster, svc *corev1.Service, credsProvider CredentialsProvider, hostname string, certPool *x509.CertPool) (RabbitMQClient, error) { - return generateRabbitholeClient(rmq, svc, credsProvider, hostname, certPool) +var RabbitholeClientFactory RabbitMQClientFactory = func(connectionCreds ConnectionCredentials, tlsEnabled bool, certPool *x509.CertPool) (RabbitMQClient, error) { + return generateRabbitholeClient(connectionCreds, tlsEnabled, certPool) } -// returns a http client for the given RabbitmqCluster -func generateRabbitholeClient(rmq *rabbitmqv1beta1.RabbitmqCluster, svc *corev1.Service, credsProvider CredentialsProvider, hostname string, certPool *x509.CertPool) (rabbitmqClient RabbitMQClient, err error) { - endpoint, err := managementEndpoint(rmq, svc, hostname) - if err != nil { - return nil, fmt.Errorf("failed to get endpoint from specified rabbitmqcluster: %w", err) +// generateRabbitholeClient returns a http client for a given creds +// if provided RabbitmqCluster is nil, generateRabbitholeClient uses username, passwords, and uri +// information from connectionCreds to generate a rabbit client +func generateRabbitholeClient(connectionCreds ConnectionCredentials, tlsEnabled bool, certPool *x509.CertPool) (rabbitmqClient RabbitMQClient, err error) { + defaultUser, found := connectionCreds.Data("username") + if !found { + return nil, keyMissingErr("username") } - defaultUser, found := credsProvider.Data("username") + defaultUserPass, found := connectionCreds.Data("password") if !found { - return nil, errors.New("failed to retrieve username: key username missing from credentials") + return nil, keyMissingErr("password") } - defaultUserPass, found := credsProvider.Data("password") + endpoint, found := connectionCreds.Data("uri") if !found { - return nil, errors.New("failed to retrieve password: key password missing from credentials") + return nil, keyMissingErr("uri") } - if rmq.TLSEnabled() { + if tlsEnabled { // create TLS config for https request cfg := new(tls.Config) cfg.RootCAs = certPool transport := &http.Transport{TLSClientConfig: cfg} - rabbitmqClient, err = rabbithole.NewTLSClient(endpoint, string(defaultUser), string(defaultUserPass), transport) + rabbitmqClient, err = rabbithole.NewTLSClient(fmt.Sprintf("https://%s", string(endpoint)), string(defaultUser), string(defaultUserPass), transport) if err != nil { return nil, fmt.Errorf("failed to instantiate rabbit rabbitmqClient: %v", err) } } else { - rabbitmqClient, err = rabbithole.NewClient(endpoint, string(defaultUser), string(defaultUserPass)) + rabbitmqClient, err = rabbithole.NewClient(fmt.Sprintf("http://%s", string(endpoint)), string(defaultUser), string(defaultUserPass)) if err != nil { return nil, fmt.Errorf("failed to instantiate rabbit rabbitmqClient: %v", err) } @@ -90,35 +89,6 @@ func generateRabbitholeClient(rmq *rabbitmqv1beta1.RabbitmqCluster, svc *corev1. return rabbitmqClient, nil } -func managementEndpoint(cluster *rabbitmqv1beta1.RabbitmqCluster, svc *corev1.Service, hostname string) (string, error) { - port := managementPort(svc) - if port == 0 { - return "", fmt.Errorf("failed to find 'management' or 'management-tls' from service %s", svc.Name) - } - - return fmt.Sprintf("%s://%s:%d", managementScheme(cluster), hostname, port), nil -} - -// returns RabbitMQ management scheme from given cluster -func managementScheme(cluster *rabbitmqv1beta1.RabbitmqCluster) string { - if cluster.TLSEnabled() { - return "https" - } else { - return "http" - } -} - -// returns RabbitMQ management port from given service -// if both "management-tls" and "management" ports are present, returns the "management-tls" port -func managementPort(svc *corev1.Service) int { - var httpPort int - for _, port := range svc.Spec.Ports { - if port.Name == "management-tls" { - return int(port.Port) - } - if port.Name == "management" { - httpPort = int(port.Port) - } - } - return httpPort +func keyMissingErr(key string) error { + return errors.New(fmt.Sprintf("failed to retrieve %s: key %s missing from credentials", key, key)) } diff --git a/internal/rabbitmq_client_factory_test.go b/internal/rabbitmq_client_factory_test.go index dc3abd8e..0b6fe8b5 100644 --- a/internal/rabbitmq_client_factory_test.go +++ b/internal/rabbitmq_client_factory_test.go @@ -3,6 +3,7 @@ package internal_test import ( "crypto/x509" "errors" + "fmt" "github.com/rabbitmq/messaging-topology-operator/internal/internalfakes" "io/ioutil" "net/http" @@ -20,185 +21,223 @@ import ( var _ = Describe("ParseRabbitmqClusterReference", func() { var ( - existingRabbitMQUsername = "abc123" - existingRabbitMQPassword = "foo1234" - existingRabbitMQCluster *rabbitmqv1beta1.RabbitmqCluster - fakeCredentialsProvider *internalfakes.FakeCredentialsProvider - existingService *corev1.Service - fakeRabbitMQServer *ghttp.Server - fakeRabbitMQURL *url.URL - fakeRabbitMQPort int - certPool *x509.CertPool - serverCertPath string - serverKeyPath string - caCertPath string - caCertBytes []byte + existingRabbitMQUsername = "abc123" + existingRabbitMQPassword = "foo1234" + existingRabbitMQCluster *rabbitmqv1beta1.RabbitmqCluster + FakeConnectionCredentials *internalfakes.FakeConnectionCredentials + existingService *corev1.Service + fakeRabbitMQServer *ghttp.Server + fakeRabbitMQURL *url.URL + fakeRabbitMQPort int + certPool *x509.CertPool + serverCertPath string + serverKeyPath string + caCertPath string + caCertBytes []byte ) BeforeEach(func() { certPool = x509.NewCertPool() }) - JustBeforeEach(func() { - fakeCredentialsProvider = &internalfakes.FakeCredentialsProvider{} - fakeCredentialsProvider.DataReturnsOnCall(0, []byte(existingRabbitMQUsername), true) - fakeCredentialsProvider.DataReturnsOnCall(1, []byte(existingRabbitMQPassword), true) - - fakeRabbitMQServer.RouteToHandler("PUT", "/api/users/example-user", func(w http.ResponseWriter, req *http.Request) { - user, password, ok := req.BasicAuth() - if !(ok && user == existingRabbitMQUsername && password == existingRabbitMQPassword) { - w.WriteHeader(http.StatusUnauthorized) - return - } - }) - }) AfterEach(func() { fakeRabbitMQServer.Close() }) - When("the RabbitmqCluster is configured without TLS", func() { - BeforeEach(func() { - fakeRabbitMQServer = mockRabbitMQServer() + When("RabbitmqCluster is provided", func() { + JustBeforeEach(func() { + FakeConnectionCredentials = &internalfakes.FakeConnectionCredentials{} + FakeConnectionCredentials.DataReturnsOnCall(0, []byte(existingRabbitMQUsername), true) + FakeConnectionCredentials.DataReturnsOnCall(1, []byte(existingRabbitMQPassword), true) + FakeConnectionCredentials.DataReturnsOnCall(2, []byte(fmt.Sprintf("%s:%s", + fakeRabbitMQURL.Hostname(), + fakeRabbitMQURL.Port())), true) + + fakeRabbitMQServer.RouteToHandler("PUT", "/api/users/example-user", func(w http.ResponseWriter, req *http.Request) { + user, password, ok := req.BasicAuth() + if !(ok && user == existingRabbitMQUsername && password == existingRabbitMQPassword) { + w.WriteHeader(http.StatusUnauthorized) + return + } + }) + }) + When("the RabbitmqCluster is configured without TLS", func() { + BeforeEach(func() { + fakeRabbitMQServer = mockRabbitMQServer() - var err error - fakeRabbitMQURL, fakeRabbitMQPort, err = mockRabbitMQURLPort(fakeRabbitMQServer) - Expect(err).NotTo(HaveOccurred()) + var err error + fakeRabbitMQURL, fakeRabbitMQPort, err = mockRabbitMQURLPort(fakeRabbitMQServer) + Expect(err).NotTo(HaveOccurred()) - existingRabbitMQCluster = &rabbitmqv1beta1.RabbitmqCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "rmq", - Namespace: "rabbitmq-system", - }, - Status: rabbitmqv1beta1.RabbitmqClusterStatus{ - Binding: &corev1.LocalObjectReference{ - Name: "rmq-default-user-credentials", + existingRabbitMQCluster = &rabbitmqv1beta1.RabbitmqCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "rmq", + Namespace: "rabbitmq-system", }, - DefaultUser: &rabbitmqv1beta1.RabbitmqClusterDefaultUser{ - ServiceReference: &rabbitmqv1beta1.RabbitmqClusterServiceReference{ - Name: "rmq-service", - Namespace: "rabbitmq-system", + Status: rabbitmqv1beta1.RabbitmqClusterStatus{ + Binding: &corev1.LocalObjectReference{ + Name: "rmq-default-user-credentials", + }, + DefaultUser: &rabbitmqv1beta1.RabbitmqClusterDefaultUser{ + ServiceReference: &rabbitmqv1beta1.RabbitmqClusterServiceReference{ + Name: "rmq-service", + Namespace: "rabbitmq-system", + }, }, }, - }, - } - existingService = &corev1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: "rmq-service", - Namespace: "rabbitmq-system", - }, - Spec: corev1.ServiceSpec{ - ClusterIP: fakeRabbitMQURL.Hostname(), - Ports: []corev1.ServicePort{ - { - Name: "management", - Port: int32(fakeRabbitMQPort), + } + existingService = &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "rmq-service", + Namespace: "rabbitmq-system", + }, + Spec: corev1.ServiceSpec{ + ClusterIP: fakeRabbitMQURL.Hostname(), + Ports: []corev1.ServicePort{ + { + Name: "management", + Port: int32(fakeRabbitMQPort), + }, }, }, - }, - } - }) + } + }) - It("generates a rabbithole client which makes successful requests to the RabbitMQ Server", func() { - generatedClient, err := internal.RabbitholeClientFactory(existingRabbitMQCluster, existingService, fakeCredentialsProvider, fakeRabbitMQURL.Hostname(), certPool) - Expect(err).NotTo(HaveOccurred()) - Expect(generatedClient).NotTo(BeNil()) + It("generates a rabbithole client which makes successful requests to the RabbitMQ Server", func() { + generatedClient, err := internal.RabbitholeClientFactory(FakeConnectionCredentials, false, certPool) + Expect(err).NotTo(HaveOccurred()) + Expect(generatedClient).NotTo(BeNil()) - _, err = generatedClient.PutUser("example-user", rabbithole.UserSettings{}) - Expect(err).NotTo(HaveOccurred()) - Expect(len(fakeRabbitMQServer.ReceivedRequests())).To(Equal(1)) + _, err = generatedClient.PutUser("example-user", rabbithole.UserSettings{}) + Expect(err).NotTo(HaveOccurred()) + Expect(len(fakeRabbitMQServer.ReceivedRequests())).To(Equal(1)) + }) }) - }) - When("the RabbitmqCluster is configured with TLS", func() { - BeforeEach(func() { - fakeRabbitMQServer, serverCertPath, serverKeyPath, caCertPath = mockRabbitMQTLSServer() + When("the RabbitmqCluster is configured with TLS", func() { + BeforeEach(func() { + fakeRabbitMQServer, serverCertPath, serverKeyPath, caCertPath = mockRabbitMQTLSServer() - var err error - fakeRabbitMQURL, fakeRabbitMQPort, err = mockRabbitMQURLPort(fakeRabbitMQServer) - Expect(err).NotTo(HaveOccurred()) + var err error + fakeRabbitMQURL, fakeRabbitMQPort, err = mockRabbitMQURLPort(fakeRabbitMQServer) + Expect(err).NotTo(HaveOccurred()) - certBytes, err := ioutil.ReadFile(serverCertPath) - Expect(err).NotTo(HaveOccurred()) - keyBytes, err := ioutil.ReadFile(serverKeyPath) - Expect(err).NotTo(HaveOccurred()) - caCertBytes, err = ioutil.ReadFile(caCertPath) - Expect(err).NotTo(HaveOccurred()) - existingRabbitMQCluster = &rabbitmqv1beta1.RabbitmqCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "rmq", - Namespace: "rabbitmq-system", - }, - Status: rabbitmqv1beta1.RabbitmqClusterStatus{ - Binding: &corev1.LocalObjectReference{ - Name: "rmq-default-user-credentials", + certBytes, err := ioutil.ReadFile(serverCertPath) + Expect(err).NotTo(HaveOccurred()) + keyBytes, err := ioutil.ReadFile(serverKeyPath) + Expect(err).NotTo(HaveOccurred()) + caCertBytes, err = ioutil.ReadFile(caCertPath) + Expect(err).NotTo(HaveOccurred()) + existingRabbitMQCluster = &rabbitmqv1beta1.RabbitmqCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "rmq", + Namespace: "rabbitmq-system", }, - DefaultUser: &rabbitmqv1beta1.RabbitmqClusterDefaultUser{ - ServiceReference: &rabbitmqv1beta1.RabbitmqClusterServiceReference{ - Name: "rmq-service", - Namespace: "rabbitmq-system", + Status: rabbitmqv1beta1.RabbitmqClusterStatus{ + Binding: &corev1.LocalObjectReference{ + Name: "rmq-default-user-credentials", }, + DefaultUser: &rabbitmqv1beta1.RabbitmqClusterDefaultUser{ + ServiceReference: &rabbitmqv1beta1.RabbitmqClusterServiceReference{ + Name: "rmq-service", + Namespace: "rabbitmq-system", + }, + }, + }, + } + existingService = &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "rmq-service", + Namespace: "rabbitmq-system", }, - }, - } - existingService = &corev1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: "rmq-service", - Namespace: "rabbitmq-system", - }, - Spec: corev1.ServiceSpec{ - ClusterIP: fakeRabbitMQURL.Hostname(), - Ports: []corev1.ServicePort{ - { - Name: "management", - Port: int32(fakeRabbitMQPort), + Spec: corev1.ServiceSpec{ + ClusterIP: fakeRabbitMQURL.Hostname(), + Ports: []corev1.ServicePort{ + { + Name: "management", + Port: int32(fakeRabbitMQPort), + }, }, }, - }, - } - existingCertSecret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "tls-certs", - Namespace: "rabbitmq-system", - }, - Type: corev1.SecretTypeTLS, - Data: map[string][]byte{ - corev1.TLSCertKey: certBytes, - corev1.TLSPrivateKeyKey: keyBytes, - }, - } - existingService.Spec.Ports = append(existingService.Spec.Ports, corev1.ServicePort{ - Name: "management-tls", - Port: int32(fakeRabbitMQPort), + } + existingCertSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tls-certs", + Namespace: "rabbitmq-system", + }, + Type: corev1.SecretTypeTLS, + Data: map[string][]byte{ + corev1.TLSCertKey: certBytes, + corev1.TLSPrivateKeyKey: keyBytes, + }, + } + existingService.Spec.Ports = append(existingService.Spec.Ports, corev1.ServicePort{ + Name: "management-tls", + Port: int32(fakeRabbitMQPort), + }) + existingRabbitMQCluster.Spec.TLS = rabbitmqv1beta1.TLSSpec{ + SecretName: existingCertSecret.Name, + DisableNonTLSListeners: true, + } }) - existingRabbitMQCluster.Spec.TLS = rabbitmqv1beta1.TLSSpec{ - SecretName: existingCertSecret.Name, - DisableNonTLSListeners: true, - } - }) - When("the CA that signed the certs is not trusted", func() { - It("generates a rabbithole client which fails to authenticate with the cluster", func() { - generatedClient, err := internal.RabbitholeClientFactory(existingRabbitMQCluster, existingService, fakeCredentialsProvider, fakeRabbitMQURL.Hostname(), certPool) - Expect(err).NotTo(HaveOccurred()) - Expect(generatedClient).NotTo(BeNil()) + When("the CA that signed the certs is not trusted", func() { + It("generates a rabbithole client which fails to authenticate with the cluster", func() { + generatedClient, err := internal.RabbitholeClientFactory(FakeConnectionCredentials, true, certPool) + Expect(err).NotTo(HaveOccurred()) + Expect(generatedClient).NotTo(BeNil()) - _, err = generatedClient.PutUser("example-user", rabbithole.UserSettings{}) - Expect(errors.As(err, &x509.UnknownAuthorityError{})).To(BeTrue()) + _, err = generatedClient.PutUser("example-user", rabbithole.UserSettings{}) + Expect(errors.As(err, &x509.UnknownAuthorityError{})).To(BeTrue()) + }) }) - }) - When("the CA that signed the certs is trusted", func() { - JustBeforeEach(func() { - ok := certPool.AppendCertsFromPEM(caCertBytes) - Expect(ok).To(BeTrue()) + When("the CA that signed the certs is trusted", func() { + JustBeforeEach(func() { + ok := certPool.AppendCertsFromPEM(caCertBytes) + Expect(ok).To(BeTrue()) + }) + It("generates a rabbithole client which makes successful requests to the RabbitMQ Server", func() { + generatedClient, err := internal.RabbitholeClientFactory(FakeConnectionCredentials, true, certPool) + Expect(err).NotTo(HaveOccurred()) + Expect(generatedClient).NotTo(BeNil()) + + _, err = generatedClient.PutUser("example-user", rabbithole.UserSettings{}) + Expect(err).NotTo(HaveOccurred()) + Expect(len(fakeRabbitMQServer.ReceivedRequests())).To(Equal(1)) + }) }) - It("generates a rabbithole client which makes successful requests to the RabbitMQ Server", func() { - generatedClient, err := internal.RabbitholeClientFactory(existingRabbitMQCluster, existingService, fakeCredentialsProvider, fakeRabbitMQURL.Hostname(), certPool) - Expect(err).NotTo(HaveOccurred()) - Expect(generatedClient).NotTo(BeNil()) + }) + }) - _, err = generatedClient.PutUser("example-user", rabbithole.UserSettings{}) - Expect(err).NotTo(HaveOccurred()) - Expect(len(fakeRabbitMQServer.ReceivedRequests())).To(Equal(1)) + When("RabbitmqCluster is not provided", func() { + BeforeEach(func() { + fakeRabbitMQServer = mockRabbitMQServer() + + var err error + fakeRabbitMQURL, fakeRabbitMQPort, err = mockRabbitMQURLPort(fakeRabbitMQServer) + Expect(err).NotTo(HaveOccurred()) + + FakeConnectionCredentials = &internalfakes.FakeConnectionCredentials{} + FakeConnectionCredentials.DataReturnsOnCall(0, []byte(existingRabbitMQUsername), true) + FakeConnectionCredentials.DataReturnsOnCall(1, []byte(existingRabbitMQPassword), true) + FakeConnectionCredentials.DataReturnsOnCall(2, []byte(fmt.Sprintf("%s:%d", fakeRabbitMQURL.Hostname(), fakeRabbitMQPort)), true) + + fakeRabbitMQServer.RouteToHandler("PUT", "/api/users/example-user", func(w http.ResponseWriter, req *http.Request) { + user, password, ok := req.BasicAuth() + if !(ok && user == existingRabbitMQUsername && password == existingRabbitMQPassword) { + w.WriteHeader(http.StatusUnauthorized) + return + } }) }) + + It("generates a rabbithole client which makes successful requests to the RabbitMQ Server", func() { + generatedClient, err := internal.RabbitholeClientFactory(FakeConnectionCredentials, false, certPool) + Expect(err).NotTo(HaveOccurred()) + Expect(generatedClient).NotTo(BeNil()) + + _, err = generatedClient.PutUser("example-user", rabbithole.UserSettings{}) + Expect(err).NotTo(HaveOccurred()) + Expect(len(fakeRabbitMQServer.ReceivedRequests())).To(Equal(1)) + }) }) }) diff --git a/internal/vault_reader.go b/internal/vault_reader.go index a3d54bc2..55448ad1 100644 --- a/internal/vault_reader.go +++ b/internal/vault_reader.go @@ -34,7 +34,7 @@ func (s VaultSecretReader) ReadSecret(path string) (*vault.Secret, error) { //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . SecretStoreClient type SecretStoreClient interface { - ReadCredentials(path string) (CredentialsProvider, error) + ReadCredentials(path string) (string, string, error) } type VaultClient struct { @@ -83,53 +83,48 @@ func InitializeClient() func() { } } -func (vc VaultClient) ReadCredentials(path string) (CredentialsProvider, error) { +func (vc VaultClient) ReadCredentials(path string) (string, string, error) { secret, err := vc.Reader.ReadSecret(path) if err != nil { - return nil, fmt.Errorf("unable to read Vault secret: %w", err) + return "", "", fmt.Errorf("unable to read Vault secret: %w", err) } if secret == nil { - return nil, errors.New("returned Vault secret is nil") + return "", "", errors.New("returned Vault secret is nil") } if secret != nil && secret.Warnings != nil && len(secret.Warnings) > 0 { - return nil, fmt.Errorf("warnings were returned from Vault: %v", secret.Warnings) + return "", "", fmt.Errorf("warnings were returned from Vault: %v", secret.Warnings) } if secret.Data == nil { - return nil, errors.New("returned Vault secret has a nil Data map") + return "", "", errors.New("returned Vault secret has a nil Data map") } if len(secret.Data) == 0 { - return nil, errors.New("returned Vault secret has an empty Data map") + return "", "", errors.New("returned Vault secret has an empty Data map") } if secret.Data["data"] == nil { - return nil, fmt.Errorf("returned Vault secret has a Data map that contains no value for key 'data'. Available keys are: %v", availableKeys(secret.Data)) + return "", "", fmt.Errorf("returned Vault secret has a Data map that contains no value for key 'data'. Available keys are: %v", availableKeys(secret.Data)) } data, ok := secret.Data["data"].(map[string]interface{}) if !ok { - return nil, fmt.Errorf("data type assertion failed for Vault secret of type: %T and value %#v read from path %s", secret.Data["data"], secret.Data["data"], path) + return "", "", fmt.Errorf("data type assertion failed for Vault secret of type: %T and value %#v read from path %s", secret.Data["data"], secret.Data["data"], path) } username, err := getValue("username", data) if err != nil { - return nil, fmt.Errorf("unable to get username from Vault secret: %w", err) + return "", "", fmt.Errorf("unable to get username from Vault secret: %w", err) } password, err := getValue("password", data) if err != nil { - return nil, fmt.Errorf("unable to get password from Vault secret: %w", err) + return "", "", fmt.Errorf("unable to get password from Vault secret: %w", err) } - return ClusterCredentials{ - data: map[string][]byte{ - "username": []byte(username), - "password": []byte(password), - }, - }, nil + return username, password, nil } func getValue(key string, data map[string]interface{}) (string, error) { diff --git a/internal/vault_reader_test.go b/internal/vault_reader_test.go index 7e9262f1..fbacd33e 100644 --- a/internal/vault_reader_test.go +++ b/internal/vault_reader_test.go @@ -15,7 +15,7 @@ import ( var _ = Describe("VaultReader", func() { var ( err error - credsProvider internal.CredentialsProvider + username, password string secretStoreClient internal.SecretStoreClient fakeSecretReader *internalfakes.FakeSecretReader credsData map[string]interface{} @@ -40,15 +40,12 @@ var _ = Describe("VaultReader", func() { }) JustBeforeEach(func() { - credsProvider, err = secretStoreClient.ReadCredentials("some/path") + username, password, err = secretStoreClient.ReadCredentials("some/path") }) It("should return a credentials provider", func() { - Expect(credsProvider).NotTo(BeNil()) - usernameBytes, _ := credsProvider.Data("username") - passwordBytes, _ := credsProvider.Data("password") - Expect(usernameBytes).To(Equal([]byte(existingRabbitMQUsername))) - Expect(passwordBytes).To(Equal([]byte(existingRabbitMQPassword))) + Expect(username).To(Equal(existingRabbitMQUsername)) + Expect(password).To(Equal(existingRabbitMQPassword)) }) It("should not error", func() { @@ -65,11 +62,12 @@ var _ = Describe("VaultReader", func() { }) JustBeforeEach(func() { - credsProvider, err = secretStoreClient.ReadCredentials("some/path") + username, password, err = secretStoreClient.ReadCredentials("some/path") }) - It("should return a nil credentials provider", func() { - Expect(credsProvider).To(BeNil()) + It("should return empty strings for username and password", func() { + Expect(username).To(BeEmpty()) + Expect(password).To(BeEmpty()) }) It("should have returned an error", func() { @@ -86,11 +84,12 @@ var _ = Describe("VaultReader", func() { }) JustBeforeEach(func() { - credsProvider, err = secretStoreClient.ReadCredentials("some/path") + username, password, err = secretStoreClient.ReadCredentials("some/path") }) - It("should return a nil credentials provider", func() { - Expect(credsProvider).To(BeNil()) + It("should return empty strings for username and password", func() { + Expect(username).To(BeEmpty()) + Expect(password).To(BeEmpty()) }) It("should have returned an error", func() { @@ -108,11 +107,12 @@ var _ = Describe("VaultReader", func() { }) JustBeforeEach(func() { - credsProvider, err = secretStoreClient.ReadCredentials("some/path") + username, password, err = secretStoreClient.ReadCredentials("some/path") }) - It("should return a nil credentials provider", func() { - Expect(credsProvider).To(BeNil()) + It("should return empty strings for username and password", func() { + Expect(username).To(BeEmpty()) + Expect(password).To(BeEmpty()) }) It("should have returned an error", func() { @@ -131,11 +131,12 @@ var _ = Describe("VaultReader", func() { }) JustBeforeEach(func() { - credsProvider, err = secretStoreClient.ReadCredentials("some/path") + username, password, err = secretStoreClient.ReadCredentials("some/path") }) - It("should return a nil credentials provider", func() { - Expect(credsProvider).To(BeNil()) + It("should return empty strings for username and password", func() { + Expect(username).To(BeEmpty()) + Expect(password).To(BeEmpty()) }) It("should have returned an error", func() { @@ -154,11 +155,12 @@ var _ = Describe("VaultReader", func() { }) JustBeforeEach(func() { - credsProvider, err = secretStoreClient.ReadCredentials("some/path") + username, password, err = secretStoreClient.ReadCredentials("some/path") }) - It("should return a nil credentials provider", func() { - Expect(credsProvider).To(BeNil()) + It("should return empty strings for username and password", func() { + Expect(username).To(BeEmpty()) + Expect(password).To(BeEmpty()) }) It("should have returned an error", func() { @@ -179,11 +181,12 @@ var _ = Describe("VaultReader", func() { }) JustBeforeEach(func() { - credsProvider, err = secretStoreClient.ReadCredentials("some/path") + username, password, err = secretStoreClient.ReadCredentials("some/path") }) - It("should return a nil credentials provider", func() { - Expect(credsProvider).To(BeNil()) + It("should return empty strings for username and password", func() { + Expect(username).To(BeEmpty()) + Expect(password).To(BeEmpty()) }) It("should have returned an error", func() { @@ -204,11 +207,12 @@ var _ = Describe("VaultReader", func() { }) JustBeforeEach(func() { - credsProvider, err = secretStoreClient.ReadCredentials("some/path") + username, password, err = secretStoreClient.ReadCredentials("some/path") }) - It("should return a nil credentials provider", func() { - Expect(credsProvider).To(BeNil()) + It("should return empty strings for username and password", func() { + Expect(username).To(BeEmpty()) + Expect(password).To(BeEmpty()) }) It("should have returned an error", func() { @@ -225,11 +229,12 @@ var _ = Describe("VaultReader", func() { }) JustBeforeEach(func() { - credsProvider, err = secretStoreClient.ReadCredentials("some/path") + username, password, err = secretStoreClient.ReadCredentials("some/path") }) - It("should return a nil credentials provider", func() { - Expect(credsProvider).To(BeNil()) + It("should return empty strings for username and password", func() { + Expect(username).To(BeEmpty()) + Expect(password).To(BeEmpty()) }) It("should have returned an error", func() { @@ -246,11 +251,12 @@ var _ = Describe("VaultReader", func() { }) JustBeforeEach(func() { - credsProvider, err = secretStoreClient.ReadCredentials("some/path") + username, password, err = secretStoreClient.ReadCredentials("some/path") }) - It("should return a nil credentials provider", func() { - Expect(credsProvider).To(BeNil()) + It("should return empty strings for username and password", func() { + Expect(username).To(BeEmpty()) + Expect(password).To(BeEmpty()) }) It("should have returned an error", func() { @@ -272,11 +278,12 @@ var _ = Describe("VaultReader", func() { }) JustBeforeEach(func() { - credsProvider, err = secretStoreClient.ReadCredentials("some/path") + username, password, err = secretStoreClient.ReadCredentials("some/path") }) - It("should return a nil credentials provider", func() { - Expect(credsProvider).To(BeNil()) + It("should return empty strings for username and password", func() { + Expect(username).To(BeEmpty()) + Expect(password).To(BeEmpty()) }) It("should have returned an error", func() { diff --git a/system_tests/rabbitmq_connection_test.go b/system_tests/rabbitmq_connection_test.go new file mode 100644 index 00000000..dfa3ceee --- /dev/null +++ b/system_tests/rabbitmq_connection_test.go @@ -0,0 +1,80 @@ +package system_tests + +import ( + "context" + rabbithole "github.com/michaelklishin/rabbit-hole/v2" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + topology "github.com/rabbitmq/messaging-topology-operator/api/v1beta1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +var _ = Describe("RabbitMQ connection using provided connection secret", func() { + var ( + namespace = MustHaveEnv("NAMESPACE") + ctx = context.Background() + q *topology.Queue + secret *corev1.Secret + ) + + BeforeEach(func() { + endpoint, err := managementURI(ctx, clientSet, rmq.Namespace, rmq.Name) + Expect(err).NotTo(HaveOccurred(), "failed to get management uri") + user, pass, err := getUsernameAndPassword(ctx, clientSet, rmq.Namespace, rmq.Name) + Expect(err).NotTo(HaveOccurred(), "failed to get user and pass") + + secret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "uri-secret", + Namespace: namespace, + }, + StringData: map[string]string{ + "username": user, + "password": pass, + "uri": endpoint, + }, + } + Expect(k8sClient.Create(ctx, secret, &client.CreateOptions{})).To(Succeed()) + }) + + AfterEach(func() { + Expect(k8sClient.Delete(ctx, secret)).To(Succeed()) + }) + + It("succeeds creating an object in a RabbitMQ cluster configured with connection URI", func() { + By("declaring queue") + q = &topology.Queue{ + ObjectMeta: metav1.ObjectMeta{ + Name: "connection-test", + Namespace: namespace, + }, + Spec: topology.QueueSpec{ + Name: "connection-test", + RabbitmqClusterReference: topology.RabbitmqClusterReference{ + ConnectionSecret: &corev1.LocalObjectReference{Name: secret.Name}, + }, + }, + } + Expect(k8sClient.Create(ctx, q, &client.CreateOptions{})).To(Succeed()) + var qInfo *rabbithole.DetailedQueueInfo + Eventually(func() error { + var err error + qInfo, err = rabbitClient.GetQueue("/", q.Name) + return err + }, 10, 2).Should(BeNil()) + + Expect(qInfo.Name).To(Equal(q.Name)) + Expect(qInfo.Vhost).To(Equal("/")) + + By("deleting queue") + Expect(k8sClient.Delete(ctx, q)).To(Succeed()) + var err error + Eventually(func() error { + _, err = rabbitClient.GetQueue(q.Spec.Vhost, q.Name) + return err + }, 30).Should(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("Object Not Found")) + }) +}) diff --git a/system_tests/utils.go b/system_tests/utils.go index 85e074f7..19f07e41 100644 --- a/system_tests/utils.go +++ b/system_tests/utils.go @@ -108,6 +108,14 @@ func getUsernameAndPassword(ctx context.Context, clientSet *kubernetes.Clientset } func managementEndpoint(ctx context.Context, clientSet *kubernetes.Clientset, namespace, name string) (string, error) { + uri, err := managementURI(ctx, clientSet, namespace, name) + if err != nil { + return "", err + } + return fmt.Sprintf("http://%s", uri), nil +} + +func managementURI(ctx context.Context, clientSet *kubernetes.Clientset, namespace, name string) (string, error) { nodeIp := kubernetesNodeIp(ctx, clientSet) if nodeIp == "" { return "", errors.New("failed to get kubernetes Node IP") @@ -118,7 +126,7 @@ func managementEndpoint(ctx context.Context, clientSet *kubernetes.Clientset, na return "", errors.New("failed to get NodePort for management") } - return fmt.Sprintf("http://%s:%s", nodeIp, nodePort), nil + return fmt.Sprintf("%s:%s", nodeIp, nodePort), nil } func managementNodePort(ctx context.Context, clientSet *kubernetes.Clientset, namespace, name string) string {