Skip to content

Commit

Permalink
Disallow updates of Queue arguments
Browse files Browse the repository at this point in the history 
Queue arguments can't be updated in RabbitMQ. The webhook now disallows
updates on queue arguments, matching the behaviour of RabbitMQ.

Signed-off-by: Aitor Perez Cedres <[email protected]>
  • Loading branch information
@Zerpet
Zerpet committed May 27, 2024
1 parent f6f3b41 commit 6d89906
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 25 deletions.
37 changes: 18 additions & 19 deletions Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,23 +51,27 @@ $(KUBEBUILDER_ASSETS):

.PHONY: unit-tests
unit-tests::install-tools ## Run unit tests
unit-test::$(KUBEBUILDER_ASSETS)
unit-test::generate
unit-test::fmt
unit-test::vet
unit-test::vuln
unit-test::manifests
unit-test::just-unit-tests
unit-tests::$(KUBEBUILDER_ASSETS)
unit-tests::generate
unit-tests::fmt
unit-tests::vet
unit-tests::manifests
unit-tests::just-unit-tests

.PHONY: just-unit-tests
just-unit-tests:
ginkgo -r --randomize-all api/ internal/ rabbitmqclient/

.PHONY: integration-tests
integration-tests: install-tools $(KUBEBUILDER_ASSETS) generate fmt vet manifests ## Run integration tests. Use GINKGO_EXTRA="-some-arg" to append arguments to 'ginkgo run'
ginkgo -r --randomize-all -p $(GINKGO_EXTRA) controllers/

just-integration-tests: $(KUBEBUILDER_ASSETS) vet
integration-tests::install-tools ## Run integration tests. Use GINKGO_EXTRA="-some-arg" to append arguments to 'ginkgo run'
integration-tests::$(KUBEBUILDER_ASSETS)
integration-tests::generate
integration-tests::fmt
integration-tests::vet
integration-tests::manifests
integration-tests::just-integration-tests

just-integration-tests: $(KUBEBUILDER_ASSETS)
ginkgo --randomize-all -r -p $(GINKGO_EXTRA) controllers/

local-tests: unit-tests integration-tests ## Run all local tests (unit & integration)
Expand Down Expand Up @@ -169,7 +173,9 @@ ifndef DOCKER_REGISTRY_SECRET
$(error DOCKER_REGISTRY_SECRET is undefined: Name of Kubernetes secret in which to store the Docker registry username and password)
endif

docker-build-dev: check-env-docker-repo git-commit-sha
GIT_COMMIT=$(shell git rev-parse --short HEAD)-dev

docker-build-dev: check-env-docker-repo
$(BUILD_KIT) buildx build --build-arg=GIT_COMMIT=$(GIT_COMMIT) -t $(DOCKER_REGISTRY_SERVER)/$(OPERATOR_IMAGE):$(GIT_COMMIT) .
$(BUILD_KIT) push $(DOCKER_REGISTRY_SERVER)/$(OPERATOR_IMAGE):$(GIT_COMMIT)

Expand All @@ -178,13 +184,6 @@ docker-registry-secret: check-env-docker-credentials operator-namespace
@kubectl -n $(K8S_OPERATOR_NAMESPACE) create secret docker-registry $(DOCKER_REGISTRY_SECRET) --docker-server='$(DOCKER_REGISTRY_SERVER)' --docker-username="$$DOCKER_REGISTRY_USERNAME" --docker-password="$$DOCKER_REGISTRY_PASSWORD" || true
@kubectl -n $(K8S_OPERATOR_NAMESPACE) patch serviceaccount messaging-topology-operator -p '{"imagePullSecrets": [{"name": "$(DOCKER_REGISTRY_SECRET)"}]}'

git-commit-sha:
ifeq ("", git diff --stat)
GIT_COMMIT=$(shell git rev-parse --short HEAD)
else
GIT_COMMIT=$(shell git rev-parse --short HEAD)-
endif

check-env-registry-server:
ifndef DOCKER_REGISTRY_SERVER
$(error DOCKER_REGISTRY_SERVER is undefined: URL of docker registry containing the Operator image (e.g. registry.my-company.com))
Expand Down
54 changes: 48 additions & 6 deletions api/v1beta1/queue_webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package v1beta1

