[jordan-simonovski]

Adds template coverage, generated chart READMEs and a Makefile for the local test loop. Before this, nothing showed which template branches the unit tests exercised, and the chart values had no reference docs.

• make test, make coverage, make docs and make integration run the same checks as CI. make setup installs the tools and a pre-commit hook that runs the tests and docs check when chart files change.
• CI runs helmcov on each PR and posts line and branch coverage as a comment. The job fails below 99% line or 90% branch coverage; the chart is at 99.85% and 91.01%, so a new template or branch without tests fails it.
• charts/*/README.md is generated from # -- comments in values.yaml by helm-docs. CI fails when the README is out of date. Only commented keys appear, so new user-facing values need a # -- comment.
• 25 new unit test suites cover the templates helmcov showed as untested.

Breaking changes: none. The installed chart is unchanged. values.yaml only gains comments, and tests/ and README.md.gotmpl are kept out of the packaged chart.

Implementation detail

• Tool versions are pinned in scripts/tool-versions.env. make coverage downloads the helmcov release binary into .tools/ and checks it against a pinned sha256 of the release's checksums.txt. The CI action is pinned by commit SHA.
• make validate renders every examples/*/values.yaml, so new examples are picked up without editing the Makefile.
• make integration runs each Kind suite in its own test-<suite> cluster and deletes it afterwards. Tested locally with api-only.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 10722c4

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[CLAassistant]

All committers have signed the CLA.

[github-actions]

Deep Review

✅ No critical issues found. This PR changes no runtime Helm templates (only values.yaml comment reformatting, new tests, scripts, Makefile, and CI). No template fails to render, no secret is introduced, and no default guarantees a broken deployment. The findings below are supply-chain/governance hardening and dev-workflow robustness for the new helmcov/helm-docs tooling, plus test-quality and standards nits.

🟡 P2 — recommended

• .github/workflows/helmcov.yaml:39 — the new PR-triggered workflow runs third-party action jordan-simonovski/helmcov@v1 (mutable major tag, personal namespace) with pull-requests: write, so a retag or account compromise executes attacker code in the trusted same-repo context.
  • Fix: pin the action to a full commit SHA and mirror it into the ClickHouse org; fork-PR tokens are already read-only under the pull_request trigger, so the residual risk is the trusted-context path.
  • _{security, adversarial}
• scripts/tool-versions.env:8 — the entire helmcov coverage gate (CI action and local make coverage image) depends on a personal jordan-simonovski namespace; deletion or visibility change breaks both, as the file's own TODO acknowledges.
  • Fix: mirror the digest-pinned image and the action into ghcr.io/clickhouse before this gate becomes load-bearing.
  • _{maintainability, adversarial, security}
• .github/workflows/helmcov.yaml:39 — the helmcov binary version is pinned independently in two places, the image digest in scripts/tool-versions.env versus the action version: v0.4.0 at @v1, so local and CI runs can silently use different binaries.
  • Fix: drive both from a single source of truth or document that the action's version input matches the pinned digest.
  • _{maintainability}
• .githooks/pre-commit:31 — a contributor without Helm/Docker staging even a trivial charts/ change is hard-rejected, because make test → chart-deps runs command -v helm which exits non-zero under set -e, and the hook also shells into a network helm-docs download.
  • Fix: warn and exit 0 when helm/docker are absent instead of failing the commit.
  • _adversarial
• charts/clickstack/tests/values/deployment-tpl-defaults.yaml:1 — the ~100-line defaultConnections/defaultSources JSON blob is copied verbatim from values.yaml with no sync enforcement, while the test asserts only two regex lines, creating a silent drift trap.
  • Fix: trim the fixture to the minimal connection and log source the two assertions actually exercise.
  • _{maintainability}

🔵 P3 nitpicks (8)

