From 798382b21489f508495482caafab44460f4bd747 Mon Sep 17 00:00:00 2001 From: nicholasyang Date: Wed, 25 Sep 2024 15:11:34 +0800 Subject: [PATCH 1/3] Fix: report: find_shell should accept hacluster user (bsc#1228899) --- crmsh/report/sh.py | 67 +++++++++++++++++++++++++++++----------------- 1 file changed, 42 insertions(+), 25 deletions(-) diff --git a/crmsh/report/sh.py b/crmsh/report/sh.py index c3627fcfe1..ec7ece1335 100644 --- a/crmsh/report/sh.py +++ b/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, + ) From 286a86e7b5d8407cf58f6991f9d68f46f8a75be4 Mon Sep 17 00:00:00 2001 From: nicholasyang Date: Tue, 24 Sep 2024 17:25:41 +0800 Subject: [PATCH 2/3] Dev: report: make error messages easier to parse for hawk2 (bsc#1228899) --- crmsh/report/core.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crmsh/report/core.py b/crmsh/report/core.py index 2f3ca66610..825c3ba150 100644 --- a/crmsh/report/core.py +++ b/crmsh/report/core.py @@ -412,8 +412,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: From 3bb92f4f7a7d59f38282f2402f47091a186c4516 Mon Sep 17 00:00:00 2001 From: nicholasyang Date: Wed, 25 Sep 2024 15:34:05 +0800 Subject: [PATCH 3/3] Dev: report: do not capture stderr when unarchiving tarballs so that errors will get reported --- crmsh/report/core.py | 17 +++++++++++------ test/unittests/test_report_core.py | 4 ++-- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/crmsh/report/core.py b/crmsh/report/core.py index 825c3ba150..573486c4ef 100644 --- a/crmsh/report/core.py +++ b/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: diff --git a/test/unittests/test_report_core.py b/test/unittests/test_report_core.py index 3be9fd67e8..54002ff677 100644 --- a/test/unittests/test_report_core.py +++ b/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,