From 8b62eb2ca3e040f93f771cefe1d5f2320edf9eaf Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 2 Jun 2026 13:31:16 -0400 Subject: [PATCH] =?UTF-8?q?feat(ansible):=20parameterized=20runs=20?= =?UTF-8?q?=E2=80=94=20extra-vars,=20limit,=20tags,=20dry-run=20(task=2054?= =?UTF-8?q?6)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Run form + executor now support optional params, passed as argv (never shell-interpolated): - extra_vars: one key=value per line -> repeated -e - limit -> --limit; tags -> --tags - dry-run checkbox -> --check --diff - executor.py: pure build_ansible_command(playbook, inventory, params); start_run gains an optional params arg (backward-compatible) - models/ansible.py + migration 0013: nullable params JSON column - routes.py: create_run parses + validates (extra-var lines need '='), stores params on the run, passes to the executor - browse.html run form: Extra vars / Limit / Tags / Dry-run fields - run_detail.html: shows the params used - tests/test_ansible_command.py: unit coverage of the builder; integration test runs a real playbook with extra-var + limit + check and asserts substitution Co-Authored-By: Claude Opus 4.8 (1M context) --- steward/ansible/executor.py | 27 +++++++++++- steward/ansible/routes.py | 26 ++++++++++++ .../versions/0013_ansible_run_params.py | 22 ++++++++++ steward/models/ansible.py | 4 +- steward/templates/ansible/browse.html | 19 +++++++++ steward/templates/ansible/run_detail.html | 14 +++++++ tests/integration/test_ansible_foundation.py | 39 +++++++++++++++++ tests/test_ansible_command.py | 42 +++++++++++++++++++ 8 files changed, 191 insertions(+), 2 deletions(-) create mode 100644 steward/migrations/versions/0013_ansible_run_params.py create mode 100644 tests/test_ansible_command.py diff --git a/steward/ansible/executor.py b/steward/ansible/executor.py index 22e62c9..92ee4ab 100644 --- a/steward/ansible/executor.py +++ b/steward/ansible/executor.py @@ -59,12 +59,37 @@ def _broadcast(run_id: str, item: str | _Done) -> None: _run_listeners.pop(run_id, None) +def build_ansible_command( + playbook_path: str, + inventory_path: str, + params: dict | None = None, +) -> list[str]: + """Build the ansible-playbook argv from a run's optional params. + + Pure (no IO). Everything is passed as argv — never shell-interpolated — so + extra-var values and limits can contain arbitrary characters safely. + params keys: extra_vars (list of "key=value"), limit, tags, check (bool). + """ + cmd = ["ansible-playbook", playbook_path, "-i", inventory_path] + params = params or {} + for ev in params.get("extra_vars") or []: + cmd += ["-e", ev] + if params.get("limit"): + cmd += ["--limit", params["limit"]] + if params.get("tags"): + cmd += ["--tags", params["tags"]] + if params.get("check"): + cmd += ["--check", "--diff"] + return cmd + + async def start_run( app: "Quart", run_id: str, playbook_path: str, inventory_path: str, source_path: str, + params: dict | None = None, ) -> None: """Execute ansible-playbook as a subprocess and update the DB run row.""" _run_lines[run_id] = [] @@ -92,7 +117,7 @@ async def start_run( lines_since_flush = 0 last_flush_at = time.monotonic() - cmd = ["ansible-playbook", playbook_path, "-i", inventory_path] + cmd = build_ansible_command(playbook_path, inventory_path, params) cwd = source_path if Path(source_path).exists() else None try: diff --git a/steward/ansible/routes.py b/steward/ansible/routes.py index 11022dc..b304bd5 100644 --- a/steward/ansible/routes.py +++ b/steward/ansible/routes.py @@ -85,6 +85,30 @@ async def create_run(): if source is None: return "Source not found", 404 + # Optional run parameters (all passed as argv by the executor — no shell). + extra_vars: list[str] = [] + for line in (form.get("extra_vars", "") or "").splitlines(): + line = line.strip() + if not line: + continue + if "=" not in line: + return f"Invalid extra var (expected key=value): {line!r}", 400 + extra_vars.append(line) + limit = (form.get("limit", "") or "").strip() + tags = (form.get("tags", "") or "").strip() + check = "check" in form + + params: dict = {} + if extra_vars: + params["extra_vars"] = extra_vars + if limit: + params["limit"] = limit + if tags: + params["tags"] = tags + if check: + params["check"] = True + params_or_none = params or None + run_id = str(uuid.uuid4()) now = datetime.now(timezone.utc) @@ -96,6 +120,7 @@ async def create_run(): triggered_by=session["user_id"], status=AnsibleRunStatus.running, started_at=now, + params=params_or_none, ) async with current_app.db_sessionmaker() as db: async with db.begin(): @@ -108,6 +133,7 @@ async def create_run(): playbook_path, inventory_path, source["path"], + params_or_none, ) ) task.add_done_callback( diff --git a/steward/migrations/versions/0013_ansible_run_params.py b/steward/migrations/versions/0013_ansible_run_params.py new file mode 100644 index 0000000..f70811b --- /dev/null +++ b/steward/migrations/versions/0013_ansible_run_params.py @@ -0,0 +1,22 @@ +"""Add ansible_runs.params (optional run parameters) + +Revision ID: 0013_ansible_run_params +Revises: 0012_ansible_run_nullable +Create Date: 2026-06-02 +""" +from typing import Sequence, Union +from alembic import op +import sqlalchemy as sa + +revision: str = "0013_ansible_run_params" +down_revision: Union[str, None] = "0012_ansible_run_nullable" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + op.add_column("ansible_runs", sa.Column("params", sa.JSON(), nullable=True)) + + +def downgrade() -> None: + op.drop_column("ansible_runs", "params") diff --git a/steward/models/ansible.py b/steward/models/ansible.py index 82d18bc..2adf06e 100644 --- a/steward/models/ansible.py +++ b/steward/models/ansible.py @@ -2,7 +2,7 @@ from __future__ import annotations import enum import uuid from datetime import datetime, timezone -from sqlalchemy import DateTime, Enum, ForeignKey, String, Text +from sqlalchemy import JSON, DateTime, Enum, ForeignKey, String, Text from sqlalchemy.orm import Mapped, mapped_column from .base import Base @@ -34,3 +34,5 @@ class AnsibleRun(Base): ) finished_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True), nullable=True) output: Mapped[str | None] = mapped_column(Text, nullable=True) + # Optional run parameters: {extra_vars: [..], limit: str, tags: str, check: bool}. + params: Mapped[dict | None] = mapped_column(JSON, nullable=True) diff --git a/steward/templates/ansible/browse.html b/steward/templates/ansible/browse.html index 93dae22..e0b95b8 100644 --- a/steward/templates/ansible/browse.html +++ b/steward/templates/ansible/browse.html @@ -83,6 +83,25 @@ +
+ + +
+
+ + +
+
+ + +
+
+ +
diff --git a/tests/integration/test_ansible_foundation.py b/tests/integration/test_ansible_foundation.py index 341755d..d15c54e 100644 --- a/tests/integration/test_ansible_foundation.py +++ b/tests/integration/test_ansible_foundation.py @@ -99,3 +99,42 @@ def test_executor_runs_local_playbook(app, tmp_path): run = asyncio.run(_go()) assert run.status == AnsibleRunStatus.success, run.output assert "STEWARD_ANSIBLE_OK" in (run.output or "") + + +@_NEEDS_DB +def test_executor_runs_parameterized(app, tmp_path): + """extra_vars + limit + check all flow through to a real run (task 546).""" + from sqlalchemy import select + from steward.ansible import executor + from steward.models.ansible import AnsibleRun, AnsibleRunStatus + + (tmp_path / "test.yml").write_text(textwrap.dedent("""\ + - hosts: localhost + connection: local + gather_facts: false + tasks: + - debug: + msg: "GOT-{{ myvar }}" + """)) + (tmp_path / "inv.ini").write_text("localhost ansible_connection=local\n") + + run_id = str(uuid.uuid4()) + params = {"extra_vars": ["myvar=HELLO"], "limit": "localhost", "check": True} + + async def _go(): + async with app.db_sessionmaker() as s: + async with s.begin(): + s.add(AnsibleRun( + id=run_id, playbook_path="test.yml", inventory_path="inv.ini", + source_name="t", triggered_by=None, + status=AnsibleRunStatus.running, params=params, + )) + await executor.start_run(app, run_id, "test.yml", "inv.ini", str(tmp_path), params) + async with app.db_sessionmaker() as s: + return (await s.execute( + select(AnsibleRun).where(AnsibleRun.id == run_id))).scalar_one() + + run = asyncio.run(_go()) + assert run.status == AnsibleRunStatus.success, run.output + assert "GOT-HELLO" in (run.output or "") # extra var substituted + assert run.params == params # params round-trip through the DB diff --git a/tests/test_ansible_command.py b/tests/test_ansible_command.py new file mode 100644 index 0000000..2c2e240 --- /dev/null +++ b/tests/test_ansible_command.py @@ -0,0 +1,42 @@ +"""Unit tests for the ansible-playbook command builder (task 546).""" +from steward.ansible.executor import build_ansible_command + +BASE = ["ansible-playbook", "site.yml", "-i", "inv.ini"] + + +def test_base_no_params(): + assert build_ansible_command("site.yml", "inv.ini") == BASE + assert build_ansible_command("site.yml", "inv.ini", None) == BASE + assert build_ansible_command("site.yml", "inv.ini", {}) == BASE + + +def test_extra_vars_each_passed_as_argv(): + cmd = build_ansible_command("site.yml", "inv.ini", {"extra_vars": ["a=1", "b=two words"]}) + assert cmd == BASE + ["-e", "a=1", "-e", "b=two words"] + + +def test_limit_and_tags(): + cmd = build_ansible_command("site.yml", "inv.ini", {"limit": "web*", "tags": "deploy,cfg"}) + assert cmd[cmd.index("--limit") + 1] == "web*" + assert cmd[cmd.index("--tags") + 1] == "deploy,cfg" + + +def test_check_adds_check_and_diff(): + cmd = build_ansible_command("site.yml", "inv.ini", {"check": True}) + assert cmd == BASE + ["--check", "--diff"] + + +def test_falsey_values_are_skipped(): + cmd = build_ansible_command( + "site.yml", "inv.ini", + {"extra_vars": [], "limit": "", "tags": "", "check": False}, + ) + assert cmd == BASE + + +def test_all_combined_order(): + cmd = build_ansible_command( + "site.yml", "inv.ini", + {"extra_vars": ["v=1"], "limit": "h1", "tags": "t1", "check": True}, + ) + assert cmd == BASE + ["-e", "v=1", "--limit", "h1", "--tags", "t1", "--check", "--diff"]