Salt: Allow +X in ACL's

[timwsuqld]

Related to #31270
Wanting to create an ACL that applies the execute permission to directories, but not files. Using chmod and setfacl you can use X instead of x, which means it'll apply the execute permission to directories, or files if they already have that permission. From the chmod man page, execute/search only if the file is a directory or already has execute permission for some user (X)

An example SLS file would look like. Note the Capital X, not lower case x

developers_acl:
  acl.present:
    - name: /srv/www
    - acl_type: default:group
    - acl_name: developers
    - perms: rwX
    - recurse: True

This unfortunately bombs with the following error (Same as #31270)

          ID: developers_acl
    Function: acl.present
        Name: /srv/www
      Result: False
     Comment: An exception occurred in this state: Traceback (most recent call last):
                File "/usr/lib/python2.7/site-packages/salt/state.py", line 1703, in call
                  **cdata['kwargs'])
                File "/usr/lib/python2.7/site-packages/salt/loader.py", line 1649, in wrapper
                  return f(*args, **kwargs)
                File "/usr/lib/python2.7/site-packages/salt/states/linux_acl.py", line 91, in present
                  if user[_search_name]['octal'] == sum([_octal.get(i, i) for i in perms]):
              TypeError: unsupported operand type(s) for +: 'int' and 'str'
     Started: 11:56:24.019937
    Duration: 8.121 ms
     Changes:   

Looking at the code, I'm not sure the easiest way to handle this. We take the easy way of comparing current permissions to intended permissions by getting the octal value of the current permissions, and calulating the new octal value. To support X we'd need to handle the execute bit on a case by case basis, as we shouldn't be removing the execute bit if it's present, but we should only be adding it to directories if it's absent.

Versions Report

$ salt --versions-report
Salt Version:
           Salt: 2015.8.10

Dependency Versions:
         Jinja2: 2.7.3
       M2Crypto: Not Installed
           Mako: Not Installed
         PyYAML: 3.11
          PyZMQ: 14.7.0
         Python: 2.7.5 (default, Nov 20 2015, 02:00:19)
           RAET: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.0.5
           cffi: 0.8.6
       cherrypy: 3.2.2
       dateutil: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
        libgit2: 0.21.0
        libnacl: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.7
   mysql-python: Not Installed
      pycparser: 2.14
       pycrypto: 2.6.1
         pygit2: 0.21.4
   python-gnupg: Not Installed
          smmap: Not Installed
        timelib: Not Installed

System Versions:
           dist: centos 7.2.1511 Core
        machine: x86_64
        release: 3.10.0-327.18.2.el7.x86_64
         system: CentOS Linux 7.2.1511 Core

[Ch3LL]

@timwsuqld I am able to replicate this error with a smaller test case as follows:

/tmp/acl1/:
  acl.present:
    - acl_type: user
    - acl_name: root
    - perms: rwX

Looks like we need to add the ability to use this X argument. Does X have a value attributed to it that would possible be an approach if it does. Then it could possibly be added to _octal = {'r': 4, 'w': 2, 'x': 1, '-': 0}

[timwsuqld]

@Ch3LL Unfortunately X doesn't have a numeric value assigned to it. It's special because it's a 1, but only if it's directory or file that already has an execute bit. If it did have an octal value, I would have easily added it to the dict and submitted a patch.

Unfortunately I think it'll need some more logic added to the code to handle it

[rsuarezsoto]

Hello,

has there been any advance on this?

Thanks!

[Ch3LL]

No one is currently working on this due to other higher priority issues. Please feel free to take a stab at a PR if you would like.

[doubletwist13]

I would very much like a fix for this as well, but I'm far from knowledgeable enough to do it myself.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

[rhoths]

Still an issue.

[stale[bot]]

Thank you for updating this issue. It is no longer marked as stale.

[knine]

I ran into this one again today. Just keeping it on the radar.