feat(clickhouse): add configurable version parameter - #3476

Actionable comments posted: 1

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 `@packages/apps/clickhouse/hack/update-versions.sh`:
- Around line 54-99: Make the supported-version resolution loop fail immediately
when any configured entry in SUPPORTED_MAJORS has no matching tag, instead of
warning and continuing. Ensure the failure occurs before writing VERSIONS_FILE
or updating the generated values.yaml section, preserving all configured
versions and the existing default such as v24.9.

Review: request changes

The runtime chart change is sound: the version enum feeds only the image tag, the default v24.9 renders 24.9.2.42 byte-identical to the previous hardcoded tags (so existing installs need no migration), it is not a mutable-identity field (no metadata.name/selector/content-hash derives from it), and helm unittest passes 17/17. The pattern correctly mirrors the sibling DB charts.

The blocking items are all in the hack/update-versions.sh generator and the reproducibility of the committed files/versions.yaml, not in the rendered chart.

Blocking

1. comm is fed version-sorted input and silently drops common tags.
fetch_tags ends with sort -Vu, and that version-sorted output is passed to comm -12. comm requires inputs sorted in the collation order it walks (lexicographic), so it mis-compares at every X.9 -> X.10 boundary where version order and lexicographic order diverge.

Reproduction:

server (sort -V): 24.9.2.42, 24.10.1.1, 25.3.14.14
keeper (sort -V): 24.10.1.1, 25.3.14.14
comm -12 (version-sorted, as the script does)  => 25.3.14.14        # 24.10.1.1 lost, exit 0, no warning
comm -12 (lexicographically sorted, correct)   => 24.10.1.1, 25.3.14.14

Consequence: on the next make update, a whole major line can drop out of COMMON_TAGS, the script prints only Warning: no tag found for X.Y, skipping, and that major disappears from the enum. Existing tenant CRs pinned to the dropped version then fail schema validation. Fix is one line: feed comm a plain sort -u (lexicographic) intersection, and keep sort -V only for the per-major tail -n1 pick, which already does it.

2. The committed files/versions.yaml is not reproducible by its own generator.
The file pins v24.9 -> 24.9.2.42, but the latest patch common to both clickhouse/clickhouse-server and clickhouse/clickhouse-keeper on the 24.9 line is 24.9.3.128 (verified against Docker Hub, present in both repos). The generator selects the latest patch per major (sort -V | tail -n1), so the first real make update would write v24.9 -> 24.9.3.128, changing the image for every default-version install and turning tests/version_test.yaml red (it hardcodes 24.9.2.42).

The PR body only promises to preserve the major, which is honoured, but the "renders byte-for-byte identically" guarantee and the test assertion both rest on a hand-pinned patch the generator will not reproduce. Please either add a patch-pin mechanism for the default line, or have version_test.yaml derive the expected tag from files/versions.yaml instead of hardcoding it, so the generator and the committed artifacts cannot drift apart.

Should fix (robustness)

3. Non-atomic regeneration. update-versions.sh writes versions.yaml before it edits values.yaml, and make generate is a separate step. A failure in between (see item 4) leaves versions.yaml updated while the enum/schema/README/types.go are stale. Collect all outputs into temp files and apply them at the end.

4. Does not run on stock macOS. declare -A requires bash 4+ (macOS ships 3.2), and BSD awk rejects -v new_section=... with embedded newlines (awk: newline in string). Combined with item 3 this corrupts the tree mid-run. If make update requires GNU awk + bash 4, state it; otherwise make the script portable.

Non-blocking notes

• Downgrade hazard. version is editable with no ordering guard, so v25.8 -> v24.9 is reachable and schema-valid. ClickHouse cannot read data written by a newer server, and Keeper snapshots are not backward compatible, so a downgrade lands the pods in CrashLoopBackOff with no legible "downgrade unsafe" signal. This matches existing precedent (postgres et al. have the same gap), so a CEL immutability guard is not required, but please add a warning to the parameter description.
• Default v24.9 is an EOL non-LTS line (LTS are 25.3 and 25.8). Preserving it for existing installs is correct, but fresh installs get an EOL default by omission.
• templates/_versions.tpl fail branch is effectively dead code behind schema-enum validation (harmless defense-in-depth).
• tests/version_test.yaml asserts on the Helm-4 jsonschema error string, which is brittle across helm versions in CI.
• The server/Keeper image references are neither digest-pinned nor routed through cozy-lib.image, so mirrored/air-gapped installs cannot rewrite the registry. Pre-existing, but the new version-map indirection is a natural place to add mirror routing.

LGTM with non-blocking notes.

Clean, well-tested feature that faithfully mirrors the established managed-DB
version-selection pattern (postgres/mariadb/redis/mongodb/opensearch/rabbitmq).
Verified by hand, not just by reading:

Verified

• Upgrade of existing installations is byte-for-byte safe: a CR with no
  spec.version renders 24.9.2.42 via the chart default, and an explicit
  empty value is fail-closed by the schema enum.
• All generated artifacts are consistent and up to date: values.yaml,
  values.schema.json, types.go, the clickhouse-rd openAPISchema and
  README.md all carry the same v25.8/v25.3/v24.9 enum with default v24.9;
  re-running cozyvalues-gen produces an empty diff.
• Tests pass: helm-unittest 5/5 and the bats generator suite 7/7 (including the
  byte-collation invariant across the X.9 -> X.10 boundary and the pinned-default
  reproduction).

Non-blocking notes (all scoped to the maintainer-run hack/update-versions.sh,
whose output is always reviewed via git diff; none affect what this PR ships):

