fix(chart): allow 'enabled' and 'global' in values schema - #97
Open
ecthelion77 wants to merge 2 commits into
Open
fix(chart): allow 'enabled' and 'global' in values schema#97ecthelion77 wants to merge 2 commits into
ecthelion77 wants to merge 2 commits into
Conversation
When the chart is used as a subchart dependency, Helm injects 'global' values from the parent and passes the 'enabled' condition flag. The strict additionalProperties:false schema rejects these, causing helm template to fail even when the subchart is disabled. Add both properties to the root schema to allow standard Helm subchart usage patterns. Signed-off-by: Olivier Gintrand <olivier.gintrand@forterro.com>
![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
This pull request updates the values.schema.json file by adding enabled and global properties to support the chart's use as a sub-chart. The reviewer recommends also adding these keys to the values.yaml file to improve discoverability and ensure they are included in generated documentation.
Per review feedback: declare sub-chart injected properties in values.yaml so they appear in helm show values and helm-docs generated README. Signed-off-by: Olivier Gintrand <olivier.gintrand@forterro.com>
Contributor
Author
|
@nalyk Friendly ping — this fix is now blocking our upgrade path from 0.5.1 (forked) to 0.9.1 (upstream). Created issue #104 with full context. The PR is a 17-line additive change with zero behavioral impact — just the two properties Helm requires for sub-chart usage ( |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When
gitlab-mcpis used as a sub-chart dependency in an umbrella chart, Helm injects two additional top-level properties into the sub-chart's values:enabled— from thecondition:field in the parent'sChart.yamlglobal— inherited global values from the parent chartSince
values.schema.jsonhas"additionalProperties": falseat the root level,helm templatefails with:This blocks any ArgoCD or Flux deployment using this chart as a dependency.
Fix
Add both properties to the schema's root
properties:enabled:{"type": "boolean"}global:{"type": "object", "additionalProperties": true}This is a standard pattern for Helm sub-charts with strict schemas (see Helm docs on subcharts and global values).
Testing
helm template test chart/ --set secret.GITLAB_PERSONAL_ACCESS_TOKEN=ok --set enabled=true --set global.imageRegistry=fooPasses without error after the fix.