OCPBUGS-98918: Use service-ca certificates for network-check-source metrics

[tpantelis]

Replace insecureSkipVerify with proper certificate validation using OpenShift's service-ca operator. This fixes a security vulnerability where Prometheus scrapes metrics without validating the server's TLS certificate, making it vulnerable to man-in-the-middle attacks.

Previously, the metrics endpoint used self-signed certificates which Prometheus could not validate, requiring insecureSkipVerify: true. Now we use service-ca to generate properly signed certificates that Prometheus can validate using the cluster's CA bundle.

Changes:

• Configure controller-runtime metrics server to use service-ca certs instead of self-signed certificates (CertDir, CertName, KeyName)
• Add service.beta.openshift.io/serving-cert-secret-name annotation to trigger automatic certificate generation and rotation
• Mount certificate secret in deployment at /etc/pki/tls/metrics-certs
• Update ServiceMonitor to validate certificates using caFile and serverName instead of insecureSkipVerify: true

Certificate rotation is handled automatically by controller-runtime's built-in certwatcher, which hot-reloads certificates when service-ca rotates them without requiring pod restart.

[openshift-merge-bot]

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

[openshift-ci-robot]

@tpantelis: This pull request references Jira Issue OCPBUGS-98918, 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:

Replace insecureSkipVerify with proper certificate validation using OpenShift's service-ca operator. This fixes a security vulnerability where Prometheus scrapes metrics without validating the server's TLS certificate, making it vulnerable to man-in-the-middle attacks.

Previously, the metrics endpoint used self-signed certificates which Prometheus could not validate, requiring insecureSkipVerify: true. Now we use service-ca to generate properly signed certificates that Prometheus can validate using the cluster's CA bundle.

Changes:

• Configure controller-runtime metrics server to use service-ca certs instead of self-signed certificates (CertDir, CertName, KeyName)
• Add service.beta.openshift.io/serving-cert-secret-name annotation to trigger automatic certificate generation and rotation
• Mount certificate secret in deployment at /etc/pki/tls/metrics-certs
• Update ServiceMonitor to validate certificates using caFile and serverName instead of insecureSkipVerify: true

Certificate rotation is handled automatically by controller-runtime's built-in certwatcher, which hot-reloads certificates when service-ca rotates them without requiring pod restart.

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.

[coderabbitai]

Summary by CodeRabbit

• Security
  • Metrics endpoints now support secure HTTPS connections using configured certificates.
  • Monitoring validates the service certificate through the platform’s CA bundle and service DNS name instead of skipping verification.
  • Certificate credentials are mounted read-only, helping protect sensitive monitoring configuration while enabling trusted metrics collection.

Walkthrough

The network diagnostics metrics endpoint now serves HTTPS with an OpenShift-generated certificate. The deployment mounts the certificate Secret, the metrics server loads the certificate files, and the ServiceMonitor validates the service CA and DNS name.

Changes

Secure metrics serving

Layer / File(s)	Summary
TLS metrics integration bindata/network-diagnostics/network-check-source.yaml, pkg/cmd/checkendpoints/cmd.go	The Service requests an OpenShift serving certificate. The deployment mounts the certificate Secret at /etc/pki/tls/metrics-certs. The metrics server loads tls.crt and tls.key. The ServiceMonitor uses the service CA bundle and service DNS name instead of skipping verification.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: tssurya, bpickard22

---

Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

• Ignore

❌ Failed checks (6 errors)

Check name	Status	Explanation	Resolution
Pr Quality	❌ Error	The description explains the security problem and implementation, and the patch is 14 lines, but it lists no automated CNO CI job or platform for testing.	Add a Testing or How to verify it section naming the automated CNO CI lane/job and platform. Include the bug link in the PR title or description if it is not present there.
Commit Message Quality	❌ Error	The sole PR commit is logical and has an explanatory body, but its subject lacks the required component prefix: it says “Use service-ca…” instead of “network-check-source: Use service-ca…”.	Amend the commit subject to begin with “network-check-source:” while retaining the descriptive subject and explanatory body.
Unit Tests For Go Changes	❌ Error	The PR modifies pkg/cmd/checkendpoints/cmd.go and a bindata YAML template, but HEAD^..HEAD contains no *_test.go changes.	Add or modify corresponding Go tests. If no tests are valid, document the reason under “How to verify it” and then use @coderabbitai ignore pre-merge checks after other checks pass.
E2e Tests For Feature Changes	❌ Error	The PR changes user-facing metrics TLS behavior to fix a security bug, but the commit has no test/e2e changes and the description has no Testing or How to verify it section.	Add relevant E2E tests and a Testing/How to verify it section with CI jobs, covered platforms, and results. If E2E is not feasible, document why and use the specified override after other checks pass.
Stale Project Docs And Config	❌ Error	docs/architecture.md:169 says CNO and core networking components cannot use service-ca, but this PR adds service-ca certificates to the CNO network-check-source.	Update docs/architecture.md to describe the service-ca dependency and late-start behavior for network diagnostics, or explain the exception in a PR comment and use the documented override.
Title check	❌ Error	The title describes the metrics certificate change and uses the imperative mood, but it is 75 characters and exceeds the 72-character limit.	Shorten the title to 72 characters or fewer while preserving the main change and required scope.

