From: Eric Ball Date: Sat, 22 Feb 2020 03:15:32 +0000 (-0800) Subject: Change the copy_archives method to use copy2 X-Git-Tag: v0.31.0~1^2 X-Git-Url: https://gerrit.linuxfoundation.org/infra/gitweb?a=commitdiff_plain;h=refs%2Fchanges%2F73%2F63173%2F3;p=releng%2Flftools.git Change the copy_archives method to use copy2 Uses copy2 with follow_symlinks set to False, which will copy symlinks as symlinks, rather than following their path. Since glob does not have an option to not follow symlinks, we instead just check for file existence to ignore issues with globbing symlinked paths that "open" fails to find. Issue: RELENG-2576 Change-Id: Ie5a6ffba5c1cb79cff7dd9c691d9b9bd2eaeeba3 Signed-off-by: Eric Ball --- diff --git a/lftools/deploy.py b/lftools/deploy.py index 6537e13f..a7bcbe56 100755 --- a/lftools/deploy.py +++ b/lftools/deploy.py @@ -50,9 +50,14 @@ def _compress_text(dir): paths.extend(glob.glob(search, recursive=True)) for _file in paths: - with open(_file, 'rb') as src, gzip.open('{}.gz'.format(_file), 'wb') as dest: - shutil.copyfileobj(src, dest) - os.remove(_file) + # glob may follow symlink paths that open can't find + if os.path.exists(_file): + log.debug("Compressing file {}".format(_file)) + with open(_file, 'rb') as src, gzip.open('{}.gz'.format(_file), 'wb') as dest: + shutil.copyfileobj(src, dest) + os.remove(_file) + else: + log.info("Could not open path from glob {}".format(_file)) os.chdir(save_dir) @@ -217,7 +222,7 @@ def copy_archives(workspace, pattern=None): :arg str pattern: Space-separated list of Unix style glob patterns. (default: None) """ - archives_dir = os.path.join(workspace, 'archives') + archives_dir = os.path.join(workspace, "archives") dest_dir = os.getcwd() log.debug('Copying files from {} with pattern \'{}\' to {}.'.format( @@ -233,11 +238,14 @@ def copy_archives(workspace, pattern=None): for file_or_dir in os.listdir(archives_dir): f = os.path.join(archives_dir, file_or_dir) try: - log.debug('Moving {}'.format(f)) - shutil.move(f, dest_dir) + log.debug('Copying {}'.format(f)) + shutil.copy2(f, dest_dir, follow_symlinks=False) + except IsADirectoryError: + os.makedirs(os.path.join(dest_dir, file_or_dir), + exist_ok=True) except shutil.Error as e: log.error(e) - raise OSError(errno.EPERM, 'Could not move to', archives_dir) + raise OSError(errno.EPERM, 'Could not copy to', archives_dir) else: log.error('Archives dir {} does not exist.'.format(archives_dir)) raise OSError(errno.ENOENT, 'Missing directory', archives_dir) @@ -262,6 +270,7 @@ def copy_archives(workspace, pattern=None): if len(os.path.basename(src)) > 255: log.warn('Filename {} is over 255 characters. Skipping...'.format( os.path.basename(src))) + continue dest = os.path.join(dest_dir, src[len(workspace)+1:]) log.debug('{} -> {}'.format(src, dest)) @@ -269,9 +278,9 @@ def copy_archives(workspace, pattern=None): if os.path.isfile(src): try: shutil.move(src, dest) - except IOError as e: # Switch to FileNotFoundError when Python 2 support is dropped. + except FileNotFoundError as e: log.debug("Missing path, will create it {}.\n{}".format(os.path.dirname(dest), e)) - os.makedirs(os.path.dirname(dest)) + os.makedirs(os.path.dirname(dest), exist_ok=True) shutil.move(src, dest) else: log.info('Not copying directories: {}.'.format(src)) diff --git a/releasenotes/notes/fix-archive-symlink-loops-0e754059d8db33ba.yaml b/releasenotes/notes/fix-archive-symlink-loops-0e754059d8db33ba.yaml new file mode 100644 index 00000000..ae78b110 --- /dev/null +++ b/releasenotes/notes/fix-archive-symlink-loops-0e754059d8db33ba.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + Change the copy_archives method used by deploy_archives to use copy2 with + follow_symlinks set to False, which will copy symlinks as symlinks, rather + than following their path. Since glob does not have an option to not follow + symlinks, we instead just check for file existence to ignore issues with + globbing symlinked paths that "open" fails to find. diff --git a/tests/fixtures/deploy/workspace-symlinks/archives/child/parent b/tests/fixtures/deploy/workspace-symlinks/archives/child/parent new file mode 120000 index 00000000..a96aa0ea --- /dev/null +++ b/tests/fixtures/deploy/workspace-symlinks/archives/child/parent @@ -0,0 +1 @@ +.. \ No newline at end of file diff --git a/tests/fixtures/deploy/workspace-symlinks/archives/non-existent-file b/tests/fixtures/deploy/workspace-symlinks/archives/non-existent-file new file mode 120000 index 00000000..ef706654 --- /dev/null +++ b/tests/fixtures/deploy/workspace-symlinks/archives/non-existent-file @@ -0,0 +1 @@ +non-existent-file \ No newline at end of file diff --git a/tests/fixtures/deploy/workspace-symlinks/archives/test.log b/tests/fixtures/deploy/workspace-symlinks/archives/test.log new file mode 100644 index 00000000..b3589bf7 --- /dev/null +++ b/tests/fixtures/deploy/workspace-symlinks/archives/test.log @@ -0,0 +1 @@ +This is a test log. diff --git a/tests/test_deploy.py b/tests/test_deploy.py index 77d0a1cc..e33ac12e 100644 --- a/tests/test_deploy.py +++ b/tests/test_deploy.py @@ -11,6 +11,7 @@ import os import sys +import tempfile import pytest import requests @@ -74,6 +75,26 @@ def test_copy_archive_dir(cli_runner, datafiles): assert os.path.exists(os.path.join(stage_dir, 'test.log')) +@pytest.mark.datafiles( + os.path.join(FIXTURE_DIR, 'deploy'), + ) +def test_copy_archive_dir_symlinks(cli_runner, datafiles): + """Test copy_archives() command with symlink loop and a broken symlink, in + order to test that these conditions are handled properly.""" + os.chdir(str(datafiles)) + workspace_dir = os.path.join(str(datafiles), 'workspace-symlinks') + stage_dir = str(datafiles.mkdir("stage_archive")) + + os.chdir(stage_dir) + result = cli_runner.invoke( + cli.cli, + ['--debug', 'deploy', 'copy-archives', workspace_dir], + obj={}) + assert result.exit_code == 0 + + assert os.path.exists(os.path.join(stage_dir, 'test.log')) + + @pytest.mark.datafiles( os.path.join(FIXTURE_DIR, 'deploy'), ) @@ -132,7 +153,7 @@ def test_deploy_archive(cli_runner, datafiles, responses): ) def test_deploy_archive2(datafiles): """Test deploy_archives() command when archives dir is missing.""" - os.chdir(str(datafiles)) + os.chdir(tempfile.mkdtemp(prefix='lftools-test.')) workspace_dir = os.path.join(str(datafiles), 'workspace-noarchives') with pytest.raises(OSError) as excinfo: @@ -145,7 +166,7 @@ def test_deploy_archive2(datafiles): ) def test_deploy_archive3(datafiles): """Test deploy_archives() command when archives dir is a file instead of a dir.""" - os.chdir(str(datafiles)) + os.chdir(tempfile.mkdtemp(prefix='lftools-test.')) workspace_dir = os.path.join(str(datafiles), 'workspace-archivesfile') with pytest.raises(OSError) as excinfo: @@ -158,7 +179,7 @@ def test_deploy_archive3(datafiles): ) def test_deploy_archive4(cli_runner, datafiles, responses): """Test deploy_archives() command when using duplicated patterns.""" - os.chdir(str(datafiles)) + os.chdir(tempfile.mkdtemp(prefix='lftools-test.')) workspace_dir = os.path.join(str(datafiles), 'workspace-patternfile') pattern=["**/*.log", "**/hs_err_*.log", "**/target/**/feature.xml", "**/target/failsafe-reports/failsafe-summary.xml", "**/target/surefire-reports/*-output.txt", "**/target/surefire-reports/*-output.txt", "**/target/failsafe-reports/failsafe-summary.xml", "**/*"] result = deploy_sys.copy_archives(workspace_dir, pattern)