Skip to content

fix: added servicemonitor support. - #628

Merged
pantierra merged 1 commit into
mainfrom
feat/servicemonitor-support
Sep 17, 2026
Merged

pantierra merged 1 commit into
mainfrom
feat/servicemonitor-support

Conversation

@pantierra

Copy link
Copy Markdown
Contributor

No description provided.

@claude

claude Bot commented Sep 17, 2026

Copy link
Copy Markdown

Claude finished @pantierra's task in 1m 37s —— View job


✅ No blocking issues — safe to merge.

Reviewed the diff against origin/main (14 files, ServiceMonitor support for raster/stac/vector + stac-auth-proxy).

Findings (non-blocking)

  • charts/eoapi/templates/core/stac-auth-proxy-servicemonitor.yaml:14-16 — the selector matches app.kubernetes.io/name: stac-auth-proxy / app.kubernetes.io/instance: {{ .Release.Name }}, which relies on the vendored stac-auth-proxy OCI subchart (ghcr.io/developmentseed/stac-auth-proxy/charts, v1.2.0) applying the standard Helm label convention to its Service. That subchart isn't vendored in this repo so I couldn't verify the labels directly. Worth a quick helm template smoke test against the real subchart to confirm the ServiceMonitor actually matches the Service (silent no-match is not the kind of thing that fails loudly — hasDocuments: count: 0-style assertions in the test suite won't catch a selector that renders but matches nothing).
  • charts/eoapi/templates/services/{raster,stac,vector}/servicemonitor.yaml — selectors use app: {{ .Release.Name }}-<svc>, which does match the corresponding service.yaml's own metadata.labels (confirmed by reading each service.yaml), so these three are correct and consistent with each other.
  • Good defensive pattern: gating every ServiceMonitor on .Capabilities.APIVersions.Has "monitoring.coreos.com/v1/ServiceMonitor" avoids hard-failing installs where the CRD isn't present — and the test suite (servicemonitor_test.yaml) explicitly covers the CRD-absent, metrics-disabled, and enabled-with-CRD paths for all four resources, which is solid coverage for a feature that's easy to get silently wrong.
  • The spec.ports[0].name: http addition to the three service.yaml files is required for the ServiceMonitor's endpoints[0].port: http to resolve (Kubernetes Endpoints need named ports for ServiceMonitor port-by-name lookups) — correct and consistent, and covered by the corresponding test additions in raster_tests.yaml/stac_tests.yaml/vector_tests.yaml.
  • values.schema.json — the new serviceMonitor block was added once to the shared definitions.apiService fragment (referenced by raster/stac/vector via $ref), not duplicated three times — good.

Simplify (ponytail)

Nothing to cut — this is a straightforward, minimal-diff feature: one new template + one values block per service, reusing the existing metrics.enabled gate and an existing shared JSON-schema definition rather than introducing new abstractions.


💰 Estimated review cost: $0.54 · 1m37s · 33 turns

@pantierra
pantierra merged commit 2b24c3a into main Sep 17, 2026
5 checks passed
@pantierra
pantierra deleted the feat/servicemonitor-support branch September 17, 2026 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant