UPSTREAM: <carry>: OCPBUGS-90506: replace kube-rbac-proxy with native metrics auth by Thealisyed · Pull Request #200 · openshift/external-dns

Thealisyed · 2026-08-13T10:13:54Z

Summary

Replaces the kube-rbac-proxy sidecar with inline TokenReview/SAR authentication served directly by the external-dns binary
Adds --metrics-tls-cert-dir flag (directory containing tls.crt and tls.key provisioned by service-ca); defaults to empty string (plain HTTP) for backward compatibility
Implements authMiddleware using k8s.io/client-go directly — zero new dependencies
/healthz remains unauthenticated for kubelet liveness/readiness probes
Operator side (removing the sidecar injection, updating Service/ServiceMonitor ports) tracked in openshift/external-dns-operator by @rfredette

Note:

Operator-side PR (removing kube-rbac-proxy sidecar, updating Service/ServiceMonitor) by @rfredette: must coordinate port changes before merging

openshift-ci-robot · 2026-08-13T10:14:01Z

@Thealisyed: This pull request references Jira Issue OCPBUGS-90506, which is invalid:

expected the bug to target the "5.0.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Summary

Replaces the kube-rbac-proxy sidecar with inline TokenReview/SAR authentication served directly by the external-dns binary

Adds --metrics-tls-cert-dir flag (directory containing tls.crt and tls.key provisioned by service-ca); defaults to empty string (plain HTTP) for backward compatibility

Implements authMiddleware using k8s.io/client-go directly — zero new dependencies

/healthz remains unauthenticated for kubelet liveness/readiness probes

Operator side (removing the sidecar injection, updating Service/ServiceMonitor ports) tracked in openshift/external-dns-operator by @rfredette

Why not filters.WithAuthenticationAndAuthorization from controller-runtime?

That function imports k8s.io/apiserver which is not in external-dns's dependency tree. The inline client-go implementation is functionally identical with no new transitive deps.

Dependencies

Operator-side PR (removing kube-rbac-proxy sidecar, updating Service/ServiceMonitor) by @rfredette — must coordinate port changes before merging

Test plan

Unit tests for authMiddleware covering: no header, non-bearer scheme, TokenReview API error, unauthenticated token, SAR API error, SAR denied, and authorized pass-through

Unit test verifying correct user/groups/NonResourceAttributes are forwarded to SAR

E2E: deploy with --metrics-tls-cert-dir set, confirm Prometheus scrapes successfully

E2E: deploy without flag, confirm plain HTTP metrics still work (backward compat)

Fixes: https://issues.redhat.com/browse/OCPBUGS-90506

🤖 Generated with Claude Code

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 openshift-eng/jira-lifecycle-plugin repository.

openshift-ci · 2026-08-13T10:14:22Z

[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 alebedev87 for approval. For more information see the Code Review Process.

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

Details

Needs approval from an approver in each of these files:

DOWNSTREAM_OWNERS

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

Thealisyed · 2026-08-18T10:06:52Z

/jira refresh

openshift-ci-robot · 2026-08-18T10:06:56Z

@Thealisyed: This pull request references Jira Issue OCPBUGS-90506, which is invalid:

expected the bug to target either version "5.1.0." or "openshift-5.1.0.", but it targets "5.0.0" instead

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

/jira refresh

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 openshift-eng/jira-lifecycle-plugin repository.

Thealisyed · 2026-08-18T10:07:18Z

/jira refresh

openshift-ci-robot · 2026-08-18T10:07:25Z

@Thealisyed: This pull request references Jira Issue OCPBUGS-90506, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (5.1.0) matches configured target version for branch (5.1.0)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @melvinjoseph86

Details

In response to this:

/jira refresh

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 openshift-eng/jira-lifecycle-plugin repository.

… metrics auth Replace the kube-rbac-proxy sidecar with inline TokenReview/SAR authentication served directly by the external-dns binary. Adds --metrics-tls-cert-dir flag (default empty = plain HTTP, backward compatible) and authMiddleware using k8s.io/client-go only — no new dependencies. Assisted with Claude.

openshift-ci · 2026-08-18T11:16:03Z

@Thealisyed: all tests passed!

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

Expose ExternalDNS operand metrics to cluster Prometheus without using a kube-rbac-proxy sidecar. The operand now serves its own metrics over HTTPS with Kubernetes TokenReview/SAR auth via --metrics-tls-cert-dir (added in openshift/external-dns#200). - Add --metrics-tls-cert-dir arg and service-ca cert volume mount to each ExternalDNS container - Change --metrics-address from 127.0.0.1 to 0.0.0.0 so metrics are reachable outside the pod - Create a Service with serving-cert annotation for auto TLS - Create a ServiceMonitor for Prometheus discovery (HTTPS, port 7979+) - Add tokenreviews/subjectaccessreviews RBAC for the operand service account Depends on openshift/external-dns#200 for the operand-side auth layer. Assisted with Claude.

alebedev87 · 2026-08-19T12:38:21Z

/assign

alebedev87

The main approach LGTM, some remarks about minimizing the impact of the carrying the patch. Also, about the commit message and PR title - kube-rbac-proxy doesn't figure in external-dns context, it's something we add in the operator. Something like add secure metrics serving with TokenReview/SAR authentication would be more fitting into the context.

alebedev87 · 2026-08-19T12:58:43Z

+	server := &http.Server{Addr: address, Handler: newMetricsMux(kubeClient, tlsCertDir)}
+	log.Fatal(server.ListenAndServeTLS(
+		filepath.Join(tlsCertDir, "tls.crt"),
+		filepath.Join(tlsCertDir, "tls.key"),
+	))


This means both healthz and metrics endpoints will be over HTTPS for EDO. Can this be handled in the operator logic? Is simple switching scheme for the probes be enough?

alebedev87 · 2026-08-19T13:06:02Z

 	ctx, cancel := context.WithCancel(context.Background())

-	go serveMetrics(cfg.MetricsAddress)
+	go serveMetrics(cfg.MetricsAddress, cfg.MetricsTLSCertDir, cfg.KubeConfig, cfg.APIServerURL)


I'm thinking about how we can ease the task of carrying this patch for the future rebases. WDYT about this approach?

Suggested change

go serveMetrics(cfg.MetricsAddress, cfg.MetricsTLSCertDir, cfg.KubeConfig, cfg.APIServerURL)

if cfg.MetricsTLSCertDir != "" {

go serveSecureMetrics(cfg)

} else {

go serveMetrics(cfg.MetricsAddress)

}

Then serveSecureMetrics() can be moved into a dedicated file (e.g. metrics_openshift.go) and it handles the creation of kubeclient, new mux server and changing the metrics handler do the token review and authorization.

This way 1) main.go changes will remain very minimal, 2) all new code (including imported packages) is in a dedicated file which is easy to carry, 3) new unit tests are in a dedicated new file too, easy to carry and recongnise (openshift in the name).

