feature request: "Strict mode"

[edmondop]

Enforcing documentation guidelines via CI/CD check is desirable in some context, and the same way a linter could complain about a non documented function, it would be great if helm-docs could complain about a non documented value.

I am happy to take care of this if there is a green light and prepare a pr

[Nepo26]

I think this could go pretty well when we are adding documentation to multiple helm charts. Currently, the only that it's done is just by seeing if the value is empty on the generated table. For multiple charts it does indeed make sense.

In the future it may even go well with subchart checking that is discussed in the following issues:

• Support for creating docs of named templates in library charts #165
• Support library charts #82
• [Enhancement] Also use values.schema.json #90

Are you still willing to add a PR? @edmondop

[edmondop]

@Nepo26 no I wanted to check if there was interest, but I am happy to take care of it soon!

[Nepo26]

I think there is interest for sure. We are revamping the repo, so I'll not be able to contribute to this issue yet. So feel free to work on it whenever you are able.

[Nepo26]

Hey @edmondop I'll look into your PR today. Sorry for the delay.

[edmondop]

@Nepo26 any release on the horizon for 1.11.1?

[Nepo26]

Yes, Sorry for the late response. By the end of next week the latest merges will be published. Just need to finish cleaning up documentation.

[Nepo26]

Hey @edmondop. Already published the release. If you can, check it and respond if everything is OK.

Sorry for the late response. There were some problems with the release cycle.

[jhoover4]

Is this close to being completed?

[Nepo26]

Hey @jhoover4, this function has already been released.

[jhoover4]

Oh wow, how do I use it then? I tried --strict and that didn't work.

Is it the -x argument? The problem I have with that is that it exits with 0 even if there are undocumented aspects

[Nepo26]

Yes. It's the -x argument. You can take a look on the README's strict linting section. Can you provide the chart that you are testing against? Or make a simpler example so I can reproduce it?

[jhoover4]

Hey @Nepo26 its really easy to see. You can run it against the example charts, like so:

➜  helm-docs git:(master) ✗ helm-docs -x -c example-charts
INFO[2023-10-13T09:59:51-05:00] Found Chart directories [best-values-example, custom-template, custom-value-notation-type, dos-line-endings, files-values, full-template, fully-documented, funky-version, helm-3, ignored-values-example, most-empty, nginx-ingress, nginx-ingress-but-auto-comments, no-requirements, no-values, special-characters, special-characters-but-auto-comments, umbrella, umbrella/charts/library, umbrella/charts/sub-a, umbrella/charts/sub-b, umbrella/charts/sub-c]
# Other things....
WARN[2023-10-13T09:59:51-05:00] Error parsing information for chart best-values-example, skipping: values without documentation: 
statefulset
template "extra.flower" not defined 
➜  helm-docs git:(master) ✗ echo $?
0

That exit value needs to be 1 since it failed. To use with a CI we need the proper exit value.

[Nepo26]

Hmm, that is correct. It reports the correct missing values but not returns the correct exit value, right?

[Nepo26]

It also seems like it is not returning all missing values.

Actually, I just tested here locally and it returns for all charts.

[edmondop]

Hey @Nepo26 its really easy to see. You can run it against the example charts, like so:

➜  helm-docs git:(master) ✗ helm-docs -x -c example-charts
INFO[2023-10-13T09:59:51-05:00] Found Chart directories [best-values-example, custom-template, custom-value-notation-type, dos-line-endings, files-values, full-template, fully-documented, funky-version, helm-3, ignored-values-example, most-empty, nginx-ingress, nginx-ingress-but-auto-comments, no-requirements, no-values, special-characters, special-characters-but-auto-comments, umbrella, umbrella/charts/library, umbrella/charts/sub-a, umbrella/charts/sub-b, umbrella/charts/sub-c]
# Other things....
WARN[2023-10-13T09:59:51-05:00] Error parsing information for chart best-values-example, skipping: values without documentation: 
statefulset
template "extra.flower" not defined 
➜  helm-docs git:(master) ✗ echo $?
0

