-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
[helm] Add support for namespaced scope #3403
Conversation
This approach makes sense given the constraints. I'll review properly on a big screen in the morning. |
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.
This looks good, but I have a couple of questions.
I add a section about namespace scoped installation. All sources that depends against namespace scoped resources are working in namespaced scoped installation. Istio is a bit tricky here, because depends against 2 namespace scoped resources, but it would only work if both are in the same namespace. I started with an quick table/support matrix, but I stop it. Most of the CRDs are namespaced scope, like RouteGroups, Openshift Routes, VirtualServices, getambassador/ingress... I guess the RBAC for nodes can be still gated, since the Node permissions are only needed, if the annotation I only test ingress objects, for the sources, I looked into the source code. Based on that, i have no idea, why |
/ok-to-test |
/cc @njuettner (your name was on the last PR for the namespace feature) |
Any updates on this? When can we merge it? |
@Raffo do you have any thoughts on this? It looks good to me and it'd make sense to get merged now and in the release I'm about to do if you're happy? |
/label tide/merge-method-squash |
@stevehipwell this works for me, I don't have too many opinions on how the helm chart should be configured. |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jkroepke, Raffo The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Description
Classical service providers may not grant you admin permissions on our own kubernetes cluster. This changes introduce a namespaced modes, where external-dns runs namespace scoped.
Fixes #ISSUE
Checklist
i though about create a dedicated role.yaml file first, but then I was the disadvantage of maintain to roles inside the helm chart.