fix(ckan): align template standards#652
Conversation
github-actions
Bot
added
area:templates
area:tests
chart:ckan
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds centralized Helm validation for external services, ingress, backups, and selector labels; updates ingress host rendering for host-less rules; adds matching tests; and rewrites NOTES.txt into structured installation and operational guidance. ChangesConfiguration Validation and Ingress
Estimated code review effort: 3 (Moderate) | ~25 minutes NOTES.txt Restructuring
Estimated code review effort: 2 (Simple) | ~12 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Standards Check (GR-079) — PASSEvery changed chart fully passes standards-check. |
🟢 Security Scan:
|
| Framework | Score |
|---|---|
| MITRE + NSA + SOC2 | 78.71212% |
✅ Security posture acceptable.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b82b640d3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
charts/ckan/tests/validation_test.yaml (1)
49-68: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing coverage for
backup.s3.bucketandbackup.s3.accessKeyfailure branches.The helper (
ckan.validate) also fails whenbackup.s3.bucketorbackup.s3.accessKey/existingSecretis missing (see_helpers.tpllines 66-71), but only theendpointandsecretKeybranches are tested here.🤖 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/ckan/tests/validation_test.yaml` around lines 49 - 68, Add validation test coverage for the missing backup S3 branches in ckan.validate. Update the validation_test.yaml cases around the existing backup.enabled scenarios to also assert failures when backup.s3.bucket is omitted and when backup.s3.accessKey is omitted without backup.s3.existingSecret, matching the checks in _helpers.tpl. Use failedTemplate assertions with the expected error messages so both branches are exercised alongside the existing endpoint and secretKey tests.
🤖 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/ckan/tests/validation_test.yaml`:
- Around line 49-68: Add validation test coverage for the missing backup S3
branches in ckan.validate. Update the validation_test.yaml cases around the
existing backup.enabled scenarios to also assert failures when backup.s3.bucket
is omitted and when backup.s3.accessKey is omitted without
backup.s3.existingSecret, matching the checks in _helpers.tpl. Use
failedTemplate assertions with the expected error messages so both branches are
exercised alongside the existing endpoint and secretKey tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2a768af5-9e1e-4d43-8692-4588271a1a33
📒 Files selected for processing (7)
charts/ckan/templates/NOTES.txtcharts/ckan/templates/_helpers.tplcharts/ckan/templates/ingress.yamlcharts/ckan/templates/validate.yamlcharts/ckan/tests/backup_test.yamlcharts/ckan/tests/ingress_test.yamlcharts/ckan/tests/validation_test.yaml
💤 Files with no reviewable changes (1)
- charts/ckan/tests/backup_test.yaml
mberlofa
force-pushed
the
fix/ckan-template-standards
branch
from
9b82b64 to
c899418
Compare
mberlofa
force-pushed
the
fix/ckan-template-standards
branch
from
c899418 to
89e8e20
Compare
|
Addressed the CodeRabbit review-summary nitpick for backup validation coverage. Changes:
Validation:
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. The existing inline thread is already resolved. |
## Summary - register ckan in the site sync playground registry - keep the playground/site sync gate aligned with the chart validation PR ## Validation - make site-sync-check CHART=ckan - npm run lint - npm run format:check - npm run build - make release-check REPO=site - make attribution-check REPO=site Chart PR: helmforgedev/charts#652 Issue: helmforgedev/charts#633
1 participant
Summary
Validation
Site PR: helmforgedev/site#332
Issue: #633
Summary by CodeRabbit
hostrule when hosts are specified without a host.host, and adjusted backup assertions for default DB host behavior.