ansible / ansible-lint

ansible-lint checks playbooks for practices and behavior that could potentially be improved and can fix some of the most common ones for you
https://ansible.readthedocs.io/projects/lint/
GNU General Public License v3.0
3.46k stars 656 forks source link

Config's kinds patterns do not always work #1774

Closed sblask closed 2 years ago

sblask commented 2 years ago
Summary

https://ansible-lint.readthedocs.io/en/latest/configuring.html describes kinds and the patterns look like glob patterns, presumably relative to the directory of the config files. However something like test.yml does not work and a pattern that usually works does not work when the ansible file is read from stdin.

Issue Type
Ansible and Ansible Lint details
~/Desktop/test $ ansible-lint --version
ansible-lint 5.3.0 using ansible 2.12.0

~/Desktop/test $ ansible --version
ansible [core 2.12.0]
  config file = None
  configured module search path = ['/Users/sebastian/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /Users/sebastian/.opt/ansible-lint/lib/python3.9/site-packages/ansible
  ansible collection location = /Users/sebastian/.ansible/collections:/usr/share/ansible/collections
  executable location = /Users/sebastian/.bin/ansible
  python version = 3.9.4 (default, Apr 29 2021, 10:26:40) [Clang 12.0.5 (clang-1205.0.22.9)]
  jinja version = 3.0.3
  libyaml = True
OS / ENVIRONMENT

MacOS

STEPS TO REPRODUCE

With an ansible file like this:

- name: foo
  git:
    repo: https://github.com/ansible-community/ansible-lint
    dest: bar

And a config like this:

skip_list:
  - git-latest
kinds:
  - tasks: "test.yml"

ansible-lint -c .ansible-lint test.yml gives this error: syntax-check: 'git' is not a valid attribute for a Play

When I change the config to:

skip_list:
  - git-latest
kinds:
  - tasks: "**/test.yml"

ansible-lint -c .ansible-lint test.yml runs without error, however ansible-lint -c .ansible-lint - < test.yml does give syntax-check: 'git' is not a valid attribute for a Play as well.

Expected behaviour is that none of the commands gives an error.

dsoares commented 2 years ago

I had a similar issue with the following directory structure, where *.yml files in root directory are playbooks:

ansible
    01-setup.yml
    ...
    09-deploy.yml
    templates/...
    handlers/...
    vars/...

I wanted to instruct ansible-lint that *.yml files in current directory are playbooks:

kinds:
  - playbook: "*.{yml,yaml}"

Since kind_from_path() uses the absolute file path, i think that every path in ansible-lint option kinds must start with ** to match. So, we can not use paths relative to the current directory. However, defining - playbook: "**/*.yml" will match every .yml file, it does not make sense.

My temporary fix was to keep the absolute().resolve() but then use the relative resolved path:

*************** def kind_from_path(path: Path, base: bool
*** 95,100 ****
--- 95,103 ----
      # pathlib.Path.match patterns are very limited, they do not support *a*.yml
      # glob.glob supports **/foo.yml but not multiple extensions
      pathex = wcmatch.pathlib.PurePath(str(path.absolute().resolve()))
+     cwd = Path.cwd()
+     if pathex.is_relative_to(cwd):
+         pathex = pathex.relative_to(cwd)
      kinds = options.kinds if not base else BASE_KINDS
      for entry in kinds:
          for k, v in entry.items():

I can do a PR if this fix also suits other cases of ansible-lint usage.

ssbarnea commented 2 years ago

I hat you say makes sense, please raise a PR. I am curious how it affects the tests. We will need to include a test to avoid regression in the future but I will probably be able to help you with that if you need that.

ssbarnea commented 2 years ago

closing this because ansible-lint never allowed validation of "floating" tasks files. A tasks file can only be validated when it is included by a playbook. Any arguments given to ansible-lint command line ar treated as playbooks, never tasks.

sblask commented 2 years ago

@ssbarnea maybe I should have included my use case: it is to be able to be able to validate a task file during development in my editor. Is that something you don't want to support? At the moment my editor always shows an error when I edit task files. I reckon all the code to make it work is already there and it should be easy to fix if you are familiar with the codebase...

ssbarnea commented 2 years ago

The tasks file validity cannot be fully evaluated in the absence of a playbook or role that includes it.

Basically you cannot pass that file to the linter directly. Still, you can get linting results by running the linter on entire repository or at least on a playbook that includes this task.

There are more details related to why it is like this but it gets very complicated. Any attempts to fix would likely break other use cases that we found more important. Look at any editor that supports our ansible language server, there are already 4, that should give you better results.

dfelton commented 2 years ago

closing this because ansible-lint never allowed validation of "floating" tasks files.

While @sblask may have opened this pertaining to validation of a task, I'd argue that the main point of the issue here is that we have no way to specify a kinds pattern (for any type of kind) that provides the ability to strictly target files in the root directory of the project. As @dsoares has pointed out.

At my company we have chosen to have the Ansible project stored directly in /etc/ansible and have files & directories such as:

/etc/ansible/hosts
/etc/ansible/hosts_production
/etc/ansbile/site.yml
/etc/ansible/playbooks/
/etc/ansible/roles/

Without having kinds definitions definitions such as - inventory: "**/hosts*" for inventory, our inventory files get skipped entirely, and our entry point playbook site.yml gets parsed as the type yaml as opposed to playbook without the kinds entry - playbook: "**/site.yml".

While not necessarily problematic for us, my OCD knows this isn't the true ideal pattern here as we're not really desiring it look in sub-directories for these files.


I see @dsoares PR was closed due to inactivity. I'll subscribe myself to both this PR and Issue to see if I can revisit this in the future.

I encourage possibly re-opening this issue though... despite @sblask wanting to validate tasks, the main issue here (in my opinion) is the inability to use relative paths for kinds definitions and thus also no true way to explicitly target root level files without potentially catching false positives deeper into the project.

sblask commented 2 years ago

I think it's safe to say that I don't understand the point of kinds if it doesn't allow for validating tasks, why would I be able to specify tasks under kinds? Either I include it in a playbook and the kind is clear or it's part of a role and the directory structure is fixed (Or am I missing something and roles can be structured another way? That would be configured on ansible level though, not ansible-lint?) So I think firstly the purpose of kinds should be cleared up and based on that it should be made to work intuitively. Paths should either be absolute or relative to the config file (I think the PR got that wrong because PWD could potentially be anywhere?)

ursetto commented 2 years ago

My simple workaround for tagging files only in the project's root directory is to include the repo's name in the match. This won't work in all cases, but it does in the usual case where you git clone your project to a directory of the same name, as long as your project doesn't contain a directory inside it with this name.

kinds:
 - playbook: "**/my-ansible-repo/*.yml"

This helps avoid a bunch of "Overriding detected file" noise for me.

Ideally, I think this should be a relative match against the path to .ansible-lint, not necessarily the cwd.