set response.encoding in acme

This reverts commit 1129d850d3.

.azure-pipelines/main.yml

@@ -5,3 +5,4 @@ pr:
 
 jobs:
   - template: templates/jobs/standard-tests-jobs.yml
+

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

@@ -21,26 +21,24 @@ jobs:
           PYTHON_VERSION: 3.7
           TOXENV: py37
           CERTBOT_NO_PIN: 1
+        linux-external-mock:
+          TOXENV: external-mock
         linux-boulder-v1-integration-certbot-oldest:
+          PYTHON_VERSION: 3.6
           TOXENV: integration-certbot-oldest
           ACME_SERVER: boulder-v1
         linux-boulder-v2-integration-certbot-oldest:
+          PYTHON_VERSION: 3.6
           TOXENV: integration-certbot-oldest
           ACME_SERVER: boulder-v2
         linux-boulder-v1-integration-nginx-oldest:
+          PYTHON_VERSION: 3.6
           TOXENV: integration-nginx-oldest
           ACME_SERVER: boulder-v1
         linux-boulder-v2-integration-nginx-oldest:
+          PYTHON_VERSION: 3.6
           TOXENV: integration-nginx-oldest
           ACME_SERVER: boulder-v2
-        linux-boulder-v1-py27-integration:
-          PYTHON_VERSION: 2.7
-          TOXENV: integration
-          ACME_SERVER: boulder-v1
-        linux-boulder-v2-py27-integration:
-          PYTHON_VERSION: 2.7
-          TOXENV: integration
-          ACME_SERVER: boulder-v2
         linux-boulder-v1-py36-integration:
           PYTHON_VERSION: 3.6
           TOXENV: integration
@@ -81,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

@@ -12,6 +12,9 @@ jobs:
             DOCKER_ARCH: arm32v6
           arm64v8:
             DOCKER_ARCH: arm64v8
+    # The default timeout of 60 minutes is a little low for compiling
+    # cryptography on ARM architectures.
+    timeoutInMinutes: 180
     steps:
       - bash: set -e && tools/docker/build.sh $(dockerTag) $DOCKER_ARCH
         displayName: Build the Docker images
@@ -56,10 +59,16 @@ jobs:
     steps:
       - task: UsePythonVersion@0
         inputs:
-          versionSpec: 3.7
+          versionSpec: 3.8
           architecture: x86
           addToPath: true
-      - script: python windows-installer/construct.py
+      - script: |
+          python -m venv venv
+          venv\Scripts\python tools\pipstrap.py
+          venv\Scripts\python tools\pip_install.py -e windows-installer
+        displayName: Prepare Windows installer build environment
+      - script: |
+          venv\Scripts\construct-windows-installer
         displayName: Build Certbot installer
       - task: CopyFiles@2
         inputs:
@@ -116,13 +125,17 @@ jobs:
   - job: snaps_build
     pool:
       vmImage: ubuntu-18.04
+    strategy:
+      matrix:
+        amd64:
+          SNAP_ARCH: amd64
+        # Do not run the heavy non-amd64 builds for test branches
+        ${{ if not(startsWith(variables['Build.SourceBranchName'], 'test-')) }}:
+          armhf:
+            SNAP_ARCH: armhf
+          arm64:
+            SNAP_ARCH: arm64
     timeoutInMinutes: 0
-    variables:
-      # Do not run the heavy non-amd64 builds for test branches
-      ${{ if not(startsWith(variables['Build.SourceBranchName'], 'test-')) }}:
-        ARCHS: amd64 arm64 armhf
-      ${{ if startsWith(variables['Build.SourceBranchName'], 'test-') }}:
-        ARCHS: amd64
     steps:
       - script: |
           set -e
@@ -144,7 +157,7 @@ jobs:
           git config --global user.name "$(Build.RequestedFor)"
           mkdir -p ~/.local/share/snapcraft/provider/launchpad
           cp $(credentials.secureFilePath) ~/.local/share/snapcraft/provider/launchpad/credentials
-          python3 tools/snap/build_remote.py ALL --archs ${ARCHS} --timeout 19800
+          python3 tools/snap/build_remote.py ALL --archs ${SNAP_ARCH} --timeout 19800
         displayName: Build snaps
       - script: |
           set -e
@@ -154,7 +167,7 @@ jobs:
       - task: PublishPipelineArtifact@1
         inputs:
           path: $(Build.ArtifactStagingDirectory)
-          artifact: snaps
+          artifact: snaps_$(SNAP_ARCH)
         displayName: Store snaps artifacts
   - job: snap_run
     dependsOn: snaps_build
@@ -175,12 +188,12 @@ jobs:
         displayName: Install dependencies
       - task: DownloadPipelineArtifact@2
         inputs:
-          artifact: snaps
+          artifact: snaps_amd64
           path: $(Build.SourcesDirectory)/snap
         displayName: Retrieve Certbot snaps
       - script: |
           set -e
-          sudo snap install --dangerous --classic snap/certbot_*_amd64.snap
+          sudo snap install --dangerous --classic snap/certbot_*.snap
         displayName: Install Certbot snap
       - script: |
           set -e
@@ -202,7 +215,7 @@ jobs:
           addToPath: true
       - task: DownloadPipelineArtifact@2
         inputs:
-          artifact: snaps
+          artifact: snaps_amd64
           path: $(Build.SourcesDirectory)/snap
         displayName: Retrieve Certbot snaps
       - script: |

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

@@ -4,10 +4,10 @@ jobs:
       PYTHON_VERSION: 3.9
     strategy:
       matrix:
-        macos-py27:
+        macos-py36:
           IMAGE_NAME: macOS-10.15
-          PYTHON_VERSION: 2.7
-          TOXENV: py27
+          PYTHON_VERSION: 3.6
+          TOXENV: py36
         macos-py39:
           IMAGE_NAME: macOS-10.15
           PYTHON_VERSION: 3.9
@@ -15,25 +15,23 @@ jobs:
         windows-py36:
           IMAGE_NAME: vs2017-win2016
           PYTHON_VERSION: 3.6
-          TOXENV: py36
-        windows-py37-cover:
+          TOXENV: py36-win
+        windows-py38-cover:
           IMAGE_NAME: vs2017-win2016
-          PYTHON_VERSION: 3.7
-          TOXENV: py37-cover
+          PYTHON_VERSION: 3.8
+          TOXENV: py38-cover-win
         windows-integration-certbot:
           IMAGE_NAME: vs2017-win2016
-          PYTHON_VERSION: 3.7
+          PYTHON_VERSION: 3.8
           TOXENV: integration-certbot
         linux-oldest-tests-1:
           IMAGE_NAME: ubuntu-18.04
-          TOXENV: py27-{acme,apache,apache-v2,certbot}-oldest
+          PYTHON_VERSION: 3.6
+          TOXENV: '{acme,apache,apache-v2,certbot}-oldest'
         linux-oldest-tests-2:
           IMAGE_NAME: ubuntu-18.04
-          TOXENV: py27-{dns,nginx}-oldest
-        linux-py27:
-          IMAGE_NAME: ubuntu-18.04
-          PYTHON_VERSION: 2.7
-          TOXENV: py27
+          PYTHON_VERSION: 3.6
+          TOXENV: '{dns,nginx}-oldest'
         linux-py36:
           IMAGE_NAME: ubuntu-18.04
           PYTHON_VERSION: 3.6
@@ -42,14 +40,14 @@ jobs:
           IMAGE_NAME: ubuntu-18.04
           PYTHON_VERSION: 3.9
           TOXENV: py39-cover
-        linux-py37-lint:
+        linux-py39-lint:
           IMAGE_NAME: ubuntu-18.04
-          PYTHON_VERSION: 3.7
-          TOXENV: lint
-        linux-py36-mypy:
+          PYTHON_VERSION: 3.9
+          TOXENV: lint-posix
+        linux-py39-mypy:
           IMAGE_NAME: ubuntu-18.04
-          PYTHON_VERSION: 3.6
-          TOXENV: mypy
+          PYTHON_VERSION: 3.9
+          TOXENV: mypy-posix
         linux-integration:
           IMAGE_NAME: ubuntu-18.04
           PYTHON_VERSION: 3.8
@@ -58,18 +56,20 @@ 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: 2.7
+          PYTHON_VERSION: 3.6
           TOXENV: apacheconftest-with-pebble
         nginxroundtrip:
           IMAGE_NAME: ubuntu-18.04
-          PYTHON_VERSION: 2.7
+          PYTHON_VERSION: 3.6
           TOXENV: nginxroundtrip
     pool:
       vmImage: $(IMAGE_NAME)
     steps:
       - template: ../steps/tox-steps.yml
+  - job: test_sphinx_builds
+    pool:
+      vmImage: ubuntu-20.04
+    steps:
+      - template: ../steps/sphinx-steps.yml

.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.
@@ -37,6 +38,14 @@ stages:
           vmImage: ubuntu-18.04
         variables:
           - group: certbot-common
+        strategy:
+          matrix:
+            amd64:
+              SNAP_ARCH: amd64
+            arm32v6:
+              SNAP_ARCH: armhf
+            arm64v8:
+              SNAP_ARCH: arm64
         steps:
           - bash: |
               set -e
@@ -46,7 +55,7 @@ stages:
             displayName: Install dependencies
           - task: DownloadPipelineArtifact@2
             inputs:
-              artifact: snaps
+              artifact: snaps_$(SNAP_ARCH)
               path: $(Build.SourcesDirectory)/snap
             displayName: Retrieve Certbot snaps
           - task: DownloadSecureFile@1
@@ -55,8 +64,7 @@ stages:
               secureFile: snapcraft.cfg
           - bash: |
               set -e
-              mkdir -p .snapcraft
-              ln -s $(snapcraftCfg.secureFilePath) .snapcraft/snapcraft.cfg
+              snapcraft login --with $(snapcraftCfg.secureFilePath)
               for SNAP_FILE in snap/*.snap; do
                 tools/retry.sh eval snapcraft upload --release=${{ parameters.snapReleaseChannel }} "${SNAP_FILE}"
               done

.azure-pipelines/templates/stages/notify-failure-stage.yml

@@ -5,7 +5,7 @@ stages:
         variables:
           - group: certbot-common
         pool:
-          vmImage: ubuntu-latest
+          vmImage: ubuntu-20.04
         steps:
           - bash: |
               set -e

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

@@ -0,0 +1,24 @@
+steps:
+  - bash: |
+      set -e
+      sudo apt-get update
+      sudo apt-get install -y --no-install-recommends libaugeas0
+      FINAL_STATUS=0
+      declare -a FAILED_BUILDS
+      tools/venv.py
+      source venv/bin/activate
+      for doc_path in */docs
+      do
+        echo ""
+        echo "##[group]Building $doc_path"
+        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}"
+        fi
+        echo "##[endgroup]"
+      done
+      if [[ $FINAL_STATUS -ne 0 ]]; then
+        echo "##[error]The following builds failed: ${FAILED_BUILDS[*]}"
+        exit 1
+      fi
+    displayName: Build Sphinx Documentation

.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
@@ -45,11 +49,7 @@ steps:
       export TARGET_BRANCH="`echo "${BUILD_SOURCEBRANCH}" | sed -E 's!refs/(heads|tags)/!!g'`"
       [ -z "${SYSTEM_PULLREQUEST_TARGETBRANCH}" ] || export TARGET_BRANCH="${SYSTEM_PULLREQUEST_TARGETBRANCH}"
       env
