Merge pull request #3390 from hosseinsh/acme-fixed-attempts

Acme reduce attempts

docs/administration.rst

@@ -362,6 +362,9 @@ Whenever a pending ACME certificate fails to be issued, Lemur will send a notifi
 and security team (as specified by the ``LEMUR_SECURITY_TEAM_EMAIL`` configuration parameter). This email is not sent if
 the pending certificate had notifications disabled.
 
+Lemur will attempt 3x times to resolve a pending certificate.
+This can at times result into 3 duplicate certificates, if all certificate attempts get resolved.
+
 **Certificate rotation**
 
 Whenever a cert is rotated, Lemur will send a notification via email to the certificate owner. This notification is
@@ -820,6 +823,20 @@ ACME Plugin
         Enables delegated DNS domain validation using CNAMES.  When enabled, Lemur will attempt to follow CNAME records to authoritative DNS servers when creating DNS-01 challenges.
 
 
+The following configration properties are optional for the ACME plugin to use. They allow reusing an existing ACME
+account. See :ref:`Using a pre-existing ACME account <AcmeAccountReuse>` for more details.
+
+
+.. data:: ACME_PRIVATE_KEY
+    :noindex:
+
+            This is the private key, the account was registered with (in JWK format)
+
+.. data:: ACME_REGR
+    :noindex:
+
+            This is the registration for the ACME account, the most important part is the uri attribute (in JSON)
+
 Active Directory Certificate Services Plugin
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
@@ -1336,23 +1353,6 @@ The following configuration properties are required to use the PowerDNS ACME Plu
 
             File/Dir path to CA Bundle: Verifies the TLS certificate was issued by a Certificate Authority in the provided CA bundle.
 
-ACME Plugin
-~~~~~~~~~~~~
-
-The following configration properties are optional for the ACME plugin to use. They allow reusing an existing ACME
-account. See :ref:`Using a pre-existing ACME account <AcmeAccountReuse>` for more details.
-
-
-.. data:: ACME_PRIVATE_KEY
-    :noindex:
-
-            This is the private key, the account was registered with (in JWK format)
-
-.. data:: ACME_REGR
-    :noindex:
-
-            This is the registration for the ACME account, the most important part is the uri attribute (in JSON)
-
 .. _CommandLineInterface:
 
 Command Line Interface

lemur/certificates/service.py

@@ -388,6 +388,7 @@ def create(**kwargs):
         cert = Certificate(**kwargs)
         kwargs["creator"].certificates.append(cert)
     else:
+        # ACME path
         cert = PendingCertificate(**kwargs)
         kwargs["creator"].pending_certificates.append(cert)
 

lemur/common/celery.py

@@ -20,6 +20,7 @@ from flask import current_app
 from lemur.authorities.service import get as get_authority
 from lemur.certificates import cli as cli_certificate
 from lemur.common.redis import RedisHandler
+from lemur.constants import ACME_ADDITIONAL_ATTEMPTS
 from lemur.destinations import service as destinations_service
 from lemur.dns_providers import cli as cli_dns_providers
 from lemur.endpoints import cli as cli_endpoints
@@ -273,7 +274,8 @@ def fetch_acme_cert(id):
         real_cert = cert.get("cert")
         # It's necessary to reload the pending cert due to detached instance: http://sqlalche.me/e/bhk3
         pending_cert = pending_certificate_service.get(cert.get("pending_cert").id)
-        if not pending_cert:
+        if not pending_cert or pending_cert.resolved:
+            # pending_cert is cleared or it was resolved by another process
             log_data[
                 "message"
             ] = "Pending certificate doesn't exist anymore. Was it resolved by another process?"
@@ -301,7 +303,7 @@ def fetch_acme_cert(id):
             error_log["last_error"] = cert.get("last_error")
             error_log["cn"] = pending_cert.cn
 
-            if pending_cert.number_attempts > 4:
+            if pending_cert.number_attempts > ACME_ADDITIONAL_ATTEMPTS:
                 error_log["message"] = "Deleting pending certificate"
                 send_pending_failure_notification(
                     pending_cert, notify_owner=pending_cert.notify

lemur/constants.py

@@ -12,6 +12,9 @@ NONSTANDARD_NAMING_TEMPLATE = "{issuer}-{not_before}-{not_after}"
 SUCCESS_METRIC_STATUS = "success"
 FAILURE_METRIC_STATUS = "failure"
 
+# when ACME attempts to resolve a certificate try in total 3 times
+ACME_ADDITIONAL_ATTEMPTS = 2
+
 CERTIFICATE_KEY_TYPES = [
     "RSA2048",
     "RSA4096",

lemur/pending_certificates/cli.py

@@ -12,10 +12,12 @@ from flask import current_app
 from flask_script import Manager
 
 from lemur.authorities.service import get as get_authority
+from lemur.constants import ACME_ADDITIONAL_ATTEMPTS
 from lemur.notifications.messaging import send_pending_failure_notification
 from lemur.pending_certificates import service as pending_certificate_service
 from lemur.plugins.base import plugins
 
+
 manager = Manager(usage="Handles pending certificate related tasks.")
 
 
@@ -107,7 +109,7 @@ def fetch_all_acme():
             error_log["last_error"] = cert.get("last_error")
             error_log["cn"] = pending_cert.cn
 
-            if pending_cert.number_attempts > 4:
+            if pending_cert.number_attempts > ACME_ADDITIONAL_ATTEMPTS:
                 error_log["message"] = "Marking pending certificate as resolved"
                 send_pending_failure_notification(
                     pending_cert, notify_owner=pending_cert.notify