fix(openreel-video): align template standards#684
Conversation
github-actions
Bot
added
area:docs
area:schema
area:templates
area:tests
area:values
chart:openreel-video
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 (1)
📝 WalkthroughWalkthroughAdds ChangesNetworkPolicy extraEgress and service validation
Estimated code review effort: 2 (Simple) | ~12 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
🟢 Security Scan:
|
| Framework | Score |
|---|---|
| MITRE + NSA + SOC2 | 92.92929% |
✅ 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
Keep Helm test hook under templates
When this pod is moved to the chart-level tests/ directory, Helm no longer renders it into the release manifest as a test hook. I checked helm test --help (tests to be run are defined in the chart that was installed) and helm template review charts/openreel-video, which now emits no helm.sh/hook: test; as a result the documented helm test openreel-video path has no connection checks to run. Keep the hook manifest under templates/tests/ and reserve tests/ for helm-unittest suites.
ℹ️ 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/openreel-video/tests/networkpolicy_test.yaml (1)
8-38: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding a combined egress+extraEgress test case.
Current tests cover "only extraEgress" and "neither set" but not both
networkPolicy.egressandnetworkPolicy.extraEgressset together, which exercises the template's concatenation logic more thoroughly.♻️ Suggested additional test case
- it: should combine egress and extraEgress rules set: networkPolicy.enabled: true networkPolicy.egress: - to: - namespaceSelector: {} ports: - protocol: TCP port: 53 networkPolicy.extraEgress: - to: - ipBlock: cidr: 10.80.0.0/16 ports: - protocol: TCP port: 443 asserts: - equal: path: spec.egress[0].ports[0].port value: 53 - equal: path: spec.egress[1].ports[0].port value: 443🤖 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/openreel-video/tests/networkpolicy_test.yaml` around lines 8 - 38, Add a test in networkpolicy_test.yaml that covers networkPolicy.egress and networkPolicy.extraEgress being set together, since the current cases only verify them separately. Update the NetworkPolicy test suite to assert the combined spec.egress ordering/contents produced by the template logic, using the existing render assertions around spec.egress and the networkPolicy.enabled/extraEgress setup as reference.
🤖 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/openreel-video/tests/networkpolicy_test.yaml`:
- Around line 8-38: Add a test in networkpolicy_test.yaml that covers
networkPolicy.egress and networkPolicy.extraEgress being set together, since the
current cases only verify them separately. Update the NetworkPolicy test suite
to assert the combined spec.egress ordering/contents produced by the template
logic, using the existing render assertions around spec.egress and the
networkPolicy.enabled/extraEgress setup as reference.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 72aa60b8-d23a-4d03-a9d3-e8909afc1857
📒 Files selected for processing (9)
charts/openreel-video/README.mdcharts/openreel-video/templates/NOTES.txtcharts/openreel-video/templates/_helpers.tplcharts/openreel-video/templates/networkpolicy.yamlcharts/openreel-video/templates/service.yamlcharts/openreel-video/tests/networkpolicy_test.yamlcharts/openreel-video/tests/test-connection.yamlcharts/openreel-video/values.schema.jsoncharts/openreel-video/values.yaml
mberlofa
force-pushed
the
fix/openreel-video-template-standards
branch
from
cece6b0 to
a2f4a45
Compare
mberlofa
force-pushed
the
fix/openreel-video-template-standards
branch
from
a2f4a45 to
aba58a0
Compare
|
Addressed the still-valid CodeRabbit review-body nitpick:
Validation completed after the fix:
This CodeRabbit note was in the review summary body, not as a separate resolvable review thread, so there was no thread ID to resolve. |
1 participant
Summary
templates/testssohelm testremains functionalservice.externalNamenetworkPolicy.extraEgresssupport with schema and unit coverageextraEgressis configuredValidation
make validate-chart CHART=openreel-video TIMEOUT=900: FULLY VALIDATED (12 layers), including k3d behavioral scenarios for default,ci/ci-values.yaml, andci/k3d-values.yaml.make site-sync-check CHART=openreel-videomake release-check REPO=chartspassed with the expected GR-077 post-merge release confirmation warning.make attribution-check REPO=chartsSite PR: helmforgedev/site#362
Issue: #633
Summary by CodeRabbit
networkPolicy.extraEgressto append additional egress rules to the chart’s NetworkPolicy.egressrules and any appendedextraEgressrules.networkPolicy.extraEgressguidance and refreshed security scan formatting.ExternalNameservices.enabled,ingress,egress, andextraEgress.egress+extraEgressrendering.