test: guard run_ansible_in_environment arg word-splitting

[ideaship]

What

Adds a unit-test regression guard for the argument-joining behaviour of
run_ansible_in_environment in osism/tasks/__init__.py. Test only — no
behaviour change.

Why

run_ansible_in_environment joins a list of arguments with plain spaces
(" ".join(arguments)) and runs the result through
subprocess.Popen(..., shell=True), so /bin/sh word-splits each element into
separate argv. Several callers deliberately rely on this — they pack multiple
shell words into a single list element:

• commands/set.py, commands/noset.py: ["-e status=True", f"-l {host}"]
• commands/validate.py: "-e kolla_action=config_validate"
• commands/apply.py: [f"-e kolla_action={action}"] + arguments

The run-<environment>.sh scripts forward args via "$@" (no re-split), so the
outer-shell split is load-bearing.

This makes the join fragile in a non-obvious way: a value containing a shell
metacharacter (e.g. tempest_include_regex=(A|B)) breaks at /bin/sh
(Syntax error: "(" unexpected). The tempting one-line "fix" —
" ".join(shlex.quote(a) for a in arguments) — would regress the callers
above by gluing -e status=True into a single token, and the existing
test_run_ansible_list_arguments_joined would not catch it (it uses only
single-word elements, on which shlex.quote is a no-op).

What the guard does

Pins the multi-token behaviour: a space-containing element must stay word-split
and must not be per-element quoted. It fails under a naive per-element
shlex.quote, and its comment records the safe alternatives (split each element
on whitespace first then quote, or normalise the callers to emit already-split
tokens) so a future metacharacter-safety fix is done without breaking the
split-dependent callers.

🤖 Generated with Claude Code

[sourcery-ai]

Hey - I've left some high level feedback:

• The regression test currently relies on command.endswith(...), which is a bit brittle; consider asserting the full exact command string or parsing the relevant flags from the command to ensure future unrelated changes to prefix/suffix formatting don’t accidentally bypass this guard.
• The explanatory comment in test_run_ansible_list_multitoken_element_word_split_not_quoted is quite long; consider extracting the rationale into a brief reference (e.g., link to an issue or dev note) and keeping only the key invariants this test is enforcing so the test remains easy to scan.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The regression test currently relies on `command.endswith(...)`, which is a bit brittle; consider asserting the full exact command string or parsing the relevant flags from the command to ensure future unrelated changes to prefix/suffix formatting don’t accidentally bypass this guard.
- The explanatory comment in `test_run_ansible_list_multitoken_element_word_split_not_quoted` is quite long; consider extracting the rationale into a brief reference (e.g., link to an issue or dev note) and keeping only the key invariants this test is enforcing so the test remains easy to scan.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}