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

yamllint config not extending correctly #1777

Closed
jantari opened this issue Dec 7, 2021 · 4 comments
Closed

yamllint config not extending correctly #1777

jantari opened this issue Dec 7, 2021 · 4 comments
Labels
documentation This issue or PR is related to the docs good first issue Bugs ideal for the first time, good for newcomers contributors

Comments

@jantari
Copy link

jantari commented Dec 7, 2021

Summary

I want to be able to configure some of yamllints rules when running ansible-lint
with a custom yamllint configuration file that is merged with ansible-lints built-in yamllint configuration.

From #1344 I gather this should be possible.

However some settings I did not extend/overwrite are different when a custom yamllint config file
is present vs when there is none. See "Steps to reproduce" for details.

Issue Type
  • Bug Report
Ansible and Ansible Lint details
# ansible --version
ansible [core 2.11.2]
  config file = /home/jantari/gitlab/platforms/fortigates-new/ansible.cfg
  configured module search path = ['/home/jantari/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /home/jantari/.local/lib/python3.8/site-packages/ansible
  ansible collection location = /home/jantari/.ansible/collections:/usr/share/ansible/collections
  executable location = /home/jantari/.local/bin/ansible
  python version = 3.8.10 (default, Sep 28 2021, 16:10:42) [GCC 9.3.0]
  jinja version = 3.0.1
  libyaml = True

# ansible-lint --version
ansible-lint 5.3.0 using ansible 2.11.2
  • ansible installation method: pip
  • ansible-lint installation method: pip
OS / ENVIRONMENT

Ubuntu 20.04

STEPS TO REPRODUCE

I have a repository where, without any ansible-lint or yamllint config done,
I get back these results from ansible-lint:

Finished with 110 failure(s), 0 warning(s) on 66 files.

all 110 failures are from "yaml". I now want to relax some of yamllints rules to align with our style requirements,
so I added the following file to the root of the repository as .yamllint.yml:

---

extends: relaxed

rules:
  comments:
    min-spaces-from-content: 1

( I tried both extends: default and extends: relaxed but similar results )

I get the message Loading custom .yamllint.yml config file, this extends our internal yamllint config.
so I know my config was extended / merged. However, despite my custom config being a relaxation of the rule,
I now get back:

Finished with 234 failure(s), 0 warning(s) on 66 files.

When looking at the results, this is mostly because of the line-length linting rule. Before my custom yamllint config was used, I only had a few results that looked like this - the limit was set to 160 characters:

yaml: line too long (177 > 160 characters) (line-length)

However when my custom yamllint config is included, the limit is reduced to just 80 characters even though I did not do that in my config:

yaml: line too long (89 > 80 characters) (line-length)

which leads to all these additional linter errors.

Desired Behaviour

When a custom yamllint configuration file is present that extends the default config then only the settings that are actually
in yhe configuration file should be changed. The line-length rule should not be affected.

Actual Behaviour

Minimum complete verifiable example:

playbook

Save the following file as problematic-yaml.yml:

---

one_space_from_comment: value # comment
two_space_from_comment: value  # comment

over_80_chars: 1234567890123456789012345678901234567890123456789012345678901234567890
over_160_chars: 123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890

yamllint config

Save the following file as .yamllint.yml:

---

extends: default

rules:
  comments:
    min-spaces-from-content: 1

To reproduce

Run ansible-lint in this directory with the .yamllint.yml file present and named as such and then rename the fole (for example to .yamllint.yml_disabled) and run ansible-lint again.

WITHOUT the yamllint config file being loaded the output is:

WARNING  Listing 2 violation(s) that are fatal
yaml: too few spaces before comment (comments)
problematic-yaml.yml:3

yaml: line too long (196 > 160 characters) (line-length)
problematic-yaml.yml:7

You can skip specific rules or tags by adding them to your configuration file:
# .ansible-lint
warn_list:  # or 'skip_list' to silence them completely
  - yaml  # Violations reported by yamllint

Finished with 2 failure(s), 0 warning(s) on 1 files.

WITH the yamllint config file present and loaded the output is:

Loading custom .yamllint.yml config file, this extends our internal yamllint config.
WARNING  Listing 2 violation(s) that are fatal
yaml: line too long (85 > 80 characters) (line-length)
problematic-yaml.yml:6

yaml: line too long (196 > 80 characters) (line-length)
problematic-yaml.yml:7

You can skip specific rules or tags by adding them to your configuration file:
# .ansible-lint
warn_list:  # or 'skip_list' to silence them completely
  - yaml  # Violations reported by yamllint

Finished with 2 failure(s), 0 warning(s) on 1 files.

even though the line-length rule was not touched.

@jantari jantari added bug new Triage required labels Dec 7, 2021
@priyamsahoo priyamsahoo removed the new Triage required label Dec 13, 2021
@webknjaz webknjaz added documentation This issue or PR is related to the docs and removed bug labels Dec 13, 2021
@webknjaz
Copy link
Member

Hi, the reason this behavior is observed is that our internal config is only used when you don't have your own file (it can be found https://github.com/ansible-community/ansible-lint/blob/8c372b6/src/ansiblelint/rules/YamllintRule.py#L28-L38).

When you define your own config and base it on the relaxed, you use the baseline that is coming from yamllint itself, not us. If you want the same modifications, you'll need to introduce them in your config too.

Action item: make this clearer in the docs @ https://ansible-lint.rtfd.io/en/latest/default_rules.html#yaml.

@webknjaz webknjaz added the good first issue Bugs ideal for the first time, good for newcomers contributors label Dec 13, 2021
@jantari
Copy link
Author

jantari commented Jan 8, 2022

I figured out shortly after that the "extend ansible-lints internal yamllint config with mine" behavior I wanted does work when I just remove the extends: directive entirely from my .yamllint.yml.

So this does not appear to be true:

Hi, the reason this behavior is observed is that our internal config is only used when you don't have your own file

However that probably should be clarified because currently the ansible-lint docs kind of just tell you to go look at the yamllint docs for "yaml"-rule related config and the yamllint docs in turn say that if you want to extend the default config you must use the extends: setting. But it appears that's not true in the specific scenario of using yamllint from inside ansible-lint.

I think it would be easiest and most user-friendly if there was an additional config added to yamllint that's called "ansible-lint".

That way we can just say extends: ansible-lint (or ofc extends: something-else if one would want that). Not specifying any extends: value would then just use yamllints default settings as is documented by them and therefore the expected behavior.

@ssbarnea
Copy link
Member

@jantari I think that this counts as a bug. I would be more than happy to review a PR that is fixing it. I never used the extend feature myself and I relied on yamllint library behavior, which seems to be different than the command line tool for this particular case.

@ssbarnea
Copy link
Member

I am going to close this because the core team has no plans to address it. Still, if someone would make a pull request, we will likely incorporate it. Maybe it would be a good idea to make a pull request to document the fact that we do not support the extends feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation This issue or PR is related to the docs good first issue Bugs ideal for the first time, good for newcomers contributors
Projects
None yet
Development

No branches or pull requests

4 participants