diff --git a/repos/system_upgrade/common/actors/targetuserspacecreator/libraries/userspacegen.py b/repos/system_upgrade/common/actors/targetuserspacecreator/libraries/userspacegen.py index 8d8044072a..d917bfd511 100644 --- a/repos/system_upgrade/common/actors/targetuserspacecreator/libraries/userspacegen.py +++ b/repos/system_upgrade/common/actors/targetuserspacecreator/libraries/userspacegen.py @@ -350,7 +350,7 @@ def _mkdir_with_copied_mode(path, mode_from): def _choose_copy_or_link(symlink, srcdir): """ - Copy file contents or create a symlink depending on where the pointee resides. + Determine whether to copy file contents or create a symlink depending on where the pointee resides. :param symlink: The source symlink to follow. This must be an absolute path. :param srcdir: The root directory that every piece of content must be present in. @@ -415,7 +415,7 @@ def _choose_copy_or_link(symlink, srcdir): # To make comparisons, we need to resolve all symlinks in the directory # structure leading up to pointee. However, we can't include pointee # itself otherwise it will resolve to the file that it points to in the - # end. + # end (which would be wrong if pointee_filename is a symlink). canonical_pointee_dir, pointee_filename = os.path.split(pointee_as_abspath) canonical_pointee_dir = os.path.realpath(canonical_pointee_dir) @@ -454,6 +454,35 @@ def _choose_copy_or_link(symlink, srcdir): return ('copy', pointee_as_abspath) +def _copy_symlinks(symlinks_to_process, srcdir): + """ + Copy file contents or create a symlink depending on where the pointee resides. + + :param symlinks_to_process: List of 2-tuples of (src_path, target_path). Each src_path + should be an absolute path to the symlink. target_path is the path to where we + need to create either a link or a copy. + :param srcdir: The root directory that every piece of content must be present in. + :raises ValueError: if the arguments are not correct + """ + for source_linkpath, target_linkpath in symlinks_to_process: + try: + action, source_path = _choose_copy_or_link(source_linkpath, srcdir) + except BrokenSymlinkError as e: + # Skip and report broken symlinks + api.current_logger().warning('{} Will not copy the file!'.format(str(e))) + continue + + if action == "copy": + # Note: source_path could be a directory, so '-a' or '-r' must be + # given to cp. + run(['cp', '-a', source_path, target_linkpath]) + elif action == 'link': + run(["ln", "-s", source_path, target_linkpath]) + else: + # This will not happen unless _copy_or_link() has a bug. + raise RuntimeError("Programming error: _copy_or_link() returned an unknown action:{}".format(action)) + + def _copy_decouple(srcdir, dstdir): """ Copy files inside of `srcdir` to `dstdir` while decoupling symlinks. @@ -467,7 +496,6 @@ def _copy_decouple(srcdir, dstdir): .. warning:: `dstdir` must already exist. """ - symlinks_to_process = [] for root, directories, files in os.walk(srcdir): # relative path from srcdir because srcdir is replaced with dstdir for # the copy. @@ -476,11 +504,24 @@ def _copy_decouple(srcdir, dstdir): # Create all directories with proper permissions for security # reasons (Putting private data into directories that haven't had their # permissions set appropriately may leak the private information.) + symlinks_to_process = [] for directory in directories: source_dirpath = os.path.join(root, directory) target_dirpath = os.path.join(dstdir, relpath, directory) + + # Defer symlinks until later because we may end up having to copy + # the file contents and the directory may not exist yet. + if os.path.islink(source_dirpath): + symlinks_to_process.append((source_dirpath, target_dirpath)) + continue + _mkdir_with_copied_mode(target_dirpath, source_dirpath) + # Link or create all directories that were pointed to by symlinks and + # then reset symlinks_to_process for use by files. + _copy_symlinks(symlinks_to_process, srcdir) + symlinks_to_process = [] + for filename in files: source_filepath = os.path.join(root, filename) target_filepath = os.path.join(dstdir, relpath, filename) @@ -494,24 +535,7 @@ def _copy_decouple(srcdir, dstdir): # Not a symlink so we can copy it now too run(['cp', '-a', source_filepath, target_filepath]) - # Now process all symlinks - for source_linkpath, target_linkpath in symlinks_to_process: - try: - action, source_path = _choose_copy_or_link(source_linkpath, srcdir) - except BrokenSymlinkError as e: - # Skip and report broken symlinks - api.current_logger().warning('{} Will not copy the file!'.format(str(e))) - continue - - if action == "copy": - # Note: source_path could be a directory, so '-a' or '-r' must be - # given to cp. - run(['cp', '-a', source_path, target_linkpath]) - elif action == 'link': - run(["ln", "-s", source_path, target_linkpath]) - else: - # This will not happen unless _copy_or_link() has a bug. - raise RuntimeError("Programming error: _copy_or_link() returned an unknown action:{}".format(action)) + _copy_symlinks(symlinks_to_process, srcdir) def _copy_certificates(context, target_userspace): diff --git a/repos/system_upgrade/common/actors/targetuserspacecreator/tests/unit_test_targetuserspacecreator.py b/repos/system_upgrade/common/actors/targetuserspacecreator/tests/unit_test_targetuserspacecreator.py index bd49f65783..19b760a184 100644 --- a/repos/system_upgrade/common/actors/targetuserspacecreator/tests/unit_test_targetuserspacecreator.py +++ b/repos/system_upgrade/common/actors/targetuserspacecreator/tests/unit_test_targetuserspacecreator.py @@ -381,6 +381,70 @@ def temp_directory_layout(tmp_path, initial_structure): }, id="Absolute_symlink_to_a_file_inside_via_a_symlink_to_the_rootdir" )), + # This should be fixed but not necessarily for this release. + # It makes sure that when we have two separate links to the + # same file outside of /etc/pki, one of the links is copied + # as a real file and the other is made a link to the copy. + # (Right now, the real file is copied in place of both links.) + # (pytest.param( + # { + # 'dir': { + # 'fileA': '/outside/fileC', + # 'fileB': '/outside/fileC', + # }, + # 'outside': { + # 'fileC': None, + # }, + # }, + # { + # 'dir': { + # 'fileA': None, + # 'fileB': '/dir/fileA', + # }, + # }, + # id="Absolute_two_symlinks_to_the_same_copied_file" + # )), + (pytest.param( + { + 'dir': { + 'fileA': None, + 'link_to_dir': '/dir/inside', + 'inside': { + 'fileB': None, + }, + }, + }, + { + 'dir': { + 'fileA': None, + 'link_to_dir': '/dir/inside', + 'inside': { + 'fileB': None, + }, + }, + }, + id="Absolute_symlink_to_a_dir_inside" + )), + (pytest.param( + { + 'dir': { + 'fileA': None, + 'link_to_dir': '/outside', + }, + 'outside': { + 'fileB': None, + }, + }, + { + 'dir': { + 'fileA': None, + 'link_to_dir': { + 'fileB': None, + }, + }, + }, + id="Absolute_symlink_to_a_dir_outside" + )), (pytest.param( # This one is very tricky: # * The user has made /etc/pki a symlink to some other directory that @@ -671,6 +735,70 @@ def temp_directory_layout(tmp_path, initial_structure): }, id="Relative_symlink_to_a_file_inside_via_a_symlink_to_the_rootdir" )), + # This should be fixed but not necessarily for this release. + # It makes sure that when we have two separate links to the + # same file outside of /etc/pki, one of the links is copied + # as a real file and the other is made a link to the copy. + # (Right now, the real file is copied in place of both links.) + # (pytest.param( + # { + # 'dir': { + # 'fileA': '../outside/fileC', + # 'fileB': '../outside/fileC', + # }, + # 'outside': { + # 'fileC': None, + # }, + # }, + # { + # 'dir': { + # 'fileA': None, + # 'fileB': 'fileA', + # }, + # }, + # id="Relative_two_symlinks_to_the_same_copied_file" + # )), + (pytest.param( + { + 'dir': { + 'fileA': None, + 'link_to_dir': '../outside', + }, + 'outside': { + 'fileB': None, + }, + }, + { + 'dir': { + 'fileA': None, + 'link_to_dir': { + 'fileB': None, + }, + }, + }, + id="Relative_symlink_to_a_dir_outside" + )), + (pytest.param( + { + 'dir': { + 'fileA': None, + 'link_to_dir': 'inside', + 'inside': { + 'fileB': None, + }, + }, + }, + { + 'dir': { + 'fileA': None, + 'link_to_dir': 'inside', + 'inside': { + 'fileB': None, + }, + }, + }, + id="Relative_symlink_to_a_dir_inside" + )), (pytest.param( # This one is very tricky: # * The user has made /etc/pki a symlink to some other directory that