-
Notifications
You must be signed in to change notification settings - Fork 16
chore: Adds the missing properties(defaults.tag and defaults.reposito… #215
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
chore: Adds the missing properties(defaults.tag and defaults.reposito… #215
Conversation
…ry) in the value schema definition
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
This PR adds repository and tag properties to the defaults section of the Helm values JSON schema, both typed as string | null.
Key Changes:
- Added
defaults.repositoryproperty to schema definition (line 5834-5836) - Added
defaults.tagproperty to schema definition (line 5837-5839) - JSON schema is syntactically valid
Analysis:
The properties appear to address schema validation requirements since defaults has additionalProperties: false. However, based on template usage patterns, these properties may be at the incorrect nesting level:
- The
generateImagehelper function receives.Values.defaults.imageas itsdefaultsparameter - Within the function, it accesses
.defaults.repositoryand.defaults.tag, which resolves todefaults.image.repositoryanddefaults.image.tag - These properties already exist in the
com.cloudzero.agent.imageschema definition referenced bydefaults.image
The change doesn't introduce breaking issues but may be organizationally redundant unless there's a use case for setting these properties directly under defaults (not nested under defaults.image).
Confidence Score: 4/5
- This PR is safe to merge with minimal risk - it only adds schema validation properties
- The changes are limited to JSON schema property definitions with no logic modifications. The JSON is syntactically valid and won't break existing functionality. Score reflects a minor organizational concern about property nesting level, but this doesn't affect runtime behavior since the properties already exist in the referenced image schema.
- No files require special attention - this is a straightforward schema update
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| charts/cloudzero-agent/values.schema.json | 4/5 | Adds repository and tag properties to defaults schema; JSON is valid but properties may be at incorrect nesting level |
Sequence Diagram
sequenceDiagram
participant User
participant HelmSchema as values.schema.json
participant HelmValues as values.yaml
participant Template as _helpers.tpl
participant Deploy as agent-deploy.yaml
User->>HelmValues: Set defaults.image.repository/tag
User->>HelmSchema: Validate values
HelmSchema->>HelmSchema: Check defaults.repository/tag (new)
HelmSchema->>HelmSchema: Check defaults.image properties (existing)
HelmSchema-->>User: Validation passes
Deploy->>Template: Call generateImage
Note over Deploy,Template: Pass .Values.defaults.image as "defaults"
Template->>Template: Access .defaults.repository
Note over Template: Resolves to defaults.image.repository
Template->>Template: Access .defaults.tag
Note over Template: Resolves to defaults.image.tag
Template-->>Deploy: Return image configuration
1 file reviewed, 1 comment
| "repository":{ | ||
| "type": ["string", "null"] | ||
| }, | ||
| "tag":{ | ||
| "type": ["string", "null"] | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: These properties may be at the incorrect nesting level. The generateImage template helper (templates/_helpers.tpl:770-789) is always called with defaults parameter set to .Values.defaults.image. This means repository and tag are accessed from the image object, where they already exist via the $ref to com.cloudzero.agent.image. Adding them directly under defaults appears redundant unless users need to set them outside of defaults.image.
Prompt To Fix With AI
This is a comment left during a code review.
Path: charts/cloudzero-agent/values.schema.json
Line: 5834:5839
Comment:
**style:** These properties may be at the incorrect nesting level. The `generateImage` template helper (templates/_helpers.tpl:770-789) is always called with `defaults` parameter set to `.Values.defaults.image`. This means `repository` and `tag` are accessed from the image object, where they already exist via the `$ref` to `com.cloudzero.agent.image`. Adding them directly under `defaults` appears redundant unless users need to set them outside of `defaults.image`.
How can I resolve this? If you propose a fix, please make it concise.|
Hey @heitorrbarros, thanks for the contribution, we really appreciate it! Unfortunately we can't actually accept it here, since the The reason those don't have defaults set is because all of the images we use would override those anyways. I suspect that what you really want to do is set Could you provide a bit more context on what you're trying to accomplish here? |
By submitting a PR to this repository, you agree to the terms within the CloudZero Code of Conduct. Please see the contributing guidelines for how to create and submit a high-quality PR for this repo.
Please note that changes to the
cloudzero-agentHelm chart should be made instead in thehelm directoryin the cloudzero-agent repository, and will automatically be mirrored to this repository as soon as they are merged.Description
On PicPay, we need to push the provider's container images to our private repository. This approach helps us conduct security scans and reduces costs associated with data transfer.
After embedding the image, I needed to point the tag and repository to our private repository. I noticed that we can apply this change by updating the values in the defaults property.
Here’s the relevant link for reference: Cloudzero Helm Chart.
After setting those values, I started encountering a schema error during the templating step. To resolve this, I created a PR to add the missing properties to the schema definition.
References
Testing
Inside cloudzero-agent chart run the helm template command ->
Output
Checklist
main