Add Helm unit tests and documentation#615
Merged
alexmv merged 22 commits intozulip:mainfrom Feb 27, 2026
Merged
Conversation
alexmv
force-pushed
the
claude/add-helm-chart-tests-py9kv
branch
3 times, most recently
from
3c07cf3 to
299daba
Compare
alexmv
force-pushed
the
claude/add-helm-chart-tests-py9kv
branch
4 times, most recently
from
cc91bd6 to
5aeea84
Compare
The post-install notes referenced a nonexistent service name `<release>-http`; the Service is actually named using the standard fullname helper, matching all other templates.
ingress-nginx is being retired in March 2026. Update the example annotation comment to reference traefik instead.
The default value "123456789" is not useful — no real SMTP server uses it, so users must always override it. Having a scalar default also causes Helm coalesce warnings when overriding with a valueFrom map (e.g., to reference a Kubernetes Secret), because Helm cannot merge a string into a map and warns about ignoring the non-table value. Removing it from values.yaml eliminates these warnings while keeping the variable documented in values-local.yaml.example and in the README's valueFrom usage example.
Now that the coalesce warning from the SECRETS_email_password type mismatch is resolved, enable --strict so any future warnings are caught immediately rather than silently passing CI.
The ConfigMap was named using {{ .Release.Name }} but the
StatefulSet volume referenced it using {{ include
"zulip.fullname" . }}. When the Helm release name does not
contain "zulip" (e.g. `helm install production …`), the two
names diverge and the pod fails to start because the referenced
ConfigMap does not exist.
Use the same fullname helper in both places so the names always
match regardless of the release name.
The readthedocs URLs are more helpful.
These add nothing.
The annotations block used bare indent while every other template uses nindent. Switch to the same with/nindent pattern used elsewhere for consistency.
The annotations key was rendered unconditionally, producing an
empty annotations: {} in the output when no annotations are set.
Every other template (serviceaccount, ingress, PVC) uses a
{{- with }} guard so the key is omitted when empty. Apply the
same pattern to the Service template for consistency.
The chart README duplicated installation instructions and the valueFrom example that now live in the Sphinx documentation at https://zulip.readthedocs.io/projects/docker/. Replace the duplicated prose with a brief quick-start snippet and prominent links to the authoritative docs. This also fixes a discrepancy where the README used "helm dependency update" while the docs correctly use "helm dependency build".
The chart README had a 40-line Minikube section that referenced cert-manager v1.8.0 (released 2022, now EOL). Replace it with a fresh write-up in the getting-started guide using current versions and best practices: cert-manager v1.17, the nginx Ingress class from the minikube addon, and inline kubectl apply for the ClusterIssuer.
Codify patterns learned from this session's audit: consistent resource naming with the fullname helper, nindent over indent, guarding optional blocks with with, avoiding scalar defaults for valueFrom-capable keys, and keeping detailed content in docs/ rather than the chart README. Also update the Helm documentation link to point at the ReadTheDocs site, which is now the authoritative source.
alexmv
force-pushed
the
claude/add-helm-chart-tests-py9kv
branch
from
5aeea84 to
ffff128
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes:
How did you test this PR?
Self-review checklist
(variable names, code reuse, readability, etc.).
Communicate decisions, questions, and potential concerns.
Individual commits are ready for review (see commit discipline).
Completed manual review and testing of the following: