Merge pull request #2937 from Netflix/powerdnsplugin_02

Update PowerDNS Plugin to support multiple tokens per domain

lemur/plugins/lemur_acme/powerdns.py

@@ -1,11 +1,10 @@
-import time
-import requests
 import json
 import sys
+import time
 
 import lemur.common.utils as utils
 import lemur.dns_providers.util as dnsutil
-
+import requests
 from flask import current_app
 from lemur.extensions import metrics, sentry
 
@@ -17,7 +16,9 @@ REQUIRED_VARIABLES = [
 
 
 class Zone:
-    """ This class implements a PowerDNS zone in JSON. """
+    """
+    This class implements a PowerDNS zone in JSON.
+    """
 
     def __init__(self, _data):
         self._data = _data
@@ -39,7 +40,9 @@ class Zone:
 
 
 class Record:
-    """ This class implements a PowerDNS record. """
+    """
+    This class implements a PowerDNS record.
+    """
 
     def __init__(self, _data):
         self._data = _data
@@ -49,20 +52,30 @@ class Record:
         return self._data["name"]
 
     @property
-    def disabled(self):
+    def type(self):
-        return self._data["disabled"]
+        return self._data["type"]
+
+    @property
+    def ttl(self):
+        return self._data["ttl"]
 
     @property
     def content(self):
         return self._data["content"]
 
     @property
-    def ttl(self):
+    def disabled(self):
-        return self._data["ttl"]
+        return self._data["disabled"]
 
 
 def get_zones(account_number):
-    """Retrieve authoritative zones from the PowerDNS API and return a list"""
+    """
+    Retrieve authoritative zones from the PowerDNS API and return a list of zones
+
+    :param account_number:
+    :raise: Exception
+    :return: list of Zone Objects
+    """
     _check_conf()
     server_id = current_app.config.get("ACME_POWERDNS_SERVERID", "localhost")
     path = f"/api/v1/servers/{server_id}/zones"
@@ -90,44 +103,41 @@ def get_zones(account_number):
 
 
 def create_txt_record(domain, token, account_number):
-    """ Create a TXT record for the given domain and token and return a change_id tuple """
+    """
+    Create a TXT record for the given domain and token and return a change_id tuple
+
+    :param domain: FQDN
+    :param token: challenge value
+    :param account_number:
+    :return: tuple of domain/token
+    """
     _check_conf()
-    zone_name = _get_zone_name(domain, account_number)
+
-    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}"
-    payload = {
-        "rrsets": [
-            {
-                "name": domain_id,
-                "type": "TXT",
-                "ttl": 300,
-                "changetype": "REPLACE",
-                "records": [
-                    {
-                        "content": f"\"{token}\"",
-                        "disabled": False
-                    }
-                ],
-                "comments": []
-            }
-        ]
-    }
     function = sys._getframe().f_code.co_name
     log_data = {
         "function": function,
         "fqdn": domain,
         "token": token,
     }
+
+    # Create new record
+    domain_id = domain + "."
+    records = [Record({'name': domain_id, 'content': f"\"{token}\"", 'disabled': False})]
+
+    # Get current records
+    cur_records = _get_txt_records(domain)
+    for record in cur_records:
+        if record.content != token:
+            records.append(record)
+
     try:
-        _patch(path, payload)
+        _patch_txt_records(domain, account_number, records)
-        log_data["message"] = "TXT record successfully created"
+        log_data["message"] = "TXT record(s) successfully created"
         current_app.logger.debug(log_data)
     except Exception as e:
         sentry.captureException()
         log_data["Exception"] = e
-        log_data["message"] = "Unable to create TXT record"
+        log_data["message"] = "Unable to create TXT record(s)"
         current_app.logger.debug(log_data)
 
     change_id = (domain, token)
@@ -136,8 +146,11 @@ def create_txt_record(domain, token, account_number):
 
 def wait_for_dns_change(change_id, account_number=None):
     """
-    Checks the authoritative DNS Server to see if changes have propagated to DNS
+    Checks the authoritative DNS Server to see if changes have propagated.
-    Retries and waits until successful.
+
+    :param change_id: tuple of domain/token
+    :param account_number:
+    :return:
     """
     _check_conf()
     domain, token = change_id
@@ -171,53 +184,115 @@ def wait_for_dns_change(change_id, account_number=None):
 
 
 def delete_txt_record(change_id, account_number, domain, token):
-    """ Delete the TXT record for the given domain and token """
+    """
+    Delete the TXT record for the given domain and token
+
+    :param change_id: tuple of domain/token
+    :param account_number:
+    :param domain: FQDN
+    :param token: challenge to delete
+    :return:
+    """
     _check_conf()
-    zone_name = _get_zone_name(domain, account_number)
+
-    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}"
-    payload = {
-        "rrsets": [
-            {
-                "name": domain_id,
-                "type": "TXT",
-                "ttl": 300,
-                "changetype": "DELETE",
-                "records": [
-                    {
-                        "content": f"\"{token}\"",
-                        "disabled": False
-                    }
-                ],
-                "comments": []
-            }
-        ]
-    }
     function = sys._getframe().f_code.co_name
     log_data = {
         "function": function,
         "fqdn": domain,
-        "token": token
+        "token": token,
     }
-    try:
+
-        _patch(path, payload)
+    """
-        log_data["message"] = "TXT record successfully deleted"
+    Get existing TXT records matching the domain from DNS
-        current_app.logger.debug(log_data)
+    The token to be deleted should already exist
-    except Exception as e:
+    There may be other records with different tokens as well
-        sentry.captureException()
+    """
-        log_data["Exception"] = e
+    cur_records = _get_txt_records(domain)
-        log_data["message"] = "Unable to delete TXT record"
+    found = False
+    new_records = []
+    for record in cur_records:
+        if record.content == f"\"{token}\"":
+            found = True
+        else:
+            new_records.append(record)
+
+    # Since the matching token is not in DNS, there is nothing to delete
+    if not found:
+        log_data["message"] = "Unable to delete TXT record: Token not found in existing TXT records"
         current_app.logger.debug(log_data)
+        return
+
+    # The record to delete has been found AND there are other tokens set on the same domain
+    # Since we only want to delete one token value from the RRSet, we need to use the Patch command to
+    # overwrite the current RRSet with the existing records.
+    elif new_records:
+        try:
+            _patch_txt_records(domain, account_number, new_records)
+            log_data["message"] = "TXT record successfully deleted"
+            current_app.logger.debug(log_data)
+        except Exception as e:
+            sentry.captureException()
+            log_data["Exception"] = e
+            log_data["message"] = "Unable to delete TXT record: patching exception"
+            current_app.logger.debug(log_data)
+
+    # The record to delete has been found AND there are no other token values set on the same domain
+    # Use the Delete command to delete the whole RRSet.
+    else:
+        zone_name = _get_zone_name(domain, account_number)
+        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}"
+        payload = {
+            "rrsets": [
+                {
+                    "name": domain_id,
+                    "type": "TXT",
+                    "ttl": 300,
+                    "changetype": "DELETE",
+                    "records": [
+                        {
+                            "content": f"\"{token}\"",
+                            "disabled": False
+                        }
+                    ],
+                    "comments": []
+                }
+            ]
+        }
+        function = sys._getframe().f_code.co_name
+        log_data = {
+            "function": function,
+            "fqdn": domain,
+            "token": token
+        }
+        try:
+            _patch(path, payload)
+            log_data["message"] = "TXT record successfully deleted"
+            current_app.logger.debug(log_data)
+        except Exception as e:
+            sentry.captureException()
+            log_data["Exception"] = e
+            log_data["message"] = "Unable to delete TXT record"
+            current_app.logger.debug(log_data)
 
 
 def _check_conf():
+    """
+    Verifies required configuration variables are set
+
+    :return:
+    """
     utils.validate_conf(current_app, REQUIRED_VARIABLES)
 
 
 def _generate_header():
-    """Generate a PowerDNS API header and return it as a dictionary"""
+    """
+    Generate a PowerDNS API header and return it as a dictionary
+
+    :return: Dict of header parameters
+    """
     api_key_name = current_app.config.get("ACME_POWERDNS_APIKEYNAME")
     api_key = current_app.config.get("ACME_POWERDNS_APIKEY")
     headers = {api_key_name: api_key}
@@ -225,7 +300,13 @@ def _generate_header():
 
 
 def _get_zone_name(domain, account_number):
-    """Get most specific matching zone for the given domain and return as a String"""
+    """
+    Get most specific matching zone for the given domain and return as a String
+
+    :param domain: FQDN
+    :param account_number:
+    :return: FQDN of domain
+    """
     zones = get_zones(account_number)
     zone_name = ""
     for z in zones:
@@ -243,8 +324,47 @@ def _get_zone_name(domain, account_number):
     return zone_name
 
 
+def _get_txt_records(domain):
+    """
+    Retrieve TXT records for a given domain and return list of Record Objects
+
+    :param domain: FQDN
+    :return: list of Record objects
+    """
+    server_id = current_app.config.get("ACME_POWERDNS_SERVERID", "localhost")
+
+    path = f"/api/v1/servers/{server_id}/search-data?q={domain}&max=100&object_type=record"
+    function = sys._getframe().f_code.co_name
+    log_data = {
+        "function": function
+    }
+    try:
+        records = _get(path)
+        log_data["message"] = "Retrieved TXT Records Successfully"
+        current_app.logger.debug(log_data)
+
+    except Exception as e:
+        sentry.captureException()
+        log_data["Exception"] = e
+        log_data["message"] = "Failed to Retrieve TXT Records"
+        current_app.logger.debug(log_data)
+        return []
+
+    txt_records = []
+    for record in records:
+        cur_record = Record(record)
+        txt_records.append(cur_record)
+    return txt_records
+
+
 def _get(path, params=None):
-    """ Execute a GET request on the given URL (base_uri + path) and return response as JSON object """
+    """
+    Execute a GET request on the given URL (base_uri + path) and return response as JSON object
+
+    :param path: Relative URL path
+    :param params: additional parameters
+    :return: json response
+    """
     base_uri = current_app.config.get("ACME_POWERDNS_DOMAIN")
     verify_value = current_app.config.get("ACME_POWERDNS_VERIFY", True)
     resp = requests.get(
@@ -257,8 +377,54 @@ def _get(path, params=None):
     return resp.json()
 
 
+def _patch_txt_records(domain, account_number, records):
+    """
+    Send Patch request to PowerDNS Server
+
+    :param domain: FQDN
+    :param account_number:
+    :param records: List of Record objects
+    :return:
+    """
+    domain_id = domain + "."
+
+    # Create records
+    txt_records = []
+    for record in records:
+        txt_records.append(
+            {'content': record.content, 'disabled': record.disabled}
+        )
+
+    # Create RRSet
+    payload = {
+        "rrsets": [
+            {
+                "name": domain_id,
+                "type": "TXT",
+                "ttl": 300,
+                "changetype": "REPLACE",
+                "records": txt_records,
+                "comments": []
+            }
+        ]
+    }
+
+    # Create Txt Records
+    server_id = current_app.config.get("ACME_POWERDNS_SERVERID", "localhost")
+    zone_name = _get_zone_name(domain, account_number)
+    zone_id = zone_name + "."
+    path = f"/api/v1/servers/{server_id}/zones/{zone_id}"
+    _patch(path, payload)
+
+
 def _patch(path, payload):
-    """ Execute a Patch request on the given URL (base_uri + path) with given payload """
+    """
+    Execute a Patch request on the given URL (base_uri + path) with given payload
+
+    :param path:
+    :param payload:
+    :return:
+    """
     base_uri = current_app.config.get("ACME_POWERDNS_DOMAIN")
     verify_value = current_app.config.get("ACME_POWERDNS_VERIFY", True)
     resp = requests.patch(

lemur/plugins/lemur_acme/tests/test_powerdns.py

@@ -48,13 +48,14 @@ class TestPowerdns(unittest.TestCase):
         self.assertEqual(result, zone)
 
     @patch("lemur.plugins.lemur_acme.powerdns.current_app")
-    def test_create_txt_record(self, mock_current_app):
+    def test_create_txt_record_write_only(self, mock_current_app):
         domain = "_acme_challenge.test.example.com"
         zone = "test.example.com"
         token = "ABCDEFGHIJ"
         account_number = "1234567890"
         change_id = (domain, token)
         powerdns._check_conf = Mock()
+        powerdns._get_txt_records = Mock(return_value=[])
         powerdns._get_zone_name = Mock(return_value=zone)
         mock_current_app.logger.debug = Mock()
         mock_current_app.config.get = Mock(return_value="localhost")
@@ -63,24 +64,74 @@ class TestPowerdns(unittest.TestCase):
             "function": "create_txt_record",
             "fqdn": domain,
             "token": token,
-            "message": "TXT record successfully created"
+            "message": "TXT record(s) successfully created"
         }
         result = powerdns.create_txt_record(domain, token, account_number)
         mock_current_app.logger.debug.assert_called_with(log_data)
         self.assertEqual(result, change_id)
 
+    @patch("lemur.plugins.lemur_acme.powerdns.current_app")
+    def test_create_txt_record_append(self, mock_current_app):
+        domain = "_acme_challenge.test.example.com"
+        zone = "test.example.com"
+        token = "ABCDEFGHIJ"
+        account_number = "1234567890"
+        change_id = (domain, token)
+        powerdns._check_conf = Mock()
+        cur_token = "123456"
+        cur_records = [powerdns.Record({'name': domain, 'content': f"\"{cur_token}\"", 'disabled': False})]
+        powerdns._get_txt_records = Mock(return_value=cur_records)
+        powerdns._get_zone_name = Mock(return_value=zone)
+        mock_current_app.logger.debug = Mock()
+        mock_current_app.config.get = Mock(return_value="localhost")
+        powerdns._patch = Mock()
+        log_data = {
+            "function": "create_txt_record",
+            "fqdn": domain,
+            "token": token,
+            "message": "TXT record(s) successfully created"
+        }
+        expected_path = f"/api/v1/servers/localhost/zones/test.example.com."
+        expected_payload = {
+            "rrsets": [
+                {
+                    "name": domain + ".",
+                    "type": "TXT",
+                    "ttl": 300,
+                    "changetype": "REPLACE",
+                    "records": [
+                        {
+                            "content": f"\"{token}\"",
+                            "disabled": False
+                        },
+                        {
+                            "content": f"\"{cur_token}\"",
+                            "disabled": False
+                        }
+                    ],
+                    "comments": []
+                }
+            ]
+        }
+
+        result = powerdns.create_txt_record(domain, token, account_number)
+        mock_current_app.logger.debug.assert_called_with(log_data)
+        powerdns._patch.assert_called_with(expected_path, expected_payload)
+        self.assertEqual(result, change_id)
+
     @patch("lemur.plugins.lemur_acme.powerdns.dnsutil")
     @patch("lemur.plugins.lemur_acme.powerdns.current_app")
     @patch("lemur.extensions.metrics")
     @patch("time.sleep")
     def test_wait_for_dns_change(self, mock_sleep, mock_metrics, mock_current_app, mock_dnsutil):
         domain = "_acme-challenge.test.example.com"
-        token = "ABCDEFG"
+        token1 = "ABCDEFG"
+        token2 = "HIJKLMN"
         zone_name = "test.example.com"
         nameserver = "1.1.1.1"
-        change_id = (domain, token)
+        change_id = (domain, token1)
         powerdns._check_conf = Mock()
-        mock_records = (token,)
+        mock_records = (token2, token1)
         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)
@@ -114,7 +165,7 @@ class TestPowerdns(unittest.TestCase):
             "function": "delete_txt_record",
             "fqdn": domain,
             "token": token,
-            "message": "TXT record successfully deleted"
+            "message": "Unable to delete TXT record: Token not found in existing TXT records"
         }
         powerdns.delete_txt_record(change_id, account_number, domain, token)
         mock_current_app.logger.debug.assert_called_with(log_data)