tarioch / flux-check-hook

MIT License
30 stars 6 forks source link

linting kustomization patches results in unhandled Exception #4

Closed tyriis closed 1 year ago

tyriis commented 1 year ago

First of all, thank you for your efforts @tarioch. I am currently testing your work and run into an issue.

When I validate my flux repository i get an Exception because the validator does not recognise a kustomization patch overlay.

This is fine, I can just exclude it! But it fails without revealing where the problem has occured:

Traceback (most recent call last):
  File "/***/py_env-python3.10/bin/check-flux-helm-values", line 8, in <module>
    sys.exit(main())
  File "/***/py_env-python3.10/lib/python3.10/site-packages/pre_commit_flux/check_flux_helm_values.py", line 13, in main
    _validateFile(arg, repos)
  File "/***/py_env-python3.10/lib/python3.10/site-packages/pre_commit_flux/check_flux_helm_values.py", line 52, in _validateFile
    chartVersion = chartSpec["version"]
KeyError: 'version'
Traceback (most recent call last):
  File "/***/py_env-python3.10/bin/check-flux-helm-values", line 8, in <module>
    sys.exit(main())
  File "/***/py_env-python3.10/lib/python3.10/site-packages/pre_commit_flux/check_flux_helm_values.py", line 13, in main
    _validateFile(arg, repos)
  File "/***/py_env-python3.10/lib/python3.10/site-packages/pre_commit_flux/check_flux_helm_values.py", line 46, in _validateFile
    chartSpec = definition["spec"]["chart"]["spec"]
KeyError: 'chart'

quick checked (and chnaged) the code to get the file

def main():
    repos = _buildRepoMap()
    for arg in sys.argv[1:]:
        try:
            _validateFile(arg, repos)
        except Exception:
            raise Exception(arg)

I would expect to get a result like this (can differ but the end user should not be required to read python stacktraces or change code):

[ERROR] path/to/my/file: invalid HelmRelease `version` is not provided but should.

you can checkout my use-case here: https://github.com/tyriis/flux.k3s.cluster/tree/main/cluster/apps/home/home-assistant

tarioch commented 1 year ago

That sounds useful, would you mind creating a pr for it?

tyriis commented 1 year ago

@tarioch I can, but the code I provided was just to debug the failing file, a report would be more useful. As far as I can see there is nothing like a report in the code. I am not sure how to do this in python, have not really worked with python since a long time. It would be required for other "Error" cases as well to stay consistent.

If it is fine we could discuss the implementation before I start.

I would go with something like the above mentioned message. My approach would be to collect all errors, and print the list at the end. (If no error occur, a success message get printed or just nothing with exit code 0). Any Error would create exit code 1.

Currently I run in an issue with validating a helm chart. Will maybe need to take a closer look at your implementation.

Let me know if I should create a PR for the mentioned (Error collecting and print). Or if you have another idea.

tarioch commented 1 year ago

Thank you very much @tyriis yes, collecting all the errors makes a lot of sense. Also the exit code is what is needed as far as I know. As far as I remember, pre-commit will show the stdout/stderr if exit code is not 0 and knows that this check failed, so that works then perfectly.

tyriis commented 1 year ago

@tarioch PR is open for review

just tested a few things: there is room for improvement concerning the helm errors

Check flux helm values......................................................Failed
- hook id: check-flux-helm-values
- exit code: 1

[ERROR] csi-driver-nfs-v4.1.0.tgz: ==> Linting csi-driver-nfs-v4.1.0.tgz
[INFO] Chart.yaml: icon is recommended
[ERROR] templates/rbac-csi-nfs.yaml: unable to parse YAML: invalid Yaml document separator: kind: ClusterRole

Error: 1 chart(s) linted, 1 chart(s) failed

[ERROR] cluster/apps/home/home-assistant/patches/db-init.yaml: KeyError ('chart',)
tarioch commented 1 year ago

Merged and released, thank you once again.

tyriis commented 1 year ago

@tarioch your welcome. Thank you for your work :)