diff --git a/roles/wip-upload-logs-swift/library/test-fixtures/links/controller/service_log.txt b/roles/wip-upload-logs-swift/library/test-fixtures/links/controller/service_log.txt new file mode 100644 index 000000000..e69de29bb diff --git a/roles/wip-upload-logs-swift/library/test-fixtures/links/job-output.json b/roles/wip-upload-logs-swift/library/test-fixtures/links/job-output.json new file mode 100644 index 000000000..c8cd7e92d --- /dev/null +++ b/roles/wip-upload-logs-swift/library/test-fixtures/links/job-output.json @@ -0,0 +1 @@ +{"test": "foo"} diff --git a/roles/wip-upload-logs-swift/library/test-fixtures/links/symlink_loop/placeholder b/roles/wip-upload-logs-swift/library/test-fixtures/links/symlink_loop/placeholder new file mode 100644 index 000000000..e69de29bb diff --git a/roles/wip-upload-logs-swift/library/test_zuul_swift_upload.py b/roles/wip-upload-logs-swift/library/test_zuul_swift_upload.py index 60aa31941..aec04a22f 100644 --- a/roles/wip-upload-logs-swift/library/test_zuul_swift_upload.py +++ b/roles/wip-upload-logs-swift/library/test_zuul_swift_upload.py @@ -14,17 +14,45 @@ # See the License for the specific language governing permissions and # limitations under the License. -import unittest -from .zuul_swift_upload import FileList, Indexer import os +import testtools +import fixtures from bs4 import BeautifulSoup +from .zuul_swift_upload import FileList, Indexer + + FIXTURE_DIR = os.path.join(os.path.dirname(__file__), 'test-fixtures') -class TestFileList(unittest.TestCase): +class SymlinkFixture(fixtures.Fixture): + links = [ + ('bad_symlink', '/etc'), + ('bad_symlink_file', '/etc/issue'), + ('good_symlink', 'controller'), + ('recursive_symlink', '.'), + ('symlink_file', 'job-output.json'), + ('symlink_loop_a', 'symlink_loop'), + ('symlink_loop/symlink_loop_b', '..'), + ] + + def _setUp(self): + self._cleanup() + for (src, target) in self.links: + path = os.path.join(FIXTURE_DIR, 'links', src) + os.symlink(target, path) + self.addCleanup(self._cleanup) + + def _cleanup(self): + for (src, target) in self.links: + path = os.path.join(FIXTURE_DIR, 'links', src) + if os.path.exists(path): + os.unlink(path) + + +class TestFileList(testtools.TestCase): def assert_files(self, result, files): self.assertEqual(len(result), len(files)) @@ -100,6 +128,25 @@ class TestFileList(unittest.TestCase): ('inventory.yaml', 'text/plain', None), ]) + def test_symlinks(self): + '''Test symlinks''' + fl = FileList() + self.useFixture(SymlinkFixture()) + fl.add(os.path.join(FIXTURE_DIR, 'links/')) + self.assert_files(fl, [ + ('', 'application/directory', None), + ('controller', 'application/directory', None), + ('good_symlink', 'application/directory', None), + ('recursive_symlink', 'application/directory', None), + ('symlink_loop', 'application/directory', None), + ('symlink_loop_a', 'application/directory', None), + ('job-output.json', 'application/json', None), + ('symlink_file', 'text/plain', None), + ('controller/service_log.txt', 'text/plain', None), + ('symlink_loop/symlink_loop_b', 'application/directory', None), + ('symlink_loop/placeholder', 'text/plain', None), + ]) + def test_index_files(self): '''Test index generation''' fl = FileList() diff --git a/roles/wip-upload-logs-swift/library/zuul_swift_upload.py b/roles/wip-upload-logs-swift/library/zuul_swift_upload.py index de17a2533..9ccf605e0 100755 --- a/roles/wip-upload-logs-swift/library/zuul_swift_upload.py +++ b/roles/wip-upload-logs-swift/library/zuul_swift_upload.py @@ -130,6 +130,15 @@ class FileList(object): def __len__(self): return len(self.file_list) + @staticmethod + def _path_in_tree(root, path): + full_path = os.path.realpath(os.path.abspath( + os.path.expanduser(path))) + if not full_path.startswith(root): + logging.debug("Skipping path outside root: %s" % (path,)) + return False + return True + def add(self, file_path): """ Generate a list of files to upload to swift. Recurses through @@ -143,6 +152,9 @@ class FileList(object): relative_path = os.path.basename(file_path) file_list.append(FileDetail(file_path, relative_path)) elif os.path.isdir(file_path): + original_root = os.path.realpath(os.path.abspath( + os.path.expanduser(file_path))) + parent_dir = os.path.dirname(file_path) if not file_path.endswith('/'): filename = os.path.basename(file_path) @@ -150,6 +162,9 @@ class FileList(object): relative_name = os.path.relpath(full_path, parent_dir) file_list.append(FileDetail(full_path, relative_name, filename)) + # TODO: this will copy the result of symlinked files, but + # it won't follow directory symlinks. If we add that, we + # should ensure that we don't loop. for path, folders, files in os.walk(file_path): # Sort folder in-place so that we recurse in order. files.sort(key=lambda x: x.lower()) @@ -158,16 +173,18 @@ class FileList(object): # and the one being currently walked. relative_path = os.path.relpath(path, parent_dir) - for f in folders: - filename = os.path.basename(f) + for filename in folders: full_path = os.path.join(path, filename) + if not self._path_in_tree(original_root, full_path): + continue relative_name = os.path.relpath(full_path, parent_dir) file_list.append(FileDetail(full_path, relative_name, filename)) - for f in files: - filename = os.path.basename(f) + for filename in files: full_path = os.path.join(path, filename) + if not self._path_in_tree(original_root, full_path): + continue relative_name = os.path.relpath(full_path, parent_dir) file_detail = FileDetail(full_path, relative_name) file_list.append(file_detail)