From becc2c3fee6718e064e09bc9df54af7898439224 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 13 Sep 2024 12:14:49 -0700 Subject: [PATCH] Remove deprecated --dns-route53-propagation-seconds (#10010) * remove dns-route53-prop-secs * document design difference --- .../certbot_dns_route53/__init__.py | 9 ----- .../_internal/dns_route53.py | 36 ++++++++++++------- .../_internal/tests/dns_route53_test.py | 27 +++++++++----- certbot/CHANGELOG.md | 1 + certbot/certbot/plugins/dns_common.py | 4 +++ 5 files changed, 47 insertions(+), 30 deletions(-) diff --git a/certbot-dns-route53/certbot_dns_route53/__init__.py b/certbot-dns-route53/certbot_dns_route53/__init__.py index c7b88658b..a48e3d1a8 100644 --- a/certbot-dns-route53/certbot_dns_route53/__init__.py +++ b/certbot-dns-route53/certbot_dns_route53/__init__.py @@ -101,13 +101,4 @@ Examples --dns-route53 \\ -d example.com \\ -d www.example.com - -.. code-block:: bash - :caption: To acquire a certificate for ``example.com``, waiting 30 seconds - for DNS propagation - - certbot certonly \\ - --dns-route53 \\ - --dns-route53-propagation-seconds 30 \\ - -d example.com """ diff --git a/certbot-dns-route53/certbot_dns_route53/_internal/dns_route53.py b/certbot-dns-route53/certbot_dns_route53/_internal/dns_route53.py index 81a5cc0d8..f61f9a0ed 100644 --- a/certbot-dns-route53/certbot_dns_route53/_internal/dns_route53.py +++ b/certbot-dns-route53/certbot_dns_route53/_internal/dns_route53.py @@ -6,18 +6,20 @@ from typing import Any from typing import Callable from typing import DefaultDict from typing import Dict +from typing import Iterable from typing import List +from typing import Type import boto3 from botocore.exceptions import ClientError from botocore.exceptions import NoCredentialsError -from acme.challenges import ChallengeResponse +from acme import challenges from certbot import achallenges from certbot import errors +from certbot import interfaces from certbot.achallenges import AnnotatedChallenge -from certbot.plugins import dns_common -from certbot.util import add_deprecated_argument +from certbot.plugins import common logger = logging.getLogger(__name__) @@ -27,7 +29,7 @@ INSTRUCTIONS = ( "and add the necessary permissions for Route53 access.") -class Authenticator(dns_common.DNSAuthenticator): +class Authenticator(common.Plugin, interfaces.Authenticator): """Route53 Authenticator This authenticator solves a DNS01 challenge by uploading the answer to AWS @@ -41,6 +43,7 @@ class Authenticator(dns_common.DNSAuthenticator): def __init__(self, *args: Any, **kwargs: Any) -> None: super().__init__(*args, **kwargs) self.r53 = boto3.client("route53") + self._attempt_cleanup = False self._resource_records: DefaultDict[str, List[Dict[str, str]]] = \ collections.defaultdict(list) @@ -48,9 +51,9 @@ class Authenticator(dns_common.DNSAuthenticator): return "Solve a DNS01 challenge using AWS Route53" @classmethod - def add_parser_arguments(cls, add: Callable[..., None], # pylint: disable=arguments-differ - default_propagation_seconds: int = 10) -> None: - add_deprecated_argument(add, 'propagation-seconds', 1) + def add_parser_arguments(cls, add: Callable[..., None]) -> None: + # This authenticator currently adds no extra arguments. + pass def auth_hint(self, failed_achalls: List[achallenges.AnnotatedChallenge]) -> str: return ( @@ -58,13 +61,13 @@ class Authenticator(dns_common.DNSAuthenticator): '--dns-route53. Ensure the above domains have their DNS hosted by AWS Route53.' ) - def _setup_credentials(self) -> None: + def prepare(self) -> None: pass - def _perform(self, domain: str, validation_name: str, validation: str) -> None: - pass + def get_chall_pref(self, unused_domain: str) -> Iterable[Type[challenges.Challenge]]: + return [challenges.DNS01] - def perform(self, achalls: List[AnnotatedChallenge]) -> List[ChallengeResponse]: + def perform(self, achalls: List[AnnotatedChallenge]) -> List[challenges.ChallengeResponse]: self._attempt_cleanup = True try: @@ -82,7 +85,16 @@ class Authenticator(dns_common.DNSAuthenticator): raise errors.PluginError("\n".join([str(e), INSTRUCTIONS])) return [achall.response(achall.account_key) for achall in achalls] - def _cleanup(self, domain: str, validation_name: str, validation: str) -> None: + def cleanup(self, achalls: List[achallenges.AnnotatedChallenge]) -> None: + if self._attempt_cleanup: + for achall in achalls: + domain = achall.domain + validation_domain_name = achall.validation_domain_name(domain) + validation = achall.validation(achall.account_key) + + self._cleanup(validation_domain_name, validation) + + def _cleanup(self, validation_name: str, validation: str) -> None: try: self._change_txt_record("DELETE", validation_name, validation) except (NoCredentialsError, ClientError) as e: diff --git a/certbot-dns-route53/certbot_dns_route53/_internal/tests/dns_route53_test.py b/certbot-dns-route53/certbot_dns_route53/_internal/tests/dns_route53_test.py index 05e22344d..730331236 100644 --- a/certbot-dns-route53/certbot_dns_route53/_internal/tests/dns_route53_test.py +++ b/certbot-dns-route53/certbot_dns_route53/_internal/tests/dns_route53_test.py @@ -6,17 +6,27 @@ from unittest import mock from botocore.exceptions import ClientError from botocore.exceptions import NoCredentialsError +import josepy as jose import pytest +from acme import challenges +from certbot import achallenges from certbot import errors from certbot.compat import os -from certbot.plugins import dns_test_common from certbot.plugins.dns_test_common import DOMAIN +from certbot.tests import acme_util +from certbot.tests import util as test_util + +DOMAIN = 'example.com' +KEY = jose.JWKRSA.load(test_util.load_vector("rsa512_key.pem")) -class AuthenticatorTest(unittest.TestCase, dns_test_common.BaseAuthenticatorTest): +class AuthenticatorTest(unittest.TestCase): # pylint: disable=protected-access + achall = achallenges.KeyAuthorizationAnnotatedChallenge( + challb=acme_util.DNS01, domain=DOMAIN, account_key=KEY) + def setUp(self): from certbot_dns_route53._internal.dns_route53 import Authenticator @@ -35,6 +45,12 @@ class AuthenticatorTest(unittest.TestCase, dns_test_common.BaseAuthenticatorTest del os.environ["AWS_ACCESS_KEY_ID"] del os.environ["AWS_SECRET_ACCESS_KEY"] + def test_more_info(self) -> None: + self.assertTrue(isinstance(self.auth.more_info(), str)) + + def test_get_chall_pref(self) -> None: + self.assertEqual(self.auth.get_chall_pref("example.org"), [challenges.DNS01]) + def test_perform(self): self.auth._change_txt_record = mock.MagicMock() self.auth._wait_for_change = mock.MagicMock() @@ -85,13 +101,6 @@ class AuthenticatorTest(unittest.TestCase, dns_test_common.BaseAuthenticatorTest self.auth.cleanup([self.achall]) - def test_parser_arguments(self) -> None: - from certbot.util import DeprecatedArgumentAction - m = mock.MagicMock() - self.auth.add_parser_arguments(m) # pylint: disable=no-member - m.assert_any_call('propagation-seconds', action=DeprecatedArgumentAction, - help=mock.ANY, nargs=1) - class ClientTest(unittest.TestCase): # pylint: disable=protected-access diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index 0b4876ba9..7b31ceee5 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -14,6 +14,7 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). * The `csr_dir` and `key_dir` attributes on `certbot.configuration.NamespaceConfig` were removed. * The `--manual-public-ip-logging-ok` command line flag was removed. +* The `--dns-route53-propagation-seconds` command line flag was removed. ### Fixed diff --git a/certbot/certbot/plugins/dns_common.py b/certbot/certbot/plugins/dns_common.py index 128024bec..92a8ed8a0 100644 --- a/certbot/certbot/plugins/dns_common.py +++ b/certbot/certbot/plugins/dns_common.py @@ -25,6 +25,10 @@ from certbot.plugins import common logger = logging.getLogger(__name__) +# As of writing this, the only one of our plugins that does not inherit from this class (either +# directly or indirectly through certbot.plugins.dns_common_lexicon.LexiconDNSAuthenticator) is +# certbot-dns-route53. If you are attempting to make changes to all of our DNS plugins, please keep +# this difference in mind. class DNSAuthenticator(common.Plugin, interfaces.Authenticator, metaclass=abc.ABCMeta): """Base class for DNS Authenticators"""