feat: per-chart helm configuration

[Zebradil]

This PR adds per-chart Helm configuration, resolving #261.

There is a new helm.charts subsection in the configuration schema:

helm:
  #! ...

  #! Per-chart configuration. Values override the global configuration.
  #! The `name` field is used to match the chart in the `charts` directory.
  #! The list is used instead of a map due to a limitation in ytt schema spec.
  #! See https://github.com/carvel-dev/ytt/issues/656 for more information.
  #@schema/validation ("chart names must be unique", lambda x: len(set([c["name"] for c in x])) == len(x))
  charts:
    - buildDependencies: false
      includeCRDs: false
      #@schema/validation min_len=1
      name: ""
      namespace: ""
      releaseName: ""

It's a list of per-chart overrides of the common options under the helm key in the config.
Selecting the chart-specific configuration is done via matching the name field with the chart's base name. For example, if a chart's path defined in the vendir config is charts/CHART_NAME, then CHART_NAME is the name of the chart and is used for selecting the options overrides.

[fritzduchardt]

Hi @Zebradil , while testing this, I conceived an app-data.ytt.yaml that would currently fail:

#@data/values-schema
#@overlay/match-child-defaults missing_ok=True
---
helm:
  namespace: monitoring
  charts:
    - buildDependencies: true
      name: "kube-prometheus-stack"
    - buildDependencies: false
      name: "kube-prometheus-stack-exporter"
application:
  namespace:
    create: true

It would lead to the following error:

Invalid schema - wrong number of items in array definition
==========================================================

data-schema.ytt.yaml:
      |
  108 |   charts:
      |

      = found: 2 array items
      = expected: exactly 1 array item, of the desired type

Is this a case you have considered?

[Zebradil]

Hi @fritzduchardt, thanks for testing!

It is a bug, and a new test case. I'll implement it shortly.

[Zebradil]

@fritzduchardt it turns out that it's not a bug, but a limitation of ytt. You can't define default values for arrays like that in a ytt schema file (#@data/values-schema). You have to either use a data file (#@data/values) or use annotations in the schema file:

#@ def charts():
- name: render-test-1
  releaseName: overriden-release-name-1
- name: render-test-2
  releaseName: overriden-release-name-2
#@ end

#@data/values-schema
---
helm:
  #@schema/default charts()
  #@overlay/replace
  #@schema/validation ("chart names must be unique", lambda x: len(set([c["name"] for c in x])) == len(x))
  charts:
    - buildDependencies: false
      includeCRDs: false
      #@schema/validation min_len=1
      name: ""
      namespace: ""
      releaseName: ""

The latter I find very inconvenient, because you have to copy the original array item definition, as well as the validation annotations.

[fritzduchardt]

The latter I find very inconvenient, because you have to copy the original array item definition, as well as the validation annotations.

Nice workaround! Since you also do validation in config_helm.go, I guess the validations are not strictly required.

[fritzduchardt]

You could also validate for charts being part of the config, but not of the actual app. Of course, this would lead to a lot of errors/warnings, if you define your chart-specific configuration in the env-data.ytt.yaml. Nevertheless, this feature seems more likely to be used in the app-data.ytt.yaml

[Zebradil]

It's an interesting idea. I think we can give it a try.

[Zebradil]

Implemented. This is how it looks like.

Currently, there is no central place for config validation (and it will require significant changes to the overall program structure to implement it), this is why it is duplicated in the sync and render steps and printed twice. But I think we can live with that :-)

Later, if these warnings are too annoying, we can add a switch to disable it.

[Zebradil]

Since you also do validation in config_helm.go, I guess the validations are not strictly required.

That's right. However, ytt validations give better error message, in my opinion. But of course, it is completely up to the user :-)