charts icon indicating copy to clipboard operation
charts copied to clipboard

[bitnami/apisix] Updated jsonschema to allow string values for fields passed to tpl

Open james-mchugh opened this issue 1 year ago • 1 comments

Description of the change

Updates all fields in values.schema.json that are directly passed to tpl to also allow for string fields.

Benefits

Chart users can take full advantage of the tpl function being used to do advanced templating from custom values.yaml files. For example,

extraEnvVars: |-
  {{- include "my-chart.apisix.authEnvVars" .  }}

Possible drawbacks

Making strings an allowed type for many of these fields increases the risk that a user may input an invalid value.

However, I've done a quick audit of the Bitnami charts and how they utilize the values.schema.json file. Of the 110 Bitnami charts, 23 use a values.schema.json file. Of those 23, only 5 (including Apisix) have over 400 lines. Most schemas used for Bitnami charts are pretty sparse, and it is not typical for all fields to be included with strict validation.

Applicable issues

  • fixes #27319

Additional information

If we want to scope this PR down a bit, we can scope it to only include the extraEnvVars fields that were mentioned in the linked issue.

Checklist

  • [X] Chart version bumped in Chart.yaml according to semver. This is not necessary when the changes only affect README.md files.
  • [X] Variables are documented in the values.yaml and added to the README.md using readme-generator-for-helm
  • [X] Title of the pull request follows this pattern [bitnami/<name_of_the_chart>] Descriptive title
  • [X] All commits signed off and in agreement of Developer Certificate of Origin (DCO)

james-mchugh avatar Jun 19 '24 04:06 james-mchugh

@migruiz4 thank you for taking the review! Please let me know if any updates are needed or if you'd like me to scope down the changes. I'm hoping we can get this merged soon so my team doesn't have to continue to fork the chart.

james-mchugh avatar Jun 26 '24 22:06 james-mchugh