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

validation webhook for queues.rabbitmq.com and exchanges.rabbitmq.com #74

Merged
merged 2 commits into from
Mar 18, 2021

Conversation

ChunyiLyu
Copy link
Contributor

Note to reviewers: remember to look at the commits in this PR and consider if they can be squashed

Summary Of Changes

  • validation webhook for queues.rabbitmq.com and exchanges.rabbitmq.com
  • validation webhook only checks for updates in the future. we should consider input validation for create in the future as well.
  • for queues, you get: 1) error type 'forbidden' for updates that the controller chooses to disallow: queue name/vhost/rabbitmqClusterReference; 2) error type 'invalid' for updates that will be rejected by rabbitmq server: queue types/autoDelete/durable. I chose to ignore queue.spec.arguments for now, since we might want to create policies under the hood in the future.
  • for exchanges, you get: 1) error type 'forbidden' for updates that the controller chooses to disallow: queue name/vhost/rabbitmqClusterReference; 2) error type 'invalid' for updates that will be rejected by rabbitmq server: exchange types/autoDelete/durable; updates on exchanges arguments are accepted by the server

Additional Context

Test coverage in unit and system tests.

- not allowing updates on queue.spec, except spec.arguments
- we might change how controller handles queue.arguments in
the future
Copy link
Member

@MirahImage MirahImage left a comment

Choose a reason for hiding this comment

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

Tried it out, works as expected

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants