Conversation
Improve commands in docs to use jsonpatch to add elements instead of replacing whole arrays Signed-off-by: Kirill Ilin <stitch14@yandex.ru>
✅ Deploy Preview for cozystack ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Summary of ChangesHello @sircthulhu, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📝 WalkthroughWalkthroughSix documentation files are updated with consistent Kubernetes patch command conversions. kubectl patch operations are replaced from JSON Merge Patch format (--type=merge) to JSON Patch format (--type=json), using explicit "add" operations to append items to arrays rather than replace them wholesale. Advisory alerts about Merge Patch behavior are removed. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The transition from merge patches to JSON patches is a significant improvement for the documentation. It ensures that users can enable multiple services or packages (like the dashboard, NFS driver, or GPU operator) without accidentally overwriting existing configuration in the arrays. This makes the guides much more robust for non-trivial setups. One minor point to consider is that JSON patches are stricter than merge patches regarding the existence of the target path; I've highlighted a couple of places where this might affect users with a minimal initial configuration.
| kubectl patch packages.cozystack.io cozystack.cozystack-platform --type=json \ | ||
| -p '[{"op": "add", "path": "/spec/components/platform/values/bundles/enabledPackages/-", "value": "nfs-driver"}]' |
There was a problem hiding this comment.
The switch to JSON patch is a great improvement for preserving existing array elements. However, unlike merge patches, JSON patches will fail if the target path does not exist. Since bundles.enabledPackages is not included in the minimal configuration example provided in the installation guides, this command will fail with a 'pointer to non-existent target' error for users who haven't manually added that section. You might want to add a note about this or ensure the base configuration in the 'Getting Started' guide includes these fields as empty arrays.
| kubectl patch packages.cozystack.io cozystack.cozystack-platform --type=json \ | ||
| -p '[{"op": "add", "path": "/spec/components/platform/values/bundles/enabledPackages/-", "value": "gpu-operator"}]' |
There was a problem hiding this comment.
Similar to the NFS guide, this JSON patch assumes that the /spec/components/platform/values/bundles/enabledPackages path already exists. If a user is following the default minimal installation, this path will be missing, and the kubectl patch command will fail. Consider mentioning that the bundles section should be present in the Platform Package configuration.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@content/en/docs/v1/install/cozystack/_index.md`:
- Around line 694-695: The documentation uses a kubectl JSON Patch that appends
"dashboard" to the exposedServices array but fails if the array doesn't exist
and will duplicate entries on repeated runs; update the doc around the kubectl
patch example to (1) note that the parent array
spec/components/platform/values/publishing/exposedServices must already exist
and instruct users to initialize it first (e.g., via kubectl edit or by adding
it to their Package YAML) and (2) warn that the patch is non‑idempotent (it will
append duplicates on repeated runs) and advise checking/specifically ensuring
"dashboard" is not already present before re-running the kubectl patch command.
In `@content/en/docs/v1/virtualization/gpu.md`:
- Around line 38-39: The JSON Patch add using the
"/spec/components/platform/values/bundles/enabledPackages/-" path will fail if
enabledPackages is missing; change the docs to show a safe two-step or merge
approach: either first ensure the array exists by adding an empty array at
"/spec/components/platform/values/bundles/enabledPackages" and then append the
element, or use a strategic/merge-style patch that sets
spec.components.platform.values.bundles.enabledPackages to include
"gpu-operator" in one operation; reference the kubectl patch invocation and the
JSON path "/spec/components/platform/values/bundles/enabledPackages" in the
instructions so users run the safe two-step or merge alternative instead of the
single add with "-" token.
---
Duplicate comments:
In `@content/en/docs/v1/getting-started/install-cozystack.md`:
- Around line 629-630: The JSON Patch command shown (kubectl ... --type=json
with op "add" and path
"/spec/components/platform/values/publishing/exposedServices/-") is correct for
the documented scenario but needs an inline caveat: add a short note next to
this command explaining the edge cases—if the exposedServices array is missing
the JSON Patch will fail and repeated runs may not be idempotent—so advise to
pre-seed the array or use a different patch strategy; also add a brief
confirmation that the later externalIPs patches intentionally use --type=merge
(full-array replacement) and should remain as-is.
In `@content/en/docs/v1/storage/nfs.md`:
- Around line 15-16: The patch command fails when bundles.enabledPackages is
absent because the JSON Patch add at path
"/spec/components/platform/values/bundles/enabledPackages/-" requires the array
to exist; update the doc around the kubectl patch for "packages.cozystack.io
cozystack.cozystack-platform" to present two variants: (1) a safe check/ensure
step to create or verify the "bundles.enabledPackages" array before using the
JSON Patch add, and (2) a fallback using kubectl patch --type=merge that
initializes the field and appends "nfs-driver" in one operation; mention the
same fix pattern as used in gpu.md and reference the exact path and command
("kubectl patch ... -p '[{\"op\": \"add\", \"path\":
\"/spec/components/platform/values/bundles/enabledPackages/-\", \"value\":
\"nfs-driver\"}]'") so readers can choose the correct variant.
In `@content/en/docs/v1/virtualization/_index.md`:
- Around line 70-71: The kubectl JSON patch shown (the command adding to path
"/spec/components/platform/values/publishing/exposedServices/-") will fail if
the exposedServices array is absent and will append duplicates if run twice;
update the _index.md text to call this out and provide an idempotent
alternative: either a two-step approach that first ensures the array exists (add
"/spec/components/platform/values/publishing/exposedServices" as an array if
missing) then appends, or use a single replace/merge-style patch that sets the
exposedServices array to ["cdi-uploadproxy"] (i.e. target
"/spec/components/platform/values/publishing/exposedServices" rather than the
"/-/ element add") so the operation is safe to re-run; mention both the
failure-on-absence and non-idempotency and include the kubectl command names and
path "/spec/components/platform/values/publishing/exposedServices" so readers
can update their commands accordingly.
In `@content/en/docs/v1/virtualization/proxmox-migration.md`:
- Around line 26-27: The JSON patch currently appends "cdi-uploadproxy" to
"/spec/components/platform/values/publishing/exposedServices/-" which fails when
the array is missing and is not idempotent; change the patch to either (a) a
two-op JSON patch that first "test" that the array path does not exist and then
"add" the full array value ["cdi-uploadproxy"] (so it creates the array when
absent), or (b) use a merge/strategic-merge style patch that sets
"exposedServices": ["cdi-uploadproxy"] under
spec.components.platform.values.publishing to ensure the array exists and avoid
duplicate entries; update the kubectl command that references the path
"/spec/components/platform/values/publishing/exposedServices" accordingly
(replace the single append op to "-" with one of these idempotent approaches).
| kubectl patch packages.cozystack.io cozystack.cozystack-platform --type=json \ | ||
| -p '[{"op": "add", "path": "/spec/components/platform/values/publishing/exposedServices/-", "value": "dashboard"}]' |
There was a problem hiding this comment.
add with - fails if exposedServices array is absent; no deduplication guard
The JSON Patch add with - path correctly appends to an existing array without clobbering other entries — the main motivation of this PR. Two caveats worth documenting:
-
Array must pre-exist. The
addoperation fails with"add operation does not apply: doc is missing path: '.../-': missing value"when the parent array doesn't exist. The Kubernetes API server will not recursively create nested objects for a JSON patch input. The base config shown in this doc always seedsexposedServices, so the common path is fine, but a user who created a minimal Package withoutexposedServiceswill hit this failure silently. A brief note like "IfexposedServicesis not yet present in your Package, initialize it first viakubectl editor add it to the YAML" would prevent confusion. -
Non-idempotent. Running the command twice appends
"dashboard"a second time, producing duplicates. A one-line note advising users to check for existing entries before re-running is worth adding.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@content/en/docs/v1/install/cozystack/_index.md` around lines 694 - 695, The
documentation uses a kubectl JSON Patch that appends "dashboard" to the
exposedServices array but fails if the array doesn't exist and will duplicate
entries on repeated runs; update the doc around the kubectl patch example to (1)
note that the parent array
spec/components/platform/values/publishing/exposedServices must already exist
and instruct users to initialize it first (e.g., via kubectl edit or by adding
it to their Package YAML) and (2) warn that the patch is non‑idempotent (it will
append duplicates on repeated runs) and advise checking/specifically ensuring
"dashboard" is not already present before re-running the kubectl patch command.
| kubectl patch packages.cozystack.io cozystack.cozystack-platform --type=json \ | ||
| -p '[{"op": "add", "path": "/spec/components/platform/values/bundles/enabledPackages/-", "value": "gpu-operator"}]' |
There was a problem hiding this comment.
add /- fails when enabledPackages doesn't exist — common first-run scenario
The enabledPackages field is absent from the base Platform Package YAML documented across these docs. For any user who has never previously set bundles.enabledPackages, the JSON Patch operation will fail:
The request is invalid: ... add operation does not apply:
doc is missing path: "/spec/components/platform/values/bundles/enabledPackages/-"
The Kubernetes API server will not recursively create nested objects for a JSON patch input. The - token requires the array to already exist at the parent path.
Consider replacing with a two-step approach or adding a note with a fallback for first-time users:
🛠️ Suggested fallback for when the array doesn't yet exist
- kubectl patch packages.cozystack.io cozystack.cozystack-platform --type=json \
- -p '[{"op": "add", "path": "/spec/components/platform/values/bundles/enabledPackages/-", "value": "gpu-operator"}]'
+ # If enabledPackages already exists in your Package:
+ kubectl patch packages.cozystack.io cozystack.cozystack-platform --type=json \
+ -p '[{"op": "add", "path": "/spec/components/platform/values/bundles/enabledPackages/-", "value": "gpu-operator"}]'
+
+ # If enabledPackages does not yet exist (first time adding a package):
+ kubectl patch packages.cozystack.io cozystack.cozystack-platform --type=merge \
+ -p '{"spec":{"components":{"platform":{"values":{"bundles":{"enabledPackages":["gpu-operator"]}}}}}}'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| kubectl patch packages.cozystack.io cozystack.cozystack-platform --type=json \ | |
| -p '[{"op": "add", "path": "/spec/components/platform/values/bundles/enabledPackages/-", "value": "gpu-operator"}]' | |
| # If enabledPackages already exists in your Package: | |
| kubectl patch packages.cozystack.io cozystack.cozystack-platform --type=json \ | |
| -p '[{"op": "add", "path": "/spec/components/platform/values/bundles/enabledPackages/-", "value": "gpu-operator"}]' | |
| # If enabledPackages does not yet exist (first time adding a package): | |
| kubectl patch packages.cozystack.io cozystack.cozystack-platform --type=merge \ | |
| -p '{"spec":{"components":{"platform":{"values":{"bundles":{"enabledPackages":["gpu-operator"]}}}}}}' |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@content/en/docs/v1/virtualization/gpu.md` around lines 38 - 39, The JSON
Patch add using the "/spec/components/platform/values/bundles/enabledPackages/-"
path will fail if enabledPackages is missing; change the docs to show a safe
two-step or merge approach: either first ensure the array exists by adding an
empty array at "/spec/components/platform/values/bundles/enabledPackages" and
then append the element, or use a strategic/merge-style patch that sets
spec.components.platform.values.bundles.enabledPackages to include
"gpu-operator" in one operation; reference the kubectl patch invocation and the
JSON path "/spec/components/platform/values/bundles/enabledPackages" in the
instructions so users run the safe two-step or merge alternative instead of the
single add with "-" token.
2 participants
Improve commands in docs to use jsonpatch to add elements instead of replacing whole arrays
Summary by CodeRabbit
Release Notes