HYPERFLEET-1362 - feat: add kube-prometheus-stack observability stack#74
HYPERFLEET-1362 - feat: add kube-prometheus-stack observability stack#74Ruclo wants to merge 1 commit into
Conversation
|
[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 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (8)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (7)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds opt-in observability configuration for Kind and GCP environments. Helmfile deploys Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant LocalSetup
participant Makefile
participant Helmfile
participant Kubernetes
LocalSetup->>Makefile: invoke local setup
Makefile->>Makefile: check OBSERVABILITY_ENABLED
Makefile->>Kubernetes: create Grafana credentials Secret
Makefile->>Helmfile: apply observability release
Helmfile->>Kubernetes: deploy kube-prometheus-stack
Kubernetes-->>Makefile: return monitoring release and pod status
Suggested reviewers: 🚥 Pre-merge checks | ✅ 10 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (10 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@helmfile/observability.yaml.gotmpl`:
- Around line 33-42: Gate the kube-prometheus-stack release in the Helmfile
template using the observability enablement flag, so it is rendered only when
OBSERVABILITY_ENABLED is exactly "true"; otherwise omit the release or fail
closed. Update the release definition under kube-prometheus-stack without
changing its existing chart, version, namespace, or values.
- Around line 28-37: Enable provenance verification for the
kube-prometheus-stack release by configuring Helm chart signature verification
with a trusted keyring and the appropriate verification setting. Keep the
existing chart repository and version pin unless switching this release to a
digest-pinned OCI artifact is required by the project’s available trust
configuration.
In `@helmfile/values/base-kube-prometheus-stack.yaml.gotmpl`:
- Around line 14-16: Update the Prometheus selector configuration in the
kube-prometheus-stack values to restrict discovery to HyperFleet-owned
ServiceMonitors, PodMonitors, and PrometheusRules. Replace the nil-selector
Helm-value overrides with explicit HyperFleet label selectors and an allowlisted
namespace selector, ensuring all three resource types are limited to the
intended namespaces and ownership labels.
In `@Makefile`:
- Around line 312-313: Update the cleanup target around the MONITORING_NAMESPACE
deletion so it never unconditionally removes a caller-selected namespace.
Require and validate a dedicated target-owned namespace, verify its ownership
label with kubectl before deletion, and skip namespace deletion when the label
is absent or does not match; retain secret cleanup without affecting unrelated
resources.
- Around line 291-292: Remove the hardcoded default from GRAFANA_ADMIN_PASSWORD
in the Makefile. Require the password to be supplied through approved secure
input or secret-manager integration, and fail closed when it is absent; keep
GRAFANA_ADMIN_USER unchanged.
- Around line 301-306: Validate and safely quote all external configuration at
each interpreter boundary: in Makefile lines 301-306, pass validated shell
variables with safe quoting and avoid placing passwords directly in command
arguments; in Makefile lines 311-313, quote HELMFILE_ENV and
MONITORING_NAMESPACE and validate the namespace before deletion; in Makefile
lines 443-448, quote MONITORING_NAMESPACE in status commands; and in
helmfile/observability.yaml.gotmpl lines 10-11, DNS-1123-validate and quote
MONITORING_NAMESPACE before rendering YAML.
- Line 311: Update the kube-prometheus-stack teardown command in the Makefile to
pass Helmfile’s --skip-charts option, ensuring destroy does not resolve remote
charts or templates while preserving the existing environment and component
selector.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 75388998-65a4-4111-93fe-1b220326b68d
📒 Files selected for processing (8)
Makefileenv.gcpenv.kindhelmfile/observability.yaml.gotmplhelmfile/values/base-adapter.yaml.gotmplhelmfile/values/base-api.yaml.gotmplhelmfile/values/base-kube-prometheus-stack.yaml.gotmplhelmfile/values/base-sentinel.yaml.gotmpl
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift-hyperfleet/architecture(manual)openshift-hyperfleet/hyperfleet-api(manual)openshift-hyperfleet/hyperfleet-sentinel(manual)openshift-hyperfleet/hyperfleet-adapter(manual)openshift-hyperfleet/hyperfleet-broker(manual)
|
|
||
| .PHONY: uninstall-grafana | ||
| uninstall-grafana: check-kubectl-context ## Uninstall kube-prometheus-stack | ||
| helmfile -f $(OBSERVABILITY_HELMFILE) -e $(HELMFILE_ENV) -l component=kube-prometheus-stack destroy |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files Makefile
wc -l Makefile
sed -n '280,330p' MakefileRepository: openshift-hyperfleet/hyperfleet-infra
Length of output: 1950
🌐 Web query:
helmfile destroy skip-charts docs destroy fetch charts templates
💡 Result:
In Helmfile, the --skip-charts flag is available for the destroy command to prevent the command from fetching and templating charts before performing the uninstallation [1][2][3]. By default, when you run helmfile destroy without this flag, Helmfile prepares all releases by fetching their charts and running templates [1][3]. This process can cause the destroy operation to fail if the charts are no longer accessible or if chart dependencies cannot be resolved [4]. To bypass this chart preparation step, you can execute the command with the flag: helmfile destroy --skip-charts When this flag is used, Helmfile proceeds to uninstall and purge the releases defined in your manifest without attempting to fetch or template the associated charts [1][2]. Note that helmfile destroy is designed to uninstall and purge all target releases, effectively running helm uninstall --purge for each [1][5].
Citations:
- 1: https://helmfile.readthedocs.io/en/stable/cli/
- 2: https://github.com/helmfile/helmfile/blob/ff60d0b5/cmd/destroy.go
- 3: https://github.com/helmfile/helmfile/blob/main/docs/cli.md
- 4: feat: skip chart preparation for local commands helmfile/helmfile#1664
- 5: https://github.com/helmfile/helmfile/blob/ff60d0b5/docs/index.md
🏁 Script executed (no clone):
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import urllib.request, re
url='https://helmfile.readthedocs.io/en/latest/cli/'
print('fetching', url)
data=urllib.request.urlopen(url, timeout=20).read().decode('utf-8', 'replace')
for pat in ['skip-charts', 'destroy', 'fetch charts', 'templates']:
print('\nPATTERN', pat)
for m in re.finditer(pat, data, re.I):
start=max(0, m.start()-200); end=min(len(data), m.end()+300)
print(data[start:end].replace('\n',' ')[:500])
break
PY
Length of output: 109
Decouple teardown from remote charts. helmfile destroy still resolves charts/templates here, so a repo outage or upstream chart change can block cleanup and keep this path tied to untrusted remote content (CWE-829/CWE-494). Add --skip-charts, or switch to a local pinned chart cache.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Makefile` at line 311, Update the kube-prometheus-stack teardown command in
the Makefile to pass Helmfile’s --skip-charts option, ensuring destroy does not
resolve remote charts or templates while preserving the existing environment and
component selector.
Sources: Path instructions, MCP tools
2c2f468 to
ca8e0d5
Compare
Add observability helmfile and Makefile targets for deploying kube-prometheus-stack, gated behind OBSERVABILITY_ENABLED env var to conditionally deploy ServiceMonitors and PrometheusRules for HyperFleet components. - Add helmfile/observability.yaml.gotmpl with kube-prometheus-stack release - Add install-grafana/uninstall-grafana targets, chained into local-up-kind/local-up-gcp - Add OBSERVABILITY_ENABLED toggle to env.kind and env.gcp - Enable ServiceMonitors for API, Sentinel, and Adapter when toggle is on - Enable PrometheusRules for API and Sentinel when toggle is on - Disable alertmanager and control plane scrapers - Store Grafana credentials in Kubernetes Secret - Add monitoring namespace to status target
ca8e0d5 to
11375b3
Compare
1 participant
Summary
helmfile/observability.yaml.gotmplOBSERVABILITY_ENABLEDenv var toggle (defaultfalse) controlling stack deployment and ServiceMonitor/PrometheusRule creationinstall-grafana/uninstall-grafanaMakefile targets, chained intolocal-up-kindandlocal-up-gcpDetails
The observability helmfile is separate from the main helmfile to keep the monitoring lifecycle independent —
install-hyperfleet/uninstall-hyperfleetwon't touch monitoring.When
OBSERVABILITY_ENABLED=true:install-grafanadeploys kube-prometheus-stack (Prometheus, Grafana, Operator, CRDs, node-exporter, kube-state-metrics)Alertmanager and control plane scrapers (etcd, scheduler, controller-manager, proxy) are disabled. Grafana admin credentials are stored in a Kubernetes Secret.
Test plan
OBSERVABILITY_ENABLED=true HELMFILE_ENV=kind make install-grafanadeploys the stackkubectl get pods -n monitoringshows Prometheus, Grafana, operator, node-exporter, kube-state-metrics runningOBSERVABILITY_ENABLED=true HELMFILE_ENV=kind make install-hyperfleetcreates ServiceMonitors and PrometheusRuleskubectl port-forward svc/kube-prometheus-stack-grafana 3000:80 -n monitoring) and verify HyperFleet metrics in ExploreHELMFILE_ENV=kind make uninstall-grafanatears down everything including namespace🤖 Generated with Claude Code