Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Bump controller runtime and k8s api 1.27 #628

Merged
merged 2 commits into from
May 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 12 additions & 11 deletions api/v1alpha1/superstream_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"k8s.io/apimachinery/pkg/util/validation/field"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/webhook"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
)

func (s *SuperStream) SetupWebhookWithManager(mgr ctrl.Manager) error {
Expand All @@ -30,48 +31,48 @@ var _ webhook.Validator = &SuperStream{}

// 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 {
func (s *SuperStream) ValidateCreate() (admission.Warnings, error) {
return s.Spec.RabbitmqClusterReference.ValidateOnCreate(s.GroupResource(), s.Name)
}

// ValidateUpdate returns error type 'forbidden' for updates on superstream name, vhost and rabbitmqClusterReference
func (s *SuperStream) ValidateUpdate(old runtime.Object) error {
func (s *SuperStream) ValidateUpdate(old runtime.Object) (admission.Warnings, error) {
oldSuperStream, ok := old.(*SuperStream)
if !ok {
return apierrors.NewBadRequest(fmt.Sprintf("expected a superstream but got a %T", old))
return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a superstream but got a %T", old))
}

detailMsg := "updates on name, vhost and rabbitmqClusterReference are all forbidden"
if s.Spec.Name != oldSuperStream.Spec.Name {
return apierrors.NewForbidden(s.GroupResource(), s.Name,
return nil, apierrors.NewForbidden(s.GroupResource(), s.Name,
field.Forbidden(field.NewPath("spec", "name"), detailMsg))
}
if s.Spec.Vhost != oldSuperStream.Spec.Vhost {
return apierrors.NewForbidden(s.GroupResource(), s.Name,
return nil, apierrors.NewForbidden(s.GroupResource(), s.Name,
field.Forbidden(field.NewPath("spec", "vhost"), detailMsg))
}

if !oldSuperStream.Spec.RabbitmqClusterReference.Matches(&s.Spec.RabbitmqClusterReference) {
return apierrors.NewForbidden(s.GroupResource(), s.Name,
return nil, apierrors.NewForbidden(s.GroupResource(), s.Name,
field.Forbidden(field.NewPath("spec", "rabbitmqClusterReference"), detailMsg))
}

if !routingKeyUpdatePermitted(oldSuperStream.Spec.RoutingKeys, s.Spec.RoutingKeys) {
return apierrors.NewForbidden(s.GroupResource(), s.Name,
return nil, apierrors.NewForbidden(s.GroupResource(), s.Name,
field.Forbidden(field.NewPath("spec", "routingKeys"), "updates may only add to the existing list of routing keys"))
}

if s.Spec.Partitions < oldSuperStream.Spec.Partitions {
return apierrors.NewForbidden(s.GroupResource(), s.Name,
return nil, apierrors.NewForbidden(s.GroupResource(), s.Name,
field.Forbidden(field.NewPath("spec", "partitions"), "updates may only increase the partition count, and may not decrease it"))
}

return nil
return nil, nil
}

// ValidateDelete no validation on delete
func (s *SuperStream) ValidateDelete() error {
return nil
func (s *SuperStream) ValidateDelete() (admission.Warnings, error) {
return nil, nil
}

// routingKeyUpdatePermitted allows updates only if adding additional keys at the end of the list of keys
Expand Down
33 changes: 22 additions & 11 deletions api/v1alpha1/superstream_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,73 +31,84 @@ var _ = Describe("superstream webhook", 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())
_, err := notAllowed.ValidateCreate()
Expect(apierrors.IsForbidden(err)).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())
_, err := notAllowed.ValidateCreate()
Expect(apierrors.IsForbidden(err)).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())
_, err := newSuperStream.ValidateUpdate(&superstream)
Expect(apierrors.IsForbidden(err)).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())
_, err := newSuperStream.ValidateUpdate(&superstream)
Expect(apierrors.IsForbidden(err)).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())
_, err := newSuperStream.ValidateUpdate(&superstream)
Expect(apierrors.IsForbidden(err)).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())
_, err := newSuperStream.ValidateUpdate(&superstream)
Expect(apierrors.IsForbidden(err)).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())
_, err := newSuperStream.ValidateUpdate(&superstream)
Expect(apierrors.IsForbidden(err)).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())
_, err := newSuperStream.ValidateUpdate(&superstream)
Expect(err).NotTo(HaveOccurred())
})

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())
_, err := newSuperStream.ValidateUpdate(&superstream)
Expect(apierrors.IsForbidden(err)).To(BeTrue())
})

