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

state.file.directory recursive chown breaks due to missing follow_symlinks handling #12209

Closed
ysolt opened this issue Apr 23, 2014 · 3 comments · Fixed by #12244
Closed

state.file.directory recursive chown breaks due to missing follow_symlinks handling #12209

ysolt opened this issue Apr 23, 2014 · 3 comments · Fixed by #12244
Labels
Bug broken, incorrect, or confusing behavior severity-low 4th level, cosemtic problems, work around exists
Milestone

Comments

@ysolt
Copy link

ysolt commented Apr 23, 2014

Hey,

I have been struggling with recursive file.directory chown issue. I noticed salt is failing with nonsense (Failed to change user to x) error message when reaching a symlink in the target directory. After changing the owner of the symlinks manually with chown -h command the script pass. Please add follow_symlinks option to state file.directory.

Thanks,
Zsolt

Reproducing the problem

Preparation:

mkdir -p /tmp/dir/1
ln -s /tmp/dir/1/ /tmp/dir/2
chown -R -h root.root /tmp/dir/

/srv/salt/test.sls content

directory_structure:
  file.directory:
    - name: /tmp/dir
    - user: nobody
    - follow_symlinks: False
    - recurse:
      - user

Test#2

root@ip-10-10-1-8:/# salt-call state.sls test      
[INFO    ] Loading fresh modules for state activity
[INFO    ] Fetching file from saltenv 'base', ** skipped ** latest already in cache 'salt://test.sls'
[INFO    ] Running state [/tmp/dir] at time 11:27:08.384227
[INFO    ] Executing state file.directory for /tmp/dir
[ERROR   ] {'user': 'nobody'}
[INFO    ] Completed state [/tmp/dir] at time 11:27:08.388084
local:
----------
          ID: directory_structure
    Function: file.directory
        Name: /tmp/dir
      Result: False
     Comment: Failed to change user to nobody
     Changes:   
              ----------
              user:
                  nobody

Summary
------------
Succeeded: 0
Failed:    1
------------
Total:     1

root@ip-10-10-1-8:/# id nobody
uid=65534(nobody) gid=65534(nogroup) groups=65534(nogroup)
root@ip-10-10-1-8:/#

Test#2 - After manual chown -h nobody /tmp/dir/2 the script pass:

root@ip-10-10-1-8:/tmp/dir# salt-call state.sls test
[INFO    ] Loading fresh modules for state activity
[INFO    ] Fetching file from saltenv 'base', ** skipped ** latest already in cache 'salt://test.sls'
[INFO    ] Running state [/tmp/dir] at time 11:29:17.678003
[INFO    ] Executing state file.directory for /tmp/dir
[INFO    ] Directory /tmp/dir is in the correct state
[INFO    ] Completed state [/tmp/dir] at time 11:29:17.764966
local:
----------
          ID: directory_structure
    Function: file.directory
        Name: /tmp/dir
      Result: True
     Comment: Directory /tmp/dir is in the correct state
     Changes:   

Summary
------------
Succeeded: 1
Failed:    0
------------
Total:     1
# salt-call --versions-report
           Salt: 2014.1.3
         Python: 2.7.3 (default, Sep 26 2013, 20:03:06)
         Jinja2: 2.6
       M2Crypto: 0.21.1
 msgpack-python: 0.1.10
   msgpack-pure: Not Installed
       pycrypto: 2.4.1
         PyYAML: 3.10
          PyZMQ: 13.0.0
            ZMQ: 3.2.2
@basepi
Copy link
Contributor

basepi commented Apr 23, 2014

Thanks for the complete report. We'll look into getting follow_symlinks added to file.directory.

@basepi basepi added this to the Approved milestone Apr 23, 2014
@terminalmage
Copy link
Contributor

It's a little more complicated than this, as the follow_symlinks support to set perms via file.check_perms is incomplete.

I'm taking care of this.

@terminalmage
Copy link
Contributor

Integration tests added for this case. See #12281 for details.

terminalmage added a commit to terminalmage/salt that referenced this issue Apr 25, 2014
terminalmage added a commit to terminalmage/salt that referenced this issue Apr 25, 2014
Not all platforms have a "nobody" user, this commit changes these tests
to use a "test12209" user/group for these tests.
thatch45 added a commit that referenced this issue Apr 26, 2014
s0undt3ch pushed a commit to s0undt3ch/salt that referenced this issue Apr 26, 2014
s0undt3ch pushed a commit to s0undt3ch/salt that referenced this issue Apr 26, 2014
Not all platforms have a "nobody" user, this commit changes these tests
to use a "test12209" user/group for these tests.
techhat added a commit that referenced this issue Apr 26, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior severity-low 4th level, cosemtic problems, work around exists
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants