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

It lints excluded paths in 6.16.2 #3477

Closed
ElieDeloumeau opened this issue May 22, 2023 · 22 comments · Fixed by #3507
Closed

It lints excluded paths in 6.16.2 #3477

ElieDeloumeau opened this issue May 22, 2023 · 22 comments · Fixed by #3507
Assignees
Labels

Comments

@ElieDeloumeau
Copy link

Summary

ansible-lint is linting excluded paths since 6.16.2

Issue Type
  • Bug Report
OS / ENVIRONMENT
ansible-lint --version
ansible-lint 6.16.2 using ansible-core:2.15.0 ruamel-yaml:0.17.26 ruamel-yaml-clib:0.2.7
  • ansible installation method: pip
  • ansible-lint installation method: pip
STEPS TO REPRODUCE
  • Install collections in project/collections (awx.awx 19.2.2 in this case)
  • Exclude paths in .ansible-lint
    • exclude_paths:
        - project/collections/
      
  • Run ansible-lint and see it lint the collections

With ansible-lint 6.16.1 (normal)

$ ansible-lint --force-color --offline
[...]
yaml[line-length]: Line too long (161 > 160 characters)
project/vars/awx/org_admin.yml:30
                Rule Violation Summary
 count tag               profile rule associated tags
     1 yaml[line-length] basic   formatting, yaml

Failed after min profile: 1 failure(s), 0 warning(s) on 14 files.

With ansible-lint 6.16.2 (bug)

$ ansible-lint --force-color --offline
[...]
risky-file-permissions: File permissions unset or incorrect. (warning)
project/collections/ansible_collections/awx/awx/tools/template_galaxy.yml:36 Task/Handler: Template the README.md source file (should be commited with your changes)

yaml[line-length]: Line too long (161 > 160 characters)
project/vars/awx/org_admin.yml:30

Read documentation for instructions on how to ignore specific rule violations.

                           Rule Violation Summary
 count tag                        profile    rule associated tags
     1 command-instead-of-module  basic      command-shell, idiom
     2 command-instead-of-shell   basic      command-shell, idiom
     2 deprecated-local-action    basic      deprecations
     1 key-order[task]            basic      formatting
     3 jinja[invalid]             basic      formatting
    18 jinja[spacing]             basic      formatting
   328 name[missing]              basic      idiom
     1 var-naming[no-reserved]    basic      idiom
    10 var-naming[no-role-prefix] basic      idiom
     7 yaml[line-length]          basic      formatting, yaml
     1 name[template]             moderate   idiom
     8 risky-file-permissions     safety     unpredictability (warning)
     2 no-changed-when            shared     command-shell, idempotency
   450 fqcn[action-core]          production formatting (warning)
    28 args[module]                          syntax, experimental (warning)

Failed after min profile: 376 failure(s), 486 warning(s) on 172 files.
Desired Behavior

Excluded paths should be excluded from linting

Actual Behavior

Excluded paths are linted

@ElieDeloumeau ElieDeloumeau added bug new Triage required labels May 22, 2023
@kpfleming
Copy link

Same behavior here; reverting to 6.16.1 restores the proper behavior.

@chrisx8
Copy link

chrisx8 commented May 22, 2023

Same here on 6.16.2, both --exclude and exclude_paths have no effect. Works as expected on 6.16.1 and earlier.

@robert914
Copy link

Same behavior here; reverting to 6.16.1 restores the proper behavior.

@OlivierCloudar
Copy link

I can confirm the same behaviour

@cayla
Copy link

cayla commented May 22, 2023

I wonder if https://github.com/ansible/ansible-lint/pull/3468/files is related. That was an obvious change in 6.16.2

PS - You can just thumbsup the original post if you are affected. You don't have to say 'me too'.

@chrisx8
Copy link

chrisx8 commented May 23, 2023

Reverting #3468 (commit ab18405) solved the issue