✅ Passed checks (18 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Rbac Least Privilege	✅ Passed	The YAML diff changes only certificate mounts, Service annotation, and ServiceMonitor TLS fields; the existing Role rules remain unchanged and use only get, list, and watch.
Docs For Feature And Behavior Changes	✅ Passed	The patch is an explicit security bug fix for insecure Prometheus TLS scraping; it changes internal metrics certificate handling and adds no user configuration or architecture changes.
Go And Test Code Quality	✅ Passed	The only Go change adds CertDir, CertName, and KeyName fields; it adds no logging, error returns, durations, shadowing, concurrency, IPv4-only logic, or test code.
Ai-Generated Code Smell	✅ Passed	The PR adds only certificate configuration and mounts; it adds no comments, tests, AI references, or unnecessary intermediate logic.
Stable And Deterministic Test Names	✅ Passed	The PR changes only a deployment YAML and runtime metrics configuration; it adds no test files or Ginkgo test title declarations.
Test Structure And Quality	✅ Passed	The patch changes only YAML and production Go code; it adds no Ginkgo test code or test files, so the listed test-quality requirements are not applicable.
Microshift Test Compatibility	✅ Passed	The pull request changes only a deployment manifest and metrics-server configuration. It adds no Ginkgo e2e tests, so MicroShift test compatibility is not applicable.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	The patch changes only a deployment manifest and metrics-server configuration; it adds no Ginkgo e2e tests or multi-node assumptions.
Topology-Aware Scheduling Compatibility	✅ Passed	The diff adds only a secret volume, service annotation, and TLS settings. Deployment scheduling remains replicas: 1 with Recreate; no new affinity, spread, PDB, node targeting, or tolerations were...
Ote Binary Stdout Contract	✅ Passed	The PR is not an OTE implementation; it changes TLS settings and YAML only, and adds no stdout writes or JSON test-listing process code.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	The commit changes only deployment YAML and metrics-server configuration; it adds no Ginkgo e2e tests or external network operations to assess.
No-Weak-Crypto	✅ Passed	The PR adds certificate paths and service-CA validation only; added lines contain no MD5, SHA1, DES, RC4, Blowfish, ECB, custom crypto, or secret comparisons, and defaults use AEAD suites.
Container-Privileges	✅ Passed	The PR adds only a read-only certificate mount and TLS settings; the Deployment retains runAsNonRoot:true and allowPrivilegeEscalation:false, with no privileged, host namespace, SYS_ADMIN, or root...
No-Sensitive-Data-In-Logs	✅ Passed	The PR adds certificate paths, a read-only Secret mount, and TLS validation settings; it adds no logging and exposes no password, token, API key, PII, hostname, or customer data.
Description check	✅ Passed	The description clearly explains the service-ca certificate, TLS, deployment, and ServiceMonitor changes in the pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[tpantelis]

/jira refresh

[openshift-ci-robot]

@tpantelis: This pull request references Jira Issue OCPBUGS-98918, 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.0.0) matches configured target version for branch (5.0.0)
• bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

