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 outside archive directory created by _make_leading_paths #1710

Closed
pmoravec opened this issue Jun 26, 2019 · 1 comment
Closed

Comments

@pmoravec
Copy link
Contributor

When archive._make_leading_paths is called to a path containing a symlink to an absolute path, the method creates a symlink outside the archive directory (to the absolute path of the destination of the symlink). Copying that archive to a different machine then causes broken symlink.

Reproducer:

mv /var/lib/mistral{,.prev}
mkdir -p /var/lib/mistral/overcloud
ln -s /var/lib/mistral/overcloud /var/lib/mistral/config-download-latest
echo "some ansible log content" > /var/lib/mistral/overcloud/ansible.log

Now, call:

sosreport -o openstack_instack --batch --build

and see that /var/tmp/sosreport-myhostname-2019-06-25-wpyemcm/var/lib/mistral/config-download-latest is a symlink to /var/lib/mistral/overcloud, while the archive directory itself does not contain a copy of the logfile.

Root cause of the problem: https://github.com/sosreport/sos/blob/master/sos/archive.py#L221-L237 calls:

target = os.readlink(src_path)
..
os.symlink(target, abs_path)

This works well for symlinks with relative destination. For absolute path destination, something like:

if os.path.isabs(target):
    target = target.strip(os.path.commonpath([target, src_path])).strip(os.sep)

must be called in between. This will make the target relative from absolute.

BUT this works well only when target and src_path are in common subdirectory. If e.g. /var/lib/mistral/config-download-latest would be a symlink to /var/log, we must set target to ../../log and create that relative path via self._make_leading_paths first.

The above proposal is sufficient for the real world scenario (Openstack Instack having /var/lib/mistral/config-download-latest: symbolic link to /var/lib/mistral/overcloud per my understanding), but it is not ideal.

@pmoravec
Copy link
Contributor Author

.. or we can create symlink to (not-tested):

target = os.readlink(src_path)
..
target = os.path.relpath(target, start=src_path)
os.symlink(target, abs_path)

(and somehow take care the target path is created)

pmoravec added a commit to pmoravec/sos that referenced this issue Jun 27, 2019
Calling _make_leading_paths for a symlink with absolute symlink target
must create the symlink relative to the source. This will prevent
creating symlinks outside sosreport build dir.

Resolves: sosreport#1710

Signed-off-by: Pavel Moravec <pmoravec@redhat.com>
pmoravec added a commit to pmoravec/sos that referenced this issue Jul 9, 2019
Calling _make_leading_paths for a symlink with absolute symlink target
must create the symlink relative to the source. This will prevent
creating symlinks outside sosreport build dir.

Resolves: sosreport#1710

Signed-off-by: Pavel Moravec <pmoravec@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

Successfully merging a pull request may close this issue.

1 participant