Skip to content

fix(automatisch): align template standards#648

Open
mberlofa wants to merge 2 commits into
mainfrom
fix/automatisch-template-standards
Open

fix(automatisch): align template standards#648
mberlofa wants to merge 2 commits into
mainfrom
fix/automatisch-template-standards

Conversation

@mberlofa

@mberlofa mberlofa commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add the canonical automatisch.validate helper and validate.yaml gate
  • fail fast for incomplete external PostgreSQL and Redis configuration, invalid ingress hosts, and selector label overrides
  • add helm-unittest coverage for the new validation paths

Validation

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

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

Summary by CodeRabbit

  • New Features
    • Added Helm chart validations to fail fast on misconfiguration.
    • Requires external database host and credentials when PostgreSQL is disabled.
    • Requires external Redis host when Redis is disabled.
    • Validates ingress hosts are present and each host entry has a non-empty host.
    • Prevents custom pod labels from overriding key selector labels.
  • Tests
    • Added validation coverage for the above failure scenarios and exact error messages.

@coderabbitai

coderabbitai Bot commented Jul 3, 2026

Copy link
Copy Markdown

Review Change Stack

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: 6e796e09-7d92-4c3d-9315-ccfee8a3414c

📥 Commits

Reviewing files that changed from the base of the PR and between a2f5b22 and d5e12c5.

📒 Files selected for processing (2)
  • charts/automatisch/templates/_helpers.tpl
  • charts/automatisch/tests/validation_test.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
  • charts/automatisch/tests/validation_test.yaml
  • charts/automatisch/templates/_helpers.tpl

📝 Walkthrough

Walkthrough

This PR adds Helm pre-render validation for external database, Redis, ingress, and podLabels configuration. It wires the validation into validate.yaml and adds tests for the expected pass and fail cases.

Changes

Configuration validation

Layer / File(s) Summary
Validation helper and rendering
charts/automatisch/templates/_helpers.tpl, charts/automatisch/templates/validate.yaml
Adds automatisch.validate with fail checks for external DB host and credentials, external Redis host, ingress host list and host fields, and podLabels selector-key overrides; renders it through validate.yaml.
Validation test suite
charts/automatisch/tests/validation_test.yaml
Adds validation tests for the default no-op case and the PostgreSQL, Redis, ingress, and podLabels failure scenarios with exact error messages.

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

Sequence Diagram(s)

sequenceDiagram
  participant HelmRender
  participant validate_yaml as validate.yaml
  participant Helper as automatisch.validate

  HelmRender->>validate_yaml: render templates
  validate_yaml->>Helper: include automatisch.validate
  Helper-->>Helper: check postgresql.enabled, redis.enabled, ingress.enabled, podLabels
  alt validation fails
    Helper-->>validate_yaml: fail with error message
    validate_yaml-->>HelmRender: render error
  else validation passes
    Helper-->>validate_yaml: no error
    validate_yaml-->>HelmRender: rendered output
  end
Loading
🚥 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 is concise and broadly matches the chart template validation and standards-alignment changes.
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/automatisch-template-standards

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

@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Standards Check (GR-079) — PASS

Every changed chart fully passes standards-check.

@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

🟢 Security Scan: automatisch

Framework Score
MITRE + NSA + SOC2 82.323235%

✅ Security posture acceptable.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
charts/automatisch/templates/_helpers.tpl (1)

62-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Selector label keys duplicated as string literals.

"app.kubernetes.io/name" and "app.kubernetes.io/instance" are hardcoded here but also defined via automatisch.selectorLabels (Lines 29-32). If a selector label key is added/changed there later, this guard would silently miss it.

Consider deriving the disallowed keys from a shared list/helper to keep both in sync.

🤖 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/automatisch/templates/_helpers.tpl` around lines 62 - 66, The
selector-label guard in automatisch.selectorLabels is hardcoding disallowed
podLabels keys, which can drift from the shared selector labels definition.
Update the check in _helpers.tpl to derive the protected keys from the same
source used by automatisch.selectorLabels, or refactor both to share a common
helper/list, so any future selector label key changes stay in sync
automatically.
🤖 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/automatisch/templates/_helpers.tpl`:
- Around line 62-66: The selector-label guard in automatisch.selectorLabels is
hardcoding disallowed podLabels keys, which can drift from the shared selector
labels definition. Update the check in _helpers.tpl to derive the protected keys
from the same source used by automatisch.selectorLabels, or refactor both to
share a common helper/list, so any future selector label key changes stay in
sync automatically.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: be273cc6-ef51-4015-99bd-6e9c73a4ba58

📥 Commits

Reviewing files that changed from the base of the PR and between 0d8168f and ee5de23.

📒 Files selected for processing (3)
  • charts/automatisch/templates/_helpers.tpl
  • charts/automatisch/templates/validate.yaml
  • charts/automatisch/tests/validation_test.yaml

@mberlofa mberlofa force-pushed the fix/automatisch-template-standards branch from ee5de23 to a2f5b22 Compare July 4, 2026 17:29
@mberlofa

mberlofa commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the CodeRabbit review-body nitpick.

Changes:

  • podLabels validation now derives protected keys from automatisch.selectorLabels instead of hardcoding selector label names.
  • Added validation coverage for overriding app.kubernetes.io/instance in addition to the existing app.kubernetes.io/name case.

Validation:

  • make validate-chart CHART=automatisch passed
  • Result: automatisch: FULLY VALIDATED (13 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:templates area:tests chart:automatisch Issues related to the automatisch chart size:M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant