[WIP] Ansible-lint: Add example config for github action

[MarkusTeufelberger]

SUMMARY

resolves #611

ISSUE TYPE

• Feature Pull Request

COMPONENT NAME

Tests

ADDITIONAL INFORMATION

Let's see how this works out.

[MarkusTeufelberger]

Yay, already their documented example is broken:

"Error: Unable to resolve action ansible/ansible-lint@v6, unable to find version v6"

[MarkusTeufelberger]

There's also https://github.com/ansible/ansible-lint-action, maybe that's what they meant to use instead? Let's try...

[MarkusTeufelberger]

Ah well... "CRITICAL Linter finished without analyzing any file, check configuration and arguments given."

Seems like it still has its issues when running against a checked out collection even in a very default layout. Unfortunately the GH-Action seems quite limited when it comes to access to parameter, so setting it to "verbose" to find out what's going on sounds like quite the journey.

[MarkusTeufelberger]

I'm actually not sure what it should even lint. We don't provide playbooks or roles outside of tests, so no files to lint seems reasonable. I would expect it to lint galaxy.yml files though, no idea why it doesn't find the one that's right in the root of the repository. Asking in ansible/ansible-lint#3482 what's actually expected to happen and marking this as a draft in the meantime...

[felixfontein]

The linter mainly lints the YAML integration tests in tests/integration/targets/. It will produce a HUGE number of errors/warnings/...

My suggestion would be to simply ignore all errors for now.

(But for that it first has to actually run...)

[MarkusTeufelberger]

Wow, it is almost impressive how much it complains!

[MarkusTeufelberger]

I guess I can add ignores for everything and then start fixing stuff one-by-one either automatically (via ansible-lint --write) or by hand if automatic rewrites are not easy or available. At least the long tail of single digit errors should be doable in this PR I hope.

[felixfontein]

But only the ones that make sense. sanity[cannot-ignore] looks pretty much wrong, for example.

[MarkusTeufelberger]

I hope that the eventual goal is to have both the collection(s) and ansible-lint rules in a state that no rule skips or any changes to default configs are necessary. For sanity[cannot-ignore] for example I am hardlocked on ansible/ansible#79700 making it into ansible-core and every version of ansible-core falling out of support until I can disable this in my collection. Similar with the shebangs in this repo here apparently?

There are some easy wins with fixing the long tail of errors (end of line whitespace and such) and automatic rewrites could be used or introduced for some things too. There are however also some issues with that (e.g. ansible/ansible-lint#3226 will hit every collection created from the default template).

[MarkusTeufelberger]

I ran against 6.17.0 locally and it complains even more. Added a few quick fixes (on:in github workflow files is a truthy name unfortunately and order apparently is a reserved name by Ansible these days) and 2 more ignores for upcoming rules that were also quite noisy. Now I can also locally start disabling ignores one by one and fix them. First though I also want to get ansible-lint to be idempotent in --write mode (fixing or "fixing" lots of YAML formatting usually). That alone looks like it'll be a horrible thing to review and likely we'll be stuck with an .ansible-lint file in the root directory because it tries to "fix" the github workflow files too.

[MarkusTeufelberger]

For example

community.crypto/tests/integration/targets/x509_crl/tests/validate.yml

Line 110 in 0d30a37

- name: Validate CRL 3 info

gets butchered HARD and there's no real way not to run YAML re-formatting when using --write mode, but there's also no way that I'm adding hundreds of FQCNs by hand or semi-automated to every task when there's already a perfectly good transform available for that. Unfortunately there is no way currently to only do transforms as ansible-lint kinda depends on being able to completely rewrite everything first.

I guess we can add it as-is and fix a few low hanging fruits over time while keeping the big whoppers around until transforms are available or there is a way that formatting doesn't get completely smashed while keeping liniting available (and non-Ansible files are no longer re-formatted by ansible-lint). I don't really want to start adding tons of config other than rule ignores to the .ansible-lint file, I kinda expect it to work without hassle in a use case like this one (established collection with maybe some issues that wants to keep things from deteriorating and gradually improving).

[MarkusTeufelberger]

Looks like they want to use the action from https://github.com/ansible/ansible-lint/blob/main/action.yml actually, not the one from that other repository. I'll update this PR next week thusly.

The other common change in other collections seems to be to use an .ansible-lint-ignore file (https://ansible.readthedocs.io/projects/lint/configuring/#ignoring-rules-for-entire-files) instead of a config file in the root of the repository so new contributions would always have to adhere to all rules instead of allowing "bad" code in and fixing all instances of a rule violation eventually. There doesn't seem to be --write mode applied though.

[felixfontein]

Fine with me, as long as ignore-errors and sanity[cannot-ignore] stay permanently disabled in .ansible-lint .

[MarkusTeufelberger]

I'm not sure if it is better to kinda document all 533 rule violations and eventually fix them while not allowing new ones because there's not much to take inspiration from (aka. copy paste) when writing new tests for example. I don't see how current violations are much better or worthy of keeping around though on the other hand it just makes more work for the future to eventually fix all this rule by rule. In any case it is easy enough (and just tedious) to fix stuff, it would just be nice if we could actually use --write mode for this which is currently completely out of question. I now pushed the "minimal rule ignore config + centrally documented ignores + lint config as given in the main repo" version as a comparison.

[felixfontein]

Also considering that .ansible-lint-ignore is 41 KiB, I think the previous version of the PR was nicer - mainly for the reasons you mentioned :-)

[felixfontein]

A checkout step needs to be added before here. There was a breaking change in the action that started requiring that.

[felixfontein]

@MarkusTeufelberger do you want to further update this? Or should we restart this with a fresh PR? (Or abandon it for now?)

[MarkusTeufelberger]

I think I'll rebase and check with the current version.