From fa8903826416bf2901f97bcd1ce8df13b5b65cff Mon Sep 17 00:00:00 2001 From: Joshua Hoblitt Date: Mon, 27 Jul 2026 16:05:21 -0700 Subject: [PATCH] helm: add helm-unittest harness for the charts The charts have no unit tests. CI runs ct lint plus a helm template and kustomize build, which prove the charts render but never assert what they render, so conditional template logic goes unverified. Add helm-unittest, pinned and installed as a standalone binary, a test.helm make target, and a Helm Unittests workflow that runs it. The first suite covers the toolbox deployment: the image composed from the toolbox and cephImage settings, the precedence of both over the older toolbox.image setting, the hostNetwork branch driven by the cluster network provider, and the revisionHistoryLimit guard. The new job is a required check in .mergify.yml, alongside the existing chart linting. Suites live under tests/ in a chart and are excluded from the packaged chart via .helmignore. The runner is invoked with --strict so that a misspelled key fails the suite instead of silently rendering defaults. Signed-off-by: Joshua Hoblitt --- .github/workflows/helm-lint.yaml | 2 +- .github/workflows/helm-unittests.yaml | 31 +++++ .mergify.yml | 4 + .../Contributing/development-flow.md | 9 ++ Makefile | 4 + build/makelib/helm.mk | 18 +++ deploy/charts/rook-ceph-cluster/.helmignore | 2 + .../rook-ceph-cluster/tests/toolbox_test.yaml | 131 ++++++++++++++++++ deploy/charts/rook-ceph/.helmignore | 2 + 9 files changed, 202 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/helm-unittests.yaml create mode 100644 deploy/charts/rook-ceph-cluster/tests/toolbox_test.yaml diff --git a/.github/workflows/helm-lint.yaml b/.github/workflows/helm-lint.yaml index 87a9cbf78..d37523f7b 100644 --- a/.github/workflows/helm-lint.yaml +++ b/.github/workflows/helm-lint.yaml @@ -32,4 +32,4 @@ jobs: - name: Lint the helm charts - run: make lint.helm \ No newline at end of file + run: make lint.helm diff --git a/.github/workflows/helm-unittests.yaml b/.github/workflows/helm-unittests.yaml new file mode 100644 index 000000000..5716ca51f --- /dev/null +++ b/.github/workflows/helm-unittests.yaml @@ -0,0 +1,31 @@ +name: Helm Unittests + +on: + push: + tags: + - v* + branches: + - master + - release-* + pull_request: + branches: + - master + - release-* + +# cancel the in-progress workflow when PR is refreshed. +concurrency: + group: ${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.head_ref || github.sha }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + helm-unittests: + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - name: Unit test the helm charts + run: make test.helm diff --git a/.mergify.yml b/.mergify.yml index 60a3f1998..6784c87ca 100644 --- a/.mergify.yml +++ b/.mergify.yml @@ -29,6 +29,7 @@ pull_request_rules: - "check-success=gen-rbac" - "check-success=golangci-lint" - "check-success=govulncheck" + - "check-success=helm-unittests" - "check-success=lint" - "check-success=lint-test" - "check-success=linux-build-all (1.23)" @@ -89,6 +90,7 @@ pull_request_rules: - "check-success=gen-rbac" - "check-success=golangci-lint" - "check-success=govulncheck" + - "check-success=helm-unittests" - "check-success=lint" - "check-success=lint-test" - "check-success=linux-build-all (1.24)" @@ -148,6 +150,7 @@ pull_request_rules: - "check-success=gen-rbac" - "check-success=golangci-lint" - "check-success=govulncheck" + - "check-success=helm-unittests" - "check-success=lint" - "check-success=lint-test" - "check-success=linux-build-all (1.25)" @@ -207,6 +210,7 @@ pull_request_rules: - "check-success=gen-rbac" - "check-success=golangci-lint" - "check-success=govulncheck" + - "check-success=helm-unittests" - "check-success=lint" - "check-success=lint-test" - "check-success=linux-build-all (1.25)" diff --git a/Documentation/Contributing/development-flow.md b/Documentation/Contributing/development-flow.md index 679fd24dc..9b87f3d0b 100644 --- a/Documentation/Contributing/development-flow.md +++ b/Documentation/Contributing/development-flow.md @@ -235,6 +235,15 @@ go test -coverprofile=coverage.out go tool cover -html=coverage.out -o coverage.html ``` +The Helm charts have their own unit tests, which render the chart templates and assert on the result: + +```console +make test.helm +``` + +Suites live in `tests/*_test.yaml` under each chart and are written in the +[helm-unittest](https://github.com/helm-unittest/helm-unittest) YAML format. + ### Writing unit tests Good unit tests start with easily testable code. Small chunks ("units") of code can be easily tested diff --git a/Makefile b/Makefile index 9da6ad0b9..647d609ef 100644 --- a/Makefile +++ b/Makefile @@ -222,6 +222,10 @@ lint.helm: $(HELM) $(KUSTOMIZE) ## Check the helm charts $(KUSTOMIZE) build >/dev/null rm templated.yaml kustomization.yaml +.PHONY: test.helm +test.helm: $(HELM_UNITTEST) ## Run the helm chart unit tests + $(HELM_UNITTEST) --strict $(addprefix $(HELM_CHARTS_DIR)/,$(HELM_CHARTS)) + .PHONY: lint.quick lint.quick: lint.yaml lint.shell lint.make lint.go lint.helm lint.markdown ## run some (faster) linters .PHONY: lint diff --git a/build/makelib/helm.mk b/build/makelib/helm.mk index 101d6b43b..baabc1653 100644 --- a/build/makelib/helm.mk +++ b/build/makelib/helm.mk @@ -131,3 +131,21 @@ $(HELM_DOCS): | $(TOOLS_HOST_DIR) ## Installs helm-docs @GOBIN=$(TOOLS_HOST_DIR)/tmp GO111MODULE=on go install $(HELM_DOCS_REPO)@$(HELM_DOCS_VERSION) @mv $(TOOLS_HOST_DIR)/tmp/helm-docs $(HELM_DOCS) @rm -fr $(TOOLS_HOST_DIR)/tmp + +# ==================================================================================== +# Makefile helper functions for helm-unittest: https://github.com/helm-unittest/helm-unittest +# + +HELM_UNITTEST_VERSION := v1.1.2 +HELM_UNITTEST := $(TOOLS_HOST_DIR)/helm-unittest-$(HELM_UNITTEST_VERSION) +# the release artifacts label macOS "macos" where go calls it "darwin" +HELM_UNITTEST_PLATFORM := $(subst darwin,macos,$(shell go env GOHOSTOS))-$(GOHOSTARCH) + +# helm-unittest ships as a helm plugin, but the binary it wraps runs standalone, +# so it is installed and invoked directly rather than through `helm plugin install`. +$(HELM_UNITTEST): | $(TOOLS_HOST_DIR) ## Installs helm-unittest + @echo === installing helm-unittest + @mkdir -p $(TOOLS_HOST_DIR)/tmp + @curl -fsSL https://github.com/helm-unittest/helm-unittest/releases/download/$(HELM_UNITTEST_VERSION)/helm-unittest-$(HELM_UNITTEST_PLATFORM)-$(patsubst v%,%,$(HELM_UNITTEST_VERSION)).tgz | tar -xz -C $(TOOLS_HOST_DIR)/tmp untt-$(HELM_UNITTEST_PLATFORM) + @mv $(TOOLS_HOST_DIR)/tmp/untt-$(HELM_UNITTEST_PLATFORM) $(HELM_UNITTEST) + @rm -fr $(TOOLS_HOST_DIR)/tmp diff --git a/deploy/charts/rook-ceph-cluster/.helmignore b/deploy/charts/rook-ceph-cluster/.helmignore index 5ce4e2e57..073dc9305 100644 --- a/deploy/charts/rook-ceph-cluster/.helmignore +++ b/deploy/charts/rook-ceph-cluster/.helmignore @@ -20,3 +20,5 @@ .idea/ *.tmproj *.tmpl +# helm-unittest suites +tests/ diff --git a/deploy/charts/rook-ceph-cluster/tests/toolbox_test.yaml b/deploy/charts/rook-ceph-cluster/tests/toolbox_test.yaml new file mode 100644 index 000000000..d28ead932 --- /dev/null +++ b/deploy/charts/rook-ceph-cluster/tests/toolbox_test.yaml @@ -0,0 +1,131 @@ +suite: toolbox deployment +templates: + - deployment.yaml +tests: + - it: renders nothing when the toolbox is disabled + set: + toolbox.enabled: false + asserts: + - hasDocuments: + count: 0 + + - it: composes the image from the cephImage settings + set: + toolbox.enabled: true + cephImage.repository: quay.io/ceph/ceph + cephImage.tag: v20.2.2 + asserts: + - equal: + path: spec.template.spec.containers[0].image + value: quay.io/ceph/ceph:v20.2.2 + + - it: tracks a custom cephImage repository and tag + set: + toolbox.enabled: true + cephImage.repository: registry.example.com/ceph/ceph + cephImage.tag: v19.2.3 + asserts: + - equal: + path: spec.template.spec.containers[0].image + value: registry.example.com/ceph/ceph:v19.2.3 + + - it: prefers toolbox.image over the cephImage settings + set: + toolbox.enabled: true + cephImage.repository: quay.io/ceph/ceph + cephImage.tag: v20.2.2 + toolbox.image: registry.example.com/rook/ceph:v1.18.0 + asserts: + - equal: + path: spec.template.spec.containers[0].image + value: registry.example.com/rook/ceph:v1.18.0 + + - it: takes the tag from cephImage when only toolbox.repository is set + set: + toolbox.enabled: true + cephImage.repository: quay.io/ceph/ceph + cephImage.tag: v20.2.2 + toolbox.repository: registry.example.com/rook/ceph + asserts: + - equal: + path: spec.template.spec.containers[0].image + value: registry.example.com/rook/ceph:v20.2.2 + + - it: takes the repository from cephImage when only toolbox.tag is set + set: + toolbox.enabled: true + cephImage.repository: quay.io/ceph/ceph + cephImage.tag: v20.2.2 + toolbox.tag: v19.2.3 + asserts: + - equal: + path: spec.template.spec.containers[0].image + value: quay.io/ceph/ceph:v19.2.3 + + - it: composes toolbox.repository and toolbox.tag when both are set + set: + toolbox.enabled: true + cephImage.repository: quay.io/ceph/ceph + cephImage.tag: v20.2.2 + toolbox.repository: registry.example.com/rook/ceph + toolbox.tag: v1.18.0 + asserts: + - equal: + path: spec.template.spec.containers[0].image + value: registry.example.com/rook/ceph:v1.18.0 + + - it: prefers toolbox.repository over toolbox.image + set: + toolbox.enabled: true + cephImage.repository: quay.io/ceph/ceph + cephImage.tag: v20.2.2 + toolbox.image: registry.example.com/legacy/ceph:v18.2.0 + toolbox.repository: registry.example.com/rook/ceph + asserts: + - equal: + path: spec.template.spec.containers[0].image + value: registry.example.com/rook/ceph:v20.2.2 + + - it: prefers toolbox.tag over toolbox.image + set: + toolbox.enabled: true + cephImage.repository: quay.io/ceph/ceph + cephImage.tag: v20.2.2 + toolbox.image: registry.example.com/legacy/ceph:v18.2.0 + toolbox.tag: v19.2.3 + asserts: + - equal: + path: spec.template.spec.containers[0].image + value: quay.io/ceph/ceph:v19.2.3 + + - it: leaves hostNetwork unset unless the cluster uses the host network + set: + toolbox.enabled: true + asserts: + - notExists: + path: spec.template.spec.hostNetwork + + - it: enables hostNetwork for a host network provider + set: + toolbox.enabled: true + cephClusterSpec.network.provider: host + asserts: + - equal: + path: spec.template.spec.hostNetwork + value: true + + - it: omits revisionHistoryLimit unless it is set + set: + toolbox.enabled: true + asserts: + - notExists: + path: spec.revisionHistoryLimit + + - it: applies revisionHistoryLimit when set + set: + toolbox.enabled: true + revisionHistoryLimit: 3 + asserts: + - equal: + path: spec.revisionHistoryLimit + value: 3 diff --git a/deploy/charts/rook-ceph/.helmignore b/deploy/charts/rook-ceph/.helmignore index 5ce4e2e57..073dc9305 100644 --- a/deploy/charts/rook-ceph/.helmignore +++ b/deploy/charts/rook-ceph/.helmignore @@ -20,3 +20,5 @@ .idea/ *.tmproj *.tmpl +# helm-unittest suites +tests/