That exit value needs to be 1 since it failed. To use with a CI we need the proper exit value.

The exit value seems to be a problem outside the change @jhoover4 . I went back and look to the change, and as you see here:

https://github.com/norwoodj/helm-docs/pull/181/files#diff-c53ebdb20abe148265a537c2829c1dfe7833427c2f94148cd936a54aa604644fR294-R315

the error is correctly returned upstream, but it doesn't seem the code above is handling the error and returning a -1.

The problem is in cmd, https://github.com/norwoodj/helm-docs/blob/master/cmd/helm-docs/main.go#L109 . Do we want to change this behavior? This wasn't a part of my PR and I honestly didn't even thought about checking

[Nepo26]

I think the correct behaviour would be to error every time that it errors, but I see it breaking some current flows.
The best approach I think is changing this behaviour on the next major change.

What do you think @edmondop and @norwoodj?

[edmondop]

I think the correct behaviour would be to error every time that it errors, but I see it breaking some current flows. The best approach I think is changing this behaviour on the next major change.

What do you think @edmondop and @norwoodj?

Yes that makes sense. Is there still a bug in validating or is the bug only in handling errors returned by the validating function @jhoover4 ?

[jhoover4]

I think there's a bug in both like you noticed @edmondop 😅. It spits out warnings when those should be errors (from what I'm getting from the docs)

[Bibz87]

Is strict linting supposed to work with the new-style comments or just the old-style one?

This fails:

# -- Number of replicas
replicaCount: 1

INFO[2023-10-18T10:40:58-04:00] Found Chart directories [foo]
WARN[2023-10-18T10:40:58-04:00] Error parsing information for chart foo, skipping: values without documentation: 
replicaCount

But this works:

# replicaCount -- Number of replicas
replicaCount: 1

INFO[2023-10-18T10:41:21-04:00] Found Chart directories [foo]
INFO[2023-10-18T10:41:21-04:00] Generating README Documentation for chart foo

[jhoover4]

Ah yeah Ive noticed that too. Where are we on this @edmondop @Nepo26? Yall need help?

[Bibz87]

Also, the Docker container doesn't seem to like the outputted error message:

time="2023-10-18T15:24:55Z" level=info msg="Found Chart directories [foo]"
time="2023-10-18T15:24:55Z" level=warning msg="Error parsing information for chart foo, skipping: values without documentation: \nreplicaCount"

[JuryA]

This novel feature is genuinely splendid! Despite minor issues with the exit code and the incorrect evaluation of the abbreviated format, I wholeheartedly appreciate it! Concerning the behavior (whether to terminate with an error or not, etc.), in my modest opinion, the optimal approach, particularly in terms of backward compatibility, would be to conclude with an error only if, for instance, an additional option is employed... Perhaps --fail, would trigger an error if documentation is absent for any of the mandatory values. I could also envision it being beneficial to have the ability to set a value in the manner of --fail=missing. Another possibility could be, for instance, --fail=lint in the case of a syntax error (currently, it is more or less discreetly disregarded), or --fail=always (I would maintain 'always' as the default for the option without an explicit value) for all scenarios, and so forth. What are your thoughts?

[jhoover4]

Hey guys just wanted to check in. Do we still need any help to get this done?

[Bibz87]

Hey guys just wanted to check in. Do we still need any help to get this done?

As it's been radio silence for a while, I guess the answer is yes? 🤔

[wolfwolker]

Same problem here, would be nice to have a non-zero exit code when there are errors running on strict mode.

I'm trying to workaround that on pre-commit hooks using this configuration:

  - repo: local
    hooks:
      - id: helm-docs-strict
        name: Helm Docs Strict
        language: system
        entry: /bin/sh
        args: [-c, "helm-docs --documentation-strict-mode 2>&1 | grep \"Error parsing information\";  exit $(($? == 0 ? 1 : 0))"]
        always_run: true
        pass_filenames: false
        require_serial: true