Use cozyvalues-gen with packages/apps/tenant #1314
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. Warning Rate limit exceeded@NickVolynkin has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 2 minutes and 9 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
WalkthroughThis update introduces the Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant Makefile
participant cozyvalues-gen
participant README.md / values.schema.json
Developer->>Makefile: Run `make generate`
Makefile->>cozyvalues-gen: Invoke with values.yaml & schema
cozyvalues-gen->>README.md / values.schema.json: Generate docs & schema
Note right of README.md / values.schema.json: Outputs updated documentation and schema
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ 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 @NickVolynkin, 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!
I've undertaken a significant effort to standardize and enhance the configuration and documentation for various applications within the system. My primary goal was to improve the clarity, validation, and maintainability of application parameters. This involved migrating to a new schema generation tool, enriching the OpenAPI schema definitions with more detailed metadata, and updating the README.md files to provide clearer documentation. These changes collectively aim to make it easier for users and developers to understand and configure the applications.
Highlights
- Migration to
cozyvalues-gen: I've migrated the schema and documentation generation process for all applications fromreadme-generator-for-helmandyqtocozyvalues-gen. This change streamlines the workflow and ensures consistency across the codebase. - Enriched OpenAPI Schema Definitions: I've significantly enhanced the OpenAPI schema definitions across various applications. This includes adding detailed descriptions, specifying data types (e.g.,
int,string,bool,object,array), defining default values, and marking required fields. For resource-related fields (CPU, memory, storage), I've introducedpatternandx-kubernetes-int-or-stringto ensure proper validation and Kubernetes compatibility. - Improved Documentation: The
README.mdfiles for each application have been updated to reflect the improved schema. Parameter tables now include a 'Type' column, providing clearer information about expected input types. Descriptions for complex and nested parameters have been expanded for better clarity. - Standardized Resource Definitions: I've standardized how resource definitions (CPU, memory, storage) are handled across all applications. This ensures a consistent approach to specifying resource requests and limits, improving predictability and manageability.
- Refined Nested Parameter Structures: I've refined the structure of nested parameters, such as user configurations, backup settings, and control plane components. This involves explicitly defining sub-properties and using
additionalPropertieswhere appropriate, leading to more precise and robust schema validation.
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-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-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 introduces a new tool, cozyvalues-gen, to automate the generation of documentation and schemas across multiple packages. This is a significant improvement for consistency and maintainability. While the overall direction is excellent, the new tool appears to have introduced several issues in the generated files. These include incorrect default values in README files, missing properties in JSON schemas which reduces validation capabilities, and some typos and inconsistencies in descriptions. My review focuses on identifying and correcting these issues to ensure the generated files are accurate and complete.
| "properties": { | ||
| "description": "Control Plane Configuration", | ||
| "default": { | ||
| "apiServer": { | ||
| "properties": { | ||
| "resources": { | ||
| "default": {}, | ||
| "description": "Explicit CPU and memory configuration for the API Server. When left empty, the preset defined in `resourcesPreset` is applied.", | ||
| "type": "object" | ||
| }, | ||
| "resourcesPreset": { | ||
| "default": "medium", | ||
| "description": "Default sizing preset used when `resources` is omitted. Allowed values: nano, micro, small, medium, large, xlarge, 2xlarge.", | ||
| "type": "string", | ||
| "enum": [ | ||
| "nano", | ||
| "micro", | ||
| "small", | ||
| "medium", | ||
| "large", | ||
| "xlarge", | ||
| "2xlarge" | ||
| ] | ||
| } | ||
| }, | ||
| "type": "object" | ||
| "resources": {}, | ||
| "resourcesPreset": "medium" | ||
| }, | ||
| "controllerManager": { | ||
| "properties": { | ||
| "resources": { | ||
| "default": {}, | ||
| "description": "Explicit CPU and memory configuration for the Controller Manager. When left empty, the preset defined in `resourcesPreset` is applied.", | ||
| "type": "object" | ||
| }, | ||
| "resourcesPreset": { | ||
| "default": "micro", | ||
| "description": "Default sizing preset used when `resources` is omitted. Allowed values: nano, micro, small, medium, large, xlarge, 2xlarge.", | ||
| "type": "string", | ||
| "enum": [ | ||
| "nano", | ||
| "micro", | ||
| "small", | ||
| "medium", | ||
| "large", | ||
| "xlarge", | ||
| "2xlarge" | ||
| ] | ||
| } | ||
| }, | ||
| "type": "object" | ||
| "resources": {}, | ||
| "resourcesPreset": "micro" | ||
| }, | ||
| "konnectivity": { | ||
| "properties": { | ||
| "server": { | ||
| "properties": { | ||
| "resources": { | ||
| "default": {}, | ||
| "description": "Explicit CPU and memory configuration for Konnectivity. When left empty, the preset defined in `resourcesPreset` is applied.", | ||
| "type": "object" | ||
| }, | ||
| "resourcesPreset": { | ||
| "default": "micro", | ||
| "description": "Default sizing preset used when `resources` is omitted. Allowed values: nano, micro, small, medium, large, xlarge, 2xlarge.", | ||
| "type": "string", | ||
| "enum": [ | ||
| "nano", | ||
| "micro", | ||
| "small", | ||
| "medium", | ||
| "large", | ||
| "xlarge", | ||
| "2xlarge" | ||
| ] | ||
| } | ||
| }, | ||
| "type": "object" | ||
| } | ||
| }, | ||
| "type": "object" | ||
| }, | ||
| "replicas": { | ||
| "default": 2, | ||
| "description": "Number of replicas for Kubernetes control-plane components.", | ||
| "type": "number" | ||
| "server": { | ||
| "resources": {}, | ||
| "resourcesPreset": "micro" | ||
| } | ||
| }, | ||
| "replicas": 2, | ||
| "scheduler": { | ||
| "properties": { | ||
| "resources": {}, | ||
| "resourcesPreset": "micro" | ||
| } | ||
| } |
There was a problem hiding this comment.
The schema definitions for addons and controlPlane are incomplete. They are missing their respective properties which were present in the previous version of the schema. This removes important validation for these configuration sections. Please restore the property definitions for these objects.
| "jetstream": { | ||
| "description": "Jetstream configuration", | ||
| "default": { | ||
| "enabled": true, | ||
| "size": "10Gi" | ||
| } |
There was a problem hiding this comment.
The schema for jetstream is missing its properties (enabled and size). It only has a default block. This removes validation for the jetstream object. Please restore the property definitions.
| "haproxy": { | ||
| "description": "HAProxy configuration", | ||
| "default": { | ||
| "replicas": 2, | ||
| "resources": {}, | ||
| "resourcesPreset": "nano" | ||
| } | ||
| }, | ||
| "nginx": { | ||
| "description": "Nginx configuration", | ||
| "default": { | ||
| "replicas": 2, | ||
| "resources": {}, | ||
| "resourcesPreset": "nano" | ||
| } |
There was a problem hiding this comment.
The schema definitions for haproxy and nginx are incomplete. They are missing their respective properties which were present in the previous version of the schema. This removes important validation for these configuration sections. Please restore the property definitions for haproxy and nginx to ensure values are properly validated.
| "kafka": { | ||
| "description": "Kafka configuration", | ||
| "default": { | ||
| "replicas": 3, | ||
| "resources": {}, | ||
| "resourcesPreset": "small", | ||
| "size": "10Gi", | ||
| "storageClass": "" | ||
| } | ||
| }, | ||
| "topics": { | ||
| "description": "Topics configuration", | ||
| "type": "array", | ||
| "default": [], | ||
| "items": { | ||
| "type": "object", | ||
| "required": [ | ||
| "name", | ||
| "partitions", | ||
| "replicas" | ||
| ], | ||
| "properties": { | ||
| "config": { | ||
| "description": "Topic configuration", | ||
| "type": "object", | ||
| "additionalProperties": { | ||
| "type": "object" | ||
| } | ||
| }, | ||
| "name": { | ||
| "description": "Topic name", | ||
| "type": "string" | ||
| }, | ||
| "partitions": { | ||
| "description": "Number of partitions", | ||
| "type": "integer" | ||
| }, | ||
| "replicas": { | ||
| "description": "Number of replicas", | ||
| "type": "integer" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "title": "Chart Values", | ||
| "type": "object" | ||
| } | ||
| "zookeeper": { | ||
| "description": "Zookeeper configuration", | ||
| "default": { | ||
| "replicas": 3, | ||
| "resources": {}, | ||
| "resourcesPreset": "small", | ||
| "size": "5Gi", | ||
| "storageClass": "" | ||
| } |
There was a problem hiding this comment.
The schema definitions for kafka and zookeeper are incomplete as they are missing their properties. The previous schema defined properties like replicas, resources, etc. which are now gone. This removes validation for these objects. Please restore the property definitions for kafka and zookeeper.
| "default": 2 | ||
| }, | ||
| "resources": { | ||
| "description": "Explicit CPU and memory configuration for each MariaDB replica. When left empty, the preset defined in `resourcesPreset` is applied. replica. When left empty, the preset defined in `resourcesPreset` is applied.", |
There was a problem hiding this comment.
The description for resources has duplicated text. Please remove the repeated sentence.
| | Name | Description | Type | Value | | ||
| | ------------------------- | ----------------------------------------------------------------------------------------------------------- | ---------- | ------------ | | ||
| | `external` | Enable external access from outside the cluster | `bool` | `false` | | ||
| | `externalMethod` | Specify method to pass through the traffic to the virtual machine. Allowed values: `WholeIP` and `PortList` | `string` | `{}` | |
There was a problem hiding this comment.
The default value for externalMethod is shown as {}, but it should be "PortList" as specified in values.yaml.
packages/apps/vm-instance/README.md
Outdated
| | Name | Description | Type | Value | | ||
| | ------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------- | ----------- | | ||
| | `external` | Enable external access from outside the cluster | `bool` | `false` | | ||
| | `externalMethod` | Specify method to pass through the traffic to the virtual machine. Allowed values: `WholeIP` and `PortList` | `string` | `{}` | |
There was a problem hiding this comment.
The default value for externalMethod is shown as {}, but it should be "PortList" as specified in values.yaml.
packages/apps/vm-instance/README.md
Outdated
| | `resources.memory` | The amount of memory allocated to the virtual machine | `*string` | `""` | | ||
| | `resources.sockets` | The number of CPU sockets allocated to the virtual machine (used to define vCPU topology) | `*string` | `""` | | ||
| | `sshKeys` | List of SSH public keys for authentication. Can be a single key or a list of keys. | `[]string` | `[]` | | ||
| | `cloudInit` | cloud-init user data config. See cloud-init documentation for more details: [format](https://cloudinit.readthedocs.io/en/latest/explanation/format.html), [examples](https://cloudinit.readthedocs.io/en/latest/explanation/format.html). | `object` | `{}` | |
There was a problem hiding this comment.
The link to cloud-init examples seems to be incorrect. It points to format.html instead of examples.html.
| | `cloudInit` | cloud-init user data config. See cloud-init documentation for more details: [format](https://cloudinit.readthedocs.io/en/latest/explanation/format.html), [examples](https://cloudinit.readthedocs.io/en/latest/explanation/format.html). | `object` | `{}` | | |
| | `cloudInit` | cloud-init user data config. See cloud-init documentation for more details: [format](https://cloudinit.readthedocs.io/en/latest/explanation/format.html), [examples](https://cloudinit.readthedocs.io/en/latest/reference/examples.html). | `object` | `{}` | |
| "default": "", | ||
| "description": "cloud-init user data config. See cloud-init documentation for more details.", | ||
| "type": "string" | ||
| "description": "cloud-init user data config. See cloud-init documentation for more details: [format](https://cloudinit.readthedocs.io/en/latest/explanation/format.html), [examples](https://cloudinit.readthedocs.io/en/latest/explanation/format.html)." |
There was a problem hiding this comment.
The link to cloud-init examples in the description is incorrect. It points to format.html twice.
"description": "cloud-init user data config. See cloud-init documentation for more details: [format](https://cloudinit.readthedocs.io/en/latest/explanation/format.html), [examples](https://cloudinit.readthedocs.io/en/latest/reference/examples.html)."
packages/apps/vpn/README.md
Outdated
| | `resources` | Explicit CPU and memory configuration for each VPN server replica. When left empty, the preset defined in `resourcesPreset` is applied. | `*object` | `{}` | | ||
| | `resources.cpu` | CPU | `*string` | `null` | | ||
| | `resources.memory` | Memory | `*string` | `null` | | ||
| | `resourcesPreset` | Default sizing preset used when `resources` is omitted. Allowed values: `nano`, `micro`, `small`, `medium`, `large`, `xlarge`, `2xlarge`. | `string` | `{}` | |
There was a problem hiding this comment.
The default value for resourcesPreset is shown as {}, but it should be "nano" as specified in values.yaml.
c53ff4c to
260a87d
Compare
260a87d to
bbda721
Compare
packages/apps/tenant/values.yaml
Outdated
| ## @param resourceQuotas {resourceQuotas} Define resource quotas for the tenant | ||
| ## @field resourceQuotas.cpu {quantity} CPU quota | ||
| ## @field resourceQuotas.memory {quantity} Memory (RAM) quota | ||
| ## TODO: unsupported syntax @field resourceQuotas.nvidia.com/gpu {quantity} GPU quota |
There was a problem hiding this comment.
Can be a problem, needs testing.
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (2)
packages/apps/virtual-machine/README.md (1)
42-44: Default value is wrong – shows{}instead of"PortList"
externalMethodis documented with default{}, butvalues.yamlsets"PortList". Keep docs and defaults in sync or users will pick the wrong setting when copy-pasting.-| `externalMethod` | Specify method to pass through the traffic to the virtual machine. Allowed values: `WholeIP` and `PortList` | `string` | `{}` | +| `externalMethod` | Specify method to pass through the traffic to the virtual machine. Allowed values: `WholeIP` and `PortList` | `string` | `"PortList"` |packages/apps/postgres/README.md (1)
75-76: Default value mismatch –resourcesPresetTable shows default
{}butvalues.yamlsets"micro".
Update the Value column so docs don’t mislead operators.-| `resourcesPreset` | Default sizing preset used when `resources` is omitted. Allowed values: `nano`, `micro`, `small`, `medium`, `large`, `xlarge`, `2xlarge`. | `string` | `{}` | +| `resourcesPreset` | Default sizing preset used when `resources` is omitted. Allowed values: `nano`, `micro`, `small`, `medium`, `large`, `xlarge`, `2xlarge`. | `string` | `"micro"` |
🧹 Nitpick comments (9)
packages/apps/tenant/values.yaml (1)
3-8: Minor grammar fix.
it's parent tenant host→its parent tenant host.packages/extra/monitoring/values.yaml (2)
8-26: Array item field names should be singular.Per earlier guidance, use singular form when documenting fields of array items (
metricsStorage.*notmetricsStorages.*). Otherwise the generator cannot match paths and will drop type hints.Replace every
metricsStorages→metricsStorage(and similar forlogsStorages).
18-26: Inconsistent casing:requestvsrequests,limitvslimits.Kubernetes uses
requests/limits; mixing singular and plural will confuse readers and tooling.packages/apps/postgres/values.yaml (1)
12-13: Enum list misses whitespace after commaFor readability and consistency with other charts, add a space after each comma in the
enumlist ("nano,micro,small..." → "nano, micro, small, ...") so that downstream generators don’t render a single very-long word.packages/apps/virtual-machine/README.md (1)
57-58: Minor wording tweak – missing subject“Can be a single key or a list of keys.” lacks a subject.
Replace with “It can be a single key or a list of keys.” to silence markdown-lintMISSING_IT_THERE.packages/extra/monitoring/README.md (1)
72-74: Bare URL violates MD034Wrap the Telegram helper-bot URL in angle brackets to satisfy markdown-lint.
-| `alerta.alerts.telegram.chatID` | Specify multiple ID's separated by comma. Get yours in https://t.me/chatid_echo_bot | `string` | `""` | +| `alerta.alerts.telegram.chatID` | Specify multiple ID's separated by comma. Get yours in <https://t.me/chatid_echo_bot> | `string` | `""` |packages/apps/virtual-machine/values.yaml (1)
6-8: Enum order inconsistent with READMEREADME lists the allowed enum values as
WholeIPthenPortList, but here the order isPortList,WholeIP.
Not a blocker, yet aligning the order avoids unnecessary diffs inside generated schemas.packages/apps/postgres/values.schema.json (1)
195-203: Negative resource quantities are allowed by the regex.The same quantity pattern used elsewhere permits an optional leading
-.
Kubernetes rejects negative quantities; drop the sign from the pattern to fail fast in CI rather than at deploy time.packages/extra/monitoring/values.schema.json (1)
217-224: InconsistentretentionPeriodunits inlogsStoragesdefaults.
metricsStoragesdefaults use suffixed values ("3d","14d"), whilelogsStoragesuses plain"1".
This inconsistency will confuse users and tooling that expects a unit suffix.- "retentionPeriod": "1", + "retentionPeriod": "1d",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
.github/workflows/pre-commit.yml(1 hunks)hack/e2e-apps/virtualmachine.bats(1 hunks)packages/apps/postgres/Makefile(1 hunks)packages/apps/postgres/README.md(1 hunks)packages/apps/postgres/values.schema.json(1 hunks)packages/apps/postgres/values.yaml(3 hunks)packages/apps/tenant/Makefile(1 hunks)packages/apps/tenant/README.md(1 hunks)packages/apps/tenant/values.schema.json(1 hunks)packages/apps/tenant/values.yaml(1 hunks)packages/apps/virtual-machine/Makefile(1 hunks)packages/apps/virtual-machine/README.md(1 hunks)packages/apps/virtual-machine/values.schema.json(3 hunks)packages/apps/virtual-machine/values.yaml(2 hunks)packages/extra/monitoring/Makefile(1 hunks)packages/extra/monitoring/README.md(1 hunks)packages/extra/monitoring/values.schema.json(1 hunks)packages/extra/monitoring/values.yaml(4 hunks)
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: NickVolynkin
PR: cozystack/cozystack#1120
File: packages/apps/ferretdb/README.md:35-37
Timestamp: 2025-07-02T09:58:11.406Z
Learning: In the cozystack repository, the maintainer NickVolynkin prefers to keep realistic-looking example credentials in README documentation rather than using generic placeholders like <ACCESS_KEY>, even though they are just examples and not real secrets.
Learnt from: NickVolynkin
PR: cozystack/cozystack#1216
File: packages/extra/monitoring/README.md:30-31
Timestamp: 2025-07-26T18:26:01.447Z
Learning: The cozyvalues-gen tool has a known issue (https://github.com/cozystack/cozyvalues-gen/issues/10) where it incorrectly places Grafana configuration entries under the wrong documentation sections, specifically placing grafana.resources.requests.* entries in the Metrics storage configuration table instead of the Grafana configuration table.
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: NickVolynkin
PR: cozystack/cozystack#1216
File: packages/apps/postgres/values.yaml:55-60
Timestamp: 2025-07-26T17:54:57.273Z
Learning: In the cozystack repository, the cozyvalues-gen tool does not support `typedef` syntax for defining custom type aliases in documentation comments.
Learnt from: NickVolynkin
PR: cozystack/cozystack#1120
File: packages/apps/clickhouse/README.md:60-67
Timestamp: 2025-07-03T05:54:51.264Z
Learning: The `cozy-lib.resources.sanitize` function in packages/library/cozy-lib/templates/_resources.tpl supports both standard Kubernetes resource format (with limits:/requests: sections) and flat format (direct resource specifications). The flat format takes priority over nested values. CozyStack apps include cozy-lib as a chart dependency through symlinks in packages/apps/*/charts/cozy-lib directories.
📚 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/extra/monitoring/Makefilepackages/apps/postgres/Makefilepackages/apps/tenant/Makefilepackages/apps/virtual-machine/Makefile.github/workflows/pre-commit.yml
📚 Learning: the cozyvalues-gen tool has a known issue (https://github.com/cozystack/cozyvalues-gen/issues/10) wh...
Learnt from: NickVolynkin
PR: cozystack/cozystack#1216
File: packages/extra/monitoring/README.md:30-31
Timestamp: 2025-07-26T18:26:01.447Z
Learning: The cozyvalues-gen tool has a known issue (https://github.com/cozystack/cozyvalues-gen/issues/10) where it incorrectly places Grafana configuration entries under the wrong documentation sections, specifically placing grafana.resources.requests.* entries in the Metrics storage configuration table instead of the Grafana configuration table.
Applied to files:
packages/extra/monitoring/Makefilepackages/apps/tenant/Makefilepackages/apps/virtual-machine/Makefilepackages/extra/monitoring/README.mdpackages/extra/monitoring/values.yamlpackages/apps/postgres/README.mdpackages/extra/monitoring/values.schema.json
📚 Learning: the `cozy-lib.resources.sanitize` function in packages/library/cozy-lib/templates/_resources.tpl sup...
Learnt from: NickVolynkin
PR: cozystack/cozystack#1120
File: packages/apps/clickhouse/README.md:60-67
Timestamp: 2025-07-03T05:54:51.264Z
Learning: The `cozy-lib.resources.sanitize` function in packages/library/cozy-lib/templates/_resources.tpl supports both standard Kubernetes resource format (with limits:/requests: sections) and flat format (direct resource specifications). The flat format takes priority over nested values. CozyStack apps include cozy-lib as a chart dependency through symlinks in packages/apps/*/charts/cozy-lib directories.
Applied to files:
packages/apps/postgres/Makefilepackages/apps/tenant/values.schema.jsonpackages/extra/monitoring/values.schema.json
📚 Learning: the cozystack project uses yq v4+ on their ci runner, so yq v4 syntax (-o json --indent 4) is compat...
Learnt from: NickVolynkin
PR: cozystack/cozystack#1117
File: packages/apps/mysql/Makefile:8-8
Timestamp: 2025-06-26T04:29:24.830Z
Learning: The cozystack project uses yq v4+ on their CI runner, so yq v4 syntax (-o json --indent 4) is compatible and version checks are not needed.
Applied to files:
packages/apps/postgres/Makefile
📚 Learning: in the cozystack repository, the cozyvalues-gen tool does not support `@typedef` syntax for defining...
Learnt from: NickVolynkin
PR: cozystack/cozystack#1216
File: packages/apps/postgres/values.yaml:55-60
Timestamp: 2025-07-26T17:54:57.273Z
Learning: In the cozystack repository, the cozyvalues-gen tool does not support `typedef` syntax for defining custom type aliases in documentation comments.
Applied to files:
packages/apps/tenant/Makefile
📚 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/Makefilepackages/apps/tenant/values.yamlpackages/apps/virtual-machine/values.yamlpackages/apps/virtual-machine/README.mdpackages/apps/virtual-machine/values.schema.jsonpackages/apps/postgres/README.md
📚 Learning: in cozystack's schema generator annotation format, when documenting fields of array items, use the s...
Learnt from: NickVolynkin
PR: cozystack/cozystack#1216
File: packages/apps/virtual-machine/values.yaml:31-33
Timestamp: 2025-07-26T18:01:52.557Z
Learning: In cozystack's schema generator annotation format, when documenting fields of array items, use the singular form of the item type rather than array notation. For example, for a parameter `gpus {[]gpu}`, use `field gpu.name` rather than `field gpus[].name` to refer to the name field of each GPU object in the array.
Applied to files:
packages/apps/tenant/values.yamlpackages/apps/postgres/values.yaml
📚 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/tenant/values.schema.json
🪛 markdownlint-cli2 (0.17.2)
packages/extra/monitoring/README.md
73-73: Bare URL used
(MD034, no-bare-urls)
🪛 LanguageTool
packages/apps/virtual-machine/README.md
[style] ~57-~57: To form a complete sentence, be sure to include a subject or ‘there’.
Context: ... of SSH public keys for authentication. Can be a single key or a list of keys. ...
(MISSING_IT_THERE)
⏰ 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 (10)
.github/workflows/pre-commit.yml (1)
32-32: LGTM! Proper tooling setup for cozyvalues-gen migration.The addition of cozyvalues-gen installation alongside the existing readme-generator-for-helm tool properly supports the migration across the repository's Makefiles.
packages/apps/tenant/Makefile (1)
4-4: LGTM! Clean migration to cozyvalues-gen.The migration from readme-generator-for-helm to cozyvalues-gen simplifies the generate target while maintaining the same input and output files. This aligns with the broader tooling migration across the repository.
hack/e2e-apps/virtualmachine.bats (1)
23-23: LGTM! Test updated to match schema changes.The change from empty string values to an empty object for resources aligns with the updated virtual machine schema requirements, maintaining consistency between test inputs and the refined resource specification format.
packages/extra/monitoring/Makefile (1)
9-9: LGTM! Simplified generation process with cozyvalues-gen.The migration from the multi-step readme-generator process to cozyvalues-gen simplifies the generate target and should resolve enum handling issues. Note that there's a known issue with cozyvalues-gen incorrectly placing Grafana configuration entries in wrong documentation sections (cozystack/cozyvalues-gen#10).
packages/apps/tenant/README.md (1)
72-81: LGTM! Enhanced documentation with explicit type information.The addition of the "Type" column to the parameters table significantly improves documentation clarity by providing explicit data type information for each parameter. The type annotations are accurate and the description refinements enhance user understanding.
packages/apps/postgres/Makefile (1)
4-4: Ignore outdated enum-injection concernThe
resourcesPreset.enumis already defined inpackages/apps/postgres/values.schema.json:
.properties.resourcesPreset.enumcontains
["nano","micro","small","medium","large","xlarge","2xlarge"]No additional yq step is required here.
Likely an incorrect or invalid review comment.
packages/apps/tenant/values.yaml (1)
15-16: Undeclared custom typeresourceQuotas.The
{resourceQuotas}alias is referenced but not defined anywhere in the file. cozyvalues-gen will treat it as an opaque type and drop the link.Add a definition block like:
## @typedef resourceQuotas ## @param cpu {*string} Max vCPU ## @param memory {*string} Max RAM ## @param nvidia.com/gpu {*int} Max GPUs ## @param storage {*string} Max PVC sizebefore the first
@section.⛔ Skipped due to learnings
Learnt from: NickVolynkin PR: cozystack/cozystack#1216 File: packages/apps/postgres/values.yaml:55-60 Timestamp: 2025-07-26T17:54:57.273Z Learning: In the cozystack repository, the cozyvalues-gen tool does not support `typedef` syntax for defining custom type aliases in documentation comments.Learnt from: NickVolynkin PR: cozystack/cozystack#1216 File: packages/extra/monitoring/README.md:30-31 Timestamp: 2025-07-26T18:26:01.447Z Learning: The cozyvalues-gen tool has a known issue (https://github.com/cozystack/cozyvalues-gen/issues/10) where it incorrectly places Grafana configuration entries under the wrong documentation sections, specifically placing grafana.resources.requests.* entries in the Metrics storage configuration table instead of the Grafana configuration table.Learnt from: NickVolynkin PR: cozystack/cozystack#1120 File: packages/apps/clickhouse/README.md:60-67 Timestamp: 2025-07-03T05:54:51.264Z Learning: The `cozy-lib.resources.sanitize` function in packages/library/cozy-lib/templates/_resources.tpl supports both standard Kubernetes resource format (with limits:/requests: sections) and flat format (direct resource specifications). The flat format takes priority over nested values. CozyStack apps include cozy-lib as a chart dependency through symlinks in packages/apps/*/charts/cozy-lib directories.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#1216 File: packages/apps/virtual-machine/values.yaml:31-33 Timestamp: 2025-07-26T18:01:52.557Z Learning: In cozystack's schema generator annotation format, when documenting fields of array items, use the singular form of the item type rather than array notation. For example, for a parameter `gpus {[]gpu}`, use `field gpu.name` rather than `field gpus[].name` to refer to the name field of each GPU object in the array.packages/apps/virtual-machine/README.md (1)
50-56: Pointer type but non-null default
systemDisk.storageClassis typed as*string(nullable), yet the default is"replicated".
Either drop the star or change the default tonullto avoid confusing consumers that auto-convert*Tto optional.packages/apps/postgres/README.md (1)
111-112: Placeholder secrets differ from values.yamlREADME still shows
<access key>/<secret key>placeholders whilevalues.yamlcontains concrete dummy credentials.
Pick one approach to avoid confusion; previous learnings indicate maintainers prefer realistic-looking examples.packages/extra/monitoring/values.schema.json (1)
146-158: Double-check README table placement for Grafana resources.
cozyvalues-genissue #10 sometimes placesgrafana.resources.requests.*under Metrics storage docs instead of the Grafana section. After regenerating the README, verify that these entries appear in the correct table.
| "s3AccessKey": { | ||
| "description": "Access key for S3, used for authentication", | ||
| "type": "string", | ||
| "default": "\u003caccess key\u003e" | ||
| }, | ||
| "quorum": { | ||
| "properties": { | ||
| "maxSyncReplicas": { | ||
| "default": 0, | ||
| "description": "Maximum number of synchronous replicas that can acknowledge a transaction (must be lower than the number of instances).", | ||
| "type": "number" | ||
| }, | ||
| "minSyncReplicas": { | ||
| "default": 0, | ||
| "description": "Minimum number of synchronous replicas that must acknowledge a transaction before it is considered committed.", | ||
| "type": "number" | ||
| } | ||
| }, | ||
| "type": "object" | ||
| "s3SecretKey": { | ||
| "description": "Secret key for S3, used for authentication", | ||
| "type": "string", | ||
| "default": "\u003csecret key\u003e" | ||
| }, |
There was a problem hiding this comment.
Avoid embedding secret-like defaults in the schema.
Hard-coding "<access key>" / "<secret key>" as defaults makes Helm output them when users run helm show values, which encourages committing placeholder secrets back to Git. Use empty strings or omit defaults entirely.
- "default": "<access key>"
+ "default": ""
…
- "default": "<secret key>"
+ "default": ""📝 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.
| "s3AccessKey": { | |
| "description": "Access key for S3, used for authentication", | |
| "type": "string", | |
| "default": "\u003caccess key\u003e" | |
| }, | |
| "quorum": { | |
| "properties": { | |
| "maxSyncReplicas": { | |
| "default": 0, | |
| "description": "Maximum number of synchronous replicas that can acknowledge a transaction (must be lower than the number of instances).", | |
| "type": "number" | |
| }, | |
| "minSyncReplicas": { | |
| "default": 0, | |
| "description": "Minimum number of synchronous replicas that must acknowledge a transaction before it is considered committed.", | |
| "type": "number" | |
| } | |
| }, | |
| "type": "object" | |
| "s3SecretKey": { | |
| "description": "Secret key for S3, used for authentication", | |
| "type": "string", | |
| "default": "\u003csecret key\u003e" | |
| }, | |
| "s3AccessKey": { | |
| "description": "Access key for S3, used for authentication", | |
| "type": "string", | |
| "default": "" | |
| }, | |
| "s3SecretKey": { | |
| "description": "Secret key for S3, used for authentication", | |
| "type": "string", | |
| "default": "" | |
| }, |
🤖 Prompt for AI Agents
In packages/apps/postgres/values.schema.json around lines 47 to 56, remove the
default values for "s3AccessKey" and "s3SecretKey" or set them to empty strings
instead of placeholder secrets. This prevents Helm from displaying fake secret
values in `helm show values` output and discourages committing placeholder
secrets to Git.
| "required": [ | ||
| "enabled", | ||
| "oldName" | ||
| ], | ||
| "properties": { | ||
| "enabled": { | ||
| "description": "Restore database cluster from a backup", | ||
| "type": "boolean", | ||
| "default": false | ||
| }, | ||
| "resources": { | ||
| "default": {}, | ||
| "description": "Explicit CPU and memory configuration for each PostgreSQL replica. When left empty, the preset defined in `resourcesPreset` is applied.", | ||
| "type": "object" | ||
| "oldName": { | ||
| "description": "Name of database cluster before deleting", | ||
| "type": "string" | ||
| }, | ||
| "resourcesPreset": { | ||
| "default": "micro", | ||
| "description": "Default sizing preset used when `resources` is omitted. Allowed values: nano, micro, small, medium, large, xlarge, 2xlarge.", | ||
| "type": "string", | ||
| "enum": [ | ||
| "nano", | ||
| "micro", | ||
| "small", | ||
| "medium", | ||
| "large", | ||
| "xlarge", | ||
| "2xlarge" | ||
| ] | ||
| "recoveryTime": { | ||
| "description": "Timestamp (PITR) up to which recovery will proceed, expressed in RFC 3339 format. If left empty, will restore latest", | ||
| "type": "string" | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
bootstrap.oldName marked required even when bootstrap is disabled.
With "enabled": false the "oldName" field is meaningless, yet the schema requires it, forcing users to supply a dummy value. Consider conditional validation (JSON-Schema if/then) or make oldName optional.
🤖 Prompt for AI Agents
In packages/apps/postgres/values.schema.json around lines 72 to 90, the
"oldName" property is marked as required even when "enabled" is false, which
forces users to provide an unnecessary value. Modify the schema to use JSON
Schema conditional validation with "if" checking if "enabled" is true, then
"oldName" is required; otherwise, "oldName" should be optional. This will ensure
"oldName" is only required when "enabled" is true.
| @@ -1,12 +1,9 @@ | |||
| include ../../../scripts/package.mk | |||
|
|
|||
| generate: | |||
| readme-generator-for-helm -v values.yaml -s values.schema.json -r README.md | |||
| cozyvalues-gen -v values.yaml -s values.schema.json -r README.md | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Enum-reinjection removed – confirm cozyvalues-gen preserves them.
externalPorts, systemDisk.image, and externalMethod enum injections were dropped when switching to cozyvalues-gen. If the new generator does not natively keep those enum values, the validation constraints will silently disappear.
Run:
🏁 Script executed:
#!/bin/bash
# Expect to see "enum" arrays present for the three properties.
jq '.properties | {externalPorts, systemDisk, externalMethod}' values.schema.jsonLength of output: 160
🏁 Script executed:
#!/bin/bash
# Locate the values.schema.json file in the repo
schema_file=$(fd values.schema.json | head -n1)
if [ -z "$schema_file" ]; then
echo "No values.schema.json file found in the repository."
exit 1
fi
echo "Using schema at: $schema_file"
# Check enums for the three properties
echo "externalPorts enum:"
jq '.properties.externalPorts.enum' "$schema_file" || echo " (not present)"
echo "systemDisk.image enum:"
jq '.properties.systemDisk.properties.image.enum' "$schema_file" || echo " (not present)"
echo "externalMethod enum:"
jq '.properties.externalMethod.enum' "$schema_file" || echo " (not present)"Length of output: 679
Re-inject missing enums after cozyvalues-gen
The cozyvalues-gen run is stripping out the enum arrays for externalPorts, systemDisk.image, and externalMethod, so your schema loses those validation constraints. You’ll need to merge them back into values.schema.json after cozyvalues-gen runs.
• File: packages/apps/virtual-machine/Makefile
After the cozyvalues-gen invocation, add yq commands to restore each enum. For example:
cozyvalues-gen -v values.yaml -s values.schema.json -r README.md
+ # Re-inject enum values for validation
+ yq eval '.properties.externalPorts.enum = ["<port1>","<port2>",…]' -i values.schema.json
+ yq eval '.properties.systemDisk.properties.image.enum = ["<imageA>","<imageB>",…]' -i values.schema.json
+ yq eval '.properties.externalMethod.enum = ["<methodX>","<methodY>",…]' -i values.schema.jsonReplace the placeholder lists with your actual enum values.
📝 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.
| cozyvalues-gen -v values.yaml -s values.schema.json -r README.md | |
| cozyvalues-gen -v values.yaml -s values.schema.json -r README.md | |
| # Re-inject enum values for validation | |
| yq eval '.properties.externalPorts.enum = ["<port1>","<port2>",…]' -i values.schema.json | |
| yq eval '.properties.systemDisk.properties.image.enum = ["<imageA>","<imageB>",…]' -i values.schema.json | |
| yq eval '.properties.externalMethod.enum = ["<methodX>","<methodY>",…]' -i values.schema.json |
🤖 Prompt for AI Agents
In packages/apps/virtual-machine/Makefile at line 4, after the cozyvalues-gen
command, add yq commands to re-inject the missing enum arrays for externalPorts,
systemDisk.image, and externalMethod back into values.schema.json. This will
restore the validation constraints stripped out by cozyvalues-gen. Use yq to
merge the correct enum arrays into the appropriate schema paths, replacing
placeholder lists with the actual enum values.
| PREFERENCES=$$(yq e '.metadata.name' -o=json -r ../../system/kubevirt-instancetypes/templates/preferences.yaml | yq 'split(" ") | . + [""]' -o json) \ | ||
| && yq -i -o json ".properties.instanceProfile.enum = $${PREFERENCES}" values.schema.json |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Shell word-splitting may corrupt the enum array.
PREFERENCES=$( … | yq 'split(" ")' … ) assumes space-separated names, but yq e '.metadata.name' emits newline-separated output. Unquoted expansion inside yq -i can also break when names contain spaces or glob chars.
-PREFERENCES=$$(yq e '.metadata.name' -o=json -r ../../system/kubevirt-instancetypes/templates/preferences.yaml | yq 'split(" ") | . + [""]' -o json) \
- && yq -i -o json ".properties.instanceProfile.enum = $${PREFERENCES}" values.schema.json
+PREFERENCES_JSON=$$(yq e -o=json '.[].metadata.name' ../../system/kubevirt-instancetypes/templates/preferences.yaml \
+ | jq -s 'add | split("\n") - [""]') \
+ && yq -i -o json ".properties.instanceProfile.enum = $${PREFERENCES_JSON}" values.schema.jsonCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/apps/virtual-machine/Makefile around lines 8 to 9, the shell command
assigns PREFERENCES by splitting on spaces, but the output from yq is
newline-separated, causing incorrect splitting and potential corruption of the
enum array. To fix this, modify the command to correctly handle
newline-separated names without word-splitting issues, and ensure that the
variable expansion inside the yq command is properly quoted or handled to
prevent breaking when names contain spaces or special characters.
| "externalPorts": { | ||
| "default": [], | ||
| "description": "Specify ports to forward from outside the cluster", | ||
| "type": "array", | ||
| "default": [ | ||
| 22 | ||
| ], | ||
| "items": { | ||
| "type": "integer" | ||
| }, | ||
| "type": "array" | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add port-range validation for externalPorts.
The items are typed as integer, but there is no schema‐level validation to ensure they fall within the legal TCP/UDP port range (1-65535). A malformed value will surface only at deploy time.
"items": {
- "type": "integer"
+ "type": "integer",
+ "minimum": 1,
+ "maximum": 65535
}📝 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.
| "externalPorts": { | |
| "default": [], | |
| "description": "Specify ports to forward from outside the cluster", | |
| "type": "array", | |
| "default": [ | |
| 22 | |
| ], | |
| "items": { | |
| "type": "integer" | |
| }, | |
| "type": "array" | |
| } | |
| "externalPorts": { | |
| "description": "Specify ports to forward from outside the cluster", | |
| "type": "array", | |
| "default": [ | |
| 22 | |
| ], | |
| "items": { | |
| "type": "integer", | |
| "minimum": 1, | |
| "maximum": 65535 | |
| } | |
| } |
🤖 Prompt for AI Agents
In packages/apps/virtual-machine/values.schema.json around lines 27 to 35, the
externalPorts array items are typed as integers but lack validation to ensure
values fall within the valid TCP/UDP port range of 1 to 65535. Add a "minimum"
property with value 1 and a "maximum" property with value 65535 inside the
"items" schema to enforce this range validation at the schema level.
| "sockets": { | ||
| "default": "", | ||
| "description": "The number of CPU sockets allocated to the virtual machine (used to define vCPU topology)", | ||
| "type": "string" | ||
| "type": "string", | ||
| "pattern": "^(\\+|-)?(([0-9]+(\\.[0-9]*)?)|(\\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\\+|-)?(([0-9]+(\\.[0-9]*)?)|(\\.[0-9]+))))?$", | ||
| "x-kubernetes-int-or-string": true | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Over-permissive regex for resources.sockets.
resources.sockets ultimately represents vCPU sockets and is intentionally a string (see team note), but unlike CPU / memory it must be a plain positive integer.
The current quantity regex allows decimals and unit suffixes (Mi, Gi, m, …) which makes invalid values (e.g. "2Gi", "0.5") appear schema-compliant.
- "type": "string",
- "pattern": "^(\\+|-)?(([0-9]+(\\.[0-9]*)?)|(\\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\\+|-)?(([0-9]+(\\.[0-9]*)?)|(\\.[0-9]+))))?$",
- "x-kubernetes-int-or-string": true
+ "type": "string",
+ "pattern": "^[1-9][0-9]*$"
/* keep x-kubernetes-int-or-string if really needed */📝 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.
| "sockets": { | |
| "default": "", | |
| "description": "The number of CPU sockets allocated to the virtual machine (used to define vCPU topology)", | |
| "type": "string" | |
| "type": "string", | |
| "pattern": "^(\\+|-)?(([0-9]+(\\.[0-9]*)?)|(\\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\\+|-)?(([0-9]+(\\.[0-9]*)?)|(\\.[0-9]+))))?$", | |
| "x-kubernetes-int-or-string": true | |
| } | |
| "sockets": { | |
| "description": "The number of CPU sockets allocated to the virtual machine (used to define vCPU topology)", | |
| "type": "string", | |
| "pattern": "^[1-9][0-9]*$" | |
| } |
🤖 Prompt for AI Agents
In packages/apps/virtual-machine/values.schema.json around lines 126 to 131, the
regex pattern for the "sockets" field is too permissive, allowing decimals and
unit suffixes which are invalid for vCPU sockets. Update the regex to only allow
plain positive integers without any decimal points or unit suffixes, ensuring
the value strictly represents a positive integer as required.
53580c1 to
4f1154a
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
packages/apps/virtual-machine/README.md (1)
42-42: Default value forexternalMethodcontradicts its declared type and allowed values
externalMethodis documented as astringwith allowed valuesWholeIP/PortList, yet its default is rendered as{}(object). This is the same issue flagged in the previous review and remains unresolved.-| `externalMethod` | Specify method to pass through the traffic to the virtual machine. Allowed values: `WholeIP` and `PortList` | `string` | `{}` | +| `externalMethod` | Specify method to pass through the traffic to the virtual machine. Allowed values: `WholeIP` and `PortList` | `string` | `PortList` |packages/extra/monitoring/values.schema.json (1)
26-33:storageClassNamestill marked required but default is an empty string
This duplicates the previously-raised issue: validation succeeds while producing a non-functional value.Either drop it from
"required"or provide a real cluster-wide default.packages/apps/postgres/values.schema.json (2)
13-15: Remove placeholder secrets from defaults
"<access key>"/"<secret key>"are still shipped as default values. Helm will surface them inhelm show values, encouraging users to commit fake secrets. Set them to an empty string or drop the defaults entirely.- "s3AccessKey": "<access key>", - "s3SecretKey": "<secret key>", + "s3AccessKey": "", + "s3SecretKey": "",Also applies to: 47-55
72-75:bootstrap.oldNameshould not be required when bootstrap is disabledThe same unconditional-required issue exists here. Make
oldNamemandatory only whenbootstrap.enabledistrueusing anif/thenclause or drop it fromrequired.
🧹 Nitpick comments (4)
packages/apps/virtual-machine/README.md (2)
50-56: Clarify the*stringnotation and its interaction with defaultsUsing Go-style pointer notation (
*string) in a README aimed at Helm/YAML users can confuse readers, especially when defaults are non-null (replicated) or the literal"null"string. Consider either:
- Dropping the pointer asterisk and documenting optionality in the description, or
- Keeping the asterisk but setting the defaults to
nullto reflect “unset”.
57-57: Minor wording nit — add a subject“Can be a single key or a list of keys.” lacks a subject; prepend “This field” for a complete sentence.
packages/extra/monitoring/values.schema.json (1)
345-388: Inconsistent resource field names (minAllowed/maxAllowed)Everywhere else resources follow Kubernetes conventions (
requests/limits).
Using different names for the VictoriaMetrics sub-components increases cognitive load and breaks tooling that expects the standard keys.Recommend renaming to
limits/requests(or introducing a backward-compatible alias) for consistency.packages/apps/postgres/values.schema.json (1)
199-210: Validate resource quantities with Kubernetes-friendly format but avoid non-standard keywords
x-kubernetes-int-or-stringis a Kubernetes OpenAPI extension and will be ignored by the Helm Docs/VSCode plugins that rely on pure JSON-Schema draft-07. Consider moving the regex validation into thepatternonly and dropping the non-standard keyword to keep tooling happy.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
.github/workflows/pre-commit.yml(1 hunks)hack/e2e-apps/virtualmachine.bats(1 hunks)packages/apps/postgres/Makefile(1 hunks)packages/apps/postgres/README.md(1 hunks)packages/apps/postgres/values.schema.json(1 hunks)packages/apps/postgres/values.yaml(3 hunks)packages/apps/tenant/Makefile(1 hunks)packages/apps/tenant/README.md(1 hunks)packages/apps/tenant/values.schema.json(1 hunks)packages/apps/tenant/values.yaml(1 hunks)packages/apps/virtual-machine/Makefile(1 hunks)packages/apps/virtual-machine/README.md(1 hunks)packages/apps/virtual-machine/values.schema.json(3 hunks)packages/apps/virtual-machine/values.yaml(2 hunks)packages/extra/monitoring/Makefile(1 hunks)packages/extra/monitoring/README.md(1 hunks)packages/extra/monitoring/values.schema.json(1 hunks)packages/extra/monitoring/values.yaml(3 hunks)
✅ Files skipped from review due to trivial changes (5)
- packages/apps/tenant/Makefile
- packages/apps/tenant/README.md
- packages/extra/monitoring/values.yaml
- packages/apps/postgres/README.md
- packages/extra/monitoring/Makefile
🚧 Files skipped from review as they are similar to previous changes (10)
- hack/e2e-apps/virtualmachine.bats
- packages/apps/tenant/values.yaml
- packages/apps/postgres/Makefile
- .github/workflows/pre-commit.yml
- packages/apps/virtual-machine/values.yaml
- packages/apps/tenant/values.schema.json
- packages/apps/postgres/values.yaml
- packages/extra/monitoring/README.md
- packages/apps/virtual-machine/Makefile
- packages/apps/virtual-machine/values.schema.json
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: NickVolynkin
PR: cozystack/cozystack#1120
File: packages/apps/ferretdb/README.md:35-37
Timestamp: 2025-07-02T09:58:11.406Z
Learning: In the cozystack repository, the maintainer NickVolynkin prefers to keep realistic-looking example credentials in README documentation rather than using generic placeholders like <ACCESS_KEY>, even though they are just examples and not real secrets.
Learnt from: NickVolynkin
PR: cozystack/cozystack#1216
File: packages/extra/monitoring/README.md:30-31
Timestamp: 2025-07-26T18:26:01.447Z
Learning: The cozyvalues-gen tool has a known issue (https://github.com/cozystack/cozyvalues-gen/issues/10) where it incorrectly places Grafana configuration entries under the wrong documentation sections, specifically placing grafana.resources.requests.* entries in the Metrics storage configuration table instead of the Grafana configuration table.
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: NickVolynkin
PR: cozystack/cozystack#1216
File: packages/apps/postgres/values.yaml:55-60
Timestamp: 2025-07-26T17:54:57.273Z
Learning: In the cozystack repository, the cozyvalues-gen tool does not support `typedef` syntax for defining custom type aliases in documentation comments.
Learnt from: NickVolynkin
PR: cozystack/cozystack#1120
File: packages/apps/clickhouse/README.md:60-67
Timestamp: 2025-07-03T05:54:51.264Z
Learning: The `cozy-lib.resources.sanitize` function in packages/library/cozy-lib/templates/_resources.tpl supports both standard Kubernetes resource format (with limits:/requests: sections) and flat format (direct resource specifications). The flat format takes priority over nested values. CozyStack apps include cozy-lib as a chart dependency through symlinks in packages/apps/*/charts/cozy-lib directories.
📚 Learning: the cozyvalues-gen tool has a known issue (https://github.com/cozystack/cozyvalues-gen/issues/10) wh...
Learnt from: NickVolynkin
PR: cozystack/cozystack#1216
File: packages/extra/monitoring/README.md:30-31
Timestamp: 2025-07-26T18:26:01.447Z
Learning: The cozyvalues-gen tool has a known issue (https://github.com/cozystack/cozyvalues-gen/issues/10) where it incorrectly places Grafana configuration entries under the wrong documentation sections, specifically placing grafana.resources.requests.* entries in the Metrics storage configuration table instead of the Grafana configuration table.
Applied to files:
packages/extra/monitoring/values.schema.json
📚 Learning: the `cozy-lib.resources.sanitize` function in packages/library/cozy-lib/templates/_resources.tpl sup...
Learnt from: NickVolynkin
PR: cozystack/cozystack#1120
File: packages/apps/clickhouse/README.md:60-67
Timestamp: 2025-07-03T05:54:51.264Z
Learning: The `cozy-lib.resources.sanitize` function in packages/library/cozy-lib/templates/_resources.tpl supports both standard Kubernetes resource format (with limits:/requests: sections) and flat format (direct resource specifications). The flat format takes priority over nested values. CozyStack apps include cozy-lib as a chart dependency through symlinks in packages/apps/*/charts/cozy-lib directories.
Applied to files:
packages/extra/monitoring/values.schema.json
📚 Learning: in the cozystack repository, the maintainer nickvolynkin prefers to keep realistic-looking example c...
Learnt from: NickVolynkin
PR: cozystack/cozystack#1120
File: packages/apps/ferretdb/README.md:35-37
Timestamp: 2025-07-02T09:58:11.406Z
Learning: In the cozystack repository, the maintainer NickVolynkin prefers to keep realistic-looking example credentials in README documentation rather than using generic placeholders like <ACCESS_KEY>, even though they are just examples and not real secrets.
Applied to files:
packages/apps/postgres/values.schema.json
📚 Learning: in helm templates, the `{{-` directive chomps all leading whitespace including newlines back to the ...
Learnt from: lllamnyp
PR: cozystack/cozystack#1160
File: packages/system/hetzner-robotlb/charts/robotlb/templates/role.yaml:6-8
Timestamp: 2025-07-11T06:28:13.696Z
Learning: In Helm templates, the `{{-` directive chomps all leading whitespace including newlines back to the previous content, so `{{- toYaml .Values.something | nindent 2 }}` will render correctly even with apparent indentation issues. However, for better style, it's cleaner to put the template directive on the same line as the parent key (e.g., `rules: {{- toYaml .Values.serviceAccount.permissions | nindent 2 }}`).
Applied to files:
packages/apps/postgres/values.schema.json
📚 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/README.md
🪛 LanguageTool
packages/apps/virtual-machine/README.md
[style] ~57-~57: To form a complete sentence, be sure to include a subject or ‘there’.
Context: ... of SSH public keys for authentication. Can be a single key or a list of keys. ...
(MISSING_IT_THERE)
⏰ 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
| "required": [ | ||
| "destinationPath", | ||
| "enabled", | ||
| "endpointURL", | ||
| "retentionPolicy", | ||
| "s3AccessKey", | ||
| "s3SecretKey", | ||
| "schedule" | ||
| ], |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Conditionally require S3 credentials only when backups are enabled
The schema marks s3AccessKey, s3SecretKey, destinationPath, etc. as unconditionally required. Users must supply them even when backup.enabled is false. Add JSON-Schema conditional validation so those fields become mandatory only when backups are turned on.
"required": [
- "destinationPath",
- "enabled",
- "endpointURL",
- "retentionPolicy",
- "s3AccessKey",
- "s3SecretKey",
- "schedule"
+ "enabled"
],
+ "if": { "properties": { "enabled": { "const": true } } },
+ "then": {
+ "required": [
+ "destinationPath",
+ "endpointURL",
+ "retentionPolicy",
+ "s3AccessKey",
+ "s3SecretKey",
+ "schedule"
+ ]
+ },📝 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.
| "required": [ | |
| "destinationPath", | |
| "enabled", | |
| "endpointURL", | |
| "retentionPolicy", | |
| "s3AccessKey", | |
| "s3SecretKey", | |
| "schedule" | |
| ], | |
| "required": [ | |
| "enabled" | |
| ], | |
| "if": { "properties": { "enabled": { "const": true } } }, | |
| "then": { | |
| "required": [ | |
| "destinationPath", | |
| "endpointURL", | |
| "retentionPolicy", | |
| "s3AccessKey", | |
| "s3SecretKey", | |
| "schedule" | |
| ] | |
| }, |
🤖 Prompt for AI Agents
In packages/apps/postgres/values.schema.json around lines 17 to 25, the fields
s3AccessKey, s3SecretKey, destinationPath, and others are currently always
required regardless of the backup.enabled flag. Modify the JSON schema to use
conditional validation with "if", "then", and "else" clauses so that these
fields are only required when the "enabled" property under backup is true. This
involves moving these fields out of the unconditional "required" array and
adding a conditional block that enforces their presence only when backups are
enabled.
| { | ||
| "title": "Chart Values", | ||
| "type": "object", | ||
| "properties": { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add "additionalProperties": false at the root to catch typos early
Without this, any miss-spelled top-level key silently passes validation and is propagated deep into the chart templates.
Consider:
{
"title": "Chart Values",
"type": "object",
+ "additionalProperties": false,
"properties": {📝 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.
| { | |
| "title": "Chart Values", | |
| "type": "object", | |
| "properties": { | |
| { | |
| "title": "Chart Values", | |
| "type": "object", | |
| "additionalProperties": false, | |
| "properties": { |
🤖 Prompt for AI Agents
In packages/extra/monitoring/values.schema.json at the root level (lines 1 to
4), add the key "additionalProperties": false to the JSON schema object. This
change will prevent any misspelled or unexpected top-level keys from passing
validation silently, helping catch typos early and avoiding propagation of
invalid keys into the chart templates.
4f1154a to
127ffaa
Compare
Signed-off-by: Nick Volynkin <nick.volynkin@gmail.com>
127ffaa to
b26dc63
Compare
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * All application parameter documentation was enhanced with explicit type annotations and structured field descriptions for improved clarity. * README files now include detailed parameter tables with type columns and refined default values. * Helm values.yaml files feature consistent type annotations and hierarchical field documentation. * **Schema Enhancements** * JSON schemas for Postgres, Tenant, Virtual Machine, and Monitoring apps were comprehensively restructured with explicit types, defaults, validation patterns, and richer nested configuration options. * **Chores** * Switched documentation and schema generation tools to a unified command (`cozyvalues-gen`) across all relevant Makefiles and CI workflows for consistency and simplification. * **Bug Fixes** * Updated resource specifications in virtual machine tests for improved accuracy. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
What this PR does
Release note
Summary by CodeRabbit
Documentation
Schema Enhancements
Chores
cozyvalues-gen) across all relevant Makefiles and CI workflows for consistency and simplification.Bug Fixes