feat(dev): evaluate helm chart schemas in dev generate-schema - #5176
feat(dev): evaluate helm chart schemas in dev generate-schema#5176mjnagel wants to merge 8 commits into
Conversation
Signed-off-by: Micah Nagel <micah.nagel@defenseunicorns.com>
Signed-off-by: Micah Nagel <micah.nagel@defenseunicorns.com>
✅ Deploy Preview for zarf-docs canceled.
|
Signed-off-by: Micah Nagel <micah.nagel@defenseunicorns.com>
AustinAbro321
left a comment
AustinAbro321
left a comment
There was a problem hiding this comment.
Overall looks like a good addition, requesting a few changes
| } else if err := value.CheckNoExternalRefs(chartSchema); err != nil { | ||
| l.Warn("unable to use Helm chart values schema; falling back to inferred types", "chart", chart.Name, "error", err) | ||
| chartSchema = nil |
There was a problem hiding this comment.
We do this check for Zarf charts, because a external reference breaks our air-gap nativeness, however for zarf dev generate-schema an external reference somewhere in the schema is fine, as we assume connectivity.
There was a problem hiding this comment.
Hm yeah let me look at this a bit further. I originally had some code to handle refs but it was starting to feel messy (especially with multiple charts and potential for clashes between their named definitions). But can try to find a sane/simple approach to handle some of this.
There was a problem hiding this comment.
Okay I think I tackled this in the latest changes - it should ensure we drop any "more complex" ref/def handling while keeping any unrelated portions of the schema that we do have more plainly.
Signed-off-by: Micah Nagel <micah.nagel@defenseunicorns.com>
Signed-off-by: Micah Nagel <micah.nagel@defenseunicorns.com>
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
Signed-off-by: Micah Nagel <micah.nagel@defenseunicorns.com>
AustinAbro321
left a comment
AustinAbro321
left a comment
There was a problem hiding this comment.
While there are JSON schema edge cases not handled, this is an improvement over baseline, and provides good value for its complexity. LGTM
3 participants
Description
Evaluates helm chart schemas as part of the
zarf dev generate-schemabehavior. This particular implementation:description,examples, and other similar "metadata" schema keys are not importedrequired/minPropertiesare not imported as they cannot be "fully evaluated" at the zarf layerallOf,anyOf,$ref, etc) are skipped and fall back to inferred typing (these can all get more messy to properly handle esp in multi-chart scenarios or when other omitted fields are present in these)The main focus is on high value / low risk schema constraints (type and bounds) while not bloating the zarf schema with excessive validations or introducing significant logic to handle some of the more complex scenarios.
Related Issue
Fixes #5157
Checklist before merging