• .githooks/pre-commit:22 — the inner case branches scripts/helmdocs.sh|scripts/install-helm-docs.sh sit inside the outer charts/*) arm and are unreachable, so editing only those scripts never triggers make docs.
  • Fix: evaluate those two patterns in an outer-level case arm independent of charts/*.
• .githooks/pre-commit:30 — editing charts/clickstack-operators/ sets chart_changes=true and runs make test, but make test hardcodes CHART=charts/clickstack, so the operators chart is reported as tested while nothing renders or asserts it.
  • Fix: loop test/validate over all charts, or document that operators is intentionally untested.
• scripts/install-helm-docs.sh:41 — the helm-docs release tarball is fetched over curl -fsSL with no checksum or signature verification and then executed, now reachable in CI via make setup.
  • Fix: verify the download against a pinned SHA256 from the release checksums before install/exec.
• scripts/helmcov.sh:13 — the coverage threshold 30 is duplicated across Makefile, scripts/helmcov.sh, and .github/workflows/helmcov.yaml.
  • Fix: define the threshold once (e.g. in scripts/tool-versions.env) and reference it everywhere.
• AGENTS.md:210 — the new scripts use #!/usr/bin/env bash + set -euo pipefail, diverging from the "Shell Script" conventions (#!/bin/bash, set -e, set -o pipefail) that this PR edits but leaves unchanged.
  • Fix: update the conventions section to permit the new style, or align the new scripts with the documented standard.
• charts/clickstack/tests/additional-manifests-multi_test.yaml:11 — the second rendered manifest (the ConfigMap) is only counted, never asserted, so its tpl rendering is unverified.
  • Fix: add isKind/equal assertions at documentIndex: 1 for the ConfigMap name and data.
• charts/clickstack/tests/deployment-features_test.yaml:17 — matchRegex with the unanchored substring custom-init is weaker than an exact match when the document index already pins the container.
  • Fix: replace with equal value custom-init.
• charts/clickstack/tests/tasks-with-args_test.yaml:9 — the fixture declares two additionalArgs (concurrency, sourceTimeoutMs) but only --concurrency/4 is asserted.
  • Fix: assert --sourceTimeoutMs/90000 too, or drop the unused fixture arg.

---

Reviewers (6): correctness, testing, maintainability, project-standards, adversarial, security.

Testing gaps:

• The README drift gate (helm-test.yaml runs make setup test docs then git diff --exit-code on both chart READMEs) could not be executed here; confirm the committed READMEs were generated by exactly helm-docs v1.14.2 with --badge-style flat --ignore-non-descriptions.
• The new dev/CI tooling (Makefile, scripts/*.sh, .githooks/pre-commit, load-tool-versions.sh) has no automated tests; the unreachable pre-commit branch and operators-not-tested gap would be caught by a small shell test.
• Two low-confidence items were suppressed by the confidence gate: scripts/helmcov.sh forcing --platform linux/amd64 on arm64 dev machines, and the # -- helm-docs convention not applied to the top-level hyperdx: key.

[github-actions]

Helm template coverage

Metric	Value
Line coverage	99.85%
Branch coverage	91.01%
Line threshold	99% (met)
Branch threshold	90% (met)

Uncovered details

charts/clickstack/templates/NOTES.txt

Uncovered lines: none

Uncovered branches: 37:default:fallback

charts/clickstack/templates/_helpers.tpl

Uncovered lines: 17

Uncovered branches: 111:or(1):true, 128:range:empty, 159:default:fallback, 160:or(1):true, 16:if:true, 176:if:false

charts/clickstack/templates/additional-manifests.yaml

Uncovered lines: none

Uncovered branches: none

charts/clickstack/templates/clickhouse/cluster.yaml

Uncovered lines: none

Uncovered branches: none

charts/clickstack/templates/clickhouse/keeper.yaml

Uncovered lines: none

Uncovered branches: none

charts/clickstack/templates/hyperdx/configmap.yaml

Uncovered lines: none

Uncovered branches: 8:range:empty

charts/clickstack/templates/hyperdx/cronjob-check-alerts.yaml

Uncovered lines: none

Uncovered branches: none

charts/clickstack/templates/hyperdx/dashboard-configmap.yaml

Uncovered lines: none

Uncovered branches: none

charts/clickstack/templates/hyperdx/dashboard-provisioner-rbac.yaml

Uncovered lines: none

Uncovered branches: 40:range:empty

charts/clickstack/templates/hyperdx/deployment.yaml

Uncovered lines: none

Uncovered branches: 117:default:fallback, 127:default:fallback, 150:if:false, 169:and(2):false, 16:with:empty, 192:default:fallback, 197:with:empty, 22:default:fallback, 38:with:empty

charts/clickstack/templates/hyperdx/hpa.yaml

Uncovered lines: none

Uncovered branches: 16:with:empty

charts/clickstack/templates/hyperdx/ingress.yaml

Uncovered lines: none

Uncovered branches: 35:if:false, 40:range:empty, 45:range:empty, 48:with:empty, 54:range:empty, 58:range:empty, 59:or(2):true, 59:or(3):true

charts/clickstack/templates/hyperdx/networkpolicy.yaml

Uncovered lines: none

Uncovered branches: 11:with:empty

charts/clickstack/templates/hyperdx/pdb.yaml

Uncovered lines: none

Uncovered branches: 10:with:empty

charts/clickstack/templates/hyperdx/secret.yaml

Uncovered lines: none

Uncovered branches: 13:range:empty, 1:or(2):true, 1:or(3):true

charts/clickstack/templates/hyperdx/service.yaml

Uncovered lines: none

Uncovered branches: 14:default:fallback, 9:with:empty

charts/clickstack/templates/hyperdx/serviceaccount.yaml

Uncovered lines: none

Uncovered branches: none

charts/clickstack/templates/mongodb/community.yaml

Uncovered lines: none

Uncovered branches: none

charts/clickstack/templates/mongodb/password-secret.yaml

Uncovered lines: none

Uncovered branches: none

charts/clickstack/templates/otel-collector/custom-config.yaml

Uncovered lines: none

Uncovered branches: none

[github-actions]

Deep Review

✅ No critical issues found. This PR introduces no runtime template changes — values.yaml only gains # -- doc comments (verified: the sole non-comment lines are two stripped inline comments), templates are untouched, tests/ and README.md.gotmpl are excluded from the packaged chart via .helmignore, and no secret or broken default is introduced. The chart renders and installs identically. Several items from the prior review are now resolved: the action moved to ClickHouse/helmcov pinned by full commit SHA, tool versions and coverage thresholds have a single source of truth (scripts/tool-versions.env), the pre-commit hook warns instead of blocking when helm is absent, scripts/install-helmcov.sh verifies a pinned checksum, the deployment-tpl-defaults fixture was trimmed to 29 lines, and the pre-commit case arms are now reachable. Remaining findings are supply-chain and dev-workflow hardening.

🟡 P2 -- recommended

• scripts/install-helm-docs.sh:41 -- downloads and executes the helm-docs release tarball over curl with no checksum or signature verification, unlike the sibling scripts/install-helmcov.sh which pins and verifies a sha256; this path now runs in CI via make ci-setup, so a replaced upstream release asset executes in the runner.
  • Fix: verify the downloaded archive against a pinned SHA256 from the release checksums.txt before install, mirroring install-helmcov.sh.
  • _{security, adversarial, previous-comments}

🔵 P3 nitpicks (3)

• Makefile:6 -- make test hardcodes CHART=charts/clickstack, but the pre-commit hook flags any charts/clickstack-operators/* edit as chart_changes=true and runs make test, so operators-chart changes trigger a test run that never renders or asserts that chart.
  • Fix: loop test/validate over every charts/* chart, or document that clickstack-operators is intentionally untested.
• Makefile:82 -- make docs-check (and make ci) regenerates the README then runs git checkout -- charts/*/README.md, silently discarding any uncommitted edits to the generated README.md when invoked outside the stash-guarded pre-commit hook.
  • Fix: restore the README only when docs-check itself modified it, or skip the git checkout and leave regeneration visible to the caller.
