fix(kubernetes): gate vmop cert-manager-crds dependency on certManager.enabled by myasnikovdaniil · Pull Request #3130 · cozystack/cozystack

myasnikovdaniil · 2026-06-29T12:06:55Z

What this PR does

The victoria-metrics-operator HelmRelease in the kubernetes app chart is rendered when addons.monitoringAgents.enabled is true, but it listed cert-manager-crds in its dependsOn unconditionally. The cert-manager-crds HelmRelease is only created when addons.certManager.enabled is true. These are two independent toggles.

The valid combination that triggers it

addons.monitoringAgents.enabled: true + addons.certManager.enabled: false (both default-able, both schema-valid). In this configuration vmop renders but the dependency it points at never does.

Before / after

Before:

  dependsOn:
  - name: {{ .Release.Name }}-cilium
    namespace: {{ .Release.Namespace }}
  - name: {{ .Release.Name }}-cert-manager-crds   # always present
    namespace: {{ .Release.Namespace }}

After:

  dependsOn:
  - name: {{ .Release.Name }}-cilium
    namespace: {{ .Release.Namespace }}
  {{- if .Values.addons.certManager.enabled }}
  - name: {{ .Release.Name }}-cert-manager-crds
    namespace: {{ .Release.Namespace }}
  {{- end }}

The cascade

With cert-manager disabled, Flux reports on the vmop HelmRelease: unable to get '...cert-manager-crds' dependency: helmreleases... not found. Because vmop never goes Ready, the dependency chain fails behind it: monitoring-agents (dependsOn cozy-victoria-metrics-operator) → not Ready → vertical-pod-autoscaler (dependsOn monitoring-agents) → not Ready.

Why gating (not unconditional CRDs) is correct

vmop does not require cert-manager. The vendored vmop chart (packages/system/victoria-metrics-operator) defaults admissionWebhooks.certManager.enabled to false, and the cozystack wrapper does not override it. With that default the chart self-generates the webhook CA/cert (genCA/genSignedCert in _helpers.tpl); the cert-manager Issuer/Certificate path is fully gated behind certManager.enabled. So the dependency only belongs there when cert-manager is actually enabled.

Principle: a dependsOn must never reference a HelmRelease that isn't created under the same value combination.

How it was verified

helm template of the template with a CI values fixture:

certManager.enabled=false → cilium only:

  dependsOn:
  - name: test-cilium
    namespace: tenant-root

certManager.enabled=true → cilium + cert-manager-crds:

  dependsOn:
  - name: test-cilium
    namespace: tenant-root
  - name: test-cert-manager-crds
    namespace: tenant-root

A new helm unittest suite (tests/victoria-metrics-operator_test.yaml) asserts the dependsOn for both toggle states. Full package suite passes (13 suites / 125 tests).

Release note

fix(kubernetes): enabling monitoring agents without cert-manager no longer blocks the victoria-metrics-operator HelmRelease (and the monitoring-agents / vertical-pod-autoscaler chain behind it) on a non-existent cert-manager-crds dependency

Summary by CodeRabbit

New Features
- Added automated template test coverage for the Victoria Metrics Operator HelmRelease.
- Validates rendered release name and dependency behavior across key configuration scenarios.
Bug Fixes
- Updated the HelmRelease dependency wiring so the cert-manager CRDs dependency is included only when cert-manager is enabled.
- Preserved existing monitoring and outer conditional behavior while refining dependency rendering.

gemini-code-assist · 2026-06-29T12:07:05Z

Summary of Changes

Hello, I'm Gemini Code Assist¹! 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 resolves a dependency issue where the victoria-metrics-operator HelmRelease would fail to reconcile when cert-manager was disabled. By gating the cert-manager-crds dependency on the certManager configuration toggle, the operator can now successfully reach a Ready state in environments where cert-manager is not deployed, preventing a cascade of failures in the dependent monitoring and autoscaling stack.

Highlights

Dependency Gating: Updated the victoria-metrics-operator HelmRelease to conditionally include the cert-manager-crds dependency only when certManager is enabled.
Regression Testing: Added a new helm unittest suite to verify the dependency logic under different configuration states.

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.

Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	Gemini (@gemini-code-assist)	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

coderabbitai · 2026-06-29T12:07:12Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: efe7d813-db26-4cb3-a966-8760f9819694

📥 Commits

Reviewing files that changed from the base of the PR and between edb573b and 2b1d3e9.

📒 Files selected for processing (2)

packages/apps/kubernetes/templates/helmreleases/victoria-metrics-operator.yaml
packages/apps/kubernetes/tests/victoria-metrics-operator_test.yaml

🚧 Files skipped from review as they are similar to previous changes (2)

packages/apps/kubernetes/templates/helmreleases/victoria-metrics-operator.yaml
packages/apps/kubernetes/tests/victoria-metrics-operator_test.yaml

