fix(answer): align template standards#644
Conversation
github-actions
Bot
added
area:templates
area:tests
area:values
chart:answer
Standards Check (GR-079) — PASSEvery changed chart fully passes standards-check. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThis PR adds Helm chart validation wiring and tests, updates CI values to use an in-cluster PostgreSQL pod, and restructures ChangesChart Value Validation
CI External Database Wiring
NOTES.txt Restructuring
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
🟢 Security Scan:
|
| Framework | Score |
|---|---|
| MITRE + NSA + SOC2 | 88.60029% |
✅ Security posture acceptable.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
charts/answer/templates/_helpers.tpl (1)
334-337: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor: unused local variable and redundant guard.
$databaseModeis assigned but never referenced afterward — the include is only used for its side effect (fail), similar to$_used at Line 336. Consider using$_here too for consistency and to signal intent.Also, the
if .Values.backup.enabledguard here duplicates the check already performed insideanswer.backupEnabled(Line 268), so it's not incorrect but is redundant.Nit fix
-{{- $databaseMode := include "answer.databaseMode" . -}} +{{- $_ := include "answer.databaseMode" . -}}🤖 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/answer/templates/_helpers.tpl` around lines 334 - 337, The helper block has an unused local assignment and a redundant outer guard: in the `_helpers.tpl` template, `answer.databaseMode` is included only for its side effect, so bind it to `$_` instead of `$databaseMode` to make that intent clear, and consider removing the extra `.Values.backup.enabled` `if` since `answer.backupEnabled` already performs the same check.charts/answer/ci/external-db-values.yaml (1)
48-67: 🩺 Stability & Availability | 🔵 TrivialOptional: no readiness gating between Answer and this PostgreSQL pod.
The disposable Postgres pod has no readiness probe, and nothing ordered ensures it accepts connections before the Answer pod attempts
AUTO_INSTALL/DB connect. If Answer doesn't retry the initial connection, this CI scenario can be flaky. Consider areadinessProbe(e.g.pg_isready) on this container and/or confirming Answer retries the DB handshake.Note: the
emptyDirmount at/var/lib/postgresqlis correct for thepostgres:18image (its VOLUME/PGDATA moved to/var/lib/postgresql/18/dockerunder that parent), so no change is needed there.🤖 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/answer/ci/external-db-values.yaml` around lines 48 - 67, The disposable PostgreSQL pod in the external DB values has no readiness gating, so Answer may try AUTO_INSTALL or connect before the database is accepting connections. Add a readinessProbe to the postgresql container (for example using pg_isready) and, if needed, ensure the Answer deployment’s startup logic can retry the initial DB handshake; use the existing postgresql container spec in external-db-values.yaml to place the change without altering the emptyDir mount.
🤖 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/answer/ci/external-db-values.yaml`:
- Around line 48-67: The disposable PostgreSQL pod in the external DB values has
no readiness gating, so Answer may try AUTO_INSTALL or connect before the
database is accepting connections. Add a readinessProbe to the postgresql
container (for example using pg_isready) and, if needed, ensure the Answer
deployment’s startup logic can retry the initial DB handshake; use the existing
postgresql container spec in external-db-values.yaml to place the change without
altering the emptyDir mount.
In `@charts/answer/templates/_helpers.tpl`:
- Around line 334-337: The helper block has an unused local assignment and a
redundant outer guard: in the `_helpers.tpl` template, `answer.databaseMode` is
included only for its side effect, so bind it to `$_` instead of `$databaseMode`
to make that intent clear, and consider removing the extra
`.Values.backup.enabled` `if` since `answer.backupEnabled` already performs the
same check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 024fe4f7-8777-4a1e-831d-de89734454bb
📒 Files selected for processing (6)
charts/answer/ci/external-db-values.yamlcharts/answer/ci/external-secrets.yamlcharts/answer/templates/NOTES.txtcharts/answer/templates/_helpers.tplcharts/answer/templates/validate.yamlcharts/answer/tests/validation_test.yaml
mberlofa
force-pushed
the
fix/answer-template-standards
branch
from
2dbcc16 to
02f25d5
Compare
|
Addressed both CodeRabbit review-body nitpicks. Changes:
Validation:
Note: these CodeRabbit items were posted in the review summary/body, not as active review threads, so there are no thread IDs to reply to or resolve for them. |
1 participant
Summary
Validation
Site PR: helmforgedev/site#324
Issue: #633
Summary by CodeRabbit