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

ansible-galaxy - Avoid infinite loop by following symlinks when building collections #65833

Open
wants to merge 4 commits into
base: devel
Choose a base branch
from

Conversation

ssato
Copy link
Contributor

@ssato ssato commented Dec 14, 2019

SUMMARY

ansible-galaxy looks trying to follow symbolic links if targets of links are dirs or files
in the collection repository when building ansible collection, and it might search
them infinitely like the following example log.

This PR fixes this problem and make ansible-galaxy list each of them only once.

$ cat galaxy.yml
---
namespace: ssato
name: an_example
version: 0.1.0
description: A collection
readme: README.md
authors:
  - Satoru SATOH (https://github.com/ssato/)
dependencies: []
license:
  - MIT
$ mkdir -p a/b/c/d/e/f/g
$ cd a/b/c/d/e/f/g
$ ln -s ../../../../../../../ ./h
$ ls -l
合計 0
lrwxrwxrwx. 1 ssato ssato 21 12月 13 16:22 h -> ../../../../../../../
$ cd -
/tmp/ansible-galaxy-pr-avoid-inifinite-loop
$ ansible-galaxy collection build -v --force --output-path /tmp
Using /etc/ansible/ansible.cfg as config file
Created collection for ssato.an_example at /tmp/ssato-an_example-0.1.0.tar.gz
$ tar tvf /tmp/ssato-an_example-0.1.0.tar.gz
-rw-r--r-- 0/0             639 2019-12-13 16:22 MANIFEST.json
-rw-r--r-- 0/0          143153 2019-12-13 16:22 FILES.json
drwxr-xr-x 0/0               0 2019-12-13 16:21 a/
drwxr-xr-x 0/0               0 2019-12-13 16:21 a/b/
drwxr-xr-x 0/0               0 2019-12-13 16:21 a/b/c/
drwxr-xr-x 0/0               0 2019-12-13 16:21 a/b/c/d/
drwxr-xr-x 0/0               0 2019-12-13 16:21 a/b/c/d/e/
drwxr-xr-x 0/0               0 2019-12-13 16:21 a/b/c/d/e/f/
drwxr-xr-x 0/0               0 2019-12-13 16:22 a/b/c/d/e/f/g/
drwxr-xr-x 0/0               0 2019-12-13 16:21 a/b/c/d/e/f/g/h/
drwxr-xr-x 0/0               0 2019-12-13 16:21 a/b/c/d/e/f/g/h/a/
drwxr-xr-x 0/0               0 2019-12-13 16:21 a/b/c/d/e/f/g/h/a/b/
drwxr-xr-x 0/0               0 2019-12-13 16:21 a/b/c/d/e/f/g/h/a/b/c/
drwxr-xr-x 0/0               0 2019-12-13 16:21 a/b/c/d/e/f/g/h/a/b/c/d/
drwxr-xr-x 0/0               0 2019-12-13 16:21 a/b/c/d/e/f/g/h/a/b/c/d/e/
drwxr-xr-x 0/0               0 2019-12-13 16:21 a/b/c/d/e/f/g/h/a/b/c/d/e/f/
drwxr-xr-x 0/0               0 2019-12-13 16:22 a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/
drwxr-xr-x 0/0               0 2019-12-13 16:21 a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/
        ... (snip) ...
drwxr-xr-x 0/0               0 2019-12-13 16:21 a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/
$
ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

ansible-galaxy

ADDITIONAL INFORMATION
  • I confrmed it works with the ansible 2.9.2.
$ ansible --version
ansible 2.9.2
  config file = /etc/ansible/ansible.cfg
  configured module search path = ['/home/ssato/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python3.7/site-packages/ansible
  executable location = /usr/bin/ansible
  python version = 3.7.5 (default, Oct 17 2019, 12:16:48) [GCC 9.2.1 20190827 (Red Hat 9.2.1-1)]
$

@ansibot ansibot added affects_2.10 bug core_review needs_triage python3 support:core needs_revision and removed core_review labels Dec 14, 2019
@ssato
Copy link
Contributor Author

@ssato ssato commented Dec 16, 2019

It seems that a shippable test failed with unknown reason again. What should I do for that?

@ssato
Copy link
Contributor Author

@ssato ssato commented Dec 20, 2019

Again, some shippable tests failed because of unknown reasons (those look a few yum repos problems but I'm not sure) and I don't think I can do something for them.

@ssato ssato force-pushed the ansible-collection-avoid-infinite-loop-of-symlinks branch from 152dbb8 to affe600 Compare Dec 20, 2019
@ansibot ansibot added core_review and removed needs_revision labels Dec 20, 2019
@ansibot ansibot added the stale_ci label Dec 28, 2019
@ssato
Copy link
Contributor Author

@ssato ssato commented Jan 5, 2020

Hello, how is the status of this PR? If any actions I should do to merge this, please let me know.

@samdoran
Copy link
Contributor

@samdoran samdoran commented Jan 7, 2020

What does the current failure look like? This solution ignores the problem and allows collections with recursive symlinks to ship, which we should not allow.

This should be a fatal error, not a traceback (which is why I was wondering what the original error you are seeing is).

needs_info

@samdoran samdoran removed the needs_triage label Jan 7, 2020
@ansibot ansibot added needs_info and removed core_review labels Jan 7, 2020
@ssato
Copy link
Contributor Author

@ssato ssato commented Jan 8, 2020

What does the current failure look like?

$ tar tvf /tmp/ssato-an_example-0.1.0.tar.gz
-rw-r--r-- 0/0             639 2019-12-13 16:22 MANIFEST.json
-rw-r--r-- 0/0          143153 2019-12-13 16:22 FILES.json
drwxr-xr-x 0/0               0 2019-12-13 16:21 a/
drwxr-xr-x 0/0               0 2019-12-13 16:21 a/b/
drwxr-xr-x 0/0               0 2019-12-13 16:21 a/b/c/
drwxr-xr-x 0/0               0 2019-12-13 16:21 a/b/c/d/
drwxr-xr-x 0/0               0 2019-12-13 16:21 a/b/c/d/e/
drwxr-xr-x 0/0               0 2019-12-13 16:21 a/b/c/d/e/f/
drwxr-xr-x 0/0               0 2019-12-13 16:22 a/b/c/d/e/f/g/
drwxr-xr-x 0/0               0 2019-12-13 16:21 a/b/c/d/e/f/g/h/
drwxr-xr-x 0/0               0 2019-12-13 16:21 a/b/c/d/e/f/g/h/a/
drwxr-xr-x 0/0               0 2019-12-13 16:21 a/b/c/d/e/f/g/h/a/b/
drwxr-xr-x 0/0               0 2019-12-13 16:21 a/b/c/d/e/f/g/h/a/b/c/
drwxr-xr-x 0/0               0 2019-12-13 16:21 a/b/c/d/e/f/g/h/a/b/c/d/
drwxr-xr-x 0/0               0 2019-12-13 16:21 a/b/c/d/e/f/g/h/a/b/c/d/e/
drwxr-xr-x 0/0               0 2019-12-13 16:21 a/b/c/d/e/f/g/h/a/b/c/d/e/f/
drwxr-xr-x 0/0               0 2019-12-13 16:22 a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/
drwxr-xr-x 0/0               0 2019-12-13 16:21 a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/
        ... (snip) ...
drwxr-xr-x 0/0               0 2019-12-13 16:21 a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/a/b/c/d/e/f/g/h/
$

There are no explicit errors out, however this is not an intended behavior apparently. (see also the description of my PR.) ansible-galaxy should stop storing symlink target dirs if those are stored
already, I think.

This solution ignores the problem and allows collections with recursive symlinks to ship...
If there are recursive links in the collection:

  • Current code looks trying to follow link targets forever.
  • The code with my change applied should try follow link targets just once.
  • Both of the code should never store links to dirs themselves, AFAIK.

And here are some additional background of this PR.

ansible-galaxy looks searching a collection in <collection_search_path>/ansible_collections/,
/usr/share/ansible/collections/ for example. So we have to put ansible collections into some dir
and set ANSIBLE_COLLECTON_PATH to that dir if we want to perform integration test of them.

But we should be able to do integration test if we can put a link to the collection top dir
such like:

  • tests/integration/collections/foo/hello (foo: namespace, hello: collection name)
    • hello is a symlink to the collection top dir (../../../../)

And IMHO, this change may resolve the issue 60215 partially as well.

@ansibot ansibot added core_review and removed needs_info labels Jan 8, 2020
@samdoran
Copy link
Contributor

@samdoran samdoran commented Jan 8, 2020

Ok, I tested this out and I see what you mean. anisble-galaxy collection build ends up creating a tar file full of directories because it followed the recursive link.

@samdoran
Copy link
Contributor

@samdoran samdoran commented Jan 8, 2020

Please create a changelog fragment. See this fragment as an example.

@samdoran samdoran requested a review from jborean93 Jan 8, 2020
@ssato ssato force-pushed the ansible-collection-avoid-infinite-loop-of-symlinks branch from affe600 to 902ad6c Compare Jan 12, 2020
@ssato
Copy link
Contributor Author

@ssato ssato commented Jan 12, 2020

@samdoran Thanks a lot for your advice! I added a changelog fragment, changelogs/fragments/65833-ansible-collection-avoid-infinite-loop-of-symlinks.yml.

@ansibot ansibot added support:community needs_revision and removed stale_ci core_review labels Jan 12, 2020
@ansibot ansibot added core_review and removed needs_revision labels Jan 14, 2020
@samdoran
Copy link
Contributor

@samdoran samdoran commented Jan 16, 2020

Thank you for adding a changelog fragment. Could you please also add integration tests here?

@ansibot ansibot added the stale_ci label Jan 24, 2020
@ansibot ansibot added needs_rebase needs_revision and removed core_review labels Jun 17, 2020
@ansibot ansibot added pre_azp and removed stale_ci labels Dec 5, 2020
@ansibot ansibot removed the support:community label Mar 4, 2021
@s-hertel s-hertel added P3 labels Apr 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects_2.10 bug needs_rebase needs_revision P3 pre_azp python3 support:core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants