updating based on suggestions in 2911

lemur/acme_providers/cli.py

@@ -5,7 +5,6 @@ from flask_script import Manager
 from flask import current_app
 
 from lemur.extensions import sentry
-from lemur.extensions import metrics
 from lemur.constants import SUCCESS_METRIC_STATUS
 from lemur.plugins.lemur_acme.plugin import AcmeHandler
 
@@ -38,9 +37,6 @@ def dnstest(domain, token):
     acme_handler = AcmeHandler()
     acme_handler.autodetect_dns_providers(domain)
     if not acme_handler.dns_providers_for_domain[domain]:
-        metrics.send(
-            "get_authorizations_no_dns_provider_for_domain", "counter", 1
-        )
         raise Exception(f"No DNS providers found for domain: {format(domain)}.")
 
     # Create TXT Records
@@ -66,7 +62,6 @@ def dnstest(domain, token):
             dns_provider_plugin.wait_for_dns_change(change_id, account_number)
             print(f"[+] Verified TXT Record in `{dns_provider.name}` provider")
         except Exception:
-            metrics.send("complete_dns_challenge_error", "counter", 1)
             sentry.captureException()
             current_app.logger.debug(
                 f"Unable to resolve DNS challenge for change_id: {change_id}, account_id: "
@@ -88,5 +83,4 @@ def dnstest(domain, token):
         print(f"[+] Deleted TXT Record in `{dns_provider.name}` provider")
 
     status = SUCCESS_METRIC_STATUS
-    metrics.send("dnstest", "counter", 1, metric_tags={"status": status})
     print("[+] Done with ACME Tests.")

lemur/dns_providers/util.py

@@ -6,6 +6,7 @@ import dns.query
 import dns.resolver
 import re
 
+from lemur.extensions import sentry
 from lemur.extensions import metrics
 
 
@@ -94,6 +95,7 @@ def get_dns_records(domain, rdtype, nameserver):
             for record in rdata.strings:
                 records.append(record.decode("utf-8"))
     except dns.exception.DNSException:
+        sentry.captureException()
         function = sys._getframe().f_code.co_name
         metrics.send(f"{function}.fail", "counter", 1)
     return records

lemur/plugins/lemur_acme/powerdns.py

@@ -3,11 +3,22 @@ import requests
 import json
 import sys
 
+import lemur.common.utils as utils
 import lemur.dns_providers.util as dnsutil
 
 from flask import current_app
 from lemur.extensions import metrics, sentry
 
+REQUIRED_VARIABLES = [
+    "ACME_POWERDNS_APIKEYNAME",
+    "ACME_POWERDNS_APIKEY",
+    "ACME_POWERDNS_DOMAIN",
+]
+
+
+def _check_conf():
+    utils.validate_conf(current_app, REQUIRED_VARIABLES)
+
 
 class Zone:
     """ This class implements a PowerDNS zone in JSON. """
@@ -56,37 +67,37 @@ class Record:
 
 def get_zones(account_number):
     """Retrieve authoritative zones from the PowerDNS API and return a list"""
-    server_id = current_app.config.get("ACME_POWERDNS_SERVERID", "")
+    _check_conf()
+    server_id = current_app.config.get("ACME_POWERDNS_SERVERID", "localhost")
     path = f"/api/v1/servers/{server_id}/zones"
     zones = []
+    function = sys._getframe().f_code.co_name
+    log_data = {
+        "function": function
+    }
     try:
         records = _get(path)
-        function = sys._getframe().f_code.co_name
+        log_data["message"] = "Retrieved Zones Successfully"
-        log_data = {
-            "function": function,
-            "message": "Retrieved Zones Successfully"
-        }
         current_app.logger.debug(log_data)
-        for record in records:
-            zone = Zone(record)
-            if zone.kind == 'Master':
-                zones.append(zone.name)
-        return zones
 
     except Exception as e:
-        function = sys._getframe().f_code.co_name
+        sentry.captureException()
-        log_data = {
+        log_data["message"] = "Failed to Retrieve Zone Data"
-            "function": function,
-            "message": "Failed to Retrieve Zone Data"
-        }
         current_app.logger.debug(log_data)
         raise
 
+    for record in records:
+        zone = Zone(record)
+        if zone.kind == 'Master':
+            zones.append(zone.name)
+    return zones
+
 
 def create_txt_record(domain, token, account_number):
     """ Create a TXT record for the given domain and token and return a change_id tuple """
+    _check_conf()
     zone_name = _get_zone_name(domain, account_number)
-    server_id = current_app.config.get("ACME_POWERDNS_SERVERID", "")
+    server_id = current_app.config.get("ACME_POWERDNS_SERVERID", "localhost")
     zone_id = zone_name + "."
     domain_id = domain + "."
     path = f"/api/v1/servers/{server_id}/zones/{zone_id}"
@@ -107,26 +118,20 @@ def create_txt_record(domain, token, account_number):
             }
         ]
     }
-
+    function = sys._getframe().f_code.co_name
+    log_data = {
+        "function": function,
+        "fqdn": domain,
+        "token": token,
+    }
     try:
         _patch(path, payload)
-        function = sys._getframe().f_code.co_name
+        log_data["message"] = "TXT record successfully created"
-        log_data = {
-            "function": function,
-            "fqdn": domain,
-            "token": token,
-            "message": "TXT record successfully created"
-        }
         current_app.logger.debug(log_data)
     except Exception as e:
-        function = sys._getframe().f_code.co_name
+        sentry.captureException()
-        log_data = {
+        log_data["Exception"] = e
-            "function": function,
+        log_data["message"] = "Unable to create TXT record"
-            "domain": domain,
-            "token": token,
-            "Exception": e,
-            "message": "Unable to create TXT record"
-        }
         current_app.logger.debug(log_data)
 
     change_id = (domain, token)
@@ -138,8 +143,9 @@ def wait_for_dns_change(change_id, account_number=None):
     Checks the authoritative DNS Server to see if changes have propagated to DNS
     Retries and waits until successful.
     """
+    _check_conf()
     domain, token = change_id
-    number_of_attempts = current_app.config.get("ACME_POWERDNS_RETRIES", "")
+    number_of_attempts = current_app.config.get("ACME_POWERDNS_RETRIES", "3")
     zone_name = _get_zone_name(domain, account_number)
     nameserver = dnsutil.get_authoritative_nameserver(zone_name)
     record_found = False
@@ -166,13 +172,13 @@ def wait_for_dns_change(change_id, account_number=None):
         metrics.send(f"{function}.success", "counter", 1, metric_tags={"fqdn": domain, "txt_record": token})
     else:
         metrics.send(f"{function}.fail", "counter", 1, metric_tags={"fqdn": domain, "txt_record": token})
-        sentry.captureException(extra={"fqdn": str(domain), "txt_record": str(token)})
 
 
 def delete_txt_record(change_id, account_number, domain, token):
     """ Delete the TXT record for the given domain and token """
+    _check_conf()
     zone_name = _get_zone_name(domain, account_number)
-    server_id = current_app.config.get("ACME_POWERDNS_SERVERID", "")
+    server_id = current_app.config.get("ACME_POWERDNS_SERVERID", "localhost")
     zone_id = zone_name + "."
     domain_id = domain + "."
     path = f"/api/v1/servers/{server_id}/zones/{zone_id}"
@@ -193,33 +199,27 @@ def delete_txt_record(change_id, account_number, domain, token):
             }
         ]
     }
-
+    function = sys._getframe().f_code.co_name
+    log_data = {
+        "function": function,
+        "fqdn": domain,
+        "token": token
+    }
     try:
         _patch(path, payload)
-        function = sys._getframe().f_code.co_name
+        log_data["message"] = "TXT record successfully deleted"
-        log_data = {
-            "function": function,
-            "fqdn": domain,
-            "token": token,
-            "message": "TXT record successfully deleted"
-        }
         current_app.logger.debug(log_data)
     except Exception as e:
-        function = sys._getframe().f_code.co_name
+        sentry.captureException()
-        log_data = {
+        log_data["Exception"] = e
-            "function": function,
+        log_data["message"] = "Unable to delete TXT record"
-            "domain": domain,
-            "token": token,
-            "Exception": e,
-            "message": "Unable to delete TXT record"
-        }
         current_app.logger.debug(log_data)
 
 
 def _generate_header():
     """Generate a PowerDNS API header and return it as a dictionary"""
-    api_key_name = current_app.config.get("ACME_POWERDNS_APIKEYNAME", "")
+    api_key_name = current_app.config.get("ACME_POWERDNS_APIKEYNAME")
-    api_key = current_app.config.get("ACME_POWERDNS_APIKEY", "")
+    api_key = current_app.config.get("ACME_POWERDNS_APIKEY")
     headers = {api_key_name: api_key}
     return headers
 
@@ -241,7 +241,7 @@ def _get_zone_name(domain, account_number):
 
 def _get(path, params=None):
     """ Execute a GET request on the given URL (base_uri + path) and return response as JSON object """
-    base_uri = current_app.config.get("ACME_POWERDNS_DOMAIN", "")
+    base_uri = current_app.config.get("ACME_POWERDNS_DOMAIN")
     resp = requests.get(
         f"{base_uri}{path}",
         headers=_generate_header(),
@@ -254,7 +254,7 @@ def _get(path, params=None):
 
 def _patch(path, payload):
     """ Execute a Patch request on the given URL (base_uri + path) with given payload """
-    base_uri = current_app.config.get("ACME_POWERDNS_DOMAIN", "")
+    base_uri = current_app.config.get("ACME_POWERDNS_DOMAIN")
     resp = requests.patch(
         f"{base_uri}{path}",
         data=json.dumps(payload),

lemur/plugins/lemur_acme/tests/test_powerdns.py

@@ -31,6 +31,7 @@ class TestPowerdns(unittest.TestCase):
          {'account': '', 'dnssec': 'False', 'id': 'test.example.com.', 'kind': 'Master', 'last_check': 0,
           'masters': [], 'name': 'test.example.com.', 'notified_serial': '2019112501', 'serial': '2019112501',
           'url': '/api/v1/servers/localhost/zones/test.example.com.'}]
+        powerdns._check_conf = Mock()
         powerdns._get = Mock(path)
         powerdns._get.side_effect = [get_response]
         mock_current_app.config.get = Mock(return_value="localhost")
@@ -53,6 +54,7 @@ class TestPowerdns(unittest.TestCase):
         token = "ABCDEFGHIJ"
         account_number = "1234567890"
         change_id = (domain, token)
+        powerdns._check_conf = Mock()
         powerdns._get_zone_name = Mock(return_value=zone)
         mock_current_app.logger.debug = Mock()
         mock_current_app.config.get = Mock(return_value="localhost")
@@ -77,8 +79,8 @@ class TestPowerdns(unittest.TestCase):
         zone_name = "test.example.com"
         nameserver = "1.1.1.1"
         change_id = (domain, token)
+        powerdns._check_conf = Mock()
         mock_records = (token,)
-
         mock_current_app.config.get = Mock(return_value=1)
         powerdns._get_zone_name = Mock(return_value=zone_name)
         mock_dnsutil.get_authoritative_nameserver = Mock(return_value=nameserver)
@@ -103,6 +105,7 @@ class TestPowerdns(unittest.TestCase):
         token = "ABCDEFGHIJ"
         account_number = "1234567890"
         change_id = (domain, token)
+        powerdns._check_conf = Mock()
         powerdns._get_zone_name = Mock(return_value=zone)
         mock_current_app.logger.debug = Mock()
         mock_current_app.config.get = Mock(return_value="localhost")