Project

General

Profile

Review #570

Please review Fail2ban role

Added by Marc Dequènes about 2 years ago. Updated almost 2 years 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 StrechResolved2017-06-19

Actions

History

#1

Updated by Marc Dequènes about 2 years ago

#2

Updated by Pierre-Louis Bonicoli almost 2 years ago

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

#3

Updated by Pierre-Louis Bonicoli almost 2 years ago

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

#4

Updated by Pierre-Louis Bonicoli almost 2 years 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 almost 2 years 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 almost 2 years ago

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

#7

Updated by Pierre-Louis Bonicoli almost 2 years 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 almost 2 years 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