mirror of
https://github.com/certbot/certbot.git
synced 2026-06-14 19:20:09 -04:00
route53: fix error handling (#4760)
Make error handling match other plugins: * Raise `PluginError` instead of errors from underlying libraries * Swallow errors during cleanup
This commit is contained in:
parent
0e4f55982a
commit
2325438b56
2 changed files with 16 additions and 16 deletions
|
|
@ -7,6 +7,7 @@ import zope.interface
|
|||
from acme import challenges
|
||||
from botocore.exceptions import NoCredentialsError, ClientError
|
||||
|
||||
from certbot import errors
|
||||
from certbot import interfaces
|
||||
from certbot.plugins import common
|
||||
|
||||
|
|
@ -59,12 +60,15 @@ class Authenticator(common.Plugin):
|
|||
time.sleep(TTL)
|
||||
return [achall.response(achall.account_key) for achall in achalls]
|
||||
except (NoCredentialsError, ClientError) as e:
|
||||
e.args = ("\n".join([str(e), INSTRUCTIONS]),)
|
||||
raise
|
||||
logger.debug('Encountered error during perform: %s', e, exc_info=True)
|
||||
raise errors.PluginError("\n".join([str(e), INSTRUCTIONS]))
|
||||
|
||||
def cleanup(self, achalls): # pylint: disable=missing-docstring
|
||||
for achall in achalls:
|
||||
self._change_txt_record("DELETE", achall)
|
||||
try:
|
||||
self._change_txt_record("DELETE", achall)
|
||||
except (NoCredentialsError, ClientError) as e:
|
||||
logger.debug('Encountered error during cleanup: %s', e, exc_info=True)
|
||||
|
||||
def _find_zone_id_for_domain(self, domain):
|
||||
"""Find the zone id responsible a given FQDN.
|
||||
|
|
@ -85,7 +89,7 @@ class Authenticator(common.Plugin):
|
|||
zones.append((zone["Name"], zone["Id"]))
|
||||
|
||||
if not zones:
|
||||
raise ValueError(
|
||||
raise errors.PluginError(
|
||||
"Unable to find a Route53 hosted zone for {0}".format(domain)
|
||||
)
|
||||
|
||||
|
|
@ -134,6 +138,6 @@ class Authenticator(common.Plugin):
|
|||
if response["ChangeInfo"]["Status"] == "INSYNC":
|
||||
return
|
||||
time.sleep(5)
|
||||
raise Exception(
|
||||
raise errors.PluginError(
|
||||
"Timed out waiting for Route53 change. Current status: %s" %
|
||||
response["ChangeInfo"]["Status"])
|
||||
|
|
|
|||
|
|
@ -3,9 +3,9 @@
|
|||
import unittest
|
||||
|
||||
import mock
|
||||
|
||||
from botocore.exceptions import NoCredentialsError, ClientError
|
||||
|
||||
from certbot import errors
|
||||
from certbot.plugins import dns_test_common
|
||||
|
||||
|
||||
|
|
@ -36,7 +36,7 @@ class AuthenticatorTest(unittest.TestCase, dns_test_common.BaseAuthenticatorTest
|
|||
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.assertRaises(errors.PluginError,
|
||||
self.auth.perform,
|
||||
[self.achall])
|
||||
|
||||
|
|
@ -44,7 +44,7 @@ class AuthenticatorTest(unittest.TestCase, dns_test_common.BaseAuthenticatorTest
|
|||
self.auth._change_txt_record = mock.MagicMock(
|
||||
side_effect=ClientError({"Error": {"Code": "foo"}}, "bar"))
|
||||
|
||||
self.assertRaises(ClientError, # TODO: Should be `errors.PluginError`
|
||||
self.assertRaises(errors.PluginError,
|
||||
self.auth.perform,
|
||||
[self.achall])
|
||||
|
||||
|
|
@ -58,17 +58,13 @@ class AuthenticatorTest(unittest.TestCase, dns_test_common.BaseAuthenticatorTest
|
|||
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])
|
||||
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])
|
||||
self.auth.cleanup([self.achall])
|
||||
|
||||
|
||||
class ClientTest(unittest.TestCase):
|
||||
|
|
@ -153,7 +149,7 @@ class ClientTest(unittest.TestCase):
|
|||
self.client.r53.get_paginator = mock.MagicMock()
|
||||
self.client.r53.get_paginator().paginate.return_value = []
|
||||
|
||||
self.assertRaises(ValueError, # TODO: Should be `errors.PluginError`
|
||||
self.assertRaises(errors.PluginError,
|
||||
self.client._find_zone_id_for_domain,
|
||||
"foo.example.com")
|
||||
|
||||
|
|
@ -168,7 +164,7 @@ class ClientTest(unittest.TestCase):
|
|||
},
|
||||
]
|
||||
|
||||
self.assertRaises(ValueError, # TODO: Should be `errors.PluginError`
|
||||
self.assertRaises(errors.PluginError,
|
||||
self.client._find_zone_id_for_domain,
|
||||
"foo.example.com")
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue