fix(prometheus-operator-crds): ship full upstream CRD bundle - #2660
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request updates the prometheus-operator-crds package to include the complete set of upstream CRDs. By removing the manual pruning of the CRD bundle and pinning the Helm chart version, the platform now provides a comprehensive set of monitoring resources. This change facilitates better interoperability for tenant applications that require standard Prometheus Operator CRDs, preventing conflicts and allowing for a cleaner management pattern within the cluster. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
github-actions
Bot
added
size/XS
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe Prometheus Operator CRDs Makefile is updated to pin the chart version to 25.0.0 and simplify the pull process by removing the post-extraction template cleanup step, instead starting with a complete directory removal and fresh extraction. ChangesCRDs Chart Update
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
dosubot
Bot
added
the
area/monitoring
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the prometheus-operator-crds package by pinning the Helm chart to version 25.0.0 and removing the manual filtering of CRD templates in the Makefile. The reviewer identified a missing release-note block in the PR description, which is required by the style guide, and suggested adding a command to remove the target directory before pulling the chart to ensure a clean state.
| helm repo update prometheus-community | ||
| helm pull prometheus-community/prometheus-operator-crds --untar --untardir charts | ||
| rm -f -- `find charts/prometheus-operator-crds/charts/crds/templates -maxdepth 1 -mindepth 1 | grep -v 'servicemonitor\|podmonitor\|prometheusrule\|probe'` | ||
| helm pull prometheus-community/prometheus-operator-crds --version 25.0.0 --untar --untardir charts |
There was a problem hiding this comment.
The pull request description is missing the required release-note block. According to the repository style guide (Rule 51), every PR must include a release note block in the following format:
```release-note
type(scope): human-readable changelog entry
<details>
<summary>References</summary>
1. PR body must contain a release note block. <sup>([link](https://github.com/cozystack/cozystack/blob/main/.gemini/styleguide.md))</sup>
</details>
| helm repo update prometheus-community | ||
| helm pull prometheus-community/prometheus-operator-crds --untar --untardir charts | ||
| rm -f -- `find charts/prometheus-operator-crds/charts/crds/templates -maxdepth 1 -mindepth 1 | grep -v 'servicemonitor\|podmonitor\|prometheusrule\|probe'` | ||
| helm pull prometheus-community/prometheus-operator-crds --version 25.0.0 --untar --untardir charts |
There was a problem hiding this comment.
To ensure a clean state and avoid stale files from previous versions or manual edits, it is recommended to remove the target directory before running helm pull.
rm -rf charts/prometheus-operator-crds
helm pull prometheus-community/prometheus-operator-crds --version 25.0.0 --untar --untardir charts
Aleksei Sviridkin (lexfrei)
removed
the
area/uncategorized
a720bdb to
f132741
Compare
github-actions
Bot
added
the
area/uncategorized
f132741 to
b3a3d5c
Compare
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
LGTM — restores the full upstream prometheus-operator CRD bundle through the package's own make update path; the change is mechanical, reproducible, version-pinned, and introduces no CRD-ownership conflicts.
Business context: the vendored bundle was pruned to the 4 service-discovery CRDs VictoriaMetrics operator needs. In tenant clusters that partial set blocks third-party apps shipping their own prometheus-operator (e.g. kube-prometheus-stack) from creating Prometheus/Alertmanager instances. Shipping the full 10-CRD bundle lets those apps set crds.enabled: false and consume the platform CRDs.
Verified
- Only the Makefile + 6 new CRD files change; the 4 pre-existing CRDs (
servicemonitors,podmonitors,prometheusrules,probes) are untouched → byte-identical, confirming the--version 25.0.0pin holds the operator CRDs atv0.87.0. helm templaterenders exactly 10monitoring.coreos.comCRDs.- None of the 6 added kinds (
Prometheus,Alertmanager,AlertmanagerConfig,PrometheusAgent,ScrapeConfig,ThanosRuler) are defined by any other package → no Flux ownership conflict. - The tenant CRD HelmRelease is reconciled by Flux helm-controller (not client-side
kubectl), so the large-CRD "metadata.annotations: Too long" failure mode does not apply. - The earlier suggestion to
rm -rfthe chart dir beforehelm pullis already implemented by the second commit.
Non-blocking follow-ups
- The tenant
prometheus-operator-crdsHelmRelease hasremediation.retries: -1and notimeout(defaults to 5m); this PR roughly 2.5×'s its payload (4→10 CRDs). CRD application is fast so 5m is very likely sufficient, but it is the same latent remediation-cycle shape that #2727 addresses for csi — atimeout: 10mhere would be consistent. - The PR body is missing the
release-noteblock (style-guide nit).
b3a3d5c to
01b7341
Compare
The chart previously stripped the bundle to 4 CRDs (ServiceMonitor, PodMonitor, PrometheusRule, Probe) — the set VictoriaMetrics operator needs for service discovery. This was correct when the chart was vendored inside system/victoria-metrics-operator (commit ba5c785, 2023), but the prune was carried over when the chart was extracted to its own package, and again when the CRDs were propagated into tenant Kubernetes clusters (commit 1e2b661, 2025). In tenant clusters the partial set blocks third-party apps that ship their own prometheus-operator (e.g. kube-prometheus-stack) — the missing CRDs (Prometheus, Alertmanager, AlertmanagerConfig, PrometheusAgent, ScrapeConfig, ThanosRuler) prevent the operator from deploying its instances against the platform-managed CRD set. Drop the rm -f line in the Makefile so the full upstream bundle ships, and pin --version 25.0.0 to keep the operator CRD version at v0.87.0 (operator version bump is a separate concern, out of scope here). Tenant apps can now set crds.enabled: false on their kube-prometheus- stack release and consume the platform CRDs directly. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Add `rm -rf charts/prometheus-operator-crds` before `helm pull` in the update target so stale files from previous chart versions don't linger across updates. This matches the convention used by cilium, external-secrets-operator, external-dns, and cert-manager Makefiles. Address review feedback from gemini-code-assist on packages/system/prometheus-operator-crds/Makefile:9. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
01b7341 to
97cf585
Compare
2 participants
Summary
rm -fline inpackages/system/prometheus-operator-crds/Makefileso the full upstream CRD bundle (10 CRDs) ships instead of the 4-CRD subset.helm pull --version 25.0.0to keep the operator CRD version atv0.87.0(operator version bump is intentionally out of scope here).make updateto repopulatecharts/crds/templates/with the 6 previously-stripped CRDs:Alertmanager,AlertmanagerConfig,Prometheus,PrometheusAgent,ScrapeConfig,ThanosRuler. The 4 already-vendored CRDs are byte-identical (version pin holds).Why
The bundle was originally trimmed to the 4 service-discovery CRDs that VictoriaMetrics operator needs (
ServiceMonitor,PodMonitor,PrometheusRule,Probe). That was correct when the chart was vendored insidesystem/victoria-metrics-operator(ba5c785cc, Dec 2023), but the prune was carried over verbatim when the chart was extracted to its own package, and again when the CRD release was propagated into tenant Kubernetes clusters (1e2b66131, Dec 2025).In tenant clusters the partial set blocks third-party apps that ship their own
prometheus-operator(e.g.kube-prometheus-stack): they cannot deployPrometheus/Alertmanagerinstances because the platform-managed CRD set is incomplete, and--server-side --force-conflictsworkarounds get reverted by Flux as it re-reconciles the narrower release manifest.With the full bundle shipping, tenant apps can set
crds.enabled: falseon theirkube-prometheus-stackrelease and consume the platform CRDs directly — the standard pattern for platform-level CRDs shared between multiple stacks.Test plan
helm template ./charts/prometheus-operator-crdsrenders 10CustomResourceDefinitionresources, all inmonitoring.coreos.comAPI group.helm template packages/apps/kubernetes -f tests/values/common.yamlrenders cleanly (75 kinds).helm unittest packages/apps/kubernetes— 6 suites, 123 tests, all pass.kube-prometheus-stackwithcrds.enabled: falseinstalls cleanly and the operator can managePrometheus/Alertmanagerinstances against the platform CRDs.Out of scope
v0.87.0— separate concern.HelmReleaseto a FluxKustomizationfor CRD application — bigger refactor; current Helm ownership is fine now that the bundle is complete and downstream apps can opt out of installing CRDs.Summary by CodeRabbit
25.0.0for consistent regeneration.