From 50c6425673ebe70a44e9c48165828a058cabcd72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9mie=20Astori?= Date: Wed, 19 Aug 2015 00:24:08 +0000 Subject: [PATCH 1/7] Fix minor whitespace issues --- files/acl.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/files/acl.py b/files/acl.py index 8b93da1661f..14851225348 100644 --- a/files/acl.py +++ b/files/acl.py @@ -21,7 +21,7 @@ module: acl version_added: "1.4" short_description: Sets and retrieves file ACL information. description: - - Sets and retrieves file ACL information. + - Sets and retrieves file ACL information. notes: - As of Ansible 2.0, this module only supports Linux distributions. options: @@ -122,6 +122,7 @@ acl: sample: [ "user::rwx", "group::rwx", "other::rwx" ] ''' + def split_entry(entry): ''' splits entry and ensures normalized return''' @@ -161,7 +162,7 @@ def build_entry(etype, entity, permissions=None): def build_command(module, mode, path, follow, default, recursive, entry=''): - '''Builds and returns agetfacl/setfacl command.''' + '''Builds and returns a getfacl/setfacl command.''' if mode == 'set': cmd = [module.get_bin_path('setfacl', True)] cmd.append('-m "%s"' % entry) From 421d3f12cf37f601943da12ea44ec941bb4ef9f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9mie=20Astori?= Date: Wed, 19 Aug 2015 00:25:18 +0000 Subject: [PATCH 2/7] Fix wrong processing of lines returned by setfacl in test mode --- files/acl.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/files/acl.py b/files/acl.py index 14851225348..64cc2281d3d 100644 --- a/files/acl.py +++ b/files/acl.py @@ -199,8 +199,8 @@ def acl_changed(module, cmd): for line in lines: if not line.endswith('*,*'): - return False - return True + return True + return False def run_acl(module, cmd, check_rc=True): From 3ac990556d5ccf9c9d5e8e3c5e1ff41cbbb726f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9mie=20Astori?= Date: Wed, 19 Aug 2015 00:26:04 +0000 Subject: [PATCH 3/7] Fix wrong expectation regarding entry format in acl module --- files/acl.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/files/acl.py b/files/acl.py index 64cc2281d3d..7d1b96b9e97 100644 --- a/files/acl.py +++ b/files/acl.py @@ -276,10 +276,10 @@ def main(): if etype or entity or permissions: module.fail_json(msg="'entry' MUST NOT be set when 'entity', 'etype' or 'permissions' are set.") - if state == 'present' and entry.count(":") != 3: + if state == 'present' and entry.count(":") != 2: module.fail_json(msg="'entry' MUST have 3 sections divided by ':' when 'state=present'.") - if state == 'absent' and entry.count(":") != 2: + if state == 'absent' and entry.count(":") != 1: module.fail_json(msg="'entry' MUST have 2 sections divided by ':' when 'state=absent'.") default, etype, entity, permissions = split_entry(entry) From e95bcaeb8a98ec280acccadbb5491efc0c1679af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9mie=20Astori?= Date: Thu, 20 Aug 2015 22:22:28 +0000 Subject: [PATCH 4/7] Remove support for `d[efault]:` in entry permissions It is not documented in [the Ansible doc page][1] nor [the BSD setfacl man entry][2] (which means it might not be compatible with BSD) so removing it does not break the API. On the other hand, it does not conform with POSIX 1003.1e DRAFT STANDARD 17 according to the [Linux setfacl man entry][3] so safer to remove. Finally, the most important reason: in non POSIX 1003.e mode, only ACL entries without the permissions field are accepted, so having an optional field here is very much error-prone. [1]: http://docs.ansible.com/ansible/acl_module.html [2]: http://www.freebsd.org/cgi/man.cgi?format=html&query=setfacl(1) [3]: http://linuxcommand.org/man_pages/setfacl1.html --- files/acl.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/files/acl.py b/files/acl.py index 7d1b96b9e97..2bd27f621f3 100644 --- a/files/acl.py +++ b/files/acl.py @@ -128,17 +128,12 @@ def split_entry(entry): a = entry.split(':') a.reverse() - if len(a) == 3: - a.append(False) try: - p, e, t, d = a + p, e, t = a except ValueError, e: print "wtf?? %s => %s" % (entry, a) raise e - if d: - d = True - if t.startswith("u"): t = "user" elif t.startswith("g"): @@ -150,7 +145,7 @@ def split_entry(entry): else: t = None - return [d, t, e, p] + return [t, e, p] def build_entry(etype, entity, permissions=None): @@ -282,7 +277,7 @@ def main(): if state == 'absent' and entry.count(":") != 1: module.fail_json(msg="'entry' MUST have 2 sections divided by ':' when 'state=absent'.") - default, etype, entity, permissions = split_entry(entry) + etype, entity, permissions = split_entry(entry) changed = False msg = "" From 0e659ad8723789310446221947256e33780e77d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9mie=20Astori?= Date: Thu, 20 Aug 2015 22:40:15 +0000 Subject: [PATCH 5/7] Make sure permission-less entries are accepted when state=absent Also, remove that try condition as, at that stage, no permissions with other than 2 or 3 fields are sent to the function. --- files/acl.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/files/acl.py b/files/acl.py index 2bd27f621f3..58550c19124 100644 --- a/files/acl.py +++ b/files/acl.py @@ -127,12 +127,10 @@ def split_entry(entry): ''' splits entry and ensures normalized return''' a = entry.split(':') - a.reverse() - try: - p, e, t = a - except ValueError, e: - print "wtf?? %s => %s" % (entry, a) - raise e + if len(a) == 2: + a.append(None) + + t, e, p = a if t.startswith("u"): t = "user" From 8eefd44aefb2b17679fe1d292eb48af52cce094a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9mie=20Astori?= Date: Thu, 20 Aug 2015 22:43:15 +0000 Subject: [PATCH 6/7] Make sure entry is not sent when acl state=query --- files/acl.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/files/acl.py b/files/acl.py index 58550c19124..06fd304e361 100644 --- a/files/acl.py +++ b/files/acl.py @@ -275,6 +275,9 @@ def main(): if state == 'absent' and entry.count(":") != 1: module.fail_json(msg="'entry' MUST have 2 sections divided by ':' when 'state=absent'.") + if state == 'query': + module.fail_json(msg="'entry' MUST NOT be set when 'state=query'.") + etype, entity, permissions = split_entry(entry) changed = False From 72fb7a0a172e648bbeb4597e109df0238ac7cec9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9mie=20Astori?= Date: Thu, 20 Aug 2015 23:35:53 +0000 Subject: [PATCH 7/7] Fix physical walk on acl module for Linux `-h` is for BSD [1] while `-P`/`--physical` is for Linux [2]. This commit fixes that option now that acl module is (temporarily) only supported for Linux. I will re-add `-h` when fixing BSD support. [1]: http://www.freebsd.org/cgi/man.cgi?format=html&query=setfacl(1) [2]: http://linuxcommand.org/man_pages/setfacl1.html --- files/acl.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/files/acl.py b/files/acl.py index 06fd304e361..ad0f4607609 100644 --- a/files/acl.py +++ b/files/acl.py @@ -172,7 +172,7 @@ def build_command(module, mode, path, follow, default, recursive, entry=''): cmd.append('--recursive') if not follow: - cmd.append('-h') + cmd.append('--physical') if default: if(mode == 'rm'):