Skip to content

UPSTREAM: <carry>: OCPBUGS-90506: add secure metrics serving with TokenReview/SAR authentication - #200

Merged
openshift-merge-bot[bot] merged 1 commit into
openshift:masterfrom
Thealisyed:OCPBUGS-90506-native-metrics-auth
Aug 26, 2026
Merged

UPSTREAM: <carry>: OCPBUGS-90506: add secure metrics serving with TokenReview/SAR authentication#200
openshift-merge-bot[bot] merged 1 commit into
openshift:masterfrom
Thealisyed:OCPBUGS-90506-native-metrics-auth

Conversation

@Thealisyed

@Thealisyed Thealisyed commented Aug 13, 2026

Copy link
Copy Markdown

Summary

  • 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 withMetricsAuth using k8s.io/client-go directly — zero new dependencies
  • All new code lives in metrics_openshift.go / metrics_openshift_test.go to minimise the carry patch diff in main.go
  • /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 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-robot openshift-ci-robot added jira/severity-moderate Referenced Jira bug's severity is moderate for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Aug 13, 2026
@openshift-ci-robot

Copy link
Copy Markdown

@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
openshift-ci Bot requested review from grzpiotrowski and rikatz August 13, 2026 10:14
@Thealisyed
Thealisyed force-pushed the OCPBUGS-90506-native-metrics-auth branch from cb3ef99 to 84286dd Compare August 13, 2026 10:18
@Thealisyed

Copy link
Copy Markdown
Author

/jira refresh

@openshift-ci-robot

Copy link
Copy Markdown

@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

Copy link
Copy Markdown
Author

/jira refresh

@openshift-ci-robot 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-robot

Copy link
Copy Markdown

@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.

@openshift-ci
openshift-ci Bot requested a review from melvinjoseph86 August 18, 2026 10:07
@Thealisyed 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 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
Thealisyed force-pushed the OCPBUGS-90506-native-metrics-auth branch from 84286dd to 729bf5d Compare August 18, 2026 11:02
Thealisyed added a commit to Thealisyed/external-dns-operator that referenced this pull request Aug 18, 2026
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.
Thealisyed added a commit to Thealisyed/external-dns-operator that referenced this pull request Aug 19, 2026
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.
Thealisyed added a commit to Thealisyed/external-dns-operator that referenced this pull request Aug 19, 2026
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

Copy link
Copy Markdown

/assign

@alebedev87 alebedev87 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread main.go Outdated
Comment thread main.go Outdated
Comment thread main.go Outdated
@Thealisyed Thealisyed changed the title UPSTREAM: <carry>: OCPBUGS-90506: replace kube-rbac-proxy with native metrics auth UPSTREAM: <carry>: OCPBUGS-90506: add secure metrics serving with TokenReview/SAR authentication Aug 25, 2026
@Thealisyed
Thealisyed force-pushed the OCPBUGS-90506-native-metrics-auth branch from 729bf5d to 8109021 Compare August 25, 2026 13:12
Comment thread metrics_openshift_test.go Outdated
@Thealisyed
Thealisyed force-pushed the OCPBUGS-90506-native-metrics-auth branch from 8109021 to 9c589ae Compare August 26, 2026 10:36
@openshift-ci

openshift-ci Bot commented Aug 26, 2026

Copy link
Copy Markdown

@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.

@alebedev87

Copy link
Copy Markdown

/approve
/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Aug 26, 2026
@openshift-ci

openshift-ci Bot commented Aug 26, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alebedev87

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 26, 2026
@openshift-merge-bot
openshift-merge-bot Bot merged commit e9cddd3 into openshift:master Aug 26, 2026
4 checks passed
@openshift-ci-robot

Copy link
Copy Markdown

@Thealisyed: Jira Issue OCPBUGS-90506: Some pull requests linked via external trackers have merged:

The following pull request, linked via external tracker, has not merged:

All associated pull requests must be merged or unlinked from the Jira bug in order for it to move to the next state. Once unlinked, request a bug refresh with /jira refresh.

Jira Issue OCPBUGS-90506 has not been moved to the MODIFIED state.

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

Note:

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

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-robot

Copy link
Copy Markdown

@Thealisyed: Jira Issue OCPBUGS-90506: Some pull requests linked via external trackers have merged:

The following pull request, linked via external tracker, has not merged:

All associated pull requests must be merged or unlinked from the Jira bug in order for it to move to the next state. Once unlinked, request a bug refresh with /jira refresh.

Jira Issue OCPBUGS-90506 has not been moved to the MODIFIED state.

Details

In response to this:

Summary

  • 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 withMetricsAuth using k8s.io/client-go directly — zero new dependencies
  • All new code lives in metrics_openshift.go / metrics_openshift_test.go to minimise the carry patch diff in main.go
  • /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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/severity-moderate Referenced Jira bug's severity is moderate for the branch this PR is targeting. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants