fix(charts): Skip cluster-scope RBAC on namespaced

[TobyTheHutt]

What does it do ?

• Restore tenant-friendly installs by keeping RBAC namespaced if .Values.gatewayNamespace is set
• namespaced=true and gatewayNamespace set results in a namespaced Role and Rolebinding for listing namespaces
• namespaced=true AND gatewayNamespace unset will retain the original ClusterRole and ClusterRoleBinding along with a Role and RoleBinding
• namespace=false will retain the original ClusterRole and ClusterRoleBinding
• No breaking changes introduced

The Helm Linter and Tests were run and the templates get rendered as expected:

• helm template charts/external-dns --set namespaced=true --set sources[0]=gateway-httproute --set gatewayNamespace=gateway-ns
• helm template charts/external-dns --set namespaced=true --set sources[0]=gateway-httproute
• helm template charts/external-dns --set namespaced=false --set sources[0]=gateway-httproute

Motivation

When installing with namespaced=true and gateway sources, a270a32 added a ClusterRole/ClusterRoleBinding which breaks tenant setups that disallow cluster-scoped permissions.

This change keeps RBAC namespaced when .Values.gatewayNamespace is set.

Fixes: #5832

More

• Yes, this PR title follows Conventional Commits
• Yes, I added unit tests
• Yes, I updated end user documentation accordingly

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign stevehipwell for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• charts/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[k8s-ci-robot]

Hi @TobyTheHutt. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[AndrewCharlesHay]

@TobyTheHutt

Thank you for addressing the RBAC scoping issue! The changes look well thought-out and restore tenant-friendly installs by correctly handling namespaced RBAC when .Values.gatewayNamespace is set. I appreciate the clear documentation and the test cases you provided.

A few points:

The logic for retaining ClusterRole/ClusterRoleBinding vs. Role/RoleBinding is clearly explained and matches expected behaviors for tenant scenarios.
Good job updating both unit tests and user documentation.
I verified the Helm template commands you listed and the rendered resources look correct.
Questions/Comments:

Are there any edge cases where setting both namespaced=true and leaving gatewayNamespace unset could lead to unexpected permission escalation?
Could you clarify if this change impacts upgrades for existing users who move from cluster-scoped to namespaced installs?
Overall, LGTM—thanks for the fix! Just want to make sure we’re covering all upgrade and edge scenarios.

[TobyTheHutt]

@AndrewCharlesHay

Thanks for the review. To your questions:

Permission escalation risk:
I see no new risks introduced regarding this.
The only cluster-scoped permission granted is ["get","watch","list"] on namespaces. This did not change and behaves exactly as before.

Upgrade path
Users which are already running namespaced with gatewayNamespace set will now lose the previous cluster RBAC during upgrade. Helm will prune the *-nemaspaces ClusterRole and ClusterRoleBinding, as they are no longer rendered.
This is documented in:

https://github.com/kubernetes-sigs/external-dns/pull/5843/files#diff-c1cb383aca0c29f8ccafa3ef682fdee07fc3b068d461a4e64617b66d480d842fR61-R65

...and...

https://github.com/kubernetes-sigs/external-dns/pull/5843/files#diff-c1cb383aca0c29f8ccafa3ef682fdee07fc3b068d461a4e64617b66d480d842fR115

If a user moves from cluster-scoped to namespaced, it still flips the main RBAC objects from ClusterRole and ClusterRoleBinding to Role and RoleBinding, as this is treated as a usual kind swap in Helm. It will delete the cluster-scoped pair and replace them with namespaced equivalents.

BUT: Users need to remember to set gatewayNamespace during the same upgrade, if they want to avoid retaining the namespace-listing ClusterRole.

Next steps

I think the changes implemented are safe and any issues that may arise from them can be remedied with simple and obvious measures.
However, if desired, I could:

• Add a chart warning regarding the upgrade limitations
• Add a note in the docs, hinting at the described upgrade limitations
• Add a test asserting the kind change

Let me know how we want to proceed.

[mloiseleur]

/ok-to-test

[stevehipwell]

Thanks for the PR @TobyTheHutt. I've commented on a minor nit to fix and could you also add this change to the chart CHANGELOG?

[stevehipwell]

Suggested change

	# When `namespaced=true`, setting this value avoids creating any cluster-scoped RBAC
	# (no ClusterRole/ClusterRoleBinding) for Gateway sources.
	# When `namespaced=true`, setting this value avoids creating any cluster-scoped RBAC
	# (no ClusterRole/ClusterRoleBinding) for Gateway sources.