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
About this issue
- Original URL
- State: closed
- Created 7 years ago
- Reactions: 6
- Comments: 26 (13 by maintainers)
Commits related to this issue
- Provide default values to satisfy helm linter. This is a workaround for https://github.com/kubernetes/helm/issues/2347 Change-Id: I2dcd761b0056d602e8454a87221e460d0be98287 — committed to streamsets/helm-charts by deleted user 6 years ago
- switch to using default instead of required See: https://github.com/kubernetes/helm/issues/2347 — committed to faraazkhan/charts by faraazkhan 6 years ago
- [stable/neo4j] Update Neo4j to 3.3.4 (#5172) * update neo4j chart * updates neo4j to 3.3.4 * Adds pod disruption budget * Removes privileged security context * Adds delete hook annotation to test po... — committed to helm/charts by faraazkhan 6 years ago
- [stable/neo4j] Update Neo4j to 3.3.4 (#5172) * update neo4j chart * updates neo4j to 3.3.4 * Adds pod disruption budget * Removes privileged security context * Adds delete hook annotation to test po... — committed to Bestmile/charts by faraazkhan 6 years ago
- [stable/neo4j] Update Neo4j to 3.3.4 (#5172) * update neo4j chart * updates neo4j to 3.3.4 * Adds pod disruption budget * Removes privileged security context * Adds delete hook annotation to test po... — committed to or1can/charts by faraazkhan 6 years ago
- [stable/neo4j] Update Neo4j to 3.3.4 (#5172) * update neo4j chart * updates neo4j to 3.3.4 * Adds pod disruption budget * Removes privileged security context * Adds delete hook annotation to test po... — committed to dysnix/helm-charts by faraazkhan 6 years ago
- Fail linting when `required` function fails. This is a revert of the original change that was made as a result of: https://github.com/helm/helm/issues/2347 And was made in: https://github.com/helm/he... — committed to mmccord-mdbuyline/helm by mmccord-mdbuyline 5 years ago
- Fail linting when `required` function fails. This is a revert of the original change that was made as a result of: https://github.com/helm/helm/issues/2347 And was made in: https://github.com/helm/he... — committed to mmccord-mdbuyline/helm by mmccord-mdbuyline 5 years ago
- Fail linting when `required` function fails. This is a revert of the original change that was made as a result of: https://github.com/helm/helm/issues/2347 And was made in: https://github.com/helm/he... — committed to mmccord-mdbuyline/helm by mmccord-mdbuyline 5 years ago
- Fail linting when `required` function fails. This is a revert of the original change that was made as a result of: https://github.com/helm/helm/issues/2347 And was made in: https://github.com/helm/he... — committed to mmccord-mdbuyline/helm by mmccord-mdbuyline 5 years ago
Having a default value for a required values in the charts values.yaml file defeats the whole purpose of the
requiredfunction, 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
requiredfunction 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 chartsvalues.yamlfile 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
requiredfailures.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
requiredfunction during linting. This would allow users to use therequiredfunction and still usehelm lintas 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 lintshould 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
-for--set) tohelm 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
requiredduring linting? Right now if you userequiredas 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
--setwhen 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
prior to linting/templating. Any values in both
values.yamlandrequired.yamlare deep merged with priority going torequired.yamlin a conflict. If there are no required values, this is effectively a noop.