• scripts/helmcov.sh:14 -- local make coverage passes --max-scenarios 20 --seed 42 while the CI ClickHouse/helmcov action step passes neither, so local and CI branch-coverage sampling can differ near the 90% branch gate (current margin is 91.01%).
  • Fix: pass the same --max-scenarios/--seed (or document the action's defaults) so local and CI coverage are reproducible.

---

Reviewers (7): correctness, testing, maintainability, project-standards, security, adversarial, previous-comments.

Testing gaps: the new Bash tooling (scripts/install-*.sh, .githooks/pre-commit, scripts/load-tool-versions.sh, Makefile logic) has no automated tests; the clickstack-operators chart has a README.md.gotmpl and doc-check but no render or unit test, so make coverage reports no signal for it.

_{No finding blocks merge automatically. A maintainer will expect P0/P1 findings in code this PR changes to be fixed; P2/P3 are your call -- fix or reply. Never fix findings about surrounding code here; reply instead. Do not widen the PR.}

[wrn14897]

This is cool, but how good is the test coverage for this library?

[wrn14897]

Following up on this. wdyt about forking or moving this to the ClickHouse repo? I think it's mainly for testing, but I'd imagine we'd still need to go through a security audit of all the dependencies. Having it in the ClickHouse repo would likely make that process easier.

[jordan-simonovski]

Yeah I could do that for both this and the versioning tool I've got if you think it makes sense

[jordan-simonovski]

Coverage on the library is about 92%. It's pretty well tested in terms of how it handles coverage assertions. And yeah I'm open to forking this. SHA pinning there should at least stop anything potentially malicious from happening, though if it's easier to use as an internal thing I'm ok with it.

[jordan-simonovski]

Moved to https://github.com/ClickHouse/helmcov

[wrn14897]

Thanks for adding the Makefile. Can we make sure it also includes other test commands?

[jordan-simonovski]

Added make integration, which runs the Kind suites the same way CI does:

• each suite gets its own test- cluster, deleted afterwards.
• make integration SUITES=api-only runs a single suite.

The full-stack suite already runs the smoke test and the Playwright checks, so this covers every test in the repo. I've left it out of make ci since it needs Docker and kind, and would take a while to run. I can do this in a follow up PR if you like

[wrn14897]

I might consider moving to helmver next instead of changesets and alleviating one more dependency in the repo in future. Open to suggestions.

We’re using changesets in the main OSS repo, and it’s been quite useful for developers to manage the changelog. We’re also using its github action to automatically create release PRs (for example: #229) and streamline the release process. Imo I don't think we need to deprecate it atm.