helm: "required" template function breaks linter

Just noticed that helm lint goes out the door as soon as you add your first {{ required ... }} to any template.

Steps to reproduce:

> helm create something
> helm lint something/
==> Linting .
[INFO] Chart.yaml: icon is recommended

1 chart(s) linted, no failures

# add a label to the deployment with value: {{ required "missing required value blafasel" .Values.blafasel }}
> helm lint something/
[ERROR] templates/: render error in "blafasel/templates/deployment.yaml": template: blafasel/templates/deployment.yaml:13:17: executing "blafasel/templates/deployment.yaml" at <required "missing re...>: error calling required: missing required value blafasel

/cc @wilkers-steve @thomastaylor312

About this issue

  • Original URL
  • State: closed
  • Created 7 years ago
  • Reactions: 6
  • Comments: 26 (13 by maintainers)

Commits related to this issue

Most upvoted comments

Having a default value for a required values in the charts values.yaml file defeats the whole purpose of the required function, no? Its never going to fire.

Some charts require values that can’t have a sensible default value, I thought the whole purpose of the required function was to provide a human readable errors message if you forget the specify a required value. If I have something like ( requiredFlag: PLEASE_TYPE_SOMETHING_HERE) in the charts values.yaml file that is not going to happen. The chart will be installed with the bogus value without any error.

I think for linting it would make sense to disable/replace the require function because as you said its a only a syntax/render checker.

Lint should ignored required failures.

IMHO, linter requiring values to be supplied through -f or --set seems wrong. Chart is either valid, or not valid without outside additional values. Seems like this will also make validation of charts complicated in a CI scenario.

@technosophos I think the simplest most straight forward solution at least for the next minor release would be to disable/noop the required function during linting. This would allow users to use the required function and still use helm lint as they are used to.

@wilkers-steve I’m not sure I understand your point gotpl also supporting generation of values. Can you maybe give a concrete example?

In general I’m still not convinced that helm lint should check/fail for missing required values that must be provided by the end user. In my mind linting should validate the internal structure of chart without requiring some additional values being passed in for it to succeed.

The original issue for adding the required function can be found here. The required function ensures that any value flagged as required as a matching entry in values.yaml, whether it’s during linting or installation.

Lint indeed does not take external values, so linting a chart leveraging required values would need a default value set in your chart’s values file. My argument here is that linting is just syntax/render checking at the end of the day, so having a default value for a required value makes sense. I feel any value seen as required should have a default value as a best practice, even if the intent is to override the value during installation. At the end of the day, that’s just my opinion though.

So is the desired solution to be able to supply values file (via -f or --set) to helm lint?

I don’t believe this has been resolved yet. Looks like #4221 would address this

With helm 2.4.0 right around the door can we agree to fix this initially by disabling required during linting? Right now if you use required as intended linting is broken without a good workaround.

I can do the PR if desired.

Any update on this? Would like to use required in my custom charts, but that breaks all my CI/CD builds due to linter errors 😕

For me linting is about checking if the syntax is right, not input validation. So in case of linting I would just replace the required function with one which does nothing and lets the validation pass (even without a value)

Its not in the charts values.yaml. Its supposed to be specified with an external values file or --set when installing the chart.

As lint does not take any external values you can’t use lint anymore on charts having required values unless I’m missing something.

I’m not sure I follow your argument that a required values has to be present in the charts values file. If I put the value in the charts internal values file the required function becomes a noop as the value always has a default set.

For anyone stumbling across this in future, we work around this by performing a

yq eval-all -Mi '. as $item ireduce ({}; . *+d $item )' values.yaml required.yaml

prior to linting/templating. Any values in both values.yaml and required.yaml are deep merged with priority going to required.yaml in a conflict. If there are no required values, this is effectively a noop.