Project

General

Profile

Review #570

Please review Fail2ban role

Added by Marc Dequènes almost 2 years ago. Updated over 1 year ago.

Status:
Resolved
Priority:
Normal
Category:
System :: Base
Start date:
2017-06-25
Branch:

Description

ssh://vcs-git.duckcorp.org/srv/vcs/git/duckcorp/ansible-role-fail2ban


Related issues

Related to DuckCorp Infrastructure - Enhancement #552: Upgrade to Debian Strech Resolved 2017-06-19

History

#1 Updated by Marc Dequènes almost 2 years ago

#2 Updated by Pierre-Louis Bonicoli over 1 year ago

Some task files (_install.yml) are prefixed with an underscore, why ?

#3 Updated by Pierre-Louis Bonicoli over 1 year ago

Content of _install.yml could be moved to a block in main.yml.

#4 Updated by Pierre-Louis Bonicoli over 1 year ago

  • Status changed from New to In Progress

In tasks/detect_config.yml, an item matching distribution name and distribution major version could be added to with_first_found loop.

#5 Updated by Pierre-Louis Bonicoli over 1 year ago

In tasks/_detect_daemon.yml, a block could be used for the last two tasks: both tasks are using the same notify entry.

#6 Updated by Pierre-Louis Bonicoli over 1 year ago

Add ansible_managed at the beginning of templates/common.conf.

#7 Updated by Pierre-Louis Bonicoli over 1 year ago

Add a comment at the beginning of files located in files/jail.d, something like:

managed by ansible

#8 Updated by Marc Dequènes over 1 year ago

  • Status changed from In Progress to Resolved

I'm using underscores to mark the task file as not being an entrypoint. It's the name I use for files part of the public API when using include_role + tasks_from. There is such kind of convention in languages where there is no private scope for method for example.

I chose readability over factorization, but implemented all other changes.

Thanks for the review.

Also available in: Atom PDF