Add role-scoped-variables ansible-lint rule

[ehelms]

Why are you introducing these changes? (Problem description, related links)

Roles currently reference shared variables (e.g. database_host, ca_certificate) directly, coupling them to the playbook's variable namespace. This makes it hard to tell which variables a role actually needs, creates implicit dependencies between roles, and means renaming a shared variable silently breaks every role that uses it.

This PR adds a custom ansible-lint rule that enforces role-scoped variables and fixes the existing violations in addressed roles.

What are the changes introduced in this pull request?

• New custom ansible-lint rule role-scoped-variables that flags variables inside role tasks not prefixed with the role name

  • Uses ansible-lint's nested_items_path to walk all task values — no manual key enumeration
  • Uses Jinja2 AST analysis to distinguish function calls (lookup, query, now) from variable references — no function allowlist needed
  • Allows: role-prefixed variables, ansible_* facts, Ansible builtins (item, omit, inventory_hostname, etc.), _-prefixed internal variables, and project globals
  • Test fixtures covering violation cases (module arg, when condition, loop, environment) and passing cases (role-prefixed, builtins, item, allowed globals, custom loop_var, task-local vars, underscore internal vars, function calls)

• Map shared variables to role-prefixed names at playbook invocation sites:

  • deploy.yaml, deploy-proxy.yaml, checks.yaml, backup.yaml, deploy-dev.yaml
  • Parent roles pass scoped vars through to sub-roles (e.g. checks passes checks_database_* to check_database_index)

• Rename role-internal variables (loop vars, include_tasks vars) to use leading underscore convention:

  • db_item to _db_item in check_database_connection
  • feature_name/feature_enabled to _feature_name/_feature_enabled in foreman_proxy
  • tuning_vars to _tuning_vars in check_system_requirements

• Remaining violations to address in follow-up: certificates (6), restore (26), and 7 dynamically-included check roles where execute_check.yml uses include_role: name: "{{ item }}" with no way to pass per-role vars

How to test this pull request

Steps to reproduce:

• Run rule unit tests: python -m pytest tests/ansible_lint/test_role_scoped_variables.py -vv
• Run ansible-lint on src: ANSIBLE_COLLECTIONS_PATH="$PWD/build/collections/foremanctl" ANSIBLE_COLLECTIONS_SCAN_SYS_PATH=false bash -c '(cd src && ansible-lint)'
• Run ansible-lint on development: ANSIBLE_COLLECTIONS_PATH="$PWD/build/collections/forge" ANSIBLE_COLLECTIONS_SCAN_SYS_PATH=false bash -c '(cd development && ansible-lint --exclude ../build/collections)'
• Deploy and verify functionality is unchanged: ./foremanctl deploy --foreman-initial-admin-password=changeme --tuning development

Checklist

• Tests added/updated (if applicable)
• Documentation updated (if applicable)

[ehelms]

I expect this to fail and will then need to apply a commit on top that fixes those. As I have kept seeing these in PRs and having to comment on them, I decided to create a rule instead. Right now there is only one exception obsah_state_path.

[evgeni]

Why do we allow any _ prefix var? The line before seems to say we only want _role_prefix to be allowed

[ehelms]

Because I was torn between whether we want to allow "internal" (_ prefixed) variables to exist however without convention other than being prefixed to allow potentially shorter names or if we want to enforce prefixing of the role name in all cases.

[evgeni]

those aren't variables, but functions.

[ehelms]

As in -- split the functions out to their own list and constant? I think the tricky part was how to only identify variables by the lint rule.

Should it instead only consider the first "token" before a pipe | ?

[evgeni]

Yeah, maybe not looking at the rest of a pipe is a good idea. There still can be some {{ a | combine(b) }} and we'd not notice the b being bad, but we probably also don't in the current code

[evgeni]

I said elsewhere that I like the rule, but dislike the code. I think it boils down to:

• The whole need to interact with Jinja2 directly (and doing weird things for implicit/explicit templates)
• The fact that this looks at a fixed set of places where variables can show up (loop etc), making it a bit of whack-a-mole
• The fact that we need to ignore special terms that are no real variables (lookup etc), making it again a whack-a-mole

I wonder if there is a better way to obtain this information from Ansible (instead of Jinja).
If there isn't - fine, but we should try :D

Also, we might have the same problem in template files, which are uncovered today too. (Not saying it belongs into this PR!).

[ehelms]

I said elsewhere that I like the rule, but dislike the code. I think it boils down to:

• The whole need to interact with Jinja2 directly (and doing weird things for implicit/explicit templates)

I think this is unavoidable as that's what Ansible lint rules do, but I did include updates to make how it interacts more clean with the standard ansible lint rules.

• The fact that this looks at a fixed set of places where variables can show up (loop etc), making it a bit of whack-a-mole
• The fact that we need to ignore special terms that are no real variables (lookup etc), making it again a whack-a-mole

I wonder if there is a better way to obtain this information from Ansible (instead of Jinja). If there isn't - fine, but we should try :D

Across the board I think this should be better relying on more builtin methods from Ansibles python code.

Also, we might have the same problem in template files, which are uncovered today too. (Not saying it belongs into this PR!).