From 25e67d804c7b830e6f0d1c35132362ca3dbef3d7 Mon Sep 17 00:00:00 2001
From: Dag Wieers <dag@wieers.com>
Date: Fri, 4 Aug 2017 20:38:42 +0200
Subject: [PATCH] iso_extract: Reimplement using 7zip (not requiring root)
 (#24937)

* Reimplement iso_extract using 7zip (not requiring root)

So one of the drawbacks of the original implementation is that it required root for mounting/unmount the ISO image.
This is now no longer needed as we use 7zip for extracting files from the ISO.

* Fall back to using mount/umount if 7zip not found

As discussed with others.

Also improved integration tests.
---
 lib/ansible/modules/files/iso_extract.py      | 166 ++++++++++++++----
 test/integration/targets/iso_extract/aliases  |   3 -
 .../targets/iso_extract/files/test.iso        | Bin 53248 -> 374784 bytes
 .../targets/iso_extract/tasks/7zip.yml        |  74 ++++++++
 .../targets/iso_extract/tasks/main.yml        |  38 ++--
 .../targets/iso_extract/tasks/prepare.yml     |  33 ++++
 .../targets/iso_extract/tasks/tests.yml       |  52 ++++++
 test/sanity/pep8/legacy-files.txt             |   1 -
 8 files changed, 307 insertions(+), 60 deletions(-)
 create mode 100644 test/integration/targets/iso_extract/tasks/7zip.yml
 create mode 100644 test/integration/targets/iso_extract/tasks/prepare.yml
 create mode 100644 test/integration/targets/iso_extract/tasks/tests.yml

diff --git a/lib/ansible/modules/files/iso_extract.py b/lib/ansible/modules/files/iso_extract.py
index 5e0639eb014..ded040ab545 100644
--- a/lib/ansible/modules/files/iso_extract.py
+++ b/lib/ansible/modules/files/iso_extract.py
@@ -3,6 +3,7 @@
 
 # Copyright: (c) 2013, Jeroen Hoekx <jeroen.hoekx@dsquare.be>
 # Copyright: (c) 2016, Matt Robinson <git@nerdoftheherd.com>
+# Copyright: (c) 2017, Dag Wieers <dag@wieers.com>
 # Copyright: (c) 2017, Ansible Project
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
@@ -13,36 +14,60 @@ ANSIBLE_METADATA = {'metadata_version': '1.0',
                     'status': ['preview'],
                     'supported_by': 'community'}
 
-
 DOCUMENTATION = r'''
 ---
 author:
 - Jeroen Hoekx (@jhoekx)
 - Matt Robinson (@ribbons)
+- Dag Wieers (@dagwieers)
 module: iso_extract
-short_description: Extract files from an ISO image.
+short_description: Extract files from an ISO image
 description:
-- This module mounts an iso image in a temporary directory and extracts
-  files from there to a given destination.
-version_added: "2.3"
+- This module has two possible ways of operation.
+- If 7zip is installed on the system, this module extracts files from an ISO
+  into a temporary directory and copies files to a given destination,
+  if needed.
+- If the user has mount-capabilities (CAP_SYS_ADMIN on Linux) this module
+  mounts the ISO image to a temporary location, and copies files to a given
+  destination, if needed.
+version_added: '2.3'
+requirements:
+- Either 7z (from I(7zip) or I(p7zip) package)
+- Or mount capabilities (root-access, or CAP_SYS_ADMIN capability on Linux)
 options:
   image:
     description:
     - The ISO image to extract files from.
-    required: true
-    aliases: ['path', 'src']
+    required: yes
+    aliases: [ path, src ]
   dest:
     description:
     - The destination directory to extract files to.
-    required: true
+    required: yes
   files:
     description:
     - A list of files to extract from the image.
     - Extracting directories does not work.
-    required: true
+    required: yes
+  force:
+    description:
+    - If C(yes), which will replace the remote file when contents are different than the source.
+    - If C(no), the file will only be extracted and copied if the destination does not already exist.
+    type: bool
+    default: 'yes'
+    aliases: [ thirsty ]
+    version_added: '2.4'
+  executable:
+    description:
+    - The path to the C(7z) executable to use for extracting files from the ISO.
+    default: '7z'
+    version_added: '2.4'
 notes:
-- Only the file hash (content) is taken into account for extracting files
-  from the ISO image.
+- Only the file checksum (content) is taken into account when extracting files
+  from the ISO image. If C(force=no),  only checks the presence of the file.
+- In Ansible v2.3 this module was using C(mount) and C(umount) commands only,
+  requiring root access. This is no longer needed with the introduction of 7zip
+  for extraction.
 '''
 
 EXAMPLES = r'''
@@ -59,63 +84,132 @@ RETURN = r'''
 #
 '''
 
-import os
+import os.path
 import shutil
 import tempfile
 
+try:  # python 3.3+
+    from shlex import quote
+except ImportError:  # older python
+    from pipes import quote
+
 from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.pycompat24 import get_exception
+
 
 def main():
     module = AnsibleModule(
-        argument_spec = dict(
-            image = dict(required=True, type='path', aliases=['path', 'src']),
-            dest = dict(required=True, type='path'),
-            files = dict(required=True, type='list'),
+        argument_spec=dict(
+            image=dict(type='path', required=True, aliases=['path', 'src']),
+            dest=dict(type='path', required=True),
+            files=dict(type='list', required=True),
+            force=dict(type='bool', default=True, aliases=['thirsty']),
+            executable=dict(type='path'),  # No default on purpose
         ),
-        supports_check_mode = True,
+        supports_check_mode=True,
     )
     image = module.params['image']
     dest = module.params['dest']
     files = module.params['files']
+    force = module.params['force']
+    executable = module.params['executable']
 
-    changed = False
+    result = dict(
+        changed=False,
+        dest=dest,
+        image=image,
+    )
+
+    # We want to know if the user provided it or not, so we set default here
+    if executable is None:
+        executable = '7z'
+
+    binary = module.get_bin_path(executable, None)
+
+    # When executable was provided and binary not found, warn user !
+    if module.params['executable'] is not None and not binary:
+        module.warn("Executable '%s' is not found on the system, trying to mount ISO instead." % executable)
 
     if not os.path.exists(dest):
-        module.fail_json(msg='Directory "%s" does not exist' % dest)
+        module.fail_json(msg="Directory '%s' does not exist" % dest)
 
     if not os.path.exists(os.path.dirname(image)):
-        module.fail_json(msg='ISO image "%s" does not exist' % image)
+        module.fail_json(msg="ISO image '%s' does not exist" % image)
+
+    result['files'] = []
+    extract_files = list(files)
+
+    if not force:
+        # Check if we have to process any files based on existence
+        for f in files:
+            dest_file = os.path.join(dest, os.path.basename(f))
+            if os.path.exists(dest_file):
+                result['files'].append(dict(
+                    checksum=None,
+                    dest=dest_file,
+                    src=f,
+                ))
+                extract_files.remove(f)
+
+    if not extract_files:
+        module.exit_json(**result)
 
     tmp_dir = tempfile.mkdtemp()
-    rc, out, err = module.run_command('mount -o loop,ro "%s" "%s"' % (image, tmp_dir))
+
+    # Use 7zip when we have a binary, otherwise try to mount
+    if binary:
+        cmd = '%s x "%s" -o"%s" %s' % (binary, image, tmp_dir, ' '.join([quote(f) for f in extract_files]))
+    else:
+        cmd = 'mount -o loop,ro "%s" "%s"' % (image, tmp_dir)
+
+    rc, out, err = module.run_command(cmd)
     if rc != 0:
-        os.rmdir(tmp_dir)
-        module.fail_json(msg='Failed to mount ISO image "%s"' % image)
+        result.update(dict(
+            cmd=cmd,
+            rc=rc,
+            stderr=err,
+            stdout=out,
+        ))
+        shutil.rmtree(tmp_dir)
+
+        if binary:
+            module.fail_json(msg="Failed to extract from ISO image '%s' to '%s'" % (image, tmp_dir), **result)
+        else:
+            module.fail_json(msg="Failed to mount ISO image '%s' to '%s', and we could not find executable '%s'." % (image, tmp_dir, executable), **result)
 
-    e = None
     try:
-        for file in files:
-            tmp_src = os.path.join(tmp_dir, file)
-            src_hash = module.sha1(tmp_src)
+        for f in extract_files:
+            tmp_src = os.path.join(tmp_dir, f)
+            if not os.path.exists(tmp_src):
+                module.fail_json(msg="Failed to extract '%s' from ISO image" % f, **result)
 
-            dest_file = os.path.join(dest, os.path.basename(file))
+            src_checksum = module.sha1(tmp_src)
+
+            dest_file = os.path.join(dest, os.path.basename(f))
 
             if os.path.exists(dest_file):
-                dest_hash = module.sha1(dest_file)
+                dest_checksum = module.sha1(dest_file)
             else:
-                dest_hash = None
+                dest_checksum = None
 
-            if src_hash != dest_hash:
+            result['files'].append(dict(
+                checksum=src_checksum,
+                dest=dest_file,
+                src=f,
+            ))
+
+            if src_checksum != dest_checksum:
                 if not module.check_mode:
                     shutil.copy(tmp_src, dest_file)
 
-                changed = True
+                result['changed'] = True
     finally:
-        module.run_command('umount "%s"' % tmp_dir)
-        os.rmdir(tmp_dir)
+        if not binary:
+            module.run_command('umount "%s"' % tmp_dir)
+
+        shutil.rmtree(tmp_dir)
+
+    module.exit_json(**result)
 
-    module.exit_json(changed=changed)
 
 if __name__ == '__main__':
     main()
diff --git a/test/integration/targets/iso_extract/aliases b/test/integration/targets/iso_extract/aliases
index 8576c423f6a..25e3fdece48 100644
--- a/test/integration/targets/iso_extract/aliases
+++ b/test/integration/targets/iso_extract/aliases
@@ -1,5 +1,2 @@
 posix/ci/group1
-needs/privileged
-needs/root
-skip/freebsd
 skip/osx
diff --git a/test/integration/targets/iso_extract/files/test.iso b/test/integration/targets/iso_extract/files/test.iso
index 909846791026605a69dd4dba14391ab47fbd005a..d06ff73ca5420483e4caa7636fafc16c9c588886 100644
GIT binary patch
delta 1942
zcmb_c%}*0S6n{foyEX(&#Kr)H3DKwtsk_CfjfrucPU*z%&SqyyN;Ki5H%v@`aB>SA
z)RRH}0goPSJaF?rB69GcCw;TKCD1nEBg}T+@6FryIrCmSUo)M<;`g9q?L)lwcM09w
z>`d4b$==YSA#*To01zw#7SstL0ND`$1oGqE&NkV5y8XNK*nVJFE6###+g8PP9@)nQ
zdvd2ZdR%~ZpR+GVSk1N_^wR_ZKD3|>bm+sN1|dlBhJK|cGqZZt+`i9uNy!ux-Auk1
zI%&uplnemE(1I|T;VE298ePon(yXFOt>*k`U9kQ|;MflW6yCYOTX(@oXeyylg@^dq
zumTOJqZUOfu!}hI%lK;KOs}vA+lHgLMn@qE?gE2*hz`1_!UA#}%4j07`FspgLtW$M
zER~)`sR3uvf`Ne<5Dc53-HDu;PSPcR$G?2bWu^`LXFN~h9zPP75^?l#V3yad?A04H
zRyxZBRFEe>e?b`L$X3w4MOIxiDefvNn3Y1ZF;v$l$yn;-BrrZrW)j5|AcBKLaPa1|
zM93v%02?ry#GB2=7(sSDml+!^nz<+V%R6P=)=wN0?33>m2sq`n&9%7jEn~8^ifU%1
zkZcT13QxSHPJ6yLfF^o037bZpvR<JW9g%#cKWydvVW+G(-Vt=koh#Ace_Q_rpQ=IT
z`%bO@&<Z#@WmtnveCJP=ioKYe@Ko=F3sC7PcggE~#KQwp&>!%6MX=&jtB$K1oQ9IU
zprNQOaw>eyL)LJ8<bzOZ$z;<oOQOaVRjnY9p{8{i(m*QFrghQeR4psbJ*v3p`dTm*
zG`XudWzyF!)3hcOqe1I=Q@m9T9>&%x;7rs7bG48@Rk9T_UZ&^*!y3(45o4mix<DJ@
yV-~`5!)3Z8n=Ox1%!gqW8dEXp^nwW=cV6ta+$GoZ8Jxu~`4@Gs9iH;OfA$~8@BqgE

delta 328
zcmZqpAl9&ec|$-0hZF-CNKIr^m|W2yVjv9RgGf;jp~N5#q8NY_2Z-WeC}m~l;F12o
z#K6D^M4Jnnn;0kCHGPpaH83`?FtIQIf)7RphUNyy0+aol#dwefHWzkJ6k!FK&A5^A
zC;y}ktjyvd<`4dQLRK;obQB|_27?61wG0drED)Ln>};_QKpx21?1p+F5h2!w8Vr&U
kc}ciBQHcLw3XLENH!|k1v!vzcb8Te&!_JbFSOjJO04yUk5dZ)H

diff --git a/test/integration/targets/iso_extract/tasks/7zip.yml b/test/integration/targets/iso_extract/tasks/7zip.yml
new file mode 100644
index 00000000000..e176111d8bf
--- /dev/null
+++ b/test/integration/targets/iso_extract/tasks/7zip.yml
@@ -0,0 +1,74 @@
+# Test code for the iso_extract module.
+# (c) 2017, James Tanner <tanner.jc@gmail.com>
+# (c) 2017, Dag Wieers <dag@wieers.com>
+
+# This file is part of Ansible
+#
+# Ansible is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# Ansible is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
+
+- name: Gather facts
+  setup:
+  become: yes
+
+- name: Add EPEL repository
+  yum_repository:
+    name: epel
+    description: EPEL yum repo
+    baseurl: https://download.fedoraproject.org/pub/epel/{{ ansible_distribution_major_version }}/{{ ansible_architecture }}/
+    state: present
+  when: ansible_distribution in ['CentOS']
+
+- name: Install 7zip package if we are on Fedora or CentOS
+  yum:
+    name: p7zip-plugins
+    state: installed
+    update_cache: yes
+  become: yes
+  when: ansible_distribution in ['Fedora', 'CentOS']
+
+- name: Install 7zip package if we are on OpenSUSE
+  zypper:
+    name: p7zip
+    state: installed
+    update_cache: yes
+  become: yes
+  when: ansible_distribution in ['openSUSE Leap']
+
+- name: Install 7zip package if we are on Ubuntu
+  apt:
+    name: p7zip-full
+    state: installed
+    update_cache: yes
+  become: yes
+  when: ansible_distribution in ['Ubuntu']
+
+# FIXME: The homebrew module no longer seems to work
+#        "Error: Running Homebrew as root is extremely dangerous."
+- name: Install 7zip package if we are on MacOSX
+#  macports:
+#    name: p7zip
+#    state: installed
+#    update_cache: yes
+  homebrew:
+    name: p7zip
+    state: present
+    update_homebrew: yes
+  when: ansible_distribution in ['MacOSX']
+
+- name: Install 7zip package if we are on FreeBSD
+  pkgng:
+    name: p7zip
+    state: present
+  become: yes
+  when: ansible_distribution in ['FreeBSD']
diff --git a/test/integration/targets/iso_extract/tasks/main.yml b/test/integration/targets/iso_extract/tasks/main.yml
index a1de638aefd..2f812b8289d 100644
--- a/test/integration/targets/iso_extract/tasks/main.yml
+++ b/test/integration/targets/iso_extract/tasks/main.yml
@@ -1,5 +1,6 @@
 # Test code for the iso_extract module.
 # (c) 2017, James Tanner <tanner.jc@gmail.com>
+# (c) 2017, Dag Wieers <dag@wieers.com>
 
 # This file is part of Ansible
 #
@@ -16,30 +17,27 @@
 # You should have received a copy of the GNU General Public License
 # along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
 
-- set_fact: output_dir_test={{output_dir}}/test_command_raw
+- set_fact:
+    output_dir_test: '{{ output_dir }}/test_iso_extract'
 
-- name: make sure our testing sub-directory does not exist
-  file: path="{{ output_dir_test }}" state=absent
+- name: Install 7zip
+  include_tasks: 7zip.yml
 
-- name: create our testing sub-directory
-  file: path="{{ output_dir_test }}" state=directory
+- name: Prepare environment
+  include_tasks: prepare.yml
 
-##
-## iso_extract
-##
+- name: Test in normal mode
+  include_tasks: tests.yml
+  vars:
+    in_check_mode: no
 
-- name: copy the iso to the test dir
-  copy:
-      src: test.iso
-      dest: "{{ output_dir_test }}"
+- name: Prepare environment
+  include_tasks: prepare.yml
 
-- name: extract the iso
-  iso_extract:
-      image: "{{ output_dir_test }}/test.iso"
-      dest: "{{ output_dir_test }}"
-      files:
-          - 1.txt
-          - 2.txt
-  register: iso_extract_test0
+- name: Test in check-mode
+  include_tasks: tests.yml
+  vars:
+    in_check_mode: yes
+  check_mode: yes
 
 # FIXME - fill this in after figuring out how to allow mounts
diff --git a/test/integration/targets/iso_extract/tasks/prepare.yml b/test/integration/targets/iso_extract/tasks/prepare.yml
new file mode 100644
index 00000000000..78c06ad52c7
--- /dev/null
+++ b/test/integration/targets/iso_extract/tasks/prepare.yml
@@ -0,0 +1,33 @@
+# Test code for the iso_extract module.
+# (c) 2017, James Tanner <tanner.jc@gmail.com>
+# (c) 2017, Dag Wieers <dag@wieers.com>
+
+# This file is part of Ansible
+#
+# Ansible is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# Ansible is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
+
+- name: Make sure our testing sub-directory does not exist
+  file:
+    path: '{{ output_dir_test }}'
+    state: absent
+
+- name: Create our testing sub-directory
+  file:
+    path: '{{ output_dir_test }}'
+    state: directory
+
+- name: copy the iso to the test dir
+  copy:
+      src: test.iso
+      dest: '{{ output_dir_test }}'
diff --git a/test/integration/targets/iso_extract/tasks/tests.yml b/test/integration/targets/iso_extract/tasks/tests.yml
new file mode 100644
index 00000000000..64c9e976182
--- /dev/null
+++ b/test/integration/targets/iso_extract/tasks/tests.yml
@@ -0,0 +1,52 @@
+# Test code for the iso_extract module.
+# (c) 2017, James Tanner <tanner.jc@gmail.com>
+# (c) 2017, Dag Wieers <dag@wieers.com>
+
+# This file is part of Ansible
+#
+# Ansible is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# Ansible is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
+
+- name: Extract the iso
+  iso_extract:
+    image: '{{ output_dir_test }}/test.iso'
+    dest: '{{ output_dir_test }}'
+    files:
+    - 1.txt
+    - 2.txt
+  register: iso_extract_test0
+
+- assert:
+    that:
+    - iso_extract_test0|changed == true
+
+- name: Extract the iso again
+  iso_extract:
+    image: '{{ output_dir_test }}/test.iso'
+    dest: '{{ output_dir_test }}'
+    files:
+    - 1.txt
+    - 2.txt
+  register: iso_extract_test0_again
+
+- name: Test iso_extract_test0_again (normal mode)
+  assert:
+    that:
+    - iso_extract_test0_again|changed == false
+  when: not in_check_mode
+
+- name: Test iso_extract_test0_again (check-mode)
+  assert:
+    that:
+    - iso_extract_test0_again|changed == true
+  when: in_check_mode
diff --git a/test/sanity/pep8/legacy-files.txt b/test/sanity/pep8/legacy-files.txt
index f18cee11c3e..2a0617d1a84 100644
--- a/test/sanity/pep8/legacy-files.txt
+++ b/test/sanity/pep8/legacy-files.txt
@@ -217,7 +217,6 @@ lib/ansible/modules/files/archive.py
 lib/ansible/modules/files/assemble.py
 lib/ansible/modules/files/blockinfile.py
 lib/ansible/modules/files/ini_file.py
-lib/ansible/modules/files/iso_extract.py
 lib/ansible/modules/files/replace.py
 lib/ansible/modules/files/synchronize.py
 lib/ansible/modules/files/tempfile.py