From 705ffc564d71dc07ea087dd9093a775189238135 Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Wed, 15 Apr 2026 22:01:06 +0200 Subject: [PATCH] [PR #11771/df252e5f backport][stable-12] incus, machinectl, run0 - fix become over pty connections (#11827) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit incus, machinectl, run0 - fix become over pty connections (#11771) * incus, machinectl, run0 - fix become over pty connections Four small fixes across three plugins, all discovered while trying to use community.general.machinectl (and later community.general.run0) as become methods over the community.general.incus connection. Core bug: machinectl and run0 both set require_tty = True, but the incus connection plugin was ignoring that hint and invoking 'incus exec' without -t. Honor require_tty by passing -t, mirroring what the OpenSSH plugin does with -tt. Once the pty is in place, both become plugins emit terminal control sequences (window-title OSC, ANSI reset) around the child command that land in captured stdout alongside the module JSON and trip the result parser with "Module invocation had junk after the JSON data". Suppress that decoration at the source by prefixing the constructed shell command with SYSTEMD_COLORS=0. TERM=dumb would work too but has a wider blast radius (it also affects interactive tools inside the become-user session); SYSTEMD_COLORS is the documented systemd-scoped knob. run0 was also missing pipelining = False. When run0 is used over a connection that honors require_tty, ansible's pipelining sends the module source on stdin to remote python3, which cannot be forwarded cleanly through the pty chain and hangs indefinitely. Disable pipelining the same way community.general.machinectl already does. Also add tests/unit/plugins/become/test_machinectl.py mirroring the existing test_run0.py. machinectl had no unit test coverage before, which is why CI did not catch the SYSTEMD_COLORS=0 prefix change when the equivalent run0 change broke test_run0_basic/test_run0_flags. * Update changelogs/fragments/11771-incus-machinectl-run0-become-pty.yml --------- (cherry picked from commit df252e5fab983688172a61db3643668708cb8b42) Co-authored-by: Martin Schürrer Co-authored-by: Claude Opus 4.6 (1M context) Co-authored-by: Alexei Znamensky <103110+russoz@users.noreply.github.com> --- ...11771-incus-machinectl-run0-become-pty.yml | 5 ++ plugins/become/machinectl.py | 5 +- plugins/become/run0.py | 12 +++- plugins/connection/incus.py | 6 ++ tests/unit/plugins/become/test_machinectl.py | 64 +++++++++++++++++++ tests/unit/plugins/become/test_run0.py | 4 +- 6 files changed, 92 insertions(+), 4 deletions(-) create mode 100644 changelogs/fragments/11771-incus-machinectl-run0-become-pty.yml create mode 100644 tests/unit/plugins/become/test_machinectl.py diff --git a/changelogs/fragments/11771-incus-machinectl-run0-become-pty.yml b/changelogs/fragments/11771-incus-machinectl-run0-become-pty.yml new file mode 100644 index 0000000000..84c10a4c42 --- /dev/null +++ b/changelogs/fragments/11771-incus-machinectl-run0-become-pty.yml @@ -0,0 +1,5 @@ +bugfixes: + - incus connection plugin - work when the active become plugin sets ``require_tty`` instead of failing silently (https://github.com/ansible-collections/community.general/pull/11771). + - machinectl become plugin - prevent printing ANSI terminal color sequences (https://github.com/ansible-collections/community.general/pull/11771). + - run0 become plugin - prevent printing ANSI terminal color sequences (https://github.com/ansible-collections/community.general/pull/11771). + - run0 become plugin - mark the plugin as incompatible with connection pipelining (see https://github.com/ansible/ansible/issues/81254, https://github.com/ansible-collections/community.general/pull/11771). diff --git a/plugins/become/machinectl.py b/plugins/become/machinectl.py index 26a8f2bc5a..11fdbe92ff 100644 --- a/plugins/become/machinectl.py +++ b/plugins/become/machinectl.py @@ -124,7 +124,10 @@ class BecomeModule(BecomeBase): flags = self.get_option("become_flags") user = self.get_option("become_user") - return f"{become} -q shell {flags} {user}@ {self._build_success_command(cmd, shell)}" + # SYSTEMD_COLORS=0 stops machinectl from appending ANSI reset + # sequences (ESC[0m, ESC[J) after the child exits, which would + # otherwise land after the module JSON and break result parsing. + return f"SYSTEMD_COLORS=0 {become} -q shell {flags} {user}@ {self._build_success_command(cmd, shell)}" def check_success(self, b_output): b_output = self.remove_ansi_codes(b_output) diff --git a/plugins/become/run0.py b/plugins/become/run0.py index 171be87958..dd46b2d24b 100644 --- a/plugins/become/run0.py +++ b/plugins/become/run0.py @@ -60,6 +60,8 @@ options: type: string notes: - This plugin only works when a C(polkit) rule is in place. + - This become plugin does not work when connection pipelining is enabled. With ansible-core 2.19+, using it automatically + disables pipelining. On ansible-core 2.18 and before, pipelining must explicitly be disabled by the user. """ EXAMPLES = r""" @@ -91,6 +93,10 @@ class BecomeModule(BecomeBase): success = ("==== AUTHENTICATION COMPLETE ====",) require_tty = True # see https://github.com/ansible-collections/community.general/issues/6932 + # See https://github.com/ansible/ansible/issues/81254, + # https://github.com/ansible/ansible/pull/78111 + pipelining = False + @staticmethod def remove_ansi_codes(line): return ansi_color_codes.sub(b"", line) @@ -105,7 +111,11 @@ class BecomeModule(BecomeBase): flags = self.get_option("become_flags") user = self.get_option("become_user") - return f"{become} --user={user} {flags} {self._build_success_command(cmd, shell)}" + # SYSTEMD_COLORS=0 stops run0 from emitting terminal control + # sequences (window title OSC, ANSI reset) around the child + # command, which would otherwise corrupt the module JSON and + # break result parsing. + return f"SYSTEMD_COLORS=0 {become} --user={user} {flags} {self._build_success_command(cmd, shell)}" def check_success(self, b_output): b_output = self.remove_ansi_codes(b_output) diff --git a/plugins/connection/incus.py b/plugins/connection/incus.py index ea3356ffd6..5fe8523d03 100644 --- a/plugins/connection/incus.py +++ b/plugins/connection/incus.py @@ -131,12 +131,18 @@ class Connection(ConnectionBase): def _build_command(self, cmd) -> list[str]: """build the command to execute on the incus host""" + # Force pseudo-terminal allocation if the active become plugin + # requires one (e.g. community.general.machinectl), otherwise the + # become helper runs without a controlling tty and silently fails. + require_tty = self.become is not None and getattr(self.become, "require_tty", False) + exec_cmd: list[str] = [ self._incus_cmd, "--project", self.get_option("project"), "exec", *(["-T"] if getattr(self._shell, "_IS_WINDOWS", False) else []), + *(["-t"] if require_tty and not getattr(self._shell, "_IS_WINDOWS", False) else []), f"{self.get_option('remote')}:{self._instance()}", "--", ] diff --git a/tests/unit/plugins/become/test_machinectl.py b/tests/unit/plugins/become/test_machinectl.py new file mode 100644 index 0000000000..d81f6c45ee --- /dev/null +++ b/tests/unit/plugins/become/test_machinectl.py @@ -0,0 +1,64 @@ +# Copyright (c) 2026 Ansible Project +# +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + +from __future__ import annotations + +import re + +from ansible import context + +from .helper import call_become_plugin + + +def test_machinectl_basic(mocker, parser, reset_cli_args): + options = parser.parse_args([]) + context._init_global_context(options) + + default_cmd = "/bin/foo" + default_exe = "/bin/sh" + machinectl_exe = "machinectl" + + success = "BECOME-SUCCESS-.+?" + + task = { + "become_method": "community.general.machinectl", + "become_user": "root", + } + var_options = {} + cmd = call_become_plugin(task, var_options, cmd=default_cmd, executable=default_exe) + assert ( + re.match( + f"SYSTEMD_COLORS=0 {machinectl_exe} -q shell root@ {default_exe} -c 'echo {success}; {default_cmd}'", + cmd, + ) + is not None + ) + + +def test_machinectl_flags(mocker, parser, reset_cli_args): + options = parser.parse_args([]) + context._init_global_context(options) + + default_cmd = "/bin/foo" + default_exe = "/bin/sh" + machinectl_exe = "machinectl" + machinectl_flags = "--setenv=FOO=bar" + + success = "BECOME-SUCCESS-.+?" + + task = { + "become_method": "community.general.machinectl", + "become_user": "root", + "become_flags": machinectl_flags, + } + var_options = {} + cmd = call_become_plugin(task, var_options, cmd=default_cmd, executable=default_exe) + assert ( + re.match( + f"SYSTEMD_COLORS=0 {machinectl_exe} -q shell --setenv=FOO=bar root@ {default_exe} -c 'echo {success}; {default_cmd}'", + cmd, + ) + is not None + ) diff --git a/tests/unit/plugins/become/test_run0.py b/tests/unit/plugins/become/test_run0.py index 01c9b026bc..a36eee6fbd 100644 --- a/tests/unit/plugins/become/test_run0.py +++ b/tests/unit/plugins/become/test_run0.py @@ -29,7 +29,7 @@ def test_run0_basic(mocker, parser, reset_cli_args): cmd = call_become_plugin(task, var_options, cmd=default_cmd, executable=default_exe) assert ( re.match( - f"{run0_exe} --user=root {default_exe} -c 'echo {success}; {default_cmd}'", + f"SYSTEMD_COLORS=0 {run0_exe} --user=root {default_exe} -c 'echo {success}; {default_cmd}'", cmd, ) is not None @@ -55,7 +55,7 @@ def test_run0_flags(mocker, parser, reset_cli_args): cmd = call_become_plugin(task, var_options, cmd=default_cmd, executable=default_exe) assert ( re.match( - f"{run0_exe} --user=root --nice=15 {default_exe} -c 'echo {success}; {default_cmd}'", + f"SYSTEMD_COLORS=0 {run0_exe} --user=root --nice=15 {default_exe} -c 'echo {success}; {default_cmd}'", cmd, ) is not None