Acme state fixup

salt/modules/acme.py

@@ -42,6 +42,7 @@
 
 # Import salt libs
 import salt.utils.path
+from salt.exceptions import SaltInvocationError
 
 log = logging.getLogger(__name__)
 
@@ -72,30 +73,31 @@ def _expires(name):
     '''
     Return the expiry date of a cert
 
-    :return datetime object of expiry date
+    :rtype: datetime
+    :return: Expiry date
     '''
     cert_file = _cert_file(name, 'cert')
     # Use the salt module if available
     if 'tls.cert_info' in __salt__:
-        expiry = __salt__['tls.cert_info'](cert_file)['not_after']
+        expiry = __salt__['tls.cert_info'](cert_file).get('not_after', 0)
     # Cobble it together using the openssl binary
     else:
         openssl_cmd = 'openssl x509 -in {0} -noout -enddate'.format(cert_file)
         # No %e format on my Linux'es here
         strptime_sux_cmd = 'date --date="$({0} | cut -d= -f2)" +%s'.format(openssl_cmd)
         expiry = float(__salt__['cmd.shell'](strptime_sux_cmd, output_loglevel='quiet'))
         # expiry = datetime.datetime.strptime(expiry.split('=', 1)[-1], '%b %e %H:%M:%S %Y %Z')
-
     return datetime.datetime.fromtimestamp(expiry)
 
 
 def _renew_by(name, window=None):
     '''
     Date before a certificate should be renewed
 
-    :param name: Common Name of the certificate (DNS name of certificate)
-    :param window: days before expiry date to renew
-    :return datetime object of first renewal date
+    :param str name: Common Name of the certificate (DNS name of certificate)
+    :param int window: days before expiry date to renew
+    :rtype: datetime
+    :return: First renewal date
     '''
     expiry = _expires(name)
     if window is not None:
@@ -131,37 +133,44 @@ def cert(name,
     :param aliases: subjectAltNames (Additional DNS names on certificate)
     :param email: e-mail address for interaction with ACME provider
     :param webroot: True or a full path to use to use webroot. Otherwise use standalone mode
-    :param test_cert: Request a certificate from the Happy Hacker Fake CA (mutually exclusive with 'server')
-    :param renew: True/'force' to force a renewal, or a window of renewal before expiry in days
+    :param test_cert: Request a certificate from the Happy Hacker Fake CA (mutually
+        exclusive with 'server')
+    :param renew: True/'force' to force a renewal, or a window of renewal before
+        expiry in days
     :param keysize: RSA key bits
     :param server: API endpoint to talk to
     :param owner: owner of the private key file
     :param group: group of the private key file
     :param mode: mode of the private key file
     :param certname: Name of the certificate to save
     :param preferred_challenges: A sorted, comma delimited list of the preferred
-                                 challenge to use during authorization with the
-                                 most preferred challenge listed first.
+        challenge to use during authorization with the most preferred challenge
+        listed first.
     :param tls_sni_01_port: Port used during tls-sni-01 challenge. This only affects
-                            the port Certbot listens on. A conforming ACME server
-                            will still attempt to connect on port 443.
+        the port Certbot listens on. A conforming ACME server will still attempt
+        to connect on port 443.
     :param tls_sni_01_address: The address the server listens to during tls-sni-01
-                               challenge.
+        challenge.
     :param http_01_port: Port used in the http-01 challenge. This only affects
-                         the port Certbot listens on. A conforming ACME server
-                         will still attempt to connect on port 80.
+        the port Certbot listens on. A conforming ACME server will still attempt
+        to connect on port 80.
     :param https_01_address: The address the server listens to during http-01 challenge.
-    :param dns_plugin: Name of a DNS plugin to use (currently only 'cloudflare' or 'digitalocean')
-    :param dns_plugin_credentials: Path to the credentials file if required by the specified DNS plugin
-    :param dns_plugin_propagate_seconds: Number of seconds to wait for DNS propogations before
-                                asking ACME servers to verify the DNS record. (default 10)
-    :return: dict with 'result' True/False/None, 'comment' and certificate's expiry date ('not_after')
+    :param dns_plugin: Name of a DNS plugin to use (currently only 'cloudflare'
+        or 'digitalocean')
+    :param dns_plugin_credentials: Path to the credentials file if required by
+        the specified DNS plugin
+    :param dns_plugin_propagate_seconds: Number of seconds to wait for DNS propogations
+        before asking ACME servers to verify the DNS record. (default 10)
+    :rtype: dict
+    :return: Dictionary with 'result' True/False/None, 'comment' and certificate's
+        expiry date ('not_after')
 
     CLI example:
 
     .. code-block:: bash
 
-        salt 'gitlab.example.com' acme.cert dev.example.com "[gitlab.example.com]" test_cert=True renew=14 webroot=/opt/gitlab/embedded/service/gitlab-rails/public
+        salt 'gitlab.example.com' acme.cert dev.example.com "[gitlab.example.com]" test_cert=True \
+        renew=14 webroot=/opt/gitlab/embedded/service/gitlab-rails/public
     '''
 
     cmd = [LEA, 'certonly', '--non-interactive', '--agree-tos']
@@ -235,9 +244,13 @@ def cert(name,
             cmd.append('--expand')
             res = __salt__['cmd.run_all'](' '.join(cmd))
             if res['retcode'] != 0:
-                return {'result': False, 'comment': 'Certificate {0} renewal failed with:\n{1}'.format(name, res['stderr'])}
+                return {'result': False,
+                        'comment': ('Certificate {0} renewal failed with:\n{1}'
+                                    ''.format(name, res['stderr']))}
         else:
-            return {'result': False, 'comment': 'Certificate {0} renewal failed with:\n{1}'.format(name, res['stderr'])}
+            return {'result': False,
+                    'comment': ('Certificate {0} renewal failed with:\n{1}'
+                                ''.format(name, res['stderr']))}
 
     if 'no action taken' in res['stdout']:
         comment = 'Certificate {0} unchanged'.format(cert_file)
@@ -268,43 +281,52 @@ def certs():
 
         salt 'vhost.example.com' acme.certs
     '''
-    return __salt__['file.readdir'](LE_LIVE)[2:]
+    return [item for item in __salt__['file.readdir'](LE_LIVE)[2:] if os.path.isdir(item)]
 
 
 def info(name):
     '''
     Return information about a certificate
 
-    .. note::
-        Will output tls.cert_info if that's available, or OpenSSL text if not
-
-    :param name: CommonName of cert
+    :param str name: CommonName of certificate
+    :rtype: dict
+    :return: Dictionary with information about the certificate.
+        If neither the ``tls`` nor the ``x509`` module can be used to determine
+        the certificate information, the information will be retrieved as one
+        big text block under the key ``text`` using the openssl cli.
 
     CLI example:
 
     .. code-block:: bash
 
         salt 'gitlab.example.com' acme.info dev.example.com
     '''
+    if not has(name):
+        return {}
     cert_file = _cert_file(name, 'cert')
-    # Use the salt module if available
+    # Use the tls salt module if available
     if 'tls.cert_info' in __salt__:
         cert_info = __salt__['tls.cert_info'](cert_file)
         # Strip out the extensions object contents;
         # these trip over our poor state output
         # and they serve no real purpose here anyway
         cert_info['extensions'] = cert_info['extensions'].keys()
-        return cert_info
-    # Cobble it together using the openssl binary
-    openssl_cmd = 'openssl x509 -in {0} -noout -text'.format(cert_file)
-    return __salt__['cmd.run'](openssl_cmd, output_loglevel='quiet')
+    elif 'x509.read_certificate' in __salt__:
+        cert_info = __salt__['x509.read_certificate'](cert_file)
+    else:
+        # Cobble it together using the openssl binary
+        openssl_cmd = 'openssl x509 -in {0} -noout -text'.format(cert_file)
+        cert_info = {'text': __salt__['cmd.run'](openssl_cmd, output_loglevel='quiet')}
+    return cert_info
 
 
 def expires(name):
     '''
     The expiry date of a certificate in ISO format
 
-    :param name: CommonName of cert
+    :param str name: CommonName of certificate
+    :rtype: str
+    :return: Expiry date in ISO format.
 
     CLI example:
 
@@ -319,7 +341,8 @@ def has(name):
     '''
     Test if a certificate is in the Let's Encrypt Live directory
 
-    :param name: CommonName of cert
+    :param str name: CommonName of certificate
+    :rtype: bool
 
     Code example:
 
@@ -335,8 +358,10 @@ def renew_by(name, window=None):
     '''
     Date in ISO format when a certificate should first be renewed
 
-    :param name: CommonName of cert
-    :param window: number of days before expiry when renewal should take place
+    :param str name: CommonName of certificate
+    :param int window: number of days before expiry when renewal should take place
+    :rtype: str
+    :return: Date of certificate renewal in ISO format.
     '''
     return _renew_by(name, window).isoformat()
 
@@ -345,8 +370,10 @@ def needs_renewal(name, window=None):
     '''
     Check if a certificate needs renewal
 
-    :param name: CommonName of cert
-    :param window: Window in days to renew earlier or True/force to just return True
+    :param str name: CommonName of certificate
+    :param bool/str/int window: Window in days to renew earlier or True/force to just return True
+    :rtype: bool
+    :return: Whether or not the certificate needs to be renewed.
 
     Code example:
 
@@ -357,7 +384,14 @@ def needs_renewal(name, window=None):
         else:
             log.info('Your certificate is still good')
     '''
-    if window is not None and window in ('force', 'Force', True):
-        return True
+    if window:
+        if str(window).lower in ('force', 'true'):
+            return True
+        if not (isinstance(window, int) or (hasattr(window, 'isdigit') and window.isdigit())):
+            raise SaltInvocationError(
+                'The argument "window", if provided, must be one of the following : '
+                'True (boolean), "force" or "Force" (str) or a numerical value in days.'
+            )
+        window = int(window)
 
     return _renew_by(name, window) <= datetime.datetime.today()

salt/states/acme.py

@@ -29,6 +29,9 @@
 from __future__ import absolute_import, print_function, unicode_literals
 import logging
 
+# Import salt libs
+import salt.utils.dictdiffer
+
 log = logging.getLogger(__name__)
 
 
@@ -88,74 +91,52 @@ def cert(name,
     :param dns_plugin: Name of a DNS plugin to use (currently only 'cloudflare')
     :param dns_plugin_credentials: Path to the credentials file if required by the specified DNS plugin
     '''
+    ret = {'name': name, 'result': 'changeme', 'comment': [], 'changes': {}}
+    action = None
 
-    if __opts__['test']:
-        ret = {
-            'name': name,
-            'changes': {},
-            'result': None
-        }
-        window = None
-        try:
-            window = int(renew)
-        except Exception:
-            pass
-
-        comment = 'Certificate {0} '.format(name)
-        if not __salt__['acme.has'](name):
-            comment += 'would have been obtained'
-        elif __salt__['acme.needs_renewal'](name, window):
-            comment += 'would have been renewed'
-        else:
-            comment += 'would not have been touched'
-            ret['result'] = True
-        ret['comment'] = comment
-        return ret
-
+    current_certificate = {}
+    new_certificate = {}
     if not __salt__['acme.has'](name):
-        old = None
-    else:
-        old = __salt__['acme.info'](name)
-
-    res = __salt__['acme.cert'](
-        name,
-        aliases=aliases,
-        email=email,
-        webroot=webroot,
-        certname=certname,
-        test_cert=test_cert,
-        renew=renew,
-        keysize=keysize,
-        server=server,
-        owner=owner,
-        group=group,
-        mode=mode,
-        preferred_challenges=preferred_challenges,
-        tls_sni_01_port=tls_sni_01_port,
-        tls_sni_01_address=tls_sni_01_address,
-        http_01_port=http_01_port,
-        http_01_address=http_01_address,
-        dns_plugin=dns_plugin,
-        dns_plugin_credentials=dns_plugin_credentials,
-    )
-
-    ret = {
-        'name': name,
-        'result': res['result'] is not False,
-        'comment': res['comment']
-    }
-
-    if res['result'] is None:
-        ret['changes'] = {}
+        action = 'obtain'
+    elif __salt__['acme.needs_renewal'](name, renew):
+        action = 'renew'
+        current_certificate = __salt__['acme.info'](name)
     else:
-        if not __salt__['acme.has'](name):
-            new = None
+        ret['result'] = True
+        ret['comment'].append('Certificate {} exists and does not need renewal.'
+                              ''.format(name))
+
+    if action:
+        if __opts__['test']:
+            ret['result'] = None
+            ret['comment'].append('Certificate {} would have been {}ed.'
+                                  ''.format(name, action))
+            ret['changes'] = {'old': 'current certificate', 'new': 'new certificate'}
         else:
-            new = __salt__['acme.info'](name)
-
-        ret['changes'] = {
-            'old': old,
-            'new': new
-        }
-
+            res = __salt__['acme.cert'](
+                name,
+                aliases=aliases,
+                email=email,
+                webroot=webroot,
+                certname=certname,
+                test_cert=test_cert,
+                renew=renew,
+                keysize=keysize,
+                server=server,
+                owner=owner,
+                group=group,
+                mode=mode,
+                preferred_challenges=preferred_challenges,
+                tls_sni_01_port=tls_sni_01_port,
+                tls_sni_01_address=tls_sni_01_address,
+                http_01_port=http_01_port,
+                http_01_address=http_01_address,
+                dns_plugin=dns_plugin,
+                dns_plugin_credentials=dns_plugin_credentials,
+            )
+            ret['result'] = res['result']
+            ret['comment'].append(res['comment'])
+            if ret['result']:
+                new_certificate = __salt__['acme.info'](name)
+            ret['changes'] = salt.utils.dictdiffer.deep_diff(current_certificate, new_certificate)
     return ret