1. update-versions.sh (default fallback): if the current default's major is
   removed from CH_SUPPORTED_MAJORS, DEFAULT_VERSION="${MAJORS[0]}" silently
   promotes the newest major as the new default. Losing the current default would
   be safer as a hard error than a silent fallback.
2. pin_for is keyed to a hardcoded v24.9 rather than to "whichever major is
   the default", so if the default ever moves it becomes unpinned and subsequent
   regenerations can patch-bump its image.
3. The values.yaml splice loop only clears in_section on a version: line, so
   a malformed input (enum header present, version: line absent) would swallow
   everything to EOF. Cannot happen with the well-formed committed file; worth a
   bounded terminator for robustness.

Not raised as a blocker: there is no CEL transition guard preventing a version
downgrade (the field doc says "only increase"), but this matches every sibling
DB chart, so it belongs to a family-wide change rather than this PR.

The note about terraform-provider-cozystack needing a matching version
attribute is correct and reasonably left as a maintainer decision.

This closes the ClickHouse half of the #1246 gap: a selectable version (v25.8 / v25.3 / v24.9, default v24.9) exposed through the API like the other managed databases. Scope is tight, the version generator is atomic and correct on collation, the upgrade path is byte-identical, and the tests are thorough and pass.

Verified without a cluster:

• No regression: the default v24.9 → 24.9.2.42 matches the previous hardcoded tag byte-for-byte, so existing instances do not silently upgrade. No hardcoded ClickHouse tags remain; both server and keeper images resolve through versionMap.
• Tests run and are green: 10/10 bats for the version generator (hack/clickhouse-update-versions_test.bats, auto-picked up via wildcard hack/*.bats) and 5/5 helm-unittest, including schema rejection of v1.0.
• The implementation mirrors the established sibling pattern (mariadb), with higher coverage since it is the only DB chart with a generator bats test.

Non-blocking notes for the maintainer:

• [MINOR] The version default is the oldest enum member (v24.9), whereas every other managed-DB chart defaults to its own newest major (mariadb v11.8, mongodb v8, redis v8, postgres v18, opensearch v2). Fresh instances come up on a ~year-old release unless the user picks v25.8. If this is a deliberate upgrade-safety choice, worth confirming for fresh installs specifically.
• [MINOR] No downgrade guard on version, even though the field docstring says downgrading is unsafe. This matches every sibling DB chart (none guard version), so it is a pre-existing platform stance rather than a regression here.
• [NIT] The downstream terraform-provider-cozystack is not updated; the PR already flags this.

Two blockers, both in hack/update-versions.sh, inline.

Generator cannot see the tags it needs, and when it half sees them it silently moves the default image. Fix for both is small: fetch with skopeo list-tags like the postgres, redis, mongodb, opensearch and kubernetes generators already do (skopeo is already required in root build-deps), and make a missing frozen tag a hard error instead of a fallback.

Chart part itself is fine and I checked it: default render is byte identical to main, v25.3 and v25.8 move both server and keeper images together, schema rejects an unknown value, bats suite passes under hack/cozytest.sh, helm-unittest is 23/23 and it does catch a broken map (mutated _versions.tpl to confirm).

Not blocking:

• .helmignore has no /hack, so update-versions.sh ships inside the chart tarball. postgres, mariadb and redis all exclude it.
• default line freezes its patch and not only its major, so default installs never get patch fixes for their own line, every other chart maps major to latest patch. v24.9 is not lts either, its last patch is 2024-11-19 and the map pins it two patches back.
• successful run leaves files/versions.yaml and values.yaml at mode 600, mktemp mode survives the mv.
• nothing boots 25.3 or 25.8. helm-unittest checks rendered tags only and the chainsaw fixture has no version. No other database chart does this either so not asking for it here.
• unrelated to this PR: templates/clickhouse.yaml:99 shadows $clusterDomain with .Values.clusterDomain which the chart does not define, so keeper hosts render as chk-...-0.<ns>.svc. with an empty domain. Separate issue.

Both blockers fixed, verified on e707475.

Ran the generator live against the registry: v25.8 to 25.8.32.4, v25.3 to 25.3.14.14, v24.9 to 24.9.2.42, exit 0, and regeneration leaves zero diff against the committed tree, so the map is exactly what the generator produces now. That was not checkable before because the generator could not complete at all. Missing frozen tag is a hard error with nothing written (checked with committed map at 24.9.2.42 and a tag list holding only 24.9.3.128). Modes stay 644, hack/ is out of the chart tarball, bats 11/11 under hack/cozytest.sh, helm-unittest 23/23, default render still byte identical to main, v25.3 and v25.8 both move server and keeper together.

On the freeze question, keep it. The danger was never the freeze itself, it was that the freeze could move the default silently, and that is gone now. Sibling charts move the default major to newest on every regeneration with no signal at all, which is worse than what you have here. The real problem is a different one: v24.9 is not lts and its last patch is 2024-11-19, so the default ships an abandoned line. That is a question about which version cozystack ships, and it wants its own PR with an upgrade note rather than a change to the generator.

Two things I am not asking for here:

• files/versions.yaml is invisible to hack/lib/image-refs.sh, image_ref_files walks only packages/*/*/values.yaml and images/*.tag. Same for all seven charts that carry a version map, so it is one glob elsewhere and not yours.
• nothing boots 25.3 or 25.8 anywhere, helm-unittest checks rendered tags only. No database chart in the tree does this, so it is separate work.

Unit and e2e checks are still running on this head. I ran locally what the unit job runs and both suites pass, and this commit does not touch anything e2e exercises.