diff --git a/steward/ansible/executor.py b/steward/ansible/executor.py index f8c7ec8..b70d694 100644 --- a/steward/ansible/executor.py +++ b/steward/ansible/executor.py @@ -186,6 +186,20 @@ def build_bootstrap(connection: dict | None, tmpdir: str) -> tuple[list[str], li return args, files +def build_extra_vars_file(merged: dict | None, tmpdir: str) -> tuple[list[str], list[tuple[str, str]]]: + """Run-time variables → a JSON extra-vars file (``-e @file``). + + Pure: returns (argv, files). JSON is valid YAML to Ansible and, unlike + ``-e key=value`` strings (which Ansible shlex-splits on whitespace), keeps + values with spaces/quotes intact. Used for both operator-entered playbook + variables and the unpersisted secret subset (merged by the caller). + """ + if not merged: + return [], [] + path = os.path.join(tmpdir, "extravars.json") + return ["-e", "@" + path], [(path, json.dumps(merged))] + + def ansible_env(creds: dict | None, base_env) -> dict: """Return a copy of base_env with ANSIBLE_HOST_KEY_CHECKING set from creds.""" env = dict(base_env) @@ -282,6 +296,7 @@ async def start_run( params: dict | None = None, inventory_content: str | None = None, connection: dict | None = None, + secret_vars: dict | None = None, ) -> None: """Execute ansible-playbook as a subprocess and update the DB run row. @@ -292,6 +307,9 @@ async def start_run( connection is an optional per-run SSH override (user/password) for first-contact provisioning. It is applied to the subprocess only — it is never persisted on the AnsibleRun row, never placed on argv, and not logged. + secret_vars are run-time playbook variables flagged sensitive; like + connection they reach the subprocess (merged into the extra-vars file) but + are never persisted. """ from steward.models.ansible import AnsibleRunStatus @@ -374,7 +392,10 @@ async def start_run( cred_args, cred_files = build_credentials(effective_creds, tmpdir) boot_args, boot_files = build_bootstrap(conn, tmpdir) - for cred_path, content in cred_files + boot_files: + # Operator-entered run-time vars (persisted) + secret vars (not). + merged_vars = {**((params or {}).get("extra_vars_map") or {}), **(secret_vars or {})} + ev_args, ev_files = build_extra_vars_file(merged_vars, tmpdir) + for cred_path, content in cred_files + boot_files + ev_files: with open(cred_path, "w", encoding="utf-8") as cf: cf.write(content) os.chmod(cred_path, 0o600) @@ -388,7 +409,7 @@ async def start_run( user_args += ["--user", ssh_user] proc = await asyncio.create_subprocess_exec( - *cmd, *cred_args, *boot_args, *user_args, + *cmd, *cred_args, *boot_args, *user_args, *ev_args, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.STDOUT, cwd=cwd, diff --git a/steward/ansible/routes.py b/steward/ansible/routes.py index 5e206b0..ccf5fce 100644 --- a/steward/ansible/routes.py +++ b/steward/ansible/routes.py @@ -95,19 +95,43 @@ async def view_playbook(source_name: str, playbook_path: str): ) -def _parse_run_params(form) -> tuple[dict | None, str | None]: - """Build the executor params dict from form fields. Returns (params, error).""" - extra_vars: list[str] = [] +def _parse_run_params(form) -> tuple[dict | None, dict | None, str | None]: + """Build executor params + the unpersisted secret-vars dict from form fields. + + Returns (params, secret_vars, error). Discovered variable fields are named + ``var__``; a sibling hidden ``secret__`` marks the sensitive + ones, which go into secret_vars (never persisted) instead of params. A + free-form ``extra_vars`` textarea (key=value per line) is also folded in. + All run-time values flow through a JSON extra-vars file downstream, so + values may contain spaces/quotes safely. + """ + extra_vars_map: dict[str, str] = {} + secret_vars: dict[str, str] = {} + + for key in set(form.keys()): + if not key.startswith("var__"): + continue + name = key[len("var__"):] + val = (form.get(key) or "").strip() + if not val: + continue # untouched → fall through to inventory/play default + if f"secret__{name}" in form: + secret_vars[name] = val + else: + extra_vars_map[name] = val + for line in (form.get("extra_vars", "") or "").splitlines(): line = line.strip() if not line: continue if "=" not in line: - return None, f"Invalid extra var (expected key=value): {line!r}" - extra_vars.append(line) + return None, None, f"Invalid extra var (expected key=value): {line!r}" + k, v = line.split("=", 1) + extra_vars_map[k.strip()] = v + params: dict = {} - if extra_vars: - params["extra_vars"] = extra_vars + if extra_vars_map: + params["extra_vars_map"] = extra_vars_map limit = (form.get("limit", "") or "").strip() if limit: params["limit"] = limit @@ -116,7 +140,37 @@ def _parse_run_params(form) -> tuple[dict | None, str | None]: params["tags"] = tags if "check" in form: params["check"] = True - return (params or None), None + return (params or None), (secret_vars or None), None + + +@ansible_bp.get("/run-form//") +@require_role(UserRole.operator) +async def run_form(source_name: str, playbook_path: str): + """HTMX fragment: the run form for one playbook, with a field per declared + variable (vars/vars_prompt) discovered from the playbook itself.""" + from steward.models.ansible_inventory import AnsibleTarget, AnsibleGroup + + source = next((s for s in _get_sources() if s["name"] == source_name), None) + if source is None: + return "Source not found", 404 + contents = src_module.read_playbook(source["path"], playbook_path) + if contents is None: + return "Playbook not found", 404 + variables = src_module.discover_playbook_variables(contents) + inventories = src_module.discover_inventories(source["path"]) + + async with current_app.db_sessionmaker() as db: + targets = (await db.execute( + select(AnsibleTarget).order_by(AnsibleTarget.name))).scalars().all() + groups = (await db.execute( + select(AnsibleGroup).order_by(AnsibleGroup.name))).scalars().all() + + return await render_template( + "ansible/_run_form.html", + source_name=source_name, playbook_path=playbook_path, + variables=variables, targets=targets, groups=groups, + inventories=inventories, + ) @ansible_bp.post("/runs") @@ -132,7 +186,7 @@ async def create_run(): if not playbook_path: return "playbook_path is required", 400 - params_or_none, err = _parse_run_params(form) + params_or_none, secret_vars, err = _parse_run_params(form) if err: return err, 400 @@ -142,6 +196,7 @@ async def create_run(): playbook_path=playbook_path, inventory_scope=inventory_scope, params=params_or_none, + secret_vars=secret_vars, triggered_by=session["user_id"], ) if err == "Source not found": @@ -304,7 +359,9 @@ def _schedule_form_fields(form) -> tuple[dict | None, str | None]: interval = 0 if not name or not source_name or not playbook_path or interval <= 0: return None, "name, source, playbook, and a positive interval are required" - params, err = _parse_run_params(form) + # Scheduled runs can't prompt, so secret vars are intentionally dropped — + # automation should rely on global creds / inventory vars, not run-time secrets. + params, _secret_vars, err = _parse_run_params(form) if err: return None, err return { diff --git a/steward/ansible/runner.py b/steward/ansible/runner.py index 5b031ef..27dfaf7 100644 --- a/steward/ansible/runner.py +++ b/steward/ansible/runner.py @@ -26,6 +26,7 @@ async def trigger_run( triggered_by: str | None = None, inventory_content: str | None = None, connection: dict | None = None, + secret_vars: dict | None = None, ): """Resolve inventory for the scope, persist an AnsibleRun, and launch it. @@ -36,6 +37,8 @@ async def trigger_run( connection is an optional per-run SSH override (user/password) for first-contact provisioning — passed to the executor but deliberately NOT stored on the AnsibleRun row (params), so the password never lands in the DB. + secret_vars (sensitive run-time playbook variables) are likewise passed + through to the executor but never persisted. Returns (run, source, error): on success error is None; on failure run is None and error is a short human-readable reason. """ @@ -77,7 +80,7 @@ async def trigger_run( executor.start_run( app, run_id, playbook_path, inventory_path or "", source["path"], params or None, inventory_content, - connection=connection, + connection=connection, secret_vars=secret_vars, ) ) task.add_done_callback( diff --git a/steward/ansible/sources.py b/steward/ansible/sources.py index a74cdf9..3e26a20 100644 --- a/steward/ansible/sources.py +++ b/steward/ansible/sources.py @@ -2,6 +2,7 @@ from __future__ import annotations import asyncio import logging import os +import re import stat import tempfile from pathlib import Path @@ -10,6 +11,12 @@ logger = logging.getLogger(__name__) INVENTORY_NAMES = {"hosts", "inventory", "inventory.yml", "inventory.ini"} +# Variable names that should be entered masked + kept out of the DB. Matched +# case-insensitively as a substring of the variable name. +_SECRET_VAR_RE = re.compile( + r"(password|passwd|secret|token|api[_-]?key|private[_-]?key|credential)", re.I +) + # Name of the always-present, read-only source of first-party playbooks shipped # inside the app (maintenance tasks, host-agent install). Not operator-editable. BUILTIN_SOURCE_NAME = "steward-builtin" @@ -188,6 +195,64 @@ def delete_playbook(source_path: str, relative_path: str) -> tuple[bool, str | N return True, None +def discover_playbook_variables(content: str) -> list[dict]: + """Parse a playbook and list the variables an operator can set at run time. + + Surfaces two sources, in this precedence (first wins on name collision): + - ``vars_prompt:`` — Ansible's explicit "ask me" mechanism. ``required`` + when it has no default; ``secret`` when ``private`` (Ansible's default + is private=yes) or the name looks sensitive. + - ``vars:`` scalar entries — shown with their default as a placeholder + (NOT prefilled, so an untouched field falls through to inventory/play + defaults rather than overriding them). + + Only top-level plays are inspected — role defaults and included var files are + not traversed (kept simple + predictable). Returns a list of dicts: + {name, default, secret, required, prompt}. + """ + import yaml + try: + plays = yaml.safe_load(content) + except yaml.YAMLError: + return [] + if not isinstance(plays, list): + return [] + + out: list[dict] = [] + seen: set[str] = set() + + def add(name, default, secret, required, prompt=None): + if not isinstance(name, str) or name in seen: + return + seen.add(name) + out.append({ + "name": name, + "default": "" if default is None else default, + "secret": secret, + "required": required, + "prompt": prompt, + }) + + for play in plays: + if not isinstance(play, dict): + continue + for vp in play.get("vars_prompt") or []: + if not isinstance(vp, dict) or "name" not in vp: + continue + name = vp["name"] + default = vp.get("default") + private = bool(vp.get("private", True)) # Ansible defaults private=yes + add(name, default, private or bool(_SECRET_VAR_RE.search(str(name))), + default is None, vp.get("prompt")) + play_vars = play.get("vars") + if isinstance(play_vars, dict): + for name, default in play_vars.items(): + # Only scalar defaults map cleanly to a single input field. + if isinstance(default, (str, int, float, bool)) or default is None: + add(name, default, bool(_SECRET_VAR_RE.search(str(name))), False) + return out + + def validate_playbook_yaml(content: str) -> tuple[bool, str | None]: """Cheap save-time validation: parses as YAML and is a non-empty play list.""" import yaml diff --git a/steward/templates/ansible/_run_form.html b/steward/templates/ansible/_run_form.html new file mode 100644 index 0000000..088f0f1 --- /dev/null +++ b/steward/templates/ansible/_run_form.html @@ -0,0 +1,96 @@ +{# Run form for a single playbook — loaded via HTMX from /ansible/run-form/… #} +
+
+

Run {{ playbook_path }}

+ Cancel +
+
+ + + +
+ + +
+ + {% if variables %} +
+ Playbook variables +
+

+ Leave a field blank to use the playbook/inventory default. Secret fields are + never stored. +

+ {% for v in variables %} +
+ + {% if v.secret %}{% endif %} + +
+ {% endfor %} + {% else %} +

+ No declared variables found in this playbook. +

+ {% endif %} + +
+ Advanced +
+ + +
+
+ + +
+
+ + +
+
+ +
+
+ +
+ +
+
+
+
diff --git a/steward/templates/ansible/browse.html b/steward/templates/ansible/browse.html index afdcbcc..4832eff 100644 --- a/steward/templates/ansible/browse.html +++ b/steward/templates/ansible/browse.html @@ -76,88 +76,17 @@ {% if sd.editable and session.user_role == 'admin' %} Edit {% endif %} - Run + Run {% endfor %} - {# Run trigger form for this source #} - + {# Run form (loaded on demand via HTMX from /ansible/run-form//) #} +
{% endif %} {% endfor %} diff --git a/tests/test_playbook_variables.py b/tests/test_playbook_variables.py new file mode 100644 index 0000000..560cf03 --- /dev/null +++ b/tests/test_playbook_variables.py @@ -0,0 +1,110 @@ +"""Unit tests for playbook variable discovery + run-time extra-vars file.""" +import json + +from steward.ansible.executor import build_extra_vars_file +from steward.ansible.sources import discover_playbook_variables + + +def _names(variables): + return [v["name"] for v in variables] + + +def test_vars_block_defaults_surface_as_fields(): + content = """ +- hosts: all + vars: + steward_url: "" + agent_interval: 30 + enabled: true + tasks: [] +""" + variables = discover_playbook_variables(content) + assert _names(variables) == ["steward_url", "agent_interval", "enabled"] + interval = next(v for v in variables if v["name"] == "agent_interval") + assert interval["default"] == 30 + assert interval["required"] is False + assert interval["secret"] is False + + +def test_secretish_names_flagged(): + content = """ +- hosts: all + vars: + db_password: "" + api_token: "" + vault_secret: "" + plain_value: "x" +""" + by = {v["name"]: v for v in discover_playbook_variables(content)} + assert by["db_password"]["secret"] is True + assert by["api_token"]["secret"] is True + assert by["vault_secret"]["secret"] is True + assert by["plain_value"]["secret"] is False + + +def test_vars_prompt_required_and_private(): + content = """ +- hosts: all + vars_prompt: + - name: release_tag + prompt: "Which release?" + - name: admin_pw + prompt: "Admin password" + private: true + default: "" + tasks: [] +""" + by = {v["name"]: v for v in discover_playbook_variables(content)} + # No default → required; vars_prompt defaults to private=yes → secret. + assert by["release_tag"]["required"] is True + assert by["release_tag"]["secret"] is True + assert by["release_tag"]["prompt"] == "Which release?" + # Explicit private + has default → secret but not required. + assert by["admin_pw"]["secret"] is True + assert by["admin_pw"]["required"] is False + + +def test_vars_prompt_wins_on_name_collision(): + content = """ +- hosts: all + vars_prompt: + - name: dup + prompt: "from prompt" + private: false + vars: + dup: "from vars" +""" + variables = discover_playbook_variables(content) + assert _names(variables) == ["dup"] + assert variables[0]["prompt"] == "from prompt" + + +def test_non_scalar_vars_skipped(): + content = """ +- hosts: all + vars: + scalar: 1 + a_list: [1, 2] + a_map: {k: v} +""" + assert _names(discover_playbook_variables(content)) == ["scalar"] + + +def test_malformed_yaml_returns_empty(): + assert discover_playbook_variables("this: : : not valid") == [] + assert discover_playbook_variables("just a string") == [] + + +def test_extra_vars_file_roundtrip_and_space_safe(): + merged = {"agent_interval": "30", "msg": "hello world", "q": 'a "quote"'} + args, files = build_extra_vars_file(merged, "/td") + assert args == ["-e", "@/td/extravars.json"] + path, content = files[0] + assert path == "/td/extravars.json" + # JSON keeps spaces/quotes intact — no shlex hazard like -e key=value. + assert json.loads(content) == merged + + +def test_extra_vars_file_empty_is_noop(): + assert build_extra_vars_file({}, "/td") == ([], []) + assert build_extra_vars_file(None, "/td") == ([], [])