From c26d459d0fc300d76adf6f0a7c7549d9e4d8cc72 Mon Sep 17 00:00:00 2001
From: Adrien Ferrand <adferrand@users.noreply.github.com>
Date: Tue, 12 Nov 2019 22:52:44 +0100
Subject: [PATCH 1/7] Remove python2 and certbot-auto references in how to set
 up a Certbot build environment. (#7549)

Fixes #7548.

This PR udpdates installation instructions to get rid of python2 and certbot-auto in the how-to explaining the Certbot development environment setup.

Instead, Python 3 is used, and appropriate instructions for APT and RPM based distributions are provided.
---
 docs/contributing.rst | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/docs/contributing.rst b/docs/contributing.rst
index cc0a74b43..2a98658e4 100644
--- a/docs/contributing.rst
+++ b/docs/contributing.rst
@@ -36,29 +36,36 @@ run Certbot in Docker. You can find instructions for how to do this :ref:`here
 install dependencies and set up a virtual environment where you can run
 Certbot.
 
+Install the OS system dependencies required to run Certbot.
+
+.. code-block:: shell
+
+   # For APT-based distributions (e.g. Debian, Ubuntu ...)
+   sudo apt update
+   sudo apt install python3-dev python3-venv gcc libaugeas0 libssl-dev \
+                    libffi-dev ca-certificates openssl
+   # For RPM-based distributions (e.g. Fedora, CentOS ...)
+   # NB1: old distributions will use yum instead of dnf
+   # NB2: RHEL-based distributions use python3X-devel instead of python3-devel (e.g. python36-devel)
+   sudo dnf install python3-devel gcc augeas-libs openssl-devel libffi-devel \
+                    redhat-rpm-config ca-certificates openssl
+
+Set up the Python virtual environment that will host your Certbot local instance.
+
 .. code-block:: shell
 
    cd certbot
-   ./certbot-auto --debug --os-packages-only
-   python tools/venv.py
-
-If you have Python3 available and want to use it, run the ``venv3.py`` script.
-
-.. code-block:: shell
-
    python tools/venv3.py
 
 .. note:: You may need to repeat this when
   Certbot's dependencies change or when a new plugin is introduced.
 
 You can now run the copy of Certbot from git either by executing
-``venv/bin/certbot``, or by activating the virtual environment. You can do the
+``venv3/bin/certbot``, or by activating the virtual environment. You can do the
 latter by running:
 
 .. code-block:: shell
 
-   source venv/bin/activate
-   # or
    source venv3/bin/activate
 
 After running this command, ``certbot`` and development tools like ``ipdb``,

From 75acdeb6454429d6a1704a10f3bfe649a074b227 Mon Sep 17 00:00:00 2001
From: Adrien Ferrand <adferrand@users.noreply.github.com>
Date: Wed, 13 Nov 2019 18:43:50 +0100
Subject: [PATCH 2/7] [Windows] Fix certbot renew task failure under NT
 AUTHORITY\SYSTEM account (#7536)

Turned out that the scheduled task that runs `certbot renew` twice a day, is failing. Without any kind of log of course, otherwise it would not be fun.

It can be revealed by opening a powershell under the `NT AUTHORITY\SYSTEM` account, under which the scheduled task is run. Under theses circumstances, the bug is revealed: Certbot breaks when trying to invoke `certbot.compat.filesystem._get_current_user()`. Indeed the logic there implied to call `win32api.GetUserNameEx(win32api.NameSamCompatible)` and this function does not return always a useful value.

For normal account, it will be typically `DOMAIN_OR_MACHINE_NAME\YOUR_USER_NAME` (e.g. `My Machine\Adrien Ferrand`). But for the account `NT AUTHORITY\SYSTEM`, it will return `MACHINE_NAME\DOMAIN$`, which is a nonsense and makes fail the resolution of the actual SID of the account at the end of `_get_current_user()`.

This PR fixes this behavior by using an explicit construction of the account name that works both for normal users and `SYSTEM`.

* Use a different way to resolve current user account, that works both for normal users and SYSTEM.

* Add a comment to run Certbot under NT AUTHORITY\SYSTEM
---
 certbot/compat/filesystem.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/certbot/compat/filesystem.py b/certbot/compat/filesystem.py
index 69a3a63c5..5fba440cc 100644
--- a/certbot/compat/filesystem.py
+++ b/certbot/compat/filesystem.py
@@ -588,7 +588,11 @@ def _get_current_user():
     """
     Return the pySID corresponding to the current user.
     """
-    account_name = win32api.GetUserNameEx(win32api.NameSamCompatible)
+    # We craft the account_name ourselves instead of calling for instance win32api.GetUserNameEx,
+    # because this function returns nonsense values when Certbot is run under NT AUTHORITY\SYSTEM.
+    # To run Certbot under NT AUTHORITY\SYSTEM, you can open a shell using the instructions here:
+    # https://blogs.technet.microsoft.com/ben_parker/2010/10/27/how-do-i-run-powershell-execommand-prompt-as-the-localsystem-account-on-windows-7/
+    account_name = r"{0}\{1}".format(win32api.GetDomainName(), win32api.GetUserName())
     # LookupAccountName() expects the system name as first parameter. By passing None to it,
     # we instruct Windows to first search the matching account in the machine local accounts,
     # then into the primary domain accounts, if the machine has joined a domain, then finally

From 595b1b212ef83c45173f141c137a45cf0a469a52 Mon Sep 17 00:00:00 2001
From: Adrien Ferrand <adferrand@users.noreply.github.com>
Date: Wed, 13 Nov 2019 19:04:45 +0100
Subject: [PATCH 3/7] [Windows] Avoid letsencrypt.log permissions error during
 scheduled certbot renew task (#7537)

While coding for #7536, I ran into another issue. It appears that Certbot logs generated during the scheduled task execution have wrong permissions that make them almost unusable: they do not have an owner, and their ACL contains nonsense values (non existant accounts name).

The class `logging.handler.RotatingFileHandler` is responsible for these logs, and become mad when it is in a Python process run under a scheduled task owned by `SYSTEM`. This is precisely our case here.

This PR avoids (but not fix) the issue, by changing the owner of the scheduled task from `SYSTEM` to the `Administrators` group, that appears to work fine.

* Use Administrators group instead of SYSTEM to run the certbot renew task
---
 windows-installer/renew-up.ps1 | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/windows-installer/renew-up.ps1 b/windows-installer/renew-up.ps1
index c6a5fd9ea..224458748 100644
--- a/windows-installer/renew-up.ps1
+++ b/windows-installer/renew-up.ps1
@@ -8,8 +8,10 @@ $action = New-ScheduledTaskAction -Execute 'Powershell.exe' -Argument '-NoProfil
 $delay = New-TimeSpan -Hours 12
 $triggerAM = New-ScheduledTaskTrigger -Daily -At 12am -RandomDelay $delay
 $triggerPM = New-ScheduledTaskTrigger -Daily -At 12pm -RandomDelay $delay
-# NB: For now scheduled task is set up under SYSTEM account because Certbot Installer installs Certbot for all users.
+# NB: For now scheduled task is set up under Administrators group account because Certbot Installer installs Certbot for all users.
 # If in the future we allow the Installer to install Certbot for one specific user, the scheduled task will need to
 # switch to this user, since Certbot will be available only for him.
-$principal = New-ScheduledTaskPrincipal -UserId SYSTEM -LogonType ServiceAccount -RunLevel Highest
+$adminsSID = New-Object System.Security.Principal.SecurityIdentifier("S-1-5-32-544")
+$adminsGroupID = $adminsSID.Translate([System.Security.Principal.NTAccount]).Value
+$principal = New-ScheduledTaskPrincipal -GroupId $adminsGroupID -RunLevel Highest
 Register-ScheduledTask -Action $action -Trigger $triggerAM,$triggerPM -TaskName $taskName -Description "Execute twice a day the 'certbot renew' command, to renew managed certificates if needed." -Principal $principal

From 46d5f7a8601d026e1a986dd3f7a1dc4b056ca886 Mon Sep 17 00:00:00 2001
From: ohemorange <ebportnoy@gmail.com>
Date: Wed, 13 Nov 2019 10:19:27 -0800
Subject: [PATCH 4/7] Move configuration.py to _internal (#7542)

Part of #5775. Methodology similar to #7528. Also refactors NGINX test util to use certbot.tests.util.ConfigTestCase.

* refactor nginx tests to no longer rely on certbot.configuration internals

* Move configuration.py to _internal
---
 .../configurators/apache/common.py            |  2 +-
 .../configurators/nginx/common.py             |  2 +-
 .../certbot_nginx/tests/configurator_test.py  |  4 +-
 .../certbot_nginx/tests/http_01_test.py       |  2 +-
 certbot-nginx/certbot_nginx/tests/util.py     | 80 +++++++++----------
 certbot/_internal/cert_manager.py             |  8 +-
 certbot/{ => _internal}/configuration.py      |  0
 certbot/_internal/main.py                     |  2 +-
 certbot/tests/cert_manager_test.py            |  2 +-
 certbot/tests/configuration_test.py           | 16 ++--
 certbot/tests/main_test.py                    |  2 +-
 certbot/tests/renewal_test.py                 |  2 +-
 certbot/tests/util.py                         |  2 +-
 docs/api/configuration.rst                    |  5 --
 14 files changed, 62 insertions(+), 67 deletions(-)
 rename certbot/{ => _internal}/configuration.py (100%)
 delete mode 100644 docs/api/configuration.rst

diff --git a/certbot-compatibility-test/certbot_compatibility_test/configurators/apache/common.py b/certbot-compatibility-test/certbot_compatibility_test/configurators/apache/common.py
index 82195264b..050a3bdb3 100644
--- a/certbot-compatibility-test/certbot_compatibility_test/configurators/apache/common.py
+++ b/certbot-compatibility-test/certbot_compatibility_test/configurators/apache/common.py
@@ -6,7 +6,7 @@ import subprocess
 import mock
 import zope.interface
 
-from certbot import configuration
+from certbot._internal import configuration
 from certbot import errors as le_errors
 from certbot import util as certbot_util
 from certbot_apache import entrypoint
diff --git a/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py b/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py
index 3207cf88a..39f69da19 100644
--- a/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py
+++ b/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py
@@ -7,7 +7,7 @@ import zope.interface
 
 from acme.magic_typing import Set  # pylint: disable=unused-import, no-name-in-module
 
-from certbot import configuration
+from certbot._internal import configuration
 from certbot_nginx import configurator
 from certbot_nginx import constants
 from certbot_compatibility_test import errors
diff --git a/certbot-nginx/certbot_nginx/tests/configurator_test.py b/certbot-nginx/certbot_nginx/tests/configurator_test.py
index 237e22d8f..4a7f7cafb 100644
--- a/certbot-nginx/certbot_nginx/tests/configurator_test.py
+++ b/certbot-nginx/certbot_nginx/tests/configurator_test.py
@@ -27,7 +27,7 @@ class NginxConfiguratorTest(util.NginxTest):
     def setUp(self):
         super(NginxConfiguratorTest, self).setUp()
 
-        self.config = util.get_nginx_configurator(
+        self.config = self.get_nginx_configurator(
             self.config_path, self.config_dir, self.work_dir, self.logs_dir)
 
     @mock.patch("certbot_nginx.configurator.util.exe_exists")
@@ -935,7 +935,7 @@ class InstallSslOptionsConfTest(util.NginxTest):
     def setUp(self):
         super(InstallSslOptionsConfTest, self).setUp()
 
-        self.config = util.get_nginx_configurator(
+        self.config = self.get_nginx_configurator(
             self.config_path, self.config_dir, self.work_dir, self.logs_dir)
 
     def _call(self):
diff --git a/certbot-nginx/certbot_nginx/tests/http_01_test.py b/certbot-nginx/certbot_nginx/tests/http_01_test.py
index c6d35b808..d05370c68 100644
--- a/certbot-nginx/certbot_nginx/tests/http_01_test.py
+++ b/certbot-nginx/certbot_nginx/tests/http_01_test.py
@@ -47,7 +47,7 @@ class HttpPerformTest(util.NginxTest):
     def setUp(self):
         super(HttpPerformTest, self).setUp()
 
-        config = util.get_nginx_configurator(
+        config = self.get_nginx_configurator(
             self.config_path, self.config_dir, self.work_dir, self.logs_dir)
 
         from certbot_nginx import http_01
diff --git a/certbot-nginx/certbot_nginx/tests/util.py b/certbot-nginx/certbot_nginx/tests/util.py
index c0a70368e..c1d7e9d34 100644
--- a/certbot-nginx/certbot_nginx/tests/util.py
+++ b/certbot-nginx/certbot_nginx/tests/util.py
@@ -2,14 +2,12 @@
 import copy
 import shutil
 import tempfile
-import unittest
 
 import josepy as jose
 import mock
 import pkg_resources
 import zope.component
 
-from certbot import configuration
 from certbot import util
 from certbot.compat import os
 from certbot.plugins import common
@@ -19,11 +17,14 @@ from certbot_nginx import configurator
 from certbot_nginx import nginxparser
 
 
-class NginxTest(unittest.TestCase):  # pylint: disable=too-few-public-methods
+class NginxTest(test_util.ConfigTestCase):  # pylint: disable=too-few-public-methods
 
     def setUp(self):
         super(NginxTest, self).setUp()
 
+        self.configuration = self.config
+        self.config = None
+
         self.temp_dir, self.config_dir, self.work_dir = common.dir_setup(
             "etc_nginx", "certbot_nginx.tests")
         self.logs_dir = tempfile.mkdtemp('logs')
@@ -45,6 +46,42 @@ class NginxTest(unittest.TestCase):  # pylint: disable=too-few-public-methods
         shutil.rmtree(self.work_dir)
         shutil.rmtree(self.logs_dir)
 
+    # pylint: disable=too-many-arguments
+    def get_nginx_configurator(self, config_path, config_dir, work_dir, logs_dir,
+            version=(1, 6, 2), openssl_version="1.0.2g"):
+        """Create an Nginx Configurator with the specified options."""
+
+        backups = os.path.join(work_dir, "backups")
+
+        self.configuration.nginx_server_root = config_path
+        self.configuration.le_vhost_ext = "-le-ssl.conf"
+        self.configuration.config_dir = config_dir
+        self.configuration.work_dir = work_dir
+        self.configuration.logs_dir = logs_dir
+        self.configuration.backup_dir = backups
+        self.configuration.temp_checkpoint_dir = os.path.join(work_dir, "temp_checkpoints")
+        self.configuration.in_progress_dir = os.path.join(backups, "IN_PROGRESS")
+        self.configuration.server = "https://acme-server.org:443/new"
+        self.configuration.http01_port = 80
+        self.configuration.https_port = 5001
+
+        with mock.patch("certbot_nginx.configurator.NginxConfigurator."
+                        "config_test"):
+            with mock.patch("certbot_nginx.configurator.util."
+                            "exe_exists") as mock_exe_exists:
+                mock_exe_exists.return_value = True
+                config = configurator.NginxConfigurator(
+                    self.configuration,
+                    name="nginx",
+                    version=version,
+                    openssl_version=openssl_version)
+                config.prepare()
+
+        # Provide general config utility.
+        zope.component.provideUtility(self.configuration)
+
+        return config
+
 
 def get_data_filename(filename):
     """Gets the filename of a test data file."""
@@ -53,43 +90,6 @@ def get_data_filename(filename):
             "testdata", "etc_nginx", filename))
 
 
-def get_nginx_configurator(
-        config_path, config_dir, work_dir, logs_dir, version=(1, 6, 2), openssl_version="1.0.2g"):
-    """Create an Nginx Configurator with the specified options."""
-
-    backups = os.path.join(work_dir, "backups")
-
-    with mock.patch("certbot_nginx.configurator.NginxConfigurator."
-                    "config_test"):
-        with mock.patch("certbot_nginx.configurator.util."
-                        "exe_exists") as mock_exe_exists:
-            mock_exe_exists.return_value = True
-            config = configurator.NginxConfigurator(
-                config=mock.MagicMock(
-                    nginx_server_root=config_path,
-                    le_vhost_ext="-le-ssl.conf",
-                    config_dir=config_dir,
-                    work_dir=work_dir,
-                    logs_dir=logs_dir,
-                    backup_dir=backups,
-                    temp_checkpoint_dir=os.path.join(work_dir, "temp_checkpoints"),
-                    in_progress_dir=os.path.join(backups, "IN_PROGRESS"),
-                    server="https://acme-server.org:443/new",
-                    http01_port=80,
-                    https_port=5001,
-                ),
-                name="nginx",
-                version=version,
-                openssl_version=openssl_version)
-            config.prepare()
-
-    # Provide general config utility.
-    nsconfig = configuration.NamespaceConfig(config.config)
-    zope.component.provideUtility(nsconfig)
-
-    return config
-
-
 def filter_comments(tree):
     """Filter comment nodes from parsed configurations."""
 
diff --git a/certbot/_internal/cert_manager.py b/certbot/_internal/cert_manager.py
index ed72b12e5..329b6cdff 100644
--- a/certbot/_internal/cert_manager.py
+++ b/certbot/_internal/cert_manager.py
@@ -33,7 +33,7 @@ def update_live_symlinks(config):
     .. note:: This assumes that the installation is using a Reverter object.
 
     :param config: Configuration.
-    :type config: :class:`certbot.configuration.NamespaceConfig`
+    :type config: :class:`certbot._internal.configuration.NamespaceConfig`
 
     """
     for renewal_file in storage.renewal_conf_files(config):
@@ -43,7 +43,7 @@ def rename_lineage(config):
     """Rename the specified lineage to the new name.
 
     :param config: Configuration.
-    :type config: :class:`certbot.configuration.NamespaceConfig`
+    :type config: :class:`certbot._internal.configuration.NamespaceConfig`
 
     """
     disp = zope.component.getUtility(interfaces.IDisplay)
@@ -70,7 +70,7 @@ def certificates(config):
     """Display information about certs configured with Certbot
 
     :param config: Configuration.
-    :type config: :class:`certbot.configuration.NamespaceConfig`
+    :type config: :class:`certbot._internal.configuration.NamespaceConfig`
     """
     parsed_certs = []
     parse_failures = []
@@ -136,7 +136,7 @@ def find_duplicative_certs(config, domains):
     undefined.
 
     :param config: Configuration.
-    :type config: :class:`certbot.configuration.NamespaceConfig`
+    :type config: :class:`certbot._internal.configuration.NamespaceConfig`
     :param domains: List of domain names
     :type domains: `list` of `str`
 
diff --git a/certbot/configuration.py b/certbot/_internal/configuration.py
similarity index 100%
rename from certbot/configuration.py
rename to certbot/_internal/configuration.py
diff --git a/certbot/_internal/main.py b/certbot/_internal/main.py
index dd5f7fe4a..8daca6514 100644
--- a/certbot/_internal/main.py
+++ b/certbot/_internal/main.py
@@ -18,7 +18,7 @@ from certbot._internal import account
 from certbot._internal import cert_manager
 from certbot import cli
 from certbot._internal import client
-from certbot import configuration
+from certbot._internal import configuration
 from certbot._internal import constants
 from certbot import crypto_util
 from certbot._internal import eff
diff --git a/certbot/tests/cert_manager_test.py b/certbot/tests/cert_manager_test.py
index 4cff508e5..25dd0f45a 100644
--- a/certbot/tests/cert_manager_test.py
+++ b/certbot/tests/cert_manager_test.py
@@ -9,7 +9,7 @@ import unittest
 import configobj
 import mock
 
-from certbot import configuration
+from certbot._internal import configuration
 from certbot import errors
 from certbot.compat import os
 from certbot.compat import filesystem
diff --git a/certbot/tests/configuration_test.py b/certbot/tests/configuration_test.py
index e1e090fb5..11dd1b967 100644
--- a/certbot/tests/configuration_test.py
+++ b/certbot/tests/configuration_test.py
@@ -1,4 +1,4 @@
-"""Tests for certbot.configuration."""
+"""Tests for certbot._internal.configuration."""
 import unittest
 
 import mock
@@ -11,18 +11,18 @@ from certbot.tests import util as test_util
 
 
 class NamespaceConfigTest(test_util.ConfigTestCase):
-    """Tests for certbot.configuration.NamespaceConfig."""
+    """Tests for certbot._internal.configuration.NamespaceConfig."""
 
     def setUp(self):
         super(NamespaceConfigTest, self).setUp()
-        self.config.foo = 'bar'
+        self.config.foo = 'bar' # pylint: disable=blacklisted-name
         self.config.server = 'https://acme-server.org:443/new'
         self.config.https_port = 1234
         self.config.http01_port = 4321
 
     def test_init_same_ports(self):
         self.config.namespace.https_port = 4321
-        from certbot.configuration import NamespaceConfig
+        from certbot._internal.configuration import NamespaceConfig
         self.assertRaises(errors.Error, NamespaceConfig, self.config.namespace)
 
     def test_proxy_getattr(self):
@@ -38,7 +38,7 @@ class NamespaceConfigTest(test_util.ConfigTestCase):
         self.assertEqual(['user:pass@acme.server:443', 'p', 'a', 't', 'h'],
                          self.config.server_path.split(os.path.sep))
 
-    @mock.patch('certbot.configuration.constants')
+    @mock.patch('certbot._internal.configuration.constants')
     def test_dynamic_dirs(self, mock_constants):
         mock_constants.ACCOUNTS_DIR = 'acc'
         mock_constants.BACKUP_DIR = 'backups'
@@ -70,7 +70,7 @@ class NamespaceConfigTest(test_util.ConfigTestCase):
             os.path.normpath(os.path.join(self.config.work_dir, 't')))
 
     def test_absolute_paths(self):
-        from certbot.configuration import NamespaceConfig
+        from certbot._internal.configuration import NamespaceConfig
 
         config_base = "foo"
         work_base = "bar"
@@ -103,7 +103,7 @@ class NamespaceConfigTest(test_util.ConfigTestCase):
         self.assertTrue(os.path.isabs(config.key_dir))
         self.assertTrue(os.path.isabs(config.temp_checkpoint_dir))
 
-    @mock.patch('certbot.configuration.constants')
+    @mock.patch('certbot._internal.configuration.constants')
     def test_renewal_dynamic_dirs(self, mock_constants):
         mock_constants.ARCHIVE_DIR = 'a'
         mock_constants.LIVE_DIR = 'l'
@@ -118,7 +118,7 @@ class NamespaceConfigTest(test_util.ConfigTestCase):
                     self.config.config_dir, 'renewal_configs'))
 
     def test_renewal_absolute_paths(self):
-        from certbot.configuration import NamespaceConfig
+        from certbot._internal.configuration import NamespaceConfig
 
         config_base = "foo"
         work_base = "bar"
diff --git a/certbot/tests/main_test.py b/certbot/tests/main_test.py
index 835951b70..e27284ac2 100644
--- a/certbot/tests/main_test.py
+++ b/certbot/tests/main_test.py
@@ -23,7 +23,7 @@ from acme.magic_typing import List  # pylint: disable=unused-import, no-name-in-
 import certbot.tests.util as test_util
 from certbot._internal import account
 from certbot import cli
-from certbot import configuration
+from certbot._internal import configuration
 from certbot._internal import constants
 from certbot import crypto_util
 from certbot import errors
diff --git a/certbot/tests/renewal_test.py b/certbot/tests/renewal_test.py
index 612a5fcb2..dcbaed6a2 100644
--- a/certbot/tests/renewal_test.py
+++ b/certbot/tests/renewal_test.py
@@ -4,7 +4,7 @@ import mock
 
 from acme import challenges
 
-from certbot import configuration
+from certbot._internal import configuration
 from certbot import errors
 from certbot._internal import storage
 
diff --git a/certbot/tests/util.py b/certbot/tests/util.py
index 00452fef9..d9ff18f1c 100644
--- a/certbot/tests/util.py
+++ b/certbot/tests/util.py
@@ -19,7 +19,7 @@ from six.moves import reload_module  # pylint: disable=import-error
 from cryptography.hazmat.backends import default_backend
 from cryptography.hazmat.primitives import serialization
 
-from certbot import configuration
+from certbot._internal import configuration
 from certbot._internal import constants
 from certbot import interfaces
 from certbot._internal import lock
diff --git a/docs/api/configuration.rst b/docs/api/configuration.rst
deleted file mode 100644
index 4e99c73d2..000000000
--- a/docs/api/configuration.rst
+++ /dev/null
@@ -1,5 +0,0 @@
-:mod:`certbot.configuration`
---------------------------------
-
-.. automodule:: certbot.configuration
-   :members:

From 4a8ede2562d0b77c23af89a7e550ba194eb4251e Mon Sep 17 00:00:00 2001
From: Amjad Mashaal <me@amjad.io>
Date: Wed, 13 Nov 2019 20:24:37 +0200
Subject: [PATCH 5/7] Deprecate certbot register --update-registration (#7556)

Closes #7452.
---
 CHANGELOG.md                                  |  1 +
 .../certbot_tests/test_main.py                |  5 --
 certbot/_internal/constants.py                |  1 -
 certbot/_internal/main.py                     |  8 ---
 certbot/cli.py                                |  6 --
 certbot/tests/main_test.py                    | 63 -------------------
 6 files changed, 1 insertion(+), 83 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7806b2ffe..e1eaed7ba 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -20,6 +20,7 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
   `certbot.plugins.common.Installer.view_config_changes`,
   `certbot.reverter.Reverter.view_config_changes`, and
   `certbot.util.get_systemd_os_info` have been removed
+* Certbot's `register --update-registration` subcommand has been removed
 
 ### Fixed
 
diff --git a/certbot-ci/certbot_integration_tests/certbot_tests/test_main.py b/certbot-ci/certbot_integration_tests/certbot_tests/test_main.py
index 50c685a57..cd4c316d2 100644
--- a/certbot-ci/certbot_integration_tests/certbot_tests/test_main.py
+++ b/certbot-ci/certbot_integration_tests/certbot_tests/test_main.py
@@ -62,11 +62,6 @@ def test_registration_override(context):
     context.certbot(['unregister'])
     context.certbot(['register', '--email', 'ex1@domain.org,ex2@domain.org'])
 
-    # TODO: When `certbot register --update-registration` is fully deprecated,
-    #  delete the two following deprecated uses
-    context.certbot(['register', '--update-registration', '--email', 'ex1@domain.org'])
-    context.certbot(['register', '--update-registration', '--email', 'ex1@domain.org,ex2@domain.org'])
-
     context.certbot(['update_account', '--email', 'example@domain.org'])
     context.certbot(['update_account', '--email', 'ex1@domain.org,ex2@domain.org'])
 
diff --git a/certbot/_internal/constants.py b/certbot/_internal/constants.py
index d8c3c2ae1..5ac7ee72d 100644
--- a/certbot/_internal/constants.py
+++ b/certbot/_internal/constants.py
@@ -32,7 +32,6 @@ CLI_DEFAULTS = dict(
     certname=None,
     dry_run=False,
     register_unsafely_without_email=False,
-    update_registration=False,
     email=None,
     eff_email=None,
     reinstall=False,
diff --git a/certbot/_internal/main.py b/certbot/_internal/main.py
index 8daca6514..bf6fd2b11 100644
--- a/certbot/_internal/main.py
+++ b/certbot/_internal/main.py
@@ -668,14 +668,6 @@ def register(config, unused_plugins):
     :rtype: None or str
 
     """
-    # TODO: When `certbot register --update-registration` is fully deprecated,
-    # delete the true case of if block
-    if config.update_registration:
-        msg = ("Usage 'certbot register --update-registration' is deprecated.\n"
-               "Please use 'certbot update_account [options]' instead.\n")
-        logger.warning(msg)
-        return update_account(config, unused_plugins)
-
     # Portion of _determine_account logic to see whether accounts already
     # exist or not.
     account_storage = account.AccountFileStorage(config)
diff --git a/certbot/cli.py b/certbot/cli.py
index 739eadd8a..f13dedbaa 100644
--- a/certbot/cli.py
+++ b/certbot/cli.py
@@ -980,12 +980,6 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
              "certificates. Updates to the Subscriber Agreement will still "
              "affect you, and will be effective 14 days after posting an "
              "update to the web site.")
-    # TODO: When `certbot register --update-registration` is fully deprecated,
-    # delete following helpful.add
-    helpful.add(
-        "register", "--update-registration", action="store_true",
-        default=flag_default("update_registration"), dest="update_registration",
-        help=argparse.SUPPRESS)
     helpful.add(
         ["register", "update_account", "unregister", "automation"], "-m", "--email",
         default=flag_default("email"),
diff --git a/certbot/tests/main_test.py b/certbot/tests/main_test.py
index e27284ac2..789c0db42 100644
--- a/certbot/tests/main_test.py
+++ b/certbot/tests/main_test.py
@@ -1400,33 +1400,6 @@ class MainTest(test_util.ConfigTestCase):  # pylint: disable=too-many-public-met
                  "user@example.org"])
             self.assertTrue("Could not find an existing account" in x[0])
 
-    # TODO: When `certbot register --update-registration` is fully deprecated,
-    # delete the following test
-    def test_update_registration_no_existing_accounts_deprecated(self):
-        # with mock.patch('certbot._internal.main.client') as mocked_client:
-        with mock.patch('certbot._internal.main.account') as mocked_account:
-            mocked_storage = mock.MagicMock()
-            mocked_account.AccountFileStorage.return_value = mocked_storage
-            mocked_storage.find_all.return_value = []
-            x = self._call_no_clientmock(
-                ["register", "--update-registration", "--email",
-                 "user@example.org"])
-            self.assertTrue("Could not find an existing account" in x[0])
-
-    # TODO: When `certbot register --update-registration` is fully deprecated,
-    # delete the following test
-    def test_update_registration_unsafely_deprecated(self):
-        # This test will become obsolete when register --update-registration
-        # supports removing an e-mail address from the account
-        with mock.patch('certbot._internal.main.account') as mocked_account:
-            mocked_storage = mock.MagicMock()
-            mocked_account.AccountFileStorage.return_value = mocked_storage
-            mocked_storage.find_all.return_value = ["an account"]
-            x = self._call_no_clientmock(
-                "register --update-registration "
-                "--register-unsafely-without-email".split())
-            self.assertTrue("--register-unsafely-without-email" in x[0])
-
     @mock.patch('certbot._internal.main.display_ops.get_email')
     @test_util.patch_get_utility()
     def test_update_account_with_email(self, mock_utility, mock_email):
@@ -1457,42 +1430,6 @@ class MainTest(test_util.ConfigTestCase):  # pylint: disable=too-many-public-met
                             email in mock_utility().add_message.call_args[0][0])
                         self.assertTrue(mock_handle.called)
 
-    # TODO: When `certbot register --update-registration` is fully deprecated,
-    # delete the following test
-    @mock.patch('certbot._internal.main.display_ops.get_email')
-    @test_util.patch_get_utility()
-    def test_update_registration_with_email_deprecated(self, mock_utility, mock_email):
-        email = "user@example.com"
-        mock_email.return_value = email
-        with mock.patch('certbot._internal.eff.handle_subscription') as mock_handle:
-            with mock.patch('certbot._internal.main._determine_account') as mocked_det:
-                with mock.patch('certbot._internal.main.account') as mocked_account:
-                    with mock.patch('certbot._internal.main.client') as mocked_client:
-                        mocked_storage = mock.MagicMock()
-                        mocked_account.AccountFileStorage.return_value = mocked_storage
-                        mocked_storage.find_all.return_value = ["an account"]
-                        mock_acc = mock.MagicMock()
-                        mock_regr = mock_acc.regr
-                        mocked_det.return_value = (mock_acc, "foo")
-                        cb_client = mock.MagicMock()
-                        mocked_client.Client.return_value = cb_client
-                        x = self._call_no_clientmock(
-                            ["register", "--update-registration"])
-                        # When registration change succeeds, the return value
-                        # of register() is None
-                        self.assertTrue(x[0] is None)
-                        # and we got supposedly did update the registration from
-                        # the server
-                        reg_arg = cb_client.acme.update_registration.call_args[0][0]
-                        # Test the return value of .update() was used because
-                        # the regr is immutable.
-                        self.assertEqual(reg_arg, mock_regr.update())
-                        # and we saved the updated registration on disk
-                        self.assertTrue(mocked_storage.save_regr.called)
-                        self.assertTrue(
-                            email in mock_utility().add_message.call_args[0][0])
-                        self.assertTrue(mock_handle.called)
-
     @mock.patch('certbot._internal.plugins.selection.choose_configurator_plugins')
     @mock.patch('certbot._internal.updater._run_updaters')
     def test_plugin_selection_error(self, mock_run, mock_choose):

From 57148b7593636e6aa1ddec21e06d2c324609e69e Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@users.noreply.github.com>
Date: Wed, 13 Nov 2019 11:14:26 -0800
Subject: [PATCH 6/7] Fix shebang in rebuild_deps (#7557)

When you try to run this script, it crashes with:
```
standard_init_linux.go:211: exec user process caused "exec format error"
```
This is caused by the script being written to have the contents:
```
\
#!/bin/sh
set -e
...
```
This fixes the problem by removing the slash and moving the shebang to the first line of the string.
---
 letsencrypt-auto-source/rebuild_dependencies.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/letsencrypt-auto-source/rebuild_dependencies.py b/letsencrypt-auto-source/rebuild_dependencies.py
index 0db787a0b..d98ae8706 100755
--- a/letsencrypt-auto-source/rebuild_dependencies.py
+++ b/letsencrypt-auto-source/rebuild_dependencies.py
@@ -62,8 +62,7 @@ CERTBOT_REPO_PATH = dirname(dirname(abspath(__file__)))
 #     without pinned dependencies, and respecting input authoritative requirements
 #   - `certbot plugins` is called to check we have an healthy environment
 #   - finally current set of dependencies is extracted out of the docker using pip freeze
-SCRIPT = r"""\
-#!/bin/sh
+SCRIPT = r"""#!/bin/sh
 set -e
 
 cd /tmp/certbot

From 4d4c83d4d8a0f1c323ec5fc4f993335685562cf6 Mon Sep 17 00:00:00 2001
From: ohemorange <ebportnoy@gmail.com>
Date: Wed, 13 Nov 2019 11:14:46 -0800
Subject: [PATCH 7/7] Internalize modules called by internal plugins (#7543)

* Move hooks.py to _internal

* Move cli.py to _internal
---
 certbot/{ => _internal}/cli.py         |  2 +-
 certbot/_internal/client.py            |  2 +-
 certbot/{ => _internal}/hooks.py       |  0
 certbot/_internal/main.py              |  4 +-
 certbot/_internal/plugins/manual.py    |  2 +-
 certbot/_internal/plugins/selection.py |  4 +-
 certbot/_internal/plugins/webroot.py   |  2 +-
 certbot/_internal/renewal.py           |  4 +-
 certbot/_internal/storage.py           |  2 +-
 certbot/plugins/enhancements.py        |  4 +-
 certbot/tests/cli_test.py              | 16 ++---
 certbot/tests/client_test.py           |  2 +-
 certbot/tests/hook_test.py             | 84 +++++++++++++-------------
 certbot/tests/main_test.py             |  6 +-
 certbot/tests/renewal_test.py          |  2 +-
 certbot/tests/storage_test.py          |  6 +-
 docs/api/cli.rst                       |  5 --
 docs/api/hooks.rst                     |  5 --
 18 files changed, 71 insertions(+), 81 deletions(-)
 rename certbot/{ => _internal}/cli.py (99%)
 rename certbot/{ => _internal}/hooks.py (100%)
 delete mode 100644 docs/api/cli.rst
 delete mode 100644 docs/api/hooks.rst

diff --git a/certbot/cli.py b/certbot/_internal/cli.py
similarity index 99%
rename from certbot/cli.py
rename to certbot/_internal/cli.py
index f13dedbaa..ad20f6494 100644
--- a/certbot/cli.py
+++ b/certbot/_internal/cli.py
@@ -25,7 +25,7 @@ import certbot._internal.plugins.selection as plugin_selection
 from certbot._internal import constants
 from certbot import crypto_util
 from certbot import errors
-from certbot import hooks
+from certbot._internal import hooks
 from certbot import interfaces
 from certbot import util
 from certbot.compat import os
diff --git a/certbot/_internal/client.py b/certbot/_internal/client.py
index f694a0ae7..2a9a52e73 100644
--- a/certbot/_internal/client.py
+++ b/certbot/_internal/client.py
@@ -20,7 +20,7 @@ from acme.magic_typing import Optional, List  # pylint: disable=unused-import,no
 import certbot
 from certbot._internal import account
 from certbot._internal import auth_handler
-from certbot import cli
+from certbot._internal import cli
 from certbot._internal import constants
 from certbot import crypto_util
 from certbot._internal import eff
diff --git a/certbot/hooks.py b/certbot/_internal/hooks.py
similarity index 100%
rename from certbot/hooks.py
rename to certbot/_internal/hooks.py
diff --git a/certbot/_internal/main.py b/certbot/_internal/main.py
index bf6fd2b11..e58c5285f 100644
--- a/certbot/_internal/main.py
+++ b/certbot/_internal/main.py
@@ -16,14 +16,14 @@ from acme.magic_typing import Union  # pylint: disable=unused-import, no-name-in
 import certbot
 from certbot._internal import account
 from certbot._internal import cert_manager
-from certbot import cli
+from certbot._internal import cli
 from certbot._internal import client
 from certbot._internal import configuration
 from certbot._internal import constants
 from certbot import crypto_util
 from certbot._internal import eff
 from certbot import errors
-from certbot import hooks
+from certbot._internal import hooks
 from certbot import interfaces
 from certbot._internal import log
 from certbot._internal import renewal
diff --git a/certbot/_internal/plugins/manual.py b/certbot/_internal/plugins/manual.py
index 4bb11de3f..43f70d650 100644
--- a/certbot/_internal/plugins/manual.py
+++ b/certbot/_internal/plugins/manual.py
@@ -7,7 +7,7 @@ from acme.magic_typing import Dict  # pylint: disable=unused-import, no-name-in-
 
 from certbot import achallenges  # pylint: disable=unused-import
 from certbot import errors
-from certbot import hooks
+from certbot._internal import hooks
 from certbot import interfaces
 from certbot import reverter
 from certbot.compat import os
diff --git a/certbot/_internal/plugins/selection.py b/certbot/_internal/plugins/selection.py
index ae3d8448b..1ac3456d3 100644
--- a/certbot/_internal/plugins/selection.py
+++ b/certbot/_internal/plugins/selection.py
@@ -197,7 +197,7 @@ def choose_configurator_plugins(config, plugins, verb):
     # Which plugins do we need?
     if verb == "run":
         need_inst = need_auth = True
-        from certbot.cli import cli_command
+        from certbot._internal.cli import cli_command
         if req_auth in noninstaller_plugins and not req_inst:
             msg = ('With the {0} plugin, you probably want to use the "certonly" command, eg:{1}'
                    '{1}    {2} certonly --{0}{1}{1}'
@@ -328,7 +328,7 @@ def diagnose_configurator_problem(cfg_type, requested, plugins):
                    "your existing configuration.\nThe error was: {1!r}"
                    .format(requested, plugins[requested].problem))
     elif cfg_type == "installer":
-        from certbot.cli import cli_command
+        from certbot._internal.cli import cli_command
         msg = ('Certbot doesn\'t know how to automatically configure the web '
           'server on this system. However, it can still get a certificate for '
           'you. Please run "{0} certonly" to do so. You\'ll need to '
diff --git a/certbot/_internal/plugins/webroot.py b/certbot/_internal/plugins/webroot.py
index 33e6530a1..b87b3092a 100644
--- a/certbot/_internal/plugins/webroot.py
+++ b/certbot/_internal/plugins/webroot.py
@@ -15,7 +15,7 @@ from acme.magic_typing import Dict, Set, DefaultDict, List
 # pylint: enable=unused-import, no-name-in-module
 
 from certbot import achallenges  # pylint: disable=unused-import
-from certbot import cli
+from certbot._internal import cli
 from certbot import errors
 from certbot import interfaces
 from certbot.compat import os
diff --git a/certbot/_internal/renewal.py b/certbot/_internal/renewal.py
index 31833c806..5f2f90d63 100644
--- a/certbot/_internal/renewal.py
+++ b/certbot/_internal/renewal.py
@@ -15,10 +15,10 @@ import zope.component
 
 from acme.magic_typing import List  # pylint: disable=unused-import, no-name-in-module
 
-from certbot import cli
+from certbot._internal import cli
 from certbot import crypto_util
 from certbot import errors
-from certbot import hooks
+from certbot._internal import hooks
 from certbot import interfaces
 from certbot._internal import storage
 from certbot._internal import updater
diff --git a/certbot/_internal/storage.py b/certbot/_internal/storage.py
index b7b878dab..f7e7d5e7f 100644
--- a/certbot/_internal/storage.py
+++ b/certbot/_internal/storage.py
@@ -12,7 +12,7 @@ import pytz
 import six
 
 import certbot
-from certbot import cli
+from certbot._internal import cli
 from certbot._internal import constants
 from certbot import crypto_util
 from certbot._internal import error_handler
diff --git a/certbot/plugins/enhancements.py b/certbot/plugins/enhancements.py
index 8896c1a98..d917b0ea4 100644
--- a/certbot/plugins/enhancements.py
+++ b/certbot/plugins/enhancements.py
@@ -78,9 +78,9 @@ def enable(lineage, domains, installer, config):
 
 def populate_cli(add):
     """
-    Populates the command line flags for certbot.cli.HelpfulParser
+    Populates the command line flags for certbot._internal.cli.HelpfulParser
 
-    :param add: Add function of certbot.cli.HelpfulParser
+    :param add: Add function of certbot._internal.cli.HelpfulParser
     :type add: func
     """
     for enh in _INDEX:
diff --git a/certbot/tests/cli_test.py b/certbot/tests/cli_test.py
index 8a398188c..a38a6fc4f 100644
--- a/certbot/tests/cli_test.py
+++ b/certbot/tests/cli_test.py
@@ -1,4 +1,4 @@
-"""Tests for certbot.cli."""
+"""Tests for certbot._internal.cli."""
 import argparse
 import copy
 import tempfile
@@ -11,7 +11,7 @@ from six.moves import reload_module  # pylint: disable=import-error
 from acme import challenges
 
 import certbot.tests.util as test_util
-from certbot import cli
+from certbot._internal import cli
 from certbot._internal import constants
 from certbot import errors
 from certbot.compat import os
@@ -94,7 +94,7 @@ class ParseTest(unittest.TestCase):  # pylint: disable=too-many-public-methods
 
         return output.getvalue()
 
-    @mock.patch("certbot.cli.flag_default")
+    @mock.patch("certbot._internal.cli.flag_default")
     def test_cli_ini_domains(self, mock_flag_default):
         with tempfile.NamedTemporaryFile() as tmp_config:
             tmp_config.close()  # close now because of compatibility issues on Windows
@@ -366,7 +366,7 @@ class ParseTest(unittest.TestCase):  # pylint: disable=too-many-public-methods
             errors.Error, self.parse, "-n --force-interactive".split())
 
     def test_deploy_hook_conflict(self):
-        with mock.patch("certbot.cli.sys.stderr"):
+        with mock.patch("certbot._internal.cli.sys.stderr"):
             self.assertRaises(SystemExit, self.parse,
                               "--renew-hook foo --deploy-hook bar".split())
 
@@ -386,7 +386,7 @@ class ParseTest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         self.assertEqual(namespace.renew_hook, value)
 
     def test_renew_hook_conflict(self):
-        with mock.patch("certbot.cli.sys.stderr"):
+        with mock.patch("certbot._internal.cli.sys.stderr"):
             self.assertRaises(SystemExit, self.parse,
                               "--deploy-hook foo --renew-hook bar".split())
 
@@ -406,7 +406,7 @@ class ParseTest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         self.assertEqual(namespace.renew_hook, value)
 
     def test_max_log_backups_error(self):
-        with mock.patch('certbot.cli.sys.stderr'):
+        with mock.patch('certbot._internal.cli.sys.stderr'):
             self.assertRaises(
                 SystemExit, self.parse, "--max-log-backups foo".split())
             self.assertRaises(
@@ -462,7 +462,7 @@ class ParseTest(unittest.TestCase):  # pylint: disable=too-many-public-methods
 
 
 class DefaultTest(unittest.TestCase):
-    """Tests for certbot.cli._Default."""
+    """Tests for certbot._internal.cli._Default."""
 
 
     def setUp(self):
@@ -536,7 +536,7 @@ class SetByCliTest(unittest.TestCase):
 
 
 def _call_set_by_cli(var, args, verb):
-    with mock.patch('certbot.cli.helpful_parser') as mock_parser:
+    with mock.patch('certbot._internal.cli.helpful_parser') as mock_parser:
         with test_util.patch_get_utility():
             mock_parser.args = args
             mock_parser.verb = verb
diff --git a/certbot/tests/client_test.py b/certbot/tests/client_test.py
index e7cf84c3c..1ebd5f7b5 100644
--- a/certbot/tests/client_test.py
+++ b/certbot/tests/client_test.py
@@ -462,7 +462,7 @@ class ClientTest(ClientTestCommon):
         names = [call[0][0] for call in mock_storage.call_args_list]
         self.assertEqual(names, ["example_cert", "example.com", "example.com"])
 
-    @mock.patch("certbot.cli.helpful_parser")
+    @mock.patch("certbot._internal.cli.helpful_parser")
     def test_save_certificate(self, mock_parser):
         # pylint: disable=too-many-locals
         certs = ["cert_512.pem", "cert-san_512.pem"]
diff --git a/certbot/tests/hook_test.py b/certbot/tests/hook_test.py
index 017edc560..2e403d8f3 100644
--- a/certbot/tests/hook_test.py
+++ b/certbot/tests/hook_test.py
@@ -1,4 +1,4 @@
-"""Tests for certbot.hooks."""
+"""Tests for certbot._internal.hooks."""
 import unittest
 
 import mock
@@ -12,14 +12,14 @@ from certbot.tests import util as test_util
 
 
 class ValidateHooksTest(unittest.TestCase):
-    """Tests for certbot.hooks.validate_hooks."""
+    """Tests for certbot._internal.hooks.validate_hooks."""
 
     @classmethod
     def _call(cls, *args, **kwargs):
-        from certbot.hooks import validate_hooks
+        from certbot._internal.hooks import validate_hooks
         return validate_hooks(*args, **kwargs)
 
-    @mock.patch("certbot.hooks.validate_hook")
+    @mock.patch("certbot._internal.hooks.validate_hook")
     def test_it(self, mock_validate_hook):
         config = mock.MagicMock()
         self._call(config)
@@ -31,30 +31,30 @@ class ValidateHooksTest(unittest.TestCase):
 
 
 class ValidateHookTest(test_util.TempDirTestCase):
-    """Tests for certbot.hooks.validate_hook."""
+    """Tests for certbot._internal.hooks.validate_hook."""
 
     @classmethod
     def _call(cls, *args, **kwargs):
-        from certbot.hooks import validate_hook
+        from certbot._internal.hooks import validate_hook
         return validate_hook(*args, **kwargs)
 
     def test_hook_not_executable(self):
         # prevent unnecessary modifications to PATH
-        with mock.patch("certbot.hooks.plug_util.path_surgery"):
+        with mock.patch("certbot._internal.hooks.plug_util.path_surgery"):
             # We just mock out filesystem.is_executable since on Windows, it is difficult
             # to get a fully working test around executable permissions. See
             # certbot.tests.compat.filesystem::NotExecutableTest for more in-depth tests.
-            with mock.patch("certbot.hooks.filesystem.is_executable", return_value=False):
+            with mock.patch("certbot._internal.hooks.filesystem.is_executable", return_value=False):
                 self.assertRaises(errors.HookCommandNotFound, self._call, 'dummy', "foo")
 
-    @mock.patch("certbot.hooks.util.exe_exists")
+    @mock.patch("certbot._internal.hooks.util.exe_exists")
     def test_not_found(self, mock_exe_exists):
         mock_exe_exists.return_value = False
-        with mock.patch("certbot.hooks.plug_util.path_surgery") as mock_ps:
+        with mock.patch("certbot._internal.hooks.plug_util.path_surgery") as mock_ps:
             self.assertRaises(errors.HookCommandNotFound, self._call, "foo", "bar")
         self.assertTrue(mock_ps.called)
 
-    @mock.patch("certbot.hooks._prog")
+    @mock.patch("certbot._internal.hooks._prog")
     def test_unset(self, mock_prog):
         self._call(None, "foo")
         self.assertFalse(mock_prog.called)
@@ -70,24 +70,24 @@ class HookTest(test_util.ConfigTestCase):
 
     @classmethod
     def _call_with_mock_execute(cls, *args, **kwargs):
-        """Calls self._call after mocking out certbot.hooks.execute.
+        """Calls self._call after mocking out certbot._internal.hooks.execute.
 
         The mock execute object is returned rather than the return value
         of self._call.
 
         """
-        with mock.patch("certbot.hooks.execute") as mock_execute:
+        with mock.patch("certbot._internal.hooks.execute") as mock_execute:
             mock_execute.return_value = ("", "")
             cls._call(*args, **kwargs)
         return mock_execute
 
 
 class PreHookTest(HookTest):
-    """Tests for certbot.hooks.pre_hook."""
+    """Tests for certbot._internal.hooks.pre_hook."""
 
     @classmethod
     def _call(cls, *args, **kwargs):
-        from certbot.hooks import pre_hook
+        from certbot._internal.hooks import pre_hook
         return pre_hook(*args, **kwargs)
 
     def setUp(self):
@@ -107,7 +107,7 @@ class PreHookTest(HookTest):
         super(PreHookTest, self).tearDown()
 
     def _reset_pre_hook_already(self):
-        from certbot.hooks import executed_pre_hooks
+        from certbot._internal.hooks import executed_pre_hooks
         executed_pre_hooks.clear()
 
     def test_certonly(self):
@@ -128,7 +128,7 @@ class PreHookTest(HookTest):
         self.config.verb = "renew"
         os.remove(self.dir_hook)
 
-        with mock.patch("certbot.hooks.logger") as mock_logger:
+        with mock.patch("certbot._internal.hooks.logger") as mock_logger:
             mock_execute = self._call_with_mock_execute(self.config)
         self.assertFalse(mock_execute.called)
         self.assertFalse(mock_logger.info.called)
@@ -154,18 +154,18 @@ class PreHookTest(HookTest):
         self._test_no_executions_common()
 
     def _test_no_executions_common(self):
-        with mock.patch("certbot.hooks.logger") as mock_logger:
+        with mock.patch("certbot._internal.hooks.logger") as mock_logger:
             mock_execute = self._call_with_mock_execute(self.config)
         self.assertFalse(mock_execute.called)
         self.assertTrue(mock_logger.info.called)
 
 
 class PostHookTest(HookTest):
-    """Tests for certbot.hooks.post_hook."""
+    """Tests for certbot._internal.hooks.post_hook."""
 
     @classmethod
     def _call(cls, *args, **kwargs):
-        from certbot.hooks import post_hook
+        from certbot._internal.hooks import post_hook
         return post_hook(*args, **kwargs)
 
     def setUp(self):
@@ -185,7 +185,7 @@ class PostHookTest(HookTest):
         super(PostHookTest, self).tearDown()
 
     def _reset_post_hook_eventually(self):
-        from certbot.hooks import post_hooks
+        from certbot._internal.hooks import post_hooks
         del post_hooks[:]
 
     def test_certonly_and_run_with_hook(self):
@@ -239,27 +239,27 @@ class PostHookTest(HookTest):
             self.assertEqual(self._get_eventually(), expected)
 
     def _get_eventually(self):
-        from certbot.hooks import post_hooks
+        from certbot._internal.hooks import post_hooks
         return post_hooks
 
 
 class RunSavedPostHooksTest(HookTest):
-    """Tests for certbot.hooks.run_saved_post_hooks."""
+    """Tests for certbot._internal.hooks.run_saved_post_hooks."""
 
     @classmethod
     def _call(cls, *args, **kwargs):
-        from certbot.hooks import run_saved_post_hooks
+        from certbot._internal.hooks import run_saved_post_hooks
         return run_saved_post_hooks()
 
     def _call_with_mock_execute_and_eventually(self, *args, **kwargs):
         """Call run_saved_post_hooks but mock out execute and eventually
 
-        certbot.hooks.post_hooks is replaced with
+        certbot._internal.hooks.post_hooks is replaced with
         self.eventually. The mock execute object is returned rather than
         the return value of run_saved_post_hooks.
 
         """
-        eventually_path = "certbot.hooks.post_hooks"
+        eventually_path = "certbot._internal.hooks.post_hooks"
         with mock.patch(eventually_path, new=self.eventually):
             return self._call_with_mock_execute(*args, **kwargs)
 
@@ -290,7 +290,7 @@ class RenewalHookTest(HookTest):
     # pylint: disable=abstract-method
 
     def _call_with_mock_execute(self, *args, **kwargs):
-        """Calls self._call after mocking out certbot.hooks.execute.
+        """Calls self._call after mocking out certbot._internal.hooks.execute.
 
         The mock execute object is returned rather than the return value
         of self._call. The mock execute object asserts that environment
@@ -311,7 +311,7 @@ class RenewalHookTest(HookTest):
             self.assertEqual(os.environ["RENEWED_LINEAGE"], lineage)
             return ("", "")
 
-        with mock.patch("certbot.hooks.execute") as mock_execute:
+        with mock.patch("certbot._internal.hooks.execute") as mock_execute:
             mock_execute.side_effect = execute_side_effect
             self._call(*args, **kwargs)
         return mock_execute
@@ -329,14 +329,14 @@ class RenewalHookTest(HookTest):
 
 
 class DeployHookTest(RenewalHookTest):
-    """Tests for certbot.hooks.deploy_hook."""
+    """Tests for certbot._internal.hooks.deploy_hook."""
 
     @classmethod
     def _call(cls, *args, **kwargs):
-        from certbot.hooks import deploy_hook
+        from certbot._internal.hooks import deploy_hook
         return deploy_hook(*args, **kwargs)
 
-    @mock.patch("certbot.hooks.logger")
+    @mock.patch("certbot._internal.hooks.logger")
     def test_dry_run(self, mock_logger):
         self.config.deploy_hook = "foo"
         self.config.dry_run = True
@@ -345,7 +345,7 @@ class DeployHookTest(RenewalHookTest):
         self.assertFalse(mock_execute.called)
         self.assertTrue(mock_logger.warning.called)
 
-    @mock.patch("certbot.hooks.logger")
+    @mock.patch("certbot._internal.hooks.logger")
     def test_no_hook(self, mock_logger):
         self.config.deploy_hook = None
         mock_execute = self._call_with_mock_execute(
@@ -363,11 +363,11 @@ class DeployHookTest(RenewalHookTest):
 
 
 class RenewHookTest(RenewalHookTest):
-    """Tests for certbot.hooks.renew_hook"""
+    """Tests for certbot._internal.hooks.renew_hook"""
 
     @classmethod
     def _call(cls, *args, **kwargs):
-        from certbot.hooks import renew_hook
+        from certbot._internal.hooks import renew_hook
         return renew_hook(*args, **kwargs)
 
     def setUp(self):
@@ -385,7 +385,7 @@ class RenewHookTest(RenewalHookTest):
             self.config, ["example.org"], "/foo/bar")
         mock_execute.assert_called_once_with("deploy-hook", self.config.renew_hook)
 
-    @mock.patch("certbot.hooks.logger")
+    @mock.patch("certbot._internal.hooks.logger")
     def test_dry_run(self, mock_logger):
         self.config.dry_run = True
         mock_execute = self._call_with_mock_execute(
@@ -397,7 +397,7 @@ class RenewHookTest(RenewalHookTest):
         self.config.renew_hook = None
         os.remove(self.dir_hook)
 
-        with mock.patch("certbot.hooks.logger") as mock_logger:
+        with mock.patch("certbot._internal.hooks.logger") as mock_logger:
             mock_execute = self._call_with_mock_execute(
                 self.config, ["example.org"], "/foo/bar")
         self.assertFalse(mock_execute.called)
@@ -417,11 +417,11 @@ class RenewHookTest(RenewalHookTest):
 
 
 class ExecuteTest(unittest.TestCase):
-    """Tests for certbot.hooks.execute."""
+    """Tests for certbot._internal.hooks.execute."""
 
     @classmethod
     def _call(cls, *args, **kwargs):
-        from certbot.hooks import execute
+        from certbot._internal.hooks import execute
         return execute(*args, **kwargs)
 
     def test_it(self):
@@ -433,10 +433,10 @@ class ExecuteTest(unittest.TestCase):
     def _test_common(self, returncode, stdout, stderr):
         given_command = "foo"
         given_name = "foo-hook"
-        with mock.patch("certbot.hooks.Popen") as mock_popen:
+        with mock.patch("certbot._internal.hooks.Popen") as mock_popen:
             mock_popen.return_value.communicate.return_value = (stdout, stderr)
             mock_popen.return_value.returncode = returncode
-            with mock.patch("certbot.hooks.logger") as mock_logger:
+            with mock.patch("certbot._internal.hooks.logger") as mock_logger:
                 self.assertEqual(self._call(given_name, given_command), (stderr, stdout))
 
         executed_command = mock_popen.call_args[1].get(
@@ -453,11 +453,11 @@ class ExecuteTest(unittest.TestCase):
 
 
 class ListHooksTest(test_util.TempDirTestCase):
-    """Tests for certbot.hooks.list_hooks."""
+    """Tests for certbot._internal.hooks.list_hooks."""
 
     @classmethod
     def _call(cls, *args, **kwargs):
-        from certbot.hooks import list_hooks
+        from certbot._internal.hooks import list_hooks
         return list_hooks(*args, **kwargs)
 
     def test_empty(self):
diff --git a/certbot/tests/main_test.py b/certbot/tests/main_test.py
index 789c0db42..ee9fe8558 100644
--- a/certbot/tests/main_test.py
+++ b/certbot/tests/main_test.py
@@ -22,7 +22,7 @@ from acme.magic_typing import List  # pylint: disable=unused-import, no-name-in-
 
 import certbot.tests.util as test_util
 from certbot._internal import account
-from certbot import cli
+from certbot._internal import cli
 from certbot._internal import configuration
 from certbot._internal import constants
 from certbot import crypto_util
@@ -715,7 +715,7 @@ class MainTest(test_util.ConfigTestCase):  # pylint: disable=too-many-public-met
         # This needed two calls to find_all(), which we're avoiding for now
         # because of possible side effects:
         # https://github.com/letsencrypt/letsencrypt/commit/51ed2b681f87b1eb29088dd48718a54f401e4855
-        #with mock.patch('certbot.cli.plugins_testable') as plugins:
+        #with mock.patch('certbot._internal.cli.plugins_testable') as plugins:
         #    plugins.return_value = {"apache": True, "nginx": True}
         #    ret, _, _, _ = self._call(args)
         #    self.assertTrue("Too many flags setting" in ret)
@@ -1145,7 +1145,7 @@ class MainTest(test_util.ConfigTestCase):  # pylint: disable=too-many-public-met
         test_util.make_lineage(self.config.config_dir, 'sample-renewal.conf')
         args = ["renew", "--dry-run", "--post-hook=no-such-command",
                 "--disable-hook-validation"]
-        with mock.patch("certbot.hooks.post_hook"):
+        with mock.patch("certbot._internal.hooks.post_hook"):
             self._test_renewal_common(True, [], args=args, should_renew=True,
                                       error_expected=False)
 
diff --git a/certbot/tests/renewal_test.py b/certbot/tests/renewal_test.py
index dcbaed6a2..9b36c8b83 100644
--- a/certbot/tests/renewal_test.py
+++ b/certbot/tests/renewal_test.py
@@ -12,7 +12,7 @@ import certbot.tests.util as test_util
 
 
 class RenewalTest(test_util.ConfigTestCase):
-    @mock.patch('certbot.cli.set_by_cli')
+    @mock.patch('certbot._internal.cli.set_by_cli')
     def test_ancient_webroot_renewal_conf(self, mock_set_by_cli):
         mock_set_by_cli.return_value = False
         rc_path = test_util.make_lineage(
diff --git a/certbot/tests/storage_test.py b/certbot/tests/storage_test.py
index 0c525fcc6..72394c2a8 100644
--- a/certbot/tests/storage_test.py
+++ b/certbot/tests/storage_test.py
@@ -43,7 +43,7 @@ class RelevantValuesTest(unittest.TestCase):
         from certbot._internal.storage import relevant_values
         return relevant_values(*args, **kwargs)
 
-    @mock.patch("certbot.cli.option_was_set")
+    @mock.patch("certbot._internal.cli.option_was_set")
     @mock.patch("certbot._internal.plugins.disco.PluginsRegistry.find_all")
     def test_namespace(self, mock_find_all, mock_option_was_set):
         mock_find_all.return_value = ["certbot-foo:bar"]
@@ -53,7 +53,7 @@ class RelevantValuesTest(unittest.TestCase):
         self.assertEqual(
             self._call(self.values.copy()), self.values)
 
-    @mock.patch("certbot.cli.option_was_set")
+    @mock.patch("certbot._internal.cli.option_was_set")
     def test_option_set(self, mock_option_was_set):
         mock_option_was_set.return_value = True
 
@@ -65,7 +65,7 @@ class RelevantValuesTest(unittest.TestCase):
 
         self.assertEqual(self._call(self.values), expected_relevant_values)
 
-    @mock.patch("certbot.cli.option_was_set")
+    @mock.patch("certbot._internal.cli.option_was_set")
     def test_option_unset(self, mock_option_was_set):
         mock_option_was_set.return_value = False
 
diff --git a/docs/api/cli.rst b/docs/api/cli.rst
deleted file mode 100644
index 91be86758..000000000
--- a/docs/api/cli.rst
+++ /dev/null
@@ -1,5 +0,0 @@
-:mod:`certbot.cli`
-----------------------
-
-.. automodule:: certbot.cli
-   :members:
diff --git a/docs/api/hooks.rst b/docs/api/hooks.rst
deleted file mode 100644
index 140fbf284..000000000
--- a/docs/api/hooks.rst
+++ /dev/null
@@ -1,5 +0,0 @@
-:mod:`certbot.hooks`
-------------------------
-
-.. automodule:: certbot.hooks
-   :members: