Release 1.20.0

It seems that all required pre-compiled wheels to install Certbot on Python 3.9 on Windows are present.

This PR upgrades Windows tests on Python 3.9 and repackages the installer on this version of Python.

.azure-pipelines/templates/jobs/extended-tests-jobs.yml

@@ -79,18 +79,15 @@ jobs:
           TOXENV: integration-dns-rfc2136
         docker-dev:
           TOXENV: docker_dev
+        le-modification:
+          IMAGE_NAME: ubuntu-18.04
+          TOXENV: modification
         macos-farmtest-apache2:
           # We run one of these test farm tests on macOS to help ensure the
           # tests continue to work on the platform.
           IMAGE_NAME: macOS-10.15
           PYTHON_VERSION: 3.8
           TOXENV: test-farm-apache2
-        farmtest-leauto-upgrades:
-          PYTHON_VERSION: 3.7
-          TOXENV: test-farm-leauto-upgrades
-        farmtest-certonly-standalone:
-          PYTHON_VERSION: 3.7
-          TOXENV: test-farm-certonly-standalone
         farmtest-sdists:
           PYTHON_VERSION: 3.7
           TOXENV: test-farm-sdists

.azure-pipelines/templates/jobs/packaging-jobs.yml

@@ -59,7 +59,7 @@ jobs:
     steps:
       - task: UsePythonVersion@0
         inputs:
-          versionSpec: 3.8
+          versionSpec: 3.9
           architecture: x86
           addToPath: true
       - script: |
@@ -100,7 +100,7 @@ jobs:
         displayName: Check Powershell 5.x is used in vs2017-win2016
       - task: UsePythonVersion@0
         inputs:
-          versionSpec: 3.8
+          versionSpec: 3.9
           addToPath: true
       - task: DownloadPipelineArtifact@2
         inputs:

.azure-pipelines/templates/jobs/standard-tests-jobs.yml

@@ -15,14 +15,14 @@ jobs:
         windows-py36:
           IMAGE_NAME: vs2017-win2016
           PYTHON_VERSION: 3.6
-          TOXENV: py36
-        windows-py38-cover:
+          TOXENV: py36-win
+        windows-py39-cover:
           IMAGE_NAME: vs2017-win2016
-          PYTHON_VERSION: 3.8
-          TOXENV: py38-cover
+          PYTHON_VERSION: 3.9
+          TOXENV: py39-cover-win
         windows-integration-certbot:
           IMAGE_NAME: vs2017-win2016
-          PYTHON_VERSION: 3.8
+          PYTHON_VERSION: 3.9
           TOXENV: integration-certbot
         linux-oldest-tests-1:
           IMAGE_NAME: ubuntu-18.04
@@ -43,11 +43,11 @@ jobs:
         linux-py39-lint:
           IMAGE_NAME: ubuntu-18.04
           PYTHON_VERSION: 3.9
-          TOXENV: lint
+          TOXENV: lint-posix
         linux-py39-mypy:
           IMAGE_NAME: ubuntu-18.04
           PYTHON_VERSION: 3.9
-          TOXENV: mypy
+          TOXENV: mypy-posix
         linux-integration:
           IMAGE_NAME: ubuntu-18.04
           PYTHON_VERSION: 3.8
@@ -56,9 +56,6 @@ jobs:
         apache-compat:
           IMAGE_NAME: ubuntu-18.04
           TOXENV: apache_compat
-        le-modification:
-          IMAGE_NAME: ubuntu-18.04
-          TOXENV: modification
         apacheconftest:
           IMAGE_NAME: ubuntu-18.04
           PYTHON_VERSION: 3.6

.azure-pipelines/templates/stages/deploy-stage.yml

@@ -19,11 +19,12 @@ stages:
       # Then the file was added as a secure file in Azure pipelines
       # with the name snapcraft.cfg by following the instructions at
       # https://docs.microsoft.com/en-us/azure/devops/pipelines/library/secure-files?view=azure-devops
-      # including authorizing the file in all pipelines as described at
-      # https://docs.microsoft.com/en-us/azure/devops/pipelines/library/secure-files?view=azure-devops#how-do-i-authorize-a-secure-file-for-use-in-all-pipelines.
+      # including authorizing the file for use in the "nightly" and "release"
+      # pipelines as described at
+      # https://docs.microsoft.com/en-us/azure/devops/pipelines/library/secure-files?view=azure-devops#q-how-do-i-authorize-a-secure-file-for-use-in-a-specific-pipeline.
       #
       # This file has a maximum lifetime of one year and the current
-      # file will expire on 2021-07-28 which is also tracked by
+      # file will expire on 2022-07-25 which is also tracked by
       # https://github.com/certbot/certbot/issues/7931. The file will
       # need to be updated before then to prevent automated deploys
       # from breaking.

.azure-pipelines/templates/steps/sphinx-steps.yml

@@ -1,15 +1,16 @@
 steps:
   - bash: |
+      set -e
+      sudo apt-get update
+      sudo apt-get install -y --no-install-recommends libaugeas0
       FINAL_STATUS=0
       declare -a FAILED_BUILDS
-      python3 -m venv .venv
-      source .venv/bin/activate
-      python tools/pipstrap.py
+      tools/venv.py
+      source venv/bin/activate
       for doc_path in */docs
       do
         echo ""
         echo "##[group]Building $doc_path"
-        pip install -q -e $doc_path/..[docs]
         if ! sphinx-build -W --keep-going -b html $doc_path $doc_path/_build/html; then
           FINAL_STATUS=1
           FAILED_BUILDS[${#FAILED_BUILDS[@]}]="${doc_path%/docs}"

.azure-pipelines/templates/steps/tox-steps.yml

@@ -1,6 +1,10 @@
 steps:
+  # We run brew update because we've seen attempts to install an older version
+  # of a package fail. See
+  # https://github.com/actions/virtual-environments/issues/3165.
   - bash: |
       set -e
+      brew update
       brew install augeas
     condition: startswith(variables['IMAGE_NAME'], 'macOS')
     displayName: Install MacOS dependencies

.github/FUNDING.yml

@@ -0,0 +1 @@
+custom: https://supporters.eff.org/donate/support-work-on-certbot

.gitignore

@@ -4,13 +4,11 @@
 build/
 dist*/
 /venv*/
-/kgs/
 /.tox/
 /releases*/
 /log*
 letsencrypt.log
 certbot.log
-letsencrypt-auto-source/letsencrypt-auto.sig.lzma.base64
 poetry.lock
 
 # coverage
@@ -32,12 +30,6 @@ tags
 # auth --cert-path --chain-path
 /*.pem
 
-# letstest
-tests/letstest/letest-*/
-tests/letstest/*.pem
-tests/letstest/venv/
-tests/letstest/venv3/
-
 .venv
 
 # pytest cache

.isort.cfg

@@ -1,6 +1,5 @@
 [settings]
 skip_glob=venv*
-skip=letsencrypt-auto-source
 force_sort_within_sections=True
 force_single_line=True
 order_by_type=False

.pylintrc

@@ -56,7 +56,23 @@ extension-pkg-whitelist=pywintypes,win32api,win32file,win32security
 #    See https://github.com/PyCQA/pylint/issues/1498.
 # 3) Same as point 2 for no-value-for-parameter.
 #    See https://github.com/PyCQA/pylint/issues/2820.
-disable=fixme,locally-disabled,locally-enabled,bad-continuation,no-self-use,invalid-name,cyclic-import,duplicate-code,design,import-outside-toplevel,useless-object-inheritance,unsubscriptable-object,no-value-for-parameter,no-else-return,no-else-raise,no-else-break,no-else-continue
+# 4) raise-missing-from makes it an error to raise an exception from except
+#    block without using explicit exception chaining. While explicit exception
+#    chaining results in a slightly more informative traceback, I don't think
+#    it's beneficial enough for us to change all of our current instances and
+#    give Certbot developers errors about this when they're working on new code
+#    in the future. You can read more about exception chaining and this pylint
+#    check at
+#    https://blog.ram.rachum.com/post/621791438475296768/improving-python-exception-chaining-with.
+# 5) wrong-import-order generates false positives and a pylint developer
+#    suggests that people using isort should disable this check at
+#    https://github.com/PyCQA/pylint/issues/3817#issuecomment-687892090.
+# 6) unspecified-encoding generates errors when encoding is not specified in
+#    in a call to the built-in open function. This relates more to a design decision
+#    (unspecified encoding makes the open function use the default encoding of the system)
+#    than a clear flaw on which a check should be enforced. Anyway the project does
+#    not need to enforce encoding on files so we disable this check.
+disable=fixme,locally-disabled,locally-enabled,bad-continuation,no-self-use,invalid-name,cyclic-import,duplicate-code,design,import-outside-toplevel,useless-object-inheritance,unsubscriptable-object,no-value-for-parameter,no-else-return,no-else-raise,no-else-break,no-else-continue,raise-missing-from,wrong-import-order,unspecified-encoding
 
 [REPORTS]
 

Dockerfile-dev

@@ -1,5 +1,5 @@
 # This Dockerfile builds an image for development.
-FROM debian:buster
+FROM ubuntu:focal
 
 # Note: this only exposes the port to other docker containers.
 EXPOSE 80 443
@@ -8,8 +8,9 @@ WORKDIR /opt/certbot/src
 
 COPY . .
 RUN apt-get update && \
-    apt-get install apache2 git python3-dev python3-venv gcc libaugeas0 \
-                    libssl-dev libffi-dev ca-certificates openssl nginx-light -y && \
+    DEBIAN_FRONTEND=noninteractive apt-get install apache2 git python3-dev \
+        python3-venv gcc libaugeas0 libssl-dev libffi-dev ca-certificates \
+        openssl nginx-light -y --no-install-recommends && \
     apt-get clean && \
     rm -rf /var/lib/apt/lists/* \
            /tmp/* \

acme/acme/challenges.py

@@ -7,10 +7,10 @@ import logging
 import socket
 from typing import Type
 
-from cryptography.hazmat.primitives import hashes  # type: ignore
+from cryptography.hazmat.primitives import hashes
 import josepy as jose
 from OpenSSL import crypto
-from OpenSSL import SSL  # type: ignore # https://github.com/python/typeshed/issues/2052
+from OpenSSL import SSL
 import requests
 
 from acme import crypto_util
@@ -30,7 +30,7 @@ class Challenge(jose.TypedJSONObjectWithFields):
     @classmethod
     def from_json(cls, jobj):
         try:
-            return super(Challenge, cls).from_json(jobj)
+            return super().from_json(jobj)
         except jose.UnrecognizedTypeError as error:
             logger.debug(error)
             return UnrecognizedChallenge.from_json(jobj)
@@ -58,7 +58,7 @@ class UnrecognizedChallenge(Challenge):
     """
 
     def __init__(self, jobj):
-        super(UnrecognizedChallenge, self).__init__()
+        super().__init__()
         object.__setattr__(self, "jobj", jobj)
 
     def to_partial_json(self):
@@ -141,7 +141,7 @@ class KeyAuthorizationChallengeResponse(ChallengeResponse):
         return True
 
     def to_partial_json(self):
-        jobj = super(KeyAuthorizationChallengeResponse, self).to_partial_json()
+        jobj = super().to_partial_json()
         jobj.pop('keyAuthorization', None)
         return jobj
 
@@ -314,6 +314,15 @@ class HTTP01Response(KeyAuthorizationChallengeResponse):
         except requests.exceptions.RequestException as error:
             logger.error("Unable to reach %s: %s", uri, error)
             return False
+        # By default, http_response.text will try to guess the encoding to use
+        # when decoding the response to Python unicode strings. This guesswork
+        # is error prone. RFC 8555 specifies that HTTP-01 responses should be
+        # key authorizations with possible trailing whitespace. Since key
+        # authorizations must be composed entirely of the base64url alphabet
+        # plus ".", we tell requests that the response should be ASCII. See
+        # https://datatracker.ietf.org/doc/html/rfc8555#section-8.3 for more
+        # info.
+        http_response.encoding = "ascii"
         logger.debug("Received %s: %s. Headers: %s", http_response,
                      http_response.text, http_response.headers)
 

acme/acme/client.py

@@ -1,4 +1,7 @@
 """ACME client API."""
+# pylint: disable=too-many-lines
+# This pylint disable can be deleted once the deprecated ACMEv1 code is
+# removed.
 import base64
 import collections
 import datetime
@@ -7,13 +10,16 @@ import heapq
 import http.client as http_client
 import logging
 import re
+import sys
 import time
+from types import ModuleType
 from typing import cast
 from typing import Dict
 from typing import List
 from typing import Set
 from typing import Text
 from typing import Union
+import warnings
 
 import josepy as jose
 import OpenSSL
@@ -224,6 +230,9 @@ class ClientBase:
 class Client(ClientBase):
     """ACME client for a v1 API.
 
+    .. deprecated:: 1.18.0
+       Use :class:`ClientV2` instead.
+
     .. todo::
        Clean up raised error types hierarchy, document, and handle (wrap)
        instances of `.DeserializationError` raised in `from_json()`.
@@ -253,7 +262,7 @@ class Client(ClientBase):
         if isinstance(directory, str):
             directory = messages.Directory.from_json(
                 net.get(directory).json())
-        super(Client, self).__init__(directory=directory,
+        super().__init__(directory=directory,
             net=net, acme_version=1)
 
     def register(self, new_reg=None):
@@ -577,7 +586,7 @@ class ClientV2(ClientBase):
         :param .messages.Directory directory: Directory Resource
         :param .ClientNetwork net: Client network.
         """
-        super(ClientV2, self).__init__(directory=directory,
+        super().__init__(directory=directory,
             net=net, acme_version=2)
 
     def new_account(self, new_account):
@@ -627,7 +636,7 @@ class ClientV2(ClientBase):
         """
         # https://github.com/certbot/certbot/issues/6155
         new_regr = self._get_v2_account(regr)
-        return super(ClientV2, self).update_registration(new_regr, update)
+        return super().update_registration(new_regr, update)
 
     def _get_v2_account(self, regr):
         self.net.account = None
@@ -649,16 +658,23 @@ class ClientV2(ClientBase):
         csr = OpenSSL.crypto.load_certificate_request(OpenSSL.crypto.FILETYPE_PEM, csr_pem)
         # pylint: disable=protected-access
         dnsNames = crypto_util._pyopenssl_cert_or_req_all_names(csr)
-
+        ipNames = crypto_util._pyopenssl_cert_or_req_san_ip(csr)
+        # ipNames is now []string
         identifiers = []
         for name in dnsNames:
             identifiers.append(messages.Identifier(typ=messages.IDENTIFIER_FQDN,
                 value=name))
+        for ips in ipNames:
+            identifiers.append(messages.Identifier(typ=messages.IDENTIFIER_IP,
+                value=ips))
         order = messages.NewOrder(identifiers=identifiers)
         response = self._post(self.directory['newOrder'], order)
         body = messages.Order.from_json(response.json())
         authorizations = []
-        for url in body.authorizations:
+        # pylint has trouble understanding our josepy based objects which use
+        # things like custom metaclass logic. body.authorizations should be a
+        # list of strings containing URLs so let's disable this check here.
+        for url in body.authorizations:  # pylint: disable=not-an-iterable
             authorizations.append(self._authzr_from_response(self._post_as_get(url), uri=url))
         return messages.OrderResource(
             body=body,
@@ -802,6 +818,9 @@ class BackwardsCompatibleClientV2:
     """ACME client wrapper that tends towards V2-style calls, but
     supports V1 servers.
 
+    .. deprecated:: 1.18.0
+       Use :class:`ClientV2` instead.
+
     .. note:: While this class handles the majority of the differences
         between versions of the ACME protocol, if you need to support an
         ACME server based on version 3 or older of the IETF ACME draft
@@ -1135,13 +1154,23 @@ class ClientNetwork:
             host, path, _err_no, err_msg = m.groups()
             raise ValueError("Requesting {0}{1}:{2}".format(host, path, err_msg))
 
-        # If content is DER, log the base64 of it instead of raw bytes, to keep
-        # binary data out of the logs.
+        # If the Content-Type is DER or an Accept header was sent in the
+        # request, the response may not be UTF-8 encoded. In this case, we
+        # don't set response.encoding and log the base64 response instead of
+        # raw bytes to keep binary data out of the logs. This code can be
+        # simplified to only check for an Accept header in the request when
+        # ACMEv1 support is dropped.
         debug_content: Union[bytes, str]
-        if response.headers.get("Content-Type") == DER_CONTENT_TYPE:
+        if (response.headers.get("Content-Type") == DER_CONTENT_TYPE or
+                "Accept" in kwargs["headers"]):
             debug_content = base64.b64encode(response.content)
         else:
-            debug_content = response.content.decode("utf-8")
+            # We set response.encoding so response.text knows the response is
+            # UTF-8 encoded instead of trying to guess the encoding that was
+            # used which is error prone. This setting affects all future
+            # accesses of .text made on the returned response object as well.
+            response.encoding = "utf-8"
+            debug_content = response.text
         logger.debug('Received response:\nHTTP %d\n%s\n\n%s',
                      response.status_code,
                      "\n".join("{0}: {1}".format(k, v)
@@ -1211,3 +1240,35 @@ class ClientNetwork:
         response = self._check_response(response, content_type=content_type)
         self._add_nonce(response)
         return response
+
+
+# This class takes a similar approach to the cryptography project to deprecate attributes
+# in public modules. See the _ModuleWithDeprecation class here:
+# https://github.com/pyca/cryptography/blob/91105952739442a74582d3e62b3d2111365b0dc7/src/cryptography/utils.py#L129
+class _ClientDeprecationModule:
+    """
+    Internal class delegating to a module, and displaying warnings when attributes
+    related to deprecated attributes in the acme.client module.
+    """
+    def __init__(self, module):
+        self.__dict__['_module'] = module
+
+    def __getattr__(self, attr):
+        if attr in ('Client', 'BackwardsCompatibleClientV2'):
+            warnings.warn('The {0} attribute in acme.client is deprecated '
+                          'and will be removed soon.'.format(attr),
+                          DeprecationWarning, stacklevel=2)
+        return getattr(self._module, attr)
+
+    def __setattr__(self, attr, value):  # pragma: no cover
+        setattr(self._module, attr, value)
+
+    def __delattr__(self, attr):  # pragma: no cover
+        delattr(self._module, attr)
+
+    def __dir__(self):  # pragma: no cover
+        return ['_module'] + dir(self._module)
+
+
+# Patching ourselves to warn about deprecation and planned removal of some elements in the module.
+sys.modules[__name__] = cast(ModuleType, _ClientDeprecationModule(sys.modules[__name__]))

acme/acme/crypto_util.py

@@ -11,7 +11,7 @@ from typing import Union
 
 import josepy as jose
 from OpenSSL import crypto
-from OpenSSL import SSL  # type: ignore # https://github.com/python/typeshed/issues/2052
+from OpenSSL import SSL
 
 from acme import errors
 
@@ -24,7 +24,7 @@ logger = logging.getLogger(__name__)
 # https://www.openssl.org/docs/ssl/SSLv23_method.html). _serve_sni
 # should be changed to use "set_options" to disable SSLv2 and SSLv3,
 # in case it's used for things other than probing/serving!
-_DEFAULT_SSL_METHOD = SSL.SSLv23_METHOD  # type: ignore
+_DEFAULT_SSL_METHOD = SSL.SSLv23_METHOD
 
 
 class _DefaultCertSelection:
@@ -169,7 +169,7 @@ def probe_sni(name, host, port=443, timeout=300, # pylint: disable=too-many-argu
             ) if any(source_address) else ""
         )
         socket_tuple: Tuple[str, int] = (host, port)
-        sock = socket.create_connection(socket_tuple, **socket_kwargs)  # type: ignore
+        sock = socket.create_connection(socket_tuple, **socket_kwargs)
     except socket.error as error:
         raise errors.Error(error)
 
@@ -187,23 +187,42 @@ def probe_sni(name, host, port=443, timeout=300, # pylint: disable=too-many-argu
     return client_ssl.get_peer_certificate()
 
 
-def make_csr(private_key_pem, domains, must_staple=False):
-    """Generate a CSR containing a list of domains as subjectAltNames.
+def make_csr(private_key_pem, domains=None, must_staple=False, ipaddrs=None):
+    """Generate a CSR containing domains or IPs as subjectAltNames.
 
     :param buffer private_key_pem: Private key, in PEM PKCS#8 format.
     :param list domains: List of DNS names to include in subjectAltNames of CSR.
     :param bool must_staple: Whether to include the TLS Feature extension (aka
         OCSP Must Staple: https://tools.ietf.org/html/rfc7633).
+    :param list ipaddrs: List of IPaddress(type ipaddress.IPv4Address or ipaddress.IPv6Address)
+    names to include in subbjectAltNames of CSR.
+    params ordered this way for backward competablity when called by positional argument.
     :returns: buffer PEM-encoded Certificate Signing Request.
     """
     private_key = crypto.load_privatekey(
         crypto.FILETYPE_PEM, private_key_pem)
     csr = crypto.X509Req()
+    sanlist = []
+    # if domain or ip list not supplied make it empty list so it's easier to iterate
+    if domains is None:
+        domains = []
+    if ipaddrs is None:
+        ipaddrs = []
+    if len(domains)+len(ipaddrs) == 0:
+        raise ValueError("At least one of domains or ipaddrs parameter need to be not empty")
+    for address in domains:
+        sanlist.append('DNS:' + address)
+    for ips in ipaddrs:
+        sanlist.append('IP:' + ips.exploded)
+    # make sure its ascii encoded
+    san_string = ', '.join(sanlist).encode('ascii')
+    # for IP san it's actually need to be octet-string,
+    # but somewhere downsteam thankfully handle it for us
     extensions = [
         crypto.X509Extension(
             b'subjectAltName',
             critical=False,
-            value=', '.join('DNS:' + d for d in domains).encode('ascii')
+            value=san_string
         ),
     ]
     if must_staple:
@@ -220,6 +239,7 @@ def make_csr(private_key_pem, domains, must_staple=False):
 
 
 def _pyopenssl_cert_or_req_all_names(loaded_cert_or_req):
+    # unlike its name this only outputs DNS names, other type of idents will ignored
     common_name = loaded_cert_or_req.get_subject().CN
     sans = _pyopenssl_cert_or_req_san(loaded_cert_or_req)
 
@@ -239,20 +259,56 @@ def _pyopenssl_cert_or_req_san(cert_or_req):
     :param cert_or_req: Certificate or CSR.
     :type cert_or_req: `OpenSSL.crypto.X509` or `OpenSSL.crypto.X509Req`.
 
-    :returns: A list of Subject Alternative Names.
+    :returns: A list of Subject Alternative Names that is DNS.
+    :rtype: `list` of `unicode`
+
+    """
+    # This function finds SANs with dns name
+
+    # constants based on PyOpenSSL certificate/CSR text dump
+    part_separator = ":"
+    prefix = "DNS" + part_separator
+
+    sans_parts = _pyopenssl_extract_san_list_raw(cert_or_req)
+
+    return [part.split(part_separator)[1]
+            for part in sans_parts if part.startswith(prefix)]
+
+
+def _pyopenssl_cert_or_req_san_ip(cert_or_req):
+    """Get Subject Alternative Names IPs from certificate or CSR using pyOpenSSL.
+
+    :param cert_or_req: Certificate or CSR.
+    :type cert_or_req: `OpenSSL.crypto.X509` or `OpenSSL.crypto.X509Req`.
+
+    :returns: A list of Subject Alternative Names that are IP Addresses.
+    :rtype: `list` of `unicode`. note that this returns as string, not IPaddress object
+
+    """
+
+    # constants based on PyOpenSSL certificate/CSR text dump
+    part_separator = ":"
+    prefix = "IP Address" + part_separator
+
+    sans_parts = _pyopenssl_extract_san_list_raw(cert_or_req)
+
+    return [part[len(prefix):] for part in sans_parts if part.startswith(prefix)]
+
+
+def _pyopenssl_extract_san_list_raw(cert_or_req):
+    """Get raw SAN string from cert or csr, parse it as UTF-8 and return.
+
+    :param cert_or_req: Certificate or CSR.
+    :type cert_or_req: `OpenSSL.crypto.X509` or `OpenSSL.crypto.X509Req`.
+
+    :returns: raw san strings, parsed byte as utf-8
     :rtype: `list` of `unicode`
 
     """
     # This function finds SANs by dumping the certificate/CSR to text and
     # searching for "X509v3 Subject Alternative Name" in the text. This method
-    # is used to support PyOpenSSL version 0.13 where the
-    # `_subjectAltNameString` and `get_extensions` methods are not available
-    # for CSRs.
-
-    # constants based on PyOpenSSL certificate/CSR text dump
-    part_separator = ":"
-    parts_separator = ", "
-    prefix = "DNS" + part_separator
+    # is used to because in PyOpenSSL version <0.17 `_subjectAltNameString` methods are
+    # not able to Parse IP Addresses in subjectAltName string.
 
     if isinstance(cert_or_req, crypto.X509):
         # pylint: disable=line-too-long
@@ -262,17 +318,17 @@ def _pyopenssl_cert_or_req_san(cert_or_req):
     text = func(crypto.FILETYPE_TEXT, cert_or_req).decode("utf-8")
     # WARNING: this function does not support multiple SANs extensions.
     # Multiple X509v3 extensions of the same type is disallowed by RFC 5280.
-    match = re.search(r"X509v3 Subject Alternative Name:(?: critical)?\s*(.*)", text)
+    raw_san = re.search(r"X509v3 Subject Alternative Name:(?: critical)?\s*(.*)", text)
+
+    parts_separator = ", "
     # WARNING: this function assumes that no SAN can include
     # parts_separator, hence the split!
-    sans_parts = [] if match is None else match.group(1).split(parts_separator)
-
-    return [part.split(part_separator)[1]
-            for part in sans_parts if part.startswith(prefix)]
+    sans_parts = [] if raw_san is None else raw_san.group(1).split(parts_separator)
+    return sans_parts
 
 
-def gen_ss_cert(key, domains, not_before=None,
-                validity=(7 * 24 * 60 * 60), force_san=True, extensions=None):
+def gen_ss_cert(key, domains=None, not_before=None,
+                validity=(7 * 24 * 60 * 60), force_san=True, extensions=None, ips=None):
     """Generate new self-signed certificate.
 
     :type domains: `list` of `unicode`
@@ -280,6 +336,7 @@ def gen_ss_cert(key, domains, not_before=None,
     :param bool force_san:
     :param extensions: List of additional extensions to include in the cert.
     :type extensions: `list` of `OpenSSL.crypto.X509Extension`
+    :type ips: `list` of (`ipaddress.IPv4Address` or `ipaddress.IPv6Address`)
 
     If more than one domain is provided, all of the domains are put into
     ``subjectAltName`` X.509 extension and first domain is set as the
@@ -287,28 +344,39 @@ def gen_ss_cert(key, domains, not_before=None,
     extension is used, unless `force_san` is ``True``.
 
     """
-    assert domains, "Must provide one or more hostnames for the cert."
+    assert domains or ips, "Must provide one or more hostnames or IPs for the cert."
+
     cert = crypto.X509()
     cert.set_serial_number(int(binascii.hexlify(os.urandom(16)), 16))
     cert.set_version(2)
 
     if extensions is None:
         extensions = []
-
+    if domains is None:
+        domains = []
+    if ips is None:
+        ips = []
     extensions.append(
         crypto.X509Extension(
             b"basicConstraints", True, b"CA:TRUE, pathlen:0"),
     )
 
-    cert.get_subject().CN = domains[0]
+    if len(domains) > 0:
+        cert.get_subject().CN = domains[0]
     # TODO: what to put into cert.get_subject()?
     cert.set_issuer(cert.get_subject())
 
-    if force_san or len(domains) > 1:
+    sanlist = []
+    for address in domains:
+        sanlist.append('DNS:' + address)
+    for ip in ips:
+        sanlist.append('IP:' + ip.exploded)
+    san_string = ', '.join(sanlist).encode('ascii')
+    if force_san or len(domains) > 1 or len(ips) > 0:
         extensions.append(crypto.X509Extension(
             b"subjectAltName",
             critical=False,
-            value=b", ".join(b"DNS:" + d.encode() for d in domains)
+            value=san_string
         ))
 
     cert.add_extensions(extensions)

acme/acme/errors.py

@@ -29,7 +29,7 @@ class NonceError(ClientError):
 class BadNonce(NonceError):
     """Bad nonce error."""
     def __init__(self, nonce, error, *args):
-        super(BadNonce, self).__init__(*args)
+        super().__init__(*args)
         self.nonce = nonce
         self.error = error
 
@@ -48,7 +48,7 @@ class MissingNonce(NonceError):
 
     """
     def __init__(self, response, *args):
-        super(MissingNonce, self).__init__(*args)
+        super().__init__(*args)
         self.response = response
 
     def __str__(self):
@@ -72,7 +72,7 @@ class PollError(ClientError):
     def __init__(self, exhausted, updated):
         self.exhausted = exhausted
         self.updated = updated
-        super(PollError, self).__init__()
+        super().__init__()
 
     @property
     def timeout(self):
@@ -90,7 +90,7 @@ class ValidationError(Error):
     """
     def __init__(self, failed_authzrs):
         self.failed_authzrs = failed_authzrs
-        super(ValidationError, self).__init__()
+        super().__init__()
 
 
 class TimeoutError(Error):  # pylint: disable=redefined-builtin
@@ -106,7 +106,7 @@ class IssuanceError(Error):
         :param messages.Error error: The error provided by the server.
         """
         self.error = error
-        super(IssuanceError, self).__init__()
+        super().__init__()
 
 
 class ConflictError(ClientError):
@@ -119,7 +119,7 @@ class ConflictError(ClientError):
     """
     def __init__(self, location):
         self.location = location
-        super(ConflictError, self).__init__()
+        super().__init__()
 
 
 class WildcardUnsupportedError(Error):

acme/acme/fields.py

@@ -12,7 +12,7 @@ class Fixed(jose.Field):
 
     def __init__(self, json_name, value):
         self.value = value
-        super(Fixed, self).__init__(
+        super().__init__(
             json_name=json_name, default=value, omitempty=False)
 
     def decode(self, value):
@@ -53,7 +53,7 @@ class Resource(jose.Field):
 
     def __init__(self, resource_type, *args, **kwargs):
         self.resource_type = resource_type
-        super(Resource, self).__init__(
+        super().__init__(
             'resource', default=resource_type, *args, **kwargs)
 
     def decode(self, value):

acme/acme/jws.py

@@ -50,7 +50,7 @@ class JWS(jose.JWS):
         # Per ACME spec, jwk and kid are mutually exclusive, so only include a
         # jwk field if kid is not provided.
         include_jwk = kid is None
-        return super(JWS, cls).sign(payload, key=key, alg=alg,
+        return super().sign(payload, key=key, alg=alg,
                                     protect=frozenset(['nonce', 'url', 'kid', 'jwk', 'alg']),
                                     nonce=nonce, url=url, kid=kid,
                                     include_jwk=include_jwk)

acme/acme/magic_typing.py

@@ -6,7 +6,7 @@ available. This code is being kept for now for backwards compatibility.
 """
 import warnings
 from typing import *  # pylint: disable=wildcard-import, unused-wildcard-import
-from typing import Collection, IO  # type: ignore
+from typing import Collection, IO
 
 warnings.warn("acme.magic_typing is deprecated and will be removed in a future release.",
               DeprecationWarning)

acme/acme/messages.py

@@ -114,7 +114,7 @@ class Error(jose.JSONObjectWithFields, errors.Error):
         :rtype: unicode
 
         """
-        code = str(self.typ).split(':')[-1]
+        code = str(self.typ).rsplit(':', maxsplit=1)[-1]
         if code in ERROR_CODES:
             return code
         return None
@@ -126,13 +126,13 @@ class Error(jose.JSONObjectWithFields, errors.Error):
             if part is not None).decode()
 
 
-class _Constant(jose.JSONDeSerializable, Hashable):  # type: ignore
+class _Constant(jose.JSONDeSerializable, Hashable):
     """ACME constant."""
     __slots__ = ('name',)
     POSSIBLE_NAMES: Dict[str, '_Constant'] = NotImplemented
 
     def __init__(self, name):
-        super(_Constant, self).__init__()
+        super().__init__()
         self.POSSIBLE_NAMES[name] = self  # pylint: disable=unsupported-assignment-operation
         self.name = name
 
@@ -172,7 +172,9 @@ STATUS_DEACTIVATED = Status('deactivated')
 class IdentifierType(_Constant):
     """ACME identifier type."""
     POSSIBLE_NAMES: Dict[str, 'IdentifierType'] = {}
+    # class def ends here
 IDENTIFIER_FQDN = IdentifierType('dns')  # IdentifierDNS in Boulder
+IDENTIFIER_IP = IdentifierType('ip') # IdentifierIP in pebble - not in Boulder yet
 
 
 class Identifier(jose.JSONObjectWithFields):
@@ -201,7 +203,7 @@ class Directory(jose.JSONDeSerializable):
 
         def __init__(self, **kwargs):
             kwargs = {self._internal_name(k): v for k, v in kwargs.items()}
-            super(Directory.Meta, self).__init__(**kwargs)
+            super().__init__(**kwargs)
 
         @property
         def terms_of_service(self):
@@ -211,7 +213,7 @@ class Directory(jose.JSONDeSerializable):
         def __iter__(self):
             # When iterating over fields, use the external name 'terms_of_service' instead of
             # the internal '_terms_of_service'.
-            for name in super(Directory.Meta, self).__iter__():
+            for name in super().__iter__():
                 yield name[1:] if name == '_terms_of_service' else name
 
         def _internal_name(self, name):
@@ -357,7 +359,7 @@ class Registration(ResourceBody):
         if 'contact' in kwargs:
             # Avoid the __setattr__ used by jose.TypedJSONObjectWithFields
             object.__setattr__(self, '_add_contact', True)
-        super(Registration, self).__init__(**kwargs)
+        super().__init__(**kwargs)
 
     def _filter_contact(self, prefix):
         return tuple(
@@ -383,12 +385,12 @@ class Registration(ResourceBody):
 
     def to_partial_json(self):
         """Modify josepy.JSONDeserializable.to_partial_json()"""
-        jobj = super(Registration, self).to_partial_json()
+        jobj = super().to_partial_json()
         return self._add_contact_if_appropriate(jobj)
 
     def fields_to_partial_json(self):
         """Modify josepy.JSONObjectWithFields.fields_to_partial_json()"""
-        jobj = super(Registration, self).fields_to_partial_json()
+        jobj = super().fields_to_partial_json()
         return self._add_contact_if_appropriate(jobj)
 
     @property
@@ -460,19 +462,19 @@ class ChallengeBody(ResourceBody):
 
     def __init__(self, **kwargs):
         kwargs = {self._internal_name(k): v for k, v in kwargs.items()}
-        super(ChallengeBody, self).__init__(**kwargs)
+        super().__init__(**kwargs)
 
     def encode(self, name):
-        return super(ChallengeBody, self).encode(self._internal_name(name))
+        return super().encode(self._internal_name(name))
 
     def to_partial_json(self):
-        jobj = super(ChallengeBody, self).to_partial_json()
+        jobj = super().to_partial_json()
         jobj.update(self.chall.to_partial_json())
         return jobj
 
     @classmethod
     def fields_from_json(cls, jobj):
-        jobj_fields = super(ChallengeBody, cls).fields_from_json(jobj)
+        jobj_fields = super().fields_from_json(jobj)
         jobj_fields['chall'] = challenges.Challenge.from_json(jobj)
         return jobj_fields
 
@@ -487,7 +489,7 @@ class ChallengeBody(ResourceBody):
     def __iter__(self):
         # When iterating over fields, use the external name 'uri' instead of
         # the internal '_uri'.
-        for name in super(ChallengeBody, self).__iter__():
+        for name in super().__iter__():
             yield name[1:] if name == '_uri' else name
 
     def _internal_name(self, name):

acme/acme/mixins.py

@@ -20,7 +20,7 @@ class VersionedLEACMEMixin:
             # Required for @property to operate properly. See comment above.
             object.__setattr__(self, key, value)
         else:
-            super(VersionedLEACMEMixin, self).__setattr__(key, value)  # pragma: no cover
+            super().__setattr__(key, value)  # pragma: no cover
 
 
 class ResourceMixin(VersionedLEACMEMixin):
@@ -30,12 +30,12 @@ class ResourceMixin(VersionedLEACMEMixin):
     """
     def to_partial_json(self):
         """See josepy.JSONDeserializable.to_partial_json()"""
-        return _safe_jobj_compliance(super(ResourceMixin, self),
+        return _safe_jobj_compliance(super(),
                                      'to_partial_json', 'resource')
 
     def fields_to_partial_json(self):
         """See josepy.JSONObjectWithFields.fields_to_partial_json()"""
-        return _safe_jobj_compliance(super(ResourceMixin, self),
+        return _safe_jobj_compliance(super(),
                                      'fields_to_partial_json', 'resource')
 
 
@@ -46,12 +46,12 @@ class TypeMixin(VersionedLEACMEMixin):
     """
     def to_partial_json(self):
         """See josepy.JSONDeserializable.to_partial_json()"""
-        return _safe_jobj_compliance(super(TypeMixin, self),
+        return _safe_jobj_compliance(super(),
                                      'to_partial_json', 'type')
 
     def fields_to_partial_json(self):
         """See josepy.JSONObjectWithFields.fields_to_partial_json()"""
-        return _safe_jobj_compliance(super(TypeMixin, self),
+        return _safe_jobj_compliance(super(),
                                      'fields_to_partial_json', 'type')
 
 

acme/acme/standalone.py

@@ -8,6 +8,7 @@ import socket
 import socketserver
 import threading
 from typing import List
+from typing import Optional
 
 from acme import challenges
 from acme import crypto_util
@@ -66,6 +67,9 @@ class BaseDualNetworkedServers:
         self.threads: List[threading.Thread] = []
         self.servers: List[socketserver.BaseServer] = []
 
+        # Preserve socket error for re-raising, if no servers can be started
+        last_socket_err: Optional[socket.error] = None
+
         # Must try True first.
         # Ubuntu, for example, will fail to bind to IPv4 if we've already bound
         # to IPv6. But that's ok, since it will accept IPv4 connections on the IPv6
@@ -82,7 +86,8 @@ class BaseDualNetworkedServers:
                 logger.debug(
                     "Successfully bound to %s:%s using %s", new_address[0],
                     new_address[1], "IPv6" if ip_version else "IPv4")
-            except socket.error:
+            except socket.error as e:
+                last_socket_err = e
                 if self.servers:
                     # Already bound using IPv6.
                     logger.debug(
@@ -101,7 +106,10 @@ class BaseDualNetworkedServers:
                 # bind to the same port for both servers.
                 port = server.socket.getsockname()[1]
         if not self.servers:
-            raise socket.error("Could not bind to IPv4 or IPv6.")
+            if last_socket_err:
+                raise last_socket_err
+            else: # pragma: no cover
+                raise socket.error("Could not bind to IPv4 or IPv6.")
 
     def serve_forever(self):
         """Wraps socketserver.TCPServer.serve_forever"""

acme/acme/util.py

@@ -1,6 +1,5 @@
 """ACME utilities."""
 
-
 def map_keys(dikt, func):
     """Map dictionary keys."""
     return {func(key): value for key, value in dikt.items()}

acme/examples/standalone/README

@@ -1,2 +0,0 @@
-python -m acme.standalone -p 1234
-curl -k https://localhost:1234

acme/examples/standalone/localhost/cert.pem

@@ -1 +0,0 @@
-../../../acme/testdata/rsa2048_cert.pem

acme/examples/standalone/localhost/key.pem

@@ -1 +0,0 @@
-../../../acme/testdata/rsa2048_key.pem

acme/setup.py

@@ -3,41 +3,36 @@ import sys
 from setuptools import find_packages
 from setuptools import setup
 
-version = '1.14.0.dev0'
+version = '1.20.0'
 
-# Please update tox.ini when modifying dependency version requirements
 install_requires = [
     'cryptography>=2.1.4',
-    # formerly known as acme.jose:
-    # 1.1.0+ is required to avoid the warnings described at
-    # https://github.com/certbot/josepy/issues/13.
-    'josepy>=1.1.0',
+    'josepy>=1.9.0',
     'PyOpenSSL>=17.3.0',
     'pyrfc3339',
     'pytz',
-    'requests>=2.6.0',
+    'requests>=2.14.2',
     'requests-toolbelt>=0.3.0',
     'setuptools>=39.0.1',
 ]
 
-dev_extras = [
-    'pytest',
-    'pytest-xdist',
-    'tox',
-]
-
 docs_extras = [
     'Sphinx>=1.0',  # autodoc_member_order = 'bysource', autodoc_default_flags
     'sphinx_rtd_theme',
 ]
 
+test_extras = [
+    'pytest',
+    'pytest-xdist',
+]
+
 setup(
     name='acme',
     version=version,
     description='ACME protocol implementation in Python',
     url='https://github.com/letsencrypt/letsencrypt',
     author="Certbot Project",
-    author_email='client-dev@letsencrypt.org',
+    author_email='certbot-dev@eff.org',
     license='Apache License 2.0',
     python_requires='>=3.6',
     classifiers=[
@@ -58,7 +53,7 @@ setup(
     include_package_data=True,
     install_requires=install_requires,
     extras_require={
-        'dev': dev_extras,
         'docs': docs_extras,
+        'test': test_extras,
     },
 )

acme/tests/challenges_test.py

@@ -292,7 +292,7 @@ class TLSALPN01ResponseTest(unittest.TestCase):
 
     def test_gen_verify_cert_gen_key(self):
         cert, key = self.response.gen_cert(self.domain)
-        self.assertTrue(isinstance(key, OpenSSL.crypto.PKey))
+        self.assertIsInstance(key, OpenSSL.crypto.PKey)
         self.assertTrue(self.response.verify_cert(self.domain, cert))
 
     def test_verify_bad_cert(self):
@@ -431,7 +431,7 @@ class DNSTest(unittest.TestCase):
             mock_gen.return_value = mock.sentinel.validation
             response = self.msg.gen_response(KEY)
         from acme.challenges import DNSResponse
-        self.assertTrue(isinstance(response, DNSResponse))
+        self.assertIsInstance(response, DNSResponse)
         self.assertEqual(response.validation, mock.sentinel.validation)
 
     def test_validation_domain_name(self):

acme/tests/client_test.py

@@ -3,6 +3,7 @@
 import copy
 import datetime
 import http.client as http_client
+import ipaddress
 import json
 import unittest
 from typing import Dict
@@ -23,6 +24,7 @@ import test_util
 CERT_DER = test_util.load_vector('cert.der')
 CERT_SAN_PEM = test_util.load_vector('cert-san.pem')
 CSR_SAN_PEM = test_util.load_vector('csr-san.pem')
+CSR_MIXED_PEM = test_util.load_vector('csr-mixed.pem')
 KEY = jose.JWKRSA.load(test_util.load_vector('rsa512_key.pem'))
 KEY2 = jose.JWKRSA.load(test_util.load_vector('rsa256_key.pem'))
 
@@ -90,7 +92,7 @@ class BackwardsCompatibleClientV2Test(ClientTestBase):
     """Tests for  acme.client.BackwardsCompatibleClientV2."""
 
     def setUp(self):
-        super(BackwardsCompatibleClientV2Test, self).setUp()
+        super().setUp()
         # contains a loaded cert
         self.certr = messages.CertificateResource(
             body=messages_test.CERT)
@@ -319,7 +321,7 @@ class ClientTest(ClientTestBase):
     """Tests for acme.client.Client."""
 
     def setUp(self):
-        super(ClientTest, self).setUp()
+        super().setUp()
 
         self.directory = DIRECTORY_V1
 
@@ -604,8 +606,8 @@ class ClientTest(ClientTestBase):
             # make sure that max_attempts is per-authorization, rather
             # than global
             max_attempts=max(len(authzrs[0].retries), len(authzrs[1].retries)))
-        self.assertTrue(cert[0] is csr)
-        self.assertTrue(cert[1] is updated_authzrs)
+        self.assertIs(cert[0], csr)
+        self.assertIs(cert[1], updated_authzrs)
         self.assertEqual(updated_authzrs[0].uri, 'a...')
         self.assertEqual(updated_authzrs[1].uri, 'b.')
         self.assertEqual(updated_authzrs[0].times, [
@@ -641,7 +643,7 @@ class ClientTest(ClientTestBase):
         authzr = self.client.deactivate_authorization(self.authzr)
         self.assertEqual(authzb, authzr.body)
         self.assertEqual(self.client.net.post.call_count, 1)
-        self.assertTrue(self.authzr.uri in self.net.post.call_args_list[0][0])
+        self.assertIn(self.authzr.uri, self.net.post.call_args_list[0][0])
 
     def test_check_cert(self):
         self.response.headers['Location'] = self.certr.uri
@@ -700,7 +702,7 @@ class ClientTest(ClientTestBase):
 
     def test_revocation_payload(self):
         obj = messages.Revocation(certificate=self.certr.body, reason=self.rsn)
-        self.assertTrue('reason' in obj.to_partial_json().keys())
+        self.assertIn('reason', obj.to_partial_json().keys())
         self.assertEqual(self.rsn, obj.to_partial_json()['reason'])
 
     def test_revoke_bad_status_raises_error(self):
@@ -716,7 +718,7 @@ class ClientV2Test(ClientTestBase):
     """Tests for acme.client.ClientV2."""
 
     def setUp(self):
-        super(ClientV2Test, self).setUp()
+        super().setUp()
 
         self.directory = DIRECTORY_V2
 
@@ -740,7 +742,7 @@ class ClientV2Test(ClientTestBase):
         self.orderr = messages.OrderResource(
             body=self.order,
             uri='https://www.letsencrypt-demo.org/acme/acct/1/order/1',
-            authorizations=[self.authzr, self.authzr2], csr_pem=CSR_SAN_PEM)
+            authorizations=[self.authzr, self.authzr2], csr_pem=CSR_MIXED_PEM)
 
     def test_new_account(self):
         self.response.status_code = http_client.CREATED
@@ -770,7 +772,7 @@ class ClientV2Test(ClientTestBase):
 
         with mock.patch('acme.client.ClientV2._post_as_get') as mock_post_as_get:
             mock_post_as_get.side_effect = (authz_response, authz_response2)
-            self.assertEqual(self.client.new_order(CSR_SAN_PEM), self.orderr)
+            self.assertEqual(self.client.new_order(CSR_MIXED_PEM), self.orderr)
 
     @mock.patch('acme.client.datetime')
     def test_poll_and_finalize(self, mock_datetime):
@@ -877,9 +879,9 @@ class ClientV2Test(ClientTestBase):
         self.response.headers['Location'] = self.regr.uri
         self.response.json.return_value = self.regr.body.to_json()
         self.assertEqual(self.regr, self.client.update_registration(self.regr))
-        self.assertNotEqual(self.client.net.account, None)
+        self.assertIsNotNone(self.client.net.account)
         self.assertEqual(self.client.net.post.call_count, 2)
-        self.assertTrue(DIRECTORY_V2.newAccount in self.net.post.call_args_list[0][0])
+        self.assertIn(DIRECTORY_V2.newAccount, self.net.post.call_args_list[0][0])
 
         self.response.json.return_value = self.regr.body.update(
             contact=()).to_json()
@@ -943,7 +945,7 @@ class ClientNetworkTest(unittest.TestCase):
         self.response.links = {}
 
     def test_init(self):
-        self.assertTrue(self.net.verify_ssl is self.verify_ssl)
+        self.assertIs(self.net.verify_ssl, self.verify_ssl)
 
     def test_wrap_in_jws(self):
         # pylint: disable=protected-access
@@ -1185,7 +1187,7 @@ class ClientNetworkWithMockedResponseTest(unittest.TestCase):
 
         def send_request(*args, **kwargs):
             # pylint: disable=unused-argument,missing-docstring
-            self.assertFalse("new_nonce_url" in kwargs)
+            self.assertNotIn("new_nonce_url", kwargs)
             method = args[0]
             uri = args[1]
             if method == 'HEAD' and uri != "new_nonce_uri":
@@ -1330,7 +1332,7 @@ class ClientNetworkSourceAddressBindingTest(unittest.TestCase):
         from acme.client import ClientNetwork
         net = ClientNetwork(key=None, alg=None, source_address=self.source_address)
         for adapter in net.session.adapters.values():
-            self.assertTrue(self.source_address in adapter.source_address)
+            self.assertIn(self.source_address, adapter.source_address)
 
     def test_behavior_assumption(self):
         """This is a test that guardrails the HTTPAdapter behavior so that if the default for

acme/tests/crypto_util_test.py

@@ -1,5 +1,6 @@
 """Tests for acme.crypto_util."""
 import itertools
+import ipaddress
 import socket
 import socketserver
 import threading
@@ -108,7 +109,6 @@ class PyOpenSSLCertOrReqAllNamesTest(unittest.TestCase):
 class PyOpenSSLCertOrReqSANTest(unittest.TestCase):
     """Test for acme.crypto_util._pyopenssl_cert_or_req_san."""
 
-
     @classmethod
     def _call(cls, loader, name):
         # pylint: disable=protected-access
@@ -174,9 +174,50 @@ class PyOpenSSLCertOrReqSANTest(unittest.TestCase):
                          ['chicago-cubs.venafi.example', 'cubs.venafi.example'])
 
 
+class PyOpenSSLCertOrReqSANIPTest(unittest.TestCase):
+    """Test for acme.crypto_util._pyopenssl_cert_or_req_san_ip."""
 
-class RandomSnTest(unittest.TestCase):
-    """Test for random certificate serial numbers."""
+    @classmethod
+    def _call(cls, loader, name):
+        # pylint: disable=protected-access
+        from acme.crypto_util import _pyopenssl_cert_or_req_san_ip
+        return _pyopenssl_cert_or_req_san_ip(loader(name))
+
+    def _call_cert(self, name):
+        return self._call(test_util.load_cert, name)
+
+    def _call_csr(self, name):
+        return self._call(test_util.load_csr, name)
+
+    def test_cert_no_sans(self):
+        self.assertEqual(self._call_cert('cert.pem'), [])
+
+    def test_csr_no_sans(self):
+        self.assertEqual(self._call_csr('csr-nosans.pem'), [])
+
+    def test_cert_domain_sans(self):
+        self.assertEqual(self._call_cert('cert-san.pem'), [])
+
+    def test_csr_domain_sans(self):
+        self.assertEqual(self._call_csr('csr-san.pem'), [])
+
+    def test_cert_ip_two_sans(self):
+        self.assertEqual(self._call_cert('cert-ipsans.pem'), ['192.0.2.145', '203.0.113.1'])
+
+    def test_csr_ip_two_sans(self):
+        self.assertEqual(self._call_csr('csr-ipsans.pem'), ['192.0.2.145', '203.0.113.1'])
+
+    def test_csr_ipv6_sans(self):
+        self.assertEqual(self._call_csr('csr-ipv6sans.pem'),
+                         ['0:0:0:0:0:0:0:1', 'A3BE:32F3:206E:C75D:956:CEE:9858:5EC5'])
+
+    def test_cert_ipv6_sans(self):
+        self.assertEqual(self._call_cert('cert-ipv6sans.pem'),
+                         ['0:0:0:0:0:0:0:1', 'A3BE:32F3:206E:C75D:956:CEE:9858:5EC5'])
+
+
+class GenSsCertTest(unittest.TestCase):
+    """Test for gen_ss_cert (generation of self-signed cert)."""
 
 
     def setUp(self):
@@ -187,11 +228,19 @@ class RandomSnTest(unittest.TestCase):
 
     def test_sn_collisions(self):
         from acme.crypto_util import gen_ss_cert
-
         for _ in range(self.cert_count):
-            cert = gen_ss_cert(self.key, ['dummy'], force_san=True)
+            cert = gen_ss_cert(self.key, ['dummy'], force_san=True,
+                               ips=[ipaddress.ip_address("10.10.10.10")])
             self.serial_num.append(cert.get_serial_number())
-        self.assertTrue(len(set(self.serial_num)) > 1)
+        self.assertGreaterEqual(len(set(self.serial_num)), self.cert_count)
+
+
+    def test_no_name(self):
+        from acme.crypto_util import gen_ss_cert
+        with self.assertRaises(AssertionError):
+            gen_ss_cert(self.key, ips=[ipaddress.ip_address("1.1.1.1")])
+            gen_ss_cert(self.key)
+
 
 class MakeCSRTest(unittest.TestCase):
     """Test for standalone functions."""
@@ -206,8 +255,8 @@ class MakeCSRTest(unittest.TestCase):
 
     def test_make_csr(self):
         csr_pem = self._call_with_key(["a.example", "b.example"])
-        self.assertTrue(b'--BEGIN CERTIFICATE REQUEST--' in csr_pem)
-        self.assertTrue(b'--END CERTIFICATE REQUEST--' in csr_pem)
+        self.assertIn(b'--BEGIN CERTIFICATE REQUEST--', csr_pem)
+        self.assertIn(b'--END CERTIFICATE REQUEST--', csr_pem)
         csr = OpenSSL.crypto.load_certificate_request(
             OpenSSL.crypto.FILETYPE_PEM, csr_pem)
         # In pyopenssl 0.13 (used with TOXENV=py27-oldest), csr objects don't
@@ -223,6 +272,27 @@ class MakeCSRTest(unittest.TestCase):
                 ).get_data(),
             )
 
+    def test_make_csr_ip(self):
+        csr_pem = self._call_with_key(["a.example"], False, [ipaddress.ip_address('127.0.0.1'), ipaddress.ip_address('::1')])
+        self.assertIn(b'--BEGIN CERTIFICATE REQUEST--' , csr_pem)
+        self.assertIn(b'--END CERTIFICATE REQUEST--' , csr_pem)
+        csr = OpenSSL.crypto.load_certificate_request(
+            OpenSSL.crypto.FILETYPE_PEM, csr_pem)
+        # In pyopenssl 0.13 (used with TOXENV=py27-oldest), csr objects don't
+        # have a get_extensions() method, so we skip this test if the method
+        # isn't available.
+        if hasattr(csr, 'get_extensions'):
+            self.assertEqual(len(csr.get_extensions()), 1)
+            self.assertEqual(csr.get_extensions()[0].get_data(),
+                             OpenSSL.crypto.X509Extension(
+                                 b'subjectAltName',
+                                 critical=False,
+                                 value=b'DNS:a.example, IP:127.0.0.1, IP:::1',
+                             ).get_data(),
+                             )
+            # for IP san it's actually need to be octet-string,
+            # but somewhere downstream thankfully handle it for us
+
     def test_make_csr_must_staple(self):
         csr_pem = self._call_with_key(["a.example"], must_staple=True)
         csr = OpenSSL.crypto.load_certificate_request(
@@ -241,6 +311,9 @@ class MakeCSRTest(unittest.TestCase):
             self.assertEqual(len(must_staple_exts), 1,
                 "Expected exactly one Must Staple extension")
 
+    def test_make_csr_without_hostname(self):
+        self.assertRaises(ValueError, self._call_with_key)
+
 
 class DumpPyopensslChainTest(unittest.TestCase):
     """Test for dump_pyopenssl_chain."""

acme/tests/errors_test.py

@@ -24,8 +24,8 @@ class MissingNonceTest(unittest.TestCase):
         self.error = MissingNonce(self.response)
 
     def test_str(self):
-        self.assertTrue("FOO" in str(self.error))
-        self.assertTrue("{}" in str(self.error))
+        self.assertIn("FOO", str(self.error))
+        self.assertIn("{}", str(self.error))
 
 
 class PollErrorTest(unittest.TestCase):

acme/tests/jws_test.py

@@ -48,7 +48,7 @@ class JWSTest(unittest.TestCase):
         self.assertEqual(jws.signature.combined.nonce, self.nonce)
         self.assertEqual(jws.signature.combined.url, self.url)
         self.assertEqual(jws.signature.combined.kid, self.kid)
-        self.assertEqual(jws.signature.combined.jwk, None)
+        self.assertIsNone(jws.signature.combined.jwk)
         # TODO: check that nonce is in protected header
 
         self.assertEqual(jws, JWS.from_json(jws.to_json()))
@@ -58,7 +58,7 @@ class JWSTest(unittest.TestCase):
         jws = JWS.sign(payload=b'foo', key=self.privkey,
                        alg=jose.RS256, nonce=self.nonce,
                        url=self.url)
-        self.assertEqual(jws.signature.combined.kid, None)
+        self.assertIsNone(jws.signature.combined.kid)
         self.assertEqual(jws.signature.combined.jwk, self.pubkey)
 
 

acme/tests/messages_test.py

@@ -41,13 +41,13 @@ class ErrorTest(unittest.TestCase):
 
     def test_description(self):
         self.assertEqual('The request message was malformed', self.error.description)
-        self.assertTrue(self.error_custom.description is None)
+        self.assertIsNone(self.error_custom.description)
 
     def test_code(self):
         from acme.messages import Error
         self.assertEqual('malformed', self.error.code)
-        self.assertEqual(None, self.error_custom.code)
-        self.assertEqual(None, Error().code)
+        self.assertIsNone(self.error_custom.code)
+        self.assertIsNone(Error().code)
 
     def test_is_acme_error(self):
         from acme.messages import is_acme_error, Error
@@ -260,10 +260,10 @@ class RegistrationTest(unittest.TestCase):
         self.assertEqual(empty_new_reg.contact, ())
         self.assertEqual(new_reg_with_contact.contact, ())
 
-        self.assertTrue('contact' not in empty_new_reg.to_partial_json())
-        self.assertTrue('contact' not in empty_new_reg.fields_to_partial_json())
-        self.assertTrue('contact' in new_reg_with_contact.to_partial_json())
-        self.assertTrue('contact' in new_reg_with_contact.fields_to_partial_json())
+        self.assertNotIn('contact', empty_new_reg.to_partial_json())
+        self.assertNotIn('contact', empty_new_reg.fields_to_partial_json())
+        self.assertIn('contact', new_reg_with_contact.to_partial_json())
+        self.assertIn('contact', new_reg_with_contact.fields_to_partial_json())
 
 
 class UpdateRegistrationTest(unittest.TestCase):
@@ -406,7 +406,7 @@ class AuthorizationResourceTest(unittest.TestCase):
         authzr = AuthorizationResource(
             uri=mock.sentinel.uri,
             body=mock.sentinel.body)
-        self.assertTrue(isinstance(authzr, jose.JSONDeSerializable))
+        self.assertIsInstance(authzr, jose.JSONDeSerializable)
 
 
 class CertificateRequestTest(unittest.TestCase):
@@ -417,7 +417,7 @@ class CertificateRequestTest(unittest.TestCase):
         self.req = CertificateRequest(csr=CSR)
 
     def test_json_de_serializable(self):
-        self.assertTrue(isinstance(self.req, jose.JSONDeSerializable))
+        self.assertIsInstance(self.req, jose.JSONDeSerializable)
         from acme.messages import CertificateRequest
         self.assertEqual(
             self.req, CertificateRequest.from_json(self.req.to_json()))
@@ -433,7 +433,7 @@ class CertificateResourceTest(unittest.TestCase):
             cert_chain_uri=mock.sentinel.cert_chain_uri)
 
     def test_json_de_serializable(self):
-        self.assertTrue(isinstance(self.certr, jose.JSONDeSerializable))
+        self.assertIsInstance(self.certr, jose.JSONDeSerializable)
         from acme.messages import CertificateResource
         self.assertEqual(
             self.certr, CertificateResource.from_json(self.certr.to_json()))

acme/tests/standalone_test.py

@@ -190,12 +190,18 @@ class BaseDualNetworkedServersTest(unittest.TestCase):
 
     @mock.patch("socket.socket.bind")
     def test_fail_to_bind(self, mock_bind):
-        mock_bind.side_effect = socket.error
+        from errno import EADDRINUSE
         from acme.standalone import BaseDualNetworkedServers
-        self.assertRaises(socket.error, BaseDualNetworkedServers,
-                          BaseDualNetworkedServersTest.SingleProtocolServer,
-                          ('', 0),
-                          socketserver.BaseRequestHandler)
+
+        mock_bind.side_effect = socket.error(EADDRINUSE, "Fake addr in use error")
+
+        with self.assertRaises(socket.error) as em:
+            BaseDualNetworkedServers(
+                BaseDualNetworkedServersTest.SingleProtocolServer,
+                ('', 0), socketserver.BaseRequestHandler)
+
+        self.assertEqual(em.exception.errno, EADDRINUSE)
+
 
     def test_ports_equal(self):
         from acme.standalone import BaseDualNetworkedServers

acme/tests/testdata/cert-ipsans.pem

@@ -0,0 +1,21 @@
+-----BEGIN CERTIFICATE-----
+MIIDizCCAnOgAwIBAgIIPNBLQXwhoUkwDQYJKoZIhvcNAQELBQAwKDEmMCQGA1UE
+AxMdUGViYmxlIEludGVybWVkaWF0ZSBDQSAxNzNiMjYwHhcNMjAwNTI5MTkxODA5
+WhcNMjUwNTI5MTkxODA5WjAWMRQwEgYDVQQDEwsxOTIuMC4yLjE0NTCCASIwDQYJ
+KoZIhvcNAQEBBQADggEPADCCAQoCggEBALyChb+NDA26GF1AfC0nzEdfOTchKw0h
+q41xEjonvg5UXgZf/aH/ntvugIkYP0MaFifNAjebOVVsemEVEtyWcUKTfBHKZGbZ
+ukTDwFIjfTccCfo6U/B2H7ZLzJIywl8DcUw9DypadeQBm8PS0VVR2ncy73dvaqym
+crhAwlASyXU0mhLqRDMMxfg5Bn/FWpcsIcDpLmPn8Q/FvdRc2t5ryBNw/aWOlwqT
+Oy16nbfLj2T0zG1A3aPuD+eT/JFUe/o3K7R+FAx7wt+RziQO46wLVVF1SueZUrIU
+zqN04Gl8Kt1WM2SniZ0gq/rORUNcPtT0NAEsEslTQfA+Trq6j2peqyMCAwEAAaOB
+yjCBxzAOBgNVHQ8BAf8EBAMCBaAwHQYDVR0lBBYwFAYIKwYBBQUHAwEGCCsGAQUF
+BwMCMAwGA1UdEwEB/wQCMAAwHQYDVR0OBBYEFHj1mwZzP//nMIH2i58NRUl/arHn
+MB8GA1UdIwQYMBaAFF5DVAKabvIUvKFHGouscA2Qdpe6MDEGCCsGAQUFBwEBBCUw
+IzAhBggrBgEFBQcwAYYVaHR0cDovLzEyNy4wLjAuMTo0MDAyMBUGA1UdEQQOMAyH
+BMAAApGHBMsAcQEwDQYJKoZIhvcNAQELBQADggEBAHjSgDg76/UCIMSYddyhj18r
+LdNKjA7p8ovnErSkebFT4lIZ9f3Sma9moNr0w64M33NamuFyHe/KTdk90mvoW8Uu
+26aDekiRIeeMakzbAtDKn67tt2tbedKIYRATcSYVwsV46uZKbM621dZKIjjxOWpo
+IY6rZYrku8LYhoXJXOqRduV3cTRVuTm5bBa9FfVNtt6N1T5JOtKKDEhuSaF4RSug
+PDy3hQIiHrVvhPfVrXU3j6owz/8UCS5549inES9ONTFrvM9o0H1R/MsmGNXR5hF5
+iJqHKC7n8LZujhVnoFIpHu2Dsiefbfr+yRYJS4I+ezy6Nq/Ok8rc8zp0eoX+uyY=
+-----END CERTIFICATE-----

acme/tests/testdata/cert-ipv6sans.pem

@@ -0,0 +1,22 @@
+-----BEGIN CERTIFICATE-----
+MIIDmzCCAoOgAwIBAgIIFdxeZP+v2rgwDQYJKoZIhvcNAQELBQAwKDEmMCQGA1UE
+AxMdUGViYmxlIEludGVybWVkaWF0ZSBDQSA0M2M5NTcwHhcNMjAwNTMwMDQwNzMw
+WhcNMjUwNTMwMDQwNzMwWjAOMQwwCgYDVQQDEwM6OjEwggEiMA0GCSqGSIb3DQEB
+AQUAA4IBDwAwggEKAoIBAQC7VidVduJvqKtrSH0fw6PjE0cqL4Kfzo7klexWUkHG
+KVAa0fRVZFZ462jxKOt417V2U4WJQ6WHHO9PJ+3gW62d/MhCw8FRtUQS4nYFjqB6
+32+RFU21VRN7cWoQEqSwnEPbh/v/zv/KS5JhQ+swWUo79AOLm1kjnZWCKtcqh1Lc
+Ug5Tkpot6luoxTKp52MkchvXDpj0q2B/XpLJ8/pw5cqjv7mH12EDOK2HXllA+WwX
+ZpstcEhaA4FqtaHOW/OHnwTX5MUbINXE5YYHVEDR6moVM31/W/3pe9NDUMTDE7Si
+lVQnZbXM9NYbzZqlh+WhemDWwnIfGI6rtsfNEiirVEOlAgMBAAGjgeIwgd8wDgYD
+VR0PAQH/BAQDAgWgMB0GA1UdJQQWMBQGCCsGAQUFBwMBBggrBgEFBQcDAjAMBgNV
+HRMBAf8EAjAAMB0GA1UdDgQWBBS8DL+MZfDIy6AKky69Tgry2Vxq5DAfBgNVHSME
+GDAWgBRAsFqVenRRKgB1YPzWKzb9bzZ/ozAxBggrBgEFBQcBAQQlMCMwIQYIKwYB
+BQUHMAGGFWh0dHA6Ly8xMjcuMC4wLjE6NDAwMjAtBgNVHREEJjAkhxAAAAAAAAAA
+AAAAAAAAAAABhxCjvjLzIG7HXQlWDO6YWF7FMA0GCSqGSIb3DQEBCwUAA4IBAQBY
+M9UTZ3uaKMQ+He9kWR3p9jh6hTSD0FNi79ZdfkG0lgSzhhduhN7OhzQH2ihUUfa6
+rtKTw74fGbszhizCd9UB8YPKlm3si1Xbg6ZUQlA1RtoQo7RUGEa6ZbR68PKGm9Go
+hTTFIl/JS8jzxBR8jywZdyqtprUx+nnNUDiNk0hJtFLhw7OJH0AHlAUNqHsfD08m
+HXRdaV6q14HXU5g31slBat9H4D6tCU/2uqBURwW0wVdnqh4QeRfAeqiatJS9EmSF
+ctbc7n894Idy2Xce7NFoIy5cht3m6Rd42o/LmBsJopBmQcDPZT70/XzRtc2qE0cS
+CzBIGQHUJ6BfmBjrCQnp
+-----END CERTIFICATE-----

acme/tests/testdata/csr-ipsans.pem

@@ -0,0 +1,16 @@
+-----BEGIN CERTIFICATE REQUEST-----
+MIICbTCCAVUCAQIwADCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAKT/
+CE7Y5EYBvI4p7Frt763upIKHDHO/R5/TWMjG8Jm9qTMui8sbMgyh2Yh+lR/j/5Xd
+tQrhgC6wx10MrW2+3JtYS88HP1p6si8zU1dbK34n3NyyklR2RivW0R7dXgnYNy7t
+5YcDYLCrbRMIPINV/uHrmzIHWYUDNcZVdAfIM2AHfKYuV6Mepcn///5GR+l4GcAh
+Nkf9CW8OdAIuKdbyLCxVr0mUW/vJz1b12uxPsgUdax9sjXgZdT4pfMXADsFd1NeF
+atpsXU073inqtHru+2F9ijHTQ75TC+u/rr6eYl3BnBntac0gp/ADtDBii7/Q1JOO
+Bhq7xJNqqxIEdiyM7zcCAwEAAaAoMCYGCSqGSIb3DQEJDjEZMBcwFQYDVR0RBA4w
+DIcEwAACkYcEywBxATANBgkqhkiG9w0BAQsFAAOCAQEADG5g3zdbSCaXpZhWHkzE
+Mek3f442TUE1pB+ITRpthmM4N3zZWETYmbLCIAO624uMrRnbCCMvAoLs/L/9ETg/
+XMMFtonQC8u9i9tV8B1ceBh8lpIfa+8b9TMWH3bqnrbWQ+YIl+Yd0gXiCZWJ9vK4
+eM1Gddu/2bR6s/k4h/XAWRgEexqk57EHr1z0N+T9OoX939n3mVcNI+u9kfd5VJ0z
+VyA3R8WR6T6KlEl5P5pcWe5Kuyhi7xMmLVImXqBtvKq4O1AMfM+gQr/yn9aE8IRq
+khP7JrMBLUIub1c/qu2TfvnynNPSM/ZcOX+6PHdHmRkR3nI0Ndpv7Ntv31FTplAm
+Dw==
+-----END CERTIFICATE REQUEST-----

acme/tests/testdata/csr-ipv6sans.pem

@@ -0,0 +1,16 @@
+-----BEGIN CERTIFICATE REQUEST-----
+MIIChTCCAW0CAQIwADCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAOIc
+UAppcqJfTkSqqOFqGt1v7lIJZPOcF4bcKI3d5cHAGbOuVxbC7uMaDuObwYLzoiED
+qnvs1NaEq2phO6KsgGESB7IE2LUjJivO7OnSZjNRpL5si/9egvBiNCn/50lULaWG
+gLEuyMfk3awZy2mVAymy7Grhbx069A4TH8TqsHuq2RpKyuDL27e/jUt6yYecb3pu
+hWMiWy3segif4tI46pkOW0/I6DpxyYD2OqOvzxm/voS9RMqE2+7YJA327H7bEi3N
+lJZEZ1zy7clZ9ga5fBQaetzbg2RyxTrZ7F919NQXSFoXgxb10Eg64wIpz0L3ooCm
+GEHehsZZexa3J5ccIvMCAwEAAaBAMD4GCSqGSIb3DQEJDjExMC8wLQYDVR0RBCYw
+JIcQAAAAAAAAAAAAAAAAAAAAAYcQo74y8yBux10JVgzumFhexTANBgkqhkiG9w0B
+AQsFAAOCAQEALvwVn0A/JPTCiNzcozHFnp5M23C9PXCplWc5u4k34d4XXzpSeFDz
+fL4gy7NpYIueme2K2ppw2j3PNQUdR6vQ5a75sriegWYrosL+7Q6Joh51ZyEUZQoD
+mNl4M4S4oX85EaChR6NFGBywTfjFarYi32XBTbFE7rK8N8KM+DQkNdwL1MXqaHWz
+F1obQKpNXlLedbCBOteV5Eg4zG3565zu/Gw/NhwzzV3mQmgxUcd1sMJxAfHQz4Vl
+ImLL+xMcR03nDsH2bgtDbK2tJm7WszSxA9tC+Xp2lRewxrnQloRWPYDz177WGQ5Q
+SoGDzTTtA6uWZxG8h7CkNLOGvA8LtU2rNA==
+-----END CERTIFICATE REQUEST-----

acme/tests/testdata/csr-mixed.pem

@@ -0,0 +1,16 @@
+-----BEGIN CERTIFICATE REQUEST-----
+MIICdjCCAV4CAQIwADCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAMXq
+v1y8EIcCbaUIzCtOcLkLS0MJ35oS+6DmV5WB1A0cIk6YrjsHIsY2lwMm13BWIvmw
+tY+Y6n0rr7eViNx5ZRGHpHEI/TL3Neb+VefTydL5CgvK3dd4ex2kSbTaed3fmpOx
+qMajEduwNcZPCcmoEXPkfrCP8w2vKQUkQ+JRPcdX1nTuzticeRP5B7YCmJsmxkEh
+Y0tzzZ+NIRDARoYNofefY86h3e5q66gtJxccNchmIM3YQahhg5n3Xoo8hGfM/TIc
+R7ncCBCLO6vtqo0QFva/NQODrgOmOsmgvqPkUWQFdZfWM8yIaU826dktx0CPB78t
+TudnJ1rBRvGsjHMsZikCAwEAAaAxMC8GCSqGSIb3DQEJDjEiMCAwHgYDVR0RBBcw
+FYINYS5leGVtcGxlLmNvbYcEwAACbzANBgkqhkiG9w0BAQsFAAOCAQEAdGMcRCxq
+1X09gn1TNdMt64XUv+wdJCKDaJ+AgyIJj7QvVw8H5k7dOnxS4I+a/yo4jE+LDl2/
+AuHcBLFEI4ddewdJSMrTNZjuRYuOdr3KP7fL7MffICSBi45vw5EOXg0tnjJCEiKu
+6gcJgbLSP5JMMd7Haf33Q/VWsmHofR3VwOMdrnakwAU3Ff5WTuXTNVhL1kT/uLFX
+yW1ru6BF4unwNqSR2UeulljpNfRBsiN4zJK11W6n9KT0NkBr9zY5WCM4sW7i8k9V
+TeypWGo3jBKzYAGeuxZsB97U77jZ2lrGdBLZKfbcjnTeRVqCvCRrui4El7UGYFmj
+7s6OJyWx5DSV8w==
+-----END CERTIFICATE REQUEST-----

certbot-apache/certbot_apache/_internal/apache_util.py

@@ -153,13 +153,10 @@ def parse_defines(apachectl):
         return {}
 
     for match in matches:
-        if match.count("=") > 1:
-            logger.error("Unexpected number of equal signs in "
-                         "runtime config dump.")
-            raise errors.PluginError(
-                "Error parsing Apache runtime variables")
-        parts = match.partition("=")
-        variables[parts[0]] = parts[2]
+        # Value could also contain = so split only once
+        parts = match.split('=', 1)
+        value = parts[1] if len(parts) == 2 else ''
+        variables[parts[0]] = value
 
     return variables
 
@@ -220,13 +217,14 @@ def _get_runtime_cfg(command):
 
     """
     try:
-        proc = subprocess.Popen(
+        proc = subprocess.run(
             command,
             stdout=subprocess.PIPE,
             stderr=subprocess.PIPE,
             universal_newlines=True,
+            check=False,
             env=util.env_no_snap_for_external_calls())
-        stdout, stderr = proc.communicate()
+        stdout, stderr = proc.stdout, proc.stderr
 
     except (OSError, ValueError):
         logger.error(

certbot-apache/certbot_apache/_internal/apacheparser.py

@@ -15,7 +15,7 @@ class ApacheParserNode(interfaces.ParserNode):
 
     def __init__(self, **kwargs):
         ancestor, dirty, filepath, metadata = util.parsernode_kwargs(kwargs)  # pylint: disable=unused-variable
-        super(ApacheParserNode, self).__init__(**kwargs)
+        super().__init__(**kwargs)
         self.ancestor = ancestor
         self.filepath = filepath
         self.dirty = dirty
@@ -39,7 +39,7 @@ class ApacheCommentNode(ApacheParserNode):
 
     def __init__(self, **kwargs):
         comment, kwargs = util.commentnode_kwargs(kwargs)  # pylint: disable=unused-variable
-        super(ApacheCommentNode, self).__init__(**kwargs)
+        super().__init__(**kwargs)
         self.comment = comment
 
     def __eq__(self, other):  # pragma: no cover
@@ -57,7 +57,7 @@ class ApacheDirectiveNode(ApacheParserNode):
 
     def __init__(self, **kwargs):
         name, parameters, enabled, kwargs = util.directivenode_kwargs(kwargs)
-        super(ApacheDirectiveNode, self).__init__(**kwargs)
+        super().__init__(**kwargs)
         self.name = name
         self.parameters = parameters
         self.enabled = enabled
@@ -83,7 +83,7 @@ class ApacheBlockNode(ApacheDirectiveNode):
     """ apacheconfig implementation of BlockNode interface """
 
     def __init__(self, **kwargs):
-        super(ApacheBlockNode, self).__init__(**kwargs)
+        super().__init__(**kwargs)
         self.children: Tuple[ApacheParserNode, ...] = ()
 
     def __eq__(self, other):  # pragma: no cover

certbot-apache/certbot_apache/_internal/assertions.py

@@ -136,6 +136,6 @@ def assertEqualPathsList(first, second):  # pragma: no cover
     if any(isPass(path) for path in second):
         return
     for fpath in first:
-        assert any([fnmatch.fnmatch(fpath, spath) for spath in second])
+        assert any(fnmatch.fnmatch(fpath, spath) for spath in second)
     for spath in second:
-        assert any([fnmatch.fnmatch(fpath, spath) for fpath in first])
+        assert any(fnmatch.fnmatch(fpath, spath) for fpath in first)

certbot-apache/certbot_apache/_internal/augeasparser.py

@@ -80,7 +80,7 @@ class AugeasParserNode(interfaces.ParserNode):
 
     def __init__(self, **kwargs):
         ancestor, dirty, filepath, metadata = util.parsernode_kwargs(kwargs)  # pylint: disable=unused-variable
-        super(AugeasParserNode, self).__init__(**kwargs)
+        super().__init__(**kwargs)
         self.ancestor = ancestor
         self.filepath = filepath
         self.dirty = dirty
@@ -169,7 +169,7 @@ class AugeasCommentNode(AugeasParserNode):
 
     def __init__(self, **kwargs):
         comment, kwargs = util.commentnode_kwargs(kwargs)  # pylint: disable=unused-variable
-        super(AugeasCommentNode, self).__init__(**kwargs)
+        super().__init__(**kwargs)
         # self.comment = comment
         self.comment = comment
 
@@ -188,7 +188,7 @@ class AugeasDirectiveNode(AugeasParserNode):
 
     def __init__(self, **kwargs):
         name, parameters, enabled, kwargs = util.directivenode_kwargs(kwargs)
-        super(AugeasDirectiveNode, self).__init__(**kwargs)
+        super().__init__(**kwargs)
         self.name = name
         self.enabled = enabled
         if parameters:
@@ -245,7 +245,7 @@ class AugeasBlockNode(AugeasDirectiveNode):
     """ Augeas implementation of BlockNode interface """
 
     def __init__(self, **kwargs):
-        super(AugeasBlockNode, self).__init__(**kwargs)
+        super().__init__(**kwargs)
         self.children = ()
 
     def __eq__(self, other):

certbot-apache/certbot_apache/_internal/configurator.py

@@ -8,16 +8,13 @@ import logging
 import re
 import socket
 import time
-from typing import cast
 from typing import DefaultDict
 from typing import Dict
 from typing import List
+from typing import Optional
 from typing import Set
 from typing import Union
 
-import zope.component
-import zope.interface
-
 from acme import challenges
 from certbot import errors
 from certbot import interfaces
@@ -25,6 +22,7 @@ from certbot import util
 from certbot.achallenges import KeyAuthorizationAnnotatedChallenge  # pylint: disable=unused-import
 from certbot.compat import filesystem
 from certbot.compat import os
+from certbot.display import util as display_util
 from certbot.plugins import common
 from certbot.plugins.enhancements import AutoHSTSEnhancement
 from certbot.plugins.util import path_surgery
@@ -36,6 +34,9 @@ from certbot_apache._internal import dualparser
 from certbot_apache._internal import http_01
 from certbot_apache._internal import obj
 from certbot_apache._internal import parser
+from certbot_apache._internal.dualparser import DualBlockNode
+from certbot_apache._internal.obj import VirtualHost
+from certbot_apache._internal.parser import ApacheParser
 
 try:
     import apacheconfig
@@ -47,6 +48,47 @@ except ImportError:  # pragma: no cover
 logger = logging.getLogger(__name__)
 
 
+class OsOptions:
+    """
+    Dedicated class to describe the OS specificities (eg. paths, binary names)
+    that the Apache configurator needs to be aware to operate properly.
+    """
+    def __init__(self,
+                 server_root="/etc/apache2",
+                 vhost_root="/etc/apache2/sites-available",
+                 vhost_files="*",
+                 logs_root="/var/log/apache2",
+                 ctl="apache2ctl",
+                 version_cmd: Optional[List[str]] = None,
+                 restart_cmd: Optional[List[str]] = None,
+                 restart_cmd_alt: Optional[List[str]] = None,
+                 conftest_cmd: Optional[List[str]] = None,
+                 enmod: Optional[str] = None,
+                 dismod: Optional[str] = None,
+                 le_vhost_ext="-le-ssl.conf",
+                 handle_modules=False,
+                 handle_sites=False,
+                 challenge_location="/etc/apache2",
+                 apache_bin: Optional[str] = None,
+                 ):
+        self.server_root = server_root
+        self.vhost_root = vhost_root
+        self.vhost_files = vhost_files
+        self.logs_root = logs_root
+        self.ctl = ctl
+        self.version_cmd = ['apache2ctl', '-v'] if not version_cmd else version_cmd
+        self.restart_cmd = ['apache2ctl', 'graceful'] if not restart_cmd else restart_cmd
+        self.restart_cmd_alt = restart_cmd_alt
+        self.conftest_cmd = ['apache2ctl', 'configtest'] if not conftest_cmd else conftest_cmd
+        self.enmod = enmod
+        self.dismod = dismod
+        self.le_vhost_ext = le_vhost_ext
+        self.handle_modules = handle_modules
+        self.handle_sites = handle_sites
+        self.challenge_location = challenge_location
+        self.bin = apache_bin
+
+
 # TODO: Augeas sections ie. <VirtualHost>, <IfModule> beginning and closing
 # tags need to be the same case, otherwise Augeas doesn't recognize them.
 # This is not able to be completely remedied by regular expressions because
@@ -75,14 +117,11 @@ logger = logging.getLogger(__name__)
 # TODO: Add directives to sites-enabled... not sites-available.
 #     sites-available doesn't allow immediate find_dir search even with save()
 #     and load()
-
-@zope.interface.implementer(interfaces.IAuthenticator, interfaces.IInstaller)
-@zope.interface.provider(interfaces.IPluginFactory)
-class ApacheConfigurator(common.Installer):
+class ApacheConfigurator(common.Installer, interfaces.Authenticator):
     """Apache configurator.
 
     :ivar config: Configuration.
-    :type config: :class:`~certbot.interfaces.IConfig`
+    :type config: certbot.configuration.NamespaceConfig
 
     :ivar parser: Handles low level parsing
     :type parser: :class:`~certbot_apache._internal.parser`
@@ -102,27 +141,7 @@ class ApacheConfigurator(common.Installer):
             " change depending on the operating system Certbot is run on.)"
         )
 
-    OS_DEFAULTS = dict(
-        server_root="/etc/apache2",
-        vhost_root="/etc/apache2/sites-available",
-        vhost_files="*",
-        logs_root="/var/log/apache2",
-        ctl="apache2ctl",
-        version_cmd=['apache2ctl', '-v'],
-        restart_cmd=['apache2ctl', 'graceful'],
-        conftest_cmd=['apache2ctl', 'configtest'],
-        enmod=None,
-        dismod=None,
-        le_vhost_ext="-le-ssl.conf",
-        handle_modules=False,
-        handle_sites=False,
-        challenge_location="/etc/apache2",
-        bin=None
-    )
-
-    def option(self, key):
-        """Get a value from options"""
-        return self.options.get(key)
+    OS_DEFAULTS = OsOptions()
 
     def pick_apache_config(self, warn_on_no_mod_ssl=True):
         """
@@ -152,14 +171,14 @@ class ApacheConfigurator(common.Installer):
         for o in opts:
             # Config options use dashes instead of underscores
             if self.conf(o.replace("_", "-")) is not None:
-                self.options[o] = self.conf(o.replace("_", "-"))
+                setattr(self.options, o, self.conf(o.replace("_", "-")))
             else:
-                self.options[o] = self.OS_DEFAULTS[o]
+                setattr(self.options, o, getattr(self.OS_DEFAULTS, o))
 
         # Special cases
-        cast(List[str], self.options["version_cmd"])[0] = self.option("ctl")
-        cast(List[str], self.options["restart_cmd"])[0] = self.option("ctl")
-        cast(List[str], self.options["conftest_cmd"])[0] = self.option("ctl")
+        self.options.version_cmd[0] = self.options.ctl
+        self.options.restart_cmd[0] = self.options.ctl
+        self.options.conftest_cmd[0] = self.options.ctl
 
     @classmethod
     def add_parser_arguments(cls, add):
@@ -174,30 +193,30 @@ class ApacheConfigurator(common.Installer):
         else:
             # cls.OS_DEFAULTS can be distribution specific, see override classes
             DEFAULTS = cls.OS_DEFAULTS
-        add("enmod", default=DEFAULTS["enmod"],
+        add("enmod", default=DEFAULTS.enmod,
             help="Path to the Apache 'a2enmod' binary")
-        add("dismod", default=DEFAULTS["dismod"],
+        add("dismod", default=DEFAULTS.dismod,
             help="Path to the Apache 'a2dismod' binary")
-        add("le-vhost-ext", default=DEFAULTS["le_vhost_ext"],
+        add("le-vhost-ext", default=DEFAULTS.le_vhost_ext,
             help="SSL vhost configuration extension")
-        add("server-root", default=DEFAULTS["server_root"],
+        add("server-root", default=DEFAULTS.server_root,
             help="Apache server root directory")
         add("vhost-root", default=None,
             help="Apache server VirtualHost configuration root")
-        add("logs-root", default=DEFAULTS["logs_root"],
+        add("logs-root", default=DEFAULTS.logs_root,
             help="Apache server logs directory")
         add("challenge-location",
-            default=DEFAULTS["challenge_location"],
+            default=DEFAULTS.challenge_location,
             help="Directory path for challenge configuration")
-        add("handle-modules", default=DEFAULTS["handle_modules"],
+        add("handle-modules", default=DEFAULTS.handle_modules,
             help="Let installer handle enabling required modules for you " +
                  "(Only Ubuntu/Debian currently)")
-        add("handle-sites", default=DEFAULTS["handle_sites"],
+        add("handle-sites", default=DEFAULTS.handle_sites,
             help="Let installer handle enabling sites for you " +
                  "(Only Ubuntu/Debian currently)")
-        add("ctl", default=DEFAULTS["ctl"],
+        add("ctl", default=DEFAULTS.ctl,
             help="Full path to Apache control script")
-        add("bin", default=DEFAULTS["bin"],
+        add("bin", default=DEFAULTS.bin,
             help="Full path to apache2/httpd binary")
 
     def __init__(self, *args, **kwargs):
@@ -210,7 +229,7 @@ class ApacheConfigurator(common.Installer):
         version = kwargs.pop("version", None)
         use_parsernode = kwargs.pop("use_parsernode", False)
         openssl_version = kwargs.pop("openssl_version", None)
-        super(ApacheConfigurator, self).__init__(*args, **kwargs)
+        super().__init__(*args, **kwargs)
 
         # Add name_server association dict
         self.assoc: Dict[str, obj.VirtualHost] = {}
@@ -232,11 +251,11 @@ class ApacheConfigurator(common.Installer):
         self.parsed_paths: List[str] = []
         # These will be set in the prepare function
         self._prepared = False
-        self.parser = None
-        self.parser_root = None
+        self.parser: ApacheParser
+        self.parser_root: Optional[DualBlockNode] = None
         self.version = version
         self._openssl_version = openssl_version
-        self.vhosts = None
+        self.vhosts: List[VirtualHost]
         self.options = copy.deepcopy(self.OS_DEFAULTS)
         self._enhance_func = {"redirect": self._enable_redirect,
                               "ensure-http-header": self._set_http_header,
@@ -286,8 +305,8 @@ class ApacheConfigurator(common.Installer):
             ssl_module_location = self.parser.standard_path_from_server_root(ssl_module_location)
         else:
             # Possibility B: ssl_module is statically linked into Apache
-            if self.option("bin"):
-                ssl_module_location = self.option("bin")
+            if self.options.bin:
+                ssl_module_location = self.options.bin
             else:
                 logger.warning("ssl_module is statically linked but --apache-bin is "
                                "missing; not disabling session tickets.")
@@ -317,7 +336,7 @@ class ApacheConfigurator(common.Installer):
         self._prepare_options()
 
         # Verify Apache is installed
-        self._verify_exe_availability(self.option("ctl"))
+        self._verify_exe_availability(self.options.ctl)
 
         # Make sure configuration is valid
         self.config_test()
@@ -345,8 +364,9 @@ class ApacheConfigurator(common.Installer):
                    "augeaspath": self.parser.get_root_augpath(),
                    "ac_ast": None}
         if self.USE_PARSERNODE:
-            self.parser_root = self.get_parsernode_root(pn_meta)
-            self.parsed_paths = self.parser_root.parsed_paths()
+            parser_root = self.get_parsernode_root(pn_meta)
+            self.parser_root = parser_root
+            self.parsed_paths = parser_root.parsed_paths()
 
         # Check for errors in parsing files with Augeas
         self.parser.check_parsing_errors("httpd.aug")
@@ -356,20 +376,20 @@ class ApacheConfigurator(common.Installer):
 
         # We may try to enable mod_ssl later. If so, we shouldn't warn if we can't find it now.
         # This is currently only true for debian/ubuntu.
-        warn_on_no_mod_ssl = not self.option("handle_modules")
+        warn_on_no_mod_ssl = not self.options.handle_modules
         self.install_ssl_options_conf(self.mod_ssl_conf,
                                       self.updated_mod_ssl_conf_digest,
                                       warn_on_no_mod_ssl)
 
         # Prevent two Apache plugins from modifying a config at once
         try:
-            util.lock_dir_until_exit(self.option("server_root"))
+            util.lock_dir_until_exit(self.options.server_root)
         except (OSError, errors.LockError):
             logger.debug("Encountered error:", exc_info=True)
             raise errors.PluginError(
                 "Unable to create a lock file in {0}. Are you running"
                 " Certbot with sufficient privileges to modify your"
-                " Apache configuration?".format(self.option("server_root")))
+                " Apache configuration?".format(self.options.server_root))
         self._prepared = True
 
     def save(self, title=None, temporary=False):
@@ -405,10 +425,10 @@ class ApacheConfigurator(common.Installer):
         :raises .errors.PluginError: If unable to recover the configuration
 
         """
-        super(ApacheConfigurator, self).recovery_routine()
+        super().recovery_routine()
         # Reload configuration after these changes take effect if needed
         # ie. ApacheParser has been initialized.
-        if self.parser:
+        if hasattr(self, "parser"):
             # TODO: wrap into non-implementation specific  parser interface
             self.parser.aug.load()
 
@@ -430,7 +450,7 @@ class ApacheConfigurator(common.Installer):
             the function is unable to correctly revert the configuration
 
         """
-        super(ApacheConfigurator, self).rollback_checkpoints(rollback)
+        super().rollback_checkpoints(rollback)
         self.parser.aug.load()
 
     def _verify_exe_availability(self, exe):
@@ -444,7 +464,7 @@ class ApacheConfigurator(common.Installer):
         """Initializes the ApacheParser"""
         # If user provided vhost_root value in command line, use it
         return parser.ApacheParser(
-            self.option("server_root"), self.conf("vhost-root"),
+            self.options.server_root, self.conf("vhost-root"),
             self.version, configurator=self)
 
     def get_parsernode_root(self, metadata):
@@ -452,9 +472,9 @@ class ApacheConfigurator(common.Installer):
 
         if HAS_APACHECONFIG:
             apache_vars = {}
-            apache_vars["defines"] = apache_util.parse_defines(self.option("ctl"))
-            apache_vars["includes"] = apache_util.parse_includes(self.option("ctl"))
-            apache_vars["modules"] = apache_util.parse_modules(self.option("ctl"))
+            apache_vars["defines"] = apache_util.parse_defines(self.options.ctl)
+            apache_vars["includes"] = apache_util.parse_includes(self.options.ctl)
+            apache_vars["modules"] = apache_util.parse_modules(self.options.ctl)
             metadata["apache_vars"] = apache_vars
 
             with open(self.parser.loc["root"]) as f:
@@ -490,6 +510,8 @@ class ApacheConfigurator(common.Installer):
         vhosts = self.choose_vhosts(domain)
         for vhost in vhosts:
             self._deploy_cert(vhost, cert_path, key_path, chain_path, fullchain_path)
+            display_util.notify("Successfully deployed certificate for {} to {}"
+                                .format(domain, vhost.filep))
 
     def choose_vhosts(self, domain, create_if_no_ssl=True):
         """
@@ -528,6 +550,19 @@ class ApacheConfigurator(common.Installer):
 
         return list(matched)
 
+    def _raise_no_suitable_vhost_error(self, target_name: str):
+        """
+        Notifies the user that Certbot could not find a vhost to secure
+        and raises an error.
+        :param str target_name: The server name that could not be mapped
+        :raises errors.PluginError: Raised unconditionally
+        """
+        raise errors.PluginError(
+            "Certbot could not find a VirtualHost for {0} in the Apache "
+            "configuration. Please create a VirtualHost with a ServerName "
+            "matching {0} and try again.".format(target_name)
+        )
+
     def _in_wildcard_scope(self, name, domain):
         """
         Helper method for _vhosts_for_wildcard() that makes sure that the domain
@@ -565,12 +600,7 @@ class ApacheConfigurator(common.Installer):
         dialog_output = display_ops.select_vhost_multiple(list(dialog_input))
 
         if not dialog_output:
-            logger.error(
-                "No vhost exists with servername or alias for domain %s. "
-                "No vhost was selected. Please specify ServerName or ServerAlias "
-                "in the Apache config.",
-                domain)
-            raise errors.PluginError("No vhost selected")
+            self._raise_no_suitable_vhost_error(domain)
 
         # Make sure we create SSL vhosts for the ones that are HTTP only
         # if requested.
@@ -694,12 +724,7 @@ class ApacheConfigurator(common.Installer):
         # Select a vhost from a list
         vhost = display_ops.select_vhost(target_name, self.vhosts)
         if vhost is None:
-            logger.error(
-                "No vhost exists with servername or alias of %s. "
-                "No vhost was selected. Please specify ServerName or ServerAlias "
-                "in the Apache config.",
-                target_name)
-            raise errors.PluginError("No vhost selected")
+            self._raise_no_suitable_vhost_error(target_name)
         if temp:
             return vhost
         if not vhost.ssl:
@@ -853,7 +878,7 @@ class ApacheConfigurator(common.Installer):
                         all_names.add(name)
 
         if vhost_macro:
-            zope.component.getUtility(interfaces.IDisplay).notification(
+            display_util.notification(
                 "Apache mod_macro seems to be in use in file(s):\n{0}"
                 "\n\nUnfortunately mod_macro is not yet supported".format(
                     "\n  ".join(vhost_macro)), force_interactive=True)
@@ -1051,6 +1076,9 @@ class ApacheConfigurator(common.Installer):
         :rtype: list
         """
 
+        if not self.parser_root:
+            raise errors.Error("This ApacheConfigurator instance is not"  # pragma: no cover
+                               " configured to use a node parser.")
         vhs = []
         vhosts = self.parser_root.find_blocks("VirtualHost", exclude=False)
         for vhblock in vhosts:
@@ -1303,7 +1331,7 @@ class ApacheConfigurator(common.Installer):
         :param boolean temp: If the change is temporary
         """
 
-        if self.option("handle_modules"):
+        if self.options.handle_modules:
             if self.version >= (2, 4) and ("socache_shmcb_module" not in
                                            self.parser.modules):
                 self.enable_mod("socache_shmcb", temp=temp)
@@ -1323,7 +1351,7 @@ class ApacheConfigurator(common.Installer):
 
         Duplicates vhost and adds default ssl options
         New vhost will reside as (nonssl_vhost.path) +
-        ``self.option("le_vhost_ext")``
+        ``self.options.le_vhost_ext``
 
         .. note:: This function saves the configuration
 
@@ -1422,15 +1450,15 @@ class ApacheConfigurator(common.Installer):
         """
 
         if self.conf("vhost-root") and os.path.exists(self.conf("vhost-root")):
-            fp = os.path.join(filesystem.realpath(self.option("vhost_root")),
+            fp = os.path.join(filesystem.realpath(self.options.vhost_root),
                               os.path.basename(non_ssl_vh_fp))
         else:
             # Use non-ssl filepath
             fp = filesystem.realpath(non_ssl_vh_fp)
 
         if fp.endswith(".conf"):
-            return fp[:-(len(".conf"))] + self.option("le_vhost_ext")
-        return fp + self.option("le_vhost_ext")
+            return fp[:-(len(".conf"))] + self.options.le_vhost_ext
+        return fp + self.options.le_vhost_ext
 
     def _sift_rewrite_rule(self, line):
         """Decides whether a line should be copied to a SSL vhost.
@@ -1504,12 +1532,11 @@ class ApacheConfigurator(common.Installer):
             raise errors.PluginError("Unable to write/read in make_vhost_ssl")
 
         if sift:
-            reporter = zope.component.getUtility(interfaces.IReporter)
-            reporter.add_message(
-                "Some rewrite rules copied from {0} were disabled in the "
-                "vhost for your HTTPS site located at {1} because they have "
-                "the potential to create redirection loops.".format(
-                    vhost.filep, ssl_fp), reporter.MEDIUM_PRIORITY)
+            display_util.notify(
+                f"Some rewrite rules copied from {vhost.filep} were disabled in the "
+                f"vhost for your HTTPS site located at {ssl_fp} because they have "
+                "the potential to create redirection loops."
+            )
         self.parser.aug.set("/augeas/files%s/mtime" % (self._escape(ssl_fp)), "0")
         self.parser.aug.set("/augeas/files%s/mtime" % (self._escape(vhost.filep)), "0")
 
@@ -1838,13 +1865,13 @@ class ApacheConfigurator(common.Installer):
             if options:
                 msg_enhancement += ": " + options
             msg = msg_tmpl.format(domain, msg_enhancement)
-            logger.warning(msg)
+            logger.error(msg)
             raise errors.PluginError(msg)
         try:
             for vhost in vhosts:
                 func(vhost, options)
         except errors.PluginError:
-            logger.warning("Failed %s for %s", enhancement, domain)
+            logger.error("Failed %s for %s", enhancement, domain)
             raise
 
     def _autohsts_increase(self, vhost, id_str, nextstep):
@@ -2274,7 +2301,7 @@ class ApacheConfigurator(common.Installer):
                             addr in self._get_proposed_addrs(ssl_vhost)),
                    servername, serveralias,
                    " ".join(rewrite_rule_args),
-                   self.option("logs_root")))
+                   self.options.logs_root))
 
     def _write_out_redirect(self, ssl_vhost, text):
         # This is the default name
@@ -2286,7 +2313,7 @@ class ApacheConfigurator(common.Installer):
             if len(ssl_vhost.name) < (255 - (len(redirect_filename) + 1)):
                 redirect_filename = "le-redirect-%s.conf" % ssl_vhost.name
 
-        redirect_filepath = os.path.join(self.option("vhost_root"),
+        redirect_filepath = os.path.join(self.options.vhost_root,
                                          redirect_filename)
 
         # Register the new file that will be created
@@ -2368,7 +2395,7 @@ class ApacheConfigurator(common.Installer):
             vhost.enabled = True
         return
 
-    def enable_mod(self, mod_name, temp=False):
+    def enable_mod(self, mod_name, temp=False):  # pylint: disable=unused-argument
         """Enables module in Apache.
 
         Both enables and reloads Apache so module is active.
@@ -2406,19 +2433,18 @@ class ApacheConfigurator(common.Installer):
 
         """
         try:
-            util.run_script(self.option("restart_cmd"))
+            util.run_script(self.options.restart_cmd)
         except errors.SubprocessError as err:
-            logger.info("Unable to restart apache using %s",
-                        self.option("restart_cmd"))
-            alt_restart = self.option("restart_cmd_alt")
+            logger.warning("Unable to restart apache using %s",
+                        self.options.restart_cmd)
+            alt_restart = self.options.restart_cmd_alt
             if alt_restart:
                 logger.debug("Trying alternative restart command: %s",
                              alt_restart)
                 # There is an alternative restart command available
                 # This usually is "restart" verb while original is "graceful"
                 try:
-                    util.run_script(self.option(
-                        "restart_cmd_alt"))
+                    util.run_script(self.options.restart_cmd_alt)
                     return
                 except errors.SubprocessError as secerr:
                     error = str(secerr)
@@ -2433,7 +2459,7 @@ class ApacheConfigurator(common.Installer):
 
         """
         try:
-            util.run_script(self.option("conftest_cmd"))
+            util.run_script(self.options.conftest_cmd)
         except errors.SubprocessError as err:
             raise errors.MisconfigurationError(str(err))
 
@@ -2449,11 +2475,11 @@ class ApacheConfigurator(common.Installer):
 
         """
         try:
-            stdout, _ = util.run_script(self.option("version_cmd"))
+            stdout, _ = util.run_script(self.options.version_cmd)
         except errors.SubprocessError:
             raise errors.PluginError(
                 "Unable to run %s -v" %
-                self.option("version_cmd"))
+                self.options.version_cmd)
 
         regex = re.compile(r"Apache/([0-9\.]*)", re.IGNORECASE)
         matches = regex.findall(stdout)
@@ -2473,6 +2499,11 @@ class ApacheConfigurator(common.Installer):
                 version=".".join(str(i) for i in self.version))
         )
 
+    def auth_hint(self, failed_achalls): # pragma: no cover
+        return ("The Certificate Authority failed to verify the temporary Apache configuration "
+                "changes made by Certbot. Ensure that the listed domains point to this Apache "
+                "server and that it is accessible from the internet.")
+
     ###########################################################################
     # Challenges Section
     ###########################################################################
@@ -2566,7 +2597,7 @@ class ApacheConfigurator(common.Installer):
                 msg_tmpl = ("Certbot was not able to find SSL VirtualHost for a "
                             "domain {0} for enabling AutoHSTS enhancement.")
                 msg = msg_tmpl.format(d)
-                logger.warning(msg)
+                logger.error(msg)
                 raise errors.PluginError(msg)
             for vh in vhosts:
                 try:
@@ -2652,7 +2683,7 @@ class ApacheConfigurator(common.Installer):
                 except errors.PluginError:
                     msg = ("Could not find VirtualHost with ID {0}, disabling "
                            "AutoHSTS for this VirtualHost").format(id_str)
-                    logger.warning(msg)
+                    logger.error(msg)
                     # Remove the orphaned AutoHSTS entry from pluginstorage
                     self._autohsts.pop(id_str)
                     continue
@@ -2692,7 +2723,7 @@ class ApacheConfigurator(common.Installer):
                 except errors.PluginError:
                     msg = ("VirtualHost with id {} was not found, unable to "
                            "make HSTS max-age permanent.").format(id_str)
-                    logger.warning(msg)
+                    logger.error(msg)
                     self._autohsts.pop(id_str)
                     continue
                 if self._autohsts_vhost_in_lineage(vhost, lineage):

certbot-apache/certbot_apache/_internal/constants.py

@@ -4,11 +4,13 @@ import pkg_resources
 from certbot.compat import os
 
 MOD_SSL_CONF_DEST = "options-ssl-apache.conf"
-"""Name of the mod_ssl config file as saved in `IConfig.config_dir`."""
+"""Name of the mod_ssl config file as saved
+in `certbot.configuration.NamespaceConfig.config_dir`."""
 
 
 UPDATED_MOD_SSL_CONF_DIGEST = ".updated-options-ssl-apache-conf-digest.txt"
-"""Name of the hash of the updated or informed mod_ssl_conf as saved in `IConfig.config_dir`."""
+"""Name of the hash of the updated or informed mod_ssl_conf as saved
+in `certbot.configuration.NamespaceConfig.config_dir`."""
 
 # NEVER REMOVE A SINGLE HASH FROM THIS LIST UNLESS YOU KNOW EXACTLY WHAT YOU ARE DOING!
 ALL_SSL_OPTIONS_HASHES = [

certbot-apache/certbot_apache/_internal/display_ops.py

@@ -1,12 +1,9 @@
 """Contains UI methods for Apache operations."""
 import logging
 
-import zope.component
-
 from certbot import errors
-from certbot import interfaces
 from certbot.compat import os
-import certbot.display.util as display_util
+from certbot.display import util as display_util
 
 logger = logging.getLogger(__name__)
 
@@ -26,7 +23,7 @@ def select_vhost_multiple(vhosts):
     # Remove the extra newline from the last entry
     if tags_list:
         tags_list[-1] = tags_list[-1][:-1]
-    code, names = zope.component.getUtility(interfaces.IDisplay).checklist(
+    code, names = display_util.checklist(
         "Which VirtualHosts would you like to install the wildcard certificate for?",
         tags=tags_list, force_interactive=True)
     if code == display_util.OK:
@@ -34,6 +31,7 @@ def select_vhost_multiple(vhosts):
         return return_vhosts
     return []
 
+
 def _reversemap_vhosts(names, vhosts):
     """Helper function for select_vhost_multiple for mapping string
     representations back to actual vhost objects"""
@@ -45,6 +43,7 @@ def _reversemap_vhosts(names, vhosts):
                 return_vhosts.append(vhost)
     return return_vhosts
 
+
 def select_vhost(domain, vhosts):
     """Select an appropriate Apache Vhost.
 
@@ -62,6 +61,7 @@ def select_vhost(domain, vhosts):
         return vhosts[tag]
     return None
 
+
 def _vhost_menu(domain, vhosts):
     """Select an appropriate Apache Vhost.
 
@@ -107,7 +107,7 @@ def _vhost_menu(domain, vhosts):
         )
 
     try:
-        code, tag = zope.component.getUtility(interfaces.IDisplay).menu(
+        code, tag = display_util.menu(
             "We were unable to find a vhost with a ServerName "
             "or Address of {0}.{1}Which virtual host would you "
             "like to choose?".format(domain, os.linesep),
@@ -119,7 +119,7 @@ def _vhost_menu(domain, vhosts):
             "guidance in non-interactive mode. Certbot may need "
             "vhosts to be explicitly labelled with ServerName or "
             "ServerAlias directives.".format(domain))
-        logger.warning(msg)
+        logger.error(msg)
         raise errors.MissingCommandlineFlag(msg)
 
     return code, tag

certbot-apache/certbot_apache/_internal/entrypoint.py

@@ -10,6 +10,7 @@ from certbot_apache._internal import override_debian
 from certbot_apache._internal import override_fedora
 from certbot_apache._internal import override_gentoo
 from certbot_apache._internal import override_suse
+from certbot_apache._internal import override_void
 
 OVERRIDE_CLASSES = {
     "arch": override_arch.ArchConfigurator,
@@ -35,6 +36,7 @@ OVERRIDE_CLASSES = {
     "sles": override_suse.OpenSUSEConfigurator,
     "scientific": override_centos.CentOSConfigurator,
     "scientific linux": override_centos.CentOSConfigurator,
+    "void": override_void.VoidConfigurator,
 }
 
 

certbot-apache/certbot_apache/_internal/http_01.py

@@ -47,7 +47,7 @@ class ApacheHttp01(common.ChallengePerformer):
     """
 
     def __init__(self, *args, **kwargs):
-        super(ApacheHttp01, self).__init__(*args, **kwargs)
+        super().__init__(*args, **kwargs)
         self.challenge_conf_pre = os.path.join(
             self.configurator.conf("challenge-location"),
             "le_http_01_challenge_pre.conf")
@@ -95,10 +95,10 @@ class ApacheHttp01(common.ChallengePerformer):
     def _mod_config(self):
         selected_vhosts: List[VirtualHost] = []
         http_port = str(self.configurator.config.http01_port)
+
+        # Search for VirtualHosts matching by name
         for chall in self.achalls:
-            # Search for matching VirtualHosts
-            for vh in self._matching_vhosts(chall.domain):
-                selected_vhosts.append(vh)
+            selected_vhosts += self._matching_vhosts(chall.domain)
 
         # Ensure that we have one or more VirtualHosts that we can continue
         # with. (one that listens to port configured with --http-01-port)
@@ -107,9 +107,13 @@ class ApacheHttp01(common.ChallengePerformer):
             if any(a.is_wildcard() or a.get_port() == http_port for a in vhost.addrs):
                 found = True
 
-        if not found:
-            for vh in self._relevant_vhosts():
-                selected_vhosts.append(vh)
+        # If there's at least one eligible VirtualHost, also add all unnamed VirtualHosts
+        # because they might match at runtime (#8890)
+        if found:
+            selected_vhosts += self._unnamed_vhosts()
+        # Otherwise, add every Virtualhost which listens on the right port
+        else:
+            selected_vhosts += self._relevant_vhosts()
 
         # Add the challenge configuration
         for vh in selected_vhosts:
@@ -167,6 +171,10 @@ class ApacheHttp01(common.ChallengePerformer):
 
         return relevant_vhosts
 
+    def _unnamed_vhosts(self) -> List[VirtualHost]:
+        """Return all VirtualHost objects with no ServerName"""
+        return [vh for vh in self.configurator.vhosts if vh.name is None]
+
     def _set_up_challenges(self):
         if not os.path.isdir(self.challenge_dir):
             old_umask = filesystem.umask(0o022)

certbot-apache/certbot_apache/_internal/interfaces.py

@@ -238,7 +238,7 @@ class CommentNode(ParserNode, metaclass=abc.ABCMeta):
             created or changed after the last save. Default: False.
         :type dirty: bool
         """
-        super(CommentNode, self).__init__(ancestor=kwargs['ancestor'],
+        super().__init__(ancestor=kwargs['ancestor'],
                                           dirty=kwargs.get('dirty', False),
                                           filepath=kwargs['filepath'],
                                           metadata=kwargs.get('metadata', {}))  # pragma: no cover
@@ -302,7 +302,7 @@ class DirectiveNode(ParserNode, metaclass=abc.ABCMeta):
         :type enabled: bool
 
         """
-        super(DirectiveNode, self).__init__(ancestor=kwargs['ancestor'],
+        super().__init__(ancestor=kwargs['ancestor'],
                                             dirty=kwargs.get('dirty', False),
                                             filepath=kwargs['filepath'],
                                             metadata=kwargs.get('metadata', {}))  # pragma: no cover
@@ -312,7 +312,7 @@ class DirectiveNode(ParserNode, metaclass=abc.ABCMeta):
         """
         Sets the sequence of parameters for this ParserNode object without
         whitespaces. While the whitespaces for parameters are discarded when using
-        this method, the whitespacing preceeding the ParserNode itself should be
+        this method, the whitespacing preceding the ParserNode itself should be
         kept intact.
 
         :param list parameters: sequence of parameters
@@ -364,7 +364,7 @@ class BlockNode(DirectiveNode, metaclass=abc.ABCMeta):
     def add_child_block(self, name, parameters=None, position=None):
         """
         Adds a new BlockNode child node with provided values and marks the callee
-        BlockNode dirty. This is used to add new children to the AST. The preceeding
+        BlockNode dirty. This is used to add new children to the AST. The preceding
         whitespaces should not be added based on the ancestor or siblings for the
         newly created object. This is to match the current behavior of the legacy
         parser implementation.
@@ -385,7 +385,7 @@ class BlockNode(DirectiveNode, metaclass=abc.ABCMeta):
         """
         Adds a new DirectiveNode child node with provided values and marks the
         callee BlockNode dirty. This is used to add new children to the AST. The
-        preceeding whitespaces should not be added based on the ancestor or siblings
+        preceding whitespaces should not be added based on the ancestor or siblings
         for the newly created object. This is to match the current behavior of the
         legacy parser implementation.
 
@@ -406,7 +406,7 @@ class BlockNode(DirectiveNode, metaclass=abc.ABCMeta):
         """
         Adds a new CommentNode child node with provided value and marks the
         callee BlockNode dirty. This is used to add new children to the AST. The
-        preceeding whitespaces should not be added based on the ancestor or siblings
+        preceding whitespaces should not be added based on the ancestor or siblings
         for the newly created object. This is to match the current behavior of the
         legacy parser implementation.
 

certbot-apache/certbot_apache/_internal/obj.py

@@ -26,7 +26,7 @@ class Addr(common.Addr):
     def __hash__(self):  # pylint: disable=useless-super-delegation
         # Python 3 requires explicit overridden for __hash__ if __eq__ or
         # __cmp__ is overridden. See https://bugs.python.org/issue2235
-        return super(Addr, self).__hash__()
+        return super().__hash__()
 
     def _addr_less_specific(self, addr):
         """Returns if addr.get_addr() is more specific than self.get_addr()."""

certbot-apache/certbot_apache/_internal/override_arch.py

@@ -1,15 +1,12 @@
 """ Distribution specific override class for Arch Linux """
-import zope.interface
-
-from certbot import interfaces
 from certbot_apache._internal import configurator
+from certbot_apache._internal.configurator import OsOptions
 
 
-@zope.interface.provider(interfaces.IPluginFactory)
 class ArchConfigurator(configurator.ApacheConfigurator):
     """Arch Linux specific ApacheConfigurator override class"""
 
-    OS_DEFAULTS = dict(
+    OS_DEFAULTS = OsOptions(
         server_root="/etc/httpd",
         vhost_root="/etc/httpd/conf",
         vhost_files="*.conf",
@@ -18,11 +15,5 @@ class ArchConfigurator(configurator.ApacheConfigurator):
         version_cmd=['apachectl', '-v'],
         restart_cmd=['apachectl', 'graceful'],
         conftest_cmd=['apachectl', 'configtest'],
-        enmod=None,
-        dismod=None,
-        le_vhost_ext="-le-ssl.conf",
-        handle_modules=False,
-        handle_sites=False,
         challenge_location="/etc/httpd/conf",
-        bin=None,
     )

certbot-apache/certbot_apache/_internal/override_centos.py

@@ -3,24 +3,21 @@ import logging
 from typing import cast
 from typing import List
 
-import zope.interface
-
 from certbot import errors
-from certbot import interfaces
 from certbot import util
 from certbot.errors import MisconfigurationError
 from certbot_apache._internal import apache_util
 from certbot_apache._internal import configurator
 from certbot_apache._internal import parser
+from certbot_apache._internal.configurator import OsOptions
 
 logger = logging.getLogger(__name__)
 
 
-@zope.interface.provider(interfaces.IPluginFactory)
 class CentOSConfigurator(configurator.ApacheConfigurator):
     """CentOS specific ApacheConfigurator override class"""
 
-    OS_DEFAULTS = dict(
+    OS_DEFAULTS = OsOptions(
         server_root="/etc/httpd",
         vhost_root="/etc/httpd/conf.d",
         vhost_files="*.conf",
@@ -30,13 +27,7 @@ class CentOSConfigurator(configurator.ApacheConfigurator):
         restart_cmd=['apachectl', 'graceful'],
         restart_cmd_alt=['apachectl', 'restart'],
         conftest_cmd=['apachectl', 'configtest'],
-        enmod=None,
-        dismod=None,
-        le_vhost_ext="-le-ssl.conf",
-        handle_modules=False,
-        handle_sites=False,
         challenge_location="/etc/httpd/conf.d",
-        bin=None,
     )
 
     def config_test(self):
@@ -51,7 +42,7 @@ class CentOSConfigurator(configurator.ApacheConfigurator):
         fedora = os_info[0].lower() == "fedora"
 
         try:
-            super(CentOSConfigurator, self).config_test()
+            super().config_test()
         except errors.MisconfigurationError:
             if fedora:
                 self._try_restart_fedora()
@@ -60,7 +51,7 @@ class CentOSConfigurator(configurator.ApacheConfigurator):
 
     def _try_restart_fedora(self):
         """
-        Tries to restart httpd using systemctl to generate the self signed keypair.
+        Tries to restart httpd using systemctl to generate the self signed key pair.
         """
 
         try:
@@ -69,20 +60,22 @@ class CentOSConfigurator(configurator.ApacheConfigurator):
             raise errors.MisconfigurationError(str(err))
 
         # Finish with actual config check to see if systemctl restart helped
-        super(CentOSConfigurator, self).config_test()
+        super().config_test()
 
     def _prepare_options(self):
         """
         Override the options dictionary initialization in order to support
         alternative restart cmd used in CentOS.
         """
-        super(CentOSConfigurator, self)._prepare_options()
-        cast(List[str], self.options["restart_cmd_alt"])[0] = self.option("ctl")
+        super()._prepare_options()
+        if not self.options.restart_cmd_alt:  # pragma: no cover
+            raise ValueError("OS option restart_cmd_alt must be set for CentOS.")
+        self.options.restart_cmd_alt[0] = self.options.ctl
 
     def get_parser(self):
         """Initializes the ApacheParser"""
         return CentOSParser(
-            self.option("server_root"), self.option("vhost_root"),
+            self.options.server_root, self.options.vhost_root,
             self.version, configurator=self)
 
     def _deploy_cert(self, *args, **kwargs):  # pylint: disable=arguments-differ
@@ -91,7 +84,7 @@ class CentOSConfigurator(configurator.ApacheConfigurator):
         has "LoadModule ssl_module..." before parsing the VirtualHost configuration
         that was created by Certbot
         """
-        super(CentOSConfigurator, self)._deploy_cert(*args, **kwargs)
+        super()._deploy_cert(*args, **kwargs)
         if self.version < (2, 4, 0):
             self._deploy_loadmodule_ssl_if_needed()
 
@@ -119,8 +112,9 @@ class CentOSConfigurator(configurator.ApacheConfigurator):
             else:
                 loadmod_args = path_args
 
-            if self.parser.not_modssl_ifmodule(noarg_path):  # pylint: disable=no-member
-                if self.parser.loc["default"] in noarg_path:
+            centos_parser: CentOSParser = cast(CentOSParser, self.parser)
+            if centos_parser.not_modssl_ifmodule(noarg_path):
+                if centos_parser.loc["default"] in noarg_path:
                     # LoadModule already in the main configuration file
                     if ("ifmodule/" in noarg_path.lower() or
                         "ifmodule[1]" in noarg_path.lower()):
@@ -168,19 +162,19 @@ class CentOSParser(parser.ApacheParser):
     def __init__(self, *args, **kwargs):
         # CentOS specific configuration file for Apache
         self.sysconfig_filep = "/etc/sysconfig/httpd"
-        super(CentOSParser, self).__init__(*args, **kwargs)
+        super().__init__(*args, **kwargs)
 
     def update_runtime_variables(self):
         """ Override for update_runtime_variables for custom parsing """
         # Opportunistic, works if SELinux not enforced
-        super(CentOSParser, self).update_runtime_variables()
+        super().update_runtime_variables()
         self.parse_sysconfig_var()
 
     def parse_sysconfig_var(self):
         """ Parses Apache CLI options from CentOS configuration file """
         defines = apache_util.parse_define_file(self.sysconfig_filep, "OPTIONS")
-        for k in defines:
-            self.variables[k] = defines[k]
+        for k, v in defines.items():
+            self.variables[k] = v
 
     def not_modssl_ifmodule(self, path):
         """Checks if the provided Augeas path has argument !mod_ssl"""

certbot-apache/certbot_apache/_internal/override_darwin.py

@@ -1,28 +1,17 @@
 """ Distribution specific override class for macOS """
-import zope.interface
-
-from certbot import interfaces
 from certbot_apache._internal import configurator
+from certbot_apache._internal.configurator import OsOptions
 
 
-@zope.interface.provider(interfaces.IPluginFactory)
 class DarwinConfigurator(configurator.ApacheConfigurator):
     """macOS specific ApacheConfigurator override class"""
 
-    OS_DEFAULTS = dict(
-        server_root="/etc/apache2",
+    OS_DEFAULTS = OsOptions(
         vhost_root="/etc/apache2/other",
         vhost_files="*.conf",
-        logs_root="/var/log/apache2",
         ctl="apachectl",
         version_cmd=['apachectl', '-v'],
         restart_cmd=['apachectl', 'graceful'],
         conftest_cmd=['apachectl', 'configtest'],
-        enmod=None,
-        dismod=None,
-        le_vhost_ext="-le-ssl.conf",
-        handle_modules=False,
-        handle_sites=False,
         challenge_location="/etc/apache2/other",
-        bin=None,
     )

certbot-apache/certbot_apache/_internal/override_debian.py

@@ -1,39 +1,25 @@
 """ Distribution specific override class for Debian family (Ubuntu/Debian) """
 import logging
 
-import zope.interface
-
 from certbot import errors
-from certbot import interfaces
 from certbot import util
 from certbot.compat import filesystem
 from certbot.compat import os
 from certbot_apache._internal import apache_util
 from certbot_apache._internal import configurator
+from certbot_apache._internal.configurator import OsOptions
 
 logger = logging.getLogger(__name__)
 
 
-@zope.interface.provider(interfaces.IPluginFactory)
 class DebianConfigurator(configurator.ApacheConfigurator):
     """Debian specific ApacheConfigurator override class"""
 
-    OS_DEFAULTS = dict(
-        server_root="/etc/apache2",
-        vhost_root="/etc/apache2/sites-available",
-        vhost_files="*",
-        logs_root="/var/log/apache2",
-        ctl="apache2ctl",
-        version_cmd=['apache2ctl', '-v'],
-        restart_cmd=['apache2ctl', 'graceful'],
-        conftest_cmd=['apache2ctl', 'configtest'],
+    OS_DEFAULTS = OsOptions(
         enmod="a2enmod",
         dismod="a2dismod",
-        le_vhost_ext="-le-ssl.conf",
         handle_modules=True,
         handle_sites=True,
-        challenge_location="/etc/apache2",
-        bin=None,
     )
 
     def enable_site(self, vhost):
@@ -58,7 +44,7 @@ class DebianConfigurator(configurator.ApacheConfigurator):
         if not os.path.isdir(os.path.dirname(enabled_path)):
             # For some reason, sites-enabled / sites-available do not exist
             # Call the parent method
-            return super(DebianConfigurator, self).enable_site(vhost)
+            return super().enable_site(vhost)
         self.reverter.register_file_creation(False, enabled_path)
         try:
             os.symlink(vhost.filep, enabled_path)
@@ -68,7 +54,7 @@ class DebianConfigurator(configurator.ApacheConfigurator):
                 # Already in shape
                 vhost.enabled = True
                 return None
-            logger.warning(
+            logger.error(
                 "Could not symlink %s to %s, got error: %s", enabled_path,
                 vhost.filep, err.strerror)
             errstring = ("Encountered error while trying to enable a " +
@@ -132,11 +118,11 @@ class DebianConfigurator(configurator.ApacheConfigurator):
         # Generate reversal command.
         # Try to be safe here... check that we can probably reverse before
         # applying enmod command
-        if not util.exe_exists(self.option("dismod")):
+        if not util.exe_exists(self.options.dismod):
             raise errors.MisconfigurationError(
                 "Unable to find a2dismod, please make sure a2enmod and "
                 "a2dismod are configured correctly for certbot.")
 
         self.reverter.register_undo_command(
-            temp, [self.option("dismod"), "-f", mod_name])
-        util.run_script([self.option("enmod"), mod_name])
+            temp, [self.options.dismod, "-f", mod_name])
+        util.run_script([self.options.enmod, mod_name])

certbot-apache/certbot_apache/_internal/override_fedora.py

@@ -1,22 +1,16 @@
 """ Distribution specific override class for Fedora 29+ """
-from typing import cast
-from typing import List
-
-import zope.interface
-
 from certbot import errors
-from certbot import interfaces
 from certbot import util
 from certbot_apache._internal import apache_util
 from certbot_apache._internal import configurator
 from certbot_apache._internal import parser
+from certbot_apache._internal.configurator import OsOptions
 
 
-@zope.interface.provider(interfaces.IPluginFactory)
 class FedoraConfigurator(configurator.ApacheConfigurator):
     """Fedora 29+ specific ApacheConfigurator override class"""
 
-    OS_DEFAULTS = dict(
+    OS_DEFAULTS = OsOptions(
         server_root="/etc/httpd",
         vhost_root="/etc/httpd/conf.d",
         vhost_files="*.conf",
@@ -26,13 +20,7 @@ class FedoraConfigurator(configurator.ApacheConfigurator):
         restart_cmd=['apachectl', 'graceful'],
         restart_cmd_alt=['apachectl', 'restart'],
         conftest_cmd=['apachectl', 'configtest'],
-        enmod=None,
-        dismod=None,
-        le_vhost_ext="-le-ssl.conf",
-        handle_modules=False,
-        handle_sites=False,
         challenge_location="/etc/httpd/conf.d",
-        bin=None,
     )
 
     def config_test(self):
@@ -43,19 +31,19 @@ class FedoraConfigurator(configurator.ApacheConfigurator):
         during the first (re)start of httpd.
         """
         try:
-            super(FedoraConfigurator, self).config_test()
+            super().config_test()
         except errors.MisconfigurationError:
             self._try_restart_fedora()
 
     def get_parser(self):
         """Initializes the ApacheParser"""
         return FedoraParser(
-            self.option("server_root"), self.option("vhost_root"),
+            self.options.server_root, self.options.vhost_root,
             self.version, configurator=self)
 
     def _try_restart_fedora(self):
         """
-        Tries to restart httpd using systemctl to generate the self signed keypair.
+        Tries to restart httpd using systemctl to generate the self signed key pair.
         """
         try:
             util.run_script(['systemctl', 'restart', 'httpd'])
@@ -63,7 +51,7 @@ class FedoraConfigurator(configurator.ApacheConfigurator):
             raise errors.MisconfigurationError(str(err))
 
         # Finish with actual config check to see if systemctl restart helped
-        super(FedoraConfigurator, self).config_test()
+        super().config_test()
 
     def _prepare_options(self):
         """
@@ -71,10 +59,12 @@ class FedoraConfigurator(configurator.ApacheConfigurator):
         instead of httpd and so take advantages of this new bash script in newer versions
         of Fedora to restart httpd.
         """
-        super(FedoraConfigurator, self)._prepare_options()
-        cast(List[str], self.options["restart_cmd"])[0] = 'apachectl'
-        cast(List[str], self.options["restart_cmd_alt"])[0] = 'apachectl'
-        cast(List[str], self.options["conftest_cmd"])[0] = 'apachectl'
+        super()._prepare_options()
+        self.options.restart_cmd[0] = 'apachectl'
+        if not self.options.restart_cmd_alt:  # pragma: no cover
+            raise ValueError("OS option restart_cmd_alt must be set for Fedora.")
+        self.options.restart_cmd_alt[0] = 'apachectl'
+        self.options.conftest_cmd[0] = 'apachectl'
 
 
 class FedoraParser(parser.ApacheParser):
@@ -82,16 +72,16 @@ class FedoraParser(parser.ApacheParser):
     def __init__(self, *args, **kwargs):
         # Fedora 29+ specific configuration file for Apache
         self.sysconfig_filep = "/etc/sysconfig/httpd"
-        super(FedoraParser, self).__init__(*args, **kwargs)
+        super().__init__(*args, **kwargs)
 
     def update_runtime_variables(self):
         """ Override for update_runtime_variables for custom parsing """
         # Opportunistic, works if SELinux not enforced
-        super(FedoraParser, self).update_runtime_variables()
+        super().update_runtime_variables()
         self._parse_sysconfig_var()
 
     def _parse_sysconfig_var(self):
         """ Parses Apache CLI options from Fedora configuration file """
         defines = apache_util.parse_define_file(self.sysconfig_filep, "OPTIONS")
-        for k in defines:
-            self.variables[k] = defines[k]
+        for k, v in defines.items():
+            self.variables[k] = v

certbot-apache/certbot_apache/_internal/override_gentoo.py

@@ -1,36 +1,19 @@
 """ Distribution specific override class for Gentoo Linux """
-from typing import cast
-from typing import List
-
-import zope.interface
-
-from certbot import interfaces
 from certbot_apache._internal import apache_util
 from certbot_apache._internal import configurator
 from certbot_apache._internal import parser
+from certbot_apache._internal.configurator import OsOptions
 
 
-@zope.interface.provider(interfaces.IPluginFactory)
 class GentooConfigurator(configurator.ApacheConfigurator):
     """Gentoo specific ApacheConfigurator override class"""
 
-    OS_DEFAULTS = dict(
+    OS_DEFAULTS = OsOptions(
         server_root="/etc/apache2",
         vhost_root="/etc/apache2/vhosts.d",
         vhost_files="*.conf",
-        logs_root="/var/log/apache2",
-        ctl="apache2ctl",
-        version_cmd=['apache2ctl', '-v'],
-        restart_cmd=['apache2ctl', 'graceful'],
         restart_cmd_alt=['apache2ctl', 'restart'],
-        conftest_cmd=['apache2ctl', 'configtest'],
-        enmod=None,
-        dismod=None,
-        le_vhost_ext="-le-ssl.conf",
-        handle_modules=False,
-        handle_sites=False,
         challenge_location="/etc/apache2/vhosts.d",
-        bin=None,
     )
 
     def _prepare_options(self):
@@ -38,13 +21,15 @@ class GentooConfigurator(configurator.ApacheConfigurator):
         Override the options dictionary initialization in order to support
         alternative restart cmd used in Gentoo.
         """
-        super(GentooConfigurator, self)._prepare_options()
-        cast(List[str], self.options["restart_cmd_alt"])[0] = self.option("ctl")
+        super()._prepare_options()
+        if not self.options.restart_cmd_alt:  # pragma: no cover
+            raise ValueError("OS option restart_cmd_alt must be set for Gentoo.")
+        self.options.restart_cmd_alt[0] = self.options.ctl
 
     def get_parser(self):
         """Initializes the ApacheParser"""
         return GentooParser(
-            self.option("server_root"), self.option("vhost_root"),
+            self.options.server_root, self.options.vhost_root,
             self.version, configurator=self)
 
 
@@ -53,7 +38,7 @@ class GentooParser(parser.ApacheParser):
     def __init__(self, *args, **kwargs):
         # Gentoo specific configuration file for Apache2
         self.apacheconfig_filep = "/etc/conf.d/apache2"
-        super(GentooParser, self).__init__(*args, **kwargs)
+        super().__init__(*args, **kwargs)
 
     def update_runtime_variables(self):
         """ Override for update_runtime_variables for custom parsing """
@@ -64,12 +49,12 @@ class GentooParser(parser.ApacheParser):
         """ Parses Apache CLI options from Gentoo configuration file """
         defines = apache_util.parse_define_file(self.apacheconfig_filep,
                                                 "APACHE2_OPTS")
-        for k in defines:
-            self.variables[k] = defines[k]
+        for k, v in defines.items():
+            self.variables[k] = v
 
     def update_modules(self):
         """Get loaded modules from httpd process, and add them to DOM"""
-        mod_cmd = [self.configurator.option("ctl"), "modules"]
+        mod_cmd = [self.configurator.options.ctl, "modules"]
         matches = apache_util.parse_from_subprocess(mod_cmd, r"(.*)_module")
         for mod in matches:
             self.add_mod(mod.strip())

certbot-apache/certbot_apache/_internal/override_suse.py

@@ -1,28 +1,19 @@
 """ Distribution specific override class for OpenSUSE """
-import zope.interface
-
-from certbot import interfaces
 from certbot_apache._internal import configurator
+from certbot_apache._internal.configurator import OsOptions
 
 
-@zope.interface.provider(interfaces.IPluginFactory)
 class OpenSUSEConfigurator(configurator.ApacheConfigurator):
     """OpenSUSE specific ApacheConfigurator override class"""
 
-    OS_DEFAULTS = dict(
-        server_root="/etc/apache2",
+    OS_DEFAULTS = OsOptions(
         vhost_root="/etc/apache2/vhosts.d",
         vhost_files="*.conf",
-        logs_root="/var/log/apache2",
         ctl="apachectl",
         version_cmd=['apachectl', '-v'],
         restart_cmd=['apachectl', 'graceful'],
         conftest_cmd=['apachectl', 'configtest'],
         enmod="a2enmod",
         dismod="a2dismod",
-        le_vhost_ext="-le-ssl.conf",
-        handle_modules=False,
-        handle_sites=False,
         challenge_location="/etc/apache2/vhosts.d",
-        bin=None,
     )

certbot-apache/certbot_apache/_internal/override_void.py

@@ -0,0 +1,19 @@
+""" Distribution specific override class for Void Linux """
+from certbot_apache._internal import configurator
+from certbot_apache._internal.configurator import OsOptions
+
+
+class VoidConfigurator(configurator.ApacheConfigurator):
+    """Void Linux specific ApacheConfigurator override class"""
+
+    OS_DEFAULTS = OsOptions(
+        server_root="/etc/apache",
+        vhost_root="/etc/apache/extra",
+        vhost_files="*.conf",
+        logs_root="/var/log/httpd",
+        ctl="apachectl",
+        version_cmd=['apachectl', '-v'],
+        restart_cmd=['apachectl', 'graceful'],
+        conftest_cmd=['apachectl', 'configtest'],
+        challenge_location="/etc/apache/extra",
+    )

certbot-apache/certbot_apache/_internal/parser.py

@@ -5,12 +5,18 @@ import logging
 import re
 from typing import Dict
 from typing import List
+from typing import Optional
 
 from certbot import errors
 from certbot.compat import os
 from certbot_apache._internal import apache_util
 from certbot_apache._internal import constants
 
+try:
+    from augeas import Augeas
+except ImportError:  # pragma: no cover
+    Augeas = None  # type: ignore
+
 logger = logging.getLogger(__name__)
 
 
@@ -39,8 +45,7 @@ class ApacheParser:
         self.configurator = configurator
 
         # Initialize augeas
-        self.aug = None
-        self.init_augeas()
+        self.aug = init_augeas()
 
         if not self.check_aug_version():
             raise errors.NotSupportedError(
@@ -48,7 +53,7 @@ class ApacheParser:
                 "version 1.2.0 or higher, please make sure you have you have "
                 "those installed.")
 
-        self.modules: Dict[str, str] = {}
+        self.modules: Dict[str, Optional[str]] = {}
         self.parser_paths: Dict[str, List[str]] = {}
         self.variables: Dict[str, str] = {}
 
@@ -76,30 +81,13 @@ class ApacheParser:
         # Must also attempt to parse additional virtual host root
         if vhostroot:
             self.parse_file(os.path.abspath(vhostroot) + "/" +
-                            self.configurator.option("vhost_files"))
+                            self.configurator.options.vhost_files)
 
         # check to see if there were unparsed define statements
         if version < (2, 4):
             if self.find_dir("Define", exclude=False):
                 raise errors.PluginError("Error parsing runtime variables")
 
-    def init_augeas(self):
-        """ Initialize the actual Augeas instance """
-
-        try:
-            import augeas
-        except ImportError:  # pragma: no cover
-            raise errors.NoInstallationError("Problem in Augeas installation")
-
-        self.aug = augeas.Augeas(
-            # specify a directory to load our preferred lens from
-            loadpath=constants.AUGEAS_LENS_DIR,
-            # Do not save backup (we do it ourselves), do not load
-            # anything by default
-            flags=(augeas.Augeas.NONE |
-                   augeas.Augeas.NO_MODL_AUTOLOAD |
-                   augeas.Augeas.ENABLE_SPAN))
-
     def check_parsing_errors(self, lens):
         """Verify Augeas can parse all of the lens files.
 
@@ -294,7 +282,7 @@ class ApacheParser:
     def update_defines(self):
         """Updates the dictionary of known variables in the configuration"""
 
-        self.variables = apache_util.parse_defines(self.configurator.option("ctl"))
+        self.variables = apache_util.parse_defines(self.configurator.options.ctl)
 
     def update_includes(self):
         """Get includes from httpd process, and add them to DOM if needed"""
@@ -304,7 +292,7 @@ class ApacheParser:
         # configuration files
         _ = self.find_dir("Include")
 
-        matches = apache_util.parse_includes(self.configurator.option("ctl"))
+        matches = apache_util.parse_includes(self.configurator.options.ctl)
         if matches:
             for i in matches:
                 if not self.parsed_in_current(i):
@@ -313,7 +301,7 @@ class ApacheParser:
     def update_modules(self):
         """Get loaded modules from httpd process, and add them to DOM"""
 
-        matches = apache_util.parse_modules(self.configurator.option("ctl"))
+        matches = apache_util.parse_modules(self.configurator.options.ctl)
         for mod in matches:
             self.add_mod(mod.strip())
 
@@ -452,7 +440,11 @@ class ApacheParser:
         :type args: list or str
         """
         first_dir = aug_conf_path + "/directive[1]"
-        self.aug.insert(first_dir, "directive", True)
+        if self.aug.get(first_dir):
+            self.aug.insert(first_dir, "directive", True)
+        else:
+            self.aug.set(first_dir, "directive")
+
         self.aug.set(first_dir, dirname)
         if isinstance(args, list):
             for i, value in enumerate(args, 1):
@@ -949,3 +941,19 @@ def get_aug_path(file_path):
 
     """
     return "/files%s" % file_path
+
+
+def init_augeas() -> Augeas:
+    """ Initialize the actual Augeas instance """
+
+    if not Augeas:  # pragma: no cover
+        raise errors.NoInstallationError("Problem in Augeas installation")
+
+    return Augeas(
+        # specify a directory to load our preferred lens from
+        loadpath=constants.AUGEAS_LENS_DIR,
+        # Do not save backup (we do it ourselves), do not load
+        # anything by default
+        flags=(Augeas.NONE |
+               Augeas.NO_MODL_AUTOLOAD |
+               Augeas.ENABLE_SPAN))

certbot-apache/local-oldest-requirements.txt

@@ -1,3 +0,0 @@
-# Remember to update setup.py to match the package versions below.
-acme[dev]==0.29.0
-certbot[dev]==1.6.0

certbot-apache/setup.py

@@ -1,17 +1,16 @@
 from setuptools import find_packages
 from setuptools import setup
 
-version = '1.14.0.dev0'
+version = '1.20.0'
 
-# Remember to update local-oldest-requirements.txt when changing the minimum
-# acme/certbot version.
 install_requires = [
-    'acme>=0.29.0',
-    'certbot>=1.6.0',
+    # We specify the minimum acme and certbot version as the current plugin
+    # version for simplicity. See
+    # https://github.com/certbot/certbot/issues/8761 for more info.
+    f'acme>={version}',
+    f'certbot>={version}',
     'python-augeas',
     'setuptools>=39.0.1',
-    'zope.component',
-    'zope.interface',
 ]
 
 dev_extras = [
@@ -24,7 +23,7 @@ setup(
     description="Apache plugin for Certbot",
     url='https://github.com/letsencrypt/letsencrypt',
     author="Certbot Project",
-    author_email='client-dev@letsencrypt.org',
+    author_email='certbot-dev@eff.org',
     license='Apache License 2.0',
     python_requires='>=3.6',
     classifiers=[

certbot-apache/tests/augeasnode_test.py

@@ -29,7 +29,7 @@ class AugeasParserNodeTest(util.ApacheTest):  # pylint: disable=too-many-public-
     """Test AugeasParserNode using available test configurations"""
 
     def setUp(self):  # pylint: disable=arguments-differ
-        super(AugeasParserNodeTest, self).setUp()
+        super().setUp()
 
         with mock.patch("certbot_apache._internal.configurator.ApacheConfigurator.get_parsernode_root") as mock_parsernode:
             mock_parsernode.side_effect = _get_augeasnode_mock(

certbot-apache/tests/autohsts_test.py

@@ -18,7 +18,7 @@ class AutoHSTSTest(util.ApacheTest):
     # pylint: disable=protected-access
 
     def setUp(self):  # pylint: disable=arguments-differ
-        super(AutoHSTSTest, self).setUp()
+        super().setUp()
 
         self.config = util.get_apache_configurator(
             self.config_path, self.vhost_path, self.config_dir, self.work_dir)
@@ -146,7 +146,7 @@ class AutoHSTSTest(util.ApacheTest):
     @mock.patch("certbot_apache._internal.display_ops.select_vhost")
     def test_autohsts_no_ssl_vhost(self, mock_select):
         mock_select.return_value = self.vh_truth[0]
-        with mock.patch("certbot_apache._internal.configurator.logger.warning") as mock_log:
+        with mock.patch("certbot_apache._internal.configurator.logger.error") as mock_log:
             self.assertRaises(errors.PluginError,
                               self.config.enable_autohsts,
                               mock.MagicMock(), "invalid.example.com")
@@ -179,7 +179,7 @@ class AutoHSTSTest(util.ApacheTest):
         self.config._autohsts_fetch_state()
         self.config._autohsts["orphan_id"] = {"laststep": 999, "timestamp": 0}
         self.config._autohsts_save_state()
-        with mock.patch("certbot_apache._internal.configurator.logger.warning") as mock_log:
+        with mock.patch("certbot_apache._internal.configurator.logger.error") as mock_log:
             self.config.deploy_autohsts(mock.MagicMock())
             self.assertTrue(mock_log.called)
             self.assertTrue(

certbot-apache/tests/centos6_test.py

@@ -1,5 +1,6 @@
 """Test for certbot_apache._internal.configurator for CentOS 6 overrides"""
 import unittest
+from unittest import mock
 
 from certbot.compat import os
 from certbot.errors import MisconfigurationError
@@ -36,9 +37,9 @@ class CentOS6Tests(util.ApacheTest):
         test_dir = "centos6_apache/apache"
         config_root = "centos6_apache/apache/httpd"
         vhost_root = "centos6_apache/apache/httpd/conf.d"
-        super(CentOS6Tests, self).setUp(test_dir=test_dir,
-                                        config_root=config_root,
-                                        vhost_root=vhost_root)
+        super().setUp(test_dir=test_dir,
+                      config_root=config_root,
+                      vhost_root=vhost_root)
 
         self.config = util.get_apache_configurator(
             self.config_path, self.vhost_path, self.config_dir, self.work_dir,
@@ -65,7 +66,8 @@ class CentOS6Tests(util.ApacheTest):
                 raise Exception("Missed: %s" % vhost)  # pragma: no cover
         self.assertEqual(found, 2)
 
-    def test_loadmod_default(self):
+    @mock.patch("certbot_apache._internal.configurator.display_util.notify")
+    def test_loadmod_default(self, unused_mock_notify):
         ssl_loadmods = self.config.parser.find_dir(
             "LoadModule", "ssl_module", exclude=False)
         self.assertEqual(len(ssl_loadmods), 1)
@@ -95,7 +97,8 @@ class CentOS6Tests(util.ApacheTest):
             ifmod_args = self.config.parser.get_all_args(lm[:-17])
             self.assertTrue("!mod_ssl.c" in ifmod_args)
 
-    def test_loadmod_multiple(self):
+    @mock.patch("certbot_apache._internal.configurator.display_util.notify")
+    def test_loadmod_multiple(self, unused_mock_notify):
         sslmod_args = ["ssl_module", "modules/mod_ssl.so"]
         # Adds another LoadModule to main httpd.conf in addtition to ssl.conf
         self.config.parser.add_dir(self.config.parser.loc["default"], "LoadModule",
@@ -115,7 +118,8 @@ class CentOS6Tests(util.ApacheTest):
         for mod in post_loadmods:
             self.assertTrue(self.config.parser.not_modssl_ifmodule(mod))  #pylint: disable=no-member
 
-    def test_loadmod_rootconf_exists(self):
+    @mock.patch("certbot_apache._internal.configurator.display_util.notify")
+    def test_loadmod_rootconf_exists(self, unused_mock_notify):
         sslmod_args = ["ssl_module", "modules/mod_ssl.so"]
         rootconf_ifmod = self.config.parser.get_ifmod(
             parser.get_aug_path(self.config.parser.loc["default"]),
@@ -142,7 +146,8 @@ class CentOS6Tests(util.ApacheTest):
             self.config.parser.get_all_args(mods[0][:-7]),
             sslmod_args)
 
-    def test_neg_loadmod_already_on_path(self):
+    @mock.patch("certbot_apache._internal.configurator.display_util.notify")
+    def test_neg_loadmod_already_on_path(self, unused_mock_notify):
         loadmod_args = ["ssl_module", "modules/mod_ssl.so"]
         ifmod = self.config.parser.get_ifmod(
             self.vh_truth[1].path, "!mod_ssl.c", beginning=True)
@@ -185,7 +190,8 @@ class CentOS6Tests(util.ApacheTest):
         # Make sure that none was changed
         self.assertEqual(pre_matches, post_matches)
 
-    def test_loadmod_not_found(self):
+    @mock.patch("certbot_apache._internal.configurator.display_util.notify")
+    def test_loadmod_not_found(self, unused_mock_notify):
         # Remove all existing LoadModule ssl_module... directives
         orig_loadmods = self.config.parser.find_dir("LoadModule",
                                                     "ssl_module",

certbot-apache/tests/centos_test.py

@@ -41,9 +41,9 @@ class FedoraRestartTest(util.ApacheTest):
         test_dir = "centos7_apache/apache"
         config_root = "centos7_apache/apache/httpd"
         vhost_root = "centos7_apache/apache/httpd/conf.d"
-        super(FedoraRestartTest, self).setUp(test_dir=test_dir,
-                                             config_root=config_root,
-                                             vhost_root=vhost_root)
+        super().setUp(test_dir=test_dir,
+                      config_root=config_root,
+                      vhost_root=vhost_root)
         self.config = util.get_apache_configurator(
             self.config_path, self.vhost_path, self.config_dir, self.work_dir,
             os_info="fedora_old")
@@ -96,9 +96,9 @@ class MultipleVhostsTestCentOS(util.ApacheTest):
         test_dir = "centos7_apache/apache"
         config_root = "centos7_apache/apache/httpd"
         vhost_root = "centos7_apache/apache/httpd/conf.d"
-        super(MultipleVhostsTestCentOS, self).setUp(test_dir=test_dir,
-                                                    config_root=config_root,
-                                                    vhost_root=vhost_root)
+        super().setUp(test_dir=test_dir,
+                      config_root=config_root,
+                      vhost_root=vhost_root)
 
         self.config = util.get_apache_configurator(
             self.config_path, self.vhost_path, self.config_dir, self.work_dir,

certbot-apache/tests/complex_parsing_test.py

@@ -11,7 +11,7 @@ class ComplexParserTest(util.ParserTest):
     """Apache Parser Test."""
 
     def setUp(self):  # pylint: disable=arguments-differ
-        super(ComplexParserTest, self).setUp(
+        super().setUp(
             "complex_parsing", "complex_parsing")
 
         self.setup_variables()

certbot-apache/tests/configurator_reverter_test.py

@@ -16,7 +16,7 @@ class ConfiguratorReverterTest(util.ApacheTest):
 
 
     def setUp(self):  # pylint: disable=arguments-differ
-        super(ConfiguratorReverterTest, self).setUp()
+        super().setUp()
 
         self.config = util.get_apache_configurator(
             self.config_path, self.vhost_path, self.config_dir, self.work_dir)

certbot-apache/tests/configurator_test.py

@@ -30,7 +30,7 @@ class MultipleVhostsTest(util.ApacheTest):
     """Test two standard well-configured HTTP vhosts."""
 
     def setUp(self):  # pylint: disable=arguments-differ
-        super(MultipleVhostsTest, self).setUp()
+        super().setUp()
 
         self.config = util.get_apache_configurator(
             self.config_path, self.vhost_path, self.config_dir, self.work_dir)
@@ -103,9 +103,9 @@ class MultipleVhostsTest(util.ApacheTest):
                       "handle_modules", "handle_sites", "ctl"]
         exp = {}
 
-        for k in ApacheConfigurator.OS_DEFAULTS:
+        for k in ApacheConfigurator.OS_DEFAULTS.__dict__.keys():
             if k in parserargs:
-                exp[k.replace("_", "-")] = ApacheConfigurator.OS_DEFAULTS[k]
+                exp[k.replace("_", "-")] = getattr(ApacheConfigurator.OS_DEFAULTS, k)
         # Special cases
         exp["vhost-root"] = None
 
@@ -128,16 +128,15 @@ class MultipleVhostsTest(util.ApacheTest):
     def test_all_configurators_defaults_defined(self):
         from certbot_apache._internal.entrypoint import OVERRIDE_CLASSES
         from certbot_apache._internal.configurator import ApacheConfigurator
-        parameters = set(ApacheConfigurator.OS_DEFAULTS.keys())
+        parameters = set(ApacheConfigurator.OS_DEFAULTS.__dict__.keys())
         for cls in OVERRIDE_CLASSES.values():
-            self.assertTrue(parameters.issubset(set(cls.OS_DEFAULTS.keys())))
+            self.assertTrue(parameters.issubset(set(cls.OS_DEFAULTS.__dict__.keys())))
 
     def test_constant(self):
         self.assertTrue("debian_apache_2_4/multiple_vhosts/apache" in
-                        self.config.option("server_root"))
-        self.assertEqual(self.config.option("nonexistent"), None)
+                        self.config.options.server_root)
 
-    @certbot_util.patch_get_utility()
+    @certbot_util.patch_display_util()
     def test_get_all_names(self, mock_getutility):
         mock_utility = mock_getutility()
         mock_utility.notification = mock.MagicMock(return_value=True)
@@ -146,7 +145,7 @@ class MultipleVhostsTest(util.ApacheTest):
              "nonsym.link", "vhost.in.rootconf", "www.certbot.demo",
              "duplicate.example.com"})
 
-    @certbot_util.patch_get_utility()
+    @certbot_util.patch_display_util()
     @mock.patch("certbot_apache._internal.configurator.socket.gethostbyaddr")
     def test_get_all_names_addrs(self, mock_gethost, mock_getutility):
         mock_gethost.side_effect = [("google.com", "", ""), socket.error]
@@ -338,7 +337,8 @@ class MultipleVhostsTest(util.ApacheTest):
         vhosts = self.config._non_default_vhosts(self.config.vhosts)
         self.assertEqual(len(vhosts), 10)
 
-    def test_deploy_cert_enable_new_vhost(self):
+    @mock.patch('certbot_apache._internal.configurator.display_util.notify')
+    def test_deploy_cert_enable_new_vhost(self, unused_mock_notify):
         # Create
         ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[0])
         self.config.parser.modules["ssl_module"] = None
@@ -376,7 +376,8 @@ class MultipleVhostsTest(util.ApacheTest):
                 self.fail("Include shouldn't be added, as patched find_dir 'finds' existing one") \
                     # pragma: no cover
 
-    def test_deploy_cert(self):
+    @mock.patch('certbot_apache._internal.configurator.display_util.notify')
+    def test_deploy_cert(self, unused_mock_notify):
         self.config.parser.modules["ssl_module"] = None
         self.config.parser.modules["mod_ssl.c"] = None
         self.config.parser.modules["socache_shmcb_module"] = None
@@ -892,7 +893,7 @@ class MultipleVhostsTest(util.ApacheTest):
             self.config.enhance, "certbot.demo", "unknown_enhancement")
 
     def test_enhance_no_ssl_vhost(self):
-        with mock.patch("certbot_apache._internal.configurator.logger.warning") as mock_log:
+        with mock.patch("certbot_apache._internal.configurator.logger.error") as mock_log:
             self.assertRaises(errors.PluginError, self.config.enhance,
                               "certbot.demo", "redirect")
             # Check that correct logger.warning was printed
@@ -1291,7 +1292,8 @@ class MultipleVhostsTest(util.ApacheTest):
             os.path.basename(inc_path) in self.config.parser.existing_paths[
                 os.path.dirname(inc_path)])
 
-    def test_deploy_cert_not_parsed_path(self):
+    @mock.patch('certbot_apache._internal.configurator.display_util.notify')
+    def test_deploy_cert_not_parsed_path(self, unused_mock_notify):
         # Make sure that we add include to root config for vhosts when
         # handle-sites is false
         self.config.parser.modules["ssl_module"] = None
@@ -1387,7 +1389,8 @@ class MultipleVhostsTest(util.ApacheTest):
             self.assertEqual(vhs[0], self.vh_truth[7])
 
 
-    def test_deploy_cert_wildcard(self):
+    @mock.patch('certbot_apache._internal.configurator.display_util.notify')
+    def test_deploy_cert_wildcard(self, unused_mock_notify):
         # pylint: disable=protected-access
         mock_choose_vhosts = mock.MagicMock()
         mock_choose_vhosts.return_value = [self.vh_truth[7]]
@@ -1477,9 +1480,9 @@ class AugeasVhostsTest(util.ApacheTest):
         td = "debian_apache_2_4/augeas_vhosts"
         cr = "debian_apache_2_4/augeas_vhosts/apache2"
         vr = "debian_apache_2_4/augeas_vhosts/apache2/sites-available"
-        super(AugeasVhostsTest, self).setUp(test_dir=td,
-                                            config_root=cr,
-                                            vhost_root=vr)
+        super().setUp(test_dir=td,
+                      config_root=cr,
+                      vhost_root=vr)
 
         self.config = util.get_apache_configurator(
             self.config_path, self.vhost_path, self.config_dir,
@@ -1556,9 +1559,9 @@ class MultiVhostsTest(util.ApacheTest):
         td = "debian_apache_2_4/multi_vhosts"
         cr = "debian_apache_2_4/multi_vhosts/apache2"
         vr = "debian_apache_2_4/multi_vhosts/apache2/sites-available"
-        super(MultiVhostsTest, self).setUp(test_dir=td,
-                                            config_root=cr,
-                                            vhost_root=vr)
+        super().setUp(test_dir=td,
+                      config_root=cr,
+                      vhost_root=vr)
 
         self.config = util.get_apache_configurator(
             self.config_path, self.vhost_path,
@@ -1607,8 +1610,8 @@ class MultiVhostsTest(util.ApacheTest):
         self.assertEqual(self.config._get_new_vh_path(without_index, both),
                          with_index_2[0])
 
-    @certbot_util.patch_get_utility()
-    def test_make_vhost_ssl_with_existing_rewrite_rule(self, mock_get_utility):
+    @mock.patch("certbot_apache._internal.configurator.display_util.notify")
+    def test_make_vhost_ssl_with_existing_rewrite_rule(self, mock_notify):
         self.config.parser.modules["rewrite_module"] = None
 
         ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[4])
@@ -1624,11 +1627,11 @@ class MultiVhostsTest(util.ApacheTest):
                                     "\"http://new.example.com/docs/$1\"  [R,L]")
         self.assertTrue(commented_rewrite_rule in conf_text)
         self.assertTrue(uncommented_rewrite_rule in conf_text)
-        mock_get_utility().add_message.assert_called_once_with(mock.ANY,
-                                                               mock.ANY)
+        self.assertEqual(mock_notify.call_count, 1)
+        self.assertIn("Some rewrite rules", mock_notify.call_args[0][0])
 
-    @certbot_util.patch_get_utility()
-    def test_make_vhost_ssl_with_existing_rewrite_conds(self, mock_get_utility):
+    @mock.patch("certbot_apache._internal.configurator.display_util.notify")
+    def test_make_vhost_ssl_with_existing_rewrite_conds(self, mock_notify):
         self.config.parser.modules["rewrite_module"] = None
 
         ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[3])
@@ -1653,15 +1656,15 @@ class MultiVhostsTest(util.ApacheTest):
         self.assertTrue(commented_cond1 in conf_line_set)
         self.assertTrue(commented_cond2 in conf_line_set)
         self.assertTrue(commented_rewrite_rule in conf_line_set)
-        mock_get_utility().add_message.assert_called_once_with(mock.ANY,
-                                                               mock.ANY)
+        self.assertEqual(mock_notify.call_count, 1)
+        self.assertIn("Some rewrite rules", mock_notify.call_args[0][0])
 
 
 class InstallSslOptionsConfTest(util.ApacheTest):
     """Test that the options-ssl-nginx.conf file is installed and updated properly."""
 
     def setUp(self): # pylint: disable=arguments-differ
-        super(InstallSslOptionsConfTest, self).setUp()
+        super().setUp()
 
         self.config = util.get_apache_configurator(
             self.config_path, self.vhost_path, self.config_dir, self.work_dir)
@@ -1774,7 +1777,7 @@ class InstallSslOptionsConfTest(util.ApacheTest):
         # ssl_module statically linked
         self.config._openssl_version = None
         self.config.parser.modules['ssl_module'] = None
-        self.config.options['bin'] = '/fake/path/to/httpd'
+        self.config.options.bin = '/fake/path/to/httpd'
         with mock.patch("certbot_apache._internal.configurator."
             "ApacheConfigurator._open_module_file") as mock_omf:
             mock_omf.return_value = some_string_contents
@@ -1810,7 +1813,7 @@ class InstallSslOptionsConfTest(util.ApacheTest):
 
         # When ssl_module is statically linked but --apache-bin not provided
         self.config._openssl_version = None
-        self.config.options['bin'] = None
+        self.config.options.bin = None
         self.config.parser.modules['ssl_module'] = None
         with mock.patch("certbot_apache._internal.configurator.logger.warning") as mock_log:
             self.assertEqual(self.config.openssl_version(), None)

certbot-apache/tests/debian_test.py

@@ -9,6 +9,7 @@ except ImportError: # pragma: no cover
 
 from certbot import errors
 from certbot.compat import os
+from certbot.tests import util as certbot_util
 from certbot_apache._internal import apache_util
 from certbot_apache._internal import obj
 import util
@@ -20,7 +21,7 @@ class MultipleVhostsTestDebian(util.ApacheTest):
     _multiprocess_can_split_ = True
 
     def setUp(self):  # pylint: disable=arguments-differ
-        super(MultipleVhostsTestDebian, self).setUp()
+        super().setUp()
         self.config = util.get_apache_configurator(
             self.config_path, self.vhost_path, self.config_dir, self.work_dir,
             os_info="debian")
@@ -49,10 +50,11 @@ class MultipleVhostsTestDebian(util.ApacheTest):
 
     @mock.patch("certbot.util.run_script")
     @mock.patch("certbot.util.exe_exists")
-    @mock.patch("certbot_apache._internal.apache_util.subprocess.Popen")
-    def test_enable_mod(self, mock_popen, mock_exe_exists, mock_run_script):
-        mock_popen().communicate.return_value = ("Define: DUMP_RUN_CFG", "")
-        mock_popen().returncode = 0
+    @mock.patch("certbot_apache._internal.apache_util.subprocess.run")
+    def test_enable_mod(self, mock_run, mock_exe_exists, mock_run_script):
+        mock_run.return_value.stdout = "Define: DUMP_RUN_CFG"
+        mock_run.return_value.stderr = ""
+        mock_run.return_value.returncode = 0
         mock_exe_exists.return_value = True
 
         self.config.enable_mod("ssl")
@@ -67,17 +69,18 @@ class MultipleVhostsTestDebian(util.ApacheTest):
         self.config.parser.modules["ssl_module"] = None
         self.config.parser.modules["mod_ssl.c"] = None
         self.assertFalse(ssl_vhost.enabled)
-        self.config.deploy_cert(
-            "encryption-example.demo", "example/cert.pem", "example/key.pem",
-            "example/cert_chain.pem", "example/fullchain.pem")
-        self.assertTrue(ssl_vhost.enabled)
-        # Make sure that we don't error out if symlink already exists
-        ssl_vhost.enabled = False
-        self.assertFalse(ssl_vhost.enabled)
-        self.config.deploy_cert(
-            "encryption-example.demo", "example/cert.pem", "example/key.pem",
-            "example/cert_chain.pem", "example/fullchain.pem")
-        self.assertTrue(ssl_vhost.enabled)
+        with certbot_util.patch_display_util():
+            self.config.deploy_cert(
+                "encryption-example.demo", "example/cert.pem", "example/key.pem",
+                "example/cert_chain.pem", "example/fullchain.pem")
+            self.assertTrue(ssl_vhost.enabled)
+            # Make sure that we don't error out if symlink already exists
+            ssl_vhost.enabled = False
+            self.assertFalse(ssl_vhost.enabled)
+            self.config.deploy_cert(
+                "encryption-example.demo", "example/cert.pem", "example/key.pem",
+                "example/cert_chain.pem", "example/fullchain.pem")
+            self.assertTrue(ssl_vhost.enabled)
 
     def test_enable_site_failure(self):
         self.config.parser.root = "/tmp/nonexistent"
@@ -100,9 +103,10 @@ class MultipleVhostsTestDebian(util.ApacheTest):
 
         # Get the default 443 vhost
         self.config.assoc["random.demo"] = self.vh_truth[1]
-        self.config.deploy_cert(
-            "random.demo", "example/cert.pem", "example/key.pem",
-            "example/cert_chain.pem", "example/fullchain.pem")
+        with certbot_util.patch_display_util():
+            self.config.deploy_cert(
+                "random.demo", "example/cert.pem", "example/key.pem",
+                "example/cert_chain.pem", "example/fullchain.pem")
         self.config.save()
 
         # Verify ssl_module was enabled.

certbot-apache/tests/display_ops_test.py

@@ -3,8 +3,8 @@ import unittest
 
 try:
     import mock
-except ImportError: # pragma: no cover
-    from unittest import mock # type: ignore
+except ImportError:  # pragma: no cover
+    from unittest import mock  # type: ignore
 
 from certbot import errors
 from certbot.display import util as display_util
@@ -25,7 +25,7 @@ class SelectVhostMultiTest(unittest.TestCase):
     def test_select_no_input(self):
         self.assertFalse(select_vhost_multiple([]))
 
-    @certbot_util.patch_get_utility()
+    @certbot_util.patch_display_util()
     def test_select_correct(self, mock_util):
         mock_util().checklist.return_value = (
             display_util.OK, [self.vhosts[3].display_repr(),
@@ -37,12 +37,13 @@ class SelectVhostMultiTest(unittest.TestCase):
         self.assertTrue(self.vhosts[3] in vhs)
         self.assertFalse(self.vhosts[1] in vhs)
 
-    @certbot_util.patch_get_utility()
+    @certbot_util.patch_display_util()
     def test_select_cancel(self, mock_util):
         mock_util().checklist.return_value = (display_util.CANCEL, "whatever")
         vhs = select_vhost_multiple([self.vhosts[2], self.vhosts[3]])
         self.assertFalse(vhs)
 
+
 class SelectVhostTest(unittest.TestCase):
     """Tests for certbot_apache._internal.display_ops.select_vhost."""
 
@@ -56,12 +57,12 @@ class SelectVhostTest(unittest.TestCase):
         from certbot_apache._internal.display_ops import select_vhost
         return select_vhost("example.com", vhosts)
 
-    @certbot_util.patch_get_utility()
+    @certbot_util.patch_display_util()
     def test_successful_choice(self, mock_util):
         mock_util().menu.return_value = (display_util.OK, 3)
         self.assertEqual(self.vhosts[3], self._call(self.vhosts))
 
-    @certbot_util.patch_get_utility()
+    @certbot_util.patch_display_util()
     def test_noninteractive(self, mock_util):
         mock_util().menu.side_effect = errors.MissingCommandlineFlag("no vhost default")
         try:
@@ -69,7 +70,7 @@ class SelectVhostTest(unittest.TestCase):
         except errors.MissingCommandlineFlag as e:
             self.assertTrue("vhost ambiguity" in str(e))
 
-    @certbot_util.patch_get_utility()
+    @certbot_util.patch_display_util()
     def test_more_info_cancel(self, mock_util):
         mock_util().menu.side_effect = [
             (display_util.CANCEL, -1),
@@ -81,16 +82,15 @@ class SelectVhostTest(unittest.TestCase):
         self.assertEqual(self._call([]), None)
 
     @mock.patch("certbot_apache._internal.display_ops.display_util")
-    @certbot_util.patch_get_utility()
     @mock.patch("certbot_apache._internal.display_ops.logger")
-    def test_small_display(self, mock_logger, mock_util, mock_display_util):
+    def test_small_display(self, mock_logger, mock_display_util):
         mock_display_util.WIDTH = 20
-        mock_util().menu.return_value = (display_util.OK, 0)
+        mock_display_util.menu.return_value = (display_util.OK, 0)
         self._call(self.vhosts)
 
         self.assertEqual(mock_logger.debug.call_count, 1)
 
-    @certbot_util.patch_get_utility()
+    @certbot_util.patch_display_util()
     def test_multiple_names(self, mock_util):
         mock_util().menu.return_value = (display_util.OK, 5)
 

certbot-apache/tests/entrypoint_test.py

@@ -41,7 +41,7 @@ class EntryPointTest(unittest.TestCase):
         with mock.patch("certbot.util.get_os_info") as mock_info:
             mock_info.return_value = ("nonexistent", "irrelevant")
             with mock.patch("certbot.util.get_systemd_os_like") as mock_like:
-                mock_like.return_value = ["unknonwn"]
+                mock_like.return_value = ["unknown"]
                 self.assertEqual(entrypoint.get_configurator(),
                                  configurator.ApacheConfigurator)
 

certbot-apache/tests/fedora_test.py

@@ -46,9 +46,9 @@ class FedoraRestartTest(util.ApacheTest):
         test_dir = "centos7_apache/apache"
         config_root = "centos7_apache/apache/httpd"
         vhost_root = "centos7_apache/apache/httpd/conf.d"
-        super(FedoraRestartTest, self).setUp(test_dir=test_dir,
-                                             config_root=config_root,
-                                             vhost_root=vhost_root)
+        super().setUp(test_dir=test_dir,
+                      config_root=config_root,
+                      vhost_root=vhost_root)
         self.config = util.get_apache_configurator(
             self.config_path, self.vhost_path, self.config_dir, self.work_dir,
             os_info="fedora")
@@ -90,9 +90,9 @@ class MultipleVhostsTestFedora(util.ApacheTest):
         test_dir = "centos7_apache/apache"
         config_root = "centos7_apache/apache/httpd"
         vhost_root = "centos7_apache/apache/httpd/conf.d"
-        super(MultipleVhostsTestFedora, self).setUp(test_dir=test_dir,
-                                                    config_root=config_root,
-                                                    vhost_root=vhost_root)
+        super().setUp(test_dir=test_dir,
+                      config_root=config_root,
+                      vhost_root=vhost_root)
 
         self.config = util.get_apache_configurator(
             self.config_path, self.vhost_path, self.config_dir, self.work_dir,

certbot-apache/tests/gentoo_test.py

@@ -50,9 +50,9 @@ class MultipleVhostsTestGentoo(util.ApacheTest):
         test_dir = "gentoo_apache/apache"
         config_root = "gentoo_apache/apache/apache2"
         vhost_root = "gentoo_apache/apache/apache2/vhosts.d"
-        super(MultipleVhostsTestGentoo, self).setUp(test_dir=test_dir,
-                                                    config_root=config_root,
-                                                    vhost_root=vhost_root)
+        super().setUp(test_dir=test_dir,
+                      config_root=config_root,
+                      vhost_root=vhost_root)
 
         # pylint: disable=line-too-long
         with mock.patch("certbot_apache._internal.override_gentoo.GentooParser.update_runtime_variables"):

certbot-apache/tests/http_01_test.py

@@ -24,7 +24,7 @@ class ApacheHttp01Test(util.ApacheTest):
     """Test for certbot_apache._internal.http_01.ApacheHttp01."""
 
     def setUp(self, *args, **kwargs):  # pylint: disable=arguments-differ
-        super(ApacheHttp01Test, self).setUp(*args, **kwargs)
+        super().setUp(*args, **kwargs)
 
         self.account_key = self.rsa512jwk
         self.achalls: List[achallenges.KeyAuthorizationAnnotatedChallenge] = []
@@ -125,6 +125,18 @@ class ApacheHttp01Test(util.ApacheTest):
                 domain="duplicate.example.com", account_key=self.account_key)]
         self.common_perform_test(achalls, vhosts)
 
+    def test_configure_name_and_blank(self):
+        domain = "certbot.demo"
+        vhosts = [v for v in self.config.vhosts if v.name == domain or v.name is None]
+        achalls = [
+            achallenges.KeyAuthorizationAnnotatedChallenge(
+                challb=acme_util.chall_to_challb(
+                    challenges.HTTP01(token=((b'a' * 16))),
+                    "pending"),
+                domain=domain, account_key=self.account_key),
+        ]
+        self.common_perform_test(achalls, vhosts)
+
     def test_no_vhost(self):
         for achall in self.achalls:
             self.http.add_chall(achall)

certbot-apache/tests/parser_test.py

@@ -16,7 +16,7 @@ class BasicParserTest(util.ParserTest):
     """Apache Parser Test."""
 
     def setUp(self):  # pylint: disable=arguments-differ
-        super(BasicParserTest, self).setUp()
+        super().setUp()
 
     def tearDown(self):
         shutil.rmtree(self.temp_dir)
@@ -105,6 +105,11 @@ class BasicParserTest(util.ParserTest):
         for i, match in enumerate(matches):
             self.assertEqual(self.parser.aug.get(match), str(i + 1))
 
+        for name in ("empty.conf", "no-directives.conf"):
+            conf = "/files" + os.path.join(self.parser.root, "sites-available", name)
+            self.parser.add_dir_beginning(conf, "AddDirectiveBeginning", "testBegin")
+            self.assertTrue(self.parser.find_dir("AddDirectiveBeginning", "testBegin", conf))
+
     def test_empty_arg(self):
         self.assertEqual(None,
                           self.parser.get_arg("/files/whatever/nonexistent"))
@@ -183,6 +188,8 @@ class BasicParserTest(util.ParserTest):
             'Define: DUMP_RUN_CFG\n'
             'Define: U_MICH\n'
             'Define: TLS=443\n'
+            'Define: WITH_ASSIGNMENT=URL=http://example.com\n'
+            'Define: EMPTY=\n'
             'Define: example_path=Documents/path\n'
             'User: name="www-data" id=33 not_used\n'
             'Group: name="www-data" id=33 not_used\n'
@@ -261,7 +268,10 @@ class BasicParserTest(util.ParserTest):
         mock_cfg.side_effect = mock_get_vars
 
         expected_vars = {"TEST": "", "U_MICH": "", "TLS": "443",
-                         "example_path": "Documents/path"}
+                         "example_path": "Documents/path",
+                         "WITH_ASSIGNMENT": "URL=http://example.com",
+                         "EMPTY": "",
+                         }
 
         self.parser.modules = {}
         with mock.patch(
@@ -296,28 +306,19 @@ class BasicParserTest(util.ParserTest):
             # path derived from root configuration Include statements
             self.assertEqual(mock_parse.call_count, 1)
 
-    @mock.patch("certbot_apache._internal.apache_util._get_runtime_cfg")
-    def test_update_runtime_vars_bad_output(self, mock_cfg):
-        mock_cfg.return_value = "Define: TLS=443=24"
-        self.parser.update_runtime_variables()
-
-        mock_cfg.return_value = "Define: DUMP_RUN_CFG\nDefine: TLS=443=24"
-        self.assertRaises(
-            errors.PluginError, self.parser.update_runtime_variables)
-
-    @mock.patch("certbot_apache._internal.configurator.ApacheConfigurator.option")
-    @mock.patch("certbot_apache._internal.apache_util.subprocess.Popen")
-    def test_update_runtime_vars_bad_ctl(self, mock_popen, mock_opt):
-        mock_popen.side_effect = OSError
-        mock_opt.return_value = "nonexistent"
+    @mock.patch("certbot_apache._internal.apache_util.subprocess.run")
+    def test_update_runtime_vars_bad_ctl(self, mock_run):
+        mock_run.side_effect = OSError
         self.assertRaises(
             errors.MisconfigurationError,
             self.parser.update_runtime_variables)
 
-    @mock.patch("certbot_apache._internal.apache_util.subprocess.Popen")
-    def test_update_runtime_vars_bad_exit(self, mock_popen):
-        mock_popen().communicate.return_value = ("", "")
-        mock_popen.returncode = -1
+    @mock.patch("certbot_apache._internal.apache_util.subprocess.run")
+    def test_update_runtime_vars_bad_exit(self, mock_run):
+        mock_proc = mock_run.return_value
+        mock_proc.stdout = ""
+        mock_proc.stderr = ""
+        mock_proc.returncode = -1
         self.assertRaises(
             errors.MisconfigurationError,
             self.parser.update_runtime_variables)
@@ -332,14 +333,14 @@ class BasicParserTest(util.ParserTest):
 
 class ParserInitTest(util.ApacheTest):
     def setUp(self):  # pylint: disable=arguments-differ
-        super(ParserInitTest, self).setUp()
+        super().setUp()
 
     def tearDown(self):
         shutil.rmtree(self.temp_dir)
         shutil.rmtree(self.config_dir)
         shutil.rmtree(self.work_dir)
 
-    @mock.patch("certbot_apache._internal.parser.ApacheParser.init_augeas")
+    @mock.patch("certbot_apache._internal.parser.init_augeas")
     def test_prepare_no_augeas(self, mock_init_augeas):
         from certbot_apache._internal.parser import ApacheParser
         mock_init_augeas.side_effect = errors.NoInstallationError

certbot-apache/tests/parsernode_configurator_test.py

@@ -20,7 +20,7 @@ class ConfiguratorParserNodeTest(util.ApacheTest):  # pylint: disable=too-many-p
     """Test AugeasParserNode using available test configurations"""
 
     def setUp(self):  # pylint: disable=arguments-differ
-        super(ConfiguratorParserNodeTest, self).setUp()
+        super().setUp()
 
         self.config = util.get_apache_configurator(
             self.config_path, self.vhost_path, self.config_dir,

certbot-apache/tests/parsernode_test.py

@@ -18,7 +18,7 @@ class DummyParserNode(interfaces.ParserNode):
         self.dirty = dirty
         self.filepath = filepath
         self.metadata = metadata
-        super(DummyParserNode, self).__init__(**kwargs)
+        super().__init__(**kwargs)
 
     def save(self, msg):  # pragma: no cover
         """Save"""
@@ -38,7 +38,7 @@ class DummyCommentNode(DummyParserNode):
         """
         comment, kwargs = util.commentnode_kwargs(kwargs)
         self.comment = comment
-        super(DummyCommentNode, self).__init__(**kwargs)
+        super().__init__(**kwargs)
 
 
 class DummyDirectiveNode(DummyParserNode):
@@ -54,7 +54,7 @@ class DummyDirectiveNode(DummyParserNode):
         self.parameters = parameters
         self.enabled = enabled
 
-        super(DummyDirectiveNode, self).__init__(**kwargs)
+        super().__init__(**kwargs)
 
     def set_parameters(self, parameters):  # pragma: no cover
         """Set parameters"""

certbot-apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-available/no-directives.conf

@@ -0,0 +1,5 @@
+<VirtualHost *:80>
+  <Location />
+        Require all denied
+  </Location>
+</VirtualHost>

certbot-apache/tests/util.py

@@ -5,16 +5,16 @@ import unittest
 
 import augeas
 import josepy as jose
+
 try:
     import mock
-except ImportError: # pragma: no cover
-    from unittest import mock # type: ignore
-import zope.component
+except ImportError:  # pragma: no cover
+    from unittest import mock  # type: ignore
 
 from certbot.compat import os
-from certbot.display import util as display_util
 from certbot.plugins import common
 from certbot.tests import util as test_util
+from certbot.display import util as display_util
 from certbot_apache._internal import configurator
 from certbot_apache._internal import entrypoint
 from certbot_apache._internal import obj
@@ -67,10 +67,7 @@ class ParserTest(ApacheTest):
     def setUp(self, test_dir="debian_apache_2_4/multiple_vhosts",
               config_root="debian_apache_2_4/multiple_vhosts/apache2",
               vhost_root="debian_apache_2_4/multiple_vhosts/apache2/sites-available"):
-        super(ParserTest, self).setUp(test_dir, config_root, vhost_root)
-
-        zope.component.provideUtility(display_util.FileDisplay(sys.stdout,
-                                                               False))
+        super().setUp(test_dir, config_root, vhost_root)
 
         from certbot_apache._internal.parser import ApacheParser
         self.aug = augeas.Augeas(
@@ -123,11 +120,11 @@ def get_apache_configurator(
                                           version=version, use_parsernode=use_parsernode,
                                           openssl_version=openssl_version)
                     if not conf_vhost_path:
-                        config_class.OS_DEFAULTS["vhost_root"] = vhost_path
+                        config_class.OS_DEFAULTS.vhost_root = vhost_path
                     else:
                         # Custom virtualhost path was requested
                         config.config.apache_vhost_root = conf_vhost_path
-                    config.config.apache_ctl = config_class.OS_DEFAULTS["ctl"]
+                    config.config.apache_ctl = config_class.OS_DEFAULTS.ctl
                     config.prepare()
     return config
 

certbot-ci/certbot_integration_tests/certbot_tests/context.py

@@ -61,7 +61,7 @@ class IntegrationTestsContext:
         Execute certbot with given args, not renewing certificates by default.
         :param args: args to pass to certbot
         :param force_renew: set to False to not renew by default
-        :return: output of certbot execution
+        :return: stdout and stderr from certbot execution
         """
         command = ['--authenticator', 'standalone', '--installer', 'null']
         command.extend(args)

certbot-ci/certbot_integration_tests/certbot_tests/test_main.py

@@ -78,9 +78,9 @@ def test_registration_override(context):
 
 def test_prepare_plugins(context):
     """Test that plugins are correctly instantiated and displayed."""
-    output = context.certbot(['plugins', '--init', '--prepare'])
+    stdout, _ = context.certbot(['plugins', '--init', '--prepare'])
 
-    assert 'webroot' in output
+    assert 'webroot' in stdout
 
 
 def test_http_01(context):
@@ -346,7 +346,8 @@ def test_renew_empty_hook_scripts(context):
     for hook_dir in misc.list_renewal_hooks_dirs(context.config_dir):
         shutil.rmtree(hook_dir)
         os.makedirs(join(hook_dir, 'dir'))
-        open(join(hook_dir, 'file'), 'w').close()
+        with open(join(hook_dir, 'file'), 'w'):
+            pass
     context.certbot(['renew'])
 
     assert_cert_count_for_lineage(context.config_dir, certname, 2)
@@ -368,7 +369,8 @@ def test_renew_hook_override(context):
     assert_hook_execution(context.hook_probe, 'deploy')
 
     # Now we override all previous hooks during next renew.
-    open(context.hook_probe, 'w').close()
+    with open(context.hook_probe, 'w'):
+        pass
     context.certbot([
         'renew', '--cert-name', certname,
         '--pre-hook', misc.echo('pre_override', context.hook_probe),
@@ -387,7 +389,8 @@ def test_renew_hook_override(context):
         assert_hook_execution(context.hook_probe, 'deploy')
 
     # Expect that this renew will reuse new hooks registered in the previous renew.
-    open(context.hook_probe, 'w').close()
+    with open(context.hook_probe, 'w'):
+        pass
     context.certbot(['renew', '--cert-name', certname])
 
     assert_hook_execution(context.hook_probe, 'pre_override')
@@ -407,9 +410,9 @@ def test_invalid_domain_with_dns_challenge(context):
         '--manual-cleanup-hook', context.manual_dns_cleanup_hook
     ])
 
-    output = context.certbot(['certificates'])
+    stdout, _ = context.certbot(['certificates'])
 
-    assert context.get_domain('fail-dns1') not in output
+    assert context.get_domain('fail-dns1') not in stdout
 
 
 def test_reuse_key(context):
@@ -430,6 +433,21 @@ def test_reuse_key(context):
         privkey3 = file.read()
     assert privkey2 != privkey3
 
+    context.certbot(['--cert-name', certname, '--domains', certname,
+                     '--reuse-key','--force-renewal'])
+    context.certbot(['renew', '--cert-name', certname, '--no-reuse-key', '--force-renewal'])
+    context.certbot(['renew', '--cert-name', certname, '--force-renewal'])
+
+    with open(join(context.config_dir, 'archive/{0}/privkey4.pem').format(certname), 'r') as file:
+        privkey4 = file.read()
+    with open(join(context.config_dir, 'archive/{0}/privkey5.pem').format(certname), 'r') as file:
+        privkey5 = file.read()
+    with open(join(context.config_dir, 'archive/{0}/privkey6.pem').format(certname), 'r') as file:
+        privkey6 = file.read()
+    assert privkey3 == privkey4
+    assert privkey4 != privkey5
+    assert privkey5 != privkey6
+
     with open(join(context.config_dir, 'archive/{0}/cert1.pem').format(certname), 'r') as file:
         cert1 = file.read()
     with open(join(context.config_dir, 'archive/{0}/cert2.pem').format(certname), 'r') as file:
@@ -614,11 +632,11 @@ def test_revoke_and_unregister(context):
 
     context.certbot(['unregister'])
 
-    output = context.certbot(['certificates'])
+    stdout, _ = context.certbot(['certificates'])
 
-    assert cert1 not in output
-    assert cert2 not in output
-    assert cert3 in output
+    assert cert1 not in stdout
+    assert cert2 not in stdout
+    assert cert3 in stdout
 
 
 def test_revoke_mutual_exclusive_flags(context):
@@ -630,7 +648,7 @@ def test_revoke_mutual_exclusive_flags(context):
             'revoke', '--cert-name', cert,
             '--cert-path', join(context.config_dir, 'live', cert, 'fullchain.pem')
         ])
-        assert 'Exactly one of --cert-path or --cert-name must be specified' in error.out
+    assert 'Exactly one of --cert-path or --cert-name must be specified' in error.value.stderr
 
 
 def test_revoke_multiple_lineages(context):
@@ -685,12 +703,12 @@ def test_wildcard_certificates(context):
 def test_ocsp_status_stale(context):
     """Test retrieval of OCSP statuses for staled config"""
     sample_data_path = misc.load_sample_data_path(context.workspace)
-    output = context.certbot(['certificates', '--config-dir', sample_data_path])
+    stdout, _ = context.certbot(['certificates', '--config-dir', sample_data_path])
 
-    assert output.count('TEST_CERT') == 2, ('Did not find two test certs as expected ({0})'
-                                            .format(output.count('TEST_CERT')))
-    assert output.count('EXPIRED') == 2, ('Did not find two expired certs as expected ({0})'
-                                          .format(output.count('EXPIRED')))
+    assert stdout.count('TEST_CERT') == 2, ('Did not find two test certs as expected ({0})'
+                                            .format(stdout.count('TEST_CERT')))
+    assert stdout.count('EXPIRED') == 2, ('Did not find two expired certs as expected ({0})'
+                                          .format(stdout.count('EXPIRED')))
 
 
 def test_ocsp_status_live(context):
@@ -699,20 +717,20 @@ def test_ocsp_status_live(context):
 
     # OSCP 1: Check live certificate OCSP status (VALID)
     context.certbot(['--domains', cert])
-    output = context.certbot(['certificates'])
+    stdout, _ = context.certbot(['certificates'])
 
-    assert output.count('VALID') == 1, 'Expected {0} to be VALID'.format(cert)
-    assert output.count('EXPIRED') == 0, 'Did not expect {0} to be EXPIRED'.format(cert)
+    assert stdout.count('VALID') == 1, 'Expected {0} to be VALID'.format(cert)
+    assert stdout.count('EXPIRED') == 0, 'Did not expect {0} to be EXPIRED'.format(cert)
 
     # OSCP 2: Check live certificate OCSP status (REVOKED)
     context.certbot(['revoke', '--cert-name', cert, '--no-delete-after-revoke'])
     # Sometimes in oldest tests (using openssl binary and not cryptography), the OCSP status is
     # not seen immediately by Certbot as invalid. Waiting few seconds solves this transient issue.
     time.sleep(5)
-    output = context.certbot(['certificates'])
+    stdout, _ = context.certbot(['certificates'])
 
-    assert output.count('INVALID') == 1, 'Expected {0} to be INVALID'.format(cert)
-    assert output.count('REVOKED') == 1, 'Expected {0} to be REVOKED'.format(cert)
+    assert stdout.count('INVALID') == 1, 'Expected {0} to be INVALID'.format(cert)
+    assert stdout.count('REVOKED') == 1, 'Expected {0} to be REVOKED'.format(cert)
 
 
 def test_ocsp_renew(context):

certbot-ci/certbot_integration_tests/nginx_tests/context.py

@@ -11,7 +11,7 @@ from certbot_integration_tests.utils import misc
 class IntegrationTestsContext(certbot_context.IntegrationTestsContext):
     """General fixture describing a certbot-nginx integration tests context"""
     def __init__(self, request):
-        super(IntegrationTestsContext, self).__init__(request)
+        super().__init__(request)
 
         self.nginx_root = os.path.join(self.workspace, 'nginx')
         os.mkdir(self.nginx_root)
@@ -29,7 +29,7 @@ class IntegrationTestsContext(certbot_context.IntegrationTestsContext):
 
     def cleanup(self):
         self._stop_nginx()
-        super(IntegrationTestsContext, self).cleanup()
+        super().cleanup()
 
     def certbot_test_nginx(self, args):
         """
@@ -51,6 +51,7 @@ class IntegrationTestsContext(certbot_context.IntegrationTestsContext):
         with open(self.nginx_config_path, 'w') as file:
             file.write(self.nginx_config)
 
+        # pylint: disable=consider-using-with
         process = subprocess.Popen(['nginx', '-c', self.nginx_config_path, '-g', 'daemon off;'])
 
         assert process.poll() is None

certbot-ci/certbot_integration_tests/rfc2136_tests/context.py

@@ -13,11 +13,10 @@ from certbot_integration_tests.utils import certbot_call
 class IntegrationTestsContext(certbot_context.IntegrationTestsContext):
     """Integration test context for certbot-dns-rfc2136"""
     def __init__(self, request):
-        super(IntegrationTestsContext, self).__init__(request)
+        super().__init__(request)
 
         self.request = request
 
-        self._dns_xdist = None
         if hasattr(request.config, 'workerinput'):  # Worker node
             self._dns_xdist = request.config.workerinput['dns_xdist']
         else:  # Primary node
@@ -45,7 +44,6 @@ class IntegrationTestsContext(certbot_context.IntegrationTestsContext):
         src_file = resource_filename('certbot_integration_tests',
                                      'assets/bind-config/rfc2136-credentials-{}.ini.tpl'
                                      .format(label))
-        contents = None
 
         with open(src_file, 'r') as f:
             contents = f.read().format(

certbot-ci/certbot_integration_tests/utils/acme_server.py

@@ -52,7 +52,7 @@ class ACMEServer:
         self._proxy = http_proxy
         self._workspace = tempfile.mkdtemp()
         self._processes: List[subprocess.Popen] = []
-        self._stdout = sys.stdout if stdout else open(os.devnull, 'w')
+        self._stdout = sys.stdout if stdout else open(os.devnull, 'w') # pylint: disable=consider-using-with
         self._dns_server = dns_server
         self._http_01_port = http_01_port
         if http_01_port != DEFAULT_HTTP_01_PORT:
@@ -240,6 +240,7 @@ class ACMEServer:
         if not env:
             env = os.environ
         stdout = sys.stderr if force_stderr else self._stdout
+        # pylint: disable=consider-using-with
         process = subprocess.Popen(
             command, stdout=stdout, stderr=subprocess.STDOUT, cwd=cwd, env=env
         )

certbot-ci/certbot_integration_tests/utils/certbot_call.py

@@ -17,7 +17,7 @@ def certbot_test(certbot_args, directory_url, http_01_port, tls_alpn_01_port,
     Invoke the certbot executable available in PATH in a test context for the given args.
     The test context consists in running certbot in debug mode, with various flags suitable
     for tests (eg. no ssl check, customizable ACME challenge ports and config directory ...).
-    This command captures stdout and returns it to the caller.
+    This command captures both stdout and stderr and returns it to the caller.
     :param list certbot_args: the arguments to pass to the certbot executable
     :param str directory_url: URL of the ACME directory server to use
     :param int http_01_port: port for the HTTP-01 challenges
@@ -25,13 +25,19 @@ def certbot_test(certbot_args, directory_url, http_01_port, tls_alpn_01_port,
     :param str config_dir: certbot configuration directory to use
     :param str workspace: certbot current directory to use
     :param bool force_renew: set False to not force renew existing certificates (default: True)
-    :return: stdout as string
-    :rtype: str
+    :return: stdout and stderr as strings
+    :rtype: `tuple` of `str`
     """
     command, env = _prepare_args_env(certbot_args, directory_url, http_01_port, tls_alpn_01_port,
                                      config_dir, workspace, force_renew)
 
-    return subprocess.check_output(command, universal_newlines=True, cwd=workspace, env=env)
+    proc = subprocess.run(command, stdout=subprocess.PIPE,
+                          stderr=subprocess.PIPE, check=False, universal_newlines=True,
+                          cwd=workspace, env=env)
+    print('--> Certbot log output was:')
+    print(proc.stderr)
+    proc.check_returncode()
+    return proc.stdout, proc.stderr
 
 
 def _prepare_environ(workspace):

certbot-ci/certbot_integration_tests/utils/dns_server.py

@@ -8,6 +8,7 @@ import subprocess
 import sys
 import tempfile
 import time
+from typing import Optional
 
 from pkg_resources import resource_filename
 
@@ -38,12 +39,13 @@ class DNSServer:
 
         self.bind_root = tempfile.mkdtemp()
 
-        self.process: subprocess.Popen = None
+        self.process: Optional[subprocess.Popen] = None
 
         self.dns_xdist = {"address": BIND_BIND_ADDRESS[0], "port": BIND_BIND_ADDRESS[1]}
 
         # Unfortunately the BIND9 image forces everything to stderr with -g and we can't
         # modify the verbosity.
+        # pylint: disable=consider-using-with
         self._output = sys.stderr if show_output else open(os.devnull, "w")
 
     def start(self):
@@ -82,6 +84,7 @@ class DNSServer:
     def _start_bind(self):
         """Launch the BIND9 server as a Docker container"""
         addr_str = "{}:{}".format(BIND_BIND_ADDRESS[0], BIND_BIND_ADDRESS[1])
+        # pylint: disable=consider-using-with
         self.process = subprocess.Popen(
             [
                 "docker",
@@ -119,6 +122,9 @@ class DNSServer:
         but otherwise the contents are ignored.
         :param int attempts: The number of attempts to make.
         """
+        if not self.process:
+            raise ValueError("DNS server has not been started. Please run start() first.")
+
         for _ in range(attempts):
             if self.process.poll():
                 raise ValueError("BIND9 server stopped unexpectedly")

certbot-ci/certbot_integration_tests/utils/misc.py

@@ -232,10 +232,15 @@ def generate_csr(domains, key_path, csr_path, key_type=RSA_KEY_TYPE):
         with warnings.catch_warnings():
             # Ignore a warning on some old versions of cryptography
             warnings.simplefilter('ignore', category=PendingDeprecationWarning)
-            key = ec.generate_private_key(ec.SECP384R1(), default_backend())
-        key = key.private_bytes(encoding=Encoding.PEM, format=PrivateFormat.TraditionalOpenSSL,
-                                encryption_algorithm=NoEncryption())
-        key = crypto.load_privatekey(crypto.FILETYPE_PEM, key)
+            _key = ec.generate_private_key(ec.SECP384R1(), default_backend())
+        # This type ignore directive is required due to an outdated version of types-cryptography.
+        # It can be removed once package types-pyOpenSSL depends on cryptography instead of
+        # types-cryptography and so types-cryptography is not installed anymore.
+        # See https://github.com/python/typeshed/issues/5618
+        _bytes = _key.private_bytes(encoding=Encoding.PEM,  # type: ignore
+                                    format=PrivateFormat.TraditionalOpenSSL,
+                                    encryption_algorithm=NoEncryption())
+        key = crypto.load_privatekey(crypto.FILETYPE_PEM, _bytes)
     else:
         raise ValueError('Invalid key type: {0}'.format(key_type))
 

certbot-ci/setup.py

@@ -1,5 +1,4 @@
 from distutils.version import LooseVersion
-import sys
 
 from setuptools import __version__ as setuptools_version
 from setuptools import find_packages
@@ -30,6 +29,7 @@ install_requires = [
     'pywin32>=300 ; sys_platform == "win32"',
     'pyyaml',
     'requests',
+    'types-python-dateutil'
 ]
 
 setup(
@@ -38,7 +38,7 @@ setup(
     description="Certbot continuous integration framework",
     url='https://github.com/certbot/certbot',
     author="Certbot Project",
-    author_email='client-dev@letsencrypt.org',
+    author_email='certbot-dev@eff.org',
     license='Apache License 2.0',
     python_requires='>=3.6',
     classifiers=[

certbot-ci/snap_integration_tests/dns_tests/test_main.py

@@ -44,4 +44,4 @@ def test_dns_plugin_install(dns_snap_path):
             'certbot:certbot-metadata'])
         subprocess.check_call(['snap', 'install', '--dangerous', dns_snap_path])
     finally:
-        subprocess.call(['snap', 'remove', 'plugin_name'])
+        subprocess.call(['snap', 'remove', plugin_name])

certbot-compatibility-test/Dockerfile

@@ -15,7 +15,7 @@ RUN tools/venv.py
 ENV PATH /opt/certbot/src/venv/bin:$PATH
 
 # install in editable mode (-e) to save space: it's not possible to
-# "rm -rf /opt/certbot/src" (it's stays in the underlaying image);
+# "rm -rf /opt/certbot/src" (it's stays in the underlying image);
 # this might also help in debugging: you can "docker run --entrypoint
 # bash" and investigate, apply patches, etc.
 

certbot-compatibility-test/certbot_compatibility_test/configurators/apache/common.py

@@ -4,25 +4,20 @@ import shutil
 import subprocess
 from unittest import mock
 
-import zope.interface
-
-from certbot import errors as le_errors
+from certbot import errors as le_errors, configuration
 from certbot import util as certbot_util
-from certbot._internal import configuration
 from certbot_apache._internal import entrypoint
 from certbot_compatibility_test import errors
-from certbot_compatibility_test import interfaces
 from certbot_compatibility_test import util
 from certbot_compatibility_test.configurators import common as configurators_common
 
 
-@zope.interface.implementer(interfaces.IConfiguratorProxy)
 class Proxy(configurators_common.Proxy):
     """A common base for Apache test configurators"""
 
     def __init__(self, args):
         """Initializes the plugin with the given command line args"""
-        super(Proxy, self).__init__(args)
+        super().__init__(args)
         self.le_config.apache_le_vhost_ext = "-le-ssl.conf"
 
         self.modules = self.server_root = self.test_conf = self.version = None
@@ -34,7 +29,7 @@ class Proxy(configurators_common.Proxy):
 
     def load_config(self):
         """Loads the next configuration for the plugin to test"""
-        config = super(Proxy, self).load_config()
+        config = super().load_config()
         self._all_names, self._test_names = _get_names(config)
 
         server_root = _get_server_root(config)
@@ -54,9 +49,9 @@ class Proxy(configurators_common.Proxy):
 
     def _prepare_configurator(self):
         """Prepares the Apache plugin for testing"""
-        for k in entrypoint.ENTRYPOINT.OS_DEFAULTS:
+        for k in entrypoint.ENTRYPOINT.OS_DEFAULTS.__dict__.keys():
             setattr(self.le_config, "apache_" + k,
-                    entrypoint.ENTRYPOINT.OS_DEFAULTS[k])
+                    getattr(entrypoint.ENTRYPOINT.OS_DEFAULTS, k))
 
         self._configurator = entrypoint.ENTRYPOINT(
             config=configuration.NamespaceConfig(self.le_config),
@@ -65,7 +60,7 @@ class Proxy(configurators_common.Proxy):
 
     def cleanup_from_tests(self):
         """Performs any necessary cleanup from running plugin tests"""
-        super(Proxy, self).cleanup_from_tests()
+        super().cleanup_from_tests()
         mock.patch.stopall()
 
 

certbot-compatibility-test/certbot_compatibility_test/configurators/common.py

@@ -1,17 +1,19 @@
 """Provides a common base for configurator proxies"""
+from abc import abstractmethod
 import logging
 import os
 import shutil
 import tempfile
 
 from certbot._internal import constants
+from certbot_compatibility_test import interfaces
 from certbot_compatibility_test import errors
 from certbot_compatibility_test import util
 
 logger = logging.getLogger(__name__)
 
 
-class Proxy:
+class Proxy(interfaces.ConfiguratorProxy):
     """A common base for compatibility test configurators"""
 
     @classmethod
@@ -20,6 +22,7 @@ class Proxy:
 
     def __init__(self, args):
         """Initializes the plugin with the given command line args"""
+        super().__init__(args)
         self._temp_dir = tempfile.mkdtemp()
         # tempfile.mkdtemp() creates folders with too restrictive permissions to be accessible
         # to an Apache worker, leading to HTTP challenge failures. Let's fix that.
@@ -33,22 +36,15 @@ class Proxy:
         self.args = args
         self.http_port = 80
         self.https_port = 443
-        self._configurator = self._all_names = self._test_names = None
-
-    def __getattr__(self, name):
-        """Wraps the configurator methods"""
-        if self._configurator is None:
-            raise AttributeError()
-
-        method = getattr(self._configurator, name, None)
-        if callable(method):
-            return method
-        raise AttributeError()
+        self._configurator: interfaces.Configurator
+        self._all_names = None
+        self._test_names = None
 
     def has_more_configs(self):
         """Returns true if there are more configs to test"""
         return bool(self._configs)
 
+    @abstractmethod
     def cleanup_from_tests(self):
         """Performs any necessary cleanup from running plugin tests"""
 
@@ -93,5 +89,51 @@ class Proxy:
         """Installs cert"""
         cert_path, key_path, chain_path = self.copy_certs_and_keys(
             cert_path, key_path, chain_path)
+        if not self._configurator:
+            raise ValueError("Configurator plugin is not set.")
         self._configurator.deploy_cert(
             domain, cert_path, key_path, chain_path, fullchain_path)
+
+
+    def cleanup(self, achalls):
+        self._configurator.cleanup(achalls)
+
+    def config_test(self):
+        self._configurator.config_test()
+
+    def enhance(self, domain, enhancement, options = None):
+        self._configurator.enhance(domain, enhancement, options)
+
+    def get_all_names(self):
+        return self._configurator.get_all_names()
+
+    def get_chall_pref(self, domain):
+        return self._configurator.get_chall_pref(domain)
+
+    @classmethod
+    def inject_parser_options(cls, parser, name):
+        pass
+
+    def more_info(self):
+        return self._configurator.more_info()
+
+    def perform(self, achalls):
+        return self._configurator.perform(achalls)
+
+    def prepare(self):
+        self._configurator.prepare()
+
+    def recovery_routine(self):
+        self._configurator.recovery_routine()
+
+    def restart(self):
+        self._configurator.restart()
+
+    def rollback_checkpoints(self, rollback = 1):
+        self._configurator.rollback_checkpoints(rollback)
+
+    def save(self, title = None, temporary = False):
+        self._configurator.save(title, temporary)
+
+    def supported_enhancements(self):
+        return self._configurator.supported_enhancements()

certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py

@@ -2,11 +2,10 @@
 import os
 import shutil
 import subprocess
+from typing import cast
 from typing import Set
 
-import zope.interface
-
-from certbot._internal import configuration
+from certbot import configuration
 from certbot_compatibility_test import errors
 from certbot_compatibility_test import interfaces
 from certbot_compatibility_test import util
@@ -15,13 +14,12 @@ from certbot_nginx._internal import configurator
 from certbot_nginx._internal import constants
 
 
-@zope.interface.implementer(interfaces.IConfiguratorProxy)
 class Proxy(configurators_common.Proxy):
     """A common base for Nginx test configurators"""
 
     def load_config(self):
         """Loads the next configuration for the plugin to test"""
-        config = super(Proxy, self).load_config()
+        config = super().load_config()
         self._all_names, self._test_names = _get_names(config)
 
         server_root = _get_server_root(config)
@@ -48,11 +46,13 @@ class Proxy(configurators_common.Proxy):
             setattr(self.le_config, "nginx_" + k, constants.os_constant(k))
 
         conf = configuration.NamespaceConfig(self.le_config)
-        zope.component.provideUtility(conf)
-        self._configurator = configurator.NginxConfigurator(
-            config=conf, name="nginx")
+        self._configurator = cast(interfaces.Configurator, configurator.NginxConfigurator(
+            config=conf, name="nginx"))
         self._configurator.prepare()
 
+    def cleanup_from_tests(self):
+        """Performs any necessary cleanup from running plugin tests"""
+
 
 def _get_server_root(config):
     """Returns the server root directory in config"""
@@ -80,11 +80,12 @@ def _get_names(config):
 def _get_server_names(root, filename):
     """Returns all names in a config file path"""
     all_names = set()
-    for line in open(os.path.join(root, filename)):
-        if line.strip().startswith("server_name"):
-            names = line.partition("server_name")[2].rpartition(";")[0]
-            for n in names.split():
-                # Filter out wildcards in both all_names and test_names
-                if not n.startswith("*."):
-                    all_names.add(n)
+    with open(os.path.join(root, filename)) as f:
+        for line in f:
+            if line.strip().startswith("server_name"):
+                names = line.partition("server_name")[2].rpartition(";")[0]
+                for n in names.split():
+                    # Filter out wildcards in both all_names and test_names
+                    if not n.startswith("*."):
+                        all_names.add(n)
     return all_names

certbot-compatibility-test/certbot_compatibility_test/interfaces.py

@@ -1,53 +1,65 @@
 """Certbot compatibility test interfaces"""
-import zope.interface
+from abc import ABCMeta
+from abc import abstractmethod
 
-import certbot.interfaces
-
-# pylint: disable=no-self-argument,no-method-argument
+from certbot import interfaces
 
 
-class IPluginProxy(zope.interface.Interface):  # pylint: disable=inherit-non-class
+class PluginProxy(interfaces.Plugin, metaclass=ABCMeta):
     """Wraps a Certbot plugin"""
 
-    http_port = zope.interface.Attribute(
-        "The port to connect to on localhost for HTTP traffic")
+    http_port: int = NotImplemented
+    """The port to connect to on localhost for HTTP traffic"""
 
-    https_port = zope.interface.Attribute(
-        "The port to connect to on localhost for HTTPS traffic")
+    https_port: int = NotImplemented
+    """The port to connect to on localhost for HTTPS traffic"""
 
+    @classmethod
+    @abstractmethod
     def add_parser_arguments(cls, parser):
         """Adds command line arguments needed by the parser"""
 
-    def __init__(args):  # pylint: disable=super-init-not-called
+    @abstractmethod
+    def __init__(self, args):
         """Initializes the plugin with the given command line args"""
+        super().__init__(args, 'proxy')
 
-    def cleanup_from_tests():  # type: ignore
+    @abstractmethod
+    def cleanup_from_tests(self):
         """Performs any necessary cleanup from running plugin tests.
 
         This is guaranteed to be called before the program exits.
 
         """
 
-    def has_more_configs():  # type: ignore
+    @abstractmethod
+    def has_more_configs(self):
         """Returns True if there are more configs to test"""
 
-    def load_config():  # type: ignore
+    @abstractmethod
+    def load_config(self):
         """Loads the next config and returns its name"""
 
-    def get_testable_domain_names():  # type: ignore
+    @abstractmethod
+    def get_testable_domain_names(self):
         """Returns the domain names that can be used in testing"""
 
 
-class IAuthenticatorProxy(IPluginProxy, certbot.interfaces.IAuthenticator):
+class AuthenticatorProxy(PluginProxy, interfaces.Authenticator, metaclass=ABCMeta):
     """Wraps a Certbot authenticator"""
 
 
-class IInstallerProxy(IPluginProxy, certbot.interfaces.IInstaller):
+class InstallerProxy(PluginProxy, interfaces.Installer, metaclass=ABCMeta):
     """Wraps a Certbot installer"""
 
-    def get_all_names_answer():  # type: ignore
+    @abstractmethod
+    def get_all_names_answer(self):
         """Returns all names that should be found by the installer"""
 
 
-class IConfiguratorProxy(IAuthenticatorProxy, IInstallerProxy):
+class ConfiguratorProxy(AuthenticatorProxy, InstallerProxy, metaclass=ABCMeta):
     """Wraps a Certbot configurator"""
+
+
+class Configurator(interfaces.Installer, interfaces.Authenticator, metaclass=ABCMeta):
+    """Represents a plugin that has both Installer and Authenticator capabilities"""

certbot-compatibility-test/certbot_compatibility_test/test_driver.py

@@ -19,6 +19,8 @@ from acme import crypto_util
 from acme import messages
 from certbot import achallenges
 from certbot import errors as le_errors
+from certbot.display import util as display_util
+from certbot._internal.display import obj as display_obj
 from certbot.tests import acme_util
 from certbot_compatibility_test import errors
 from certbot_compatibility_test import util
@@ -327,10 +329,17 @@ def setup_logging(args):
     root_logger.addHandler(handler)
 
 
+def setup_display():
+    """"Prepares a display utility instance for the Certbot plugins """
+    displayer = display_util.NoninteractiveDisplay(sys.stdout)
+    display_obj.set_display(displayer)
+
+
 def main():
     """Main test script execution."""
     args = get_args()
     setup_logging(args)
+    setup_display()
 
     if args.plugin not in PLUGINS:
         raise errors.Error("Unknown plugin {0}".format(args.plugin))

certbot-compatibility-test/setup.py

@@ -1,22 +1,14 @@
-import sys
-
 from setuptools import find_packages
 from setuptools import setup
 
-version = '1.14.0.dev0'
+version = '1.20.0'
 
 install_requires = [
     'certbot',
     'certbot-apache',
     'requests',
-    'zope.interface',
 ]
 
-if sys.version_info < (2, 7, 9):
-    # For secure SSL connexion with Python 2.7 (InsecurePlatformWarning)
-    install_requires.append('ndg-httpsclient')
-    install_requires.append('pyasn1')
-
 
 setup(
     name='certbot-compatibility-test',
@@ -24,7 +16,7 @@ setup(
     description="Compatibility tests for Certbot",
     url='https://github.com/letsencrypt/letsencrypt',
     author="Certbot Project",
-    author_email='client-dev@letsencrypt.org',
+    author_email='certbot-dev@eff.org',
     license='Apache License 2.0',
     python_requires='>=3.6',
     classifiers=[

certbot-dns-cloudflare/certbot_dns_cloudflare/_internal/dns_cloudflare.py

@@ -3,21 +3,19 @@ import logging
 from typing import Any
 from typing import Dict
 from typing import List
+from typing import Optional
 
 import CloudFlare
-import zope.interface
 
 from certbot import errors
-from certbot import interfaces
 from certbot.plugins import dns_common
+from certbot.plugins.dns_common import CredentialsConfiguration
 
 logger = logging.getLogger(__name__)
 
 ACCOUNT_URL = 'https://dash.cloudflare.com/?to=/:account/profile/api-tokens'
 
 
-@zope.interface.implementer(interfaces.IAuthenticator)
-@zope.interface.provider(interfaces.IPluginFactory)
 class Authenticator(dns_common.DNSAuthenticator):
     """DNS Authenticator for Cloudflare
 
@@ -29,12 +27,12 @@ class Authenticator(dns_common.DNSAuthenticator):
     ttl = 120
 
     def __init__(self, *args, **kwargs):
-        super(Authenticator, self).__init__(*args, **kwargs)
-        self.credentials = None
+        super().__init__(*args, **kwargs)
+        self.credentials: Optional[CredentialsConfiguration] = None
 
     @classmethod
     def add_parser_arguments(cls, add):  # pylint: disable=arguments-differ
-        super(Authenticator, cls).add_parser_arguments(add)
+        super().add_parser_arguments(add)
         add('credentials', help='Cloudflare credentials INI file.')
 
     def more_info(self):  # pylint: disable=missing-function-docstring
@@ -79,6 +77,8 @@ class Authenticator(dns_common.DNSAuthenticator):
         self._get_cloudflare_client().del_txt_record(domain, validation_name, validation)
 
     def _get_cloudflare_client(self):
+        if not self.credentials:  # pragma: no cover
+            raise errors.Error("Plugin has not been prepared.")
         if self.credentials.conf('api-token'):
             return _CloudflareClient(None, self.credentials.conf('api-token'))
         return _CloudflareClient(self.credentials.conf('email'), self.credentials.conf('api-key'))