ansible-lint icon indicating copy to clipboard operation
ansible-lint copied to clipboard

single-entry-point: identify role with possible multiple entry points

Open ssbarnea opened this issue 3 years ago • 8 comments

Valid:

  • only one main.yml file
  • all other files have _ prefix
  • all other files are in subdirectories

Invalid:

  • multiple files in tasks/

Reasonint:

We want to discourage people from writing roles that would be used with tasks_from, mainly because this is fragile and requires extra maintenance, also prone to bugs.

People should use a dispatcher pattern for implementing roles that need multiple kind of actions.

ssbarnea avatar Jul 20 '22 16:07 ssbarnea

This is another controversial rule that got cut from the CoP good practices for good reason. I'm really opposed to forcing it on users by putting it in the production profile similar in #2204. I get wanting to discourage the use of task_from to a degree, but then create a rule that looks for task_from and throw a warning.

I also have an issue with proposing python-isms like the use of a _ prefix. Ansible users and almost always not python programmers. We need to stop letting that stuff bleed through. Also, "all other files are in subdirectories" can be interpreted different ways by different developers and would need to be more specific. Doesn't matter though because this shouldn't be in here.

So I am a big corporation who's developers have written hundreds of playbooks using hundreds of roles that are tested and in production. This tool gets released with this rule in the production profile. Then a senior IT manager overseeing automation tells all their developers "nothing goes into production until it passes Ansible's production profile" as a matter of good practice. Those developers now have to refactor and re-test all of their working roles even though they aren't using task_from. Why? (#2204 has the same effect here.)

It's fine to recommend encourage this pattern, but this isn't the place to do that. Do it in a blog posts or workshops or scaffolding tools.

tima avatar Aug 02 '22 19:08 tima

I use roles with many tasks files to help group the tasks logically. I explicitly use tasks_from as well in many cases where I need a subset of what a common role provides.

cognifloyd avatar Aug 02 '22 19:08 cognifloyd

Same here... this one would go into my skip list very quickly :-)

kpfleming avatar Sep 26 '22 20:09 kpfleming

I see no reason at all for this rule!

And there is no sense for: "this is fragile and requires extra maintenance, also prone to bugs."

"tasks_from" create a powerful way to create good entry points in a role. Any developer can create bugs regardless of this option.

I kindly ask that you review carefully this option.

lucianoventura avatar Nov 30 '22 00:11 lucianoventura

First, be assured that this rule is far from being our too priority and if it will see the light of days, then will only be as a notification, warning or opt-in.

To understand why is bad, one needs to understand why "goto" was basically banned from most high level languages. It is like starting a program by jumping in the middle if it.

Now, if it was possible to declare (role meta?) which task files are entry points and which one are not, it would be possible to make is less dangerous. Still, this assumes that the role author can do that, which might not be the same person as the role consumer.

ssbarnea avatar Nov 30 '22 08:11 ssbarnea

To some extent that is possible now, by defining the valid entrypoints in meta/argument_specs.yml.

kpfleming avatar Nov 30 '22 11:11 kpfleming

That is indeed great as we did not want to blindly tell everyone to stop using tasks_from. I guess we can consider tasks that is not exposed inside argument_specs as being "private interface", and obviously rename the rule to advise again use of private interfaces.

ssbarnea avatar Nov 30 '22 17:11 ssbarnea

It is funny how programming concepts and terms are thrown around here.

And when we hit yet another limitation in Ansible, one of the standard answers is "Ansible is not programming".

Looping blocks? Blocks as handlers? include_role as handler? A look at some of the long, long issues at ansible repository and it is always the same responses: "Ansible is not programming"! "We should not support this"!

bluikko avatar Dec 01 '22 09:12 bluikko