Retire metrics-proxy self-test and its integration test#528
Merged
Conversation
The metrics-proxy self-test driver in test/integration/metrics_test.go never runs in upstream CI. 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 the test guards itself with the same check, hitting t.Skip on every nightly run. Delete metrics_test.go and move its podExec helper to common.go, which also uses podExec for link-ordering checks. The rest of the test file -- ServiceAccount/token plumbing, pod selector, agentMetricTestResult JSON types -- goes with the file. See bpfman#527. Signed-off-by: Andrew McDermott <amcdermo@redhat.com>
The /metrics-proxy test subcommand exists solely to be kubectl exec'd by an integration test that has now been retired. Delete the argv[1] == "test" dispatcher and all the code it reached: runSelfTests, testHealthEndpoint, testMetricsEndpoint, getServerCertPool, outputResults, and the TestResult/TestResults JSON types. The self-test's TLS verification was theatre. It dialled localhost:8443 with InsecureSkipVerify: true, scraped the peer cert, built a RootCAs pool, then performed the real request with InsecureSkipVerify: false. The bootstrap dial trusted whatever certificate the peer presented, so the subsequent "verified" call only confirmed that the second peer matched the first. If the bootstrap failed the code fell back silently to InsecureSkipVerify: true. See bpfman#527. Signed-off-by: Andrew McDermott <amcdermo@redhat.com>
Contributor
Author
|
cc @alebedev87 |
4 tasks
alebedev87
reviewed
May 19, 2026
Contributor
|
/lgtm |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This addresses #527 by retiring the metrics-proxy self-test in two commits. The first removes
test/integration/metrics_test.go, which has beent.Skip-ing on every nightly run because the upstream KIND cluster doesn't install the prometheus-operator CRDs that the bpfman-operator gates the metrics-proxy DaemonSet reconcile on. The second removes the matching/metrics-proxy testsubcommand from the production binary, along with the bootstrap-and-pin TLS pattern that SAST tooling has been flagging.podExecwas the only piece ofmetrics_test.gowith another caller (common.gouses it for link-ordering checks), so it moves intocommon.go. Everything else -- the dispatcher incmd/metrics-proxy/main.go, therunSelfTests/testHealthEndpoint/testMetricsEndpoint/getServerCertPool/outputResultshelpers, theTestResult/TestResultsJSON types, and the ServiceAccount/token plumbing in the test file -- goes away. Net effect is roughly -560 lines of code that ships in the agent image but never runs, and the SAST finding goes with it.If a downstream consumer (e.g. openshift/bpfman-operator) deploys with prometheus-operator CRDs installed and wants metrics coverage in their CI, that test lives more naturally in their tree than as an always-skipping test here. Happy to discuss alternatives if you'd rather keep the test in place with a different fix.
Test plan
make buildandmake testpassgo test -c -tags integration_tests ./test/integration/.github/workflows/integration_test.yml) passes on the remaining tests (app_test.go,kprobe_test.go,tc_test.go,tcx_test.go,tracepoint_test.go,uprobe_test.go,xdp_test.go)