mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-12 23:54:01 +09:00
IvanHunters
0639ce39aa
## What this PR does A subchart nested under the vendored kamaji chart would deploy no matter what the values say, so this PR removes that directory in `make update` and adds a test that fails if anything renders from it. The vendored chart declares a `kamaji-etcd` dependency gated on `kamaji-etcd.deploy`, and the subchart itself is not vendored. The issue proposed pinning `kamaji-etcd.deploy: false` in the package values. I tested that, and it would not have helped: through this package the condition is never evaluated at all. Helm skips the condition because the umbrella chart declares no dependencies. `processDependencyEnabled` returns immediately when the chart it starts from has no `dependencies` in its `Chart.yaml`, and `cozy-kamaji` has none. The vendored `kamaji` chart is loaded from `charts/` without being declared, so Helm never recurses into it and never reads its `kamaji-etcd.deploy` condition. The same early return is in Helm v3 and v4. I put `kamaji-etcd-0.15.0.tgz` into `charts/kamaji/charts/` and rendered the package three ways: - default values: StatefulSet, Job, DataStore and four Certificates from `kamaji-etcd` - `kamaji.kamaji-etcd.deploy: false` in the package values: the same etcd - `deploy: false` edited into the vendored `charts/kamaji/values.yaml`: the same etcd Rendering `charts/kamaji` on its own with `kamaji-etcd.deploy=false` renders no etcd, which confirms the umbrella is what skips the condition. So any chart that ends up under `charts/kamaji/charts/` deploys whatever the values say, and no values change can close the hazard. This PR changes two things: - `make update` removes `charts/kamaji/charts` after extracting the upstream chart. Upstream does not ship that directory today, so the recipe still reproduces the current tree exactly. I checked that by running it on a copy of the package. I also ran it against a fake upstream tarball that carried the subchart: the directory was extracted and then removed. - The package gets its first helm-unittest suite. It asserts that no template of a subchart nested under `charts/kamaji/charts/` renders a document, and a `test` target lets `hack/helm-unit-tests.sh` pick it up in `make unit-tests`. With the subchart tgz dropped in, the suite fails. With the `rm` line removed from the recipe, the fake-upstream update leaves the tgz behind and the suite fails too. `/tests` is anchored in `.helmignore`, so the packaged chart's file list is unchanged. I left the `kamaji-etcd` block in the vendored values alone. It is not quite dead: `certmanager_issuer.yaml` reads it to decide whether to keep the self-signed issuer. That only matters when `certManager.issuerRef.name` is set, and this package does not set it. `helm template` output is byte-identical to main. Closes #4397 ### Screenshots Not a UI change. ### Downstream repositories The change touches only the kamaji package's update recipe, its test suite and its `.helmignore`. The closest trigger is package `Makefile`s → the website's `development.md`, but that page describes `make update` and `make test` in general terms that still hold, and the rendered chart is unchanged. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(kamaji): `make update` now drops any subchart nested under the vendored kamaji chart, and a unit test fails if one appears, because such a subchart (for example the bundled kamaji-etcd) would deploy regardless of `kamaji-etcd.deploy`. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Helm chart updates now avoid including nested charts that could deploy unexpectedly. * Helm chart packages exclude test files, keeping them out of distributed chart artifacts. * **Tests** * Added a chart check to verify nested charts render no resources. <!-- end of auto-generated comment: release notes by coderabbit.ai -->