-      if [[ "${TOXENV}" == *"oldest"* ]]; then
-        tools/run_oldest_tests.sh
-      else
-        python -m tox
-      fi
+      python -m tox
     env:
       AWS_ACCESS_KEY_ID: $(AWS_ACCESS_KEY_ID)
       AWS_SECRET_ACCESS_KEY: $(AWS_SECRET_ACCESS_KEY)

.dockerignore

@@ -8,5 +8,4 @@
 .git
 .tox
 venv
-venv3
 docs

.envrc

@@ -3,7 +3,7 @@
 # activated and then deactivated when you cd elsewhere. Developers have to have
 # direnv set up and run `direnv allow` to allow this file to execute on their
 # system. You can find more information at https://direnv.net/.
-. venv3/bin/activate
+. venv/bin/activate
 # direnv doesn't support modifying PS1 so we unset it to squelch the error
 # it'll otherwise print about this being done in the activate script. See
 # https://github.com/direnv/direnv/wiki/PS1. If you would like your shell

.github/FUNDING.yml

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

.github/issue_template.md

@@ -7,7 +7,7 @@ questions.
 ## My operating system is (include version):
 
 
-## I installed Certbot with (certbot-auto, OS package manager, pip, etc):
+## I installed Certbot with (snap, OS package manager, pip, certbot-auto, etc):
 
 
 ## I ran this command and it produced this output:

.gitignore

@@ -4,13 +4,12 @@
 build/
 dist*/
 /venv*/
-/kgs/
 /.tox/
 /releases*/
 /log*
 letsencrypt.log
 certbot.log
-letsencrypt-auto-source/letsencrypt-auto.sig.lzma.base64
+poetry.lock
 
 # coverage
 .coverage
@@ -31,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

@@ -8,7 +8,10 @@ jobs=0
 
 # Python code to execute, usually for sys.path manipulation such as
 # pygtk.require().
-#init-hook=
+# CERTBOT COMMENT
+# This is needed for pylint to import linter_plugin.py since
+# https://github.com/PyCQA/pylint/pull/3396.
+init-hook="import pylint.config, os, sys; sys.path.append(os.path.dirname(pylint.config.PYLINTRC))"
 
 # Profiled execution.
 profile=no
@@ -53,7 +56,18 @@ 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.
+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
 
 [REPORTS]
 
@@ -254,7 +268,7 @@ ignore-mixin-members=yes
 # List of module names for which member attributes should not be checked
 # (useful for modules/projects where namespaces are manipulated during runtime
 # and thus existing member attributes cannot be deduced by static analysis
-ignored-modules=pkg_resources,confargparse,argparse,six.moves,six.moves.urllib
+ignored-modules=pkg_resources,confargparse,argparse
 # import errors ignored only in 1.4.4
 # https://bitbucket.org/logilab/pylint/commits/cd000904c9e2
 

AUTHORS.md

@@ -1,6 +1,7 @@
 Authors
 =======
 
