bpfman / bpfman/bpfman-operator

Retire metrics-proxy self-test and its integration test

Open
#527 0 comments 0 reactions 1 assignee Claimed by @frobware View on GitHub
Dominant language
Go
Stars
38
Forks
27
PR merge metrics
No merged PRs in 30d

Description

`cmd/metrics-proxy/main.go` ships a self-test mode reached by `/metrics-proxy test`. Its only caller anywhere in the tree is `test/integration/metrics_test.go`, which `kubectl exec`s into the running pod with `env TOKEN= /metrics-proxy test` and parses the JSON output. The daemonset entrypoint is `["/metrics-proxy"]` with no `test` argument; the liveness and readiness probes hit `/healthz` and `/readyz` over plain HTTP on port 8081; CI workflows outside the nightly integration job only invoke the binary with `--version`. The self-test never runs in any deployed cluster.

The integration test that drives the self-test never runs in upstream CI either. The nightly KIND cluster does not install the prometheus-operator CRDs, so `monitoring.coreos.com` is absent. The bpfman-operator gates its metrics-proxy DaemonSet reconcile on that API group (`controllers/bpfman-operator/config.go:328`) and `test/integration/metrics_test.go` guards itself with the same check ("Skipping: monitoring.coreos.com API group not available"). The DaemonSet is never deployed in CI, the test always `t.Skip`s, and the nightly run verifies nothing.

Separately, the self-test's TLS verification is theatre. It dials `localhost:8443` with `InsecureSkipVerify: true`, scrapes the peer cert, builds a `RootCAs` pool, then performs the real request with `InsecureSkipVerify: false`. The bootstrap dial trusts whatever certificate the peer presents, so the subsequent "verified" call only confirms that the second peer matches the first, not that either is legitimate; if the bootstrap fails the code falls back silently to `InsecureSkipVerify: true` for the real request. SAST tooling reasonably flags both sites.

Proposed action: delete `test/integration/metrics_test.go` and retire the self-test from `cmd/metrics-proxy/main.go` (the `argv[1] == "test"` dispatcher, `runSelfTests`, `testHealthEndpoint`, `testMetricsEndpoint`, `getServerCertPool`, `outputResults`, and the `TestResult`/`TestResults` JSON types). `podExec` is the only piece of the test file with another caller (`common.go` uses it for link-ordering checks) and moves to `common.go`. The rest -- SA/token/RBAC plumbing, pod selector, `agentMetricTestResult` JSON types -- goes with the file. Net effect: roughly -560 lines of code that ships and never runs in upstream, the SAST finding eliminated by deletion, and a smaller agent image surface.

If a downstream consumer (e.g. openshift/bpfman-operator) deploys with prometheus-operator CRDs present and wants metrics coverage in their CI, that test lives more naturally in their tree than as an always-skipping test here.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.