feat(metrics): authenticate /metrics endpoint (bearer token + optional mTLS) by abhinavuser · Pull Request #77 · optiqor/kerno

abhinavuser · 2026-05-15T16:00:17Z

closes #41

implements production-grade auth modes for the /metrics endpoint. defaults to none to preserve backward compatibility.

features:

bearer token mode: constant-time comparison to prevent timing side channels.
- sighup token rotation: bearer token file is watched and reloaded on sighup without dropping inflight requests.
- mtls mode: config builder for mutual tls with tls 1.3 minimum enforcement.
- integration: wired seamlessly into config.go and start.go.
- helm chart: added prometheus auth configuration for token secrets, tls secrets, and ServiceMonitor examples.
- /healthz and /readyz remain fully unauthenticated.
  added test coverage for bearer token modes, sighup hot-reload, empty file handling, and pass-through.

Signed-off-by: abhinavuser abhinavkumarv2101@gmail.com

implements production-grade auth modes for the /metrics endpoint as requested in optiqor#41. defaults to 'none' to preserve backwards compatibility. features: - bearer token mode: constant-time comparison to prevent timing side channels. - sighup token rotation: bearer token file is watched and reloaded on sighup without dropping inflight requests. - mtls mode: config builder for mutual tls with tls 1.3 minimum enforcement. - prometheus auth config: wired seamlessly into config.go and start.go - /healthz and /readyz remain fully unauthenticated. added test coverage for bearer token modes, sighup hot-reload, and pass-through. closes optiqor#41 Signed-off-by: abhinavuser <abhinavuser@users.noreply.github.com>

Signed-off-by: abhinavuser <abhinavuser@users.noreply.github.com>

github-actions · 2026-05-15T16:00:28Z

🚀 First PR — welcome aboard!

A few things to expect:

CI: every PR runs build + race tests + lint + (eventually) the kernel matrix. If something fails, the log will tell you exactly which gate.
DCO: every commit needs Signed-off-by: — git commit -s adds it automatically.
Conventional Commits: PR titles like feat(doctor): add new rule or fix(bpf): handle X. We squash-merge by default.
Review: a maintainer will review within 72 hours. Suggestions are conversations, not orders — push back if something doesn't fit your context.

If you get stuck, reply here or jump to Discussions. We want this PR to land.

btwshivam

mtls mode breaks kubelet probes. /healthz and /readyz share a mux with /metrics, so ListenAndServeTLS puts the health endpoints behind RequireAndVerifyClientCert and the chart's httpGet probes will fail every interval.

btwshivam · 2026-05-16T22:52:05Z

