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 Lint fails to recognize requirement files within the Molecule directory #1369

Closed
alessfg opened this issue Feb 17, 2021 · 8 comments · Fixed by #1371 or #1374
Closed

Ansible Lint fails to recognize requirement files within the Molecule directory #1369

alessfg opened this issue Feb 17, 2021 · 8 comments · Fixed by #1371 or #1374
Labels

Comments

@alessfg
Copy link
Contributor

alessfg commented Feb 17, 2021

Summary

Ansible Lint fails to recognize requirement files within the Molecule directory.

Issue Type
  • Bug Report
Ansible and Ansible Lint details
ansible --version
ansible 2.10.5
  config file = /Users/faelgarcia/.ansible.cfg
  configured module search path = ['/Users/faelgarcia/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/local/lib/python3.9/site-packages/ansible
  executable location = /usr/local/bin/ansible
  python version = 3.9.1 (default, Feb  3 2021, 07:04:15) [Clang 12.0.0 (clang-1200.0.32.29)]
ansible-lint --version
ansible-lint 5.0.1
  • ansible installation method: pip
  • ansible-lint installation method: pip
OS / ENVIRONMENT

Bug can be reproduced in both MacOS environments and Ubuntu 20.04.

STEPS TO REPRODUCE
  1. Clone or download nginx_config role from Ansible Galaxy (https://github.com/nginxinc/ansible-role-nginx-config)
  2. Run ansible-lint
  3. Ansible Lint reports an error (due to a failed ansible-playbook --syntax-check).

See https://github.com/nginxinc/ansible-role-nginx-config/pull/75/checks?check_run_id=1912453583#step:7:34 for a concrete example.

Desired Behaviour

Ansible Lint should detect that there is a requirements file within the Molecule directory and treat it as such. There might be some use cases where some roles are only required within a Molecule workflow and as such it makes more sense to contain the relevant requirement files within the Molecule directory. Furthermore, depending on the test case, different requirement files might make sense. Ideally, I'd like to see some regex matching for requirement files not unlike the regex matching that's available for playbooks in https://github.com/ansible-community/ansible-lint/blob/master/src/ansiblelint/config.py#L19.

Actual Behaviour
  The error appears to be in '/home/runner/work/ansible-role-nginx-config/ansible-role-nginx-config/molecule/common/playbooks/oss_requirements.yml': line 2, column 1, but may
  be elsewhere in the file depending on the exact syntax problem.
  
  The offending line appears to be:
  
  ---
  roles:
  ^ here

Ansible Lint fails to recognize that there might be some requirements files contained within the Molecule directory. My best guess it that this has to do with the file mappings detailed in https://github.com/ansible-community/ansible-lint/blob/master/src/ansiblelint/config.py#L15-L29.

@alessfg alessfg added the bug label Feb 17, 2021
@ssbarnea
Copy link
Member

The only supported filename for requirements is requirements.yml and at this moment anything else inside the scenario folder that is not named molecule.yml would be assumed to be a playbook.

You will likely have to rename the file and/or move it into a folder like files/ to avoid an error.

You pointed to the correct source code location that determines the file types, but in this case it does not look like a bug in our patterns and more of a local ambiguity.

I will likely update molecule docs as I find the example with collections.yml misleading. I am included to drop support for old requirements.yml (v1 version and only support v2, which allows both roles and collections inside) but I am not sure if that works with 2.8. If it does not, i will postpone until we drop support for 2.8

@fgierlinger
Copy link

The only supported filename for requirements is requirements.yml and at this moment anything else inside the scenario folder that is not named molecule.yml would be assumed to be a playbook.

The same error occurs when the file is named requirements.yml.

The error appears to be in '/fgierlinger.docker_swarm/molecule/default/requirements.yml': line 2, column 3, but may
be elsewhere in the file depending on the exact syntax problem.

The offending line appears to be:

---
- src: geerlingguy.docker
  ^ here

You will likely have to rename the file and/or move it into a folder like files/ to avoid an error.

I strongly disagree with this suggestion. When using the dependency manager of galaxy it looks per default for a file called requirements.yml (for roles) and collections.yml (for collections) inside the scenario directory. Moving the requirements.yml file into a folder like files/ would break the configuration of all molecule deployments. https://molecule.readthedocs.io/en/latest/configuration.html#dependency

@ssbarnea
Copy link
Member

I will make a patch to recognize collections.yml files, but I am not going to support any filenames other than these two hardcoded ones.

Keep in mind that neither molecule nor ansible-lint officially support for Ansible 2.8 and these are needed only for 2.8, a 2.9+ can make use of newer combined format. I do expect to see both making presence of ansible 2.8 an error or at least a big warning saying that no bugs related to 2.8 will be addressed.

ssbarnea added a commit that referenced this issue Feb 18, 2021
@boutetnico
Copy link

boutetnico commented Feb 18, 2021

[...] anything else inside the scenario folder that is not named molecule.yml would be assumed to be a playbook.

I believe this breaks the case where you have multiple molecule scenarios that extend a shared scenario such as in this example from molecule doc: molecule --base-config base.yml check.

base.yml will be considered as a playbook whereas it should be treated as a molecule.yml file.

Example repo with multiple molecule scenarios that worked until ansible-lint 5.0.0: https://github.com/boutetnico/ansible-role-pmm-server/tree/master/molecule.

@fgierlinger
Copy link

fgierlinger commented Feb 18, 2021

Keep in mind that neither molecule nor ansible-lint officially support for Ansible 2.8 and these are needed only for 2.8, a 2.9+ can make use of newer combined format.

Molecule does support the two latest stable ansible releases which are as of today ansible 2.8 and ansible 2.9. From the latest molecule documentation: "Molecule supports only the latest two major versions of Ansible (N/N-1), meaning that if the latest version is 2.9.x, we will also test our code with 2.8.x."

@ssbarnea
Copy link
Member

ssbarnea commented Feb 18, 2021

@fgierlinger I guess I will have to add a check sooner, read https://pypi.org/project/ansible/ --- 2.10 was released in September 2020 ! -- I don't care what is the latest version shipped with a platfort distribution, ansible core project is 2.10, and the community version 3.0 already as a rc. Keep in mind that 2.8 happens to mostly work with molecule, but the tool does not offer any guarantees and any bug mentioning it will be closed right away.

@boutetnico Thanks for mentioning base.yml, another special case to add for improving detection via #1374

@alessfg
Copy link
Contributor Author

alessfg commented Feb 18, 2021

As far as I know I am using the v2 requirements format whenever I use a requirements file (see https://github.com/nginxinc/ansible-role-nginx-config/blob/main/molecule/common/playbooks/oss_requirements.yml). That being said, I am very much against throwing a requirements file that is only used when running Molecule tests (it's not a real dependency) anywhere outside the Molecule directory.

If that's no longer going to be allowed by Ansible Lint as valid requirements location, I can probably work around it (e.g. creating a requirements folder inside a nested Molecule directory seems to work -- molecule/common/requirements/). However, in turn, I would maybe suggest updating the Molecule docs around Galaxy dependencies to clarify this change/(fix?) in behaviour? The current role-file parameter makes it look as if any path is valid.

@fgierlinger
Copy link

@fgierlinger I guess I will have to add a check sooner, read https://pypi.org/project/ansible/ --- 2.10 was released in September 2020 !
@ssbarnea I am aware of the releases of ansible and in my comment I did not deny that 2.10 has been released. It was about the support status of the releases. I think we should take care not to get confused between the ansible and the ansible-base support.

Currently maintained are:

  • ansible version 2.8 and 2.9
  • ansible-base version 2.10

https://docs.ansible.com/ansible/devel/reference_appendices/release_and_maintenance.html

Be aware (but you probably are) that ansible and ansible-base differ in version numbering (e.g. ansible-2.9 must not include ansible-base-2.9).

When releasing ansible-lint-5.0.0 you did choose to make a major version upgrade, which leads me to the assumption that there is some incompability to the previous versions. Just make sure that if you drop support for ansible<2.9 you make a clear statement of the supported ansible versions.

Further, as pointed out already by other people, there has not been any official statement from ansible, that standalone roles, nor the old requirements.yml format is deprecated or will get deprecated soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants