feat(openshift): secure component metrics endpoints with mutual TLS - #3848
Open
jkhelil wants to merge 2 commits into
Open
feat(openshift): secure component metrics endpoints with mutual TLS#3848jkhelil wants to merge 2 commits into
jkhelil wants to merge 2 commits into
Conversation
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3848 +/- ##
==========================================
+ Coverage 25.49% 25.87% +0.37%
==========================================
Files 449 451 +2
Lines 23508 23908 +400
==========================================
+ Hits 5994 6185 +191
- Misses 16824 17011 +187
- Partials 690 712 +22
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Secure component metrics endpoints with mutual TLS on OpenShift. Each reconciler syncs the Prometheus client CA bundle from kube-system/extension-apiserver-authentication into the component namespace as a metrics-client-ca ConfigMap. OpenShift's serving-cert controller is triggered via annotation on each metrics Service to provision a per-component TLS Secret. Two new manifest transformers (InjectMetricsServingCert, ApplyMetricsTLS) wire the Secret and ConfigMap as volumes and inject METRICS_PROMETHEUS_TLS_* env vars so that the knative/pkg prometheus.Server enables mTLS with require client auth. ServiceMonitor resources are updated with scheme: https, scrapeClass tls-client-certificate-auth, and tlsConfig.serverName. The existing UpdateServiceMonitorTargetNamespace transformer is extended to also patch the namespace segment inside serverName at runtime. Relates-To: SRVKP-8172 Signed-off-by: Jawed khelil <jkhelil@redhat.com> Assisted-by: Claude Sonnet 4.6 (via Cursor) Co-authored-by: Cursor <cursoragent@cursor.com>
Add opt-in feature flag spec.platforms.openshift.enableMetricsMTLS to TektonConfig. When false (default) all mTLS transformers are skipped so existing clusters are unaffected. Key changes: - Add EnableMetricsMTLS *bool to OpenShift API struct + CRD schema - EnsureMetricsClientCA (TektonConfig-only) returns a clear error when kube-system/extension-apiserver-authentication is absent, asking the user to disable mTLS or install CMO - Add ResolveMetricsMTLS helper (flag check + CM existence in one call) used by all 6 component PreReconcile methods - Gate all mTLS transformers behind metricsMTLSReady on each component extension struct - Include MetricsCABundle content in GetPlatformData() hash so component CRs are automatically re-annotated when TektonConfig creates the CM, triggering immediate component re-reconciles - Add unit tests for IsMetricsMTLSEnabled, AnnotateMetricsServingCert, RenameServicePort, UpdateServiceMonitorForMetricsMTLS Fix tekton-results ServiceMonitor/Service port mismatch: 05-results-monitoring.yaml unconditionally hardcoded scheme: https and https-* port names for both results ServiceMonitors, while the underlying Services keep plain port names unless the flag is on. This broke Prometheus scraping for tekton-results-api and tekton-results-watcher by default, regardless of the flag. The static manifest now stays plain HTTP, and tektonpipeline's filterAndTransformMonitoring conditionally upgrades the results-watcher ServiceMonitor to mTLS in lockstep with the Service port rename, matching every other component. tekton-results-api is excluded from the mTLS upgrade entirely: its Prometheus metrics endpoint (cmd/api/main.go in tektoncd/results) is a bare net/http server that never reads METRICS_PROMETHEUS_TLS_* env vars, so it cannot serve mTLS. Its Service/ServiceMonitor stay on plain HTTP regardless of this flag; this is a known upstream limitation, not something this operator can fix. Signed-off-by: Jawed khelil <jkhelil@redhat.com> Assisted-by: Claude Sonnet 4.6 (via Cursor) Co-authored-by: Cursor <cursoragent@cursor.com>
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
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.
Changes
Secure component metrics endpoints with mutual TLS (mTLS) on OpenShift, gated
behind an opt-in feature flag.
Each reconciler syncs the Prometheus client CA bundle from
kube-system/extension-apiserver-authenticationinto the component namespaceas a
metrics-client-caConfigMap. OpenShift's serving-cert controller istriggered via annotation on each metrics
Serviceto provision aper-component TLS
Secret. Manifest transformers (AnnotateMetricsServingCert,RenameServicePort,ApplyMetricsTLS) wire the Secret and ConfigMap asvolumes and inject
METRICS_PROMETHEUS_TLS_*env vars soknative/pkg'sprometheus.Serverenables mTLS with required client auth.ServiceMonitorresources are updated withscheme: https,tlsConfig.serverName, and the correct port name.The feature is opt-in via
spec.platforms.openshift.enableMetricsMTLSonTektonConfig(defaultfalse). When disabled, all mTLS transformers areskipped and metrics stay on plain HTTP — existing clusters are unaffected.
Known limitation:
tekton-results-apitekton-results-api's Prometheus metrics endpoint(
cmd/api/main.gointektoncd/results) is a barenet/httpserver thatnever reads
METRICS_PROMETHEUS_TLS_*env vars, so it cannot serve mTLS.It is therefore excluded from the mTLS upgrade: its
ServiceandServiceMonitorstay on plain HTTP regardless of the flag.tekton-results-watcheris unaffected by this and gets full mTLS like everyother component, since it is a standard Knative-based reconciler. Fixing
tekton-results-apiitself would require an upstream change intektoncd/results, tracked separately.Testing performed
mTLS-capable component (pipeline, triggers, chains, pruner,
results-watcher): the
metrics-client-caConfigMap synced, the metricsServiceport renamed tohttps-*, the serving-certSecretwasprovisioned, and the
ServiceMonitorwas updated tohttps+tlsConfig.curl: requests with a valid Prometheusclient cert succeed (
200, real metrics returned); requests without aclient cert fail the TLS handshake (explicit "certificate required" alert).
tekton-results-apimetrics remain reachable over plain HTTPwith the flag enabled (no regression from the mTLS work).
components, with matching
Service/ServiceMonitorports.Submitter Checklist
make test lintbefore submitting a PRRelease Notes
Made with Cursor