feat: add helm lint

[ThomasSanson]

close #429

How to test ?

create an alias :

# run container as command
alias helm="docker run -ti --rm -v $(pwd):/apps -w /apps \
    -v ~/.kube:/root/.kube -v ~/.helm:/root/.helm -v ~/.config/helm:/root/.config/helm \
    -v ~/.cache/helm:/root/.cache/helm \
    alpine/helm"

and lint bad Chart :

helm lint .automation/test/kubernetes/helm_lint/bad/

and lint good Chart :

helm lint .automation/test/kubernetes/helm_lint/good/

[nvuillam]

This PR seems promising :)
Don't worry, the CI jobs will test :)

[bdovaz]

@ThomasSanson if you want to be more agile and not rely on CI that takes more than half an hour to run, follow these steps:

https://megalinter.io/beta/contributing/#execute-linter-tests-inside-the-container

[ThomasSanson]

I fixed the problem with the missing tag in the Dockerfile (in the descriptor section) and added the word "subcharts" to the cspell configuration for the helm lint options.

I created a script like this:

docker buildx build -f linters/kubernetes_helm_lint/Dockerfile . --tag kubernetes_helm_lint
TEST_KEYWORDS_TO_USE="kubernetes_helm_lint"
docker run -e TEST_CASE_RUN=true -e OUTPUT_DETAIL=detailed -e TEST_KEYWORDS="${TEST_KEYWORDS_TO_USE}" -e MEGALINTER_VOLUME_ROOT="." -v "/var/run/docker.sock:/var/run/docker.sock:rw" -v $(pwd):/tmp/lint kubernetes_helm_lint

But when I run it, I get an error that I don't know how to fix:

=========================== short test summary info ============================
FAILED megalinter/tests/test_megalinter/linters/kubernetes_helm_lint_test.py::kubernetes_helm_lint_test::test_failure
FAILED megalinter/tests/test_megalinter/linters/kubernetes_helm_lint_test.py::kubernetes_helm_lint_test::test_get_linter_help
FAILED megalinter/tests/test_megalinter/linters/kubernetes_helm_lint_test.py::kubernetes_helm_lint_test::test_success
====== 3 failed, 1 passed, 3 skipped, 841 deselected, 2 warnings in 2.08s ======
Pytest exited 1
Error(s) found by Pytest"

Did I make a mistake in setting up tests under .automation/test/kubernetes/helm_lint/{bad,good} ?

[bdovaz]

@ThomasSanson in the log generated by the Docker container you should have more information, take a closer look, don't just look at the last messages, above you should see stacktraces, etc.

[ThomasSanson]

I figured out how to make Helm lint work.

Previously, I was using the dockerfile option, but now I'm using apk.

However, I'm running into an issue with Helm lint because it only accepts a PATH as an argument, not individual files, which is causing it to fail.

Here's an example error message:

Error unable to check Chart.yaml file in chart: stat /tmp/lint/.automation/test/kubernetes/kubernetes/kubeval_good_1.yaml/Chart.yaml: not a directory

Can you guide me on how to ignore the file and just lint the directory instead ?

I'll deal with this tomorrow.

[ThomasSanson]

@bdovaz @nvuillam

Can you guide me on how to ignore the file and just lint the directory instead please ?

[ThomasSanson]

ok, I found the option cli_lint_mode: project

[nvuillam]

If you want the linter to run on a folder, yes that's the way to do it :)
About extends issues, they have been solved in another PR by @bdovaz , if you merge main branch in your branch they will pass :)

[nvuillam]

@ThomasSanson why creating a new descriptor for HELM ?
As it is a subset of kubernetes, it was at the good place in kubernetes descriptor ^^
Note: You can redefine properties at linter level any prop defined on descriptor level

[ThomasSanson]

@nvuillam I am creating a new descriptor because it's difficult to add Helm to the Kubernetes descriptor for me

With all due respect, I have been facing some difficulties while contributing to your project, and I believe it would be helpful if you could review and improve the "CONTRIBUTING" section to make it more welcoming and user-friendly for contributors. As we all know, contributing to open-source projects is voluntary, and it would be great to have a more straightforward and streamlined process for new contributors.

I have just pushed the code, but I am still encountering some errors. If you have any suggestions or guidance to offer, I would greatly appreciate it.

[nvuillam]

@ThomasSanson I understand your frustration, CI jobs are too long, MegaLinter is none of us's main job so we are not always available for fast support ^^

I've been busy these days because of my CTO job at Cloudity and an article I'm writing about sfdx-hardis for https://salesforcedevops.net , but I promise I'll try to be more reactive about your PR, especially knowing that I'd love to see it merged :)

About contributing... there is already many infos, but we're of course accepting any enhancements before we find the time to do it ourselves ^^
cc @bdovaz @Kurt-von-Laven @echoix

[Kurt-von-Laven]

@ThomasSanson, it may help to look at a previous PR that added a new linter as a reference? I know @bdovaz has improved the contributing guide recently based on issues he encountered, and @echoix added the ability to run slash commands by commenting on PRs. If you have more specifics about what issues you encountered, that will help.

[echoix]

@nvuillam Hence why I wanted #2381 to be merged quickly, it was ready at the beginning of the week. The goal there was to help accept more easily incomplete PRs, reducing the complexity for new contributors.

And for me, since there are some code-only changes that could be done on my cellphone, and the redundant syncing all files could be done without booting up my full environment when I don't have much time to help :)

[ThomasSanson]

@nvuillam @Kurt-von-Laven @echoix @bdovaz

Thank you for all the provided information. I understand that we all lead busy lives, and it can be challenging to find time to contribute to open-source projects

When I mention that it would be beneficial to have more detailed contribution documentation, I am willing to participate and make the exercise easier for contributors. However, for that, I need a concrete understanding of how everything works and to find the simplest method to add a linter

After reviewing other pull requests, I find it quite challenging to navigate through all the modified files

If I understand correctly, the current issue is related to the Salesforce linter ? However, if I have missed something, please let me know

I am currently connected via VPN and will only be able to resume work this evening

[nvuillam]

@ThomasSanson yep, the current issue is related to sfdx-scanner that is broken -> forcedotcom/sfdx-scanner#986

They should probably release a new version soon, as everyone is complaining it breaks their CI ^^

You are very welcome to propose better documentation, and if you need guidance about how MegaLinter works, "on peut même le faire en français si tu veux" ;)

[nvuillam]

About the many files, most of them are automatically generated by build :)

Why the helm.sh at the root of the repo ? that's what makes MegaLinter fail because of shellcheck

[ThomasSanson]

@nvuillam Oh damn, I didn't see that in my logs.

The helm.sh file wasn't supposed to be committed, my bad, I'll remove it tonight.

But just to make sure, is the content of the helm.sh file what we should use for testing?

[nvuillam]

I've never used helm, can't answer ^^
I know that you need good and bad files in a test folder in .automation/tests :)

[ThomasSanson]

@nvuillam No, no, the helm.sh file contains the code to test the helm linter based on :

https://megalinter.io/beta/contributing/#execute-linter-tests-inside-the-container:~:text=Execute%20the%20following%20commands%20in%20a%20*.sh%20script.%20Example%3A

[Kurt-von-Laven]

After reviewing other pull requests, I find it quite challenging to navigate through all the modified files

I wonder how much of this is because of the changes that are produced by automation. Maybe we should document which files are automatically generated in the contributing guide since this has been a common point of confusion. Sometimes people (including myself) make the mistake of disregarding the comments at the tops of these files and attempt to modify them directly.

[bdovaz]

After reviewing other pull requests, I find it quite challenging to navigate through all the modified files

I wonder how much of this is because of the changes that are produced by automation. Maybe we should document which files are automatically generated in the contributing guide since this has been a common point of confusion. Sometimes people (including myself) make the mistake of disregarding the comments at the tops of these files and attempt to modify them directly.

The same thing happened to me.

Initially in my first PR I started editing files that were auto-generated. This is a clear point of improvement and documentation improvement.

[ThomasSanson]

#2399

I will deal with the documentation issue later.

For now, my priority is to understand how everything works in practice.

Since I am still on VPN, I will try to remove 'helm.sh' directly through GitHub.

[ThomasSanson]

