Fix Helm linting: treat rendered manifests as temporary#1299
Conversation
| } | ||
|
|
||
| pub trait Linter { | ||
| // The lint function takes path, rather than manifest. This is not ideal as it requires a tmp file |
There was a problem hiding this comment.
I'm not sure why you removed this comment?
This comment and link to the still unfixed bug in Argo CLI is important context.
| /// | ||
| /// This indirection allows unit tests to inject a fake Helm implementation | ||
| /// and avoids requiring the `helm` binary to be installed when running tests. | ||
| pub type HelmToManifestFn = |
There was a problem hiding this comment.
This PR includes a major refactor of the way that helm testing is done, and a bug fix.
The testing changes seem to be unnecessary for the bug fix?
Please consider splitting these into separate PRs.
| .filter_map(Result::ok) | ||
| .map(|entry| entry.path()) | ||
| // FILTER: skip Helm-generated temp files | ||
| .filter(|path| !is_generated_helm_file(path)) |
There was a problem hiding this comment.
I'm a bit confused.
If you filter out these generated files here,
When do these files generated from helm templates get linted?
|
To pass CI you need to change your commits to follow conventional commit (see: https://www.conventionalcommits.org/en/v1.0.0/ and the repo git commit history for examples). You also need to run And it looks like there is a build error in the tests. The |
Fix Helm linting so that rendered templates in /tmp/argo-lint are treated as internal artifacts and excluded from lint results.