From 46d5f7a8601d026e1a986dd3f7a1dc4b056ca886 Mon Sep 17 00:00:00 2001 From: ohemorange Date: Wed, 13 Nov 2019 10:19:27 -0800 Subject: [PATCH] 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: