From b84aeecffe5bd182bdc864d692705dd754b0b62b Mon Sep 17 00:00:00 2001 From: Erik Johnson Date: Tue, 14 Feb 2017 15:21:33 -0600 Subject: [PATCH] salt.fileserver.roots: Fix regression in symlink_list A recent PR of mine removed the logic in symlink_list and fell back to the cached file list generated in _file_lists(). However, this code dates back from before the fileserver backends' symlink_list() functions were modified to return a dict mapping links to their destinations. This fixes the code in _file_lists() so that it returns the correct data. It also fixes the fact that '.' was showing up in the dir list produced by _file_lists(), and updates the associated integration test to include the cachedir in the mocked opts. --- salt/fileserver/roots.py | 97 ++++++++++++++++------ tests/integration/fileserver/roots_test.py | 11 +-- 2 files changed, 77 insertions(+), 31 deletions(-) diff --git a/salt/fileserver/roots.py b/salt/fileserver/roots.py index a1d9d6a8a55d..2fe545cebd18 100644 --- a/salt/fileserver/roots.py +++ b/salt/fileserver/roots.py @@ -280,37 +280,82 @@ def _file_lists(load, form): return cache_match if refresh_cache: ret = { - 'files': [], - 'dirs': [], - 'empty_dirs': [], - 'links': [] + 'files': set(), + 'dirs': set(), + 'empty_dirs': set(), + 'links': {} } + + def _add_to(tgt, fs_root, parent_dir, items): + ''' + Add the files to the target set + ''' + def _translate_sep(path): + ''' + Translate path separators for Windows masterless minions + ''' + return path.replace('\\', '/') if os.path.sep == '\\' else path + + for item in items: + abs_path = os.path.join(parent_dir, item) + log.trace('roots: Processing %s', abs_path) + is_link = os.path.islink(abs_path) + log.trace( + 'roots: %s is %sa link', + abs_path, 'not ' if not is_link else '' + ) + if is_link and __opts__['fileserver_ignoresymlinks']: + continue + rel_path = _translate_sep(os.path.relpath(abs_path, fs_root)) + log.trace('roots: %s relative path is %s', abs_path, rel_path) + if salt.fileserver.is_file_ignored(__opts__, rel_path): + continue + tgt.add(rel_path) + try: + if not os.listdir(abs_path): + ret['empty_dirs'].add(rel_path) + except Exception: + # Generic exception because running os.listdir() on a + # non-directory path raises an OSError on *NIX and a + # WindowsError on Windows. + pass + if is_link: + link_dest = os.readlink(abs_path) + log.trace( + 'roots: %s symlink destination is %s', + abs_path, link_dest + ) + if link_dest.startswith('..'): + joined = os.path.join(abs_path, link_dest) + else: + joined = os.path.join( + os.path.dirname(abs_path), link_dest + ) + rel_dest = os.path.relpath( + os.path.realpath(os.path.normpath(joined)), + fs_root + ) + log.trace( + 'roots: %s relative path is %s', + abs_path, rel_dest + ) + if not rel_dest.startswith('..'): + # Only count the link if it does not point + # outside of the root dir of the fileserver + # (i.e. the "path" variable) + ret['links'][rel_path] = rel_dest + for path in __opts__['file_roots'][load['saltenv']]: for root, dirs, files in os.walk( path, followlinks=__opts__['fileserver_followsymlinks']): - dir_rel_fn = os.path.relpath(root, path) - if __opts__.get('file_client', 'remote') == 'local' and os.path.sep == "\\": - dir_rel_fn = dir_rel_fn.replace('\\', '/') - ret['dirs'].append(dir_rel_fn) - if len(dirs) == 0 and len(files) == 0: - if dir_rel_fn not in ('.', '..') \ - and not salt.fileserver.is_file_ignored(__opts__, dir_rel_fn): - ret['empty_dirs'].append(dir_rel_fn) - for fname in files: - is_link = os.path.islink(os.path.join(root, fname)) - if is_link: - ret['links'].append(fname) - if __opts__['fileserver_ignoresymlinks'] and is_link: - continue - rel_fn = os.path.relpath( - os.path.join(root, fname), - path - ) - if not salt.fileserver.is_file_ignored(__opts__, rel_fn): - if __opts__.get('file_client', 'remote') == 'local' and os.path.sep == "\\": - rel_fn = rel_fn.replace('\\', '/') - ret['files'].append(rel_fn) + _add_to(ret['dirs'], path, root, dirs) + _add_to(ret['files'], path, root, files) + + ret['files'] = sorted(ret['files']) + ret['dirs'] = sorted(ret['dirs']) + ret['empty_dirs'] = sorted(ret['empty_dirs']) + if save_cache: try: salt.fileserver.write_file_list_cache( diff --git a/tests/integration/fileserver/roots_test.py b/tests/integration/fileserver/roots_test.py index 208706aac0da..a7339c50404d 100644 --- a/tests/integration/fileserver/roots_test.py +++ b/tests/integration/fileserver/roots_test.py @@ -148,11 +148,12 @@ def test_dir_list(self): self.assertIn('empty_dir', ret) def test_symlink_list(self): - with patch.dict(roots.__opts__, {'file_roots': self.master_opts['file_roots'], - 'fileserver_ignoresymlinks': False, - 'fileserver_followsymlinks': False, - 'file_ignore_regex': False, - 'file_ignore_glob': False}): + with patch.dict(roots.__opts__, {'cachedir': self.master_opts['cachedir'], + 'file_roots': self.master_opts['file_roots'], + 'fileserver_ignoresymlinks': False, + 'fileserver_followsymlinks': False, + 'file_ignore_regex': False, + 'file_ignore_glob': False}): ret = roots.symlink_list({'saltenv': 'base'}) self.assertDictEqual(ret, {'dest_sym': 'source_sym'})