Add CI check for variable/dictionary keys/filenames compliance

[tyll]

As reported in #189, dictionary keys and variables should be named according to the following rules:

• use only ASCII letters and numbers or the underscore sign
• the first character must not be a number

The CI check should check the following:

• example files including the used variables and dictionary keys
• the examples in the README
• the identifier keys in the README
• the names of all yaml files (*.yml)
• all variables used in other yaml files (defaults, tasks, ...)
• the accepted values for the role module (use parse_validator from scripts/print_all_options.py) (to be merged in Add script to print all options the role accepts #208)

[Precious000]

Hi there!
Can you please assign this project to me? I’ll like to work on it.

[richm]

@tyll I don't think this issue is entirely valid anymore - can we close it or substantially change it?

[tyll]

@tyll I don't think this issue is entirely valid anymore - can we close it or substantially change it?

What changes do you propose @richm?

[richm]

@tyll I don't think this issue is entirely valid anymore - can we close it or substantially change it?

What changes do you propose @richm?

So all of these are still valid?

Then the only change I would propose to the requirements is that the .yaml suffix is now invalid - all YAML encoded files must have a .yml prefix.

[spetrosi]

Role argument validation might help, e.g. for string variables that expect specific values.

[tyll]

@tyll I don't think this issue is entirely valid anymore - can we close it or substantially change it?

What changes do you propose @richm?

So all of these are still valid?

The naming rules are a requirement that came from the system roles team, so you would need to tell me.

Then the only change I would propose to the requirements is that the .yaml suffix is now invalid - all YAML encoded files must have a .yml prefix.

I removed the "yaml" suffix. The places to check still look valid. The general problem that this is supposed to solve is to codify code policies (such as the variable naming) into CI checks so they are immediately found and it does not depend on the right person doing the review. If we have some other tool that does this check already, then this would be invalid. In general the CI task might be useful for other system roles as well, except they might have a different architecture (no custom module), which would make it more complex.

[richm]

@tyll I don't think this issue is entirely valid anymore - can we close it or substantially change it?

What changes do you propose @richm?

So all of these are still valid?

The naming rules are a requirement that came from the system roles team, so you would need to tell me.

Then the only change I would propose to the requirements is that the .yaml suffix is now invalid - all YAML encoded files must have a .yml prefix.

I removed the "yaml" suffix. The places to check still look valid. The general problem that this is supposed to solve is to codify code policies (such as the variable naming) into CI checks so they are immediately found and it does not depend on the right person doing the review. If we have some other tool that does this check already, then this would be invalid.

Seems like ansible-lint and ansible-test should check for anything/everything that Ansible considers to be invalid. I don't think they check for all of these, so having someone see what ansible-lint/ansible-test check for, then adding another check to cover the gaps would be useful.

In general the CI task might be useful for other system roles as well, except they might have a different architecture (no custom module), which would make it more complex.

The CI task would be useful for all system roles.

@Precious000 thank you for your interest in this issue - you do not have to be assigned in order to work on the issue, as I don't think anyone else will be working on it. You can submit a pull request. If you still want to be assigned, please let me know. You have offered to work on other issues, so perhaps you should decide on which one you would like to work on first, then you can be assigned to one of them.

[tyll]

Seems like ansible-lint and ansible-test should check for anything/everything that Ansible considers to be invalid. I don't think they check for all of these, so having someone see what ansible-lint/ansible-test check for, then adding another check to cover the gaps would be useful.

I just realized the background here was that the network role module accepted a dictionary key in network_connections. AFAIU, ansible-lint/ansible-test probably stops their checks at whether network_connections is a valid variable name. So they key check is

• the accepted values for the role module (use parse_validator from scripts/print_all_options.py)