@nvuillam , I reviewed the tests (folder) for the Kubernetes linters to make them closer to reality. I sorted all the files into good/bad directories and moved the files corresponding to kubeval and kubeconform into the templates folder for each. This way, we have the Chart.yml files at the root of each group

I was able to test it properly by implementing @bdovaz 's script, whom I thank again. I'm starting to understand better how it all works, and in this case, the problem is mainly related to the organization of the tests

I also modified the regex for kubeval and kubeconform so that they use the kind rather than the apiVersion, as Chart.yml doesn't have a kind, so it shouldn't be linted with kubeval and kubeconform

This doesn't prevent work from needing to be done on the contribution documentation (in progress #2399)

In principle, everything should work correctly now

Thank you for your support

[ThomasSanson]

@nvuillam pipeline failed... snif, I'll look tomorrow

[bdovaz]

You are very close, you have passed the linter tests, now all that remains is to solve the problems with the KUBERNETES linters.

[ThomasSanson]

@bdovaz do you know how to test locally ? I mean, everything

because when I run the script for the kubernetes linters, it passes...

this one : #2399 (comment)

all good for :

• kubeval
• kubeconform
• helm

[nvuillam]

@ThomasSanson if you don't want to mess with other kubernetes linters, just put your test files in a kubernetes_helm folder, and add test_folder: kubernetes_helm in the helm linter descriptor :)

[bdovaz]

@ThomasSanson download the artifact and look at the logs with ERROR- in front which are the linters that fail.

https://github.com/oxsecurity/megalinter/actions/runs/4291864006

@nvuillam but those errors are generated by this PR?

[nvuillam]

@bdovaz > It's because the parent descriptor has been updated... before that, kubernetes directory had to be identified to trigger kubernetes linters, but without this one, now any yml file is scanned

@ThomasSanson > would it possible to override these properties at helm level in the descriptor instead of updating the parent ones ?

[ThomasSanson]

@nvuillam Ok, I understand better thanks

The missing option should be test_folder, I'll do that tomorrow, as I can't today

I'll start from the beginning, it will be faster

[ThomasSanson]

@nvuillam @bdovaz

Regarding Helm, it is not an integral part of Kubernetes, which means that it is possible to use Kubernetes without using Helm. However, the reverse is not true. Currently, the Kubernetes check group fails when running the Helm linter check if it cannot find a Chart.yml file.

Therefore, I am inclined to remove it from Kubernetes. Alternatively, we could consider making the linter optional, but I am not sure how to do that. If you have any advice on this, I would appreciate it.

In summary, we have two options:

• either we completely remove the Helm linter from the Kubernetes check group
• or we leave it, but disable it by default.

What do you think about this ?

[bdovaz]

If the second option is chosen, each linter has a disabled field:

https://megalinter.io/latest/json-schemas/descriptor.html

Let's see what @nvuillam thinks

[ThomasSanson]

Thanks @bdovaz because I discovered the option active_only_if_file_found.

This way, the linter will only be triggered if it finds a file named Chart.yml or Chart.yaml.

This solution solves all my problems since these files are only required for Helm.

[nvuillam]

Seems great but plz check comments :)

[nvuillam]

Why not using cli_version_arg_name: version ?
With a hardcoded version, when there will be a new one we won't see it
You may need to define version_extract_regex if the default semver regex does not work

[ThomasSanson]

feat: use cli_version_arg_name for helm linter (autoupdate version)

I leave it to you to resolve this conversation if you wish.

[nvuillam]

@ThomasSanson solve conflict in CHANGELOG, then once CI is ok it's ok for me :)

[nvuillam]

Great :)

We plan a MegaLinter release today and I'd prefer that helm lint would be tested a while with beta version, so I'll merge this great PR just after the release :)

[nvuillam]

@ThomasSanson we did not forget about you, I merged conflicts , if CI is ok we'll merge the PR :)

[nvuillam]

The CI error is not your fault, we'll fix it in another PR then we'll be able to merge this one :)

[nvuillam]

@ThomasSanson here we are finally :)
Wait some half an hour for beta to be built, and you'll be able to test helm lint with megalinter beta version :)

Thanks again for this great PR :)