Skip to content

Commit

Permalink
Bump controller runtime to k8s api 1.27
Browse files Browse the repository at this point in the history 
- controller runtime v0.15 contains breaking changes where
Validator interfaces have been modified to include warning
in return types: kubernetes-sigs/controller-runtime#2014
other breaking changes do not affect us
  • Loading branch information
@ChunyiLyu
ChunyiLyu committed May 31, 2023
1 parent 3860450 commit de677e2
Show file tree
Hide file tree
Showing 41 changed files with 377 additions and 830 deletions.
23 changes: 12 additions & 11 deletions api/v1alpha1/superstream_webhook.go
View file
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
View file
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
View file
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
View file
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
View file
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
}

0 comments on commit de677e2

Please sign in to comment.