+* [Aaron Gable](https://github.com/aarongable)
 * [Aaron Zirbes](https://github.com/aaronzirbes)
 * Aaron Zuehlke
 * Ada Lovelace
@@ -60,6 +61,7 @@ Authors
 * [DanCld](https://github.com/DanCld)
 * [Daniel Albers](https://github.com/AID)
 * [Daniel Aleksandersen](https://github.com/da2x)
+* [Daniel Almasi](https://github.com/almasen)
 * [Daniel Convissor](https://github.com/convissor)
 * [Daniel "Drex" Drexler](https://github.com/aeturnum)
 * [Daniel Huang](https://github.com/dhuang)

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,13 +8,14 @@ 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/* \
            /var/tmp/*
 
-RUN VENV_NAME="../venv3" python3 tools/venv3.py
+RUN VENV_NAME="../venv" python3 tools/venv.py
 
-ENV PATH /opt/certbot/venv3/bin:$PATH
+ENV PATH /opt/certbot/venv/bin:$PATH

acme/acme/__init__.py

@@ -6,7 +6,6 @@ This module is an implementation of the `ACME protocol`_.
 
 """
 import sys
-import warnings
 
 # This code exists to keep backwards compatibility with people using acme.jose
 # before it became the standalone josepy package.
@@ -20,10 +19,3 @@ for mod in list(sys.modules):
     # preserved (acme.jose.* is josepy.*)
     if mod == 'josepy' or mod.startswith('josepy.'):
         sys.modules['acme.' + mod.replace('josepy', 'jose', 1)] = sys.modules[mod]
-
-if sys.version_info[0] == 2:
-    warnings.warn(
-        "Python 2 support will be dropped in the next release of acme. "
-        "Please upgrade your Python version.",
-        PendingDeprecationWarning,
-    )  # pragma: no cover

acme/acme/challenges.py

@@ -5,18 +5,19 @@ import functools
 import hashlib
 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
-import requests
-import six
-from OpenSSL import SSL  # type: ignore # https://github.com/python/typeshed/issues/2052
 from OpenSSL import crypto
+from OpenSSL import SSL
+import requests
 
 from acme import crypto_util
 from acme import errors
 from acme import fields
-from acme.mixins import ResourceMixin, TypeMixin
+from acme.mixins import ResourceMixin
+from acme.mixins import TypeMixin
 
 logger = logging.getLogger(__name__)
 
@@ -24,12 +25,12 @@ logger = logging.getLogger(__name__)
 class Challenge(jose.TypedJSONObjectWithFields):
     # _fields_to_partial_json
     """ACME challenge."""
-    TYPES = {}  # type: dict
+    TYPES: dict = {}
 
     @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)
@@ -38,7 +39,7 @@ class Challenge(jose.TypedJSONObjectWithFields):
 class ChallengeResponse(ResourceMixin, TypeMixin, jose.TypedJSONObjectWithFields):
     # _fields_to_partial_json
     """ACME challenge response."""
-    TYPES = {}  # type: dict
+    TYPES: dict = {}
     resource_type = 'challenge'
     resource = fields.Resource(resource_type)
 
@@ -57,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):
@@ -140,21 +141,20 @@ 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
 
 
-@six.add_metaclass(abc.ABCMeta)
-class KeyAuthorizationChallenge(_TokenChallenge):
+class KeyAuthorizationChallenge(_TokenChallenge, metaclass=abc.ABCMeta):
     """Challenge based on Key Authorization.
 
     :param response_cls: Subclass of `KeyAuthorizationChallengeResponse`
-        that will be used to generate `response`.
+        that will be used to generate ``response``.
     :param str typ: type of the challenge
     """
-    typ = NotImplemented
-    response_cls = NotImplemented
+    typ: str = NotImplemented
+    response_cls: Type[KeyAuthorizationChallengeResponse] = NotImplemented
     thumbprint_hash_function = (
         KeyAuthorizationChallengeResponse.thumbprint_hash_function)
 
@@ -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

@@ -4,10 +4,17 @@ import collections
 import datetime
 from email.utils import parsedate_tz
 import heapq
+import http.client as http_client
 import logging
 import re
-import sys
 import time
+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
@@ -15,38 +22,21 @@ import requests
 from requests.adapters import HTTPAdapter
 from requests.utils import parse_header_links
 from requests_toolbelt.adapters.source import SourceAddressAdapter
-import six
-from six.moves import http_client
 
 from acme import crypto_util
 from acme import errors
 from acme import jws
 from acme import messages
-from acme.magic_typing import Dict
-from acme.magic_typing import List
-from acme.magic_typing import Set
-from acme.magic_typing import Text
 from acme.mixins import VersionedLEACMEMixin
 
 logger = logging.getLogger(__name__)
 
-# Prior to Python 2.7.9 the stdlib SSL module did not allow a user to configure
-# many important security related options. On these platforms we use PyOpenSSL
-# for SSL, which does allow these options to be configured.
-# https://urllib3.readthedocs.org/en/latest/security.html#insecureplatformwarning
-if sys.version_info < (2, 7, 9):  # pragma: no cover
-    try:
-        requests.packages.urllib3.contrib.pyopenssl.inject_into_urllib3()  # type: ignore
-    except AttributeError:
-        import urllib3.contrib.pyopenssl
-        urllib3.contrib.pyopenssl.inject_into_urllib3()
-
 DEFAULT_NETWORK_TIMEOUT = 45
 
 DER_CONTENT_TYPE = 'application/pkix-cert'
 
 
-class ClientBase(object):
+class ClientBase:
     """ACME client base object.
 
     :ivar messages.Directory directory:
@@ -125,8 +115,9 @@ class ClientBase(object):
         """
         return self.update_registration(regr, update={'status': 'deactivated'})
 
-    def deactivate_authorization(self, authzr):
-        # type: (messages.AuthorizationResource) -> messages.AuthorizationResource
+    def deactivate_authorization(self,
+                                 authzr: messages.AuthorizationResource
+                                 ) -> messages.AuthorizationResource:
         """Deactivate authorization.
 
         :param messages.AuthorizationResource authzr: The Authorization resource
@@ -234,6 +225,9 @@ class ClientBase(object):
 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()`.
@@ -256,14 +250,16 @@ class Client(ClientBase):
             URI from which the resource will be downloaded.
 
         """
+        warnings.warn("acme.client.Client (ACMEv1) is deprecated, "
+                      "use acme.client.ClientV2 instead.", PendingDeprecationWarning)
         self.key = key
         if net is None:
             net = ClientNetwork(key, alg=alg, verify_ssl=verify_ssl)
 
-        if isinstance(directory, six.string_types):
+        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):
@@ -436,7 +432,7 @@ class Client(ClientBase):
 
         """
         assert max_attempts > 0
-        attempts = collections.defaultdict(int) # type: Dict[messages.AuthorizationResource, int]
+        attempts: Dict[messages.AuthorizationResource, int] = collections.defaultdict(int)
         exhausted = set()
 
         # priority queue with datetime.datetime (based on Retry-After) as key,
@@ -475,7 +471,7 @@ class Client(ClientBase):
                     exhausted.add(authzr)
 
         if exhausted or any(authzr.body.status == messages.STATUS_INVALID
-                            for authzr in six.itervalues(updated)):
+                            for authzr in updated.values()):
             raise errors.PollError(exhausted, updated)
 
         updated_authzrs = tuple(updated[authzr] for authzr in authzrs)
@@ -549,7 +545,7 @@ class Client(ClientBase):
         :rtype: `list` of `OpenSSL.crypto.X509` wrapped in `.ComparableX509`
 
         """
-        chain = [] # type: List[jose.ComparableX509]
+        chain: List[jose.ComparableX509] = []
         uri = certr.cert_chain_uri
         while uri is not None and len(chain) < max_length:
             response, cert = self._get_cert(uri)
@@ -587,7 +583,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):
@@ -637,7 +633,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
@@ -668,7 +664,10 @@ class ClientV2(ClientBase):
         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,
@@ -808,10 +807,13 @@ class ClientV2(ClientBase):
                 if 'rel' in l and 'url' in l and l['rel'] == relation_type]
 
 
-class BackwardsCompatibleClientV2(object):
+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
@@ -828,8 +830,11 @@ class BackwardsCompatibleClientV2(object):
     """
 
     def __init__(self, net, key, server):
+        warnings.warn("acme.client.BackwardsCompatibleClientV2 is deprecated, use "
+                      "acme.client.ClientV2 instead.", PendingDeprecationWarning)
         directory = messages.Directory.from_json(net.get(server).json())
         self.acme_version = self._acme_version_from_directory(directory)
+        self.client: Union[Client, ClientV2]
         if self.acme_version == 1:
             self.client = Client(directory, key=key, net=net)
         else:
@@ -849,16 +854,18 @@ class BackwardsCompatibleClientV2(object):
             if check_tos_cb is not None:
                 check_tos_cb(tos)
         if self.acme_version == 1:
-            regr = self.client.register(regr)
+            client_v1 = cast(Client, self.client)
+            regr = client_v1.register(regr)
             if regr.terms_of_service is not None:
                 _assess_tos(regr.terms_of_service)
-                return self.client.agree_to_tos(regr)
+                return client_v1.agree_to_tos(regr)
             return regr
         else:
-            if "terms_of_service" in self.client.directory.meta:
-                _assess_tos(self.client.directory.meta.terms_of_service)
+            client_v2 = cast(ClientV2, self.client)
+            if "terms_of_service" in client_v2.directory.meta:
+                _assess_tos(client_v2.directory.meta.terms_of_service)
                 regr = regr.update(terms_of_service_agreed=True)
-            return self.client.new_account(regr)
+            return client_v2.new_account(regr)
 
     def new_order(self, csr_pem):
         """Request a new Order object from the server.
@@ -876,14 +883,15 @@ class BackwardsCompatibleClientV2(object):
 
         """
         if self.acme_version == 1:
+            client_v1 = cast(Client, self.client)
             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)
             authorizations = []
             for domain in dnsNames:
-                authorizations.append(self.client.request_domain_challenges(domain))
+                authorizations.append(client_v1.request_domain_challenges(domain))
             return messages.OrderResource(authorizations=authorizations, csr_pem=csr_pem)
-        return self.client.new_order(csr_pem)
+        return cast(ClientV2, self.client).new_order(csr_pem)
 
     def finalize_order(self, orderr, deadline, fetch_alternative_chains=False):
         """Finalize an order and obtain a certificate.
@@ -898,8 +906,9 @@ class BackwardsCompatibleClientV2(object):
 
         """
         if self.acme_version == 1:
+            client_v1 = cast(Client, self.client)
             csr_pem = orderr.csr_pem
-            certr = self.client.request_issuance(
+            certr = client_v1.request_issuance(
                 jose.ComparableX509(
                     OpenSSL.crypto.load_certificate_request(OpenSSL.crypto.FILETYPE_PEM, csr_pem)),
                     orderr.authorizations)
@@ -907,7 +916,7 @@ class BackwardsCompatibleClientV2(object):
             chain = None
             while datetime.datetime.now() < deadline:
                 try:
-                    chain = self.client.fetch_chain(certr)
+                    chain = client_v1.fetch_chain(certr)
                     break
                 except errors.Error:
                     time.sleep(1)
@@ -922,7 +931,8 @@ class BackwardsCompatibleClientV2(object):
             chain = crypto_util.dump_pyopenssl_chain(chain).decode()
 
             return orderr.update(fullchain_pem=(cert + chain))
-        return self.client.finalize_order(orderr, deadline, fetch_alternative_chains)
+        return cast(ClientV2, self.client).finalize_order(
+            orderr, deadline, fetch_alternative_chains)
 
     def revoke(self, cert, rsn):
         """Revoke certificate.
@@ -948,10 +958,10 @@ class BackwardsCompatibleClientV2(object):
         Always return False for ACMEv1 servers, as it doesn't use External Account Binding."""
         if self.acme_version == 1:
             return False
-        return self.client.external_account_required()
+        return cast(ClientV2, self.client).external_account_required()
 
 
-class ClientNetwork(object):
+class ClientNetwork:
     """Wrapper around requests that signs POSTs for authentication.
 
     Also adds user agent, and handles Content-Type.
@@ -981,7 +991,7 @@ class ClientNetwork(object):
         self.account = account
         self.alg = alg
         self.verify_ssl = verify_ssl
-        self._nonces = set() # type: Set[Text]
+        self._nonces: Set[Text] = set()
         self.user_agent = user_agent
         self.session = requests.Session()
         self._default_timeout = timeout
@@ -1139,12 +1149,19 @@ class ClientNetwork(object):
             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 response.headers.get("Content-Type") == DER_CONTENT_TYPE:
+        # 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 or
+                "Accept" in kwargs["headers"]):
             debug_content = base64.b64encode(response.content)
         else:
-            debug_content = response.content.decode("utf-8")
+            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)

acme/acme/crypto_util.py

@@ -5,15 +5,15 @@ import logging
 import os
 import re
 import socket
+from typing import Callable
+from typing import Tuple
+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
-from acme.magic_typing import Callable
-from acme.magic_typing import Tuple
-from acme.magic_typing import Union
 
 logger = logging.getLogger(__name__)
 
@@ -24,10 +24,10 @@ 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(object):
+class _DefaultCertSelection:
     def __init__(self, certs):
         self.certs = certs
 
@@ -36,7 +36,7 @@ class _DefaultCertSelection(object):
         return self.certs.get(server_name, None)
 
 
-class SSLSocket(object):  # pylint: disable=too-few-public-methods
+class SSLSocket:  # pylint: disable=too-few-public-methods
     """SSL wrapper for sockets.
 
     :ivar socket sock: Original wrapped socket.
@@ -93,7 +93,7 @@ class SSLSocket(object):  # pylint: disable=too-few-public-methods
             new_context.set_alpn_select_callback(self.alpn_selection)
         connection.set_context(new_context)
 
-    class FakeConnection(object):
+    class FakeConnection:
         """Fake OpenSSL.SSL.Connection."""
 
         # pylint: disable=missing-function-docstring
@@ -166,10 +166,10 @@ def probe_sni(name, host, port=443, timeout=300, # pylint: disable=too-many-argu
             " from {0}:{1}".format(
                 source_address[0],
                 source_address[1]
-            ) if socket_kwargs else ""
+            ) if any(source_address) else ""
         )
-        socket_tuple = (host, port)  # type: Tuple[str, int]
-        sock = socket.create_connection(socket_tuple, **socket_kwargs)  # type: ignore
+        socket_tuple: Tuple[str, int] = (host, port)
+        sock = socket.create_connection(socket_tuple, **socket_kwargs)
     except socket.error as error:
         raise errors.Error(error)
 
@@ -256,7 +256,7 @@ def _pyopenssl_cert_or_req_san(cert_or_req):
 
     if isinstance(cert_or_req, crypto.X509):
         # pylint: disable=line-too-long
-        func = crypto.dump_certificate # type: Union[Callable[[int, crypto.X509Req], bytes], Callable[[int, crypto.X509], bytes]]
+        func: Union[Callable[[int, crypto.X509Req], bytes], Callable[[int, crypto.X509], bytes]] = crypto.dump_certificate
     else:
         func = crypto.dump_certificate_request
     text = func(crypto.FILETYPE_TEXT, cert_or_req).decode("utf-8")

acme/acme/errors.py

@@ -28,13 +28,8 @@ class NonceError(ClientError):
 
 class BadNonce(NonceError):
     """Bad nonce error."""
-    def __init__(self, nonce, error, *args, **kwargs):
-        # MyPy complains here that there is too many arguments for BaseException constructor.
-        # This is an error fixed in typeshed, see https://github.com/python/mypy/issues/4183
-        # The fix is included in MyPy>=0.740, but upgrading it would bring dozen of errors due to
-        #   new types definitions. So we ignore the error until the code base is fixed to match
-        #   with MyPy>=0.740 referential.
-        super(BadNonce, self).__init__(*args, **kwargs)  # type: ignore
+    def __init__(self, nonce, error, *args):
+        super().__init__(*args)
         self.nonce = nonce
         self.error = error
 
@@ -49,12 +44,11 @@ class MissingNonce(NonceError):
     Replay-Nonce header field in each successful response to a POST it
     provides to a client (...)".
 
-    :ivar requests.Response response: HTTP Response
+    :ivar requests.Response ~.response: HTTP Response
 
     """
-    def __init__(self, response, *args, **kwargs):
-        # See comment in BadNonce constructor above for an explanation of type: ignore here.
-        super(MissingNonce, self).__init__(*args, **kwargs)  # type: ignore
+    def __init__(self, response, *args):
+        super().__init__(*args)
         self.response = response
 
     def __str__(self):
@@ -78,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):
@@ -96,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
@@ -112,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):
@@ -125,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

@@ -14,7 +14,9 @@ class Header(jose.Header):
     kid = jose.Field('kid', omitempty=True)
     url = jose.Field('url', omitempty=True)
 
-    @nonce.decoder
+    # Mypy does not understand the josepy magic happening here, and falsely claims
+    # that nonce is redefined. Let's ignore the type check here.
+    @nonce.decoder  # type: ignore
     def nonce(value):  # pylint: disable=no-self-argument,missing-function-docstring
         try:
             return jose.decode_b64jose(value)
@@ -48,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

@@ -1,16 +1,17 @@
-"""Shim class to not have to depend on typing module in prod."""
-import sys
+"""Simple shim around the typing module.
 
+This was useful when this code supported Python 2 and typing wasn't always
+available. This code is being kept for now for backwards compatibility.
 
-class TypingClass(object):
+"""
+import warnings
+from typing import *  # pylint: disable=wildcard-import, unused-wildcard-import
+from typing import Collection, IO
+
+warnings.warn("acme.magic_typing is deprecated and will be removed in a future release.",
+              DeprecationWarning)
+
+class TypingClass:
     """Ignore import errors by getting anything"""
     def __getattr__(self, name):
-        return None
-
-try:
-    # mypy doesn't respect modifying sys.modules
-    from typing import *  # pylint: disable=wildcard-import, unused-wildcard-import
-    from typing import Collection, IO  # type: ignore
-except ImportError:
-    # mypy complains because TypingClass is not a module
-    sys.modules[__name__] = TypingClass()  # type: ignore
+        return None  # pragma: no cover

acme/acme/messages.py

@@ -1,8 +1,11 @@
 """ACME protocol messages."""
+from collections.abc import Hashable
 import json
+from typing import Any
+from typing import Dict
+from typing import Type
 
 import josepy as jose
-import six
 
 from acme import challenges
 from acme import errors
@@ -11,13 +14,6 @@ from acme import jws
 from acme import util
 from acme.mixins import ResourceMixin
 
-try:
-    from collections.abc import Hashable
-except ImportError:  # pragma: no cover
-    from collections import Hashable
-
-
-
 OLD_ERROR_PREFIX = "urn:acme:error:"
 ERROR_PREFIX = "urn:ietf:params:acme:error:"
 
@@ -68,7 +64,6 @@ def is_acme_error(err):
     return False
 
 
-@six.python_2_unicode_compatible
 class Error(jose.JSONObjectWithFields, errors.Error):
     """ACME error.
 
@@ -95,7 +90,9 @@ class Error(jose.JSONObjectWithFields, errors.Error):
             raise ValueError("The supplied code: %s is not a known ACME error"
                              " code" % code)
         typ = ERROR_PREFIX + code
-        return cls(typ=typ, **kwargs)
+        # Mypy will not understand that the Error constructor accepts a named argument
+        # "typ" because of josepy magic. Let's ignore the type check here.
+        return cls(typ=typ, **kwargs)  # type: ignore
 
     @property
     def description(self):
@@ -117,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
@@ -129,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 = NotImplemented
+    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
 
@@ -158,13 +155,10 @@ class _Constant(jose.JSONDeSerializable, Hashable):  # type: ignore
     def __hash__(self):
         return hash((self.__class__, self.name))
 
-    def __ne__(self, other):
-        return not self == other
-
 
 class Status(_Constant):
     """ACME "status" field."""
-    POSSIBLE_NAMES = {}  # type: dict
+    POSSIBLE_NAMES: dict = {}
 STATUS_UNKNOWN = Status('unknown')
 STATUS_PENDING = Status('pending')
 STATUS_PROCESSING = Status('processing')
@@ -177,7 +171,7 @@ STATUS_DEACTIVATED = Status('deactivated')
 
 class IdentifierType(_Constant):
     """ACME identifier type."""
-    POSSIBLE_NAMES = {}  # type: dict
+    POSSIBLE_NAMES: Dict[str, 'IdentifierType'] = {}
 IDENTIFIER_FQDN = IdentifierType('dns')  # IdentifierDNS in Boulder
 
 
@@ -195,7 +189,7 @@ class Identifier(jose.JSONObjectWithFields):
 class Directory(jose.JSONDeSerializable):
     """Directory."""
 
-    _REGISTERED_TYPES = {}  # type: dict
+    _REGISTERED_TYPES: Dict[str, Type[Any]] = {}
 
     class Meta(jose.JSONObjectWithFields):
         """Directory Meta."""
@@ -207,7 +201,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):
@@ -217,7 +211,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):
@@ -229,7 +223,7 @@ class Directory(jose.JSONDeSerializable):
         return getattr(key, 'resource_type', key)
 
     @classmethod
-    def register(cls, resource_body_cls):
+    def register(cls, resource_body_cls: Type[Any]) -> Type[Any]:
         """Register resource."""
         resource_type = resource_body_cls.resource_type
         assert resource_type not in cls._REGISTERED_TYPES
@@ -275,7 +269,7 @@ class Resource(jose.JSONObjectWithFields):
 class ResourceWithURI(Resource):
     """ACME Resource with URI.
 
-    :ivar unicode uri: Location of the resource.
+    :ivar unicode ~.uri: Location of the resource.
 
     """
     uri = jose.Field('uri')  # no ChallengeResource.uri
@@ -285,7 +279,7 @@ class ResourceBody(jose.JSONObjectWithFields):
     """ACME Resource Body."""
 
 
-class ExternalAccountBinding(object):
+class ExternalAccountBinding:
     """ACME External Account Binding"""
 
     @classmethod
@@ -363,7 +357,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(
@@ -389,12 +383,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
@@ -466,19 +460,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
 
@@ -493,7 +487,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):
@@ -539,7 +533,9 @@ class Authorization(ResourceBody):
     expires = fields.RFC3339Field('expires', omitempty=True)
     wildcard = jose.Field('wildcard', omitempty=True)
 
-    @challenges.decoder
+    # Mypy does not understand the josepy magic happening here, and falsely claims
+    # that challenge is redefined. Let's ignore the type check here.
+    @challenges.decoder  # type: ignore
     def challenges(value):  # pylint: disable=no-self-argument,missing-function-docstring
         return tuple(ChallengeBody.from_json(chall) for chall in value)
 
@@ -627,7 +623,7 @@ class Order(ResourceBody):
     :ivar str finalize: URL to POST to to request issuance once all
         authorizations have "valid" status.
     :ivar datetime.datetime expires: When the order expires.
-    :ivar .Error error: Any error that occurred during finalization, if applicable.
+    :ivar ~.Error error: Any error that occurred during finalization, if applicable.
     """
     identifiers = jose.Field('identifiers', omitempty=True)
     status = jose.Field('status', decoder=Status.from_json,
@@ -638,7 +634,9 @@ class Order(ResourceBody):
     expires = fields.RFC3339Field('expires', omitempty=True)
     error = jose.Field('error', omitempty=True, decoder=Error.from_json)
 
-    @identifiers.decoder
+    # Mypy does not understand the josepy magic happening here, and falsely claims
+    # that identifiers is redefined. Let's ignore the type check here.
+    @identifiers.decoder  # type: ignore
     def identifiers(value):  # pylint: disable=no-self-argument,missing-function-docstring
         return tuple(Identifier.from_json(identifier) for identifier in value)
 

acme/acme/mixins.py

@@ -1,7 +1,7 @@
 """Useful mixins for Challenge and Resource objects"""
 
 
-class VersionedLEACMEMixin(object):
+class VersionedLEACMEMixin:
     """This mixin stores the version of Let's Encrypt's endpoint being used."""
     @property
     def le_acme_version(self):
@@ -20,7 +20,7 @@ class VersionedLEACMEMixin(object):
             # 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

@@ -1,17 +1,17 @@
 """Support for standalone client challenge solvers. """
 import collections
 import functools
+import http.client as http_client
+import http.server as BaseHTTPServer
 import logging
 import socket
+import socketserver
 import threading
-
-from six.moves import BaseHTTPServer  # type: ignore
-from six.moves import http_client
-from six.moves import socketserver  # type: ignore
+from typing import List
+from typing import Optional
 
 from acme import challenges
 from acme import crypto_util
-from acme.magic_typing import List
 
 logger = logging.getLogger(__name__)
 
@@ -54,7 +54,7 @@ class ACMEServerMixin:
     allow_reuse_address = True
 
 
-class BaseDualNetworkedServers(object):
+class BaseDualNetworkedServers:
     """Base class for a pair of IPv6 and IPv4 servers that tries to do everything
        it's asked for both servers, but where failures in one server don't
        affect the other.
@@ -64,8 +64,11 @@ class BaseDualNetworkedServers(object):
 
     def __init__(self, ServerClass, server_address, *remaining_args, **kwargs):
         port = server_address[1]
-        self.threads = [] # type: List[threading.Thread]
-        self.servers = [] # type: List[ACMEServerMixin]
+        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
@@ -83,7 +86,8 @@ class BaseDualNetworkedServers(object):
                 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(
@@ -102,7 +106,10 @@ class BaseDualNetworkedServers(object):
                 # 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"""
@@ -204,8 +211,24 @@ class HTTP01RequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
 
     def __init__(self, *args, **kwargs):
         self.simple_http_resources = kwargs.pop("simple_http_resources", set())
-        self.timeout = kwargs.pop('timeout', 30)
+        self._timeout = kwargs.pop('timeout', 30)
         BaseHTTPServer.BaseHTTPRequestHandler.__init__(self, *args, **kwargs)
+        self.server: HTTP01Server
+
+    # In parent class BaseHTTPRequestHandler, 'timeout' is a class-level property but we
+    # need to define its value during the initialization phase in HTTP01RequestHandler.
+    # However MyPy does not appreciate that we dynamically shadow a class-level property
+    # with an instance-level property (eg. self.timeout = ... in __init__()). So to make
+    # everyone happy, we statically redefine 'timeout' as a method property, and set the
+    # timeout value in a new internal instance-level property _timeout.
+    @property
+    def timeout(self):
+        """
+        The default timeout this server should apply to requests.
+        :return: timeout to apply
+        :rtype: int
+        """
+        return self._timeout
 
     def log_message(self, format, *args):  # pylint: disable=redefined-builtin
         """Log arbitrary message."""

acme/acme/util.py

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

acme/docs/conf.py

@@ -85,7 +85,9 @@ language = 'en'
 
 # List of patterns, relative to source directory, that match files and
 # directories to ignore when looking for source files.
-exclude_patterns = ['_build']
+exclude_patterns = [
+    '_build',
+]
 
 # The reST default role (used for this markup: `text`) to use for all
 # documents.

acme/docs/man/jws.rst

@@ -1 +1,3 @@
+:orphan:
+
 .. literalinclude:: ../jws-help.txt

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

@@ -1,44 +1,22 @@
-from distutils.version import LooseVersion
 import sys
 
-from setuptools import __version__ as setuptools_version
 from setuptools import find_packages
 from setuptools import setup
 
-version = '1.11.0.dev0'
+version = '1.19.0.dev0'
 
-# Please update tox.ini when modifying dependency version requirements
 install_requires = [
-    # load_pem_private/public_key (>=0.6)
-    # rsa_recover_prime_factors (>=0.8)
-    'cryptography>=1.2.3',
+    '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',
-    # Connection.set_tlsext_host_name (>=0.13) + matching Xenial requirements (>=0.15.1)
-    'PyOpenSSL>=0.15.1',
+    'PyOpenSSL>=17.3.0',
     'pyrfc3339',
     'pytz',
-    'requests[security]>=2.6.0',  # security extras added in 2.4.1
+    'requests>=2.14.2',
     'requests-toolbelt>=0.3.0',
-    'setuptools',
-    'six>=1.9.0',  # needed for python_2_unicode_compatible
-]
-
-setuptools_known_environment_markers = (LooseVersion(setuptools_version) >= LooseVersion('36.2'))
-if setuptools_known_environment_markers:
-    install_requires.append('mock ; python_version < "3.3"')
-elif 'bdist_wheel' in sys.argv[1:]:
-    raise RuntimeError('Error, you are trying to build certbot wheels using an old version '
-                       'of setuptools. Version 36.2+ of setuptools is required.')
-elif sys.version_info < (3,3):
-    install_requires.append('mock')
-
-dev_extras = [
-    'pytest',
-    'pytest-xdist',
-    'tox',
+    'setuptools>=39.0.1',
 ]
 
 docs_extras = [
@@ -46,22 +24,25 @@ docs_extras = [
     '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='>=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*, !=3.5.*',
+    python_requires='>=3.6',
     classifiers=[
         'Development Status :: 5 - Production/Stable',
         'Intended Audience :: Developers',
         'License :: OSI Approved :: Apache Software License',
         'Programming Language :: Python',
-        'Programming Language :: Python :: 2',
-        'Programming Language :: Python :: 2.7',
         'Programming Language :: Python :: 3',
         'Programming Language :: Python :: 3.6',
         'Programming Language :: Python :: 3.7',
@@ -75,7 +56,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

@@ -1,14 +1,11 @@
 """Tests for acme.challenges."""
+import urllib.parse as urllib_parse
 import unittest
+from unittest import mock
 
 import josepy as jose
 import OpenSSL
-try:
-    import mock
-except ImportError: # pragma: no cover
-    from unittest import mock # type: ignore
 import requests
-from six.moves.urllib import parse as urllib_parse
 
 from acme import errors
 
@@ -295,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):
@@ -434,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

@@ -2,17 +2,15 @@
 # pylint: disable=too-many-lines
 import copy
 import datetime
+import http.client as http_client
 import json
 import unittest
+from typing import Dict
+from unittest import mock
 
 import josepy as jose
-try:
-    import mock
-except ImportError: # pragma: no cover
-    from unittest import mock # type: ignore
 import OpenSSL
 import requests
-from six.moves import http_client  # pylint: disable=import-error
 
 from acme import challenges
 from acme import errors
@@ -64,7 +62,7 @@ class ClientTestBase(unittest.TestCase):
         self.contact = ('mailto:cert-admin@example.com', 'tel:+12025551212')
         reg = messages.Registration(
             contact=self.contact, key=KEY.public_key())
-        the_arg = dict(reg)  # type: Dict
+        the_arg: Dict = dict(reg)
         self.new_reg = messages.NewRegistration(**the_arg)
         self.regr = messages.RegistrationResource(
             body=reg, uri='https://www.letsencrypt-demo.org/acme/reg/1')
@@ -92,7 +90,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)
@@ -321,7 +319,7 @@ class ClientTest(ClientTestBase):
     """Tests for acme.client.Client."""
 
     def setUp(self):
-        super(ClientTest, self).setUp()
+        super().setUp()
 
         self.directory = DIRECTORY_V1
 
@@ -606,8 +604,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, [
@@ -643,7 +641,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
@@ -702,7 +700,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):
@@ -718,7 +716,7 @@ class ClientV2Test(ClientTestBase):
     """Tests for acme.client.ClientV2."""
 
     def setUp(self):
-        super(ClientV2Test, self).setUp()
+        super().setUp()
 
         self.directory = DIRECTORY_V2
 
@@ -879,9 +877,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()
@@ -945,7 +943,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
@@ -1187,7 +1185,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":
@@ -1332,7 +1330,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,14 +1,14 @@
 """Tests for acme.crypto_util."""
 import itertools
 import socket
+import socketserver
 import threading
 import time
 import unittest
+from typing import List
 
 import josepy as jose
 import OpenSSL
-import six
-from six.moves import socketserver  # type: ignore  # pylint: disable=import-error
 
 from acme import errors
 import test_util
@@ -27,8 +27,6 @@ class SSLSocketAndProbeSNITest(unittest.TestCase):
 
         class _TestServer(socketserver.TCPServer):
 
-            # six.moves.* | pylint: disable=attribute-defined-outside-init,no-init
-
             def server_bind(self):  # pylint: disable=missing-docstring
                 self.socket = SSLSocket(socket.socket(),
                         certs)
@@ -62,7 +60,6 @@ class SSLSocketAndProbeSNITest(unittest.TestCase):
         self.assertRaises(errors.Error, self._probe, b'bar')
 
     def test_probe_connection_error(self):
-        # pylint has a hard time with six
         self.server.server_close()
         original_timeout = socket.getdefaulttimeout()
         try:
@@ -121,9 +118,9 @@ class PyOpenSSLCertOrReqSANTest(unittest.TestCase):
     @classmethod
     def _get_idn_names(cls):
         """Returns expected names from '{cert,csr}-idnsans.pem'."""
-        chars = [six.unichr(i) for i in itertools.chain(range(0x3c3, 0x400),
-                                                        range(0x641, 0x6fc),
-                                                        range(0x1820, 0x1877))]
+        chars = [chr(i) for i in itertools.chain(range(0x3c3, 0x400),
+                                                 range(0x641, 0x6fc),
+                                                 range(0x1820, 0x1877))]
         return [''.join(chars[i: i + 45]) + '.invalid'
                 for i in range(0, len(chars), 45)]
 
@@ -184,7 +181,7 @@ class RandomSnTest(unittest.TestCase):
 
     def setUp(self):
         self.cert_count = 5
-        self.serial_num = [] # type: List[int]
+        self.serial_num: List[int] = []
         self.key = OpenSSL.crypto.PKey()
         self.key.generate_key(OpenSSL.crypto.TYPE_RSA, 2048)
 
@@ -194,7 +191,7 @@ class RandomSnTest(unittest.TestCase):
         for _ in range(self.cert_count):
             cert = gen_ss_cert(self.key, ['dummy'], force_san=True)
             self.serial_num.append(cert.get_serial_number())
-        self.assertTrue(len(set(self.serial_num)) > 1)
+        self.assertGreater(len(set(self.serial_num)), 1)
 
 class MakeCSRTest(unittest.TestCase):
     """Test for standalone functions."""
@@ -209,8 +206,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

acme/tests/errors_test.py

@@ -1,10 +1,6 @@
 """Tests for acme.errors."""
 import unittest
-
-try:
-    import mock
-except ImportError: # pragma: no cover
-    from unittest import mock # type: ignore
+from unittest import mock
 
 
 class BadNonceTest(unittest.TestCase):
@@ -28,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/magic_typing_test.py

@@ -1,11 +1,8 @@
 """Tests for acme.magic_typing."""
 import sys
 import unittest
-
-try:
-    import mock
-except ImportError: # pragma: no cover
-    from unittest import mock # type: ignore
+import warnings
+from unittest import mock
 
 
 class MagicTypingTest(unittest.TestCase):
@@ -13,32 +10,21 @@ class MagicTypingTest(unittest.TestCase):
     def test_import_success(self):
         try:
             import typing as temp_typing
-        except ImportError: # pragma: no cover
-            temp_typing = None # pragma: no cover
+        except ImportError:  # pragma: no cover
+            temp_typing = None  # pragma: no cover
         typing_class_mock = mock.MagicMock()
         text_mock = mock.MagicMock()
         typing_class_mock.Text = text_mock
         sys.modules['typing'] = typing_class_mock
         if 'acme.magic_typing' in sys.modules:
-            del sys.modules['acme.magic_typing'] # pragma: no cover
-        from acme.magic_typing import Text
+            del sys.modules['acme.magic_typing']  # pragma: no cover
+        with warnings.catch_warnings():
+            warnings.filterwarnings("ignore", category=DeprecationWarning)
+            from acme.magic_typing import Text
         self.assertEqual(Text, text_mock)
         del sys.modules['acme.magic_typing']
         sys.modules['typing'] = temp_typing
 
-    def test_import_failure(self):
-        try:
-            import typing as temp_typing
-        except ImportError: # pragma: no cover
-            temp_typing = None # pragma: no cover
-        sys.modules['typing'] = None
-        if 'acme.magic_typing' in sys.modules:
-            del sys.modules['acme.magic_typing'] # pragma: no cover
-        from acme.magic_typing import Text
-        self.assertTrue(Text is None)
-        del sys.modules['acme.magic_typing']
-        sys.modules['typing'] = temp_typing
-
 
 if __name__ == '__main__':
     unittest.main()  # pragma: no cover

acme/tests/messages_test.py

@@ -1,11 +1,9 @@
 """Tests for acme.messages."""
+from typing import Dict
 import unittest
+from unittest import mock
 
 import josepy as jose
-try:
-    import mock
-except ImportError: # pragma: no cover
-    from unittest import mock # type: ignore
 
 from acme import challenges
 import test_util
@@ -43,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
@@ -84,7 +82,7 @@ class ConstantTest(unittest.TestCase):
         from acme.messages import _Constant
 
         class MockConstant(_Constant):  # pylint: disable=missing-docstring
-            POSSIBLE_NAMES = {} # type: Dict
+            POSSIBLE_NAMES: Dict = {}
 
         self.MockConstant = MockConstant  # pylint: disable=invalid-name
         self.const_a = MockConstant('a')
@@ -262,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):
@@ -408,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):
@@ -419,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()))
@@ -435,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

@@ -1,16 +1,14 @@
 """Tests for acme.standalone."""
+import http.client as http_client
 import socket
+import socketserver
 import threading
 import unittest
+from typing import Set
+from unittest import mock
 
 import josepy as jose
-try:
-    import mock
-except ImportError: # pragma: no cover
-    from unittest import mock # type: ignore
 import requests
-from six.moves import http_client  # pylint: disable=import-error
-from six.moves import socketserver  # type: ignore  # pylint: disable=import-error
 
 from acme import challenges
 from acme import crypto_util
@@ -44,7 +42,7 @@ class HTTP01ServerTest(unittest.TestCase):
     def setUp(self):
         self.account_key = jose.JWK.load(
             test_util.load_vector('rsa1024_key.pem'))
-        self.resources = set() # type: Set
+        self.resources: Set = set()
 
         from acme.standalone import HTTP01Server
         self.server = HTTP01Server(('', 0), resources=self.resources)
@@ -192,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
@@ -221,7 +225,7 @@ class HTTP01DualNetworkedServersTest(unittest.TestCase):
     def setUp(self):
         self.account_key = jose.JWK.load(
             test_util.load_vector('rsa1024_key.pem'))
-        self.resources = set() # type: Set
+        self.resources: Set = set()
 
         from acme.standalone import HTTP01DualNetworkedServers
         self.servers = HTTP01DualNetworkedServers(('', 0), resources=self.resources)

certbot-apache/certbot_apache/_internal/apache_util.py

@@ -9,7 +9,6 @@ import pkg_resources
 
 from certbot import errors
 from certbot import util
-
 from certbot.compat import os
 
 logger = logging.getLogger(__name__)
@@ -154,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
 
@@ -221,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

@@ -1,4 +1,5 @@
 """ apacheconfig implementation of the ParserNode interfaces """
+from typing import Tuple
 
 from certbot_apache._internal import assertions
 from certbot_apache._internal import interfaces
@@ -14,14 +15,14 @@ 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
         self.metadata = metadata
         self._raw = self.metadata["ac_ast"]
 
-    def save(self, msg): # pragma: no cover
+    def save(self, msg):  # pragma: no cover
         pass
 
     def find_ancestors(self, name):  # pylint: disable=unused-variable
@@ -38,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
@@ -56,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
@@ -82,8 +83,8 @@ class ApacheBlockNode(ApacheDirectiveNode):
     """ apacheconfig implementation of BlockNode interface """
 
     def __init__(self, **kwargs):
-        super(ApacheBlockNode, self).__init__(**kwargs)
-        self.children = ()
+        super().__init__(**kwargs)
+        self.children: Tuple[ApacheParserNode, ...] = ()
 
     def __eq__(self, other):  # pragma: no cover
         if isinstance(other, self.__class__):

certbot-apache/certbot_apache/_internal/assertions.py

@@ -3,7 +3,6 @@ import fnmatch
 
 from certbot_apache._internal import interfaces
 
-
 PASS = "CERTBOT_PASS_ASSERT"
 
 
@@ -137,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

@@ -64,10 +64,10 @@ Translates over to:
     "/files/etc/apache2/apache2.conf/bLoCk[1]",
 ]
 """
-from acme.magic_typing import Set
+from typing import Set
+
 from certbot import errors
 from certbot.compat import os
-
 from certbot_apache._internal import apache_util
 from certbot_apache._internal import assertions
 from certbot_apache._internal import interfaces
@@ -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):
@@ -355,7 +355,7 @@ class AugeasBlockNode(AugeasDirectiveNode):
         ownpath = self.metadata.get("augeaspath")
 
         directives = self.parser.find_dir(name, start=ownpath, exclude=exclude)
-        already_parsed = set()  # type: Set[str]
+        already_parsed: Set[str] = set()
         for directive in directives:
             # Remove the /arg part from the Augeas path
             directive = directive.partition("/arg")[0]

certbot-apache/certbot_apache/_internal/configurator.py

@@ -1,34 +1,28 @@
 """Apache Configurator."""
 # pylint: disable=too-many-lines
 from collections import defaultdict
-from distutils.version import LooseVersion
 import copy
+from distutils.version import LooseVersion
 import fnmatch
 import logging
 import re
 import socket
 import time
-
-import zope.component
-import zope.interface
-try:
-    import apacheconfig
-    HAS_APACHECONFIG = True
-except ImportError:  # pragma: no cover
-    HAS_APACHECONFIG = False
+from typing import DefaultDict
+from typing import Dict
+from typing import List
+from typing import Optional
+from typing import Set
+from typing import Union
 
 from acme import challenges
-from acme.magic_typing import DefaultDict
-from acme.magic_typing import Dict
-from acme.magic_typing import List
-from acme.magic_typing import Set
-from acme.magic_typing import Union
 from certbot import errors
 from certbot import interfaces
 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
@@ -40,10 +34,61 @@ 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
+    HAS_APACHECONFIG = True
+except ImportError:  # pragma: no cover
+    HAS_APACHECONFIG = False
+
 
 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
@@ -72,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`
@@ -99,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):
         """
@@ -149,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
-        self.options["version_cmd"][0] = self.option("ctl")
-        self.options["restart_cmd"][0] = self.option("ctl")
-        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):
@@ -171,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):
@@ -207,33 +229,33 @@ 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 = {}  # type: Dict[str, obj.VirtualHost]
+        self.assoc: Dict[str, obj.VirtualHost] = {}
         # Outstanding challenges
-        self._chall_out = set()  # type: Set[KeyAuthorizationAnnotatedChallenge]
+        self._chall_out: Set[KeyAuthorizationAnnotatedChallenge] = set()
         # List of vhosts configured per wildcard domain on this run.
         # used by deploy_cert() and enhance()
-        self._wildcard_vhosts = {}  # type: Dict[str, List[obj.VirtualHost]]
+        self._wildcard_vhosts: Dict[str, List[obj.VirtualHost]] = {}
         # Maps enhancements to vhosts we've enabled the enhancement for
-        self._enhanced_vhosts = defaultdict(set)  # type: DefaultDict[str, Set[obj.VirtualHost]]
+        self._enhanced_vhosts: DefaultDict[str, Set[obj.VirtualHost]] = defaultdict(set)
         # Temporary state for AutoHSTS enhancement
-        self._autohsts = {}  # type: Dict[str, Dict[str, Union[int, float]]]
+        self._autohsts: Dict[str, Dict[str, Union[int, float]]] = {}
         # Reverter save notes
         self.save_notes = ""
         # Should we use ParserNode implementation instead of the old behavior
         self.USE_PARSERNODE = use_parsernode
         # Saves the list of file paths that were parsed initially, and
         # not added to parser tree by self.conf("vhost-root") for example.
-        self.parsed_paths = []  # type: List[str]
+        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,
@@ -283,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.")
@@ -314,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()
@@ -342,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")
@@ -353,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):
@@ -402,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()
 
@@ -427,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):
@@ -441,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):
@@ -449,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:
@@ -487,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):
         """
@@ -525,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
@@ -562,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.
@@ -691,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:
@@ -832,7 +860,7 @@ class ApacheConfigurator(common.Installer):
         :rtype: set
 
         """
-        all_names = set()  # type: Set[str]
+        all_names: Set[str] = set()
 
         vhost_macro = []
 
@@ -850,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)
@@ -996,8 +1024,8 @@ class ApacheConfigurator(common.Installer):
 
         """
         # Search base config, and all included paths for VirtualHosts
-        file_paths = {}  # type: Dict[str, str]
-        internal_paths = defaultdict(set)  # type: DefaultDict[str, Set[str]]
+        file_paths: Dict[str, str] = {}
+        internal_paths: DefaultDict[str, Set[str]] = defaultdict(set)
         vhs = []
         # Make a list of parser paths because the parser_paths
         # dictionary may be modified during the loop.
@@ -1048,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:
@@ -1300,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)
@@ -1320,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
 
@@ -1419,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.
@@ -1501,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")
 
@@ -1835,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):
@@ -2156,7 +2186,7 @@ class ApacheConfigurator(common.Installer):
         # There can be other RewriteRule directive lines in vhost config.
         # rewrite_args_dict keys are directive ids and the corresponding value
         # for each is a list of arguments to that directive.
-        rewrite_args_dict = defaultdict(list)  # type: DefaultDict[str, List[str]]
+        rewrite_args_dict: DefaultDict[str, List[str]] = defaultdict(list)
         pat = r'(.*directive\[\d+\]).*'
         for match in rewrite_path:
             m = re.match(pat, match)
@@ -2250,7 +2280,7 @@ class ApacheConfigurator(common.Installer):
         if ssl_vhost.aliases:
             serveralias = "ServerAlias " + " ".join(ssl_vhost.aliases)
 
-        rewrite_rule_args = []  # type: List[str]
+        rewrite_rule_args: List[str] = []
         if self.get_version() >= (2, 3, 9):
             rewrite_rule_args = constants.REWRITE_HTTPS_ARGS_WITH_END
         else:
@@ -2271,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
@@ -2283,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
@@ -2365,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.
@@ -2403,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)
@@ -2430,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))
 
@@ -2446,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)
@@ -2470,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
     ###########################################################################
@@ -2563,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:
@@ -2649,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
@@ -2689,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/dualparser.py

@@ -1,10 +1,10 @@
 """ Dual ParserNode implementation """
+from certbot_apache._internal import apacheparser
 from certbot_apache._internal import assertions
 from certbot_apache._internal import augeasparser
-from certbot_apache._internal import apacheparser
 
 
-class DualNodeBase(object):
+class DualNodeBase:
     """ Dual parser interface for in development testing. This is used as the
     base class for dual parser interface classes. This class handles runtime
     attribute value assertions."""

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

@@ -1,9 +1,9 @@
 """A class that performs HTTP-01 challenges for Apache"""
-import logging
 import errno
+import logging
+from typing import List
+from typing import Set
 
-from acme.magic_typing import List
-from acme.magic_typing import Set
 from certbot import errors
 from certbot.compat import filesystem
 from certbot.compat import os
@@ -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")
@@ -57,7 +57,7 @@ class ApacheHttp01(common.ChallengePerformer):
         self.challenge_dir = os.path.join(
             self.configurator.config.work_dir,
             "http_challenges")
-        self.moded_vhosts = set()  # type: Set[VirtualHost]
+        self.moded_vhosts: Set[VirtualHost] = set()
 
     def perform(self):
         """Perform all HTTP-01 challenges."""
@@ -93,12 +93,12 @@ class ApacheHttp01(common.ChallengePerformer):
                     self.configurator.enable_mod(mod, temp=True)
 
     def _mod_config(self):
-        selected_vhosts = []  # type: List[VirtualHost]
+        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 elgible 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

@@ -100,12 +100,9 @@ For this reason the internal representation of data should not ignore the case.
 """
 
 import abc
-import six
 
 
-
-@six.add_metaclass(abc.ABCMeta)
-class ParserNode(object):
+class ParserNode(object, metaclass=abc.ABCMeta):
     """
     ParserNode is the basic building block of the tree of such nodes,
     representing the structure of the configuration. It is largely meant to keep
@@ -204,9 +201,7 @@ class ParserNode(object):
         """
 
 
-# Linter rule exclusion done because of https://github.com/PyCQA/pylint/issues/179
-@six.add_metaclass(abc.ABCMeta)  # pylint: disable=abstract-method
-class CommentNode(ParserNode):
+class CommentNode(ParserNode, metaclass=abc.ABCMeta):
     """
     CommentNode class is used for representation of comments within the parsed
     configuration structure. Because of the nature of comments, it is not able
@@ -243,14 +238,13 @@ class CommentNode(ParserNode):
             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
 
 
-@six.add_metaclass(abc.ABCMeta)
-class DirectiveNode(ParserNode):
+class DirectiveNode(ParserNode, metaclass=abc.ABCMeta):
     """
     DirectiveNode class represents a configuration directive within the configuration.
     It can have zero or more parameters attached to it. Because of the nature of
@@ -308,7 +302,7 @@ class DirectiveNode(ParserNode):
         :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
@@ -325,8 +319,7 @@ class DirectiveNode(ParserNode):
         """
 
 
-@six.add_metaclass(abc.ABCMeta)
-class BlockNode(DirectiveNode):
+class BlockNode(DirectiveNode, metaclass=abc.ABCMeta):
     """
     BlockNode class represents a block of nested configuration directives, comments
     and other blocks as its children. A BlockNode can have zero or more parameters

certbot-apache/certbot_apache/_internal/obj.py

@@ -1,7 +1,7 @@
 """Module contains classes used by the Apache Configurator."""
 import re
+from typing import Set
 
-from acme.magic_typing import Set
 from certbot.plugins import common
 
 
@@ -20,16 +20,13 @@ class Addr(common.Addr):
                      self.is_wildcard() and other.is_wildcard()))
         return False
 
-    def __ne__(self, other):
-        return not self.__eq__(other)
-
     def __repr__(self):
         return "certbot_apache._internal.obj.Addr(" + repr(self.tup) + ")"
 
     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()."""
@@ -98,7 +95,7 @@ class Addr(common.Addr):
         return self.get_addr_obj(port)
 
 
-class VirtualHost(object):
+class VirtualHost:
     """Represents an Apache Virtualhost.
 
     :ivar str filep: file path of VH
@@ -140,7 +137,7 @@ class VirtualHost(object):
 
     def get_names(self):
         """Return a set of all names."""
-        all_names = set()  # type: Set[str]
+        all_names: Set[str] = set()
         all_names.update(self.aliases)
         # Strip out any scheme:// and <port> field from servername
         if self.name is not None:
@@ -191,9 +188,6 @@ class VirtualHost(object):
 
         return False
 
-    def __ne__(self, other):
-        return not self.__eq__(other)
-
     def __hash__(self):
         return hash((self.filep, self.path,
                      tuple(self.addrs), tuple(self.get_names()),
@@ -251,7 +245,7 @@ class VirtualHost(object):
 
         # already_found acts to keep everything very conservative.
         # Don't allow multiple ip:ports in same set.
-        already_found = set()  # type: Set[str]
+        already_found: Set[str] = set()
 
         for addr in vhost.addrs:
             for local_addr in self.addrs:

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

@@ -1,25 +1,23 @@
 """ Distribution specific override class for CentOS family (RHEL, Fedora) """
 import logging
+from typing import cast
+from typing import List
 
-import zope.interface
-
-from acme.magic_typing import List
 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",
@@ -29,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):
@@ -50,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()
@@ -68,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()
-        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
@@ -90,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()
 
@@ -102,9 +96,9 @@ class CentOSConfigurator(configurator.ApacheConfigurator):
 
         loadmods = self.parser.find_dir("LoadModule", "ssl_module", exclude=False)
 
-        correct_ifmods = []  # type: List[str]
-        loadmod_args = []  # type: List[str]
-        loadmod_paths = []  # type: List[str]
+        correct_ifmods: List[str] = []
+        loadmod_args: List[str] = []
+        loadmod_paths: List[str] = []
         for m in loadmods:
             noarg_path = m.rpartition("/")[0]
             path_args = self.parser.get_all_args(noarg_path)
@@ -118,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()):
@@ -167,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,19 +1,16 @@
 """ Distribution specific override class for Fedora 29+ """
-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",
@@ -23,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):
@@ -40,14 +31,14 @@ 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):
@@ -60,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):
         """
@@ -68,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()
-        self.options["restart_cmd"][0] = 'apachectl'
-        self.options["restart_cmd_alt"][0] = 'apachectl'
-        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):
@@ -79,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,33 +1,19 @@
 """ Distribution specific override class for Gentoo Linux """
-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):
@@ -35,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()
-        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)
 
 
@@ -50,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 """
@@ -61,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="apache2ctl",
-        version_cmd=['apache2ctl', '-v'],
-        restart_cmd=['apache2ctl', 'graceful'],
-        conftest_cmd=['apache2ctl', 'configtest'],
+        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

@@ -3,21 +3,24 @@ import copy
 import fnmatch
 import logging
 import re
-import sys
+from typing import Dict
+from typing import List
+from typing import Optional
 
-import six
-
-from acme.magic_typing import Dict
-from acme.magic_typing import List
 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__)
 
 
-class ApacheParser(object):
+class ApacheParser:
     """Class handles the fine details of parsing the Apache Configuration.
 
     .. todo:: Make parsing general... remove sites-available etc...
@@ -42,8 +45,7 @@ class ApacheParser(object):
         self.configurator = configurator
 
         # Initialize augeas
-        self.aug = None
-        self.init_augeas()
+        self.aug = init_augeas()
 
         if not self.check_aug_version():
             raise errors.NotSupportedError(
@@ -51,9 +53,9 @@ class ApacheParser(object):
                 "version 1.2.0 or higher, please make sure you have you have "
                 "those installed.")
 
-        self.modules = {}  # type: Dict[str, str]
-        self.parser_paths = {}  # type: Dict[str, List[str]]
-        self.variables = {}  # type: Dict[str, str]
+        self.modules: Dict[str, Optional[str]] = {}
+        self.parser_paths: Dict[str, List[str]] = {}
+        self.variables: Dict[str, str] = {}
 
         # Find configuration root and make sure augeas can parse it.
         self.root = os.path.abspath(root)
@@ -79,30 +81,13 @@ class ApacheParser(object):
         # 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.
 
@@ -266,7 +251,7 @@ class ApacheParser(object):
             the iteration issue.  Else... parse and enable mods at same time.
 
         """
-        mods = {}  # type: Dict[str, str]
+        mods: Dict[str, str] = {}
         matches = self.find_dir("LoadModule")
         iterator = iter(matches)
         # Make sure prev_size != cur_size for do: while: iteration
@@ -275,7 +260,7 @@ class ApacheParser(object):
         while len(mods) != prev_size:
             prev_size = len(mods)
 
-            for match_name, match_filename in six.moves.zip(
+            for match_name, match_filename in zip(
                     iterator, iterator):
                 mod_name = self.get_arg(match_name)
                 mod_filename = self.get_arg(match_filename)
@@ -297,7 +282,7 @@ class ApacheParser(object):
     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"""
@@ -307,7 +292,7 @@ class ApacheParser(object):
         # 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):
@@ -316,7 +301,7 @@ class ApacheParser(object):
     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())
 
@@ -455,7 +440,11 @@ class ApacheParser(object):
         :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):
@@ -553,7 +542,7 @@ class ApacheParser(object):
         else:
             arg_suffix = "/*[self::arg=~regexp('%s')]" % case_i(arg)
 
-        ordered_matches = []  # type: List[str]
+        ordered_matches: List[str] = []
 
         # TODO: Wildcards should be included in alphabetical order
         # https://httpd.apache.org/docs/2.4/mod/core.html#include
@@ -738,9 +727,6 @@ class ApacheParser(object):
         :rtype: str
 
         """
-        if sys.version_info < (3, 6):
-            # This strips off final /Z(?ms)
-            return fnmatch.translate(clean_fn_match)[:-7]  # pragma: no cover
         # Since Python 3.6, it returns a different pattern like (?s:.*\.load)\Z
         return fnmatch.translate(clean_fn_match)[4:-3]  # pragma: no cover
 
@@ -955,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,32 +1,18 @@
-from distutils.version import LooseVersion
-import sys
-
-from setuptools import __version__ as setuptools_version
 from setuptools import find_packages
 from setuptools import setup
 
-version = '1.11.0.dev0'
+version = '1.19.0.dev0'
 
-# 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',
-    'zope.component',
-    'zope.interface',
+    'setuptools>=39.0.1',
 ]
 
-setuptools_known_environment_markers = (LooseVersion(setuptools_version) >= LooseVersion('36.2'))
-if setuptools_known_environment_markers:
-    install_requires.append('mock ; python_version < "3.3"')
-elif 'bdist_wheel' in sys.argv[1:]:
-    raise RuntimeError('Error, you are trying to build certbot wheels using an old version '
-                       'of setuptools. Version 36.2+ of setuptools is required.')
-elif sys.version_info < (3,3):
-    install_requires.append('mock')
-
 dev_extras = [
     'apacheconfig>=0.3.2',
 ]
@@ -37,9 +23,9 @@ 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='>=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*, !=3.5.*',
+    python_requires='>=3.6',
     classifiers=[
         'Development Status :: 5 - Production/Stable',
         'Environment :: Plugins',
@@ -47,8 +33,6 @@ setup(
         'License :: OSI Approved :: Apache Software License',
         'Operating System :: POSIX :: Linux',
         'Programming Language :: Python',
-        'Programming Language :: Python :: 2',
-        'Programming Language :: Python :: 2.7',
         'Programming Language :: Python :: 3',
         'Programming Language :: Python :: 3.6',
         'Programming Language :: Python :: 3.7',

certbot-apache/tests/augeasnode_test.py

@@ -1,4 +1,6 @@
 """Tests for AugeasParserNode classes"""
+from typing import List
+
 try:
     import mock
 except ImportError: # pragma: no cover
@@ -27,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(
@@ -107,7 +109,7 @@ class AugeasParserNodeTest(util.ApacheTest):  # pylint: disable=too-many-public-
 
     def test_set_parameters(self):
         servernames = self.config.parser_root.find_directives("servername")
-        names = []  # type: List[str]
+        names: List[str] = []
         for servername in servernames:
             names += servername.parameters
         self.assertFalse("going_to_set_this" in names)

certbot-apache/tests/autohsts_test.py

@@ -7,7 +7,6 @@ try:
     import mock
 except ImportError: # pragma: no cover
     from unittest import mock # type: ignore
-import six  # pylint: disable=unused-import  # six is used in mock.patch()
 
 from certbot import errors
 from certbot_apache._internal import constants
@@ -19,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)
@@ -147,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")
@@ -180,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

@@ -10,7 +10,6 @@ try:
     import mock
 except ImportError: # pragma: no cover
     from unittest import mock # type: ignore
-import six  # pylint: disable=unused-import  # six is used in mock.patch()
 
 from acme import challenges
 from certbot import achallenges
@@ -31,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)
@@ -104,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
 
@@ -129,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)
@@ -147,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]
@@ -339,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
@@ -377,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
@@ -726,7 +726,7 @@ class MultipleVhostsTest(util.ApacheTest):
         # This calls open
         self.config.reverter.register_file_creation = mock.Mock()
         mock_open.side_effect = IOError
-        with mock.patch("six.moves.builtins.open", mock_open):
+        with mock.patch("builtins.open", mock_open):
             self.assertRaises(
                 errors.PluginError,
                 self.config.make_vhost_ssl, self.vh_truth[0])
@@ -893,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
@@ -1292,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
@@ -1388,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]]
@@ -1478,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,
@@ -1557,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,
@@ -1608,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])
@@ -1625,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])
@@ -1654,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)
@@ -1775,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
@@ -1811,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)
@@ -1834,7 +1836,7 @@ class InstallSslOptionsConfTest(util.ApacheTest):
 
     def test_open_module_file(self):
         mock_open = mock.mock_open(read_data="testing 12 3")
-        with mock.patch("six.moves.builtins.open", mock_open):
+        with mock.patch("builtins.open", mock_open):
             self.assertEqual(self.config._open_module_file("/nonsense/"), "testing 12 3")
 
 if __name__ == "__main__":

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/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

@@ -1,6 +1,7 @@
 """Test for certbot_apache._internal.http_01."""
 import unittest
 import errno
+from typing import List
 
 try:
     import mock
@@ -23,10 +24,10 @@ 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 = []  # type: List[achallenges.KeyAuthorizationAnnotatedChallenge]
+        self.achalls: List[achallenges.KeyAuthorizationAnnotatedChallenge] = []
         vh_truth = util.get_vh_truth(
             self.temp_dir, "debian_apache_2_4/multiple_vhosts")
         # Takes the vhosts for encryption-example.demo, certbot.demo
@@ -124,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/assets/hook.py

@@ -1,5 +1,4 @@
 #!/usr/bin/env python
-from __future__ import print_function
 import os
 import sys
 

certbot-ci/certbot_integration_tests/certbot_tests/context.py

@@ -7,14 +7,14 @@ import tempfile
 from certbot_integration_tests.utils import certbot_call
 
 
-class IntegrationTestsContext(object):
+class IntegrationTestsContext:
     """General fixture describing a certbot integration tests context"""
     def __init__(self, request):
         self.request = request
 
-        if hasattr(request.config, 'slaveinput'):  # Worker node
-            self.worker_id = request.config.slaveinput['slaveid']
-            acme_xdist = request.config.slaveinput['acme_xdist']
+        if hasattr(request.config, 'workerinput'):  # Worker node
+            self.worker_id = request.config.workerinput['workerid']
+            acme_xdist = request.config.workerinput['acme_xdist']
         else:  # Primary node
             self.worker_id = 'primary'
             acme_xdist = request.config.acme_xdist
@@ -61,7 +61,7 @@ class IntegrationTestsContext(object):
         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

@@ -1,5 +1,4 @@
 """Module executing integration tests against certbot core."""
-from __future__ import print_function
 
 import os
 from os.path import exists
@@ -9,19 +8,20 @@ import shutil
 import subprocess
 import time
 
-from cryptography.hazmat.primitives.asymmetric.ec import SECP256R1, SECP384R1
+from cryptography.hazmat.primitives.asymmetric.ec import SECP256R1
+from cryptography.hazmat.primitives.asymmetric.ec import SECP384R1
+from cryptography.hazmat.primitives.asymmetric.ec import SECP521R1
 from cryptography.x509 import NameOID
-
 import pytest
 
 from certbot_integration_tests.certbot_tests import context as certbot_context
 from certbot_integration_tests.certbot_tests.assertions import assert_cert_count_for_lineage
 from certbot_integration_tests.certbot_tests.assertions import assert_elliptic_key
-from certbot_integration_tests.certbot_tests.assertions import assert_rsa_key
 from certbot_integration_tests.certbot_tests.assertions import assert_equals_group_owner
 from certbot_integration_tests.certbot_tests.assertions import assert_equals_group_permissions
 from certbot_integration_tests.certbot_tests.assertions import assert_equals_world_read_permissions
 from certbot_integration_tests.certbot_tests.assertions import assert_hook_execution
+from certbot_integration_tests.certbot_tests.assertions import assert_rsa_key
 from certbot_integration_tests.certbot_tests.assertions import assert_saved_renew_hook
 from certbot_integration_tests.certbot_tests.assertions import assert_world_no_permissions
 from certbot_integration_tests.certbot_tests.assertions import assert_world_read_permissions
@@ -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):
@@ -148,6 +148,17 @@ def test_certonly(context):
     """Test the certonly verb on certbot."""
     context.certbot(['certonly', '--cert-name', 'newname', '-d', context.get_domain('newname')])
 
+    assert_cert_count_for_lineage(context.config_dir, 'newname', 1)
+
+
+def test_certonly_webroot(context):
+    """Test the certonly verb with webroot plugin"""
+    with misc.create_http_server(context.http_01_port) as webroot:
+        certname = context.get_domain('webroot')
+        context.certbot(['certonly', '-a', 'webroot', '--webroot-path', webroot, '-d', certname])
+
+    assert_cert_count_for_lineage(context.config_dir, certname, 1)
+
 
 def test_auth_and_install_with_csr(context):
     """Test certificate issuance and install using an existing CSR."""
@@ -335,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)
@@ -357,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),
@@ -376,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')
@@ -396,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):
@@ -476,6 +490,28 @@ def test_default_curve_type(context):
     assert_elliptic_key(key1, SECP256R1)
 
 
+@pytest.mark.parametrize('curve,curve_cls,skip_servers', [
+    # Curve name, Curve class, ACME servers to skip
+    ('secp256r1', SECP256R1, []),
+    ('secp384r1', SECP384R1, []),
+    ('secp521r1', SECP521R1, ['boulder-v1', 'boulder-v2'])]
+)
+def test_ecdsa_curves(context, curve, curve_cls, skip_servers):
+    """Test issuance for each supported ECDSA curve"""
+    if context.acme_server in skip_servers:
+        pytest.skip('ACME server {} does not support ECDSA curve {}'
+                    .format(context.acme_server, curve))
+
+    domain = context.get_domain('curve')
+    context.certbot([
+        'certonly',
+        '--key-type', 'ecdsa', '--elliptic-curve', curve,
+        '--force-renewal', '-d', domain,
+    ])
+    key = join(context.config_dir, "live", domain, 'privkey.pem')
+    assert_elliptic_key(key, curve_cls)
+
+
 def test_renew_with_ec_keys(context):
     """Test proper renew with updated private key complexity."""
     certname = context.get_domain('renew')
@@ -581,11 +617,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):
@@ -597,7 +633,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):
@@ -652,12 +688,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):
@@ -666,20 +702,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/conftest.py

@@ -6,7 +6,6 @@ for a directory a specific configuration using built-in pytest hooks.
 
 See https://docs.pytest.org/en/latest/reference.html#hook-reference
 """
-from __future__ import print_function
 import contextlib
 import subprocess
 import sys
@@ -35,7 +34,7 @@ def pytest_configure(config):
     Standard pytest hook used to add a configuration logic for each node of a pytest run.
     :param config: the current pytest configuration
     """
-    if not hasattr(config, 'slaveinput'):  # If true, this is the primary node
+    if not hasattr(config, 'workerinput'):  # If true, this is the primary node
         with _print_on_err():
             _setup_primary_node(config)
 
@@ -45,8 +44,8 @@ def pytest_configure_node(node):
     Standard pytest-xdist hook used to configure a worker node.
     :param node: current worker node
     """
-    node.slaveinput['acme_xdist'] = node.config.acme_xdist
-    node.slaveinput['dns_xdist'] = node.config.dns_xdist
+    node.workerinput['acme_xdist'] = node.config.acme_xdist
+    node.workerinput['dns_xdist'] = node.config.dns_xdist
 
 
 @contextlib.contextmanager

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/nginx_tests/test_main.py

@@ -1,8 +1,8 @@
 """Module executing integration tests against certbot with nginx plugin."""
 import os
 import ssl
-
 from typing import List
+
 import pytest
 
 from certbot_integration_tests.nginx_tests import context as nginx_context
@@ -32,8 +32,8 @@ def test_context(request):
         '--preferred-challenges', 'http'
     ], {'default_server': False}),
 ], indirect=['context'])
-def test_certificate_deployment(certname_pattern, params, context):
-    # type: (str, List[str], nginx_context.IntegrationTestsContext) -> None
+def test_certificate_deployment(certname_pattern: str, params: List[str],
+                                context: nginx_context.IntegrationTestsContext) -> None:
     """
     Test various scenarios to deploy a certificate to nginx using certbot.
     """

certbot-ci/certbot_integration_tests/rfc2136_tests/context.py

@@ -1,7 +1,7 @@
 """Module to handle the context of RFC2136 integration tests."""
 
-import tempfile
 from contextlib import contextmanager
+import tempfile
 
 from pkg_resources import resource_filename
 from pytest import skip
@@ -13,13 +13,12 @@ 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, 'slaveinput'):  # Worker node
-            self._dns_xdist = request.config.slaveinput['dns_xdist']
+        if hasattr(request.config, 'workerinput'):  # Worker node
+            self._dns_xdist = request.config.workerinput['dns_xdist']
         else:  # Primary node
             self._dns_xdist = request.config.dns_xdist
 
@@ -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

@@ -1,6 +1,5 @@
 #!/usr/bin/env python
 """Module to setup an ACME CA server environment able to run multiple tests in parallel"""
-from __future__ import print_function
 
 import argparse
 import errno
@@ -12,18 +11,18 @@ import subprocess
 import sys
 import tempfile
 import time
-
 from typing import List
+
 import requests
 
+# pylint: disable=wildcard-import,unused-wildcard-import
 from certbot_integration_tests.utils import misc
 from certbot_integration_tests.utils import pebble_artifacts
 from certbot_integration_tests.utils import proxy
-# pylint: disable=wildcard-import,unused-wildcard-import
 from certbot_integration_tests.utils.constants import *
 
 
-class ACMEServer(object):
+class ACMEServer:
     """
     ACMEServer configures and handles the lifecycle of an ACME CA server and an HTTP reverse proxy
     instance, to allow parallel execution of integration tests against the unique http-01 port
@@ -52,8 +51,8 @@ class ACMEServer(object):
         self._acme_type = 'pebble' if acme_server == 'pebble' else 'boulder'
         self._proxy = http_proxy
         self._workspace = tempfile.mkdtemp()
-        self._processes = []  # type: List[subprocess.Popen]
-        self._stdout = sys.stdout if stdout else open(os.devnull, 'w')
+        self._processes: List[subprocess.Popen] = []
+        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:
@@ -241,6 +240,7 @@ class ACMEServer(object):
         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

@@ -1,11 +1,10 @@
 #!/usr/bin/env python
 """Module to call certbot in test mode"""
-from __future__ import absolute_import
 
+from distutils.version import LooseVersion
 import os
 import subprocess
 import sys
-from distutils.version import LooseVersion
 
 import certbot_integration_tests
 # pylint: disable=wildcard-import,unused-wildcard-import
@@ -18,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
@@ -26,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

@@ -1,7 +1,5 @@
 #!/usr/bin/env python
 """Module to setup an RFC2136-capable DNS server"""
-from __future__ import print_function
-
 import os
 import os.path
 import shutil
@@ -10,6 +8,7 @@ import subprocess
 import sys
 import tempfile
 import time
+from typing import Optional
 
 from pkg_resources import resource_filename
 
@@ -21,7 +20,7 @@ BIND_BIND_ADDRESS = ("127.0.0.1", 45953)
 BIND_TEST_QUERY = bytearray.fromhex("0011cb37000000010000000000000000010003")
 
 
-class DNSServer(object):
+class DNSServer:
     """
     DNSServer configures and handles the lifetime of an RFC2136-capable server.
     DNServer provides access to the dns_xdist parameter, listing the address and port
@@ -40,12 +39,13 @@ class DNSServer(object):
 
         self.bind_root = tempfile.mkdtemp()
 
-        self.process = None  # type: subprocess.Popen
+        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):
@@ -84,6 +84,7 @@ class DNSServer(object):
     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",
@@ -113,8 +114,7 @@ class DNSServer(object):
             self.stop()
             raise
 
-    def _wait_until_ready(self, attempts=30):
-        # type: (int) -> None
+    def _wait_until_ready(self, attempts: int = 30) -> None:
         """
         Polls the DNS server over TCP until it gets a response, or until
         it runs out of attempts and raises a ValueError.
@@ -122,6 +122,9 @@ class DNSServer(object):
         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

@@ -4,10 +4,12 @@ or outside during setup/teardown of the integration tests environment.
 """
 import contextlib
 import errno
+import http.server as SimpleHTTPServer
 import multiprocessing
 import os
 import re
 import shutil
+import socketserver
 import stat
 import sys
 import tempfile
@@ -23,11 +25,9 @@ from cryptography.x509 import load_pem_x509_certificate
 from OpenSSL import crypto
 import pkg_resources
 import requests
-from six.moves import SimpleHTTPServer
-from six.moves import socketserver
 
-from certbot_integration_tests.utils.constants import \
-     PEBBLE_ALTERNATE_ROOTS, PEBBLE_MANAGEMENT_URL
+from certbot_integration_tests.utils.constants import PEBBLE_ALTERNATE_ROOTS
+from certbot_integration_tests.utils.constants import PEBBLE_MANAGEMENT_URL
 
 RSA_KEY_TYPE = 'rsa'
 ECDSA_KEY_TYPE = 'ecdsa'
@@ -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))
 
@@ -311,7 +316,7 @@ def echo(keyword, path=None):
     if not re.match(r'^\w+$', keyword):
         raise ValueError('Error, keyword `{0}` is not a single keyword.'
                          .format(keyword))
-    return '{0} -c "from __future__ import print_function; print(\'{1}\')"{2}'.format(
+    return '{0} -c "print(\'{1}\')"{2}'.format(
         os.path.basename(sys.executable), keyword, ' >> "{0}"'.format(path) if path else '')
 
 

certbot-ci/certbot_integration_tests/utils/pebble_artifacts.py

@@ -7,7 +7,8 @@ import stat
 import pkg_resources
 import requests
 
-from certbot_integration_tests.utils.constants import DEFAULT_HTTP_01_PORT, MOCK_OCSP_SERVER_PORT
+from certbot_integration_tests.utils.constants import DEFAULT_HTTP_01_PORT
+from certbot_integration_tests.utils.constants import MOCK_OCSP_SERVER_PORT
 
 PEBBLE_VERSION = 'v2.3.0'
 ASSETS_PATH = pkg_resources.resource_filename('certbot_integration_tests', 'assets')

certbot-ci/certbot_integration_tests/utils/pebble_ocsp_server.py

@@ -4,6 +4,7 @@ This runnable module interfaces itself with the Pebble management interface in o
 to serve a mock OCSP responder during integration tests against Pebble.
 """
 import datetime
+import http.server as BaseHTTPServer
 import re
 
 from cryptography import x509
@@ -13,7 +14,6 @@ from cryptography.hazmat.primitives import serialization
 from cryptography.x509 import ocsp
 from dateutil import parser
 import requests
-from six.moves import BaseHTTPServer
 
 from certbot_integration_tests.utils.constants import MOCK_OCSP_SERVER_PORT
 from certbot_integration_tests.utils.constants import PEBBLE_MANAGEMENT_URL
@@ -29,10 +29,7 @@ class _ProxyHandler(BaseHTTPServer.BaseHTTPRequestHandler):
         request = requests.get(PEBBLE_MANAGEMENT_URL + '/intermediates/0', verify=False)
         issuer_cert = x509.load_pem_x509_certificate(request.content, default_backend())
 
-        try:
-            content_len = int(self.headers.getheader('content-length', 0))
-        except AttributeError:
-            content_len = int(self.headers.get('Content-Length'))
+        content_len = int(self.headers.get('Content-Length'))
 
         ocsp_request = ocsp.load_der_ocsp_request(self.rfile.read(content_len))
         response = requests.get('{0}/cert-status-by-serial/{1}'.format(

certbot-ci/certbot_integration_tests/utils/proxy.py

@@ -1,12 +1,12 @@
 #!/usr/bin/env python
 # pylint: disable=missing-module-docstring
 
+import http.server as BaseHTTPServer
 import json
 import re
 import sys
 
 import requests
-from six.moves import BaseHTTPServer
 
 from certbot_integration_tests.utils.misc import GracefulTCPServer