diff --git a/steward/ansible/executor.py b/steward/ansible/executor.py index c2fdefa..4fbdbff 100644 --- a/steward/ansible/executor.py +++ b/steward/ansible/executor.py @@ -316,9 +316,13 @@ def build_extra_vars_file(merged: dict | None, tmpdir: str) -> tuple[list[str], def ansible_env(creds: dict | None, base_env) -> dict: - """Return a copy of base_env with ANSIBLE_HOST_KEY_CHECKING set from creds.""" + """Return a copy of base_env with ANSIBLE_HOST_KEY_CHECKING set from creds, + and the interpreter found quietly (#5334): Ansible still discovers each + host's Python, but no longer warns about it on every run. A target or group + that sets ansible_python_interpreter still wins.""" env = dict(base_env) env["ANSIBLE_HOST_KEY_CHECKING"] = "True" if (creds or {}).get("host_key_checking") else "False" + env.setdefault("ANSIBLE_PYTHON_INTERPRETER", "auto_silent") return env diff --git a/steward/ansible/inventory_gen.py b/steward/ansible/inventory_gen.py index 2dcce90..2c4ec76 100644 --- a/steward/ansible/inventory_gen.py +++ b/steward/ansible/inventory_gen.py @@ -1,6 +1,36 @@ """Ansible --list inventory generation from DB records.""" from __future__ import annotations +import re + +_UNSAFE = re.compile(r"[^a-z0-9_]+") + + +def ansible_group_name(name: str) -> str: + """A Steward group's name as Ansible accepts it (#5334): lower case, each + run of anything but letters, digits and _ made one _, never starting with + a digit. "Docker Swarm" is docker_swarm, which a play can name in + `hosts:`; the display name stays as typed everywhere else.""" + safe = _UNSAFE.sub("_", name.lower()).strip("_") or "group" + return f"g_{safe}" if safe[0].isdigit() else safe + + +def _group_names(targets: list) -> dict[str, str]: + """{display name: Ansible name} for every group these targets are in. Two + names that clean to the same thing ("Web servers", "web-servers") get _2, + _3 in name order, so neither group's hosts or vars fold into the other's.""" + out: dict[str, str] = {} + taken: set[str] = set() + for name in sorted({g.name for t in targets for g in t.groups}): + base = safe = ansible_group_name(name) + n = 1 + while safe in taken or safe in ("all", "ungrouped", "_meta"): + n += 1 + safe = f"{base}_{n}" + taken.add(safe) + out[name] = safe + return out + def generate_inventory(targets: list) -> dict: """Build an Ansible --list JSON inventory from a list of AnsibleTarget objects. @@ -13,6 +43,7 @@ def generate_inventory(targets: list) -> dict: group vars (sorted alphabetically by group name) → target ansible_vars → ansible_host """ inv: dict = {"all": {"hosts": []}, "_meta": {"hostvars": {}}} + names = _group_names(targets) for target in targets: inv["all"]["hosts"].append(target.name) @@ -27,12 +58,13 @@ def generate_inventory(targets: list) -> dict: inv["_meta"]["hostvars"][target.name] = merged for group in target.groups: - if group.name not in inv: - inv[group.name] = { + key = names[group.name] + if key not in inv: + inv[key] = { "hosts": [], "vars": dict(group.ansible_vars or {}), } - inv[group.name]["hosts"].append(target.name) + inv[key]["hosts"].append(target.name) return inv diff --git a/steward/ansible/inventory_routes.py b/steward/ansible/inventory_routes.py index 4b4bb8d..8839d2e 100644 --- a/steward/ansible/inventory_routes.py +++ b/steward/ansible/inventory_routes.py @@ -8,6 +8,7 @@ from sqlalchemy import select from sqlalchemy.orm import selectinload from steward.ansible import guards +from steward.ansible.inventory_gen import ansible_group_name from steward.auth.middleware import require_role from steward.core.audit import audit_request as _audit from steward.models.ansible_inventory import AnsibleGroup, AnsibleTarget @@ -149,6 +150,7 @@ async def group_detail(group_id: str): return await render_template( "ansible/inventory/group_detail.html", group=group, + ansible_name=ansible_group_name(group.name), used=used, all_targets=all_targets, member_ids=member_ids, diff --git a/steward/templates/ansible/inventory/group_detail.html b/steward/templates/ansible/inventory/group_detail.html index a5fb922..430a662 100644 --- a/steward/templates/ansible/inventory/group_detail.html +++ b/steward/templates/ansible/inventory/group_detail.html @@ -10,12 +10,14 @@ {% set can_edit = session.user_role in ('operator', 'admin') %} {{ ansible_tabs("inventory") }} {{ page_header(group.name) }} +{#- The name a play uses for it (#5334): Ansible takes only letters, digits and _. #}

{%- set members = group.targets|sort(attribute='name') %} {%- if members %}{{ members|length }} member{{ 's' if members|length != 1 }}: {% for t in members %}{{ t.name }}{{ ", " if not loop.last }}{% endfor %} {%- else %}no members{% endif %} + · plays call it {{ ansible_name }} {%- if used %} · scheduled by {% for s in used %}{{ s.name }}{{ ", " if not loop.last }}{% endfor %}{% endif %}

diff --git a/tests/core/test_ansible_inventory_page.py b/tests/core/test_ansible_inventory_page.py index 25ed180..57c5286 100644 --- a/tests/core/test_ansible_inventory_page.py +++ b/tests/core/test_ansible_inventory_page.py @@ -102,11 +102,13 @@ async def test_a_group_page_names_its_members_and_what_runs_on_it(app): async with app.test_request_context("/ansible/inventory/groups/g1"): session["user_role"] = "operator" page = await render_template( - "ansible/inventory/group_detail.html", group=swarm, used=[NS(id="s1", name="Nightly prune")], + "ansible/inventory/group_detail.html", group=swarm, ansible_name="docker_swarm", + used=[NS(id="s1", name="Nightly prune")], all_targets=[d2, util], member_ids={"t1"}, ansible_vars_yaml="") main = page[page.index("VDNT-Docker02' in main assert 'scheduled by Nightly prune' in main + assert "plays call it docker_swarm" in main # #5334 # Deleting a group a schedule runs on says so before it goes. assert 'data-confirm="Delete group Docker Swarm? 1 schedule aimed at it would find nothing to run on."' in main assert not hex_colours(main), hex_colours(main) diff --git a/tests/core/test_inventory_gen.py b/tests/core/test_inventory_gen.py index bf2e357..fc829c8 100644 --- a/tests/core/test_inventory_gen.py +++ b/tests/core/test_inventory_gen.py @@ -138,3 +138,30 @@ def test_describe_scope_names_what_a_run_targeted(): assert describe_scope("steward:group:g1", targets, groups) == "group Docker Swarm" assert describe_scope("steward:target:abcdef0123", targets, groups) == "target abcdef01 (deleted)" assert describe_scope("repo:homelab:hosts.yml", targets, groups) == "homelab/hosts.yml" + + +# ── Group names Ansible accepts (#5334) ─────────────────────────────────────── + +def test_a_group_name_is_cleaned_for_ansible(): + from steward.ansible.inventory_gen import ansible_group_name + assert ansible_group_name("Docker Swarm") == "docker_swarm" + assert ansible_group_name("web-servers (prod)") == "web_servers_prod" + assert ansible_group_name("2nd floor") == "g_2nd_floor" + assert ansible_group_name("!!!") == "group" + assert ansible_group_name("already_fine") == "already_fine" + + +def test_the_inventory_uses_the_clean_names_and_keeps_clashing_groups_apart(): + swarm = _group("Docker Swarm", {"role": "swarm"}) + a, b = _group("Web servers", {"tier": "a"}), _group("web-servers", {"tier": "b"}) + inv = generate_inventory([_target("d1", "10.0.0.1", groups=[swarm]), + _target("w1", "10.0.0.2", groups=[a]), + _target("w2", "10.0.0.3", groups=[b])]) + assert "Docker Swarm" not in inv and inv["docker_swarm"]["hosts"] == ["d1"] + assert inv["docker_swarm"]["vars"] == {"role": "swarm"} + # "Web servers" sorts before "web-servers", so it keeps the plain name. + assert inv["web_servers"] == {"hosts": ["w1"], "vars": {"tier": "a"}} + assert inv["web_servers_2"] == {"hosts": ["w2"], "vars": {"tier": "b"}} + import yaml + children = yaml.safe_load(inventory_to_yaml(inv))["all"]["children"] + assert set(children) == {"docker_swarm", "web_servers", "web_servers_2"} diff --git a/tests/integration/test_ansible_inventory.py b/tests/integration/test_ansible_inventory.py index 711f4e0..38db75c 100644 --- a/tests/integration/test_ansible_inventory.py +++ b/tests/integration/test_ansible_inventory.py @@ -109,7 +109,7 @@ def test_ansible_run_scope_column_exists(): def test_create_target_and_group_and_fetch(): """Can create a target+group, assign membership, and fetch via fetch_scope_targets.""" from steward.models.ansible_inventory import AnsibleTarget, AnsibleGroup - from steward.ansible.inventory_gen import fetch_scope_targets, generate_inventory + from steward.ansible.inventory_gen import ansible_group_name, fetch_scope_targets, generate_inventory app = _make_app() target_id = str(uuid.uuid4()) @@ -157,7 +157,7 @@ def test_create_target_and_group_and_fetch(): targets = await fetch_scope_targets(db, f"steward:group:{group_id}") inv = generate_inventory(targets) assert target_name in inv["all"]["hosts"] - assert target_name in inv[group_name]["hosts"] + assert target_name in inv[ansible_group_name(group_name)]["hosts"] # test_group_… (#5334) assert inv["_meta"]["hostvars"][target_name]["ansible_host"] == "10.0.99.1" assert inv["_meta"]["hostvars"][target_name]["ansible_user"] == "ubuntu" assert inv["_meta"]["hostvars"][target_name]["env"] == "test" diff --git a/tests/test_ansible_credentials.py b/tests/test_ansible_credentials.py index 9311c50..beda3a6 100644 --- a/tests/test_ansible_credentials.py +++ b/tests/test_ansible_credentials.py @@ -49,6 +49,10 @@ def test_ansible_env_host_key_checking_toggle(): assert ansible_env({"host_key_checking": True}, {})["ANSIBLE_HOST_KEY_CHECKING"] == "True" assert ansible_env({"host_key_checking": False}, {})["ANSIBLE_HOST_KEY_CHECKING"] == "False" assert ansible_env({}, {})["ANSIBLE_HOST_KEY_CHECKING"] == "False" + # Interpreter discovery stays, without its warning on every run (#5334); + # an interpreter set in the environment is left as it is. + assert ansible_env({}, {})["ANSIBLE_PYTHON_INTERPRETER"] == "auto_silent" + assert ansible_env({}, {"ANSIBLE_PYTHON_INTERPRETER": "/usr/bin/python3"})["ANSIBLE_PYTHON_INTERPRETER"] == "/usr/bin/python3" assert ansible_env(None, {})["ANSIBLE_HOST_KEY_CHECKING"] == "False"