docs(immich): sync template standards updates#354
Open
mberlofa wants to merge 2 commits into
Open
Conversation
|
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)
📝 WalkthroughWalkthroughThis PR adds immich network policy playground fields, enables immich in the playground chart list, and reformats the immich chart values table. ChangesImmich playground and docs
Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/data/playground-configs.ts (1)
236-262: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winDuplicate "Network Policy" section for apache.
Two separate field groups named
Network Policy, both gated bynetworkPolicy.enabled, now exist in the same chart config array (lines 236-262 and 285-305). This renders two identically-titled collapsible sections in the playground UI instead of one, which is confusing and likely unintended — the new egress CIDR/port fields should be merged into the existing group.♻️ Proposed fix: merge into the existing Network Policy group
{ name: 'Network Policy', collapsible: true, gateField: 'networkPolicy.enabled', fields: [ { label: 'Restrict Ingress', key: 'networkPolicy.ingress.enabled', type: 'toggle', default: 'false', description: 'Restrict inbound traffic only when selectors are configured', }, { label: 'Allow DNS', key: 'networkPolicy.egress.allowDns', type: 'toggle', default: 'true', description: 'Allow DNS egress', }, { label: 'Allow Internet', key: 'networkPolicy.egress.allowInternet', type: 'toggle', default: 'true', description: 'Allow general outbound traffic', }, + { + label: 'Extra Egress CIDR', + key: 'networkPolicy.extraEgress[0].to[0].ipBlock.cidr', + type: 'text', + default: '10.0.0.0/8', + description: 'Additional destination allowed by NetworkPolicy', + }, + { + label: 'Extra Egress Port', + key: 'networkPolicy.extraEgress[0].ports[0].port', + type: 'number', + default: '443', + description: 'Additional TCP egress port', + }, ], }, ... - { - name: 'Network Policy', - collapsible: true, - gateField: 'networkPolicy.enabled', - fields: [ - { - label: 'Extra Egress CIDR', - key: 'networkPolicy.extraEgress[0].to[0].ipBlock.cidr', - type: 'text', - default: '10.0.0.0/8', - description: 'Additional destination allowed by NetworkPolicy', - }, - { - label: 'Extra Egress Port', - key: 'networkPolicy.extraEgress[0].ports[0].port', - type: 'number', - default: '443', - description: 'Additional TCP egress port', - }, - ], - },Also applies to: 285-305
🤖 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 `@src/data/playground-configs.ts` around lines 236 - 262, The playground config defines two separate collapsible groups named Network Policy in the same chart array, both using networkPolicy.enabled, which creates duplicate UI sections. Merge the new egress CIDR/port fields into the existing Network Policy group in playground-configs.ts rather than keeping a second group, and keep the unique field keys under the same fields array so the apache config shows only one section.src/pages/playground.astro (1)
23-32: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAdd Immich’s Network Policy controls to the curated playground config
src/pages/playground.astro:23-32
Addingimmichhere only makes the chart selectable in the playground; the curated Immich config still overrides schema-generated fields and has noNetwork Policygroup, sonetworkPolicy.extraEgresswon’t surface.🤖 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 `@src/pages/playground.astro` around lines 23 - 32, Update the curated Immich playground config so it includes the Network Policy controls instead of only adding immich to siteSyncPlaygroundConfigs. In the playground config source used by immich, preserve the schema-generated Network Policy group and ensure networkPolicy.extraEgress is exposed rather than being overridden by custom curated fields. Use the immich entry and the playground config definitions to locate the override and merge in the missing group.
🤖 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.
Outside diff comments:
In `@src/data/playground-configs.ts`:
- Around line 236-262: The playground config defines two separate collapsible
groups named Network Policy in the same chart array, both using
networkPolicy.enabled, which creates duplicate UI sections. Merge the new egress
CIDR/port fields into the existing Network Policy group in playground-configs.ts
rather than keeping a second group, and keep the unique field keys under the
same fields array so the apache config shows only one section.
In `@src/pages/playground.astro`:
- Around line 23-32: Update the curated Immich playground config so it includes
the Network Policy controls instead of only adding immich to
siteSyncPlaygroundConfigs. In the playground config source used by immich,
preserve the schema-generated Network Policy group and ensure
networkPolicy.extraEgress is exposed rather than being overridden by custom
curated fields. Use the immich entry and the playground config definitions to
locate the override and merge in the missing group.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: bd0c49e2-ab2a-468e-9710-272a7f746575
📒 Files selected for processing (3)
src/data/playground-configs.tssrc/pages/docs/charts/immich.mdxsrc/pages/playground.astro
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validation
Chart PR: helmforgedev/charts#676
Summary by CodeRabbit