Fix idempotency for Unix permissions in zip files. (#24580)
* Fix idempotency for Unix permissions in zip files. This fix prevents the unarchive module from reporting 'changed' when a zipfile contains items with Unix permissions that differ from the system default. * Update zip unarchive tests. Additional tests for the unarchive module with zip files: - Test file in zip archive with non-default permissions - Test file added to zip archive with Windows permissions * Additional fix for mixed win/unix archives. Turns out my original fix fails under some mixed archives, as setting the umask to zero can be applied to those files. This creates a per-file umask variable, so a mix of permission types don't cause problems. * CI Checks CI checks for archives with: * non default Unix permissions * Windows permissions * Workaround for BSD differences. Using Zipinfo due to lack of support in BSD unzip. Permissions handling is also different in BSD -- always applies UMASK to file permissions. * Added checks for creating directories and SSH keys for existing users.
This commit is contained in:
chriskarel
Toshio Kuratomi
parent
3cb1c38ecc
commit
991918e9d2
2 changed files with 71 additions and 14 deletions
|
|
@ -125,6 +125,7 @@ import codecs
|
|||
import datetime
|
||||
import grp
|
||||
import os
|
||||
import platform
|
||||
import pwd
|
||||
import re
|
||||
import stat
|
||||
|
|
@ -180,6 +181,7 @@ class ZipArchive(object):
|
|||
self.excludes = module.params['exclude']
|
||||
self.includes = []
|
||||
self.cmd_path = self.module.get_bin_path('unzip')
|
||||
self.zipinfocmd_path = self.module.get_bin_path('zipinfo')
|
||||
self._files_in_archive = []
|
||||
self._infodict = dict()
|
||||
|
||||
|
|
@ -261,7 +263,8 @@ class ZipArchive(object):
|
|||
return self._files_in_archive
|
||||
|
||||
def is_unarchived(self):
|
||||
cmd = [self.cmd_path, '-ZT', '-s', self.src]
|
||||
# BSD unzip doesn't support zipinfo listings with timestamp.
|
||||
cmd = [self.zipinfocmd_path, '-T', '-s', self.src]
|
||||
if self.excludes:
|
||||
cmd.extend(['-x', ] + self.excludes)
|
||||
rc, out, err = self.module.run_command(cmd)
|
||||
|
|
@ -277,6 +280,7 @@ class ZipArchive(object):
|
|||
# Get some information related to user/group ownership
|
||||
umask = os.umask(0)
|
||||
os.umask(umask)
|
||||
systemtype = platform.system()
|
||||
|
||||
# Get current user and group information
|
||||
groups = os.getgroups()
|
||||
|
|
@ -376,6 +380,12 @@ class ZipArchive(object):
|
|||
ftype = 'f'
|
||||
|
||||
# Some files may be storing FAT permissions, not Unix permissions
|
||||
# For FAT permissions, we will use a base permissions set of 777 if the item is a directory or has the execute bit set. Otherwise, 666.
|
||||
# This permission will then be modified by the system UMask.
|
||||
# BSD always applies the Umask, even to Unix permissions.
|
||||
# For Unix style permissions on Linux or Mac, we want to use them directly.
|
||||
# So we set the UMask for this file to zero. That permission set will then be unchanged when calling _permstr_to_octal
|
||||
|
||||
if len(permstr) == 6:
|
||||
if path[-1] == '/':
|
||||
permstr = 'rwxrwxrwx'
|
||||
|
|
@ -383,6 +393,11 @@ class ZipArchive(object):
|
|||
permstr = 'rwxrwxrwx'
|
||||
else:
|
||||
permstr = 'rw-rw-rw-'
|
||||
file_umask = umask
|
||||
elif 'bsd' in systemtype.lower():
|
||||
file_umask = umask
|
||||
else:
|
||||
file_umask = 0
|
||||
|
||||
# Test string conformity
|
||||
if len(permstr) != 9 or not ZIP_FILE_MODE_RE.match(permstr):
|
||||
|
|
@ -484,7 +499,7 @@ class ZipArchive(object):
|
|||
elif ztype == '?':
|
||||
mode = self._permstr_to_octal(permstr, 0)
|
||||
else:
|
||||
mode = self._permstr_to_octal(permstr, umask)
|
||||
mode = self._permstr_to_octal(permstr, file_umask)
|
||||
|
||||
if mode != stat.S_IMODE(st.st_mode):
|
||||
change = True
|
||||
|
|
|
|||
|
|
@ -17,18 +17,23 @@
|
|||
# along with Ansible. If not, see <http://www.gnu.org/licenses/>.
|
||||
# Make sure we start fresh
|
||||
|
||||
- name: Ensure zip is present to create test archive (yum)
|
||||
yum: name=zip state=latest
|
||||
# Need unzup for unarchive module, and zip for archive creation.
|
||||
- name: Ensure zip and unzip is present to create test archive (yum)
|
||||
yum: name=zip,unzip state=latest
|
||||
when: ansible_pkg_mgr == 'yum'
|
||||
|
||||
#- name: Ensure zip is present to create test archive (dnf)
|
||||
# dnf: name=zip state=latest
|
||||
# when: ansible_pkg_mgr == 'dnf'
|
||||
|
||||
- name: Ensure zip is present to create test archive (apt)
|
||||
apt: name=zip state=latest
|
||||
- name: Ensure zip & unzup is present to create test archive (apt)
|
||||
apt: name=zip,unzip state=latest
|
||||
when: ansible_pkg_mgr == 'apt'
|
||||
|
||||
- name: Ensure zip & unzup is present to create test archive (pkg)
|
||||
pkgng: name=zip,unzip state=present
|
||||
when: ansible_pkg_mgr == 'pkgng'
|
||||
|
||||
- name: prep our file
|
||||
copy: src=foo.txt dest={{output_dir}}/foo-unarchive.txt
|
||||
|
||||
|
|
@ -38,8 +43,26 @@
|
|||
- name: prep a tar.gz file
|
||||
shell: tar czvf test-unarchive.tar.gz foo-unarchive.txt chdir={{output_dir}}
|
||||
|
||||
- name: prep a chmodded file for zip
|
||||
copy: src=foo.txt dest={{output_dir}}/foo-unarchive-777.txt mode=0777
|
||||
|
||||
- name: prep a windows permission file for our zip
|
||||
copy: src=foo.txt dest={{output_dir}}/FOO-UNAR.TXT
|
||||
|
||||
# This gets around an unzip timestamp bug in some distributions
|
||||
# Recent unzip on Fedora, Ubuntu, and BSD will randomly round some timestamps up.
|
||||
# But that doesn't seem to happen when the timestamp has an even second.
|
||||
- name: Bug work around
|
||||
command: touch -t "201705111530.00" {{output_dir}}/foo-unarchive.txt {{output_dir}}/foo-unarchive-777.txt {{output_dir}}/FOO-UNAR.TXT
|
||||
# See Fedora bug 1451953: https://bugzilla.redhat.com/show_bug.cgi?id=1451953
|
||||
# See Ubuntu bug 1691636: https://bugs.launchpad.net/ubuntu/+source/unzip/+bug/1691636
|
||||
# When these are fixed, this code should be removed.
|
||||
|
||||
- name: prep a zip file
|
||||
shell: zip test-unarchive.zip foo-unarchive.txt chdir={{output_dir}}
|
||||
shell: zip test-unarchive.zip foo-unarchive.txt foo-unarchive-777.txt chdir={{output_dir}}
|
||||
|
||||
- name: add a file with Windows permissions to zip file
|
||||
shell: zip -k test-unarchive.zip FOO-UNAR.TXT chdir={{output_dir}}
|
||||
|
||||
- name: prep a subdirectory
|
||||
file: path={{output_dir}}/unarchive-dir state=directory
|
||||
|
|
@ -141,17 +164,36 @@
|
|||
- "unarchive03.changed == true"
|
||||
# Verify that file list is generated
|
||||
- "'files' in unarchive03"
|
||||
- "{{unarchive03['files']| length}} == 1"
|
||||
- "{{unarchive03['files']| length}} == 3"
|
||||
- "'foo-unarchive.txt' in unarchive03['files']"
|
||||
- "'foo-unarchive-777.txt' in unarchive03['files']"
|
||||
- "'FOO-UNAR.TXT' in unarchive03['files']"
|
||||
|
||||
- name: verify that the file was unarchived
|
||||
file: path={{output_dir}}/test-unarchive-zip/foo-unarchive.txt state=file
|
||||
file: path={{output_dir}}/test-unarchive-zip/{{item}} state=file
|
||||
with_items:
|
||||
- foo-unarchive.txt
|
||||
- foo-unarchive-777.txt
|
||||
- FOO-UNAR.TXT
|
||||
|
||||
- name: repeat the last request to verify no changes
|
||||
unarchive: src={{output_dir}}/test-unarchive.zip dest={{output_dir | expanduser}}/test-unarchive-zip remote_src=yes list_files=True
|
||||
register: unarchive03b
|
||||
|
||||
- name: verify that the task was not marked as changed
|
||||
assert:
|
||||
that:
|
||||
- "unarchive03b.changed == false"
|
||||
|
||||
- name: remove our zip unarchive destination
|
||||
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
|
||||
- name: remove our test files for the archive
|
||||
file: path={{output_dir}}/{{item}} state=absent
|
||||
with_items:
|
||||
- foo-unarchive.txt
|
||||
- foo-unarchive-777.txt
|
||||
- FOO-UNAR.TXT
|
||||
|
||||
- name: check if /tmp/foo-unarchive.text exists
|
||||
stat: path=/tmp/foo-unarchive.txt
|
||||
|
|
|
|||
Loading…
Reference in a new issue