fix(immich): align template standards#676
Conversation
github-actions
Bot
added
area:docs
area:schema
area:templates
area:tests
area:values
chart:immich
size:M
labels
|
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 (8)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThis PR adds ChangesNetworkPolicy extraEgress Feature
NOTES.txt Restructuring
Helm connection test suite
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 |
Standards Check (GR-079) — PASSEvery changed chart fully passes standards-check. |
🟢 Security Scan:
|
| Framework | Score |
|---|---|
| MITRE + NSA + SOC2 | 88.611115% |
✅ 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 (1)
charts/immich/tests/networkpolicy_test.yaml (1)
27-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test verifying egress + extraEgress ordering.
The values.yaml comment promises extraEgress rules are "appended after
networkPolicy.egress", but the current fixture only setsextraEgress(baseegressstays empty). No test verifies the concatenation order when both lists are non-empty.♻️ Suggested additional test case
- it: should append extra egress rules values: - networkpolicy-extra-egress-values.yaml asserts: - equal: path: spec.egress[0].to[0].ipBlock.cidr value: 10.0.0.0/8 - equal: path: spec.egress[0].ports[0].port value: 443 + + - it: should append extraEgress after base egress rules + set: + networkPolicy.enabled: true + networkPolicy.egress[0].to[0].ipBlock.cidr: 172.16.0.0/12 + networkPolicy.extraEgress[0].to[0].ipBlock.cidr: 10.0.0.0/8 + asserts: + - equal: + path: spec.egress[0].to[0].ipBlock.cidr + value: 172.16.0.0/12 + - equal: + path: spec.egress[1].to[0].ipBlock.cidr + value: 10.0.0.0/8🤖 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/immich/tests/networkpolicy_test.yaml` around lines 27 - 37, The networkpolicy test currently only covers extraEgress by itself, so it does not verify the documented append order when both networkPolicy.egress and extraEgress are set. Update the networkpolicy_test.yaml fixture to include a case using both lists and assert through the existing helm-unittest paths that the base egress entries appear first and the extraEgress entries are appended afterward; use the current test structure and the spec.egress assertions to locate the right place.
🤖 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/immich/tests/networkpolicy_test.yaml`:
- Around line 27-37: The networkpolicy test currently only covers extraEgress by
itself, so it does not verify the documented append order when both
networkPolicy.egress and extraEgress are set. Update the networkpolicy_test.yaml
fixture to include a case using both lists and assert through the existing
helm-unittest paths that the base egress entries appear first and the
extraEgress entries are appended afterward; use the current test structure and
the spec.egress assertions to locate the right place.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6b3e6ec8-4c91-49cd-b65e-ec460e034fa8
📒 Files selected for processing (8)
charts/immich/README.mdcharts/immich/templates/NOTES.txtcharts/immich/templates/networkpolicy.yamlcharts/immich/tests/networkpolicy-extra-egress-values.yamlcharts/immich/tests/networkpolicy_test.yamlcharts/immich/tests/test-connection.yamlcharts/immich/values.schema.jsoncharts/immich/values.yaml
![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
charts/charts/immich/tests/test-connection.yaml
Lines 2 to 3 in 7a633e4
Keep the Helm test hook under templates
Because this test Pod was moved to chart-root tests/, it is no longer rendered into the installed release as a Helm test hook; I checked helm create --help, which shows test files under templates/tests/, and helm test --help, which says tests are defined in the installed chart. In this location the annotated Pod is just an inert chart file, so the helm test command advertised in NOTES has no Immich connection test to run.
ℹ️ 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".
mberlofa
force-pushed
the
fix/immich-template-standards
branch
from
bfebd2f to
4511e4a
Compare
mberlofa
force-pushed
the
fix/immich-template-standards
branch
from
4511e4a to
495cdae
Compare
|
Addressed the CodeRabbit review-summary nitpick about What changed:
Validation:
This feedback was present in the CodeRabbit review summary rather than an unresolved review thread, so there is no thread ID to reply to or resolve. |
1 participant
Summary
templates/tests/sohelm testrenders and executes it, with unittest coverage for the hook pod.networkPolicy.extraEgressand unit coverage.Related
Validation
helm template test charts/immich | rg -n "helm.sh/hook|test-connection"(hook rendered fromtemplates/tests/test-connection.yaml)helm unittest charts/immich(45 tests, 8 suites)make template-standards-check CHART=immichnode scripts/charts/validate-chart.js --chart immich --no-k3dmake validate-chart CHART=immich TIMEOUT=900(FULLY VALIDATED, 12 layers)make site-sync-check CHART=immichmake release-check REPO=chartsmake attribution-check REPO=chartsSite Validation
npm run lintnpm run format:checknpm run buildSummary by CodeRabbit
networkPolicy.extraEgress, allowing additional egress destinations and ports.extraEgressis appended after the base egress rules.enabled,ingress,egress, andextraEgress.