rabbitmq: resolve internal_interface through Ansible

[ideaship]

Fixes the reported bug. Two commits: the reusable helper, then the switch-over.

The bug

get_rabbitmq_node_addresses() read internal_interface with ansible-inventory --host, which returns variables as defined, so a Jinja-valued internal_interface came back as the raw {{ ... }}. It then resolved that by hand: a regex captured the contents of the first {{ ... }} and the dotted path was walked through the ansible facts.

That covers exactly one shape — a single expression whose dotted path is rooted in facts, i.e. the testbed's internal_interface: "{{ ansible_local.testbed_network_devices.management }}" — and nothing else:

• An internal_interface pointing at an inventory variable fails, because the walk only looks in the facts: Could not resolve template '{{ some_var }}' from facts for <host>. This is what the reporter hit.
• A mixed literal and template such as vlan{{ vlan_id }} is passed through verbatim, because re.match requires {{ at offset 0; the lookup then asks for a fact named ansible_vlan{{ vlan_id }}.
• The regex is unanchored, so {{ base }}.100 matches only the leading expression and silently discards the .100 tail, resolving to the wrong interface — a wrong answer rather than an error.
• Filters, hostvars lookups and default() are not evaluated at all.

The fix

Delegate the lookup to Ansible, which is what osism/defaults all/README.md ("Consuming these values from code") prescribes for external consumers of these variables. The supported set becomes "whatever Jinja2 supports" instead of a list of anticipated shapes. The resolver goes away, along with the subsequent walk from interface name to ansible_<name> to ipv4.address, since the expression covers all of it.

resolve_in_host_context() has Ansible template the expression and copy the result into a file, run with -c local so the module executes on the controller: no connection is made, a node that is down still resolves, and the value is read back byte-exact rather than parsed out of human-readable output.

The package runs under more than one ansible-core — 2.19.11 in its own image, 2.18.x in the osism-ansible, kolla-ansible and ceph-ansible images — so the lookup avoids every surface where those differ:

• Facts travel as extra vars, not a fact-cache plugin. The redis plugin lives in community.general, which is not installed here, and a jsonfile cache is honoured by an ad-hoc lookup on 2.18 but not on 2.19.
• The value comes from a file, not a callback. Callback wording and stream differ between the two, and --tree is deprecated for removal in 2.23.
• Success is decided by the return code, never by matching an error string: the undefined-variable sentinel is VARIABLE IS NOT DEFINED! on 2.18 and <<error1-'x' is undefined>> on 2.19.

The resolved value is validated as an IPv4 address before use, so a non-address string cannot be passed on as one.

Coverage

The xfail(strict=True) case added in the previous PR passes now, so its marker goes — with strict set, leaving it would fail the suite on the unexpected pass. That transition is the demonstration: the shapes that already worked are still covered by the same tests, and the one that did not now passes against real Ansible.

The two remaining unit tests that described only the deleted resolver's internals (a facts-walk yielding a non-string, and hitting a non-dict) go with the code they covered — Ansible has no such failure modes, so there is nothing to express at the integration level.

Verification. Against ansible-core 2.18.9 and 2.19.11: the reported shape, a dotted interface name, a dashed one, the fact-derived shape the old resolver supported, and a missing fact.

One behaviour change for review: a missing internal_interface is now reported through Ansible's undefined-variable message rather than a dedicated one.

osism status rabbitmq shares the helper and is fixed with it.

Fixes:

• Jinja2 variables not rendered by rabbitmq3to4 issues#1425

🤖 Generated with Claude Code

[sourcery-ai]

Hey - I've found 1 security issue, and 1 other issue

Security issues:

• Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'. (link)

Fixed security issues:

• Command injection from untrusted input passed to OS command execution (link)

Prompt for AI Agents

Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="osism/utils/inventory.py" line_range="172" />
<code_context>
+            )
+
+        try:
+            with open(value_path) as fp:
+                return fp.read()
+        except OSError as exc:
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Open the result file with an explicit encoding to avoid locale-dependent behavior.

Using the platform default encoding is brittle for non-ASCII content and non‑UTF‑8 locales. Please specify `encoding="utf-8"` here and ensure the producing side also writes UTF‑8 so the encoding is consistent end to end.

Suggested implementation:

```python
        try:
            with open(value_path, encoding="utf-8") as fp:
                return fp.read()
        except OSError as exc:
            raise HostContextResolutionError("ansible wrote no value") from exc

```

To fully implement your suggestion end-to-end, you should also verify that the Ansible task or plugin producing `value_path` writes the file using UTF-8 encoding (e.g., ensure templates or modules use UTF-8 and that any explicit file writes specify `encoding: utf-8` or equivalent). Those changes will be in the Ansible side rather than in `osism/utils/inventory.py`.
</issue_to_address>

### Comment 2
<location path="osism/utils/inventory.py" line_range="150-156" />
<code_context>
            result = subprocess.run(
                command,
                capture_output=True,
                text=True,
                timeout=timeout,
                env=env,
            )
</code_context>
<issue_to_address>
**security (python.lang.security.audit.dangerous-subprocess-use-audit):** Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'.

*Source: opengrep*
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

suggestion (bug_risk): Open the result file with an explicit encoding to avoid locale-dependent behavior.

Using the platform default encoding is brittle for non-ASCII content and non‑UTF‑8 locales. Please specify encoding="utf-8" here and ensure the producing side also writes UTF‑8 so the encoding is consistent end to end.

Suggested implementation:

        try:
            with open(value_path, encoding="utf-8") as fp:
                return fp.read()
        except OSError as exc:
            raise HostContextResolutionError("ansible wrote no value") from exc

To fully implement your suggestion end-to-end, you should also verify that the Ansible task or plugin producing value_path writes the file using UTF-8 encoding (e.g., ensure templates or modules use UTF-8 and that any explicit file writes specify encoding: utf-8 or equivalent). Those changes will be in the Ansible side rather than in osism/utils/inventory.py.

[ideaship]

@sourcery-ai dismiss