Skip to content

fix(valkey-operator): align ServiceMonitor with metrics.secure (skip-verify opt-in) #231

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
daanvinken wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix(valkey-operator): align ServiceMonitor with metrics.secure (skip-verify opt-in) #231

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0a9ca86
fix(valkey-operator): align ServiceMonitor with metrics.secure
daanvinken Jul 24, 2026
5d89dce
fix(valkey-operator): opt-in insecureSkipVerify for secure ServiceMon…
daanvinken Jul 24, 2026
d3dd681
chore(valkey-operator): bump chart to 0.4.2 after rebase
daanvinken Jul 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions valkey-operator/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,22 @@
# Changelog

## 0.4.2

### Fixed

- Align ServiceMonitor endpoint port name and scrape scheme with `metrics.secure`.
When secure metrics are enabled the metrics Service exposes port `https`, but
the ServiceMonitor previously always targeted port `http`, so scrapes failed.
Defaults `scheme: https` when `metrics.secure` is true and
`serviceMonitor.scheme` is unset.

### Added

- `metrics.serviceMonitor.insecureSkipVerify` (default `false`). When true and
`metrics.secure` is true and `serviceMonitor.tlsConfig` is empty, injects
`tlsConfig.insecureSkipVerify` for the operator's self-signed metrics cert.
Explicit `serviceMonitor.tlsConfig` always wins.

## 0.4.1

### Changed
Expand Down
2 changes: 1 addition & 1 deletion valkey-operator/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ apiVersion: v2
name: valkey-operator
description: A Helm chart for the Valkey Operator
type: application
version: 0.4.1
version: 0.4.2
appVersion: "v0.4.0"
kubeVersion: ">=1.20.0-0"
home: https://valkey.io/valkey-helm/
Expand Down
55 changes: 52 additions & 3 deletions valkey-operator/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# valkey-operator

