Helm linter is not instantiating templates correctly, balking on dependencies.

[originalrkk]

We have the following files in a directory:

common/BUILD                      -- Just helm_chart().
common/Chart.yaml                 -- Nothing interesting.
common/values.yaml                -- Contains 'value_a: a' (a semantically required value)
common/templates/_cronJob.yaml    -- Wrapped in `{{- define "common.cronjob.tpl" -}}`;
                                  -- otherwise typical and needs .Values.value_a, value_b, value_c.

actual-project/BUILD                      -- Just helm_chart().
actual-project/Chart.yaml                 -- Contains dependency `file://../common/`
actual-project/values.yaml                -- Contains 'value_b: b' (a semantically required value)
actual-project/values.schema.json         -- Describes 'value_a', 'value_b', 'value_c' as required.
actual-project/templates/cronJob.yaml     -- Just `{{- include "common.cronjob.tpl" . -}}`.
actual-project/actual-task/values.yaml    -- Contains 'value_c: c' (a semantically required value)

A BUILD file at the root of this contains:

helm_deployment(
    name = 'actual-project-actual-task-prod',
    dependencies = ['//path/to/actual-project:actual-project'],
    sources = [
        'common/values.yaml',
        'actual-project/values.yaml',
        'actual-project/actual-task/values.yaml',
    ],
    tags = ['actual-project'],
)

(It would also be nice if these rules could live in each project directory, but that's a separate issue, i.e. supporting a reference to ../common/values.yaml the same way Chart.yaml supports file://../common/.)

Anyway, when running linting in this setup, such as pants lint --only=helm actual_project::, this generates errors such as this:

[ERROR] templates/: template: actual-project/templates/cronJob.yaml:X:XX: executing "actual-project/templates/cronJob.yaml" at <.Values.value_c>: wrong type for value; expected string; got interface {}

Or this:

[ERROR] templates/: template: actual-project/templates/cronJob.yaml:X:XX: executing "actual-project/templates/cronJob.yaml" at <include "common.cronjob.tpl" .>: error calling include: template: actual-project/charts/common/templates/_cronJob.yaml:XX:XX: executing "common.cronjob.tpl" at <.Values.value_a>: nil pointer evaluating interface ...

If, however, I run the following in actual-project, I get the correctly-filled-in result:

helm dependency build .
helm template . -f ../common/values.yaml -f values.yaml -f actual-task/values.yaml

Pants version: 2.17
OS: Linux

[lilatomic]

Seems we don't use file://... for dependency inference, we just use the chart name and glue it together ref. I think it makes sense to also resolve those.
Seems I spoke too soon, it seems like file://... dependencies are resolved. Let me dig into this some more.

[lilatomic]

Hm, I'm not able to get a repro. I've committed the files I'm using to a subtree here. you can try with

cp -r testprojects/src/helm/ /tmp/helm19966/
pushd /tmp/helm19966/
pants lint --only=helm actual-project::

Let me know if you spot what I haven't copied correctly

[originalrkk]

I think the template is too simple here. Try:

metadata:
  name: .Values.value_c
  labels:
    a: {{ .Values.value_a | required "test a" }}
    b: {{ .Values.value_b | required "test b" }}
    c: {{ .Values.value_c | required "test c" }}

Only the chart's values.yaml seems to be plumbed through. I get

[WARNING] templates/cronJob.yaml: object name does not conform to Kubernetes naming requirements: ".Values.value_c": metadata.name: Invalid value: ".Values.value_c": a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')

1 chart(s) linted, 0 chart(s) failed

engine.go:168: [INFO] Missing required value: test a
engine.go:168: [INFO] Missing required value: test c

(Thanks for looking into this!)

[lilatomic]

Thanks for that sample. I'm not sure what to do, though, as I'm not really a Helm power user.
The error pants lint gives is the same as the error helm lint gives. I tried adding the --with-subcharts flag, but that just seems to have also linted the subchart too, rather than the combination. Do you know of a Helm command we could run that would consider these charts together?
The only way I know of combining the charts is to use the helm template command. Pants does that as part of the pants experimental-deploy command. So pants experimental-deploy //:actual-project-actual-task-prod results in "cronjob.batch/hello" with the expected metadata.labels .
Again, I'm not an advanced Helm user. If you know of the Helm commands to run, I can add those to Pants!

[originalrkk]

So I think a couple points here:

• actual-task isn't a subchart, it's an instance (we prefer to keep information about instances in the same directory, but that directory and its values.yaml could be anywhere).
• helm lint would also need -f common/values.yaml -f actual-project/values.yaml -f actual-project/actual-task/values.yaml to use the base values of the charts as well as the instance values.

Not sure beyond that. I'd have to dig more. I realized we also adopted:

exports:
  defaults:
      value_a: "a"

in common/values.yaml and

dependencies:
  - name: common
     < ... >
    import-values:
      - defaults

in actual-project/Chart.lock.

[lilatomic]

Oh neat, I didn't realise that you could pass vars files to helm lint. I think we could implement that.
I tried fiddling with exports and import-values, but couldn't get it to fix the issue.

[alonsodomin]

I don't think this is a bug. The Helm backend is not capable of resolving chart dependencies (i.e. subcharts) using file://... references. This is somewhat intentional as Pants works internally with a virtual filesystem and therefore references like file://.../common are most of the time invalid. It's true that Helm itself does support that but adding that to Pants is non-trivial as Pants is designed to be able to run distributed builds using remote servers (hence the reason it uses a virtual filesystem).

To use chart dependencies you should instead use one of the two approaches documented here: https://www.pantsbuild.org/2.21/docs/helm#managing-chart-dependencies

That way Pants will properly identify the subcharts and their sources, build a compound chart that includes the root one plus all of its dependencies and use that compound chart when running tasks like lint or publish.

Besides, a values.yml file at the root of a chart (sibling to Chart.yml) is considered to be the default values of a chart. It will be included by default when running a deployment and there is no need to reference them when using helm_deployment targets.

In the other hand, the location of actual-project/actual-task/values.yaml is particularly odd. The helm_chart target will ignore that file since, even though it's under the same file structure of the chart, it doesn't correspond to any of the standard chart file locations documented here: https://helm.sh/docs/topics/charts/#the-chart-file-structure

If you want to pass values to the deployment of a chart (via value files), we recommend to follow what is documented here: https://www.pantsbuild.org/2.21/docs/helm/deployments#value-files. This is, create a specific folder in your repo where to put all the value files that a given deployment may require (outside the structure of any Chart) and then add a BUILD file to that folder where you put your helm_deployment target configured as suggested in our docs.

[alonsodomin]

That said, it is quite likely that current Helm implementation is missing some recent features supported by Helm, which could involve the possibility of importing custom value files from other charts or similar.

Happy to discuss those as some sort of feature request though.