fix(ansible): group names Ansible accepts, and interpreter discovery without the warning (#5334)

Every run warned twice because of Steward's own inventory:
- 'Invalid characters were found in group names': the group 'Docker
  Swarm' went into the inventory as typed. ansible_group_name() cleans it
  to docker_swarm (lower case, [a-z0-9_], never a leading digit); two
  names that clean alike get _2, _3 so neither folds into the other. The
  group page says what plays call it. The display name is unchanged.
- Python interpreter discovery: ansible_env sets
  ANSIBLE_PYTHON_INTERPRETER=auto_silent unless the environment already
  sets one; a target or group's ansible_python_interpreter still wins.

Runs are scoped by target set (steward:group:<id>), not by group name,
and --limit is free text, so neither path depends on the name.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

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
 
 

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
 

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,

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 _. #}
 
 <p class="page-verdict">
   {%- set members = group.targets|sort(attribute='name') %}
   {%- if members %}{{ members|length }} member{{ 's' if members|length != 1 }}:
   {% for t in members %}<a href="/ansible/inventory/targets/{{ t.id }}">{{ t.name }}</a>{{ ", " if not loop.last }}{% endfor %}
   {%- else %}<span class="tone-warn">no members</span>{% endif %}
+  · plays call it <code>{{ ansible_name }}</code>
   {%- if used %} · scheduled by {% for s in used %}<a href="/ansible/schedules/{{ s.id }}">{{ s.name }}</a>{{ ", " if not loop.last }}{% endfor %}{% endif %}
 </p>
 

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("<main"):]
     assert '1 member:' in main and 'href="/ansible/inventory/targets/t1">VDNT-Docker02</a>' in main
     assert 'scheduled by <a href="/ansible/schedules/s1">Nightly prune</a>' in main
+    assert "plays call it <code>docker_swarm</code>" 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)

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"}

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"

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"