[collector] Saltstack transport remote_exec handle stderr like other transports

[TrevorBenson]

---

Please place an 'X' inside each '[]' to confirm you adhere to our Contributor Guidelines

• Is the commit message split over multiple lines and hard-wrapped at 72 characters?
• Is the subject and message clear and concise?
• Does the subject start with [plugin_name] if submitting a plugin patch or a [section_name] if part of the core sosreport code?
• Does the commit contain a Signed-off-by: First Lastname email@example.com?
• Are any related Issues or existing PRs properly referenced via a Closes (Issue) or Resolved (PR) line?
• Are all passwords or private data gathered by this PR obfuscated?

Closes #3860

[packit-as-a-service]

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

• sudo yum install -y dnf-plugins-core on RHEL 8
• sudo dnf install -y dnf-plugins-core on Fedora
• dnf copr enable packit/sosreport-sos-3886
• And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

[packit-as-a-service]

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

• sudo yum install -y dnf-plugins-core on RHEL 8
• sudo dnf install -y dnf-plugins-core on Fedora
• dnf copr enable packit/sosreport-sos-3886
• And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

[TurboTurtle]

Not knowing saltstack myself, can you give some context as to the implications of this change? At a high level I'm assuming this means we're wrapping everything inside a bash invocation, but what's the significance of the cmd.exec_code bit?

[TrevorBenson]

Not knowing saltstack myself, can you give some context as to the implications of this change? At a high level I'm assuming this means we're wrapping everything inside a bash invocation, but what's the significance of the cmd.exec_code bit?

Short answer, it returns only the exit code 127 for command not found, but not the string.

---

The long answer:

The cmd.shell with either /bin/sh, or /bin/bash when use_shell=True, returned the exit code, but includes the stderr in the output the master receives. This in result appears as if it was stdout.

Adding some debug I examined pkg_list and found it containing the stderr (which control persist did not):

[server-1:run_command] Shell requested, command is now /bin/bash -c 'flatpak list --columns=name,version,branch'
[FlatpakPackageManager._parse_pkg_list] pkg_list: /bin/bash: flatpak: command not found

Applicable lines:

sos/sos/policies/package_managers/flatpak.py

Lines 23 to 28 in b7cdc23

	def _parse_pkg_list(self, pkg_list):
	for line in pkg_list.splitlines():
	pkg = line.split("\t")
	yield (pkg[0], pkg[1], pkg[2])

Changing the module to cmd.exec_code bash the exit code 127 is still reported, but stderr no longer shows up in the output returned to the master from each minion (only the actual stdout does), and pkg_list is empty when flatpak is not installed.

[TrevorBenson]

Looking back at run_command and the logic from use_shell this should probably instead be cmd.exec_code sh, which should also fix the issue. Then the behavior should be more consistent with prior functionality and other transports.

sos/sos/collector/transports/__init__.py

Lines 230 to 233 in b7cdc23

	if (use_shell is True or
	(self._need_shell if use_shell == 'auto' else False)):
	cmd = f"/bin/bash -c {quote(cmd)}"
	self.log_debug(f"Shell requested, command is now {cmd}")

[TurboTurtle]

Thanks for the explanation! Makes sense to me.