This repository has been archived by the owner on Feb 22, 2022. It is now read-only.
Adding linting to Chart and values yaml files #2429
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
k8s-ci-robot
added
cncf-cla: yes
unguiculus
suggested changes
Oct 8, 2017
test/circle/lint.sh
Outdated
|
|
||
| # If a Chart.yaml file is present lint it. Otherwise report an error | ||
| # because one should exist | ||
| if [ -e ${directory}/Chart.yaml ] |
There was a problem hiding this comment.
I'd prefer if you stick to the existing style and put then on the same line.
if [ -e ${directory}/Chart.yaml ]; then
test/circle/lint.sh
Outdated
|
|
||
| # If a values.yaml file is present lint it. Otherwise report an error | ||
| # because one should exist | ||
| if [ -e ${directory}/values.yaml ] |
test/circle/lint.sh
Outdated
| # because one should exist | ||
| if [ -e ${directory}/Chart.yaml ] | ||
| then | ||
| run yamllint -c test/circle/lintconf.yml ${directory}/Chart.yaml |
There was a problem hiding this comment.
The rest of the file seems to be indented with two spaces. Can we agree on one style?
test/circle/lint.sh
Outdated
| $@ | ||
| local ret=$? | ||
| if [ $ret -ne 0 ]; then | ||
| exitCode=1 |
This does not lint yaml templates as they are templates rather than valid yaml files. They cannot be linted with a normal linter yamllint is used for linting. This is an existing Python project https://github.com/adrienverge/yamllint The rules are not the default rules and are stored in their entirity so they can be controlled over time The existance of a Chart.yaml file and values.yaml file is checked and an error is thrown if one is missing. Helm lint will not detect a chart if Chart.yaml is missing and if a values.yaml file is missing it is noted as info. The run function is introduced to enable running all the linters, capturing non-zero exit codes, and exiting with a non-zdero code if any of them fail. This is used instead of exiting when the first failure happens to provide more feedback to chart developers.
|
@unguiculus Sorry about that. Thanks for catching those. I'm too used to my editor fixing issues like these automatically. I see I don't have that setup for shell scripts. I've updated with a fix for the formatting. |
|
/lgtm |
unguiculus
approved these changes
Oct 9, 2017
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This does not lint yaml templates as they are templates rather
than valid yaml files. They cannot be linted with a normal linter
yamllint is used for linting. This is an existing Python project
https://github.com/adrienverge/yamllint
The rules are not the default rules and are stored in their
entirity so they can be controlled over time
The existance of a Chart.yaml file and values.yaml file is checked
and an error is thrown if one is missing. Helm lint will not
detect a chart if Chart.yaml is missing and if a values.yaml file
is missing it is noted as info.
The run function is introduced to enable running all the linters,
capturing non-zero exit codes, and exiting with a non-zdero code
if any of them fail. This is used instead of exiting when the
first failure happens to provide more feedback to chart developers.
/cc @prydonius @viglesiasce @unguiculus
You can see an example of a failure at https://circleci.com/gh/mattfarina/charts/41
There are docs on configuring yamllint including the rules. I tried to tailor our rules to the existing charts. Though, many of the charts fail the rules and are inconsistent.
You can take a look at all the lint failures we have in the existing stable and incubator charts in this gist based on the rules in the config file here.
Once we agree on a set of rules I can create separate PRs to clean up the existing charts.
References #2373