From afe6eb574ef8e53e805cc1ff2b25dd9108b29e1c Mon Sep 17 00:00:00 2001 From: Sam Doran Date: Wed, 16 Jun 2021 18:08:34 -0400 Subject: [PATCH] slurp - improve error code and test coverage (#75038) * Improve the error handling code Rather than multiple return paths, have a single return and set the message based on the type of failure. * Add another test for non-specific failures * Reorganize tests so failure tests are in one tasks file * Remove os.stat() call and add changelog --- ...urp-improve-error-handling-readability.yml | 2 + lib/ansible/modules/slurp.py | 11 ++-- test/integration/targets/slurp/tasks/main.yml | 35 +---------- .../targets/slurp/tasks/test_unreadable.yml | 61 ++++++++++++++----- 4 files changed, 54 insertions(+), 55 deletions(-) create mode 100644 changelogs/fragments/slurp-improve-error-handling-readability.yml diff --git a/changelogs/fragments/slurp-improve-error-handling-readability.yml b/changelogs/fragments/slurp-improve-error-handling-readability.yml new file mode 100644 index 00000000000..099c5b3e649 --- /dev/null +++ b/changelogs/fragments/slurp-improve-error-handling-readability.yml @@ -0,0 +1,2 @@ +bugfixes: + - slurp - improve the logic in the error handling and remove ``os.stat()`` call (https://github.com/ansible/ansible/pull/75038) diff --git a/lib/ansible/modules/slurp.py b/lib/ansible/modules/slurp.py index fdfe66a9a9e..0b9d8b6e217 100644 --- a/lib/ansible/modules/slurp.py +++ b/lib/ansible/modules/slurp.py @@ -94,18 +94,19 @@ def main(): source = module.params['src'] try: - os.stat(source) with open(source, 'rb') as source_fh: source_content = source_fh.read() except (IOError, OSError) as e: if e.errno == errno.ENOENT: - module.fail_json(msg="file not found: %s" % source) + msg = "file not found: %s" % source elif e.errno == errno.EACCES: - module.fail_json(msg="file is not readable: %s" % source) + msg = "file is not readable: %s" % source elif e.errno == errno.EISDIR: - module.fail_json(msg="source is a directory and must be a file: %s" % source) + msg = "source is a directory and must be a file: %s" % source else: - module.fail_json(msg="unable to slurp file: %s" % to_native(e, errors='surrogate_then_replace')) + msg = "unable to slurp file: %s" % to_native(e, errors='surrogate_then_replace') + + module.fail_json(msg) data = base64.b64encode(source_content) diff --git a/test/integration/targets/slurp/tasks/main.yml b/test/integration/targets/slurp/tasks/main.yml index 4c0e733953f..6d14f4b0033 100644 --- a/test/integration/targets/slurp/tasks/main.yml +++ b/test/integration/targets/slurp/tasks/main.yml @@ -54,37 +54,6 @@ - "slurp_binary is not changed" - "slurp_binary is not failed" -- name: test slurping a non-existent file - slurp: - src: '{{ output_dir }}/i_do_not_exist' - register: slurp_missing - ignore_errors: true - -- name: check slurp missing result - assert: - that: - - "slurp_missing is failed" - - "slurp_missing.msg" - - "slurp_missing is not changed" - -- name: Create a directory to test with - file: - path: '{{ output_dir }}/baz/' - state: directory - -- name: test slurping a directory - slurp: - src: '{{ output_dir }}/baz' - register: slurp_dir - ignore_errors: true - -- name: check slurp directory result - assert: - that: - - slurp_dir is failed - - slurp_dir.msg is search('source is a directory and must be a file') - - slurp_dir is not changed - - name: test slurp with missing argument action: slurp register: slurp_no_args @@ -97,6 +66,4 @@ - "slurp_no_args.msg" - "slurp_no_args is not changed" -# Ensure unreadable file and directory handling and error messages -# https://github.com/ansible/ansible/issues/67340 -- import_tasks: 'test_unreadable.yml' +- import_tasks: test_unreadable.yml diff --git a/test/integration/targets/slurp/tasks/test_unreadable.yml b/test/integration/targets/slurp/tasks/test_unreadable.yml index f7b6bf8590c..da5e36af044 100644 --- a/test/integration/targets/slurp/tasks/test_unreadable.yml +++ b/test/integration/targets/slurp/tasks/test_unreadable.yml @@ -1,3 +1,22 @@ +- name: test slurping a non-existent file + slurp: + src: '{{ output_dir }}/i_do_not_exist' + register: slurp_missing + ignore_errors: yes + +- name: Create a directory to test with + file: + path: '{{ output_dir }}/baz/' + state: directory + +- name: test slurping a directory + slurp: + src: '{{ output_dir }}/baz' + register: slurp_dir + ignore_errors: yes + +# Ensure unreadable file and directory handling and error messages +# https://github.com/ansible/ansible/issues/67340 - name: create test user user: name: "{{ become_test_user }}" @@ -16,37 +35,47 @@ slurp: src: "{{ output_dir }}/qux.txt" register: slurp_unreadable_file - become: true + become: yes become_user: "{{ become_test_user }}" become_method: su - ignore_errors: true - -- name: check slurp unreadable file result - assert: - that: - - "slurp_unreadable_file is failed" - - "slurp_unreadable_file.msg is regex('^file is not readable:')" - - "slurp_unreadable_file is not changed" + ignore_errors: yes - name: create unreadable directory file: path: "{{ output_dir }}/test_data" state: directory - mode: 0700 + mode: '0700' owner: root - name: test slurp unreadable directory slurp: src: "{{ output_dir }}/test_data" register: slurp_unreadable_dir - become: true + become: yes become_user: "{{ become_test_user }}" become_method: su - ignore_errors: true + ignore_errors: yes + +- name: Try to access file as directory + slurp: + src: "{{ output_dir }}/qux.txt/somefile" + ignore_errors: yes + register: slurp_path_file_as_dir -- name: check slurp unreadable directory result +- name: check slurp failures assert: that: - - "slurp_unreadable_dir is failed" - - "slurp_unreadable_dir.msg is regex('^file is not readable:')" - - "slurp_unreadable_dir is not changed" + - slurp_missing is failed + - slurp_missing.msg is search('file not found') + - slurp_missing is not changed + - slurp_unreadable_file is failed + - slurp_unreadable_file.msg is regex('^file is not readable:') + - slurp_unreadable_file is not changed + - slurp_unreadable_dir is failed + - slurp_unreadable_dir.msg is regex('^file is not readable:') + - slurp_unreadable_dir is not changed + - slurp_path_file_as_dir is failed + - slurp_path_file_as_dir is search('unable to slurp file') + - slurp_dir is failed + - slurp_dir.msg is search('source is a directory and must be a file') + - slurp_dir is not changed