+			var err error
+			if tlsCfg != nil {
+				// Certs are already loaded into TLSConfig
+				err = httpServer.ListenAndServeTLS("", "")


mtls puts the entire listener behind TLS plus client-cert-required, but /healthz and /readyz at lines 157-158 are on the same mux. kubelet probes hit /healthz over plain HTTP without a client cert (the chart's livenessProbe at deploy/helm/kerno/templates/daemonset.yaml uses httpGet:), so once mtls is on every probe fails and the pod restarts every 30s. either split health onto a second plain-HTTP listener, or scope tls to just the /metrics mux. the bearer path got this right (only wraps /metrics), mtls broke the pattern.

btwshivam · 2026-05-16T22:52:05Z

@@ -0,0 +1,65 @@
+// Copyright 2026 Optiqor contributors


bearer.go has six unit tests, mtls.go has zero. at minimum cover the four branches: empty certFile returns nil, certFile without keyFile errors, cert+key without CA produces one-way TLS, cert+key+CA sets RequireAndVerifyClientCert. these are pure-function checks, no real cert generation needed beyond httptest-style helpers.

btwshivam

ci's red because the reload test rewrites a 0o400 file (permission-denied) and two lint issues remain. the mtls path also has no tests.

btwshivam · 2026-05-16T22:54:16Z

+	}
+
+	// Rotate the token on disk and reload.
+	if err := os.WriteFile(path, []byte("new-token\n"), 0o400); err != nil {


this rewrite fails with permission-denied because writeTokenFile at line 25 creates the file 0o400 (read-only) and you're writing back to the same path. switch the initial mode to 0o600, or os.Chmod(path, 0o600) before this line. that's the only test failure in the ci run.

btwshivam · 2026-05-16T22:54:17Z

 		mux.HandleFunc("/readyz", healthzHandler(loadedCount, len(loaders)))
-		mux.Handle("/metrics", promhttp.HandlerFor(metrics.Registry, promhttp.HandlerOpts{}))
+
+		var metricsHandler http.Handler = promhttp.HandlerFor(metrics.Registry, promhttp.HandlerOpts{})


staticcheck QF1011: the explicit http.Handler type is redundant since promhttp.HandlerFor already returns http.Handler and reassigning to guard.Wrap(metricsHandler) keeps that type. drop the type annotation:

metricsHandler := promhttp.HandlerFor(metrics.Registry, promhttp.HandlerOpts{})

btwshivam · 2026-05-16T22:54:17Z

+// loadCertPool reads a PEM-encoded CA certificate bundle and returns a cert
+// pool suitable for use as tls.Config.ClientCAs.
+func loadCertPool(path string) (*x509.CertPool, error) {
+	pem, err := os.ReadFile(path)


gosec G304: the path comes from operator config not user input, so it's a false positive here. same flag fires on bearer.go load() for the same reason. add // #nosec G304 -- path comes from operator config on both call sites, or wrap reads in a helper that asserts the path lives under a known config dir. either's fine, but lint needs to be green.

btwshivam · 2026-05-16T22:54:17Z

+//
+// Returns nil when certFile is empty, indicating that plain HTTP should be
+// used (i.e. auth.mode is "none" or "bearer" without TLS).
+func TLSConfig(certFile, keyFile, caFile string) (*tls.Config, error) {


no test coverage for the mtls path. bearer is well-tested in bearer_test.go, but TLSConfig has none. at minimum: a test that verifies RequireAndVerifyClientCert is set when caFile is non-empty, and a negative test that loadCertPool fails on a non-PEM file. with how security-sensitive this is, an end-to-end test using httptest.NewTLSServer plus a client cert that validates against the configured CA would be ideal.

This addresses the review by splitting the health probe listener onto port 9092, adding mTLS unit and end-to-end tests, fixing bearer tests permission issues, and removing gosec/staticcheck lint warnings. Signed-off-by: abhinavuser <abhinavkumarv2101@gmail.com>

This fixes bodyclose, noctx, gofmt, and IP SAN missing from the test certificate generated in mtls_test.go. Signed-off-by: abhinavuser <abhinavkumarv2101@gmail.com>

Signed-off-by: abhinavuser <abhinavkumarv2101@gmail.com>

abhinavuser · 2026-05-17T04:11:01Z

hey @btwshivam , sorry for the noise with the multiple PRs, i've closed out the logging one and i'm pausing work on the sinks PR until this one is clean and merged

i've pushed a new commit that addresses all your feedback:

health probes: split the HTTP servers. /healthz and /readyz now live on a dedicated plain-HTTP listener (default :9092), and the Helm chart livenessProbe/readinessProbe have been updated to point to this new health port. mTLS now exclusively wraps the /metrics listener
2.mtls tests: added mtls_test.go covering all 4 pure-function branches, plus an end-to-end httptest.NewUnstartedServer test that validates client cert enforcement
ci test fix: updated writeTokenFile in bearer_test.go to use 0o600 so the reload test can overwrite it without permission errors
linters: dropped the redundant http.Handler type to fix QF1011, and added the #nosec G304 directives to fix the gosec false positives. (Also fixed a few minor test linting flakes)

let me know if you need anythin or if there s any issues

btwshivam · 2026-05-23T00:36:38Z

ill review them shortly

btwshivam

two blockers: mode mtls without ca_cert_file silently drops client verification, and the helm auth config only wires the prometheus side so the kerno container never enforces it. details inline.

btwshivam · 2026-05-30T23:10:05Z

+		if c.Prometheus.Auth.Mode == "bearer" && c.Prometheus.Auth.TokenFile == "" {
+			return fmt.Errorf("prometheus.auth.token_file is required when auth mode is bearer")
+		}
+		if c.Prometheus.Auth.Mode == "mtls" && (c.Prometheus.Auth.CertFile == "" || c.Prometheus.Auth.KeyFile == "") {


mode mtls requires cert_file and key_file here but not ca_cert_file. TLSConfig only sets RequireAndVerifyClientCert when caFile is non-empty (mtls.go), so mode: mtls with no ca_cert_file comes up as one-way TLS and clients are never asked for a cert. that's a server advertising mtls while not enforcing it, which is worse than failing closed. require ca_cert_file here when mode is mtls.

btwshivam · 2026-05-30T23:10:05Z

      interval: {{ .Values.serviceMonitor.interval }}
      path: /metrics
+      {{- if eq .Values.prometheus.auth.mode "bearer" }}
+      authorization:


this configures Prometheus to send a bearer token, but nothing configures kerno to require one. the daemonset still runs start --prometheus-addr=... with no auth.mode, no token_file, and no secret mounted, so the container stays on the default mode: none.

net effect: auth.mode: bearer leaves /metrics unauthenticated while Prometheus sends a token nobody checks, and auth.mode: mtls breaks scraping outright since Prometheus does TLS but kerno serves plain http. the kerno side needs the same values: pass auth.mode plus the token/cert paths to the container, and mount the token/tls secret as a volume.

btwshivam · 2026-05-30T23:10:05Z

+			healthAddr = ":9092"
+		}
+
+		healthServer = &http.Server{


the health server moves to :9092 unconditionally, including mode: none. anything hitting :9090/healthz today (systemd unit, external LB check, the old default) loses it after upgrade, even though the PR says none preserves backward compatibility. the split only matters once metrics gets tls. for none and bearer, keep /healthz and /readyz on the metrics addr, or gate the second listener on mtls.

btwshivam · 2026-05-30T23:10:05Z

+		metricsMux := http.NewServeMux()
+		metricsHandler := promhttp.HandlerFor(metrics.Registry, promhttp.HandlerOpts{})
+
+		if cfg.Prometheus.Auth.Mode == "bearer" {


bearer mode never sets up tls, tlsCfg stays nil unless mode is mtls, so the token travels in cleartext over plain http. on a hostNetwork daemonset that's the node wire. TLSConfig already supports cert+key without a CA (one-way tls), but there's no way to reach it in bearer mode. either let bearer run over tls too, or document the cleartext exposure.

btwshivam · 2026-05-30T23:12:12Z

good work you are going in ryt direction.. just some blockers.. fix ill re-review

- config: validate ca_cert_file for mtls mode to prevent silent fallback to one-way TLS - helm: wire prometheus auth mode and secret mounts to kerno daemonset so the container actually enforces auth - cli/start: only split health endpoints to a separate listener in mtls mode (preserves backward compat for none/bearer) - cli/start: allow bearer mode to optionally pick up TLS if cert_file and key_file are provided, preventing cleartext token transmission Signed-off-by: abhinavuser <abhinavkumarv2101@gmail.com>

abhinavuser · 2026-05-31T12:13:19Z

hey @btwshivam i ve pushed a new commit that addresses all four blockers

Config Validation: config.Validate() now strictly requires ca_cert_file when mode is mtls to prevent the silent one way TLS fallback
Helm Daemonset: wired up the auth environment variables (KERNO_PROMETHEUS_AUTH_MODE, etc.) and the secret volumeMounts in daemonset.yaml so the container actually enforces the auth that ServiceMonitor configures
Health Probes: only split /healthz and /readyz onto the :9092 listener when mode is mtls. For none and bearer, they remain on the metrics mux to preserve backward compatibility
Bearer TLS: bearer mode now builds a tls.Config if cert_file and key_file are provided, preventin the token from travellin in cleartext
lmk if this looks good to merge

btwshivam

good progress, both code blockers are fixed: mtls now requires ca_cert_file, and the health server only splits off in mtls mode. but the helm auth wireup routes config through KERNO_* env vars, which viper doesn't actually read (#112), so the container still comes up mode=none and /metrics stays unauthenticated. and a stray pr_body.md from a different PR slipped in.

btwshivam · 2026-06-06T17:12:52Z

            - name: KERNO_LOG_FORMAT
              value: {{ .Values.log.format }}
+            - name: KERNO_PROMETHEUS_AUTH_MODE
+              value: {{ .Values.prometheus.auth.mode | quote }}


this is the right shape, but it won't take effect. KERNO_PROMETHEUS_AUTH_MODE (and the token/cert file env vars below) go through viper's AutomaticEnv, and per #112 viper never reads those because initConfig registers no defaults or BindEnv keys, so Unmarshal has nothing to populate from env. net result is the container still runs auth.mode=none: bearer leaves /metrics unauthenticated while prometheus sends a token nobody checks, and mtls breaks scraping. the secrets are mounted as files (good), so the robust fix is to render the auth settings into a ConfigMap mounted at /etc/kerno/config.yaml, which viper does read via ReadInConfig, rather than KERNO_* env. otherwise this depends on #112 landing first, please verify end to end before merge since a silently-unenforced auth is worse than no feature.

btwshivam · 2026-06-06T17:12:52Z

@@ -0,0 +1,16 @@
+fixes #29


this file doesn't belong in the PR, and it's not even this PR, the content is for #29 (webhook sinks, --sink flags). looks like it got committed from another branch. git rm pr_body.md and drop it.

abhinavuser added 2 commits May 15, 2026 21:24

chore(helm): add prometheus auth configuration for /metrics endpoint

d0df993

Signed-off-by: abhinavuser <abhinavuser@users.noreply.github.com>

abhinavuser requested a review from btwshivam as a code owner May 15, 2026 16:00

github-actions Bot added testing Tests and test coverage area/k8s Kubernetes integration area/security Security and supply chain area/ops Operations, deployment, runtime ergonomics labels May 15, 2026

abhinavuser changed the title ~~feat(security): authenticate /metrics endpoint (bearer token + optional mTLS)~~ feat(metrics): authenticate /metrics endpoint (bearer token + optional mTLS) May 15, 2026

btwshivam requested changes May 16, 2026

View reviewed changes

github-actions Bot added the level:critical Touches BPF, security, or release surfaces (auto-applied) label May 17, 2026

abhinavuser added 4 commits May 17, 2026 09:17

fix: address CI lint and test failures in mTLS implementation

8be6c45

This fixes bodyclose, noctx, gofmt, and IP SAN missing from the test certificate generated in mtls_test.go. Signed-off-by: abhinavuser <abhinavkumarv2101@gmail.com>

fix: add missing context import to mtls_test.go

cef3868

Signed-off-by: abhinavuser <abhinavkumarv2101@gmail.com>

chore: formatting

6aecb55

Signed-off-by: abhinavuser <abhinavkumarv2101@gmail.com>

chore: fix trailing whitespace in start.go for gofmt

cd42322

Signed-off-by: abhinavuser <abhinavkumarv2101@gmail.com>

abhinavuser requested a review from btwshivam May 17, 2026 04:11

btwshivam requested changes May 30, 2026

View reviewed changes

btwshivam mentioned this pull request May 31, 2026

test(metrics): integration-test /metrics scrape with a Prometheus container #146

Open

github-actions Bot added the documentation Improvements or additions to documentation label May 31, 2026

abhinavuser requested a review from btwshivam May 31, 2026 12:13

btwshivam requested changes Jun 6, 2026

View reviewed changes

Conversation

abhinavuser commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented May 15, 2026

Uh oh!

btwshivam left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

btwshivam left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abhinavuser commented May 17, 2026

Uh oh!

btwshivam commented May 23, 2026

Uh oh!

btwshivam left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

btwshivam commented May 30, 2026

Uh oh!

abhinavuser commented May 31, 2026

Uh oh!

btwshivam left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

abhinavuser commented May 15, 2026 •

edited

Loading