No GitHub users were found matching the public email listed for the QA contact in Jira (core-networking-bot@redhat.com), skipping review request.

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.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/cmd/checkendpoints/cmd.go`:
- Around line 78-80: Add or update tests covering the cross-layer TLS contract:
in pkg/cmd/checkendpoints/cmd.go lines 78-80, test the metrics server options
and certificate rotation path; in
bindata/network-diagnostics/network-check-source.yaml lines 74-77, 105-108, 118,
and 146-147, add bindata tests verifying the certificate directory, filenames,
Secret name, mount path, HTTPS scheme, CA path, and serverName. If existing
tests already cover all requirements, document that exception instead.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 64682a83-429d-44ee-8f62-f5d8f8ea619c

📥 Commits

Reviewing files that changed from the base of the PR and between 1dcce83 and f30f2de.

📒 Files selected for processing (2)

• bindata/network-diagnostics/network-check-source.yaml
• pkg/cmd/checkendpoints/cmd.go

[tpantelis]

@coderabbitai approve

[coderabbitai]

✅ Action performed

Comments resolved and changes approved.

[danwinship]

sure
/lgtm

[openshift-merge-bot]

Scheduling required tests:
/test e2e-aws-ovn-upgrade-ipsec
/test e2e-gcp-ovn-techpreview

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aws-ovn-fdp-qe
/test e2e-aws-ovn-hypershift-conformance
/test e2e-aws-ovn-serial-1of2
/test e2e-aws-ovn-serial-2of2
/test e2e-aws-ovn-upgrade
/test e2e-aws-ovn-windows
/test e2e-azure-ovn-upgrade
/test e2e-gcp-ovn
/test e2e-gcp-ovn-upgrade
/test e2e-metal-ipi-ovn-dualstack-bgp
/test e2e-metal-ipi-ovn-dualstack-bgp-local-gw
/test e2e-metal-ipi-ovn-ipv6
/test e2e-metal-ipi-ovn-ipv6-ipsec
/test e2e-ovn-ipsec-step-registry
/test hypershift-e2e-aks

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: danwinship, tpantelis

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:

• OWNERS [danwinship]

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

[weliang1]

/verified by CI

[openshift-ci-robot]

@weliang1: This PR has been marked as verified by CI.

Details

In response to this:

/verified by CI

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.

[danwinship]

/retest

[danwinship]

/override ci/prow/e2e-aws-ovn-fdp-qe ci/prow/e2e-metal-ipi-ovn-ipv6-ipsec ci/prow/hypershift-e2e-aks

[openshift-ci]

@danwinship: Overrode contexts on behalf of danwinship: ci/prow/e2e-aws-ovn-fdp-qe, ci/prow/e2e-metal-ipi-ovn-ipv6-ipsec, ci/prow/hypershift-e2e-aks

Details

In response to this:

/override ci/prow/e2e-aws-ovn-fdp-qe ci/prow/e2e-metal-ipi-ovn-ipv6-ipsec ci/prow/hypershift-e2e-aks

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.

[danwinship]

/retest

[openshift-merge-bot]

/retest-required

Remaining retests: 0 against base HEAD d3ad345 and 2 for PR HEAD 25dc7f4 in total

[danwinship]

/override ci/prow/e2e-metal-ipi-ovn-ipv6
job is mostly broken, failure is not relevant to this PR

[openshift-ci]

@danwinship: Overrode contexts on behalf of danwinship: ci/prow/e2e-metal-ipi-ovn-ipv6

Details

In response to this:

/override ci/prow/e2e-metal-ipi-ovn-ipv6
job is mostly broken, failure is not relevant to this PR

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.

[openshift-ci]

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

[openshift-ci-robot]

@tpantelis: Jira Issue Verification Checks: Jira Issue OCPBUGS-98918
✔️ This pull request was pre-merge verified.
✔️ All associated pull requests have merged.
✔️ All associated, merged pull requests were pre-merge verified.

Jira Issue OCPBUGS-98918 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓

Details

In response to this:

Replace insecureSkipVerify with proper certificate validation using OpenShift's service-ca operator. This fixes a security vulnerability where Prometheus scrapes metrics without validating the server's TLS certificate, making it vulnerable to man-in-the-middle attacks.

Previously, the metrics endpoint used self-signed certificates which Prometheus could not validate, requiring insecureSkipVerify: true. Now we use service-ca to generate properly signed certificates that Prometheus can validate using the cluster's CA bundle.

Changes:

• Configure controller-runtime metrics server to use service-ca certs instead of self-signed certificates (CertDir, CertName, KeyName)
• Add service.beta.openshift.io/serving-cert-secret-name annotation to trigger automatic certificate generation and rotation
• Mount certificate secret in deployment at /etc/pki/tls/metrics-certs
• Update ServiceMonitor to validate certificates using caFile and serverName instead of insecureSkipVerify: true

Certificate rotation is handled automatically by controller-runtime's built-in certwatcher, which hot-reloads certificates when service-ca rotates them without requiring pod restart.

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.