Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[archive] symlinks not copied when directory of the same name created automatically before #1404

Closed
pmoravec opened this issue Aug 29, 2018 · 3 comments

Comments

@pmoravec
Copy link
Contributor

Reproducer (of Fedora):

# have /etc/sysconfig/rhn (collected by system plugin) as a symlink
mv /etc/sysconfig/rhn /etc/sysconfig/rhn.target
ln -s /etc/sysconfig/rhn.target /etc/sysconfig/rhn

# have a symlink (collected by yum plugin) pointing "into" the rhn symlink
date > /etc/sysconfig/rhn/rhn.link.repo
ln -s /etc/sysconfig/rhn/rhn.link.repo /etc/yum.repos.d/rhn.link.repo

# call sos for those 2 relevant plugins:
sosreport -o yum,system --batch --build

returns in sos_logs/system-plugin-errors.txt :

Traceback (most recent call last):
  File "/root/sos-master/sos/sosreport.py", line 1104, in collect_plugin
    plug.collect()
  File "/root/sos-master/sos/plugins/__init__.py", line 947, in collect
    self._collect_copy_specs()
  File "/root/sos-master/sos/plugins/__init__.py", line 905, in _collect_copy_specs
    self._do_copy_path(path)
  File "/root/sos-master/sos/plugins/__init__.py", line 435, in _do_copy_path
    self._copy_dir(srcpath)
  File "/root/sos-master/sos/plugins/__init__.py", line 381, in _copy_dir
    self._do_copy_path(os.path.join(srcpath, afile), dest=None)
  File "/root/sos-master/sos/plugins/__init__.py", line 431, in _do_copy_path
    self._copy_symlink(srcpath)
  File "/root/sos-master/sos/plugins/__init__.py", line 353, in _copy_symlink
    self.archive.add_link(reldest, dstpath)
  File "/root/sos-master/sos/archive.py", line 315, in add_link
    dest = self._check_path(link_name, P_LINK)
  File "/root/sos-master/sos/archive.py", line 224, in _check_path
    raise ValueError(ve_msg % (dest, "symbolic link"))
ValueError: path '/var/tmp/sos.qQBlL1/sosreport-pmoravec-sat63-2018-08-28-etvyibz/etc/sysconfig/rhn' exists and is not a symbolic link

The cause is:

  • (note that yum plugin does its work before system plugin here; calling sosreport with 1 thread prevents this but it is just matter of renaming plugins to see the same error, then)
  • yum plugin copies the file (here target of a symlink but that doesnt matter much) from within /etc/sysconfig/rhn
  • _makedirs method in https://github.com/sosreport/sos/blob/master/sos/archive.py#L347 creates relative destination directory etc/sysconfig/rhn instead of creating that as symlink
  • system plugin attempts to collect /etc/sysconfig/rhn as a symlink, but detects it was collected as directory already, and raises the above exception

Two possible approaches:

  • "fix cause": _makedirs method shall stat all source subpaths before it creates them and if these are symlinks, it creates relevant symlink instead
    • but then system plugin will not attempt to copy the symlink (and its content) as the symlink will be there, already - thus content of the symlink target will not be copied
  • "fix consequence": when adding a link (somewhere at https://github.com/sosreport/sos/blob/master/sos/plugins/__init__.py#L353), we shall detect if we havent collected the destination as a directory and if so, either:
    • move that directory from source symlink name to destination symlink name and collect the symlink again (wont it suffer by same problem like "fix cause"?)
    • or remove the directory first and then collect the symlink - as we will collect whole subdirectory, including previously collected content, this should be sane
@pmoravec pmoravec added the high label Aug 29, 2018
@bmr-cymru
Copy link
Member

It's a result of how we handle intermediate path components in symbolic link resolution. I don't have a fix right now (or rather, there's a possible hack, and a much more complex fix).

@bmr-cymru
Copy link
Member

Fixed in commit 75d7590

@pmoravec
Copy link
Contributor Author

pmoravec commented Sep 5, 2018

Sadly this causes uncaught exceptions in several plugins. I was able to limit it to running only a plugin that collects only /lib/systemd/user where:

# file $(find /lib/systemd/user)
/lib/systemd/user:                        directory
/lib/systemd/user/basic.target:           ASCII text
/lib/systemd/user/bluetooth.target:       symbolic link to `../system/bluetooth.target'
..
#

and the error is:

Traceback (most recent call last):
  File "/root/sos-master/sos/sosreport.py", line 1105, in collect_plugin
    plug.collect()
  File "/root/sos-master/sos/plugins/__init__.py", line 965, in collect
    self._collect_copy_specs()
  File "/root/sos-master/sos/plugins/__init__.py", line 923, in _collect_copy_specs
    self._do_copy_path(path)
  File "/root/sos-master/sos/plugins/__init__.py", line 453, in _do_copy_path
    self._copy_dir(srcpath)
  File "/root/sos-master/sos/plugins/__init__.py", line 405, in _copy_dir
    raise e
OSError: [Errno 2] No such file or directory: '/var/tmp/sos.yJS3mf/sosreport-pmoravec-sat63-2018-09-05-rkavymt/lib/systemd'

Here, lib was created in the archive as a broken symbolic link to usr/lib (like /lib is a symlink to /usr/lib). An attempt to create subdirectory lib/systemd then fails due to the broken symlink :(

@pmoravec pmoravec reopened this Sep 5, 2018
bmr-cymru added a commit that referenced this issue Sep 10, 2018
Fix the creation of leading path components for both paths that
contain intermediate components that are symbolic links (with both
absolute and relative targets), and those that contain only
directory components.

Since symlinks may link to other files, and other symlinks, it is
necessary to handle these paths recursively and to include any
intermediate symlinked directories, or symlink targets in the set
of paths added to the archive.

Related: #1404

Signed-off-by: Bryn M. Reeves <bmr@redhat.com>
bmr-cymru added a commit that referenced this issue Sep 12, 2018
Since we may be dealing with paths that contain intermediate
symlinked directories, it is necessary to canonicalize the path
for the link target in order to eliminate additional levels of
symbolic links, and to calculate the correct relative path to
use within the archive.

Related: #1404

Signed-off-by: Bryn M. Reeves <bmr@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants