fix(changedetection): align template standards#650
Conversation
github-actions
Bot
added
area:templates
area:tests
chart:changedetection
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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR adds chart-time validation for ingress, gateway, externalSecrets, and podLabels, makes ingress host rendering omit empty hosts, adds tests for those paths, and renumbers NOTES.txt sections. ChangesChangedetection chart validation
NOTES.txt Section Renumbering
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Helm
participant validate.yaml
participant changedetection.validate
Helm->>validate.yaml: render chart
validate.yaml->>changedetection.validate: include current context
changedetection.validate->>changedetection.validate: validate ingress, gateway, externalSecrets, podLabels
changedetection.validate-->>Helm: fail or continue
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
🟢 Security Scan:
|
| Framework | Score |
|---|---|
| MITRE + NSA + SOC2 | 87.878784% |
✅ 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: 9421840e3a
ℹ️ 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 (2)
charts/changedetection/templates/validate.yaml (1)
1-2: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winyamllint flags this file; consider excluding it from raw YAML linting.
Since this file's raw content is entirely Go template directives (never valid standalone YAML), yamllint understandably fails to parse it. If the yamllint config doesn't already exclude pure-template files, add a targeted disable or glob exclusion for
validate.yamlfiles to avoid recurring false positives in CI.🤖 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/changedetection/templates/validate.yaml` around lines 1 - 2, The raw template in changedetection’s validate.yaml is being parsed by yamllint even though it is not valid standalone YAML. Update the yamllint configuration to exclude this pure Go-template file pattern, or add a targeted disable for validate.yaml so the lint step skips it. Make the change in the lint config rather than the template itself, and ensure the exclusion covers the changedetection chart’s validate.yaml files.Source: Linters/SAST tools
charts/changedetection/templates/_helpers.tpl (1)
69-73: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
changedetection.selectorLabelsin this guard
The chart already defineschangedetection.selectorLabels; derive the disallowed keys from that helper instead of hardcoding"app.kubernetes.io/name"and"app.kubernetes.io/instance". That keeps the validation aligned if selector labels change.🤖 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/changedetection/templates/_helpers.tpl` around lines 69 - 73, The podLabels validation in changedetection.selectorLabels is hardcoded to specific keys, so update the guard in the _helpers.tpl range to derive the forbidden labels from changedetection.selectorLabels instead of comparing against literal app.kubernetes.io/name and app.kubernetes.io/instance values. Use the existing helper to determine which selector labels must not be overridden, and keep the fail message behavior the same while making the validation follow any future selector label changes.
🤖 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/changedetection/templates/_helpers.tpl`:
- Around line 69-73: The podLabels validation in changedetection.selectorLabels
is hardcoded to specific keys, so update the guard in the _helpers.tpl range to
derive the forbidden labels from changedetection.selectorLabels instead of
comparing against literal app.kubernetes.io/name and app.kubernetes.io/instance
values. Use the existing helper to determine which selector labels must not be
overridden, and keep the fail message behavior the same while making the
validation follow any future selector label changes.
In `@charts/changedetection/templates/validate.yaml`:
- Around line 1-2: The raw template in changedetection’s validate.yaml is being
parsed by yamllint even though it is not valid standalone YAML. Update the
yamllint configuration to exclude this pure Go-template file pattern, or add a
targeted disable for validate.yaml so the lint step skips it. Make the change in
the lint config rather than the template itself, and ensure the exclusion covers
the changedetection chart’s validate.yaml files.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1ab1fc99-8ca2-407a-a3c7-e3e01e2c2600
📒 Files selected for processing (4)
charts/changedetection/templates/NOTES.txtcharts/changedetection/templates/_helpers.tplcharts/changedetection/templates/validate.yamlcharts/changedetection/tests/validation_test.yaml
mberlofa
force-pushed
the
fix/changedetection-template-standards
branch
from
9421840 to
31178f6
Compare
## Summary - register changedetection in the site sync playground registry - keep the playground/site sync gate aligned with the chart validation PR ## Related - Chart PR: helmforgedev/charts#650 - Issue: helmforgedev/charts#633 ## Validation - make site-sync-check CHART=changedetection - npm run lint - npm run format:check - npm run build - make release-check REPO=site - make attribution-check REPO=site <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added support for the **changedetection** chart in the playground, so it now appears in chart selection and filtering. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
mberlofa
force-pushed
the
fix/changedetection-template-standards
branch
from
31178f6 to
a287a19
Compare
|
Addressed the still-valid CodeRabbit review-body nitpick for selector label validation. Changes:
Validation:
Skipped item:
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. The existing inline thread is already resolved. |
1 participant
Summary
Validation
Site PR: helmforgedev/site#330 (merged)
Issue: #633
Summary by CodeRabbit
spec.rules[].hostis omitted when no host is provided, while still producing a valid catch-all rule.