📝 Walkthrough

Walkthrough

The Victoria Metrics Operator HelmRelease now conditionally includes the cert-manager-crds dependency based on addons.certManager.enabled. A new helm-unittest suite checks the rendered release name, the cilium dependency, and cert-manager-crds presence or absence in both flag states.

Changes

Victoria Metrics Operator dependency gating

Layer / File(s)	Summary
Conditional dependsOn and template tests `packages/apps/kubernetes/templates/helmreleases/victoria-metrics-operator.yaml`, `packages/apps/kubernetes/tests/victoria-metrics-operator_test.yaml`	The HelmRelease template conditionally renders the cert-manager-crds dependsOn entry, and the test suite verifies the HelmRelease name, the cilium dependency, and cert-manager-crds inclusion or omission based on `addons.certManager.enabled`.

Estimated code review effort: 2 (Simple) | ~5 minutes

Possibly related PRs

cozystack/cozystack#3141: Also changes Flux HelmRelease spec.dependsOn wiring around cozy-victoria-metrics-operator.

Suggested labels: area/monitoring

Suggested reviewers: kvaps, androndo, IvanHunters, lllamnyp

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main fix: gating the vmop cert-manager-crds dependency on certManager.enabled.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/kubernetes-vmop-certmanager-dep

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

gemini-code-assist

Code Review

This pull request gates the cert-manager-crds dependency in the victoria-metrics-operator HelmRelease on the addons.certManager.enabled value. It also introduces a comprehensive test suite to verify that the dependency is correctly included or excluded based on this setting. There are no review comments, and I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

…r.enabled The victoria-metrics-operator HelmRelease is gated on monitoringAgents.enabled but listed cert-manager-crds in dependsOn unconditionally, while the cert-manager-crds HelmRelease is only created when certManager.enabled is true. The valid combination monitoringAgents.enabled=true + certManager.enabled=false therefore left vmop blocked on a non-existent dependency ("...cert-manager-crds ... not found"), cascading to monitoring-agents and vertical-pod-autoscaler. Gate the dependency on the same condition that creates it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>

IvanHunters

Verdict

LGTM

The change gates the cert-manager-crds entry in the vmop HelmRelease dependsOn on the exact same addons.certManager.enabled toggle that gates the cert-manager-crds HelmRelease itself; verified correct across both toggle states, on upgrade and fresh install, with matching test coverage.

Caveats

Root-cause verified: templates/helmreleases/cert-manager-crds.yaml:1 gates the target HR on and .Values.addons.certManager.enabled .Values._namespace.etcd, while the pre-fix victoria-metrics-operator.yaml (gated only on monitoringAgents.enabled + _namespace.etcd) referenced it unconditionally. With monitoringAgents.enabled=true + certManager.enabled=false (both default to false in values.yaml:180,201, so this is a valid, schema-reachable combination) vmop pointed at a HelmRelease that is never rendered. The fix at templates/helmreleases/victoria-metrics-operator.yaml:34-37 is the idiomatic Flux resolution.
Cascade claim confirmed: vertical-pod-autoscaler.yaml dependsOn monitoring-agents, monitoring-agents.yaml dependsOn cozy-victoria-metrics-operator. A stuck vmop does block the whole monitoring to VPA chain. grep -rn cert-manager-crds templates/ shows vmop was the only mis-gated consumer; the sibling cert-manager.yaml:45-46 reference is itself inside a block gated on certManager.enabled, so it is consistent.
vmop genuinely does not need cert-manager: the vendored chart default admissionWebhooks.certManager.enabled: false (packages/system/victoria-metrics-operator/charts/victoria-metrics-operator/values.yaml:388) is not overridden by the cozystack wrapper (packages/system/victoria-metrics-operator/values.yaml), so the webhook CA is self-generated. Gating (rather than making the CRDs HR unconditional) is the correct direction.
Phase 5b, upgrade (scenario A): no state migration, no CRD/RBAC/Secret/PVC/image/default-value change. For certManager=true clusters the rendered dependsOn is byte-identical (both entries present); for certManager=false clusters the broken entry is simply dropped and a previously-stuck vmop becomes reconcilable. No numbered migration script needed and migrations.targetVersion correctly left unchanged. No one-way state change, clean rollback.
Phase 5b, fresh install (scenario B): cold-start dependency graph strictly improves (vmop no longer awaits a non-existent HR when cert-manager is off; cilium dep retained). No new PackageSource/bundle/cozyrds/_cluster/_namespace keys. Only a template and a test file are touched (files.txt), so no make generate obligation and no values.schema.json/README drift.
Tests verified by execution: the new suite tests/victoria-metrics-operator_test.yaml asserts both toggle states with precise lengthEqual counts (1 without cert-manager, 2 with), backed by notContains/contains on the exact dependency name. helm unittest . passes 15 suites / 144 tests locally, no regression. The suite's tests/values/common.yaml sets _namespace.etcd: cozy-etcd, satisfying the outer render guard so the assertions are meaningful.
Envelope is clean: 0 cozy_invariants, 0 repo_invariants, 0 warnings. The one chart_lint render_error is in talos-reconcile-job.yaml:487 from empty runtime-injected _cluster values during lint (a known non-bug) and is unrelated to the touched files.

