Allows some more default values, based on an example values file

[solsson]

When editing a values file in VSCode with yaml and # yaml-language-server: $schema=https://github.com/redpanda-data/helm-charts/raw/redpanda-4.0.52/charts/redpanda/values.schema.json quite a few values that do have defaults show up as required.

Here I have only removed required entries for the example yaml given in the first commit comment, which as far as I can see produces a working cluster.

Using defaults as much as possible is an important strategy for depending on helm charts. Validating values files using the schema helps a lot too.

Or is the schema meant for CI to validate the default values file? If so would it be possible to somehow maintain two variants of the schema?

[CLAassistant]

All committers have signed the CLA.

[RafalKorepta]

Hi @solsson,

I agree that schema should reflect only values that needs to be set.

Can you sign CLA?

Thanks

[joejulian]

Even though they have defaults, a helm user can remove them by setting the value to null. The restrictions are in place because the chart will fail to render if those values are removed.

I would rather keep these restrictions in place unless there's a good reason to remove them.

[solsson]

@joejulian The PR only removes the prop names from the required sections in the schema, so that a values file isn't marked as invalid when it relies on defaults.

The alternative for a user of the helm chart is to skip validation and thus get a degraded editing experience, or to copy paste defaults from the chart's values.yaml which is unwise because they might change as the chart evolves.

[joejulian]

Perhaps you're using values incorrectly. You're not supposed to replace the values.yaml in the chart, but rather you're supposed to create a file with values you wish to override. On the helm command line, you'll specify your override file with the --values flag. Your overrides are then merged with the chart's default values.yaml file satisfying the schema.

[solsson]

I restored my original title for the PR. I don't know how to describe intent better, but the purpose is only to allow a values file like the one in 2cd442a to be validated. It's not meant to change anything about how default values or the chart in general behaves.

The change aligns with this opinion from above:

I agree that schema should reflect only values that needs to be set.

[joejulian]

Using your branch, if image, for example, is not required I should be able to remove it successfully:

helm template xx charts/redpanda --set image=null

unfortunately, that does not work:

Error: template: redpanda/templates/tests/test-schemaregistry-internal-tls-status.yaml:17:119: executing "redpanda/templates/tests/test-schemaregistry-internal-tls-status.yaml" at <include "redpanda-22-2-x-without-sasl" .>: error calling include: template: redpanda/templates/_helpers.tpl:542:17: executing "redpanda-22-2-x-without-sasl" at <include "redpanda-atleast-22-3-0" .>: error calling include: template: redpanda/templates/_helpers.tpl:526:44: executing "redpanda-atleast-22-3-0" at <.Values.image.repository>: nil pointer evaluating interface {}.repository

so the image key is required.

The schema validates the values after the 3-way merge so regardless of defaults, the keys the schema validates ensures that the fields that are required to render the template are included.

If I run that same command on main:

Error: values don't meet the specifications of the schema(s) in the following chart(s):
redpanda:
- (root): image is required

[joejulian]

Blocking as this is currently implementing invalid assumptions and cannot be merged.

[joejulian]

This seems to have been abandoned. Closing.