fix(helm): resolve RBAC permissions for namespaced gateway sources #5578

u-kai · 2025-06-27T02:20:34Z

What does it do ?

This PR resolves RBAC permission issues when using Gateway API sources with namespaced: true configuration in the Helm chart.
It implements proper conditional RBAC creation that supports both same-namespace and cross-namespace gateway access scenarios while maintaining backward compatibility.

Motivation

Fixes #5300 - Gateway API sources require ClusterRole permissions when using namespaced: true, but the current implementation creates insufficient Role permissions, causing external-dns to fail with RBAC errors.

Problem: When namespaced: true is set with gateway sources, external-dns needs:

Namespace informer access (ClusterRole for namespaces resource)
Gateway resource access (varies based on gatewayNamespace configuration)

Root Cause: The namespace informer uses NamespacesFromSelector functionality which requires cluster-wide namespace access, but namespaced: true only creates Role permissions.

Solution

Implements Split RBAC approach with conditional logic:

Scenarios Supported:

namespaced=false + gateway sources → ClusterRole with all permissions
namespaced=true + gateway sources + no gatewayNamespace → Main Role (with gateway permissions) + ClusterRole for namespaces
namespaced=true + gateway sources + gatewayNamespace specified → Main Role + ClusterRole for namespaces + Cross-namespace Gateway Role
namespaced=false/true + no gateway sources → Standard behavior (unchanged)

More

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

k8s-ci-robot · 2025-06-27T02:20:44Z

Hi @u-kai. 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.

ivankatliarchuk

Not sure if this is the correct solution. Why gateway have a distinct flag, when rest of the sources rely on --namespace ? Is there is a specific reason or a mistake?

ivankatliarchuk · 2025-06-27T06:07:11Z

charts/external-dns/templates/deployment.yaml

            - --namespace={{ .Release.Namespace }}
            {{- end }}
+            {{- if .Values.gatewayNamespace }}
+            - --gateway-namespace={{ .Values.gatewayNamespace }}


I think one of the issues is, that Gateway actually uses --gateway-namespace when it should be unified and --namespace is just enough. This is just a confusion.

u-kai · 2025-06-27T22:23:55Z

@ivankatliarchuk
Thanks for the feedback!

The separate --gateway-namespace flag aligns with Gateway API's design for cross-namespace routing, where Gateways (often managed by cluster operators) and Routes (managed by app teams) typically reside in different namespaces.

The flag already exists in the external-dns, so this change maintains consistency with the existing CLI interface while fixing the RBAC permissions for namespaced deployments.

ivankatliarchuk · 2025-06-28T16:23:58Z

/ok-to-test

ivankatliarchuk · 2025-06-28T16:28:58Z

I found an initial PR #2292

As short term it seems like a fix. But not clear

Long term solutions are

Watch all or multiple namespaces --namespace should accept a list of values #5263
Per source configurations Consider adding per source configurations #5397

In my opinion, if --namespace is set, only resouces in this exact namespace should be watched. If there are cross-namespace resource references (it should be ignored with a message that the resource is outside of current namespace scope), it should be either all namespaces or we need to add multiple namespace support.

ivankatliarchuk · 2025-06-28T16:36:59Z

Related issues:

"failed to sync *v1.Namespace: context deadline exceeded" with gateway-httproute source #3537
failed to sync *v1alpha3.Gateway: context deadline exceeded #4901
It could be that we need to review documentation as well

u-kai · 2025-06-29T03:28:41Z

@ivankatliarchuk
I agree that supporting multiple namespaces with the --namespace option would be a valuable improvement in the long term.

That said, I’d like to clarify one point — does this proposal also imply deprecating or integrating the --gateway-namespace option?

If so, my personal opinion is that --gateway-namespace still has value and should remain.
When supporting multiple namespaces, we’ll likely need to create one informer per namespace. Without a --gateway-namespace option, we wouldn't know which namespace the Gateway actually exists in, which could result in unnecessary informer overhead, especially when the Gateway is deployed in a single, shared namespace (as is often the case in common Gateway setups).

In such cases, having a dedicated --gateway-namespace provides a useful optimization and avoids wasteful resource watching.

So in conclusion, I’m in favor of enabling multi-namespace support for --namespace, but I think retaining --gateway-namespace is also beneficial from a performance and configuration clarity standpoint.

ivankatliarchuk · 2025-06-30T06:11:16Z

@mloiseleur wdyt?

mloiseleur · 2025-07-03T15:40:52Z

In External DNS doc on flags, it says:

--namespace: Limit resources queried for endpoints to a specific namespace (default: all namespaces)
--gateway-namespace: Limit Gateways of Route endpoints to a specific namespace (default: all namespaces)

With current state of External DNS, this PR looks valid to me. It allows user to use the same namespace or different namespace, with similar names between the binary and the chart. For instance, a user may want to use external dns CRD on external-dns namespace and Gateway on gateway namespaces.

@ivankatliarchuk An answer to your idea would be to implement a --namespaces options, allowing to set multiple namespaces. Then it would make sense to remove specific namespace options tailored for a specific source. But that's clearly beyond the scope of this PR.

mloiseleur

@u-kai About the implementation, why are you adding a specific ClusterRole and ClusterRoleBinding ? Wouldn't it be simpler to use the same CR & CRB with just extended required permissions ?

ivankatliarchuk · 2025-07-03T19:03:08Z

Make sense

u-kai · 2025-07-04T13:19:25Z

@mloiseleur
Thank you for the feedback!

