feat(ansible): parameterized runs — extra-vars, limit, tags, dry-run (task 546)
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) <noreply@anthropic.com>
This commit is contained in:
@@ -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:
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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")
|
||||
@@ -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)
|
||||
|
||||
@@ -83,6 +83,25 @@
|
||||
<label>Or enter inventory path manually</label>
|
||||
<input type="text" id="manual-inv-{{ sd.source.name }}" placeholder="inventories/production/hosts">
|
||||
</div>
|
||||
<div class="form-group">
|
||||
<label>Extra vars <span style="color:var(--text-muted);font-weight:normal;">(optional, one key=value per line)</span></label>
|
||||
<textarea name="extra_vars" rows="2" placeholder="version=1.2.3 restart=true"
|
||||
style="width:100%;font-family:ui-monospace,monospace;font-size:0.85rem;"></textarea>
|
||||
</div>
|
||||
<div class="form-group">
|
||||
<label>Limit to host(s) <span style="color:var(--text-muted);font-weight:normal;">(optional, --limit)</span></label>
|
||||
<input type="text" name="limit" placeholder="web01,db*">
|
||||
</div>
|
||||
<div class="form-group">
|
||||
<label>Tags <span style="color:var(--text-muted);font-weight:normal;">(optional, --tags)</span></label>
|
||||
<input type="text" name="tags" placeholder="deploy,config">
|
||||
</div>
|
||||
<div class="form-group">
|
||||
<label style="display:flex;align-items:center;gap:0.5rem;font-weight:normal;">
|
||||
<input type="checkbox" name="check">
|
||||
Dry-run (--check --diff) — preview changes without applying
|
||||
</label>
|
||||
</div>
|
||||
<div style="display:flex;gap:0.75rem;align-items:center;">
|
||||
<button type="button" class="btn"
|
||||
onclick="var manual=document.getElementById('manual-inv-{{ sd.source.name }}').value.trim();
|
||||
|
||||
@@ -19,6 +19,20 @@
|
||||
{% if run.finished_at %}
|
||||
<span style="color:var(--text-muted);">Finished</span><span style="color:var(--text-dim);">{{ run.finished_at.strftime("%Y-%m-%d %H:%M:%S UTC") }}</span>
|
||||
{% endif %}
|
||||
{% if run.params %}
|
||||
{% if run.params.limit %}
|
||||
<span style="color:var(--text-muted);">Limit</span><span style="font-family:ui-monospace,monospace;">{{ run.params.limit }}</span>
|
||||
{% endif %}
|
||||
{% if run.params.tags %}
|
||||
<span style="color:var(--text-muted);">Tags</span><span style="font-family:ui-monospace,monospace;">{{ run.params.tags }}</span>
|
||||
{% endif %}
|
||||
{% if run.params.check %}
|
||||
<span style="color:var(--text-muted);">Mode</span><span style="color:var(--orange);">dry-run (--check --diff)</span>
|
||||
{% endif %}
|
||||
{% if run.params.extra_vars %}
|
||||
<span style="color:var(--text-muted);">Extra vars</span><span style="font-family:ui-monospace,monospace;">{{ run.params.extra_vars | join(", ") }}</span>
|
||||
{% endif %}
|
||||
{% endif %}
|
||||
</div>
|
||||
</div>
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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"]
|
||||
Reference in New Issue
Block a user