feat: values.schema.json to support air-gapped environments

[tumido]

Description of the change

Air-gapped environments should be supported. The only blocker we had here were remote references in values.schema.json. While these remote references reduce maintenance and repetition on our side, it requires helm to query for those references when processing values file.

Therefore to keep the benefits of remote references and allow consumers to install in air-gapped environment, I've added a small pre-commit script allowing us to treat the values.schema.json with remote references as a kind of template. I've moved our current values.schema.json to values.schema.tmpl.json. This template is dereferenced via pre-commit and the resulting schema with expanded references is stored as values.schema.json. This schema is then understood by Helm and can be used in air-gapped environments.

Existing or Associated Issue(s)

Resolves #127

Additional Information

Checklist

• Chart version bumped in Chart.yaml according to semver.
• Variables are documented in the values.yaml and added to the README.md. The helm-docs utility can be used to generate the necessary content. Use helm-docs --dry-run to preview the content.
• JSON Schema generated.
• List tests pass for Chart using the Chart Testing tool and the ct lint command.

[nickboldt]

Can we merge this?

[tumido]

@vinzscam @sabre1041 can I get a review here please?

[vinzscam]

@vinzscam @sabre1041 can I get a review here please?

Nice workaround 😄
This solution won’t provide any recursive support for $ref defined inside definitions, right? If for example values.schema.json references a schema which is referencing another schema using $ref, the problem will still persist.
I wonder if it makes sense to relax values.schema.json a bit, not referencing any external definition.

Other downsides I see of using this approach are:

• Each definition is around 1MB, which means the size of the chart will be increase by xxMB
• If a definition changes we would need to deploy a new version of the chart

[sabre1041]

@vinzscam sure thing. will review today

[sabre1041]

@vinzscam Understand the concern related to size of the schema. an alternative approach is to source references from a relative files to reduce the overall size of the primary schema file

[tumido]

This solution won’t provide any recursive support for $ref defined inside definitions, right? If for example values.schema.json references a schema which is referencing another schema using $ref, the problem will still persist.

Oh, I think it does work recursively. I've used a similar approach in our RH derived chart and it resolves this chart's schema refs as well..

Each definition is around 1MB, which means the size of the chart will be increase by xxMB

Oh, 1MB? Where do you see that, @vinzscam ? The resulting (dereferenced) values.schema.json here is 326K on my end... Which I think is acceptable. 🙂 WDYT?

If a definition changes we would need to deploy a new version of the chart

Good point. While I don't find it very likely - especially in the case of kubernetes schema references. It can be true for the postgresql subchart though. We can either lock it to the same revision we use in Chart.lock or even remove it completely, since helm install validates subcharts against their own schema. This is however not the case for many Helm UIs that depend on a full schema to be present inside the top-most chart, so if we want to support all users I would prefer to keep the complete schema inside here.

In our derived chart we use similar solution which locks subchart's remote references to the exact version found in Chart.lock, you can find it here:

https://github.com/janus-idp/helm-backstage/blob/main/.pre-commit/jsonschema-dereference.py

Would that be a better solution @vinzscam? 🙂

[vinzscam]

Oh, I think it does work recursively. I've used a similar approach in our RH derived chart and it resolves this chart's schema refs as well..

I see, nice! I was just worried whether there might be some corner cases which aren't covered by the script, which might add more work on our side. But the same approach is already battle tested in your subchart I'm fine with that.
Otherwise other approaches I see are:

• relax the schema definition, removing any external references
• publish a separate version of the chart, removing any external references, specific for air-gapped environment

Curious to hear your thoughts 😄

[nickboldt]

Could we merge this as-is, and open a followup issue for future enhancements?

Or are we waiting to enhance this before merging it?

[tumido]

@vinzscam I would rather avoid publishing a special air-gapped version of the chart. 🙂 Relaxing the schema is a step backward in my eyes, a complete schema is a better guide to the user than a flexible one which "works" but can lead to improper configurations.

Do you think I should change anything in this PR? Do you think this implementation should cause any trouble to the chart users?

[vinzscam]

@vinzscam I would rather avoid publishing a special air-gapped version of the chart. 🙂 Relaxing the schema is a step backward in my eyes, a complete schema is a better guide to the user than a flexible one which "works" but can lead to improper configurations.

yeah good point

Do you think I should change anything in this PR? Do you think this implementation should cause any trouble to the chart users?

nope, let's 🚀
maybe just as a minor thing we could include values.schema.tmpl.json into .helmignore and avoid checking-in values.schema.json and just generate it at build time? But I'm not sure, probably you need values.schema.json in git as it is used in your subchart

[tumido]

maybe just as a minor thing we could include values.schema.tmpl.json into .helmignore and avoid checking-in values.schema.json and just generate it at build time? But I'm not sure, probably you need values.schema.json in git as it is used in your subchart

I've added values.schema.tmpl.json to .helmignore, good call 🙂 No need to package that.

I would prefer to leave the generated values.schema.json in the repo so we can $ref it in our schema 🙂

[sabre1041]

LGTM