Let me explain the implementation. For namespaced: true configurations, multiple Role/ClusterRole resources are necessary for the following reasons:

Technical Requirements:

Base Permissions - When namespaced: true, typically a Role (not ClusterRole) is created for namespace-scoped permissions
Namespace Informer - When using Gateway API sources like HTTPRoute, cluster-wide namespace read access is required for the NamespacesFromSelector functionality
Principle of Least Privilege - We should only grant permissions when actually needed:

namespaced: true → base permissions via Role
Gateway API sources → additional ClusterRole for namespace access only when used

Cross-namespace Gateway - In practice, Gateways often exist in different namespaces than Routes (e.g., infrastructure namespace vs application namespace). When users desire this setup (e.g., gatewayNamespace: default), a separate Role is needed for cross-namespace access

Implementation Approach:

Base permissions: Role (when namespaced)
Namespace access: ClusterRole (only when Gateway sources are used)
Cross-namespace Gateway: Role in target namespace (only when specified)

This design grants exactly the permissions needed for each scenario while maintaining security isolation.

mloiseleur · 2025-07-04T15:14:09Z

/assign @stevehipwell
for review

stevehipwell

Thanks for the PR @u-kai. I've added a comment suggesting an improvement but as I'd like to include this in the next release we can leave that for the next time we need to make changes.

/approve

stevehipwell · 2025-07-14T08:41:06Z

charts/external-dns/templates/_helpers.tpl

+Check if any Gateway API sources are enabled
+*/}}
+{{- define "external-dns.hasGatewaySources" -}}
+{{- if or (has "gateway-httproute" .Values.sources) (has "gateway-grpcroute" .Values.sources) (has "gateway-tlsroute" .Values.sources) (has "gateway-tcproute" .Values.sources) (has "gateway-udproute" .Values.sources) -}}


Why not use hasPrefix in a range loop so the code is less likely to need updating in the future?

k8s-ci-robot · 2025-07-14T08:45:23Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: stevehipwell

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:

~~charts/OWNERS~~ [stevehipwell]

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

stevehipwell · 2025-07-14T08:45:59Z

@mloiseleur @ivankatliarchuk could one of you please add the LGTM if you're happy with this?

ivankatliarchuk

/lgtm

…ubernetes-sigs#5578) * fix(helm): resolve RBAC permissions for namespaced gateway sources * feat(helm): add support for gateway namespace in RBAC configuration * chore(helm): update docs and fix formatting issues * fix(helm): revert README changes and add gatewayNamespace docs * chore lint fmt

u-kai added 2 commits June 27, 2025 11:13

fix(helm): resolve RBAC permissions for namespaced gateway sources

c87a737

feat(helm): add support for gateway namespace in RBAC configuration

0d2a702

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. chart labels Jun 27, 2025

k8s-ci-robot requested review from mloiseleur and stevehipwell June 27, 2025 02:20

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jun 27, 2025

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jun 27, 2025

ivankatliarchuk reviewed Jun 27, 2025

View reviewed changes

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jun 28, 2025

chore(helm): update docs and fix formatting issues

6589ee3

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 29, 2025

fix(helm): revert README changes and add gatewayNamespace docs

c35607a

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jun 29, 2025

chore lint fmt

fdc1843

mloiseleur reviewed Jul 3, 2025

View reviewed changes

k8s-ci-robot assigned stevehipwell Jul 4, 2025

stevehipwell approved these changes Jul 14, 2025

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 14, 2025

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 14, 2025

ivankatliarchuk reviewed Jul 14, 2025

View reviewed changes

k8s-ci-robot assigned ivankatliarchuk Jul 14, 2025

stevehipwell mentioned this pull request Jul 14, 2025

chore: release chart for v0.18.0 #5633

Merged

3 tasks

k8s-ci-robot merged commit a270a32 into kubernetes-sigs:master Jul 14, 2025
15 checks passed

fix(helm): resolve RBAC permissions for namespaced gateway sources #5578

fix(helm): resolve RBAC permissions for namespaced gateway sources #5578

Uh oh!

Conversation

u-kai commented Jun 27, 2025

What does it do ?

Motivation

Solution

Scenarios Supported:

More

Uh oh!

k8s-ci-robot commented Jun 27, 2025

Uh oh!

ivankatliarchuk left a comment

Choose a reason for hiding this comment

Uh oh!

ivankatliarchuk Jun 27, 2025

Choose a reason for hiding this comment

Uh oh!

u-kai commented Jun 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ivankatliarchuk commented Jun 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ivankatliarchuk commented Jun 28, 2025

Uh oh!

ivankatliarchuk commented Jun 28, 2025

Uh oh!

u-kai commented Jun 29, 2025

Uh oh!

ivankatliarchuk commented Jun 30, 2025

Uh oh!

mloiseleur commented Jul 3, 2025

Uh oh!

mloiseleur left a comment

Choose a reason for hiding this comment

Uh oh!

ivankatliarchuk commented Jul 3, 2025

Uh oh!

u-kai commented Jul 4, 2025

Uh oh!

mloiseleur commented Jul 4, 2025

Uh oh!

stevehipwell left a comment

Choose a reason for hiding this comment

Uh oh!

stevehipwell Jul 14, 2025

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Jul 14, 2025

Uh oh!

stevehipwell commented Jul 14, 2025

Uh oh!

ivankatliarchuk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

u-kai commented Jun 27, 2025 •

edited

Loading

ivankatliarchuk commented Jun 28, 2025 •

edited

Loading