rabbitmq: resolve internal_interface through Ansible

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 ever looks in the facts:
  "Could not resolve template '{{ some_var }}' from facts for <host>".
- A mixed literal and template such as "vlan{{ vlan_id }}" is passed
  through verbatim, because re.match requires "{{" at offset 0, and 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 defaults are not evaluated at all.

Use resolve_in_host_context() instead, so Ansible does the templating in
the host's own variable context. This is what osism/defaults
all/README.md ("Consuming these values from code") prescribes for
external consumers, and it makes the supported set "whatever Jinja2
supports" rather than a list of anticipated shapes. The whole resolver
goes away, along with the subsequent walk from interface name to
ansible_<name> to ipv4.address, since the expression covers all of it.

The resolved value is validated as an IPv4 address before use. The return
code already reports a templating failure, but a value that comes back
looking nothing like an address must not be passed on as one either.

The integration case for an internal_interface that points at an inventory
variable was marked xfail(strict) when it was added, because the resolver
could not resolve it. It passes now, so the marker goes: with strict set,
leaving it would fail the suite on the unexpected pass. That is the
demonstration this change needed -- the shapes that already worked are
still covered by the same tests, and the one that did not now passes
against real Ansible rather than against a mock.

The tests for the deleted resolver go with it: Jinja2 traversal, the
non-string and non-dict cases, the interface-name to fact-key mapping and
the ipv4 extraction all tested behaviour that is now Ansible's. What
replaces them asserts the contract that remains -- that the expression and
the cached facts are handed over unchanged, that a resolution failure
surfaces Ansible's own message, and that a non-address result is refused.

Verified against ansible-core 2.18.9 and 2.19.11 with the reported
variable shape ("{{ vlan_var }}" where vlan_var is itself
"vlan{{ id }}"), a dotted interface name, a dashed one, and the
fact-derived shape the old resolver supported, plus a missing fact.

One behaviour change worth noting 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.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Roger Luethi <luethi@osism.tech>

Author: ideaship

osism/utils/rabbitmq.py

@@ -1,13 +1,27 @@
 # SPDX-License-Identifier: Apache-2.0
 
+import ipaddress
 import json
 import os
-import re
 import subprocess
 
 from loguru import logger
 
-from osism.utils.inventory import get_hosts_from_inventory, get_inventory_path
+from osism.utils.inventory import (
+    HostContextResolutionError,
+    get_hosts_from_inventory,
+    get_inventory_path,
+    resolve_in_host_context,
+)
+
+# The node's internal address. Ansible names interface facts with "-" replaced
+# by "_" and dots left alone (PrefixFactNamespace._underscore), so "br-ex" is
+# ansible_br_ex while "bond0.100" is ansible_bond0.100.
+INTERNAL_ADDRESS_EXPRESSION = (
+    "hostvars[inventory_hostname]"
+    "['ansible_' + (internal_interface | replace('-', '_'))]"
+    "['ipv4']['address']"
+)
 
 
 def get_rabbitmq_node_addresses():
@@ -51,73 +65,31 @@ def get_rabbitmq_node_addresses():
 
                 facts = json.loads(facts_data)
 
-                # Get hostvars for this host to find internal_interface
+                # Resolve internal_interface and the address it carries in one
+                # templated lookup, so that any Jinja2 shape works -- not just
+                # the ones a hand-written resolver anticipated.
                 hostvar_inventory_path = get_inventory_path(
                     "/ansible/inventory/hosts.yml", prefer_minified=False
                 )
-                result = subprocess.check_output(
-                    f"ansible-inventory -i {hostvar_inventory_path} --host {host}",
-                    shell=True,
-                    stderr=subprocess.DEVNULL,
-                )
-                hostvars = json.loads(result)
-
-                internal_interface_raw = hostvars.get("internal_interface")
-                if not internal_interface_raw:
-                    logger.error(f"internal_interface not found in hostvars for {host}")
-                    continue
-
-                # Resolve Jinja2 template if present (e.g., "{{ ansible_local.testbed_network_devices.management }}")
-                internal_interface = internal_interface_raw
-                template_match = re.match(
-                    r"\{\{\s*(.+?)\s*\}\}", internal_interface_raw
-                )
-                if template_match:
-                    path = template_match.group(1).strip()
-                    parts = path.split(".")
-                    value = facts
-                    for part in parts:
-                        if isinstance(value, dict):
-                            value = value.get(part)
-                        else:
-                            value = None
-                            break
-                    if value and isinstance(value, str):
-                        internal_interface = value
-                    else:
-                        logger.error(
-                            f"Could not resolve template '{internal_interface_raw}' from facts for {host}"
-                        )
-                        continue
-
-                logger.debug(f"Internal interface for {host}: {internal_interface}")
-
-                # Look for the interface in ansible facts. Ansible replaces "-"
-                # with "_" in fact names and leaves dots alone
-                # (PrefixFactNamespace._underscore), so "br-ex" is
-                # ansible_br_ex while "bond0.100" is ansible_bond0.100.
-                normalized_interface = internal_interface.replace("-", "_")
-                interface_key = f"ansible_{normalized_interface}"
-
-                interface_facts = facts.get(interface_key)
-                if not interface_facts:
-                    logger.error(
-                        f"Interface {internal_interface} ({interface_key}) not found in ansible facts for {host}"
-                    )
-                    continue
-
-                # Get IPv4 address
-                ipv4_info = interface_facts.get("ipv4")
-                if not ipv4_info:
-                    logger.error(
-                        f"No IPv4 address found for interface {internal_interface} on {host}"
+                try:
+                    ipv4_address = resolve_in_host_context(
+                        host,
+                        INTERNAL_ADDRESS_EXPRESSION,
+                        hostvar_inventory_path,
+                        facts=facts,
                     )
+                except HostContextResolutionError as exc:
+                    logger.error(f"Could not resolve address for {host}: {exc}")
                     continue
 
-                ipv4_address = ipv4_info.get("address")
-                if not ipv4_address:
+                # A templating failure is reported by the return code, but a
+                # module that returns a non-address string must not be trusted
+                # either -- validate rather than pass it on as an address.
+                try:
+                    ipaddress.IPv4Address(ipv4_address)
+                except ValueError:
                     logger.error(
-                        f"No IPv4 address found for interface {internal_interface} on {host}"
+                        f"Resolved address for {host} is not an IPv4 address: {ipv4_address!r}"
                     )
                     continue
 

tests/integration/test_rabbitmq_addresses.py

@@ -134,15 +134,11 @@ def test_dashed_interface_name(scenario):
     assert rabbitmq.get_rabbitmq_node_addresses() == [("10.74.34.13", host)]
 
 
-@pytest.mark.xfail(
-    strict=True,
-    reason="internal_interface pointing at an inventory variable is not resolved; "
-    "the resolver only walks dotted paths through the facts (osism/issues#1425)",
-)
 def test_interface_from_inventory_variable(scenario):
-    # The shape reported by a client: internal_interface refers to an inventory
-    # variable, which is itself a literal plus a template. Nothing here is a
-    # fact, so a facts-only walk cannot resolve it.
+    # The shape reported in osism/issues#1425: internal_interface refers to an
+    # inventory variable, which is itself a literal plus a template. Nothing
+    # here is a fact, which is why resolving it needs Ansible's templating
+    # rather than a walk through the facts. Marked xfail until that landed.
     host = scenario(
         "ctl5",
         {