Announcing ansible-lint

Archives Ansible Project
1

Hi all,
One of the things that the alternatives have that ansible doesn’t is a static analysis tool such as puppet-lint or food-critic.

To rectify this I’ve created ansible-lint, which is an extensible rules-based static analysis tool. It currently handles all the kinds of playbook inclusion mechanisms that I’m aware of (playbook include, task/handler include, roles, role dependencies).

At the moment it has a few obvious rules but am open to new ideas. If there’s a burning desire to merge this into ansible core it shouldn’t be too hard. I’ve tried to reuse ansible libraries where possible.

https://github.com/willthames/ansible-lint

Pull requests (with tests) and bug reports are welcome via the github repo, it’s in its infancy.

Will

2

I generally view the idea that a program needing a linter is a sign of things in the program should be made better instead.

I’ve got a reasonably long-standing ticket to pre-validate the YAML for things like “xyz: {{ unquoted }}” and report on them, though I think that should be done exclusively at the YAML syntax level.

Old-style variables can be disabled in ansible.cfg, and I’d be fine with adding a mode that just makes them a warning.

But that’s just a couple of things – more sign it doesn’t really need a linter IMHO.

3

A linter allows the checking of rules that don’t necessarily need to be rigidly enforced but should at least throw up warnings to novice users.

Having an extensible set of rules allows people to customise things to their own environment.

ansible-lint provides a warning when people use ‘command cp’ or ‘shell ln’ - this also acts as a way to notify users that there are better ways of doing things, even though you might not necessarily want to prevent them (maybe there are things that do need to execute rpm in a shell, even though typically using the yum module is better practice)

What about enforcing repeatability - some people might want to ensure that git or hg checkouts always reference a specific commit or tag rather than just checking out HEAD. Some people won’t care though.

Will

4

Yeah, I know what a linter is, I disagree with need for it, it feels like solving a sprained ankle with an airplane.

Basically this – 95% of our users aren’t software developers, I don’t WANT them to worry about the noise of thinking about these kind of things, about how to write and extend their own rules, etc.
Thus if we reply with “I have a problem, hey, go use this linter extra thing” they have a really bad experience and get the totally wrong impression about whether something is simple or easy or not, and now have all these extra things to juggle in their heads.

So, I’m mostly replying to the idea about whether this fits in core (I believe it doesn’t) and should be be advocating this to users of Ansible (we shouldn’t).

Rather, we should concentrate on improving the things that are hard to use with good, clean, sensible defaults.

Old style variables, when deprecated, will cause deprecation warnings – right now, that’s not done.

I don’t mean to imply what you did was not a good piece of work, I’m just explaining my views on idea of applying it to core (OR) recommending it to users here in future threads.

So how I’d really like to refocus the conversation is, how can we improve stock Ansible to detect things that don’t result in clear errors and make them produce clearer errors – and in what cases should it prescan YAML for obvious problems. I know of a few because they are mentioned on the YAML syntax page.

The other thing I want to do is have errors report what line of YAML they were executing when they hit the error, which is something we can only do at runtime, but would be absolutely great to have.

5

Also, I should point out the amount of infrastructure I see duct-taped on top of Puppet and Chef (look at Etsy) is a sign good things don’t happen in the core, and that the community doesn’t work together to build things into core.

Contribution rates seem to support this.

This is something that I am REALLY not wanting in Ansible – it should be inclusive of all contributors – so that’s why I don’t particularly want to see things duct-taped on the side, especially things that increase complexity towards following the “infrastructure as code” track which I believe is fundamentally broken.

6

(The stuff about users who want to look for git without a tag, that seems ok-ish, I just want to avoid the idea that we shouldn’t work on perceived confusing things in core because we can code-scan things and tell people to fix them).

7

My belief is that in general having something easy to configure that notifies people of best practices and where they’re not meeting them is a sound idea. I saw one of your code review emails the other day that was quite lengthy and if we could automate some of that, it is worth doing.

Would it be better if we called it ‘best practices scanner’? I don’t think you’re really disagreeing with the principles here, just that it’s a separate tool and you don’t want confusing messages or that it be switched on by default.

There’s a difference between configuration that is genuinely in error and stuff that’s just a bit dodgy and could be done better. (The mismatched bracket rule was added after I spent hours in the debugger trying to work out why a variable wasn’t being substituted - in hindsight turning on errors on substitution failure would have helped!). I would like to help new users of configuration management be aware of where there is scope for improvement, but definitely not turn off new users.

Will

8

Nope, I’m disagreeing with my approach of making this in a seperate tool at all.

We upgrade ansible where needed, we don’t bolt something on the side.

The bracket rule is something the main application should let you know about, because it can.

9

We can just call this a proof of concept if you like. It doesn’t need to be a separate tool.

Do you want me to integrate the idea into core? If you do we can come up with some agreement of how that should work (what flags it should take, what directories the rules should live in etc.)

Otherwise it will just continue to live on the side as I at least will find it useful.

Will

10

We should definitely figure out what that would look like, though I also would want to wait just a little while as 1.3 is going out in a week and this would be a reasonably large addition when we’re trying to polish up and test a few remaining things.

But yes, definitely, core should yell about these sort of things and be able to.

Definitely don’t want flags, but config options, yes, and some of them should be on by default IMHO.

11

Offhand, I’m thinking out loud.

[warnings]
no_pre_jinja2_variables=error|yes|no # default error
jinja2_line_quotes=error|yes|no #default error
colon_quotes=error|yes|no # default error
hash_key_reuse=error|yes|no # default error

question is what else should be there…

12

include_with_items? … i feel i just shot myself in the foot

13

Might as well.

14

To add my 2cents this will be extremely useful for us where we have our plays and roles open-sourced and many people contributing. Adding a lint to a jenkins task for pre-approving pulls will save us time, will definitely check it out.
I definitely want to create a syntastic checker for this too.

15

one that I see too often in IRC and mailing list

variable_with_dashes=

16

@bcoca +1

17

BTW, one concern I have about implementation is that it should be welded to the main code as much as possible.

This is to say if we add a new way that does an import of a YAML file, i.e. “doctor_seuss_import” (not going to be, just an example) it should just run from the utils function where possible (import YAML from file) at runtime (and of course still reporting line numbers).

This means it will need to know a little context about what kind of file it is importing in a few cases, but I don’t want the case that we have now with things like --list-tags where it’s mostly a simulation of playbook code and can (if we are not careful) diverge ever so slightly.

18

I think the whole "Infrastructure as Code" vs "Infrastructure as Data" has gone to extreams of religiouse proportions...
Ansible is not purely declerative, it already is a DSL on it's own - YAML is just a file-format - ansible is already interpreting it in imperative ways via imperative semantics.
A purely-declerative semantics would have been much too restrictive, especially in terms of reuse.
A linter is semantics-analyser, not limited to purely-imperative semantics, nor should it be viewed as such. The association of a linter to imperative semantics is errorneouse, every language can (and should) have a linter - including ansible's dsl.
If you only cater to newcomers, at the expense of advanced users, you would end-up losing the latter to justify the former - which would result in ansible being used by inexperienced users. Once they would get passed that, they would get stuck and forced to immigrate again to something else...

19

I’m hardly not catering to advanced users, rather, simplicity benefits everyone. I myself am a very advanced user, and the need for a tool that didn’t monopolize my time was the reason I started this project.

And yes, I care about this quite a lot, and this is why Ansible has been so successful at building up such a large user base.

20

I just don’t agree with the assertion that a linter is “added complexity” - if anything, it’s the other-way around - if it checks your syntax, that is one thing “less” you need to worry about, and if it checks your semantics and suggest better ways of doing things, it would thus most of the times actually get you to make your code “simpler”… So in both cases, I can’t see how a linter is a move in the increase-of-complexity direction.
And in both cases it could cater advanced-users and newcomers alike.