Issue 2487 - Introduce fail2ban module

modules/fail2ban/manifests/init.pp

Index: modules/fail2ban/manifests/init.pp
===================================================================
new file mode 100644
--- /dev/null
+++ b/modules/fail2ban/manifests/init.pp
@@ -0,0 +1,80 @@
+# == Class: fail2ban
+#
+# Create and maintain fail2ban (http://www.fail2ban.org/) setups.
+#
+# == Parameters:
+#
+# [*jail_config*]
+#   Adds jail.local to the default configuration of fail2ban 

[f.nicolaisen, 2016/11/25 15:09:09]

Unnecessary white space at end of line.

+#
+# [*package*]
+#   Overwrite the default package options, to fine-tune the target version (i.e.
+#   ensure => 'latest') or remove Fluentd (ensure => 'absent' or 'purged')

[mathias, 2016/11/24 16:08:48]

Fluentd?

[f.lopez, 2016/11/25 15:13:49]

Acknowledged.

+#
+# [*service*]
+#   Overwrite the default service options.
+#
+# [*filters*]
+#   Adds adittional filters to the filters.d folder
+# === Examples:
+#
+# class {'fail2ban':
+#   package => {ensure => 'present',},
+#   service => {},
+#   jail_config => {
+#     'wordpress' => {
+#       logpath => '/var/log/nginx/access.log',        

[f.nicolaisen, 2016/11/25 15:09:09]

Unnecessary white spaces at end of line.

+#     }
+#   },
+#   filters => {
+#     'wordpress' => {
+#       failregex => [
+# 	  '^<HOST>.*\"WordPress\/.*',
+# 	],
+#     }
+#   },
+# }

[mathias, 2016/11/24 16:08:48]

The example code is not properly indented. Also it should follow the same style
practices as regular code within this repository.

[f.lopez, 2016/11/25 15:13:49]

Acknowledged.

+class fail2ban (
+  $package = {},

[mathias, 2016/11/24 16:08:48]

Please make sure to wrap all default arguments according to this pattern:
$parameter = hiera('class::parameter', $default),

This emulates Puppet 3 behavior and allows for i.e. designating parameter values
as global keys within Hiera.

[f.lopez, 2016/11/25 15:13:49]

Acknowledged.

+  $service = {},
+  $jail_config = {},
+  $filters = {},
+) {
+
+  include stdlib
+
+  ensure_resource('package', $title, $package)
+
+  # Used as default $ensure parameter for most resources below
+  $ensure = getparam(Package[$title], 'ensure') ? {
+    /^(absent|purged|held)$/ => 'absent',

[mathias, 2016/11/24 16:08:48]

By now I wouldn't consider a "held" package as "absent" any more. Usually one
would mark a package that way if there is a customized local version available.

[f.lopez, 2016/11/25 15:13:49]

Acknowledged.

+    default => 'present',
+  }
+
+  # Service resources don't properly support the concept of absence
+  if ($ensure == 'present') or ($service['ensure'] != undef) {

[mathias, 2016/11/24 16:08:48]

Why checking for $service['ensure'] being defined? Shouldn't the default be
totally fine?

[f.lopez, 2016/11/25 15:13:49]

Acknowledged.

+
+    ensure_resource('service', $title, $service)
+    # See modules/fail2ban/manifests/filter.pp
+    create_resources('fail2ban::filter', $filters)
+    

[f.nicolaisen, 2016/11/25 15:09:09]

Unnecessary white spaces here.

+    # According to the docs one can also enable filters that are
+    # already in there, so the config file should be done appart.

[mathias, 2016/11/24 16:08:48]

I don't really get this point, but I assume you meant "separately" when you
wrote "appart"?

[f.lopez, 2016/11/25 15:13:49]

Acknowledged.

+    if jail_config != undef {

[mathias, 2016/11/24 16:08:48]

The $jail_config defaults to an empty hash, so I doubt that this condition makes
much sense here. Especially not if this "magic" with undef is not intended
and/or documented accordingly. 

[f.lopez, 2016/11/25 15:13:49]

One can have filters without activating them, so you can still create filters
without activating them.

+      file {'/etc/fail2ban/jail.local':
+        ensure => present,
+        group => 'root',
+        mode => '0644',
+        owner => 'root',
+        content => template("fail2ban/jail.erb"),
+        notify => Service[$title],
+      }
+    }
+    

[f.nicolaisen, 2016/11/25 15:09:09]

Unnecessary white spaces here.

+    Service[$title] <~ Package[$title]

[mathias, 2016/11/24 16:08:48]

Usually package updates imply reloading/restarting the associated service (if
any), hence I am not sure if one would want to notify the service resource here
(rather than just declaring an order relationship).

[f.lopez, 2016/11/25 15:13:49]

Notify is a kind of relation, but I agree that when the package is updated or
installed is handles that part, I'm removing this.

+  }
+
+  Package[$title] -> File['/etc/fail2ban/jail.local']

[mathias, 2016/11/24 16:08:48]

Since the file resource is just declared under certain circumstances, the
dependency on the package can trigger an error if denoted in this fashion and
position.

[f.lopez, 2016/11/25 15:13:49]

Acknowledged.

+
+
+}