[virtual-machine] Disable instanceType validation - #1300
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe changes update version mappings and chart versions for the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Makefile
participant values_schema
User->>Makefile: Run generate target
Makefile--x values_schema: (Old) Update instanceType enum from instancetypes.yaml (now commented out)
Makefile->>values_schema: Update other schema fields (instanceProfile enum, etc.)
User->>values_schema: Validate config (instanceType unrestricted)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested labels
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Summary of Changes
Hello Timofei Larkin (@lllamnyp), 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 provides a critical workaround for an existing limitation in Cozystack that prevented the use of custom virtual machine instance types. By removing the static instanceType validation from the Helm chart schemas and adjusting the build process, users can now freely define and utilize their own instance types. This change impacts both the virtual-machine and vm-instance applications, ensuring greater flexibility in VM provisioning.
Highlights
- Disable Instance Type Validation: The primary change in this PR is the removal of static validation for
instanceTypein thevirtual-machineandvm-instanceHelm charts. This addresses an issue where custom instance types could not be used due to rigid schema validation. - Schema Modifications: The
enumconstraint for theinstanceTypeproperty has been removed fromvalues.schema.jsonin bothvirtual-machineandvm-instanceapplications. This allows users to specify any string as an instance type, enabling the use of custom types. - Build Process Adjustment: The
Makefilefor bothvirtual-machineandvm-instancehas been updated to comment out the step that dynamically populates theinstanceTypeenum in thevalues.schema.jsonfromkubevirt-instancetypestemplates. This ensures the schema remains flexible. - Version Bumps: The Helm chart versions for
virtual-machine(0.12.2to0.12.3) andvm-instance(0.10.1to0.10.2) have been incremented. Corresponding updates were made in theversions_mapfile. - SSH Keys Schema Improvement: A minor improvement was made to the
values.schema.jsonfor bothvirtual-machineandvm-instanceto explicitly define theitemstype asstringfor thesshKeysarray, enhancing schema clarity.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | Gemini (@gemini-code-assist) | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
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 (@gemini-code-assist) comments or fill out our survey to provide feedback.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request disables static validation for instanceType in the virtual-machine and vm-instance Helm charts, enabling the use of custom instance types. The changes involve commenting out validation logic in Makefiles and removing the enum from values.schema.json. Additionally, the sshKeys schema is updated to specify the item type as string. Consider removing the commented-out code in the Makefiles to improve maintainability.
| # INSTANCE_TYPES=$$(yq e '.metadata.name' -o=json -r ../../system/kubevirt-instancetypes/templates/instancetypes.yaml | yq 'split(" ") | . + [""]' -o json) \ | ||
| # && yq -i -o json ".properties.instanceType.enum = $${INSTANCE_TYPES}" values.schema.json |
There was a problem hiding this comment.
Consider removing these commented-out lines to reduce clutter. The code can be retrieved from Git history if needed.
| #INSTANCE_TYPES=$$(yq e '.metadata.name' -o=json -r ../../system/kubevirt-instancetypes/templates/instancetypes.yaml | yq 'split(" ") | . + [""]' -o json) \ | ||
| # && yq -i -o json ".properties.instanceType.enum = $${INSTANCE_TYPES}" values.schema.json |
There was a problem hiding this comment.
Consider removing these commented-out lines to reduce clutter. The code can be retrieved from Git history if needed.
| "default": [], | ||
| "description": "List of SSH public keys for authentication. Can be a single key or a list of keys.", | ||
| "items": {}, | ||
| "items": { |
There was a problem hiding this comment.
Adding type: string to the items property of sshKeys ensures that the schema accurately reflects the expected data type. This improves schema validation and helps prevent potential errors.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/apps/virtual-machine/Makefile (1)
6-7: Commented-out enum generation may leave README/schema out of syncRemoving the
instanceTypeenum injection fulfils the PR goal, but it also means:
•values.schema.jsonwill no longer list available instance types.
• The generated README will lose the autocomplete/details those enums provided.Please verify that downstream tooling (UI forms, docs, validation in CI) tolerates the open string and that README expectations are updated.
packages/apps/vm-instance/Makefile (1)
7-8: Ensure documentation still renders correctly after disablinginstanceTypeenumSame concern as in the
virtual-machineMakefile: the README generator will now emit an unconstrained string field. Double-check user guidance/docs so that operators know what values are acceptable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/apps/versions_map(2 hunks)packages/apps/virtual-machine/Chart.yaml(1 hunks)packages/apps/virtual-machine/Makefile(1 hunks)packages/apps/virtual-machine/values.schema.json(2 hunks)packages/apps/vm-instance/Chart.yaml(1 hunks)packages/apps/vm-instance/Makefile(1 hunks)packages/apps/vm-instance/values.schema.json(2 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: NickVolynkin
PR: cozystack/cozystack#1216
File: packages/apps/virtual-machine/values.yaml:0-0
Timestamp: 2025-07-26T18:12:05.641Z
Learning: In the cozystack repository, for the virtual-machine app's resources.sockets parameter, the value is intentionally kept as a string in values.yaml despite being documented as {int} type, because the string-to-integer parsing happens elsewhere in the system pipeline rather than at the YAML validation level.
Learnt from: NickVolynkin
PR: cozystack/cozystack#1196
File: packages/apps/http-cache/Makefile:24-27
Timestamp: 2025-07-14T16:23:12.803Z
Learning: In the cozystack repository, the `readme-generator` tool removes enum contents from values.schema.json files during its operation. Therefore, when using readme-generator in Makefiles, any enum values need to be injected back into the schema using yq commands after readme-generator has run, not before.
Learnt from: lllamnyp
PR: cozystack/cozystack#1233
File: packages/system/hetzner-robotlb/charts/robotlb/templates/deployment.yaml:33-35
Timestamp: 2025-07-23T09:15:09.658Z
Learning: The `./charts/robotlb/` directory in the hetzner-robotlb package contains vendored code, and the team generally avoids modifying vendored code to maintain clean separation from upstream dependencies.
📚 Learning: the `./charts/robotlb/` directory in the hetzner-robotlb package contains vendored code, and the tea...
Learnt from: lllamnyp
PR: cozystack/cozystack#1233
File: packages/system/hetzner-robotlb/charts/robotlb/templates/deployment.yaml:33-35
Timestamp: 2025-07-23T09:15:09.658Z
Learning: The `./charts/robotlb/` directory in the hetzner-robotlb package contains vendored code, and the team generally avoids modifying vendored code to maintain clean separation from upstream dependencies.
Applied to files:
packages/apps/virtual-machine/Chart.yamlpackages/apps/vm-instance/Chart.yaml
📚 Learning: in the cozystack repository, the `readme-generator` tool removes enum contents from values.schema.js...
Learnt from: NickVolynkin
PR: cozystack/cozystack#1196
File: packages/apps/http-cache/Makefile:24-27
Timestamp: 2025-07-14T16:23:12.803Z
Learning: In the cozystack repository, the `readme-generator` tool removes enum contents from values.schema.json files during its operation. Therefore, when using readme-generator in Makefiles, any enum values need to be injected back into the schema using yq commands after readme-generator has run, not before.
Applied to files:
packages/apps/virtual-machine/Makefilepackages/apps/vm-instance/Makefile
📚 Learning: in packages/apps/versions_map, using head for certain versions (like ferretdb 1.0.0) is intentional ...
Learnt from: kvaps
PR: cozystack/cozystack#1206
File: packages/apps/versions_map:30-31
Timestamp: 2025-07-16T13:17:07.580Z
Learning: In packages/apps/versions_map, using HEAD for certain versions (like ferretdb 1.0.0) is intentional and part of the core logic, not an oversight that needs to be fixed for reproducibility.
Applied to files:
packages/apps/versions_map
📚 Learning: in the cozystack repository, for the virtual-machine app's resources.sockets parameter, the value is...
Learnt from: NickVolynkin
PR: cozystack/cozystack#1216
File: packages/apps/virtual-machine/values.yaml:0-0
Timestamp: 2025-07-26T18:12:05.641Z
Learning: In the cozystack repository, for the virtual-machine app's resources.sockets parameter, the value is intentionally kept as a string in values.yaml despite being documented as {int} type, because the string-to-integer parsing happens elsewhere in the system pipeline rather than at the YAML validation level.
Applied to files:
packages/apps/virtual-machine/values.schema.jsonpackages/apps/vm-instance/values.schema.json
🪛 GitHub Actions: Pre-Commit Checks
packages/apps/virtual-machine/values.schema.json
[error] 123-131: Make generate step failed: The file was modified by the hook, indicating schema changes in the 'sshKeys.items' property from type 'string' to an empty object.
packages/apps/vm-instance/values.schema.json
[error] 131-139: Make generate step failed: The file was modified by the hook, indicating schema changes in the 'sshKeys.items' property from type 'string' to an empty object.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (5)
packages/apps/virtual-machine/Chart.yaml (1)
20-20: Chart version bump LGTMPatch-level increment to 0.12.3 is consistent with a backwards-compatible schema relaxation. No further issues noticed.
packages/apps/vm-instance/Chart.yaml (1)
20-20: Chart version bump LGTMVersion 0.10.2 correctly reflects the minor schema change while keeping semantic compatibility.
packages/apps/versions_map (1)
177-178: Version pinning looks correct—confirm commit hashesPinning former
HEADversions (0.12.2 / 0.10.1) to8ddbe32eand advancingHEADis the expected pattern. Please verify that8ddbe32eindeed corresponds to the last commit before this PR landed to avoid reproducibility gaps.Also applies to: 197-198
packages/apps/vm-instance/values.schema.json (1)
101-105: Enum removal weakens validation – consider a minimal safeguardDropping the
enumentirely achieves the goal of allowing custom types, but it also accepts empty strings or obviously invalid identifiers. Adding a lightweight constraint such as"minLength": 1inside the same object would still permit arbitrary custom values while preventing empty/accidental input.
packages/apps/virtual-machine/values.schema.json (1)
93-97: Same validation relaxation as vm-instance – ensure runtime handles unknown typesWith the enum removed, any string (e.g. typos) reaches the controller. Double-check that downstream components fail fast or surface clear errors when an invalid instance type is requested, otherwise user experience may degrade.
| "sshKeys": { | ||
| "default": [], | ||
| "description": "List of SSH public keys for authentication. Can be a single key or a list of keys.", | ||
| "items": {}, | ||
| "items": { | ||
| "type": "string" | ||
| }, | ||
| "type": "array" |
There was a problem hiding this comment.
CI failure identical to vm-instance – fix generation drift
sshKeys.items was changed to "type": "string" but the pre-commit hook expects {}. Align the generator or regenerate the schema as noted in the sibling file.
🧰 Tools
🪛 GitHub Actions: Pre-Commit Checks
[error] 123-131: Make generate step failed: The file was modified by the hook, indicating schema changes in the 'sshKeys.items' property from type 'string' to an empty object.
🤖 Prompt for AI Agents
In packages/apps/virtual-machine/values.schema.json around lines 123 to 129, the
sshKeys.items property is set to "type": "string" but the pre-commit hook
expects an empty object {}. To fix this, update sshKeys.items to be an empty
object {} to match the expected schema format or regenerate the schema file
using the same process as the sibling vm-instance file to avoid generation
drift.
| "sshKeys": { | ||
| "default": [], | ||
| "description": "List of SSH public keys for authentication. Can be a single key or a list of keys.", | ||
| "items": {}, | ||
| "items": { | ||
| "type": "string" | ||
| }, | ||
| "type": "array" |
There was a problem hiding this comment.
Pre-commit pipeline is red – generated schema disagrees on sshKeys.items
CI shows Make generate step failed: The file was modified by the hook, meaning the generator reverted "items": {"type": "string"} back to {}.
Either:
- Re-run
make generateto commit the generator’s output, or - Update the generation logic (Makefile/yq step) so that it preserves the explicit string type.
Until this mismatch is fixed the PR cannot merge.
🧰 Tools
🪛 GitHub Actions: Pre-Commit Checks
[error] 131-139: Make generate step failed: The file was modified by the hook, indicating schema changes in the 'sshKeys.items' property from type 'string' to an empty object.
🤖 Prompt for AI Agents
In packages/apps/vm-instance/values.schema.json around lines 131 to 137, the
pre-commit pipeline fails because the generated schema disagrees on the
definition of sshKeys.items, which should explicitly specify "type": "string"
but the generator reverts it to an empty object. To fix this, either re-run the
make generate command to update and commit the generated schema output
reflecting this change, or modify the generation logic in the Makefile or yq
step to ensure it preserves the explicit "type": "string" for sshKeys.items.
This will align the committed schema with the generator output and allow the PR
to pass CI.
Workaround for #1299. If a Cozystack installation provides custom instance types for virtual machines, the static validation rules prevent such instance types from being used, as they are included in the OpenAPI schema of the Cozystack API server and then once more applied in the dependent HelmRelease, offering users no easy way to remedy this in runtime. [virtual-machine] Disable instanceType validation to enable using custom instance types. Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
a6eb7a6 to
44fabd4
Compare
|
Successfully created backport PR for |
What this PR does
Workaround for #1299. If a Cozystack installation provides custom instance types for virtual machines, the static validation rules prevent such instance types from being used, as they are included in the OpenAPI schema of the Cozystack API server and then once more applied in the dependent HelmRelease, offering users no easy way to remedy this in runtime.
Release note
Summary by CodeRabbit