import (
"context"
"encoding/json"
"fmt"
"maps"

apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
Expand All @@ -23,8 +25,8 @@ func (q *Queue) SetupWebhookWithManager(mgr ctrl.Manager) error {

var _ webhook.CustomValidator = &Queue{}

// 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
// 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(_ context.Context, obj runtime.Object) (warnings admission.Warnings, err error) {
inQueue, ok := obj.(*Queue)
if !ok {
Expand All @@ -38,10 +40,11 @@ func (q *Queue) ValidateCreate(_ context.Context, obj runtime.Object) (warnings
return nil, q.Spec.RabbitmqClusterReference.validate(inQueue.RabbitReference())
}

// 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: queue name/vhost/rabbitmqClusterReference
// returns error type 'invalid' for updates that will be rejected by rabbitmq server: queue types/autoDelete/durable
// queue arguments not handled because implementation couldn't change
// 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: queue name/vhost/rabbitmqClusterReference
//
// Returns error type 'invalid' for updates that will be rejected by rabbitmq server: queue types/autoDelete/durable
func (q *Queue) ValidateUpdate(_ context.Context, oldObj, newObj runtime.Object) (warnings admission.Warnings, err error) {
oldQueue, ok := oldObj.(*Queue)
if !ok {
Expand Down Expand Up @@ -94,10 +97,49 @@ func (q *Queue) ValidateUpdate(_ context.Context, oldObj, newObj runtime.Object)
))
}

if oldQueue.Spec.Arguments != nil && newQueue.Spec.Arguments == nil {
allErrs = append(allErrs, field.Invalid(
field.NewPath("spec", "arguments"),
newQueue.Spec.Arguments,
"queue arguments cannot be updated",
))
}

if oldQueue.Spec.Arguments == nil && newQueue.Spec.Arguments != nil {
allErrs = append(allErrs, field.Invalid(
field.NewPath("spec", "arguments"),
newQueue.Spec.Arguments,
"queue arguments cannot be updated",
))
}

if oldQueue.Spec.Arguments != nil && newQueue.Spec.Arguments != nil {
previousArgs := make(map[string]any)
err := json.Unmarshal(oldQueue.Spec.Arguments.Raw, &previousArgs)
if err != nil {
return nil, apierrors.NewInternalError(fmt.Errorf("error unmarshalling previous Queue arguments: %w", err))
}

updatedArgs := make(map[string]any)
err = json.Unmarshal(newQueue.Spec.Arguments.Raw, &updatedArgs)
if err != nil {
return nil, apierrors.NewInternalError(fmt.Errorf("error unmarshalling current Queue arguments: %w", err))
}

if !maps.Equal(previousArgs, updatedArgs) {
allErrs = append(allErrs, field.Invalid(
field.NewPath("spec", "arguments"),
newQueue.Spec.Arguments,
"queue arguments cannot be updated",
))
}
}

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

//goland:noinspection GoDfaNilDereference
return nil, allErrs.ToAggregate()
}

Expand Down
24 changes: 24 additions & 0 deletions api/v1beta1/queue_webhook_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
. "github.com/onsi/gomega"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
)

var _ = Describe("queue webhook", func() {
Expand All @@ -23,6 +24,7 @@ var _ = Describe("queue webhook", func() {
AutoDelete: true,
DeleteIfEmpty: true,
DeleteIfUnused: false,
Arguments: &runtime.RawExtension{Raw: []byte(`{"this-argument": "cannot be updated"}`)},
RabbitmqClusterReference: RabbitmqClusterReference{
Name: "some-cluster",
},
Expand Down Expand Up @@ -130,5 +132,27 @@ var _ = Describe("queue webhook", func() {
_, err := newQueue.ValidateUpdate(rootCtx, &queue, newQueue)
Expect(err).To(MatchError(ContainSubstring("autoDelete cannot be updated")))
})

It("does not allow updates on queue arguments", func() {
By("not allowing value changes, nor adding new values")
newQueue := queue.DeepCopy()
newQueue.Spec.Arguments = &runtime.RawExtension{
Raw: []byte(`{"this-argument": "really cant be updated", "adding-args": "not possible"}`),
}
_, err := newQueue.ValidateUpdate(rootCtx, &queue, newQueue)
Expect(err).To(MatchError(ContainSubstring("queue arguments cannot be updated")))

By("now allowing removal")
newQueue.Spec.Arguments = nil
_, err = newQueue.ValidateUpdate(rootCtx, &queue, newQueue)
Expect(err).To(MatchError(ContainSubstring("queue arguments cannot be updated")))

By("not allowing emptying the arguments")
newQueue.Spec.Arguments = &runtime.RawExtension{
Raw: []byte(`{}`),
}
_, err = newQueue.ValidateUpdate(rootCtx, &queue, newQueue)
Expect(err).To(MatchError(ContainSubstring("queue arguments cannot be updated")))
})
})
})

0 comments on commit 6d89906

Please sign in to comment.