diff --git a/certbot-route53/certbot_route53/authenticator.py b/certbot-route53/certbot_route53/authenticator.py index 1f1f78bfc..d53fd102f 100644 --- a/certbot-route53/certbot_route53/authenticator.py +++ b/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 diff --git a/certbot-route53/certbot_route53/authenticator_test.py b/certbot-route53/certbot_route53/authenticator_test.py new file mode 100644 index 000000000..545fd01a4 --- /dev/null +++ b/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 diff --git a/tools/venv.sh b/tools/venv.sh index a8332cc6e..75ef017be 100755 --- a/tools/venv.sh +++ b/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 diff --git a/tools/venv3.sh b/tools/venv3.sh index 4d5a07f21..6f5d96262 100755 --- a/tools/venv3.sh +++ b/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 diff --git a/tox.cover.sh b/tox.cover.sh index 51425faf2..eabe6ba7e 100755 --- a/tox.cover.sh +++ b/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 diff --git a/tox.ini b/tox.ini index 6040dd818..414dac790 100644 --- a/tox.ini +++ b/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