diff --git a/docs/core/mcp.md b/docs/core/mcp.md index 271576e..f4411fb 100644 --- a/docs/core/mcp.md +++ b/docs/core/mcp.md @@ -84,7 +84,7 @@ Each role includes the tools of the roles above it in this list. | Area | Tools | |---|---| -| Automation | `run_playbook`, `cancel_ansible_run`, `create_schedule`, `update_schedule`, `delete_schedule` | +| Automation | `run_playbook`, `cancel_ansible_run`, `create_schedule`, `update_schedule`, `delete_schedule`, `update_group_members` (put hosts into an inventory group or take them out; a group alert rule follows within a minute) | | Monitors | `create_monitor`, `update_monitor`, `set_monitor_enabled` | | Alerts | `create_alert_rule`, `update_alert_rule`, `acknowledge_alert`, `send_test_alert`, `create_maintenance_window`, `end_maintenance_window` | diff --git a/steward/mcp/tools/automation.py b/steward/mcp/tools/automation.py index 797b51a..0ca8fbb 100644 --- a/steward/mcp/tools/automation.py +++ b/steward/mcp/tools/automation.py @@ -1,4 +1,5 @@ -"""Automation tools: playbooks, inventory, runs, schedules and alert actions. +"""Automation tools: playbooks, inventory and its groups, runs, schedules and +alert actions. Reads are viewer tools. Anything that starts a run or changes what runs is an operator tool (set_alert_action is admin, as on the alert rule form), and each @@ -267,6 +268,77 @@ async def list_inventory() -> dict: } +def member_targets(names: list[str] | None, targets: list, host_names: dict[str, str]) -> list: + """The targets `names` mean, each a target's name or id, or the name of the + Steward host a target is linked to. Refuses the whole list when one is + unknown, so nothing changes on a typo.""" + by_host = {host_names[t.host_id].lower(): t for t in targets + if t.host_id and host_names.get(t.host_id)} + found, unknown = [], [] + for want in names or []: + want = (want or "").strip() + if not want: + continue + t = next((t for t in targets if t.id == want or t.name.lower() == want.lower()), None) \ + or by_host.get(want.lower()) + (found if t else unknown).append(t or want) + if unknown: + raise ValueError(f"no target or linked host named {', '.join(map(repr, unknown))}; " + "list_inventory lists them. Nothing was changed.") + return list({t.id: t for t in found}.values()) + + +async def update_group_members(group: str, add: list[str] | None = None, + remove: list[str] | None = None) -> dict: + """Put targets into an inventory group, or take them out. + + Inventory groups are Steward's grouping system: an alert rule covering a + group watches every host in it, the Hosts list filters by them, and a run + can be scoped to one. A host is in a group through its linked target. + + group: the group's name or id (list_inventory). + add, remove: targets by name or id, or by the name of the Steward host a + target is linked to. One already in (or out of) the group is left as it + is; an unknown name refuses the whole change. + + Returns the group's targets after the change and the alert rules that + cover it; their per-host rules follow within a minute. Recorded in the + audit log as " via MCP". + """ + if not (add or remove): + raise ValueError("pass add and/or remove: the targets to put in or take out.") + async with db_session() as db, db.begin(): + g = pick(group, (await db.execute( + select(AnsibleGroup).options(selectinload(AnsibleGroup.targets)))).scalars(), + kind="group", list_tool="list_inventory") + targets = list((await db.execute(select(AnsibleTarget))).scalars()) + host_names = dict((await db.execute(select(Host.id, Host.name))).all()) + to_add = member_targets(add, targets, host_names) + to_remove = member_targets(remove, targets, host_names) + both = {t.name for t in to_add} & {t.name for t in to_remove} + if both: + raise ValueError(f"{', '.join(sorted(both))} is in both add and remove. Nothing was changed.") + have = {t.id for t in g.targets} + added = [t for t in to_add if t.id not in have] + removed = [t for t in to_remove if t.id in have] + g.targets = [t for t in g.targets if t.id not in {r.id for r in removed}] + added + members = sorted(t.name for t in g.targets) + covering = sorted((await db.execute(select(AlertRule.name).where( + AlertRule.covers == "group", AlertRule.cover_group == g.name))).scalars()) + name, gid = g.name, g.id + if added or removed: + # The Inventory page's own record of a group's membership changing. + await audit_mcp("ansible.group.updated", "ansible_group", gid, + {"name": name, "targets": members, + "added": sorted(t.name for t in added), + "removed": sorted(t.name for t in removed)}) + return {"group": name, "group_id": gid, + "added": sorted(t.name for t in added), "removed": sorted(t.name for t in removed), + "targets": members, "alert_rules": covering, + **({"note": "Alert rules covering this group pick up the change within a minute."} + if covering and (added or removed) else {})} + + # ── runs ───────────────────────────────────────────────────────────────────── def recap_totals(results: dict | None) -> dict | None: @@ -712,6 +784,7 @@ def register(register_tool) -> None: for fn in (list_playbooks, get_playbook, list_inventory, list_ansible_runs, get_ansible_run, list_schedules, wait_for_run): register_tool(fn, role=UserRole.viewer, read_only=True) - for fn in (run_playbook, cancel_ansible_run, create_schedule, update_schedule, delete_schedule): + for fn in (run_playbook, cancel_ansible_run, create_schedule, update_schedule, delete_schedule, + update_group_members): register_tool(fn, role=UserRole.operator, read_only=False) register_tool(set_alert_action, role=UserRole.admin, read_only=False) diff --git a/tests/integration/test_mcp_group_members.py b/tests/integration/test_mcp_group_members.py new file mode 100644 index 0000000..c31eaf7 --- /dev/null +++ b/tests/integration/test_mcp_group_members.py @@ -0,0 +1,80 @@ +"""Integration: update_group_members over MCP (#5487) against Postgres. A host +joins a group by its linked target, a group rule covering that group is named +in the answer, and the change is audited as " via MCP".""" +from __future__ import annotations + +import asyncio +import json +import os +import uuid + +import pytest + +pytestmark = pytest.mark.integration + + +@pytest.fixture +def app(): + if not os.environ.get("STEWARD_DATABASE_URL"): + pytest.skip("needs Postgres") + from steward.app import create_app + return create_app(testing=False) + + +def test_a_host_joins_and_leaves_a_group_through_mcp(app): + from sqlalchemy import select + from steward.core.api_tokens import Caller + from steward.mcp import _context + from steward.mcp.tools import automation as a + from steward.models.alerts import AlertOperator, AlertRule + from steward.models.ansible_inventory import AnsibleGroup, AnsibleTarget + from steward.models.audit import AuditEvent + from steward.models.hosts import Host + from steward.models.users import User, UserRole + + tag = uuid.uuid4().hex[:8] + uid, username = str(uuid.uuid4()), f"gm-op-{tag}" + group, host = f"gm-group-{tag}", f"gm-host-{tag}" + + async def _go(): + async with app.db_sessionmaker() as s: + async with s.begin(): + s.add(User(id=uid, username=username, email=f"{tag}@e.test", + password_hash="x", role=UserRole.operator, is_active=True)) + h = Host(name=host, address="10.9.9.9") + s.add(h) + await s.flush() + s.add(AnsibleTarget(name=f"gm-target-{tag}", address="10.9.9.9", host_id=h.id)) + s.add(AnsibleTarget(name=f"gm-other-{tag}", address="10.9.9.8")) + s.add(AnsibleGroup(name=group)) + s.add(AlertRule(name=f"gm-rule-{tag}", source_module="host_agent", resource_name="", + metric_name="cpu_pct", operator=AlertOperator.gt, threshold=90.0, + covers="group", cover_group=group, created_by=uid)) + _context._app_ctx.set(app) + _context._caller_ctx.set(Caller(user_id=uid, username=username, + role=UserRole.operator, token_id=7)) + + got = await a.update_group_members(group, add=[host, f"gm-other-{tag}"]) + assert got["targets"] == sorted([f"gm-other-{tag}", f"gm-target-{tag}"]) + assert got["added"] == got["targets"] and got["alert_rules"] == [f"gm-rule-{tag}"] + assert "within a minute" in got["note"] + inv = await a.list_inventory() + assert next(t for t in inv["targets"] if t["host_name"] == host)["groups"] == [group] + + # Adding again changes nothing; an unknown name refuses the lot. + again = await a.update_group_members(group, add=[f"gm-target-{tag}"]) + assert again["added"] == [] and "note" not in again + with pytest.raises(ValueError, match="Nothing was changed"): + await a.update_group_members(group, remove=[host, "no-such-host"]) + + got = await a.update_group_members(group, remove=[host]) + assert got["removed"] == [f"gm-target-{tag}"] and got["targets"] == [f"gm-other-{tag}"] + + async with app.db_sessionmaker() as s: + rows = list((await s.execute(select(AuditEvent).where( + AuditEvent.user_id == uid, AuditEvent.action == "ansible.group.updated") + .order_by(AuditEvent.timestamp))).scalars()) + assert len(rows) == 2 and rows[0].username == f"{username} via MCP" + assert json.loads(rows[1].detail_json)["removed"] == [f"gm-target-{tag}"] + + asyncio.run(_go()) diff --git a/tests/mcp/test_automation.py b/tests/mcp/test_automation.py index c4fa4bd..8ce64e1 100644 --- a/tests/mcp/test_automation.py +++ b/tests/mcp/test_automation.py @@ -24,7 +24,7 @@ VARS = [ VIEWER_TOOLS = {"list_playbooks", "get_playbook", "list_inventory", "list_ansible_runs", "get_ansible_run", "list_schedules", "wait_for_run"} OPERATOR_TOOLS = {"run_playbook", "cancel_ansible_run", "create_schedule", "update_schedule", - "delete_schedule"} + "delete_schedule", "update_group_members"} def test_split_vars_keeps_secrets_apart_from_what_is_stored(): @@ -156,3 +156,15 @@ def test_schedule_update_holds_only_changed_variables(monkeypatch): automation.check_caller_vars({**stored, "release": "2"}, stored) with pytest.raises(ValueError, match="Only an admin"): automation.check_caller_vars({"ansible_ssh_common_args": "-o ProxyCommand=x"}, stored) + + +def test_group_members_are_named_by_target_or_by_linked_host(): + """#5487: a target by its name or id, or the Steward host linked to it; an + unknown name refuses the lot, so a typo changes nothing.""" + web = NS(id="t1", name="web01", host_id="h1") + db = NS(id="t2", name="db01", host_id=None) + hosts = {"h1": "VDNT-Docker02"} + got = automation.member_targets(["vdnt-docker02", "t2", "web01", " "], [web, db], hosts) + assert [t.name for t in got] == ["web01", "db01"] + with pytest.raises(ValueError, match="no target or linked host named 'nas'.*Nothing was changed"): + automation.member_targets(["web01", "nas"], [web, db], hosts)