Major cleanup.

8 files changed, 74 insertions, 44 deletions

diff --git a/manifests/cert.pp b/manifests/cert.pp
index a8cc94e..061ace1 100644
--- a/manifests/cert.pp
+++ b/manifests/cert.pp
@@ -1,29 +1,34 @@
-# A single certificate
+# @summary A single certificate
 # TODO possibly default cert_name to $::fqdn instead
+# @param cert_name Name of the certificate
+# @param ensure    Present or absent (currently does nothing)
+# @param include_self Should the certificates name be one of its domains?
 define letsencrypt::cert (
-  String $cert_name                 => $::name,
-  Enum['present', 'absent'] $ensure => 'present',
-  Boolean $include_self             => true,
+  String $cert_name                 = $name,
+  Enum['present', 'absent'] $ensure = 'present',
+  Boolean $include_self             = true,
 ) {
-
   # TODO these env files are systemd specific
   # TODO concat::fragment is clumsy, look at re-implementing the
   # functionallity internally
 
   concat { "${letsencrypt::config_dir}/env/${cert_name}":
-    ensure         => present,
-    warn           => true,
+    ensure => present,
+    warn   => true,
   }
 
-  concat::fragment { "letsencrypt ${cert_name} preamble":
-    target  => "${letsencrypt::config_dir}/env/${cert_name}",
-    order   => '0',
-    content => @(EOF)
+  $cert_preamble = @(EOF)
     AUTHENTICATOR = ''
     POST_HOOK = ''
     DOMAINS =
     |- EOF
+
+  concat::fragment { "letsencrypt ${cert_name} preamble":
+    target  => "${letsencrypt::config_dir}/env/${cert_name}",
+    order   => '0',
+    content => $cert_preamble,
   }
+
   concat::fragment { "letsencrypt ${cert_name} postamble":
     target  => "${letsencrypt::config_dir}/env/${cert_name}",
     order   => '99',
diff --git a/manifests/domain.pp b/manifests/domain.pp
index cc9e2af..9e6b377 100644
--- a/manifests/domain.pp
+++ b/manifests/domain.pp
@@ -1,14 +1,16 @@
 # A single domain belonging to a certificate
-# Example
+# @example
 # letsencrypt::domain { 'www.example.com':
 #   cert_name => 'example.com',
 # }
+# @param domain_name Hostname which should be included in the target certificate
+# @param cert_name Certificate to add the hostname to
 define letsencrypt::domain (
-  String $domain_name => $name,
-  String $cert_name   => $::fqdn,
+  String $domain_name = $name,
+  String $cert_name   = $::facts['fqdn'],
 ) {
   ensure_resource('letsencrypt::cert', $cert_name, {
-    ensure => present,
+      ensure => present,
   })
 
   concat::fragment { "letsencrypt ${cert_name} - ${domain_name}":
diff --git a/manifests/init.pp b/manifests/init.pp
index 0fedb85..cc72b32 100644
--- a/manifests/init.pp
+++ b/manifests/init.pp
@@ -1,28 +1,38 @@
+# @summary Sets up letsencrypt for other classes
+# @param email Contact email sent to letsencrypt
+# @param config_dir Location of configuration files
+# @param default_cert Should a certificate be automatically configured
+# @param default_cert_name
+#   The name (and domain) of the automatically configured centificate.
 class letsencrypt (
   String $email,
-  String $default_cert_name = $::fqdn,
   Stdlib::Unixpath $config_dir = '/etc/letsencrypt',
+  String $default_cert_name = $::facts['fqdn'],
   Boolean $default_cert = true,
   # TODO renewal provider here?
 ) {
-
   if $default_cert {
     letsencrypt::cert { $default_cert_name:
       ensure => present,
     }
   }
 
-
   file { $config_dir:
     ensure => directory,
   }
 
-  file { "${config_dir}/cli.ini":
-    content = @("EOF")
-    email = $email
+  $cli_conf = @("EOF")
+    email = ${email}
     | EOF
-  }
 
+  file { "${config_dir}/cli.ini":
+    content => $cli_conf,
+  }
 
   include letsencrypt::renew::setup
+
+  # Boolean indicating if ssl is configured. Mainly used by
+  # letsencrypt::nginx and similar classes to determine their export
+  # of their variable $ssl.
+  $ssl_configured = 'letsencrypt_director$' in keys($facts)
 }
diff --git a/manifests/nginx.pp b/manifests/nginx.pp
index 82fcda4..75b5b48 100644
--- a/manifests/nginx.pp
+++ b/manifests/nginx.pp
@@ -1,22 +1,28 @@
 # Sets up nginx specific configuration, and provides access to
 # variables for enterpolating into nginx configurations
-# Usage:
 #
 # These use the default cert name
-#
+# @example
 # nginx::resource::server { 'servername':
 #   * => $letsescrypt::nginx::server_ssl
 # }
 # $letsencrypt::nginx::location_ssl
+#
+# @param certbot_plugin_package
+#   Name of the system package providing this plugin.
+#   Populated through hiera.
+# @param manage_package
+#   If this class should manage the package.
 class letsencrypt::nginx (
-  Boolean $manage_package: true,
   String $certbot_plugin_package,
+  Boolean $manage_package = true,
 ) {
+  # TODO $cert_path should use the default certificate name.
+  # There should however also be a hash of all configured
+  # certificates.
+  $cert_path = "${letsencrypt::config_dir}/live/${letsencrypt::config_dir::default_cert_name}"
 
-  # TODO $cert_path
-  $cert_path = "/etc/letsencrypt/live/${certname}"
-
-  $server_ssl = if $ssl_configured {
+  $server_ssl = if $letsencrypt::ssl_configured {
     {
       ssl          => true,
       ssl_redirect => true,
@@ -29,7 +35,7 @@ class letsencrypt::nginx (
     }
   }
 
-  $location_ssl = if $ssl_configured {
+  $location_ssl = if $letsencrypt::ssl_configured {
     {
       ssl      => true,
       ssl_only => true,
@@ -39,4 +45,8 @@ class letsencrypt::nginx (
       ssl => false,
     }
   }
+
+  if $manage_package {
+    ensure_packages([$certbot_plugin_package])
+  }
 }
diff --git a/manifests/renew.pp b/manifests/renew.pp
index 681a236..97cf5e9 100644
--- a/manifests/renew.pp
+++ b/manifests/renew.pp
@@ -1,13 +1,15 @@
+# @summary Configures automatic renewal for the given certificate
+# @param cert_name
+#   which certificate to renew. A letsencrypt::cert of the same name
+#   must exists.
+# @api private
 define letsencrypt::renew (
   String $cert_name = $name,
 ) {
-
   # TODO this is systemd specific
   # TODO ensure letsencrypt::renew::setup is included beforehand
   service { "${letsencrypt::renew::systemd::service_name}@${cert_name}.timer":
     ensure => 'running',
     enable => true,
   }
-
 }
-
diff --git a/manifests/renew/cron.pp b/manifests/renew/cron.pp
index 91d5483..37aa3fb 100644
--- a/manifests/renew/cron.pp
+++ b/manifests/renew/cron.pp
@@ -2,5 +2,5 @@
 # private
 class letsencrypt::renew::cron (
 ) {
-  fail("Not yet implemented")
+  fail('Not yet implemented')
 }
diff --git a/manifests/renew/setup.pp b/manifests/renew/setup.pp
index 360136c..8b4708b 100644
--- a/manifests/renew/setup.pp
+++ b/manifests/renew/setup.pp
@@ -2,16 +2,17 @@
 # TODO
 # - make provider OS dependant
 # - is provider the correct name?
-# private
+# @param provider
+#   How the renewal should be managed.
+# @api private
 class letsencrypt::renew::setup (
   Enum['systemd', 'cron'] $provider = 'systemd',
 ) {
   file { [
-    '/etc/letsencrypt/env',
-  ]:
-    ensure => directory,
+      '/etc/letsencrypt/env',
+    ]:
+      ensure => directory,
   }
 
-  include "letsencrypt::renew::${provider}"
+  include "::letsencrypt::renew::${provider}"
 }
-
diff --git a/manifests/renew/systemd.pp b/manifests/renew/systemd.pp
index 4b6f23e..8c63f23 100644
--- a/manifests/renew/systemd.pp
+++ b/manifests/renew/systemd.pp
@@ -1,11 +1,11 @@
 # Handles renewal certificates through systemd timers
-# private
+# @param service_name Target name of the service file
+# @param service_path Where the service file should be installed
+# @api private
 class letsencrypt::renew::systemd (
-  String $service_name = 'letsencrypt-renew'
+  String $service_name = 'letsencrypt-renew',
   String $service_path = '/etc/systemd/system',
 ) {
-
-
   file { "${service_path}/${service_name}@.service":
     source => "puppet:///modules/${module_name}/letsencrypt-renew.service",
   }