-
Notifications
You must be signed in to change notification settings - Fork 488
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
Support cluster-scoped resources when using watchNamespaces #611
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
2a9db19
to
72632e0
Compare
the behaviour of charts is (almost) correct, when we are running kong in the cluster we should not require cluster role, the problem is that chart is not disable watching the ingressClass in that case |
@jasiek-zywczak while the controller should indeed disable that lookup when disabling the controller, the plan is to continue with this change (we haven't received objections from the stakeholders we reached out to), which should resolve your issue, correct? The install would have permissions for IngressClass, and the attempts to list them would not fail. Did you have any concerns with this approach? |
It is a matter of being consequent. Why disabling access to cluster plugins then? I mean, in case watch namespace is set? |
When watchNamespaces is non-empty, create a ClusterRole that contains cluster-scoped resources only. Remove endpoints from the cluster resource template. They appear to have been included by mistake. Configure one of the CI values to use watchNamespaces.
72632e0
to
950f5a0
Compare
950f5a0
to
f2de383
Compare
That is a good catch--this change should indeed remove the KongClusterPlugin disable as well. I've updated this to do so and document the new behavior.
That will be the case for namespaced resources. The library we use to implement watchNamespaces has two modes: it can add cluster-level watches for everything or create separate watches for each namespace and a cluster-level watch for cluster-scoped resources only. It's not possible to watch cluster-scoped resources at anything other than the cluster level, so trying to impose a namespace-restricted model on them doesn't make sense. |
What this PR does / why we need it:
When watchNamespaces is non-empty, create a ClusterRole that contains
cluster-scoped resources only.
Remove endpoints from the cluster resource template. They appear to have
been included by mistake.
Configure one of the CI values to use watchNamespaces.
Which issue this PR fixes
(optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close that issue when PR gets merged)When watchNamespaces was originally developed, the only cluster-scoped resource we had was KongClusterPlugin, and KongPlugin could substitute for it with an acceptable loss of functionality. Since, CNCF has introduced IngressClass and GatewayClass, which have no namespace-scoped equivalent. We need to read these to properly implement the v1 Ingress and vAny Gateway specs.
Special notes for your reviewer:
Ready for review, but do not merge pending discussion with product and field team stakeholders in KUBE-61.
Checklist
[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]
main
branch.