-
Notifications
You must be signed in to change notification settings - Fork 0
Feat/helm pre commit hooks #22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds three new pre-commit hooks for Helm and YAML validation, corresponding shell scripts, updates Changes
Sequence Diagram(s)sequenceDiagram
participant PreCommit as Pre-commit
participant Hook as Hook Script
participant Tool as Validator (helm / yamllint)
participant Repo as Repository
PreCommit->>Hook: Invoke with file list
Hook->>Tool: Check installation
alt Missing
Tool-->>Hook: Not installed
Hook-->>PreCommit: Exit 1 (dependency missing)
else Installed
Hook->>Repo: Collect targets (from args or full scan)
alt None found
Hook-->>PreCommit: Exit 0 (no targets)
else Targets found
loop For each target
Hook->>Tool: Run validation
alt Success
Tool-->>Hook: OK
else Failure
Tool-->>Hook: Error (capture output)
end
end
Hook-->>PreCommit: Exit with cumulative code
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (5)
README.md (1)
5-5: Consider using markdown heading instead of emphasis (MD036).Line 5 (
**Quality gates for your Infrastructure as Code**) uses bold text where a markdown heading would be more semantically correct, though using a heading inside the centered div may affect layout. This is a minor linting issue; feel free to keep the current approach if the visual design is important.hooks/yamllint.sh (1)
8-15: Minor: Error message language mixes Spanish and English.The error message (lines 10-13) is in Spanish while the rest of the codebase (README, hook ID, etc.) is in English. This could confuse non-Spanish-speaking users. Consider translating to English for consistency.
Current:
echo -e "${RED}✗ Error: yamllint no está instalado${RST}" echo " Instalá yamllint:"Suggested:
echo -e "${RED}✗ Error: yamllint is not installed${RST}" echo " Install yamllint:"hooks/helm-template-check.sh (1)
8-13: Minor: Spanish error message should be English for consistency.Same as yamllint.sh, line 11 error message is in Spanish. Translate to English:
- echo -e "${RED}✗ Error: helm no está instalado${RST}" - echo " Instalá helm desde: https://helm.sh/docs/intro/install/" + echo -e "${RED}✗ Error: helm is not installed${RST}" + echo " Install helm from: https://helm.sh/docs/intro/install/"hooks/helm-lint.sh (2)
8-13: Minor: Spanish error message should be English for consistency.Translate error message to English to match the rest of the codebase:
- echo -e "${RED}✗ Error: helm no está instalado${RST}" - echo " Instalá helm desde: https://helm.sh/docs/intro/install/" + echo -e "${RED}✗ Error: helm is not installed${RST}" + echo " Install helm from: https://helm.sh/docs/intro/install/"
15-34: Code duplication: Chart discovery logic is identical to helm-template-check.sh.Lines 15-34 in this file are verbatim copies of the same lines in
hooks/helm-template-check.sh. Additionally, both scripts share identical color setup (lines 5-6) and similar directory traversal patterns.Consider extracting the shared logic into a common library/helper script to reduce duplication and improve maintainability:
# hooks/lib/chart-discovery.sh discover_charts() { if [[ "$#" -gt 0 ]]; then mapfile -t CHART_DIRS < <( for f in "$@"; do # ... existing logic ... done | sort -u ) else mapfile -t CHART_DIRS < <(find . -name "Chart.yaml" -exec dirname {} \; | sort -u) fi echo "${CHART_DIRS[@]}" }Then source this from both helm-lint.sh and helm-template-check.sh. This would also eliminate the need to duplicate the color setup.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.pre-commit-hooks.yaml(2 hunks)README.md(1 hunks)hooks/helm-lint.sh(1 hunks)hooks/helm-template-check.sh(1 hunks)hooks/yamllint.sh(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~104-~104: Consider using a different verb for a more formal wording.
Context: ...commit will be blocked, allowing you to fix issues before they're committed. --- ...
(FIX_RESOLVE)
🪛 markdownlint-cli2 (0.18.1)
README.md
5-5: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
72-72: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
94-94: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (5)
.pre-commit-hooks.yaml (1)
30-55: ✓ Hook definitions are well-structured and consistent.All three new hooks follow the established pattern with proper YAML syntax, sensible file patterns, and appropriate
require_serialsettings. The exclusions align with existing hooks, and the entry points correctly reference the new bash scripts.hooks/yamllint.sh (1)
25-43: ✓ Proper error handling and output formatting.The main loop correctly handles file iteration, captures exit codes with
set +e/set -e, and provides clear, colorized feedback. Output indentation (line 38) works well.hooks/helm-template-check.sh (2)
15-34: ✓ Smart chart discovery logic handles both targeted and repo-wide validation.The directory walking (lines 18-30) correctly finds Chart.yaml by traversing upward from modified files, reducing unnecessary validations. Fallback to repo-wide scan (lines 32-33) ensures full coverage when no files are provided. Proper null-byte handling (line 72) for filenames with spaces.
65-102: ✓ Comprehensive template validation with multiple test scenarios.Testing with both default values.yaml and example files (lines 65-73, 78-102) catches issues across different configurations. Proper handling of
set +e/set -earound helm template (lines 88-93) and clear output formatting with line indentation.hooks/helm-lint.sh (1)
44-68: ✓ Clean implementation with good UX for warnings.The helm lint execution is straightforward and correct. Showing warnings even on success (lines 57-60) provides helpful feedback without blocking commits. Proper handling of output formatting and exit codes.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (3)
README.md (3)
28-47: Add missing YAML Hooks section with yamllint documentation.The Available Hooks table documents Terraform and Helm hooks but completely omits the
yamllinthook defined in.pre-commit-hooks.yaml. Users cannot discover or understand this hook from the README.Add a YAML Hooks section after line 46:
> **Note:** Helm hooks automatically discover charts by locating `Chart.yaml` files in your repository. +### YAML Hooks + +| Hook | Description | What it does | +|------|-------------|--------------| +| `yamllint` | **YAML validator** | Validates YAML syntax and style; checks structure, formatting, and consistency | + +> **Note:** YAML hooks automatically validate all `.yaml` and `.yml` files in your repository. + ---
76-90: Example configuration missing yamllint hook.The
.pre-commit-config.yamlexample includes Terraform and Helm hooks but omits theyamllinthook. Users following this example won't enable YAML linting.# Helm hooks - id: helm-lint - id: helm-template-check + + # YAML hooks + - id: yamllint
72-105: Fix markdown heading violations (MD036) on lines 94 and 102.Lines 94 and 102 use bold text (
**...**) where proper markdown headings should be used.-**Step 2: Install the hooks** +### Step 2: Install the hooks Run this command in your repository: @@ -100,7 +100,7 @@ pre-commit install pre-commit install-Step 3: You're all set!
+### Step 3: You're all set!Now, every time you run
git commit, the configured hooks will automatically execute. If any hook fails, the commit will be blocked, allowing you to fix issues before they're committed.</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: CodeRabbit UI **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 6f080620f6c25a7c9c5b1ac20fecce11067da7aa and 2ea6b34911ddca784ca09838bc05371e2939ae26. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `README.md` (1 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 LanguageTool</summary> <details> <summary>README.md</summary> [style] ~104-~104: Consider using a different verb for a more formal wording. Context: ...commit will be blocked, allowing you to fix issues before they're committed. --- ... (FIX_RESOLVE) </details> </details> <details> <summary>🪛 markdownlint-cli2 (0.18.1)</summary> <details> <summary>README.md</summary> 5-5: Emphasis used instead of a heading (MD036, no-emphasis-as-heading) --- 94-94: Emphasis used instead of a heading (MD036, no-emphasis-as-heading) </details> </details> </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| * **terraform-fmt**: Checks that all Terraform files (`*.tf`) are properly formatted (`terraform fmt --check -diff`). | ||
| * **terraform-validate**: Runs `terraform init -backend=false` and then `terraform validate`. | ||
| > Notes: directories requiring a private registry and lacking credentials are marked as **skipped** (do not fail the commit). Both hooks ignore `.terraform/` and `examples/`. | ||
| **Quality gates for your Infrastructure as Code** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace bold text with proper markdown heading per MD036.
Line 5 uses emphasis (**...**) for what should be a subtitle/heading. Replace with a proper heading format.
-**Quality gates for your Infrastructure as Code**
+### Quality gates for your Infrastructure as Code📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| **Quality gates for your Infrastructure as Code** | |
| ### Quality gates for your Infrastructure as Code |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
5-5: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🤖 Prompt for AI Agents
In README.md around line 5, the subtitle currently uses bold markup (**Quality
gates for your Infrastructure as Code**) which violates MD036; replace the
bold-emphasis with a proper Markdown heading (e.g., use a suitable heading level
like "## Quality gates for your Infrastructure as Code" or "#" / "###" as
appropriate for document structure) so the line is a real heading rather than
bolded text.
matiri132
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
🚀 Release Notes
📌 Summary
Se agregaron Hooks para repositorios de Helm, como la Kube-app. Tambien se modifico el README.md para tener mas detalle de los nuevos hooks asi como mejor formato y estilo.
🔧 Changes
📖 Description
Hooks creados:
helm-lint: Valida charts de Helm usando la herramienta nativa helm lint, detecta errores de sintaxis, valores faltantes y problemas de configuración. Busca automáticamente archivos Chart.yaml y reporta errores con output coloreado en terminal.
helm-template-check: Renderiza templates usando helm template para verificar que generen manifiestos de Kubernetes válidos. Valida la sintaxis de los templates antes de deployment, mostrando errores específicos por archivo.
yamllint: Valida sintaxis de archivos YAML usando yamllint, analiza estructura y formato, reportando errores en formato parseable con línea y columna exacta.
Summary by CodeRabbit
New Features
Documentation