From e42d859d416d98750757e4c515e404e9c4ac5964 Mon Sep 17 00:00:00 2001 From: Jean Raby Date: Wed, 24 Nov 2021 20:48:20 -0500 Subject: [PATCH 1/6] Keep stderr when pick_handler fails msg is a lot longer but the real error message will be in there. --- lib/ansible/modules/unarchive.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/ansible/modules/unarchive.py b/lib/ansible/modules/unarchive.py index e27cb37c8de68f..da2845ce79858d 100644 --- a/lib/ansible/modules/unarchive.py +++ b/lib/ansible/modules/unarchive.py @@ -739,7 +739,7 @@ def can_handle_archive(self): rc, out, err = self.module.run_command(cmd) if rc == 0: return True, None - return False, 'Command "%s" could not handle archive.' % self.cmd_path + return False, 'Command "%s" could not handle archive: %s' % (self.cmd_path, err) class TgzArchive(object): @@ -788,7 +788,7 @@ def files_in_archive(self): locale = get_best_parsable_locale(self.module) rc, out, err = self.module.run_command(cmd, cwd=self.b_dest, environ_update=dict(LANG=locale, LC_ALL=locale, LC_MESSAGES=locale, LANGUAGE=locale)) if rc != 0: - raise UnarchiveError('Unable to list files in the archive') + raise UnarchiveError(err) for filename in out.splitlines(): # Compensate for locale-related problems in gtar output (octal unicode representation) #11348 @@ -907,8 +907,8 @@ def can_handle_archive(self): try: if self.files_in_archive: return True, None - except UnarchiveError: - return False, 'Command "%s" could not handle archive.' % self.cmd_path + except UnarchiveError as e: + return False, 'Command "%s" could not handle archive: %s' % (self.cmd_path, e) # Errors and no files in archive assume that we weren't able to # properly unarchive it return False, 'Command "%s" found no files in archive. Empty archive files are not supported.' % self.cmd_path @@ -952,15 +952,15 @@ def __init__(self, src, b_dest, file_args, module): # try handlers in order and return the one that works or bail if none work def pick_handler(src, dest, file_args, module): handlers = [ZipArchive, TgzArchive, TarArchive, TarBzipArchive, TarXzArchive, TarZstdArchive] - reasons = set() + reasons = [] for handler in handlers: obj = handler(src, dest, file_args, module) (can_handle, reason) = obj.can_handle_archive() if can_handle: return obj - reasons.add(reason) - reason_msg = ' '.join(reasons) - module.fail_json(msg='Failed to find handler for "%s". Make sure the required command to extract the file is installed. %s' % (src, reason_msg)) + reasons.append("%s: %s" % (handler.__name__, reason)) + reason_msg = '\n'.join(reasons) + module.fail_json(msg='Failed to find handler for "%s". Make sure the required command to extract the file is installed.\n%s' % (src, reason_msg)) def main(): From 1909f990beb21d4236232d9d5fc0139e54f620e9 Mon Sep 17 00:00:00 2001 From: Jean Raby Date: Mon, 13 Dec 2021 21:13:43 -0500 Subject: [PATCH 2/6] Apply suggestions from code review Co-authored-by: Sloane Hertel <19572925+s-hertel@users.noreply.github.com> --- lib/ansible/modules/unarchive.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ansible/modules/unarchive.py b/lib/ansible/modules/unarchive.py index da2845ce79858d..d1fc991003d55b 100644 --- a/lib/ansible/modules/unarchive.py +++ b/lib/ansible/modules/unarchive.py @@ -788,7 +788,7 @@ def files_in_archive(self): locale = get_best_parsable_locale(self.module) rc, out, err = self.module.run_command(cmd, cwd=self.b_dest, environ_update=dict(LANG=locale, LC_ALL=locale, LC_MESSAGES=locale, LANGUAGE=locale)) if rc != 0: - raise UnarchiveError(err) + raise UnarchiveError('Unable to list files in the archive: %s' % err) for filename in out.splitlines(): # Compensate for locale-related problems in gtar output (octal unicode representation) #11348 @@ -908,7 +908,7 @@ def can_handle_archive(self): if self.files_in_archive: return True, None except UnarchiveError as e: - return False, 'Command "%s" could not handle archive: %s' % (self.cmd_path, e) + return False, 'Command "%s" could not handle archive: %s' % (self.cmd_path, to_native(e)) # Errors and no files in archive assume that we weren't able to # properly unarchive it return False, 'Command "%s" found no files in archive. Empty archive files are not supported.' % self.cmd_path From 7f22e243e7d789b132d1a2c726f15c6703395243 Mon Sep 17 00:00:00 2001 From: Jean Raby Date: Mon, 13 Dec 2021 21:18:14 -0500 Subject: [PATCH 3/6] revert adding handler name to error msg --- lib/ansible/modules/unarchive.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ansible/modules/unarchive.py b/lib/ansible/modules/unarchive.py index d1fc991003d55b..ee90f974a72a25 100644 --- a/lib/ansible/modules/unarchive.py +++ b/lib/ansible/modules/unarchive.py @@ -952,13 +952,13 @@ def __init__(self, src, b_dest, file_args, module): # try handlers in order and return the one that works or bail if none work def pick_handler(src, dest, file_args, module): handlers = [ZipArchive, TgzArchive, TarArchive, TarBzipArchive, TarXzArchive, TarZstdArchive] - reasons = [] + reasons = set() for handler in handlers: obj = handler(src, dest, file_args, module) (can_handle, reason) = obj.can_handle_archive() if can_handle: return obj - reasons.append("%s: %s" % (handler.__name__, reason)) + reasons.add(reason) reason_msg = '\n'.join(reasons) module.fail_json(msg='Failed to find handler for "%s". Make sure the required command to extract the file is installed.\n%s' % (src, reason_msg)) From 053709e9344e1a20083d1458b2e5d6d4305ee64f Mon Sep 17 00:00:00 2001 From: s-hertel <19572925+s-hertel@users.noreply.github.com> Date: Tue, 21 Dec 2021 17:11:58 -0500 Subject: [PATCH 4/6] Include error listing files in the archive for ZipArchive too --- lib/ansible/modules/unarchive.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ansible/modules/unarchive.py b/lib/ansible/modules/unarchive.py index ee90f974a72a25..738df631623539 100644 --- a/lib/ansible/modules/unarchive.py +++ b/lib/ansible/modules/unarchive.py @@ -391,9 +391,9 @@ def files_in_archive(self): break if not exclude_flag: self._files_in_archive.append(to_native(member)) - except Exception: + except Exception as e: archive.close() - raise UnarchiveError('Unable to list files in the archive') + raise UnarchiveError('Unable to list files in the archive: %s' % to_native(e)) archive.close() return self._files_in_archive From b892e9d39f6aca556b1aca48c84e1fbd54c7a84d Mon Sep 17 00:00:00 2001 From: s-hertel <19572925+s-hertel@users.noreply.github.com> Date: Tue, 21 Dec 2021 17:49:16 -0500 Subject: [PATCH 5/6] Add a test to unarchive a tar file with an invalid extra option --- .../targets/unarchive/tasks/main.yml | 1 + .../unarchive/tasks/test_invalid_options.yml | 27 +++++++++++++++++++ 2 files changed, 28 insertions(+) create mode 100644 test/integration/targets/unarchive/tasks/test_invalid_options.yml diff --git a/test/integration/targets/unarchive/tasks/main.yml b/test/integration/targets/unarchive/tasks/main.yml index baa2a8cf22819d..60bdd1b08f37d7 100644 --- a/test/integration/targets/unarchive/tasks/main.yml +++ b/test/integration/targets/unarchive/tasks/main.yml @@ -18,3 +18,4 @@ - import_tasks: test_download.yml - import_tasks: test_unprivileged_user.yml - import_tasks: test_different_language_var.yml +- import_tasks: test_invalid_options.yml diff --git a/test/integration/targets/unarchive/tasks/test_invalid_options.yml b/test/integration/targets/unarchive/tasks/test_invalid_options.yml new file mode 100644 index 00000000000000..68a0621355d620 --- /dev/null +++ b/test/integration/targets/unarchive/tasks/test_invalid_options.yml @@ -0,0 +1,27 @@ +- name: create our tar unarchive destination + file: + path: '{{remote_tmp_dir}}/test-unarchive-tar' + state: directory + +- name: unarchive a tar file with an invalid option + unarchive: + src: '{{remote_tmp_dir}}/test-unarchive.tar' + dest: '{{remote_tmp_dir}}/test-unarchive-tar' + remote_src: yes + extra_opts: + - "--invalid-éxtra-optら" + ignore_errors: yes + register: unarchive + +- name: verify that the invalid option is in the error message + assert: + that: + - "unarchive is failed" + - "unarchive['msg'] is search(msg)" + vars: + msg: "Unable to list files in the archive: /.*/(tar|gtar): unrecognized option '--invalid-éxtra-optら'" + +- name: remove our tar unarchive destination + file: + path: '{{remote_tmp_dir}}/test-unarchive-tar' + state: absent From 019f2d7c320d5bdfffce7c05f4540e8ad1d8b7f3 Mon Sep 17 00:00:00 2001 From: s-hertel <19572925+s-hertel@users.noreply.github.com> Date: Tue, 21 Dec 2021 17:54:24 -0500 Subject: [PATCH 6/6] add a changelog --- .../76365-unarchive-include-stderr-in-error-message.yml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelogs/fragments/76365-unarchive-include-stderr-in-error-message.yml diff --git a/changelogs/fragments/76365-unarchive-include-stderr-in-error-message.yml b/changelogs/fragments/76365-unarchive-include-stderr-in-error-message.yml new file mode 100644 index 00000000000000..79e273ae0b2701 --- /dev/null +++ b/changelogs/fragments/76365-unarchive-include-stderr-in-error-message.yml @@ -0,0 +1,2 @@ +bugfixes: + - unarchive - include the original error when a handler cannot manage the archive (https://github.com/ansible/ansible/issues/28977).