It("allows superstream.spec.partitions to be increased", func() {
newSuperStream := superstream.DeepCopy()
newSuperStream.Spec.Partitions = 1000
Expect(newSuperStream.ValidateUpdate(&superstream)).To(Succeed())
_, err := newSuperStream.ValidateUpdate(&superstream)
Expect(err).NotTo(HaveOccurred())
})

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())
_, err := newSuperStream.ValidateUpdate(&superstream)
Expect(apierrors.IsForbidden(err)).To(BeTrue())
})
})
})
19 changes: 10 additions & 9 deletions api/v1beta1/binding_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"reflect"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/webhook"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
)

func (b *Binding) SetupWebhookWithManager(mgr ctrl.Manager) error {
Expand All @@ -22,27 +23,27 @@ var _ webhook.Validator = &Binding{}

// 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 {
func (b *Binding) ValidateCreate() (admission.Warnings, error) {
return b.Spec.RabbitmqClusterReference.ValidateOnCreate(b.GroupResource(), b.Name)
}

// ValidateUpdate updates on vhost and rabbitmqClusterReference are forbidden
func (b *Binding) ValidateUpdate(old runtime.Object) error {
func (b *Binding) ValidateUpdate(old runtime.Object) (admission.Warnings, error) {
oldBinding, ok := old.(*Binding)
if !ok {
return apierrors.NewBadRequest(fmt.Sprintf("expected a binding but got a %T", old))
return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a binding but got a %T", old))
}

var allErrs field.ErrorList
detailMsg := "updates on vhost and rabbitmqClusterReference are all forbidden"

if b.Spec.Vhost != oldBinding.Spec.Vhost {
return apierrors.NewForbidden(b.GroupResource(), b.Name,
return nil, apierrors.NewForbidden(b.GroupResource(), b.Name,
field.Forbidden(field.NewPath("spec", "vhost"), detailMsg))
}

if !oldBinding.Spec.RabbitmqClusterReference.Matches(&b.Spec.RabbitmqClusterReference) {
return apierrors.NewForbidden(b.GroupResource(), b.Name,
return nil, apierrors.NewForbidden(b.GroupResource(), b.Name,
field.Forbidden(field.NewPath("spec", "rabbitmqClusterReference"), detailMsg))
}

Expand Down Expand Up @@ -87,12 +88,12 @@ func (b *Binding) ValidateUpdate(old runtime.Object) error {
}

if len(allErrs) == 0 {
return nil
return nil, nil
}

return apierrors.NewInvalid(GroupVersion.WithKind("Binding").GroupKind(), b.Name, allErrs)
return nil, apierrors.NewInvalid(GroupVersion.WithKind("Binding").GroupKind(), b.Name, allErrs)
}

func (b *Binding) ValidateDelete() error {
return nil
func (b *Binding) ValidateDelete() (admission.Warnings, error) {
return nil, nil
}
21 changes: 10 additions & 11 deletions api/v1beta1/binding_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,30 +30,30 @@ var _ = Describe("Binding webhook", 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())
Expect(apierrors.IsForbidden(ignoreNilWarning(notAllowed.ValidateCreate()))).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())
Expect(apierrors.IsForbidden(ignoreNilWarning(notAllowed.ValidateCreate()))).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())
Expect(apierrors.IsForbidden(ignoreNilWarning(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())
Expect(apierrors.IsForbidden(ignoreNilWarning(newBinding.ValidateUpdate(&oldBinding)))).To(BeTrue())
})

It("does not allow updates on rabbitmqClusterReference.connectionSecret", func() {
Expand All @@ -71,38 +71,37 @@ var _ = Describe("Binding webhook", func() {
}
new := connectionScr.DeepCopy()
new.Spec.RabbitmqClusterReference.ConnectionSecret.Name = "new-secret"
Expect(apierrors.IsForbidden(new.ValidateUpdate(&connectionScr))).To(BeTrue())
Expect(apierrors.IsForbidden(ignoreNilWarning(new.ValidateUpdate(&connectionScr)))).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())
Expect(apierrors.IsInvalid(ignoreNilWarning(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())
Expect(apierrors.IsInvalid(ignoreNilWarning(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())
Expect(apierrors.IsInvalid(ignoreNilWarning(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())
Expect(apierrors.IsInvalid(ignoreNilWarning(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())
Expect(apierrors.IsInvalid(ignoreNilWarning(newBinding.ValidateUpdate(&oldBinding)))).To(BeTrue())
})
})

})
21 changes: 11 additions & 10 deletions api/v1beta1/exchange_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"k8s.io/apimachinery/pkg/util/validation/field"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/webhook"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
)

func (r *Exchange) SetupWebhookWithManager(mgr ctrl.Manager) error {
Expand All @@ -21,34 +22,34 @@ var _ webhook.Validator = &Exchange{}

// 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 {
func (e *Exchange) ValidateCreate() (admission.Warnings, error) {
return e.Spec.RabbitmqClusterReference.ValidateOnCreate(e.GroupResource(), e.Name)
}

// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
// returns error type 'forbidden' for updates that the controller chooses to disallow: exchange name/vhost/rabbitmqClusterReference
// returns error type 'invalid' for updates that will be rejected by rabbitmq server: exchange types/autoDelete/durable
// exchange.spec.arguments can be updated
func (e *Exchange) ValidateUpdate(old runtime.Object) error {
func (e *Exchange) ValidateUpdate(old runtime.Object) (admission.Warnings, error) {
oldExchange, ok := old.(*Exchange)
if !ok {
return apierrors.NewBadRequest(fmt.Sprintf("expected an exchange but got a %T", old))
return nil, apierrors.NewBadRequest(fmt.Sprintf("expected an exchange but got a %T", old))
}

var allErrs field.ErrorList
detailMsg := "updates on name, vhost, and rabbitmqClusterReference are all forbidden"
if e.Spec.Name != oldExchange.Spec.Name {
return apierrors.NewForbidden(e.GroupResource(), e.Name,
return nil, apierrors.NewForbidden(e.GroupResource(), e.Name,
field.Forbidden(field.NewPath("spec", "name"), detailMsg))
}

if e.Spec.Vhost != oldExchange.Spec.Vhost {
return apierrors.NewForbidden(e.GroupResource(), e.Name,
return nil, apierrors.NewForbidden(e.GroupResource(), e.Name,
field.Forbidden(field.NewPath("spec", "vhost"), detailMsg))
}

if !oldExchange.Spec.RabbitmqClusterReference.Matches(&e.Spec.RabbitmqClusterReference) {
return apierrors.NewForbidden(e.GroupResource(), e.Name,
return nil, apierrors.NewForbidden(e.GroupResource(), e.Name,
field.Forbidden(field.NewPath("spec", "rabbitmqClusterReference"), detailMsg))
}

Expand Down Expand Up @@ -77,12 +78,12 @@ func (e *Exchange) ValidateUpdate(old runtime.Object) error {
}

if len(allErrs) == 0 {
return nil
return nil, nil
}

return apierrors.NewInvalid(GroupVersion.WithKind("Exchange").GroupKind(), e.Name, allErrs)
return nil, apierrors.NewInvalid(GroupVersion.WithKind("Exchange").GroupKind(), e.Name, allErrs)
}

func (e *Exchange) ValidateDelete() error {
return nil
func (e *Exchange) ValidateDelete() (admission.Warnings, error) {
return nil, nil
}
Loading
Loading