fix(oauth2-proxy): align template standards#682
Conversation
github-actions
Bot
added
area:docs
area:schema
area:templates
area:tests
area:values
chart:oauth2-proxy
Standards Check (GR-079) — PASSEvery changed chart fully passes standards-check. |
🟢 Security Scan:
|
| Framework | Score |
|---|---|
| MITRE + NSA + SOC2 | 94.9495% |
✅ Security posture acceptable.
|
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 feature
NOTES.txt restructuring
Estimated code review effort: 2 (Simple) | ~12 minutes Sequence Diagram(s)sequenceDiagram
participant Values as values.yaml
participant Schema as values.schema.json
participant Template as networkpolicy.yaml
Values->>Schema: define networkPolicy.extraEgress
Template->>Values: read extraEgress
Template->>Template: render appended egress entries
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![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 the Helm test hook under templates
When this chart is installed, this test pod is no longer part of the rendered release because Helm chart tests must live under the chart's templates/ directory; the local check helm template test charts/oauth2-proxy --show-only tests/test-connection.yaml also reports that this is not a template. As a result helm test has no connection test hook to run even though the chart notes still instruct users to run it, so the hook should remain under templates/tests/ or another path below templates/.
ℹ️ 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/oauth2-proxy/tests/networkpolicy_test.yaml (1)
28-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding a test for
extraEgresscombined with customnetworkPolicy.egress.Since the template appends
extraEgressafter either the default or a customegress(seenetworkpolicy.yamllines 29-45), a test exercising bothnetworkPolicy.egressandnetworkPolicy.extraEgresstogether would close a coverage gap for that combination.♻️ Suggested additional test case
- it: should append extraEgress after custom egress set: networkPolicy.enabled: true networkPolicy.egress: - ports: - protocol: TCP port: 9000 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: 9000 - equal: path: spec.egress[1].to[0].ipBlock.cidr value: 10.80.0.0/16🤖 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/oauth2-proxy/tests/networkpolicy_test.yaml` around lines 28 - 44, Add a test that covers `networkPolicy.egress` and `networkPolicy.extraEgress` together, since `networkpolicy.yaml` appends `extraEgress` after any custom egress. Update `charts/oauth2-proxy/tests/networkpolicy_test.yaml` by adding a case like `should append extraEgress after custom egress` that sets both values, then assert the custom egress entry stays first and the extra egress entry is appended afterward. Use the existing `spec.egress` assertions in `networkPolicy`/`networkpolicy_test.yaml` to locate the coverage gap.
🤖 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/oauth2-proxy/tests/networkpolicy_test.yaml`:
- Around line 28-44: Add a test that covers `networkPolicy.egress` and
`networkPolicy.extraEgress` together, since `networkpolicy.yaml` appends
`extraEgress` after any custom egress. Update
`charts/oauth2-proxy/tests/networkpolicy_test.yaml` by adding a case like
`should append extraEgress after custom egress` that sets both values, then
assert the custom egress entry stays first and the extra egress entry is
appended afterward. Use the existing `spec.egress` assertions in
`networkPolicy`/`networkpolicy_test.yaml` to locate the coverage gap.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ee061303-57d6-46e2-8d65-7f2ca4d5cecb
📒 Files selected for processing (8)
charts/oauth2-proxy/README.mdcharts/oauth2-proxy/templates/NOTES.txtcharts/oauth2-proxy/templates/networkpolicy.yamlcharts/oauth2-proxy/tests/networkpolicy_test.yamlcharts/oauth2-proxy/tests/test-connection.yamlcharts/oauth2-proxy/tests/test_connection_test.yamlcharts/oauth2-proxy/values.schema.jsoncharts/oauth2-proxy/values.yaml
💤 Files with no reviewable changes (1)
- charts/oauth2-proxy/tests/test_connection_test.yaml
mberlofa
force-pushed
the
fix/oauth2-proxy-template-standards
branch
from
2889150 to
ddc8146
Compare
mberlofa
force-pushed
the
fix/oauth2-proxy-template-standards
branch
from
ddc8146 to
cdad333
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 functionalnetworkPolicy.extraEgresssupport with schema and unit coverageValidation
make validate-chart CHART=oauth2-proxy 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=oauth2-proxymake release-check REPO=chartspassed with the expected GR-077 post-merge release confirmation warning.make attribution-check REPO=chartsSite PR: helmforgedev/site#360
Issue: #633
Summary by CodeRabbit
networkPolicy.extraEgressto append additional full egress rules (preserving chart default DNS/HTTP(S) egress behavior when custom egress isn’t provided).networkPolicy.enabledand configuringextraEgress.extraEgress, and combinedegress+extraEgress.