Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix wrong 'recurse' behavior on for linux_acl.present/absent states #49532

Merged

Conversation

Projects
None yet
3 participants
@meaksh
Copy link
Member

commented Sep 6, 2018

What does this PR do?

This PR fixes an issue on the behavior of the linux_acl.present and linux_acl.absent states when using recurse=True.

Currently, when the recurse parameter is set to True for a linux_acl.present state, it will only be recursively fixing the ACL in case main directory (the name parameter) is in a wrong state, otherwise nothing is done even when subdirectories are actually in a wrong state.

Given this SLS file:

change_acls_for_user:
  acl.present:
    - name: /srv/acl_test
    - acl_type: user
    - acl_name: salt
    - perms: rw
    - recurse: True

And we have this ACL setup on /srv/acl_test:

# find /srv/acl_test/
/srv/acl_test/
/srv/acl_test/test1
/srv/acl_test/test2

# getfacl -R /srv/acl_test
# file: srv/acl_test/
# owner: tomcat
# group: tomcat
user::rwx
user:salt:rw-
group::r-x
mask::rwx
other::r-x

# file: srv/acl_test/test2
# owner: root
# group: root
user::rwx
group::r-x
other::r-x

# file: srv/acl_test/test1
# owner: root
# group: root
user::rwx
group::r-x
other::r-x

As you can see the defined ACL on the SLS is prorperly set for /srv/acl_test but not either /srv/acl_test/test1 nor /srv/acl_test/test2.

So, if I apply the previous state on the SLS now, I would expect those the subdirectories will be fixed with the right ACL but they won't. Salt will report that /srv/acl_test is on the right state and nothing happens:

local:
----------
          ID: change_acls_for_user
    Function: acl.present
        Name: /srv/acl_test
      Result: True
     Comment: Permissions are in the desired state
     Started: 12:50:29.117809
    Duration: 10.405 ms
     Changes:   
----------

But in case the main /srv/acl_test is on a wrong state, then linux_acl.present will properly set the defined ACL also for ALL subdirectories (which IMHO is the right behavior when using recurse=True:

          ID: change_acls_for_user
    Function: acl.present
        Name: /srv/acl_test
      Result: True
     Comment: Updated permissions for salt
     Started: 12:50:27.251659
    Duration: 11.322 ms
     Changes:   
              ----------
              new:
                  ----------
                  acl_name:
                      salt
                  acl_type:
                      user
                  perms:
                      rw
              old:
                  ----------
                  acl_name:
                      salt
                  acl_type:
                      user
                  perms:
                      6

Previous Behavior

Even with recurse=True, the linux_acl.present and linux_acl.absent states only work recursively when the main path (set by name parameter) is in a wrong ACL state. Otherwise it will return "no changes" even when subdirectories needs to be fixed with the right ACL.

New Behavior

Now with this PR, both linux_acl.present and linux_acl.absent states will check into all subdirectories to figure out if changes needs to be make or not.

Tests written?

Yes

Commits signed with GPG?

Yes

meaksh added some commits Sep 6, 2018

@rallytime
Copy link
Contributor

left a comment

LGTM

@rallytime rallytime requested a review from isbm Sep 11, 2018

@isbm

isbm approved these changes Sep 12, 2018

@isbm

This comment has been minimized.

Copy link
Contributor

commented Sep 12, 2018

@rallytime there is no lint errors... 😉

@rallytime rallytime merged commit c1f97c4 into saltstack:2018.3 Sep 12, 2018

5 of 10 checks passed

continuous-integration/jenkins/pr-merge This commit cannot be built
Details
jenkins/pr/py2-centos-7 The py2-centos-7 job has failed
Details
jenkins/pr/py2-windows-2016 The py2-windows-2016 job has failed
Details
jenkins/pr/py3-ubuntu-1604 The py3-ubuntu-1604 job has failed
Details
jenkins/pr/py3-windows-2016 The py3-windows-2016 job has failed
Details
WIP ready for review
Details
jenkins/pr/docs The docs job has passed
Details
jenkins/pr/lint The lint job has passed
Details
jenkins/pr/py2-ubuntu-1604 The py2-ubuntu-1604 job has passed
Details
jenkins/pr/py3-centos-7 The py3-centos-7 job has passed
Details

@meaksh meaksh deleted the meaksh:2018.3-fix-linux_acl-recursive-problems branch Sep 17, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.