Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
116 changes: 116 additions & 0 deletions osism/utils/inventory.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,17 @@
# SPDX-License-Identifier: Apache-2.0

import json
import os
import subprocess
import tempfile

from loguru import logger


class HostContextResolutionError(Exception):
"""Raised when an expression cannot be templated in a host's context."""


def get_inventory_path(base_path: str, prefer_minified: bool = True) -> str:
"""Return the best available inventory path.

Expand Down Expand Up @@ -57,3 +64,112 @@ def get_hosts_from_inventory(data: dict) -> list:
if isinstance(value, dict) and "hosts" in value:
hosts.update(value["hosts"])
return sorted(hosts)


def resolve_in_host_context(
host: str,
expression: str,
inventory_path: str,
facts: dict | None = None,
timeout: int = 60,
) -> str:
"""Template a Jinja2 expression in a host's Ansible variable context.

``ansible-inventory --host`` returns variables *as defined*, so anything
Jinja-valued comes back as the raw ``{{ ... }}``. Resolving such a value
requires Ansible's own templating, in the host's variable context -- see
``osism/defaults`` ``all/README.md``, section "Consuming these values from
code". Re-implementing the templating in the consumer is not an option: it
only ever covers the shapes that were thought of.

The value is produced by having Ansible ``copy`` the templated expression
into a file, run with ``-c local`` so the module executes on the controller.
No connection is made to the host, so this works for hosts that are down or
unreachable, and the value is read back byte-exact instead of being parsed
out of human-readable output. That matters because the callback formats
differ between the ansible-core versions this package runs under, and
because ``--tree``, the other way to get structured output, is deprecated
for removal in ansible-core 2.23.

Facts are passed as extra vars rather than through a fact-cache plugin.
That keeps the call independent of which cache plugin is configured (the
``redis`` plugin lives in ``community.general``, which is not installed
here) and of the Ansible version: ansible-core 2.18 exposes cached facts to
an ad-hoc ``debug`` while 2.19 does not, whereas extra vars behave
identically on both. Extra vars outrank everything, which is what we want
for facts -- they already outrank host_vars in normal precedence.

Args:
host: Inventory hostname to evaluate the expression for.
expression: Jinja2 expression *without* the surrounding braces.
inventory_path: Inventory to resolve the host and its variables from.
facts: Ansible facts for the host, as stored in the fact cache.
timeout: Seconds to wait for Ansible.

Returns:
The templated value, as a string.

Raises:
HostContextResolutionError: If Ansible could not evaluate the
expression. The message carries Ansible's own explanation, which
names the undefined variable or attribute.
"""
env = os.environ.copy()
# The module runs locally, but be explicit: never gather facts, so a host
# that is down cannot turn a lookup into an SSH timeout.
env["ANSIBLE_GATHERING"] = "explicit"
env["ANSIBLE_RETRY_FILES_ENABLED"] = "False"
env["ANSIBLE_NOCOLOR"] = "1"

with tempfile.TemporaryDirectory(prefix="osism-resolve-") as workdir:
value_path = os.path.join(workdir, "value")
# JSON module args rather than key=value, so a value containing spaces
# survives.
module_args = json.dumps(
{"content": "{{ %s }}" % expression, "dest": value_path}
)
command = [
"ansible",
host,
"-i",
inventory_path,
"-c",
"local",
"-m",
"copy",
"-a",
module_args,
]
if facts:
facts_path = os.path.join(workdir, "facts.json")
with open(facts_path, "w") as fp:
json.dump(facts, fp)
command += ["-e", f"@{facts_path}"]

try:
result = subprocess.run(
command,
capture_output=True,
text=True,
timeout=timeout,
env=env,
)
Comment thread
sourcery-ai[bot] marked this conversation as resolved.
except subprocess.TimeoutExpired as exc:
raise HostContextResolutionError(
f"ansible timed out after {timeout}s"
) from exc

if result.returncode != 0:
# Ansible names the undefined variable, which is the most useful
# thing to pass on. Its wording and stream differ between versions,
# so take whichever of the two is non-empty and do not parse it.
detail = (result.stdout or "").strip() or (result.stderr or "").strip()
raise HostContextResolutionError(
detail or f"ansible exited {result.returncode}"
)

try:
with open(value_path) as fp:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

return fp.read()
except OSError as exc:
raise HostContextResolutionError("ansible wrote no value") from exc
96 changes: 34 additions & 62 deletions osism/utils/rabbitmq.py
Original file line number Diff line number Diff line change
@@ -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():
Expand Down Expand Up @@ -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

Expand Down
12 changes: 4 additions & 8 deletions tests/integration/test_rabbitmq_addresses.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
{
Expand Down
Loading