@ssbarnea
Copy link
Member

Can one of you mention how you are calling ansible-lint? I have the impression that you might be calling it from an incorrect pre-commit config, that is because by default pre-commit might attempt to pass each file as an argument to the tool, and in this case the excludes will not longer work.

Please help us reproduce it, and so we can make a hotfix.

The only case where excludes will be ignored is if you pass the file as an argument. For example ansible-lint **/*.yml will ignore all excludes because the shell will expand the glob pattern and each file will be and explicit argument.

Still, without arguments or with something like ansible-lint --exclude-path=foo/var foo it should work as expected because only foo directory would be explicit but its children should not be explicit, so the exclude should apply to it.

@ssbarnea ssbarnea self-assigned this May 23, 2023
@OlivierCloudar
Copy link

I am not using it in pre-commit, but similar to the first post:

  • a .ansible-lint file with the exclude_paths directive containing a list of paths to ignore
  • calling the executable directly with ansible-lint without any extra arguments

This works if I use v6.16.1 but stopped working since the release of v6.16.2

@Dual-0
Copy link

Dual-0 commented May 23, 2023

I exclude it via .ansible-lint config file.

exclude_paths:
  - .cache/ # implicit unless exclude_paths is defined in config
  - .git/
  - roles/monitoring/files/alerts.d/

exec:

user@instance:~/ansible$ ansible-lint
WARNING  Listing 30 violation(s) that are fatal
yaml[trailing-spaces]: Trailing spaces
roles/monitoring/files/alerts.d/dl.yml:5

yaml[line-length]: Line too long (178 > 160 characters)
roles/monitoring/files/alerts.d/dl.yml:6

@ssbarnea

This comment was marked as outdated.

@ssbarnea

This comment was marked as outdated.

@ssbarnea ssbarnea removed the new Triage required label May 23, 2023
@kpfleming
Copy link

The folder paths in my .ansible-lint file do not have trailing slashes, but they are still being included in the scan using 6.16.2.

%YAML 1.2
---
exclude_paths:
  - .cache
  - requirements.yml
  - roles/willshersystems.sshd
  - venv
  - collections
  - factcache
  - playbooks/test.yml

skip_list:
  - latest[git]
  - meta-no-info
  - name[casing]
  - no-changed-when
  - no-handler
  - package-latest
  - risky-shell-pipe
  - template-instead-of-copy
  - unnamed-task
  - var-naming[no-jinja]
  - var-naming[no-role-prefix]
  - var-spacing
  - yaml[line-length]

write_list:
  - all

kinds:
  - tasks: "**/imports/*.yml"

ssbarnea added a commit that referenced this issue May 23, 2023
@ssbarnea
Copy link
Member

If one of you can confirm if #3487 is fixing the issue it would be great. I will wait for at least one confirmation before getting the fix in and making a new release.

@OlivierCloudar
Copy link

OlivierCloudar commented May 23, 2023

Unfortunately I still get the same error with the fix, see below.

Version:

a8dfed8d3bcb:/repo# ansible-lint --version
ansible-lint 6.16.3.dev9 using ansible-core:2.15.0 ruamel-yaml:0.17.26 ruamel-yaml-clib:0.2.7
You are using a pre-release version of ansible-lint.

.ansible-lint has no trailing /:

exclude_paths:
  - .releaserc.yaml
  - buildspec-build.yaml
  - .pre-commit-config.yaml
  - node_modules/
  - files/espconfig
  - files/hass
  - .github

Run:

a8dfed8d3bcb:/repo# ansible-lint
WARNING  Listing 17 violation(s) that are fatal
load-failure[runtimeerror]: Failed to load YAML file
files/espconfig/ep-one-basement.yaml:1 could not determine a constructor for the tag '!secret'
  in "<unicode string>", line 12, column 9

...

load-failure[runtimeerror]: Failed to load YAML file
files/hass/blueprints/automation/notify-appliance-start-stop.yaml:1 could not determine a constructor for the tag '!input'
  in "<unicode string>", line 73, column 16


                    Rule Violation Summary
 count tag                        profile rule associated tags
    17 load-failure[runtimeerror] min     core, unskippable

Failed after : 17 failure(s), 0 warning(s) on 56 files.

Edit: Ok, I admit that there's a / after the node_modules 😄 but I just tested with removing that and still the same result. Also none of the errors were in the node_modules anyway.

@chrisx8
Copy link

chrisx8 commented May 24, 2023

I'm also still having the same issue with #3487.

See below (there is no .ansible-lint file):

$ ansible-lint --version
ansible-lint 6.16.3.dev9 using ansible-core:2.15.0 ruamel-yaml:0.17.26 ruamel-yaml-clib:0.2.7
You are using a pre-release version of ansible-lint.

$ ansible-lint --exclude roles/tailscale
WARNING  Listing 1 violation(s) that are fatal
fqcn[action-core]: Use FQCN for builtin module actions (lineinfile).
roles/tailscale/tasks/main.yml:16 Use `ansible.builtin.lineinfile` or `ansible.legacy.lineinfile` instead.

Read documentation for instructions on how to ignore specific rule violations.

                 Rule Violation Summary
 count tag               profile    rule associated tags
     1 fqcn[action-core] production formatting

Failed after shared profile, 4/5 star rating: 1 failure(s), 0 warning(s) on 135 files.
You are using a pre-release version of ansible-lint.

Note that a violation is found at roles/tailscale/tasks/main.yml:16, even if it should have been excluded.

osfrickler added a commit to osism/zuul-jobs that referenced this issue May 24, 2023
ansible-lint version 6.16.2 is broken, ignoring exclude_path
definitions[0]. Don't use it.

[0] ansible/ansible-lint#3477

Signed-off-by: Dr. Jens Harbott <[email protected]>
osfrickler added a commit to osism/zuul-jobs that referenced this issue May 24, 2023
ansible-lint version 6.16.2 is broken, ignoring exclude_path
definitions[0]. Don't use it.

[0] ansible/ansible-lint#3477

Signed-off-by: Dr. Jens Harbott <[email protected]>
@ssbarnea
Copy link
Member

Please point me to some repository where I can reproduce the bug, posting console dumps is not always enough for reproducing a bug. Quite often some other settings are triggering the bug.

@osfrickler
Copy link

Please point me to some repository where I can reproduce the bug, posting console dumps is not always enough for reproducing a bug. Quite often some other settings are triggering the bug.

https://github.com/osism/ansible-playbooks/ is where I first discovered this.

ssbarnea added a commit that referenced this issue May 24, 2023
@ssbarnea
Copy link
Member

I was able to discover that wcmatch.wcmatch.WcMatch fails to filter out excluded files, basically the exclude_pattern does not seem to work.

I am not trying to get rid of the need to use wcmatch as we already have an utility function that is expected to test if a file path is excluded or not.

ssbarnea added a commit that referenced this issue May 25, 2023
@ssbarnea
Copy link
Member

I just pushed a rewrite of detection location that drops use of wcmatch, please test #3487 and report the outcomes.

This impacts case where linter is called without any positional arguments in locations that are not git repos.

@chrisx8
Copy link

chrisx8 commented May 25, 2023

#3487 fixed the issue for me. Thanks @ssbarnea!

@osfrickler
Copy link

Works for me, too. Note that in my case the invocation is indeed without pos args, but in the root of a git repo.

Also while testing I found that despite having e.g. .tox in exclude_paths, there is a huge performance regression if there is some content there, but this isn't a new regression, happening at least also with 6.16.1, I'll create a separate issue.

ssbarnea added a commit that referenced this issue May 25, 2023
@hille721
Copy link
Contributor

@ssbarnea : I can confirm that the fix will work for me as well

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