unarchive with zip file: 'owner' and 'group' set files but not directories

[candlerb]

ISSUE TYPE

• Bug Report

COMPONENT NAME

unarchive

ANSIBLE VERSION

ansible 2.2.0.0
  config file = /home/ubuntu/vtp/ansible/nmm/ansible.cfg
  configured module search path = Default w/o overrides

CONFIGURATION

Nothing significant

$ cat /home/ubuntu/vtp/ansible/nmm/ansible.cfg
[defaults]
inventory = ./hosts
roles_path = ../shared/roles

OS / ENVIRONMENT

Running ansible from Ubuntu 16.04, managing Ubuntu 16.04

SUMMARY

unarchive module 'owner' and 'group' set the file ownership but not the directory ownership

STEPS TO REPRODUCE

- name: unzip website content
  unarchive:
    src: nmm.zip
    dest: /var/www/html
    owner: www-data
    group: www-data

EXPECTED RESULTS

That all files and directories in the unpacked filetree would be owned by www-data:www-data

ACTUAL RESULTS

Files are owned by www-data:www-data, but directories are owned by root:root

root@www:~# ls -l /var/www/html
total 0
drwxr-xr-x 1 root root 76 Nov 27 16:51 NMM 5 days
root@www:~# ls -l /var/www/html/NMM\ 5\ days/
total 8
-rw-rw-rw- 1 www-data www-data 6811 Nov 27 15:37 agenda.md.html
drwxr-xr-x 1 root     root       24 Nov 27 16:51 css
drwxr-xr-x 1 root     root      164 Nov 27 16:51 images
drwxr-xr-x 1 root     root        4 Nov 27 16:44 netmgmt
drwxr-xr-x 1 root     root       30 Nov 27 16:51 software

[ansibot]

@dagwieers, @pileofrogs, ping. This issue is waiting on your response. click here for bot help

[dagwieers]

Interesting, can you do the following on your zip-file ?

unzip -Z -T --h-t path/to/your/file.zip

It should list all the files (and directories) in your archive. My only guess is that some directories are not part of the archive. needs_info

[candlerb]

Your guess is correct: the zip contains only files, not directories.

$ unzip -Z -T --h-t nmm.zip
-rw-r--r--  2.0 unx    18701 b- defN 20160621.185646 NMM 5 days/images/nsrc_logo.png
-rw-r--r--  2.0 unx    20486 b- defN 20160621.185620 NMM 5 days/images/icon_facebook.png
-rw-r--r--  2.0 unx    21441 b- defN 20160621.185626 NMM 5 days/images/icon_google_plus.png
-rw-r--r--  2.0 unx    21094 b- defN 20160621.185632 NMM 5 days/images/icon_twitter.png
-rw-r--r--  2.0 unx    21278 b- defN 20160621.185640 NMM 5 days/images/icon_youtube.png
-rw-r--r--  2.0 unx     1536 t- defN 20160621.185612 NMM 5 days/css/template.css
-rw-r--r--  2.0 unx    68614 t- defN 20160621.185654 NMM 5 days/software/markdeep.min.js
-rw-rw-rw-  2.0 unx   959934 b- stor 20161127.153708 NMM 5 days/netmgmt/en/cisco-config-elements/cisco-config-elements.pdf
-rw-rw-rw-  2.0 unx     8868 b- stor 20161127.153708 NMM 5 days/netmgmt/en/cisco-config-elements/exercises-cisco-config.md.html
-rw-rw-rw-  2.0 unx      486 b- stor 20161127.153708 NMM 5 days/netmgmt/en/cisco-config-elements/README.md.html
...
-rw-rw-rw-  2.0 unx   168081 b- stor 20161127.153710 NMM 5 days/netmgmt/en/snmp/snmp.pdf
-rw-rw-rw-  2.0 unx     9660 b- stor 20161127.153710 NMM 5 days/netmgmt/en/snmp/exercises-snmp.md.html
-rw-rw-rw-  2.0 unx     6811 b- stor 20161127.153710 NMM 5 days/agenda.md.html

[dagwieers]

Hmm, that's unfortunate, but it explains why we haven't seen this issue before.

If this is your own archive, I would advice to include directories (and directory-permissions) as part of the archive. The alternative is that we would add directories (and guess permissies) ourselves, but that would mean that unzip would modify pre-existing directories not part of the archive, and maybe some people are using this behaviour in their own use-case.

So I am not confident in changing this behaviour. We should at least document this !

[candlerb]

Fair enough. The ideal behaviour would be if unzip would apply owner:group permissions only if it creates the directory itself; any pre-existing directories (which are not in the zipfile) would be left alone.

However for now I have a simple workaround:

- name: unzip website content
  unarchive:
    src: '{{src_archive}}'
    dest: /var/www/html
    owner: www-data
    group: www-data

- name: fix permissions
  file:
    path: /var/www/html
    owner: www-data
    group: www-data
    recurse: yes

and it's good enough. +1 for documenting the behaviour.

[dagwieers]

Ok, I will add a note to the module so it becomes part of the documentation.

Thanks for reporting ! We learn something new about ZIP every day ;-)

[dagwieers]

This is notabug per se, but induced an improvement nevertheless.

PR #5721 add the following not to the documentation:

• ZIP files consisting of files, but not directories may find that ownership or permissions of parent directories may not be set as expected when created.

[dagwieers]

Hmm, it seems I misinterpreted the solution you proposed.

If the directory did not already exist you prefer we would create it with the requested ownership. But we do not know what permissions. So I am not sure if this is expected behaviour.

[candlerb]

If the directory did not already exist you prefer we would create it with the requested ownership. But we do not know what permissions.

Well, the 'template' and 'copy' modules have 'owner', 'group' and 'mode' options, and if you don't specify those, it falls back to some sort of default. Last time I looked at this, I think the ownership was whatever user ansible was running as; as for the mode, maybe it just came from umask?

I note that the unarchive module does have a mode option. However if you set it, I presume it would have to be the same for both files and directories, which isn't ideal.

[dagwieers]

@candlerb Can you open this issue again ? Maybe update the title to reflect our new findings. I agree we need a solution for this and what you propose makes sense. I don't know if I will have the time for this, but we should keep this issue around until it is dealt with. needs_contributor

[candlerb]

I don't see a way to reopen - do you want me to submit a new issue?

[candlerb]

(Related issue: #234)

Also: what do you think about having separate file_mode and directory_mode options? Unfortunately I don't know of any other module which does this. It could be useful for other modules which touch both files and directories, as requested here and here, but I don't know if there's a github ticket for it.

[dagwieers]

As the creator of this ticket you should be able to reopen though ?

Wrt. mode-support, the main issue is that unarchive does not set the permissions, it leaves this up to a mod_util function set_fs_attributes_if_different(). I would have to look into this, initially it was possible but the problem was that it did not support directory modes and as such it was disabled in a subsequent PR.

Also, I would prefer to use my time to create a pure-python implementation rather than using gtar/unzip and parsing output. As this whole approach is error-prone and full of corner-cases and workarounds... But as the (new) maintainer I don't want to block others improving this module though.

[candlerb]

I can edit the title. According to this and this I can only reopen an issue if I closed it myself - which I didn't.

Pure python sounds like the right approach to avoid all the problems with incompatible tar/zip implementations.

[dagwieers]

botbroken

Feature request for @ansibot : Add a command to reopen an issue.