![Version: 0.4.0](https://img.shields.io/badge/Version-0.4.0-informational?style=flat-square) ![Type: application](https://img.shields.io/badge/Type-application-informational?style=flat-square) ![AppVersion: v0.4.0](https://img.shields.io/badge/AppVersion-v0.4.0-informational?style=flat-square)
![Version: 0.4.2](https://img.shields.io/badge/Version-0.4.2-informational?style=flat-square) ![Type: application](https://img.shields.io/badge/Type-application-informational?style=flat-square) ![AppVersion: v0.4.0](https://img.shields.io/badge/AppVersion-v0.4.0-informational?style=flat-square)

A Helm chart for deploying the [Valkey Operator](https://github.com/valkey-io/valkey-operator) on Kubernetes.

Expand Down Expand Up @@ -73,8 +73,9 @@ See [values.yaml](values.yaml) for the full list of configurable parameters.
| `metrics.serviceMonitor.annotations` | Annotations for the ServiceMonitor | `{}` |
| `metrics.serviceMonitor.interval` | Scrape interval | `""` |
| `metrics.serviceMonitor.scrapeTimeout` | Scrape timeout | `""` |
| `metrics.serviceMonitor.scheme` | HTTP scheme used for scraping | `""` |
| `metrics.serviceMonitor.tlsConfig` | TLS configuration used when scraping | `{}` |
| `metrics.serviceMonitor.scheme` | Scrape scheme. Empty: `https` when `metrics.secure` is true, else Prometheus default (`http`) | `""` |
| `metrics.serviceMonitor.insecureSkipVerify` | When `metrics.secure` is true and `tlsConfig` is empty, set `tlsConfig.insecureSkipVerify` for the self-signed metrics cert (opt-in) | `false` |
| `metrics.serviceMonitor.tlsConfig` | Full scrape TLS config; when set, overrides `insecureSkipVerify` | `{}` |
| `metrics.serviceMonitor.honorLabels` | Keep labels from the scraped target on collision | `false` |
| `metrics.serviceMonitor.relabelings` | relabelings applied before scraping | `[]` |
| `metrics.serviceMonitor.metricRelabelings` | metricRelabelings applied before ingestion | `[]` |
Expand Down Expand Up @@ -170,6 +171,54 @@ metrics:
Both `metrics.secure` and `metrics.reader.binding.create` default to `false`, so existing
installs keep serving metrics over insecure HTTP unless you opt in.

### ServiceMonitor

Set `metrics.serviceMonitor.enabled: true` to create a Prometheus Operator ServiceMonitor
for the operator metrics Service. The endpoint **port name** always matches the metrics
Service: `http` when `metrics.secure` is `false`, `https` when it is `true`.

When `metrics.secure` is `true`:

- If `serviceMonitor.scheme` is empty, the chart sets `scheme: https`.
- The operator's metrics cert is self-signed by default. Scrapes will fail TLS
verification until you either:
- set `serviceMonitor.insecureSkipVerify: true` (opt-in; injects
`tlsConfig.insecureSkipVerify`), or
- set `serviceMonitor.tlsConfig` yourself (e.g. a CA). Explicit `tlsConfig` wins
over `insecureSkipVerify`.

Example with secure metrics and a ServiceMonitor that accepts the self-signed cert:

```yaml
metrics:
secure: true
reader:
binding:
create: true
serviceAccountName: prometheus
namespace: monitoring
serviceMonitor:
enabled: true
labels:
# match your Prometheus serviceMonitorSelector
release: prometheus
insecureSkipVerify: true
```

Example with a custom CA instead of skip-verify:

```yaml
metrics:
secure: true
serviceMonitor:
enabled: true
tlsConfig:
ca:
secret:
name: operator-metrics-ca
key: ca.crt
```

## Source Code

* <https://github.com/valkey-io/valkey-operator>
17 changes: 12 additions & 5 deletions valkey-operator/templates/servicemonitor.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,27 @@ metadata:
{{- end }}
spec:
endpoints:
- port: http
# Port name must match metrics-service.yaml (http when insecure, https when metrics.secure).
- port: {{ if .Values.metrics.secure }}https{{ else }}http{{ end }}
honorLabels: {{ .Values.metrics.serviceMonitor.honorLabels }}
{{- with .Values.metrics.serviceMonitor.interval }}
interval: {{ . }}
{{- end }}
{{- with .Values.metrics.serviceMonitor.scrapeTimeout }}
scrapeTimeout: {{ . }}
{{- end }}
{{- with .Values.metrics.serviceMonitor.scheme }}
scheme: {{ . }}
{{- if .Values.metrics.serviceMonitor.scheme }}
scheme: {{ .Values.metrics.serviceMonitor.scheme }}
{{- else if .Values.metrics.secure }}
scheme: https
Comment on lines +29 to +30

@greptile-apps greptile-apps Bot Jul 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Secure ServiceMonitor lacks scrape authentication

When metrics.secure is enabled, this endpoint selects HTTPS but does not configure authorization or a bearer-token secret. The protected operator metrics endpoint requires the metrics-reader service-account token, so Prometheus sends an unauthenticated scrape and receives 401 even when the intended scraper identity has the required RBAC binding. Add a documented Secret-backed authorization configuration to the ServiceMonitor endpoint for secure metrics.

Artifacts

Secure metrics authentication-path test source

  • Authored Python test source that checks the chart's secure endpoint and exercises an HTTPS metrics endpoint requiring a Bearer token; it demonstrates the required authentication condition.

HTTPS metrics request without ServiceMonitor authentication

  • Executed HTTPS `/metrics` request after checking the chart's secure endpoint fields; no Authorization header was sent and the auth-enforcing endpoint returned 401, confirming rejection.

HTTPS metrics request with bound service account Bearer token

  • Executed the same HTTPS `/metrics` request with the expected Bearer token; the endpoint returned 200 and metrics, confirming that token authentication is required.

View artifacts

T-Rex Ran code and verified through T-Rex

{{- end }}
{{- with .Values.metrics.serviceMonitor.tlsConfig }}
{{- if .Values.metrics.serviceMonitor.tlsConfig }}
tlsConfig:
{{- toYaml . | nindent 6 }}
{{- toYaml .Values.metrics.serviceMonitor.tlsConfig | nindent 6 }}
{{- else if and .Values.metrics.secure .Values.metrics.serviceMonitor.insecureSkipVerify }}
# Opt-in: operator metrics use a self-signed cert by default.
tlsConfig:
insecureSkipVerify: true
Comment on lines +18 to +38

@greptile-apps greptile-apps Bot Jul 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Secure ServiceMonitor branches lack regression coverage

The focused ServiceMonitor suite only asserts the default insecure http endpoint. It never enables metrics.secure or exercises the explicit scheme, tlsConfig, and insecureSkipVerify paths, so regressions in secure port selection, HTTPS defaults, TLS-config precedence, or self-signed certificate handling pass the current tests. Add focused Helm unit tests for each secure rendering path.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Artifacts

Secure metrics authentication-path test source

  • Authored Python test source that checks the chart's secure endpoint and exercises an HTTPS metrics endpoint requiring a Bearer token; it demonstrates the required authentication condition.

HTTPS metrics request without ServiceMonitor authentication

  • Executed HTTPS `/metrics` request after checking the chart's secure endpoint fields; no Authorization header was sent and the auth-enforcing endpoint returned 401, confirming rejection.

HTTPS metrics request with bound service account Bearer token

  • Executed the same HTTPS `/metrics` request with the expected Bearer token; the endpoint returned 200 and metrics, confirming that token authentication is required.

ServiceMonitor coverage and rendering harness source

  • Authored shell harness searches the focused test suite for the claimed inputs and renders the three secure ServiceMonitor cases, providing a reproducible narrow check; it establishes how the observed outputs were produced.

Current ServiceMonitor unit suite passes without secure branch cases

  • Executed `helm unittest valkey-operator -f tests/servicemonitor_test.yaml` with the helm-unittest plugin and recorded 10 passing tests; it shows the existing suite passes before any added coverage.

Secure ServiceMonitor branch coverage check and live renders

  • Executed the authored harness with Helm and captured absent test-value inputs plus live secure-default, explicit-TLS-precedence, and insecure-skip-verify renders; it confirms the missing coverage and distinct untested outcomes.

Valkey operator chart lint result

  • Executed Helm lint against the valkey-operator chart and captured one chart linted with zero failures; it confirms the rendered template is valid.

View artifacts

T-Rex Ran code and verified through T-Rex

{{- end }}
{{- with .Values.metrics.serviceMonitor.relabelings }}
relabelings:
Expand Down
10 changes: 8 additions & 2 deletions valkey-operator/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,9 +235,15 @@ metrics:
interval: ""
# Scrape timeout (defaults to the Prometheus global timeout when empty)
scrapeTimeout: ""
# HTTP scheme used for scraping (defaults to http when empty)
# HTTP scheme used for scraping. When empty: https if metrics.secure is true,
# otherwise Prometheus defaults to http.
scheme: ""
# TLS configuration used when scraping the endpoint
# When metrics.secure is true and tlsConfig is empty, set
# tlsConfig.insecureSkipVerify so scrapers accept the operator's self-signed
# metrics cert. Defaults to false (opt-in). Ignored when tlsConfig is set.
insecureSkipVerify: false
# Full TLS configuration for the scrape endpoint. When set, wins over
# insecureSkipVerify. Use this to trust a real CA instead of skip-verify.
tlsConfig: {}
# Keep labels from the scraped target on collision with server-side labels
honorLabels: false
Expand Down
Loading