Namespace access

[njbarber]

This closes rabbitmq/messaging-topology-operator#147

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

Summary Of Changes

Effectively just adds a new messagingTopologyNamespaces field.

Additional Context

The field is used by the messaging topology operator when creating resources to determine if they have access to given cluster.

Related to rabbitmq/messaging-topology-operator#201.

Local Testing

Please ensure you run the unit, integration and system tests before approving the PR.

To run the unit and integration tests:

$ make unit-tests integration-tests

You will need to target a k8s cluster and have the operator deployed for running the system tests.

For example, for a Kubernetes context named dev-bunny:

$ kubectx dev-bunny
$ make destroy deploy-dev
# wait for operator to be deployed
$ make system-tests

[michaelklishin]

I find this to be fairly confusing to those who do not have any background on this. That said, I don't have a much better wording to offer. To me this suggests it may be a difficult feature to explain (or a highly unusual one?)

[michaelklishin]

So while previously any namespace could contribute topology definitions, now all of them have to be explicitly whitelisted.
This sounds like a significant change in terms of usability.

[Zerpet]

Hello @njbarber,

Thanks for contributing to this project! I'm afraid we can't accept the PR with the current API bump, as it does not adhere to our compatibility commitments, documented here. Specifically, this change does not comply with rule 4a of Kubernetes API deprecation policy. This link can be found in our compatibility commitments, shared in the first link.

Rule #4a: Other than the most recent API versions in each track, older API versions must be supported after their announced deprecation for a duration of no less than:

• GA: 12 months or 3 releases (whichever is longer)
• Beta: 9 months or 3 releases (whichever is longer)
• Alpha: 0 releases

Would you share the reasoning to bump the API version as part of this PR?

I'm happy to explore potential solutions to the problem described in rabbitmq/messaging-topology-operator#147. Given that this issue involves multiple repos, let's move the conversation about how to tackle this problem to the GitHub issue.

Any conversation related to the code in this PR, I'm happy to keep it here.

[njbarber]

@michaelklishin

So while previously any namespace could contribute topology definitions, now all of them have to be explicitly whitelisted.
This sounds like a significant change in terms of usability.

Not quite. Only if a cluster is planning on accepting messaging topology resources from another namespace does it need to define messagingTopologyNamespaces. Additionally if a messaging topology resource defines a rabbitmqClusterReference without a namespace the operator assumes the cluster is in the same namespace as the resource (which is identical to the previous behavior).

[njbarber]

@Zerpet my understanding is that spec changes require an API version bump. I'd love to know if that's not necessarily always the case (as I had to do a lot of find and replace that wasn't particularly fun).

[Zerpet]

@Zerpet my understanding is that spec changes require an API version bump. I'd love to know if that's not necessarily always the case (as I had to do a lot of find and replace that wasn't particularly fun).

If you add an optional field to the spec, you don't need to bump the API version. Adding an optional field keeps the API compatible with previous versions, since older clients will still work, even if they don't send the new field in the request.

We refer to this guide on API changes, from SIG Architecture.

[njbarber]

@Zerpet

If you add an optional field to the spec, you don't need to bump the API version.

I did not know that.

In that case I believe both sets of changes do not require a version bump.

https://github.com/njbarber/messaging-topology-operator/blob/namespace-access/internal/cluster_reference.go#L32-L43 will never be executed for existing messaging topology resources that have a cluster reference, for instance (meaning messagingTopologyNamespaces isn't required on the cluster spec).

[njbarber]

This is no longer relevant due to recent changes on rabbitmq/messaging-topology-operator#201.