Recommended follow-ups

The PR body states "Full package suite passes (13 suites / 125 tests)", but the current tree runs 15 suites / 144 tests including the new one. Cosmetic staleness in the description only; no code impact.

github-actions · 2026-07-03T09:07:10Z

Successfully created backport PR for release-1.5:

[Backport release-1.5] fix(kubernetes): gate vmop cert-manager-crds dependency on certManager.enabled #3185

github-actions · 2026-07-03T09:07:10Z

Successfully created backport PR for release-1.4:

[Backport release-1.4] fix(kubernetes): gate vmop cert-manager-crds dependency on certManager.enabled #3186

…ependency on certManager.enabled (#3186) # Description Backport of #3130 to `release-1.4`.

…ependency on certManager.enabled (#3185) # Description Backport of #3130 to `release-1.5`.

myasnikovdaniil requested review from IvanHunters, Andrey Kolkov (androndo), Andrei Kvapil (kvaps), Aleksei Sviridkin (lexfrei), Timofei Larkin (lllamnyp) and Kirill Ilin (sircthulhu) as code owners June 29, 2026 12:06

github-actions Bot added area/kubernetes Issues or PRs related to the tenant Kubernetes app kind/bug Categorizes issue or PR as related to a bug labels Jun 29, 2026

github-actions Bot added the size/M This PR changes 30-99 lines, ignoring generated files label Jun 29, 2026

dosubot Bot added the area/testing Issues or PRs related to testing (e2e, bats, unit tests) label Jun 29, 2026

gemini-code-assist Bot reviewed Jun 29, 2026

View reviewed changes

myasnikovdaniil force-pushed the fix/kubernetes-vmop-certmanager-dep branch from edb573b to 2b1d3e9 Compare July 3, 2026 06:30

IvanHunters approved these changes Jul 3, 2026

View reviewed changes

myasnikovdaniil added backport Should change be backported on previous release backport-previous Backport target — previous release line labels Jul 3, 2026

myasnikovdaniil merged commit 5b261b9 into main Jul 3, 2026
21 checks passed

myasnikovdaniil deleted the fix/kubernetes-vmop-certmanager-dep branch July 3, 2026 09:06

This was referenced Jul 3, 2026

[Backport release-1.5] fix(kubernetes): gate vmop cert-manager-crds dependency on certManager.enabled #3185

Merged

[Backport release-1.4] fix(kubernetes): gate vmop cert-manager-crds dependency on certManager.enabled #3186

Merged

myasnikovdaniil added a commit that referenced this pull request Jul 3, 2026

[Backport release-1.4] fix(kubernetes): gate vmop cert-manager-crds d…

11339aa

…ependency on certManager.enabled (#3186) # Description Backport of #3130 to `release-1.4`.

myasnikovdaniil added a commit that referenced this pull request Jul 3, 2026

[Backport release-1.5] fix(kubernetes): gate vmop cert-manager-crds d…

3e25f19

…ependency on certManager.enabled (#3185) # Description Backport of #3130 to `release-1.5`.

Navigation Menu

Uh oh!

fix(kubernetes): gate vmop cert-manager-crds dependency on certManager.enabled - #3130

fix(kubernetes): gate vmop cert-manager-crds dependency on certManager.enabled#3130
myasnikovdaniil merged 1 commit into
mainfrom
fix/kubernetes-vmop-certmanager-dep

myasnikovdaniil commented Jun 29, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

gemini-code-assist Bot commented Jun 29, 2026

Uh oh!

coderabbitai Bot commented Jun 29, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

IvanHunters left a comment

Uh oh!

Uh oh!

github-actions Bot commented Jul 3, 2026

Uh oh!

github-actions Bot commented Jul 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

myasnikovdaniil commented Jun 29, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does

The valid combination that triggers it

Before / after

The cascade

Why gating (not unconditional CRDs) is correct

How it was verified

Release note

Summary by CodeRabbit

Summary by CodeRabbit

Uh oh!

gemini-code-assist Bot commented Jun 29, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

coderabbitai Bot commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

IvanHunters left a comment

Choose a reason for hiding this comment

Verdict

Caveats

Recommended follow-ups

Uh oh!

Uh oh!

github-actions Bot commented Jul 3, 2026

Uh oh!

github-actions Bot commented Jul 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

myasnikovdaniil commented Jun 29, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 29, 2026 •

edited

Loading