14fa763399
- Print nice error msg when device and driver deprecation data is malformed - Fix another cornercase when preserving symlinks to certificates in /etc/pki - Update the leapp upgrade data files - fixing upgrades with idm-tomcatjss - Resolves: RHEL-16729
284 lines
11 KiB
Diff
284 lines
11 KiB
Diff
From bec6615a9c6fda68153d4d1d76930438a233ae83 Mon Sep 17 00:00:00 2001
|
|
From: Toshio Kuratomi <a.badger@gmail.com>
|
|
Date: Fri, 19 Jan 2024 11:42:15 -0800
|
|
Subject: [PATCH 67/69] Fix another cornercase with symlink handling
|
|
|
|
* Symlinks to a directory inside of /etc/pki were being created as empty directories.
|
|
* Note: unittests being added for both this problem and a second problem:
|
|
two links to the same external file will be copied as two separate files but ideally we want one
|
|
to be a copy and the other to link to the copy. The unittests for the second problem are
|
|
commented out.
|
|
|
|
Fixes: https://issues.redhat.com/browse/RHEL-3284
|
|
---
|
|
.../libraries/userspacegen.py | 66 ++++++---
|
|
.../tests/unit_test_targetuserspacecreator.py | 128 ++++++++++++++++++
|
|
2 files changed, 173 insertions(+), 21 deletions(-)
|
|
|
|
diff --git a/repos/system_upgrade/common/actors/targetuserspacecreator/libraries/userspacegen.py b/repos/system_upgrade/common/actors/targetuserspacecreator/libraries/userspacegen.py
|
|
index 8d804407..d917bfd5 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 bd49f657..19b760a1 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
|
|
--
|
|
2.42.0
|
|
|