Fix: report: find_shell should accept hacluster user (bsc#1228899) (C…

…lusterLabs#1564)

1. Fix `crm.report.sh.Shell.subprocess_run_without_input`. It is
expected to run commands as a specified user on a specified host. The
user and host is carried in `Shell` instance, so that callers do not
need to specify one when calling method `subprocess_run_without_input`.
2. Modify `crm.report.sh.Shell. _try_create_report_shell` to make it
work for 2 different senerios:
1. When the specified user is `root` or `hacluster`, returns a
`crmsh.sh.SSHShell` wrapped in an adaptor `crm.report.sh.SSHShell`, so
that commands runs directly as the specified user.
2. When the specified user is not `root` or `hacluster`, try to run
`sudo` with the `SSHShell`. If
1. sudo works, returns a `crmsh.report.sh.SSHSudoShell`, which prepends
a `sudo` to the command and runs it with `crmsh.SSHShell`.
2. sudo does not work, return None, failing to create a usable instance
of `crm.report.sh.ShellShell`.

crmsh/report/core.py

@@ -11,7 +11,6 @@
 import shutil
 import json
 import ast
-import typing
 from inspect import getmembers, isfunction
 from io import StringIO
 from typing import List
@@ -280,25 +279,31 @@ def start_collector(node: str, context: Context) -> None:
     elif ret.stderr:
         print(crmsh.sh.Utils.decode_str(ret.stderr), file=sys.stderr)
 
-    compress_data = ""
+    archive_data_literal = ""
     for data in ret.stdout.decode('utf-8').split("\n"):
         if data.startswith(constants.COMPRESS_DATA_FLAG):
             # crm report data from collector
-            compress_data = data.lstrip(constants.COMPRESS_DATA_FLAG)
+            archive_data_literal = data.lstrip(constants.COMPRESS_DATA_FLAG)
         else:
             # INFO log data from collector
             print(data)
 
     try:
         # Safely evaluate the string representation of a tarball from push_data
-        data_object = ast.literal_eval(compress_data)
+        archive_data = ast.literal_eval(archive_data_literal)
     except (SyntaxError, ValueError) as e:
         logger.error(f"Error evaluating data: {e}")
         return
 
     # Extract the tarball in the specified working directory
-    cmd = f"cd {context.work_dir} && tar x"
-    ShellUtils().get_stdout(cmd, input_s=data_object)
+    child = subprocess.Popen(
+        ['tar', '-x'],
+        cwd=context.work_dir,
+        stdin=subprocess.PIPE,
+    )
+    child.stdin.write(archive_data)
+    child.stdin.close()
+    child.wait()
 
 
 def process_dest(context: Context) -> None:
@@ -412,8 +417,9 @@ def find_ssh_user(context: Context) -> None:
             if not crmutils.can_ask():
                 logger.error('Cannot create a report non-interactively. Interactive authentication is required.')
                 if userdir.getuser() == 'hacluster':
-                    logger.warning('Passwordless ssh does not work. Run "crm cluster health hawk2 --fix" to set it up.')
-                raise ValueError('Cannot create a report.')
+                    raise ValueError('Passwordless ssh does not work. Run "crm cluster health hawk2 --fix" to set it up.')
+                else:
+                    raise ValueError('Cannot create a report.')
 
 
 def load_from_crmsh_config(context: Context) -> None:

crmsh/report/sh.py

@@ -43,41 +43,58 @@ def _try_create_report_shell(local_shell: crmsh.sh.LocalShell, host: str, user:
         # call can_run_as here to populate know_hosts
         if not ssh_shell.can_run_as(host, user):
             return None
-        # check for root privilege
-        ret = ssh_shell.subprocess_run_without_input(
-                host, user,
-                'true' if user == 'root' else 'sudo true',
-                start_new_session=True,
-                stdout=subprocess.DEVNULL,
-                stderr=subprocess.DEVNULL,
-        )
-        if ret.returncode == 0:
-            return Shell(local_shell, host, user)
+        if user == 'root' or user == 'hacluster':
+            return SSHShell(ssh_shell, host, user)
         else:
-            return None
+            # check for root/hacluster privilege
+            ret = ssh_shell.subprocess_run_without_input(
+                    host, user,
+                    'sudo true',
+                    start_new_session=True,
+                    stdout=subprocess.DEVNULL,
+                    stderr=subprocess.DEVNULL,
+            )
+            if ret.returncode == 0:
+                return SSHSudoShell(ssh_shell, host, user)
+            else:
+                return None
+
+    def subprocess_run_without_input(self, cmd: str, **kwargs):
+        raise NotImplementedError
+
+
+class ClusterShellAdaptor(Shell):
+    def __init__(self, cluster_shell: crmsh.sh.ClusterShell, host):
+        self.cluster_shell = cluster_shell
+        self._host = host
+
+    def subprocess_run_without_input(self, cmd: str, **kwargs):
+        return self.cluster_shell.subprocess_run_without_input(self._host, None, cmd, **kwargs)
+
 
-    def __init__(self, local_shell: crmsh.sh.LocalShell, host: str, user: str):
-        self.local_shell = local_shell
+class SSHShell(Shell):
+    def __init__(self, ssh_shell: crmsh.sh.SSHShell, host, user):
+        self.ssh_shell = ssh_shell
         self._host = host
         self._user = user
 
     def subprocess_run_without_input(self, cmd: str, **kwargs):
-        if self._user == self.local_shell.get_effective_user_name():
-            args = ['/bin/sh']
-        else:
-            args = ['sudo', '-H', '-u', self._user, '/bin/sh']
-        return subprocess.run(
-            args,
-            input=cmd.encode('utf-8'),
-            env=os.environ,  # bsc#1205925
+        return self.ssh_shell.subprocess_run_without_input(
+            self._host, self._user,
+            cmd,
             **kwargs,
         )
 
 
-class ClusterShellAdaptor:
-    def __init__(self, cluster_shell: crmsh.sh.ClusterShell, host):
-        self.cluster_shell = cluster_shell
+class SSHSudoShell(Shell):
+    def __init__(self, ssh_shell: crmsh.sh.SSHShell, host, user):
+        self.ssh_shell = ssh_shell
         self._host = host
+        self._user = user
 
     def subprocess_run_without_input(self, cmd: str, **kwargs):
-        return self.cluster_shell.subprocess_run_without_input(self._host, None, cmd, **kwargs)
+        return self.ssh_shell.subprocess_run_without_input(
+            self._host, self._user,
+            f'sudo {cmd}',
+            **kwargs,
+        )

test/unittests/test_report_core.py

@@ -491,9 +491,9 @@ def test_process_arguments(self, mock_dest, mock_node_list):
         core.process_arguments(mock_ctx_inst)
 
 
-    @mock.patch('crmsh.sh.ShellUtils.get_stdout')
+    @mock.patch('subprocess.Popen')
     @mock.patch('ast.literal_eval')
-    def test_start_collector(self, mock_literal_eval, mock_get_stdout):
+    def test_start_collector(self, mock_literal_eval, mock_popen):
         mock_shell = mock.Mock(crmsh.report.sh.Shell)
         mock_context = mock.Mock(
             ssh_user=None,