From d9b895b319551b4b0bfcc6a09cfe76334a6752db Mon Sep 17 00:00:00 2001 From: Will Thames Date: Tue, 5 Aug 2014 10:41:26 +1000 Subject: [PATCH] Unarchive should work when parent directory is not writable Correct unarchive so that the checks for writeability are sensible. Added a test for when parent directory is not writable --- library/files/unarchive | 10 +++---- .../roles/test_unarchive/tasks/main.yml | 26 ++++++++++++++++++- 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/library/files/unarchive b/library/files/unarchive index 460353a63b..096f035512 100644 --- a/library/files/unarchive +++ b/library/files/unarchive @@ -115,8 +115,6 @@ class TgzFile(object): self.zipflag = 'z' def is_unarchived(self): - dirof = os.path.dirname(self.dest) - destbase = os.path.basename(self.dest) cmd = '%s -v -C "%s" --diff -%sf "%s"' % (self.cmd_path, self.dest, self.zipflag, self.src) rc, out, err = self.module.run_command(cmd) unarchived = (rc == 0) @@ -220,10 +218,10 @@ def main(): ) # is dest OK to receive tar file? - if not os.path.exists(os.path.dirname(dest)): - module.fail_json(msg="Destination directory '%s' does not exist" % (os.path.dirname(dest))) - if not os.access(os.path.dirname(dest), os.W_OK): - module.fail_json(msg="Destination '%s' not writable" % (os.path.dirname(dest))) + if not os.path.isdir(dest): + module.fail_json(msg="Destination '%s' is not a directory" % dest) + if not os.access(dest, os.W_OK): + module.fail_json(msg="Destination '%s' not writable" % dest) handler = pick_handler(src, dest, module) diff --git a/test/integration/roles/test_unarchive/tasks/main.yml b/test/integration/roles/test_unarchive/tasks/main.yml index 56b31e6b2d..073ccf9145 100644 --- a/test/integration/roles/test_unarchive/tasks/main.yml +++ b/test/integration/roles/test_unarchive/tasks/main.yml @@ -110,4 +110,28 @@ file: path={{output_dir}}/test-unarchive-zip state=absent - name: remove our test file for the archive - file: path={{output_dir}}/foo-unarchive.txt state=absent \ No newline at end of file + file: path={{output_dir}}/foo-unarchive.txt state=absent + +- name: check if /tmp/foo-unarchive.text exists + stat: path=/tmp/foo-unarchive.txt + ignore_errors: True + register: unarchive04 + +- name: fail if the proposed destination file exists for safey + fail: msg="/tmp/foo-unarchive.txt already exists, aborting" + when: unarchive04.stat.exists + +- name: try unarchiving to /tmp + unarchive: src={{output_dir}}/test-unarchive.tar.gz dest=/tmp copy=no + register: unarchive05 + +- name: verify that the file was marked as changed + assert: + that: + - "unarchive05.changed == true" + +- name: verify that the file was unarchived + file: path=/tmp/foo-unarchive.txt state=file + +- name: remove our unarchive destination + file: path=/tmp/foo-unarchive.txt state=absent