route53: add unit tests (#4725)

This change introduces unit tests to cover all lines of the route53
plugin except for the timeout in `_wait_for_change`.

certbot-route53/certbot_route53/authenticator.py

@@ -1,26 +1,22 @@
 """Certbot Route53 authenticator plugin."""
 import logging
 import time
-import datetime
-
-import zope.interface
 
 import boto3
-from botocore.exceptions import NoCredentialsError, ClientError
-
+import zope.interface
 from acme import challenges
+from botocore.exceptions import NoCredentialsError, ClientError
 
 from certbot import interfaces
 from certbot.plugins import common
 
-
 logger = logging.getLogger(__name__)
 
 TTL = 10
 
 INSTRUCTIONS = (
     "To use certbot-route53, configure credentials as described at "
-    "https://boto3.readthedocs.io/en/latest/guide/configuration.html#best-practices-for-configuring-credentials "
+    "https://boto3.readthedocs.io/en/latest/guide/configuration.html#best-practices-for-configuring-credentials "  # pylint: disable=line-too-long
     "and add the necessary permissions for Route53 access.")
 
 @zope.interface.implementer(interfaces.IAuthenticator)
@@ -94,7 +90,7 @@ class Authenticator(common.Plugin):
 
         if not zones:
             raise ValueError(
-                "Unable to find a Route53 hosted zone for {}".format(domain)
+                "Unable to find a Route53 hosted zone for {0}".format(domain)
             )
 
         # Order the zones that are suffixes for our desired to domain by
@@ -124,7 +120,7 @@ class Authenticator(common.Plugin):
                             "ResourceRecords": [
                                 # For some reason TXT records need to be
                                 # manually quoted.
-                                {"Value": '"{}"'.format(value)}
+                                {"Value": '"{0}"'.format(value)}
                             ],
                         }
                     }
@@ -137,7 +133,7 @@ class Authenticator(common.Plugin):
         """Wait for a change to be propagated to all Route53 DNS servers.
            https://docs.aws.amazon.com/Route53/latest/APIReference/API_GetChange.html
         """
-        for n in range(0, 120):
+        for unused_n in range(0, 120):
             response = self.r53.get_change(Id=change_id)
             if response["ChangeInfo"]["Status"] == "INSYNC":
                 return

certbot-route53/certbot_route53/authenticator_test.py

@@ -0,0 +1,195 @@
+"""Tests for certbot_route53.authenticator"""
+
+import unittest
+
+import mock
+
+from botocore.exceptions import NoCredentialsError, ClientError
+
+from certbot.plugins import dns_test_common
+
+
+class AuthenticatorTest(unittest.TestCase, dns_test_common.BaseAuthenticatorTest):
+    # pylint: disable=protected-access
+
+    def setUp(self):
+        from certbot_route53.authenticator import Authenticator
+
+        super(AuthenticatorTest, self).setUp()
+
+        self.config = mock.MagicMock()
+
+        self.auth = Authenticator(self.config, "route53")
+
+    def test_parser_arguments(self):
+        pass  # TODO: follow convention of defining optional argument for DNS propagation delay
+
+    def test_perform(self):
+        self.auth._change_txt_record = mock.MagicMock()
+        self.auth._wait_for_change = mock.MagicMock()
+
+        self.auth.perform([self.achall])
+
+        self.auth._change_txt_record.assert_called_once_with("UPSERT", self.achall)
+        self.auth._wait_for_change.assert_called_once()
+
+    def test_perform_no_credentials_error(self):
+        self.auth._change_txt_record = mock.MagicMock(side_effect=NoCredentialsError)
+
+        self.assertRaises(NoCredentialsError,  # TODO: Should be `errors.PluginError`
+                          self.auth.perform,
+                          [self.achall])
+
+    def test_perform_client_error(self):
+        self.auth._change_txt_record = mock.MagicMock(
+            side_effect=ClientError({"Error": {"Code": "foo"}}, "bar"))
+
+        self.assertRaises(ClientError,  # TODO: Should be `errors.PluginError`
+                          self.auth.perform,
+                          [self.achall])
+
+    def test_cleanup(self):
+        self.auth._change_txt_record = mock.MagicMock()
+
+        self.auth.cleanup([self.achall])
+
+        self.auth._change_txt_record.assert_called_once_with("DELETE", self.achall)
+
+    def test_cleanup_no_credentials_error(self):
+        self.auth._change_txt_record = mock.MagicMock(side_effect=NoCredentialsError)
+
+        self.assertRaises(NoCredentialsError,  # TODO: Should not raise
+                          self.auth.cleanup,
+                          [self.achall])
+
+    def test_cleanup_client_error(self):
+        self.auth._change_txt_record = mock.MagicMock(
+            side_effect=ClientError({"Error": {"Code": "foo"}}, "bar"))
+
+        self.assertRaises(ClientError,  # TODO: Should not raise
+                          self.auth.cleanup,
+                          [self.achall])
+
+
+class ClientTest(unittest.TestCase):
+    # pylint: disable=protected-access
+
+    PRIVATE_ZONE = {
+                        "Id": "BAD-PRIVATE",
+                        "Name": "example.com",
+                        "Config": {
+                            "PrivateZone": True
+                        }
+                    }
+
+    EXAMPLE_NET_ZONE = {
+                            "Id": "BAD-WRONG-TLD",
+                            "Name": "example.net",
+                            "Config": {
+                                "PrivateZone": False
+                            }
+                        }
+
+    EXAMPLE_COM_ZONE = {
+                            "Id": "EXAMPLE",
+                            "Name": "example.com",
+                            "Config": {
+                                "PrivateZone": False
+                            }
+                        }
+
+    FOO_EXAMPLE_COM_ZONE = {
+                                "Id": "FOO",
+                                "Name": "foo.example.com",
+                                "Config": {
+                                    "PrivateZone": False
+                                }
+                            }
+
+    def setUp(self):
+        from certbot_route53.authenticator import Authenticator
+
+        super(ClientTest, self).setUp()
+
+        self.config = mock.MagicMock()
+
+        self.client = Authenticator(self.config, "route53")
+
+    def test_find_zone_id_for_domain(self):
+        self.client.r53.get_paginator = mock.MagicMock()
+        self.client.r53.get_paginator().paginate.return_value = [
+            {
+                "HostedZones": [
+                    self.EXAMPLE_NET_ZONE,
+                    self.EXAMPLE_COM_ZONE,
+                ]
+            }
+        ]
+
+        result = self.client._find_zone_id_for_domain("foo.example.com")
+        self.assertEqual(result, "EXAMPLE")
+
+    def test_find_zone_id_for_domain_pagination(self):
+        self.client.r53.get_paginator = mock.MagicMock()
+        self.client.r53.get_paginator().paginate.return_value = [
+            {
+                "HostedZones": [
+                    self.PRIVATE_ZONE,
+                    self.EXAMPLE_COM_ZONE,
+                ]
+            },
+            {
+                "HostedZones": [
+                    self.PRIVATE_ZONE,
+                    self.FOO_EXAMPLE_COM_ZONE,
+                ]
+            }
+        ]
+
+        result = self.client._find_zone_id_for_domain("foo.example.com")
+        self.assertEqual(result, "FOO")
+
+    def test_find_zone_id_for_domain_no_results(self):
+        self.client.r53.get_paginator = mock.MagicMock()
+        self.client.r53.get_paginator().paginate.return_value = []
+
+        self.assertRaises(ValueError,  # TODO: Should be `errors.PluginError`
+                          self.client._find_zone_id_for_domain,
+                          "foo.example.com")
+
+    def test_find_zone_id_for_domain_no_correct_results(self):
+        self.client.r53.get_paginator = mock.MagicMock()
+        self.client.r53.get_paginator().paginate.return_value = [
+            {
+                "HostedZones": [
+                    self.PRIVATE_ZONE,
+                    self.EXAMPLE_NET_ZONE,
+                ]
+            },
+        ]
+
+        self.assertRaises(ValueError,  # TODO: Should be `errors.PluginError`
+                          self.client._find_zone_id_for_domain,
+                          "foo.example.com")
+
+    def test_change_txt_record(self):
+        self.client._find_zone_id_for_domain = mock.MagicMock()
+        self.client.r53.change_resource_record_sets = mock.MagicMock(
+            return_value={"ChangeInfo": {"Id": 1}})
+
+        self.client._change_txt_record("FOO", dns_test_common.BaseAuthenticatorTest.achall)
+
+        self.client.r53.change_resource_record_sets.assert_called_once()
+
+    def test_wait_for_change(self):
+        self.client.r53.get_change = mock.MagicMock(
+            side_effect=[{"ChangeInfo": {"Status": "PENDING"}},
+                         {"ChangeInfo": {"Status": "INSYNC"}}])
+
+        self.client._wait_for_change(1)
+
+        self.client.r53.get_change.assert_called()
+
+
+if __name__ == "__main__":
+    unittest.main()  # pragma: no cover

tools/venv.sh

@@ -21,5 +21,6 @@ fi
   -e certbot-dns-google \
   -e certbot-dns-nsone \
   -e certbot-nginx \
+  -e certbot-route53 \
   -e letshelp-certbot \
   -e certbot-compatibility-test

tools/venv3.sh

@@ -20,5 +20,6 @@ fi
   -e certbot-dns-google \
   -e certbot-dns-nsone \
   -e certbot-nginx \
+  -e certbot-route53 \
   -e letshelp-certbot \
   -e certbot-compatibility-test

tox.cover.sh

@@ -9,7 +9,7 @@
 # -e makes sure we fail fast and don't submit coveralls submit
 
 if [ "xxx$1" = "xxx" ]; then
-  pkgs="certbot acme certbot_apache certbot_dns_cloudflare certbot_dns_cloudxns certbot_dns_digitalocean certbot_dns_dnsimple certbot_dns_google certbot_dns_nsone certbot_nginx letshelp_certbot"
+  pkgs="certbot acme certbot_apache certbot_dns_cloudflare certbot_dns_cloudxns certbot_dns_digitalocean certbot_dns_dnsimple certbot_dns_google certbot_dns_nsone certbot_nginx certbot_route53 letshelp_certbot"
 else
   pkgs="$@"
 fi
@@ -35,6 +35,8 @@ cover () {
     min=99
   elif [ "$1" = "certbot_nginx" ]; then
     min=97
+  elif [ "$1" = "certbot_route53" ]; then
+    min=99
   elif [ "$1" = "letshelp_certbot" ]; then
     min=100
   else

tox.ini

@@ -37,8 +37,10 @@ dns_plugin_commands =
     nosetests -v certbot_dns_digitalocean --processes=-1
     pip install -e certbot-dns-google
     nosetests -v certbot_dns_google --processes=-1
-dns_plugin_install_args = -e certbot-dns-cloudflare -e certbot-dns-digitalocean -e certbot-dns-google
-dns_plugin_paths = certbot-dns-cloudflare/certbot_dns_cloudflare certbot-dns-digitalocean/certbot_dns_digitalocean certbot-dns-google/certbot_dns_google
+    pip install -e certbot-route53
+    nosetests -v certbot_route53 --processes=-1 --process-timeout=25
+dns_plugin_install_args = -e certbot-dns-cloudflare -e certbot-dns-digitalocean -e certbot-dns-google -e certbot-route53
+dns_plugin_paths = certbot-dns-cloudflare/certbot_dns_cloudflare certbot-dns-digitalocean/certbot_dns_digitalocean certbot-dns-google/certbot_dns_google certbot-route53/certbot_route53
 
 lexicon_dns_plugin_commands =
     pip install -e certbot-dns-cloudxns