fix(docmost): centralize template validation

[mberlofa]

Summary

• add the canonical docmost.validate helper expected by template standards
• keep the existing validation behavior and messages by delegating templates/validate.yaml to the helper

Validation

• make template-standards-check CHART=docmost
• helm unittest charts/docmost
• helm lint --strict charts/docmost
• make standards-check CHART=docmost
• make validate-chart CHART=docmost TIMEOUT=900: FULLY VALIDATED (18 layers)
• make site-sync-check CHART=docmost
• make release-check REPO=charts
• make attribution-check REPO=charts

Site PR: helmforgedev/site#338
Issue: #633

Summary by CodeRabbit

• Bug Fixes
  • Improved Helm chart configuration validation to catch invalid replica and storage settings before deployment.
  • Prevents multi-replica deployments with local storage, ensures required local storage options are provided, and enforces required S3 bucket details and credentials when using S3.
  • Centralized validation logic so misconfigurations fail consistently and clearly during chart rendering.

[github-actions]

Standards Check (GR-079) — PASS

Every changed chart fully passes standards-check.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 421bd0f8-dbbd-46ad-b9f8-e2978b81d30f

📥 Commits

Reviewing files that changed from the base of the PR and between ddb70f7 and fe069f9.

📒 Files selected for processing (1)

• charts/docmost/templates/_helpers.tpl

🚧 Files skipped from review as they are similar to previous changes (1)

• charts/docmost/templates/_helpers.tpl

---

📝 Walkthrough

Walkthrough

Helm chart validation logic is moved from inline checks in validate.yaml into a shared docmost.validate helper in _helpers.tpl. The helper enforces replica, local storage, and S3 configuration rules, and validate.yaml now calls it.

Changes

Validation Refactor

Layer / File(s)	Summary
Define validation helper and wire it into validate.yaml charts/docmost/templates/_helpers.tpl, charts/docmost/templates/validate.yaml	Adds docmost.validate with fail checks for replicaCount/storage.mode, local storage requirements, and S3 bucket/credentials; validate.yaml now delegates to it via include.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested labels: size:M

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: moving Docmost template validation into a central helper.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/docmost-template-standards

---

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

🟢 Security Scan: docmost

Framework	Score
MITRE + NSA + SOC2	87.878784%

✅ Security posture acceptable.

[coderabbitai]

🧹 Nitpick comments (1)

charts/docmost/templates/_helpers.tpl (1)

259-270: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Consider guarding against an invalid storage.mode value.

None of the four branches fail when storage.mode is neither "local" nor "s3" (e.g. a typo). In that case, deployment.yaml's conditionals for local/s3 volumes/env vars would silently no-op rather than surfacing a clear validation error. If this isn't already enforced by a values schema, consider adding a catch-all fail for unrecognized modes.

♻️ Optional catch-all guard

 {{- if and (eq .Values.storage.mode "s3") (not .Values.storage.s3.existingSecret) (or (not .Values.storage.s3.accessKey) (not .Values.storage.s3.secretKey)) -}}
 {{- fail "docmost: storage.mode=s3 requires storage.s3.existingSecret or both storage.s3.accessKey and storage.s3.secretKey" -}}
 {{- end -}}
+{{- if and (ne .Values.storage.mode "local") (ne .Values.storage.mode "s3") -}}
+{{- fail (printf "docmost: storage.mode must be 'local' or 's3', got %q" .Values.storage.mode) -}}
+{{- end -}}
 {{- end -}}

Please check whether values.schema.json (or similar) already enforces storage.mode as an enum before applying this.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@charts/docmost/templates/_helpers.tpl` around lines 259 - 270, The Helm
helper validations in _helpers.tpl do not reject unknown storage.mode values, so
a typo can slip through and leave deployment.yaml with silent no-op
conditionals. Add a catch-all validation in the same storage checks that fails
for any storage.mode other than the supported values, and first verify whether
values.schema.json already constrains storage.mode to an enum so you don’t
duplicate existing validation.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@charts/docmost/templates/_helpers.tpl`:
- Around line 259-270: The Helm helper validations in _helpers.tpl do not reject
unknown storage.mode values, so a typo can slip through and leave
deployment.yaml with silent no-op conditionals. Add a catch-all validation in
the same storage checks that fails for any storage.mode other than the supported
values, and first verify whether values.schema.json already constrains
storage.mode to an enum so you don’t duplicate existing validation.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: fe2c9770-17b8-4c76-ab75-9c9457be8ad5

📥 Commits

Reviewing files that changed from the base of the PR and between 870b4c7 and 1bf71b5.

📒 Files selected for processing (2)

• charts/docmost/templates/_helpers.tpl
• charts/docmost/templates/validate.yaml

[mberlofa]

Reviewed the CodeRabbit review-summary nitpick for storage.mode validation.

Result:

• values.schema.json already enforces storage.mode with enum values local and s3.
• Added a defensive catch-all in docmost.validate as well, so deployments that intentionally skip schema validation still fail clearly instead of silently rendering without local/S3 storage branches.
• I did not add a unit test with storage.mode: typo because the standard Helm unittest path fails earlier on the values schema enum, before the template helper can run. I verified this locally and kept the schema as the primary testable guard.

Validation:

• make validate-chart CHART=docmost passed
• Result: docmost: FULLY VALIDATED (18 layers), including all k3d GR-027 scenarios
• make release-check REPO=charts passed with only the expected post-merge release confirmation warning
• make attribution-check REPO=charts passed
• git diff --check passed

Note: this CodeRabbit item was posted in the review summary/body, not as an active review thread, so there is no thread ID to reply to or resolve for it.