Check dir_mode recursively in file.directory

salt/states/file.py

@@ -700,7 +700,7 @@ def _check_directory(name,
                      user=None,
                      group=None,
                      recurse=False,
-                     mode=None,
+                     dir_mode=None,
                      file_mode=None,
                      clean=False,
                      require=False,
@@ -730,14 +730,15 @@ def _check_directory(name,
         if 'group' not in recurse_set:
             group = None
         if 'mode' not in recurse_set:
-            mode = None
+            dir_mode = None
+            file_mode = None
+
         check_files = 'ignore_files' not in recurse_set
         check_dirs = 'ignore_dirs' not in recurse_set
         for root, dirs, files in walk_l:
             if check_files:
                 for fname in files:
                     fchange = {}
-                    mode = file_mode
                     path = os.path.join(root, fname)
                     stats = __salt__['file.stats'](
                         path, None, follow_symlinks
@@ -746,18 +747,18 @@ def _check_directory(name,
                         fchange['user'] = user
                     if group is not None and group != stats.get('group'):
                         fchange['group'] = group
-                    if mode is not None and mode != stats.get('mode'):
-                        fchange['mode'] = mode
+                    if file_mode is not None and salt.utils.files.normalize_mode(file_mode) != salt.utils.files.normalize_mode(stats.get('mode')):
+                        fchange['mode'] = file_mode
                     if fchange:
                         changes[path] = fchange
             if check_dirs:
                 for name_ in dirs:
                     path = os.path.join(root, name_)
-                    fchange = _check_dir_meta(path, user, group, mode, follow_symlinks)
+                    fchange = _check_dir_meta(path, user, group, dir_mode, follow_symlinks)
                     if fchange:
                         changes[path] = fchange
     # Recurse skips root (we always do dirs, not root), so always check root:
-    fchange = _check_dir_meta(name, user, group, mode, follow_symlinks)
+    fchange = _check_dir_meta(name, user, group, dir_mode, follow_symlinks)
     if fchange:
         changes[name] = fchange
     if clean:

tests/unit/states/test_file.py

@@ -18,6 +18,7 @@
 # Import Salt Testing libs
 from tests.support.mixins import LoaderModuleMockMixin
 from tests.support.unit import skipIf, TestCase
+from tests.support.helpers import destructiveTest
 from tests.support.mock import (
     NO_MOCK,
     NO_MOCK_REASON,
@@ -26,17 +27,20 @@
     call,
     mock_open,
     patch)
+from tests.support.paths import TMP
 
 # Import salt libs
 import salt.utils.files
 import salt.utils.json
 import salt.utils.platform
 import salt.utils.yaml
+import salt.modules.file as filemod
 import salt.states.file as filestate
 import salt.serializers.yaml as yamlserializer
 import salt.serializers.json as jsonserializer
 import salt.serializers.python as pythonserializer
 from salt.exceptions import CommandExecutionError
+from salt.ext.six.moves import range
 import salt.utils.win_functions
 
 
@@ -2029,3 +2033,69 @@ def test__bad_input(self):
             'comment': '/bad-directory-name/ does not exist or is not a directory.',
         }
         assert filestate.tidied(name='/bad-directory-name/') == exp
+
+
+class TestFilePrivateFunctions(TestCase, LoaderModuleMockMixin):
+    def setup_loader_modules(self):
+        return {
+            filestate: {
+                '__salt__': {'file.stats': filemod.stats},
+            }
+        }
+
+    @destructiveTest
+    @skipIf(salt.utils.platform.is_windows(), 'File modes do not exist on windows')
+    def test__check_directory(self):
+        '''
+        Test the _check_directory function
+        Make sure that recursive file permission checks return correctly
+        '''
+        # set file permissions
+        # Run _check_directory function
+        # Verify that it returns correctly
+        # Delete tmp directory structure
+        root_tmp_dir = os.path.join(TMP, 'test__check_dir')
+        expected_dir_mode = 0o777
+        depth = 3
+        try:
+            def create_files(tmp_dir):
+                for f in range(depth):
+                    path = os.path.join(tmp_dir, 'file_{:03}.txt'.format(f))
+                    with salt.utils.files.fopen(path, 'w+'):
+                        os.chmod(path, expected_dir_mode)
+
+            # Create tmp directory structure
+            os.mkdir(root_tmp_dir)
+            os.chmod(root_tmp_dir, expected_dir_mode)
+            create_files(root_tmp_dir)
+
+            for d in range(depth):
+                dir_name = os.path.join(root_tmp_dir, 'dir{:03}'.format(d))
+                os.mkdir(dir_name)
+                os.chmod(dir_name, expected_dir_mode)
+                create_files(dir_name)
+                for s in range(depth):
+                    sub_dir_name = os.path.join(dir_name, 'dir{:03}'.format(s))
+                    os.mkdir(sub_dir_name)
+                    os.chmod(sub_dir_name, expected_dir_mode)
+                    create_files(sub_dir_name)
+
+            # Set some bad permissions
+            changed_files = {
+                os.path.join(root_tmp_dir, 'file_000.txt'),
+                os.path.join(root_tmp_dir, 'dir002', 'file_000.txt'),
+                os.path.join(root_tmp_dir, 'dir000', 'dir001', 'file_002.txt'),
+                os.path.join(root_tmp_dir, 'dir001', 'dir002'),
+                os.path.join(root_tmp_dir, 'dir002', 'dir000'),
+                os.path.join(root_tmp_dir, 'dir001'),
+            }
+            for c in changed_files:
+                os.chmod(c, 0o770)
+
+            ret = filestate._check_directory(root_tmp_dir, dir_mode=oct(expected_dir_mode),
+                                             file_mode=oct(expected_dir_mode), recurse=['mode'])
+            self.assertSetEqual(changed_files, set(ret[-1].keys()))
+
+        finally:
+            # Cleanup
+            shutil.rmtree(root_tmp_dir)