mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-09 05:25:14 +09:00
fix(kubernetes): gate vmop cert-manager-crds dependency on certManager.enabled (#3130)
## 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:
```yaml
dependsOn:
- name: {{ .Release.Name }}-cilium
namespace: {{ .Release.Namespace }}
- name: {{ .Release.Name }}-cert-manager-crds # always present
namespace: {{ .Release.Namespace }}
```
After:
```yaml
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:
```yaml
dependsOn:
- name: test-cilium
namespace: tenant-root
```
`certManager.enabled=true` → cilium + cert-manager-crds:
```yaml
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
```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
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
## 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.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
2 files changed
+79
No files matched your search
@@ -31,6 +31,8 @@ spec:
|
||||
dependsOn:
|
||||
- name: {{ .Release.Name }}-cilium
|
||||
namespace: {{ .Release.Namespace }}
|
||||
{{- if .Values.addons.certManager.enabled }}
|
||||
- name: {{ .Release.Name }}-cert-manager-crds
|
||||
namespace: {{ .Release.Namespace }}
|
||||
{{- end }}
|
||||
{{- end }}
|
||||
@@ -0,0 +1,77 @@
|
||||
suite: victoria-metrics-operator.yaml cert-manager-crds dependency gating
|
||||
|
||||
templates:
|
||||
- templates/helmreleases/victoria-metrics-operator.yaml
|
||||
|
||||
values:
|
||||
- values/common.yaml
|
||||
|
||||
# The vmop HelmRelease is gated on monitoringAgents.enabled, but the
|
||||
# cert-manager-crds HelmRelease it can depend on is only created when
|
||||
# certManager.enabled is true. The dependsOn entry must therefore be gated on
|
||||
# the same condition, otherwise the valid combination
|
||||
# monitoringAgents.enabled=true + certManager.enabled=false leaves vmop blocked
|
||||
# on a non-existent dependency.
|
||||
|
||||
tests:
|
||||
- it: renders the vmop HelmRelease when monitoringAgents is enabled
|
||||
release:
|
||||
name: kubernetes-test
|
||||
namespace: tenant-test
|
||||
set:
|
||||
addons.monitoringAgents.enabled: true
|
||||
asserts:
|
||||
- hasDocuments:
|
||||
count: 1
|
||||
- isKind:
|
||||
of: HelmRelease
|
||||
- equal:
|
||||
path: metadata.name
|
||||
value: kubernetes-test-cozy-victoria-metrics-operator
|
||||
|
||||
- it: always depends on cilium
|
||||
release:
|
||||
name: kubernetes-test
|
||||
namespace: tenant-test
|
||||
set:
|
||||
addons.monitoringAgents.enabled: true
|
||||
asserts:
|
||||
- contains:
|
||||
path: spec.dependsOn
|
||||
content:
|
||||
name: kubernetes-test-cilium
|
||||
namespace: tenant-test
|
||||
|
||||
- it: does NOT depend on cert-manager-crds when certManager is disabled
|
||||
release:
|
||||
name: kubernetes-test
|
||||
namespace: tenant-test
|
||||
set:
|
||||
addons.monitoringAgents.enabled: true
|
||||
addons.certManager.enabled: false
|
||||
asserts:
|
||||
- notContains:
|
||||
path: spec.dependsOn
|
||||
content:
|
||||
name: kubernetes-test-cert-manager-crds
|
||||
namespace: tenant-test
|
||||
- lengthEqual:
|
||||
path: spec.dependsOn
|
||||
count: 1
|
||||
|
||||
- it: depends on cert-manager-crds when certManager is enabled
|
||||
release:
|
||||
name: kubernetes-test
|
||||
namespace: tenant-test
|
||||
set:
|
||||
addons.monitoringAgents.enabled: true
|
||||
addons.certManager.enabled: true
|
||||
asserts:
|
||||
- contains:
|
||||
path: spec.dependsOn
|
||||
content:
|
||||
name: kubernetes-test-cert-manager-crds
|
||||
namespace: tenant-test
|
||||
- lengthEqual:
|
||||
path: spec.dependsOn
|
||||
count: 2
|
||||
Reference in new issue
Block a user