alebedev87 · 2026-08-19T13:12:39Z

+	return mux
+}
+
+func authMiddleware(kubeClient kubernetes.Interface, next http.Handler) http.Handler {


authMiddleware seems a little vague, wdyt about something like this?

Suggested change

func authMiddleware(kubeClient kubernetes.Interface, next http.Handler) http.Handler {

func withMetricsAuth(kubeClient kubernetes.Interface, next http.Handler) http.Handler {

openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 13, 2026

openshift-ci Bot requested review from grzpiotrowski and rikatz August 13, 2026 10:14

Thealisyed force-pushed the OCPBUGS-90506-native-metrics-auth branch from cb3ef99 to 84286dd Compare August 13, 2026 10:18

openshift-ci-robot added jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. and removed jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Aug 18, 2026

openshift-ci Bot requested a review from melvinjoseph86 August 18, 2026 10:07

Thealisyed changed the title ~~[WIP] UPSTREAM: <carry>: OCPBUGS-90506: replace kube-rbac-proxy with native metrics auth~~ UPSTREAM: <carry>: OCPBUGS-90506: replace kube-rbac-proxy with native metrics auth Aug 18, 2026

openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 18, 2026

Thealisyed force-pushed the OCPBUGS-90506-native-metrics-auth branch from 84286dd to 729bf5d Compare August 18, 2026 11:02

Thealisyed mentioned this pull request Aug 18, 2026

OCPBUGS-58102: Expose ExternalDNS operand metrics via kube-rbac-proxy sidecar openshift/external-dns-operator#371

Open

openshift-ci Bot assigned alebedev87 Aug 19, 2026

alebedev87 reviewed Aug 19, 2026

View reviewed changes

-	go serveMetrics(cfg.MetricsAddress, cfg.MetricsTLSCertDir, cfg.KubeConfig, cfg.APIServerURL)
+  if cfg.MetricsTLSCertDir != "" {
+      go serveSecureMetrics(cfg)
+  } else {
+      go serveMetrics(cfg.MetricsAddress)
+  }

	func authMiddleware(kubeClient kubernetes.Interface, next http.Handler) http.Handler {
	func withMetricsAuth(kubeClient kubernetes.Interface, next http.Handler) http.Handler {

Uh oh!

Conversation

Thealisyed commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Note:

Uh oh!

openshift-ci-robot commented Aug 13, 2026

Summary

Why not filters.WithAuthenticationAndAuthorization from controller-runtime?

Dependencies

Test plan

Uh oh!

openshift-ci Bot commented Aug 13, 2026

Uh oh!

Thealisyed commented Aug 18, 2026

Uh oh!

openshift-ci-robot commented Aug 18, 2026

Uh oh!

Thealisyed commented Aug 18, 2026

Uh oh!

openshift-ci-robot commented Aug 18, 2026

Uh oh!

openshift-ci Bot commented Aug 18, 2026

Uh oh!

alebedev87 commented Aug 19, 2026

Uh oh!

alebedev87 left a comment

Choose a reason for hiding this comment

Uh oh!

alebedev87 Aug 19, 2026

Choose a reason for hiding this comment

Uh oh!

alebedev87 Aug 19, 2026

Choose a reason for hiding this comment

Uh oh!

alebedev87 Aug 19, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Thealisyed commented Aug 13, 2026 •

edited

Loading

Why not `filters.WithAuthenticationAndAuthorization` from controller-runtime?