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

Current working directory affects execution environment generated on prerun #1654

Closed
xabinapal opened this issue Jul 10, 2021 · 4 comments
Closed
Labels

Comments

@xabinapal
Copy link
Contributor

Summary

Not sure if this should be a bug, a proposal or even if it is the intended behaviour.

When running on a collection repository, ansible-lint caches it on a ~/.cache/ansible-lint/<HASH>/collections/ansible_collections directory as part of the prerun process. When being launched on a directory other than the root one, such as a role, it does not detect that the role is part of a collection, and only caches the role directory, not the collection one.

I think that this behaviour is wrong because ansible-lint always tries to find the project root directory and work from there, except during the prerun. In fact, the hash is generated based on the root directory even when being launched on a subdirectory. After #1483, prerun was made public so other packages can use it, and this can cause issues to those packages.

I already have a PR ready to be submitted in case this is accepted.

Issue Type
  • Bug Report
Ansible and Ansible Lint details
$ ansible --version
ansible [core 2.11.2]
  config file = None
  configured module search path = ['~/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = ~/.pyenv/versions/3.9.1/envs/ansible-tests/lib/python3.9/site-packages/ansible
  ansible collection location = ~/.ansible/collections:/usr/share/ansible/collections
  executable location = ~/.pyenv/versions/ansible-tests/bin/ansible
  python version = 3.9.1 (default, Feb 21 2021, 17:37:26) [Clang 12.0.0 (clang-1200.0.31.1)]
  jinja version = 3.0.1
  libyaml = True

$ ansible-lint --version
ansible-lint 5.1.0a1 using ansible 2.11.2
  • ansible installation method: pip
  • ansible-lint installation method: pip
OS / ENVIRONMENT

Can be reproduced both in ansible-lint 5.0.12 and 5.1.0a1.

STEPS TO REPRODUCE
$ ansible-galaxy collection init xabinapal.test
$ cd xabinapal/test
$ touch .ansible-lint

$ cd roles
$ ansible-galaxy role init test

$ cd ..
$ git init
$ git add -A

Linting from the repository root detects it as a collection (as it finds a galaxy.yml file) and sets the ANSIBLE_COLLECTIONS_PATH environment variable:

$ ansible-lint -v
INFO     Added ANSIBLE_COLLECTIONS_PATH=~/.cache/ansible-lint/b9d075/collections
INFO     Added ANSIBLE_ROLES_PATH=~/.ansible/roles:/usr/share/ansible/roles:/etc/ansible/roles:roles:~/.cache/ansible-lint/b9d075/roles
INFO     Added ANSIBLE_COLLECTIONS_PATH=~/.cache/ansible-lint/b9d075/collections:~/.cache/ansible-lint/b9d075/collections
INFO     Added ANSIBLE_ROLES_PATH=~/.ansible/roles:/usr/share/ansible/roles:/etc/ansible/roles:roles:~/.cache/ansible-lint/b9d075/roles:roles:~/.cache/ansible-lint/b9d075/roles

Executing in a role directory (or, any other directory other than the root) also includes the ANSIBLE_COLLECTIONS_PATH environment variable, as the cached directory already contains the collections/ansible_collections directory:

$ cd roles/test
$ ansible-lint -v
INFO     Using ~/.cache/ansible-lint/b9d075/roles/test symlink to current repository in order to enable Ansible to find the role using its expected full name.
INFO     Added ANSIBLE_COLLECTIONS_PATH=~/.cache/ansible-lint/b9d075/collections
INFO     Added ANSIBLE_ROLES_PATH=~/.ansible/roles:/usr/share/ansible/roles:/etc/ansible/roles:~/.cache/ansible-lint/b9d075/roles

After removing the cache, running from a diferent directory does not treat the repository as a collection one:

$ rm -rf ~/.cache/ansible-lint/b9d075

$ cd roles/test
$ ansible-lint -v
INFO     Using ~/.cache/ansible-lint/b9d075/roles/test symlink to current repository in order to enable Ansible to find the role using its expected full name.
INFO     Added ANSIBLE_ROLES_PATH=~/.ansible/roles:/usr/share/ansible/roles:/etc/ansible/roles:~/.cache/ansible-lint/b9d075/roles:~/.cache/ansible-lint/b9d075/roles
Desired Behaviour

Behaviour should be deterministic instead of depending on execution path and cache contents. It should determine the repository type based on where the .ansible-lint file is located and not on the current directory.

Actual Behaviour

Follow the steps to reproduce. Current working directory and previous executions affect the environment of the prerun process.

@xabinapal xabinapal added bug new Triage required labels Jul 10, 2021
xabinapal added a commit to xabinapal/ansible-lint that referenced this issue Jul 10, 2021
xabinapal added a commit to xabinapal/ansible-lint that referenced this issue Jul 10, 2021
@ssbarnea
Copy link
Member

That is indeed a bug and you explained it well: as ansible-lint uses the concept of project-dir, the same should happen for the prerun phase. In few cases, like missing git root, that may happen to be determined by cwd, but that is not what you described.

@xabinapal
Copy link
Contributor Author

xabinapal commented Jul 12, 2021

So, if I have understood it correctly, the directory choice for the project_dir should be, in order:

  • --project-dir CLI variable
  • git repository root
  • .ansible-lint location
  • current working directory

I'm going to submit a PR with the required prerun changes to use the project_dir variable. Then, if you like, I can also check the guess_project_dir() function in another PR, as currently it only checks for the git repository root and if not present, it reverts to the user's home directory, not the CWD: https://github.com/ansible-community/ansible-lint/blob/master/src/ansiblelint/file_utils.py#L254-L271

@ssbarnea
Copy link
Member

You described the order it perfectly.

xabinapal added a commit to xabinapal/ansible-lint that referenced this issue Jul 12, 2021
xabinapal added a commit to xabinapal/ansible-lint that referenced this issue Jul 12, 2021
@ssbarnea
Copy link
Member

I think this was fixed by #1661 - please let me know if that is not true so we can reopen it.

@ssbarnea ssbarnea removed the new Triage required label Jan 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants