Merge pull request #2915 from Netflix/powerdnsplugin_01

fixed get_domains() to remove duplicate entries, updated usage and tests

lemur/plugins/lemur_acme/plugin.py

@@ -254,8 +254,9 @@ class AcmeHandler(object):
 
         domains = [options["common_name"]]
         if options.get("extensions"):
-            for name in options["extensions"]["sub_alt_names"]["names"]:
-                domains.append(name)
+            for dns_name in options["extensions"]["sub_alt_names"]["names"]:
+                if dns_name.value not in domains:
+                    domains.append(dns_name.value)
 
         current_app.logger.debug("Got these domains: {0}".format(domains))
         return domains
@@ -640,15 +641,8 @@ class ACMEIssuerPlugin(IssuerPlugin):
         domains = self.acme.get_domains(issuer_options)
         if not create_immediately:
             # Create pending authorizations that we'll need to do the creation
-            authz_domains = []
-            for d in domains:
-                if type(d) == str:
-                    authz_domains.append(d)
-                else:
-                    authz_domains.append(d.value)
-
             dns_authorization = authorization_service.create(
-                account_number, authz_domains, provider_type
+                account_number, domains, provider_type
             )
             # Return id of the DNS Authorization
             return None, None, dns_authorization.id

lemur/plugins/lemur_acme/tests/test_acme.py

@@ -1,4 +1,6 @@
 import unittest
+
+from cryptography.x509 import DNSName
 from requests.models import Response
 
 from mock import MagicMock, Mock, patch
@@ -74,12 +76,14 @@ class TestAcme(unittest.TestCase):
     @patch("acme.client.Client")
     @patch("lemur.plugins.lemur_acme.plugin.current_app")
     @patch("lemur.plugins.lemur_acme.cloudflare.wait_for_dns_change")
+    @patch("time.sleep")
     def test_complete_dns_challenge_success(
-        self, mock_wait_for_dns_change, mock_current_app, mock_acme
+        self, mock_sleep, mock_wait_for_dns_change, mock_current_app, mock_acme
     ):
         mock_dns_provider = Mock()
         mock_dns_provider.wait_for_dns_change = Mock(return_value=True)
         mock_authz = Mock()
+        mock_sleep.return_value = False
         mock_authz.dns_challenge.response = Mock()
         mock_authz.dns_challenge.response.simple_verify = Mock(return_value=True)
         mock_authz.authz = []
@@ -179,7 +183,7 @@ class TestAcme(unittest.TestCase):
         options = {
             "common_name": "test.netflix.net",
             "extensions": {
-                "sub_alt_names": {"names": ["test2.netflix.net", "test3.netflix.net"]}
+                "sub_alt_names": {"names": [DNSName("test2.netflix.net"), DNSName("test3.netflix.net")]}
             },
         }
         result = self.acme.get_domains(options)
@@ -187,6 +191,19 @@ class TestAcme(unittest.TestCase):
             result, [options["common_name"], "test2.netflix.net", "test3.netflix.net"]
         )
 
+    @patch("lemur.plugins.lemur_acme.plugin.current_app")
+    def test_get_domains_san(self, mock_current_app):
+        options = {
+            "common_name": "test.netflix.net",
+            "extensions": {
+                "sub_alt_names": {"names": [DNSName("test.netflix.net"), DNSName("test2.netflix.net")]}
+            },
+        }
+        result = self.acme.get_domains(options)
+        self.assertEqual(
+            result, [options["common_name"], "test2.netflix.net"]
+        )
+
     @patch(
         "lemur.plugins.lemur_acme.plugin.AcmeHandler.start_dns_challenge",
         return_value="test",