Salt: [Fluorine] State file.directory recurse: When both file_mode and dir_mode are supplied, file_mode overrides dir_mode for the directory itself

[ssiuhk]

Description of Issue/Question

Hi, I have an issue with file.directory state, when both file_mode and dir_mode are supplied, file_mode overrides dir_mode for the directory itself

According to Saltstack Document, file_mode for permission of files, dir_mode for permission of directories.

Setup

[root@localhost salttest]# pwd
/tmp/salttest
[root@localhost salttest]# tree -up .
.
├── [drwxr-xr-x root    ]  dir_one
│   └── [drwxr-xr-x root    ]  dir_two
│       └── [-rw-r--r-- root    ]  testfile.txt
├── [-rw-r--r-- root    ]  file_perm.sls
└── [-rw-r--r-- root    ]  top.sls

2 directories, 3 files

[root@localhost salttest]# cat file_perm.sls
/tmp/salttest/dir_one:
  file.directory:
    - user: root
    - group: root
    - dir_mode: 755
    - file_mode: 644
    - recurse:
      - user
      - group
      - mode

/tmp/salttest/dir_one/dir_two:
  file.directory:
    - user: root
    - group: root
    - dir_mode: 755
    - makedirs: True

Steps to Reproduce Issue

[root@localhost salttest]# salt-call --local --file-root=/tmp/salttest/ state.apply test=True file_perm
local:
----------
          ID: /tmp/salttest/dir_one
    Function: file.directory
      Result: None
     Comment: The following files will be changed:
              /tmp/salttest/dir_one: mode - 0644
     Started: 14:41:07.828673
    Duration: 6.19 ms
     Changes:
----------
          ID: /tmp/salttest/dir_one/dir_two
    Function: file.directory
      Result: True
     Comment: The directory /tmp/salttest/dir_one/dir_two is in the correct state
     Started: 14:41:07.835033
    Duration: 0.738 ms
     Changes:

Summary for local
------------
Succeeded: 2 (unchanged=1)
Failed:    0
------------
Total states run:     2
Total run time:   6.928 ms

Versions Report

[root@localhost salttest]# salt-call --versions-report
Salt Version:
           Salt: 2019.2.0

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: Not Installed
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.7.2
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.6
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.5 (default, Apr 11 2018, 07:36:10)
   python-gnupg: Not Installed
         PyYAML: 3.10
          PyZMQ: 15.3.0
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.1.4

System Versions:
           dist: centos 7.5.1804 Core
         locale: UTF-8
        machine: x86_64
        release: 3.10.0-862.3.2.el7.x86_64
         system: Linux
        version: CentOS Linux 7.5.1804 Core

[dwoz]

@ssiuhk Thanks for reporting the issue and the PR!

[9numbernine9]

I'm getting this exact same issue as described here, but with Salt Master and Minion at 2018.3.4 instead of 2019.2 - I've tried the same setup as in @ssiuhk 's example. I don't think this problem occurred in 2018.3.3 !

[ssiuhk]

@9numbernine9 Yeah, I checked the history of file.py, the change was introduced in 2018.3.4 but I skipped that version

[danfoster]

PR #51909 did not solve this issue for me when backporting the patch to 2018.3.4. I can also confirm this did not occur in 2018.3.3.

Here is a simple reproducible case:

$ umask 007
$ for i in `seq 1 3`; do mkdir -p /tmp/test/d$i; touch /tmp/test/f$i; done        
ls -l /tmp/test
total 0
drwx------ 2 root root 6 Mar  7 13:16 d1
drwx------ 2 root root 6 Mar  7 13:16 d2
drwx------ 2 root root 6 Mar  7 13:16 d3
-rw------- 1 root root 0 Mar  7 13:16 f1
-rw------- 1 root root 0 Mar  7 13:16 f2
-rw------- 1 root root 0 Mar  7 13:16 f3
$ salt-call state.single file.directory name="/tmp/test" dir_mode="700" file_mode="600" recurse="[mode]" test=false
local:
----------
          ID: /tmp/test
    Function: file.directory
      Result: True
     Comment: Directory /tmp/test is in the correct state
     Started: 13:54:50.775917
    Duration: 8.027 ms
     Changes:
              ----------
              /tmp/test/d1:
                  ----------
                  mode:
                      0600
              /tmp/test/d2:
                  ----------
                  mode:
                      0600
              /tmp/test/d3:
                  ----------
                  mode:
                      0600

Summary for local
------------
Succeeded: 1 (changed=1)
Failed:    0
------------
Total states run:     1
Total run time:   8.027 ms

[ssiuhk]

Yes, mode was override above in file section for line https://github.com/saltstack/salt/blob/develop/salt/states/file.py#L746 ....
Maybe a cleaner way is to use two variables to store file_mode and dir_mode without playing these override around
Please take a note to https://github.com/saltstack/salt/blob/develop/salt/states/file.py#L722 as well if using new variables

[hktb]

I'm afraid all proposed solutions do not fix the issue. Please read my gist The saltstack file.directory state module fails to handle the mode for directories correctly

[danlsgiga]

Also facing the same issue in 2018.3.4

[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.

[9numbernine9]

Please don't close this stale-bot. :-)

[stale[bot]]

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