chore: Support helm deploy on KinD in makefile#232
Conversation
🔐 TruffleHog Secret Scan✅ No secrets or credentials found! Your code has been scanned for 700+ types of secrets and credentials. All clear! 🎉 🕐 Last updated: 2026-03-11 12:04:46 UTC | Commit: 6bb4192 |
🛡️ Vulnerability Scan🚨 Found 64 vulnerability(ies) Severity Breakdown:
🔗 View full details in Security tab 🕐 Last updated: 2026-03-11 12:04:56 UTC | Commit: 6bb4192 |
thossain-nv
left a comment
There was a problem hiding this comment.
Looks good, thank you @lachen-nv for getting the Helm installation complete. Looks like we should increment the version.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdded Helm and Kustomize deployment orchestration and new Makefile targets for multi-path Kind deployments; updated README with Helm and Kustomize workflows; added Role "create" verbs for certificaterequests/secrets and a new Temporal TLS Secret template in the carbide-rest-site-agent chart. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can use OpenGrep to find security vulnerabilities and bugs across 17+ programming languages.OpenGrep is compatible with Semgrep configurations. Add an |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Makefile (1)
254-254: Add a conventionalalltarget for Makefile best practices.The Makefile lacks a standard
alltarget entry point. While all helm-related targets on line 254 are properly defined, adding analltarget (or similar convention) would align with Makefile standards and reduce checkmake linter warnings.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` at line 254, Add a conventional top-level "all" Makefile target and include it in the .PHONY list alongside the existing targets; implement "all" as a short alias/dependency that invokes the existing collective target (e.g., make all should depend on helm-deploy-all or another appropriate aggregate target), and update the .PHONY declaration (the line containing .PHONY: helm-lint helm-template ... helm-uninstall) to include "all" so the linter and users have a standard entry point.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@helm/charts/carbide-rest-site-agent/templates/temporal-certs-secret.yaml`:
- Line 23: The template uses a whitespace-trim left delimiter at the labels
include which breaks YAML linting; modify the include invocation in the
temporal-certs-secret template to use the standard delimiter (remove the leading
'-') so the line reads an ordinary include of "carbide-rest-site-agent.labels"
piped to nindent 4, ensuring the nindent 4 filter still controls indentation and
the rendered YAML remains valid.
In `@Makefile`:
- Around line 541-543: The Helm upgrade/install command for the site agent
currently swallows failures via "|| true"; remove that failure suppression so
the Make target fails on real Helm errors (locate the line containing "helm
upgrade --install carbide-rest-site-agent $(SITE_AGENT_CHART)/ --namespace
carbide-rest $(HELM_SET) --timeout 1m || true" and delete the "|| true"). If you
want retries, implement explicit retry logic or a conditional that logs and
exits non‑zero instead of silencing errors.
---
Nitpick comments:
In `@Makefile`:
- Line 254: Add a conventional top-level "all" Makefile target and include it in
the .PHONY list alongside the existing targets; implement "all" as a short
alias/dependency that invokes the existing collective target (e.g., make all
should depend on helm-deploy-all or another appropriate aggregate target), and
update the .PHONY declaration (the line containing .PHONY: helm-lint
helm-template ... helm-uninstall) to include "all" so the linter and users have
a standard entry point.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7fe1ecef-0228-48b1-9c9d-bc2708bd27fc
📒 Files selected for processing (4)
MakefileREADME.mdhelm/charts/carbide-rest-site-agent/templates/rbac.yamlhelm/charts/carbide-rest-site-agent/templates/temporal-certs-secret.yaml
Description
Add helm deploy command into the Makefile
Type of Change
Services Affected
Related Issues (Optional)
Breaking Changes
Testing
Additional Notes
Test Report: Helm Deployment via make kind-reset
Environment
Test: make kind-reset (Helm path)
Ran make kind-reset end-to-end on a fresh remote machine. All steps completed successfully:
Final pod status (carbide-rest namespace)
carbide-rest-api 1/1 Running 0
carbide-rest-cert-manager 1/1 Running 0
carbide-rest-cloud-worker 1/1 Running 0
carbide-rest-db-migration 0/1 Completed 0
carbide-rest-mock-core 1/1 Running 0
carbide-rest-site-agent-0 1/1 Running 0
carbide-rest-site-manager 1/1 Running 0
carbide-rest-site-worker 1/1 Running 0
keycloak 1/1 Running 0
Bug found & fixed during testing
Site-agent was CrashLooping after bootstrap.
Root cause: the temporal-client-site-agent-certs secret was not pre-created by the Helm chart, so the bootstrap code's secretIf.Get() returned NotFound and failed. The Kustomize path already
had a placeholder secret (temporal-client-site-agent-certs.yaml) but it was missing from the Helm chart.
Fix: added helm/charts/carbide-rest-site-agent/templates/temporal-certs-secret.yaml as an empty placeholder secret, matching the Kustomize behavior. Also added create verb to the site-agent RBAC Role for completeness.
Summary by CodeRabbit
New Features
Documentation
Chores