From a0dbe1e85035f12e194d91148836d830871ec554 Mon Sep 17 00:00:00 2001 From: Mads Jensen Date: Mon, 3 Jan 2022 22:05:21 +0100 Subject: [PATCH 1/7] Improve assertions in certbot-apache tests. (#9131) * Improve assertions in certbot-apache tests. Replacements inspired by flake8-assertive. * Fix test failures * assertEqual is not for None :D * Pass all tests :) --- certbot-apache/tests/augeasnode_test.py | 50 ++-- certbot-apache/tests/autohsts_test.py | 23 +- certbot-apache/tests/centos6_test.py | 29 +- certbot-apache/tests/centos_test.py | 13 +- certbot-apache/tests/complex_parsing_test.py | 11 +- .../tests/configurator_reverter_test.py | 19 +- certbot-apache/tests/configurator_test.py | 260 ++++++++---------- certbot-apache/tests/debian_test.py | 31 +-- certbot-apache/tests/display_ops_test.py | 16 +- certbot-apache/tests/dualnode_test.py | 91 +++--- certbot-apache/tests/fedora_test.py | 12 +- certbot-apache/tests/gentoo_test.py | 11 +- certbot-apache/tests/http_01_test.py | 24 +- certbot-apache/tests/obj_test.py | 63 +++-- certbot-apache/tests/parser_test.py | 28 +- .../tests/parsernode_configurator_test.py | 2 +- certbot-apache/tests/util.py | 2 - 17 files changed, 330 insertions(+), 355 deletions(-) diff --git a/certbot-apache/tests/augeasnode_test.py b/certbot-apache/tests/augeasnode_test.py index 85a17ab31..1e11b5eb3 100644 --- a/certbot-apache/tests/augeasnode_test.py +++ b/certbot-apache/tests/augeasnode_test.py @@ -25,24 +25,29 @@ def _get_augeasnode_mock(filepath): metadata=metadata) return augeasnode_mock + class AugeasParserNodeTest(util.ApacheTest): # pylint: disable=too-many-public-methods """Test AugeasParserNode using available test configurations""" def setUp(self): # pylint: disable=arguments-differ super().setUp() - with mock.patch("certbot_apache._internal.configurator.ApacheConfigurator.get_parsernode_root") as mock_parsernode: + with mock.patch( + "certbot_apache._internal.configurator.ApacheConfigurator.get_parsernode_root" + ) as mock_parsernode: mock_parsernode.side_effect = _get_augeasnode_mock( os.path.join(self.config_path, "apache2.conf")) self.config = util.get_apache_configurator( - self.config_path, self.vhost_path, self.config_dir, self.work_dir, use_parsernode=True) + self.config_path, self.vhost_path, self.config_dir, self.work_dir, + use_parsernode=True, + ) self.vh_truth = util.get_vh_truth( self.temp_dir, "debian_apache_2_4/multiple_vhosts") def test_save(self): with mock.patch('certbot_apache._internal.parser.ApacheParser.save') as mock_save: self.config.parser_root.save("A save message") - self.assertTrue(mock_save.called) + self.assertIs(mock_save.called, True) self.assertEqual(mock_save.call_args[0][0], "A save message") def test_unsaved_files(self): @@ -67,7 +72,8 @@ class AugeasParserNodeTest(util.ApacheTest): # pylint: disable=too-many-public- "/Anything": "Anything", } for test in testcases: - self.assertEqual(block._aug_get_name(test), testcases[test]) # pylint: disable=protected-access + # pylint: disable=protected-access + self.assertEqual(block._aug_get_name(test), testcases[test]) def test_find_blocks(self): blocks = self.config.parser_root.find_blocks("VirtualHost", exclude=False) @@ -81,7 +87,7 @@ class AugeasParserNodeTest(util.ApacheTest): # pylint: disable=too-many-public- def test_find_directive_found(self): directives = self.config.parser_root.find_directives("Listen") self.assertEqual(len(directives), 1) - self.assertTrue(directives[0].filepath.endswith("/apache2/ports.conf")) + self.assertIs(directives[0].filepath.endswith("/apache2/ports.conf"), True) self.assertEqual(directives[0].parameters, (u'80',)) def test_find_directive_notfound(self): @@ -96,29 +102,29 @@ class AugeasParserNodeTest(util.ApacheTest): # pylint: disable=too-many-public- servername = vh.find_directives("servername") self.assertEqual(servername[0].parameters[0], "certbot.demo") found = True - self.assertTrue(found) + self.assertIs(found, True) def test_find_comments(self): rootcomment = self.config.parser_root.find_comments( "This is the main Apache server configuration file. " ) self.assertEqual(len(rootcomment), 1) - self.assertTrue(rootcomment[0].filepath.endswith( + self.assertIs(rootcomment[0].filepath.endswith( "debian_apache_2_4/multiple_vhosts/apache2/apache2.conf" - )) + ), True) def test_set_parameters(self): servernames = self.config.parser_root.find_directives("servername") names: List[str] = [] for servername in servernames: names += servername.parameters - self.assertFalse("going_to_set_this" in names) + self.assertNotIn("going_to_set_this", names) servernames[0].set_parameters(["something", "going_to_set_this"]) servernames = self.config.parser_root.find_directives("servername") names = [] for servername in servernames: names += servername.parameters - self.assertTrue("going_to_set_this" in names) + self.assertIn("going_to_set_this", names) def test_set_parameters_atinit(self): from certbot_apache._internal.augeasparser import AugeasDirectiveNode @@ -131,7 +137,7 @@ class AugeasParserNodeTest(util.ApacheTest): # pylint: disable=too-many-public- ancestor=assertions.PASS, metadata=servernames[0].metadata ) - self.assertTrue(mock_set.called) + self.assertIs(mock_set.called, True) self.assertEqual( mock_set.call_args_list[0][0][0], ["test", "setting", "these"] @@ -151,7 +157,7 @@ class AugeasParserNodeTest(util.ApacheTest): # pylint: disable=too-many-public- self.assertEqual(len(servername.parameters), 3) servername.set_parameters(["thisshouldnotexistpreviously"]) found = True - self.assertTrue(found) + self.assertIs(found, True) # Verify params servernames = self.config.parser_root.find_directives("servername") @@ -161,7 +167,7 @@ class AugeasParserNodeTest(util.ApacheTest): # pylint: disable=too-many-public- self.assertEqual(len(servername.parameters), 1) servername.set_parameters(["thisshouldnotexistpreviously"]) found = True - self.assertTrue(found) + self.assertIs(found, True) def test_add_child_comment(self): newc = self.config.parser_root.add_child_comment("The content") @@ -201,7 +207,7 @@ class AugeasParserNodeTest(util.ApacheTest): # pylint: disable=too-many-public- rpath, self.config.parser_root.metadata["augeaspath"] ) - self.assertTrue(directive.startswith("NewBlock")) + self.assertIs(directive.startswith("NewBlock"), True) def test_add_child_block_beginning(self): self.config.parser_root.add_child_block( @@ -212,7 +218,7 @@ class AugeasParserNodeTest(util.ApacheTest): # pylint: disable=too-many-public- root_path = self.config.parser_root.metadata["augeaspath"] # Get first child first = parser.aug.match("{}/*[1]".format(root_path)) - self.assertTrue(first[0].endswith("Beginning")) + self.assertIs(first[0].endswith("Beginning"), True) def test_add_child_block_append(self): self.config.parser_root.add_child_block( @@ -222,7 +228,7 @@ class AugeasParserNodeTest(util.ApacheTest): # pylint: disable=too-many-public- root_path = self.config.parser_root.metadata["augeaspath"] # Get last child last = parser.aug.match("{}/*[last()]".format(root_path)) - self.assertTrue(last[0].endswith("VeryLast")) + self.assertIs(last[0].endswith("VeryLast"), True) def test_add_child_block_append_alt(self): self.config.parser_root.add_child_block( @@ -233,7 +239,7 @@ class AugeasParserNodeTest(util.ApacheTest): # pylint: disable=too-many-public- root_path = self.config.parser_root.metadata["augeaspath"] # Get last child last = parser.aug.match("{}/*[last()]".format(root_path)) - self.assertTrue(last[0].endswith("VeryLastAlt")) + self.assertIs(last[0].endswith("VeryLastAlt"), True) def test_add_child_block_middle(self): self.config.parser_root.add_child_block( @@ -244,7 +250,7 @@ class AugeasParserNodeTest(util.ApacheTest): # pylint: disable=too-many-public- root_path = self.config.parser_root.metadata["augeaspath"] # Augeas indices start at 1 :( middle = parser.aug.match("{}/*[6]".format(root_path)) - self.assertTrue(middle[0].endswith("Middle")) + self.assertIs(middle[0].endswith("Middle"), True) def test_add_child_block_existing_name(self): parser = self.config.parser_root.parser @@ -257,7 +263,7 @@ class AugeasParserNodeTest(util.ApacheTest): # pylint: disable=too-many-public- ) new_block = parser.aug.match("{}/VirtualHost[2]".format(root_path)) self.assertEqual(len(new_block), 1) - self.assertTrue(vh.metadata["augeaspath"].endswith("VirtualHost[2]")) + self.assertIs(vh.metadata["augeaspath"].endswith("VirtualHost[2]"), True) def test_node_init_error_bad_augeaspath(self): from certbot_apache._internal.augeasparser import AugeasBlockNode @@ -302,7 +308,7 @@ class AugeasParserNodeTest(util.ApacheTest): # pylint: disable=too-many-public- self.assertEqual(len(dirs), 1) self.assertEqual(dirs[0].parameters, ("with", "parameters")) # The new directive was added to the very first line of the config - self.assertTrue(dirs[0].metadata["augeaspath"].endswith("[1]")) + self.assertIs(dirs[0].metadata["augeaspath"].endswith("[1]"), True) def test_add_child_directive_exception(self): self.assertRaises( @@ -328,8 +334,8 @@ class AugeasParserNodeTest(util.ApacheTest): # pylint: disable=too-many-public- ancs = vh.find_ancestors("Macro") self.assertEqual(len(ancs), 0) nonmacro_test = True - self.assertTrue(macro_test) - self.assertTrue(nonmacro_test) + self.assertIs(macro_test, True) + self.assertIs(nonmacro_test, True) def test_find_ancestors_bad_path(self): self.config.parser_root.metadata["augeaspath"] = "" diff --git a/certbot-apache/tests/autohsts_test.py b/certbot-apache/tests/autohsts_test.py index 72d26a33a..664d791bd 100644 --- a/certbot-apache/tests/autohsts_test.py +++ b/certbot-apache/tests/autohsts_test.py @@ -47,7 +47,7 @@ class AutoHSTSTest(util.ApacheTest): self.config.parser.modules.pop("headers_module", None) self.config.parser.modules.pop("mod_header.c", None) self.config.enable_autohsts(mock.MagicMock(), ["ocspvhost.com"]) - self.assertTrue(mock_enable.called) + self.assertIs(mock_enable.called, True) @mock.patch("certbot_apache._internal.configurator.ApacheConfigurator.restart") def test_autohsts_deploy_already_exists(self, _restart): @@ -74,7 +74,7 @@ class AutoHSTSTest(util.ApacheTest): # Verify increased value self.assertEqual(self.get_autohsts_value(self.vh_truth[7].path), inc_val) - self.assertTrue(mock_prepare.called) + self.assertIs(mock_prepare.called, True) @mock.patch("certbot_apache._internal.configurator.ApacheConfigurator.restart") @mock.patch("certbot_apache._internal.configurator.ApacheConfigurator._autohsts_increase") @@ -88,7 +88,7 @@ class AutoHSTSTest(util.ApacheTest): self.config.update_autohsts(mock.MagicMock()) # Freq not patched, so value shouldn't increase - self.assertFalse(mock_increase.called) + self.assertIs(mock_increase.called, False) @mock.patch("certbot_apache._internal.configurator.ApacheConfigurator.restart") @@ -135,13 +135,13 @@ class AutoHSTSTest(util.ApacheTest): # Time mock is used to make sure that the execution does not # continue when no autohsts entries exist in pluginstorage self.config.update_autohsts(mock.MagicMock()) - self.assertFalse(mock_time.called) + self.assertIs(mock_time.called, False) def test_autohsts_make_permanent_noop(self): self.config.storage.put = mock.MagicMock() self.config.deploy_autohsts(mock.MagicMock()) # Make sure that the execution does not continue when no entries in store - self.assertFalse(self.config.storage.put.called) + self.assertIs(self.config.storage.put.called, False) @mock.patch("certbot_apache._internal.display_ops.select_vhost") def test_autohsts_no_ssl_vhost(self, mock_select): @@ -150,15 +150,13 @@ class AutoHSTSTest(util.ApacheTest): self.assertRaises(errors.PluginError, self.config.enable_autohsts, mock.MagicMock(), "invalid.example.com") - self.assertTrue( - "Certbot was not able to find SSL" in mock_log.call_args[0][0]) + self.assertIn("Certbot was not able to find SSL", mock_log.call_args[0][0]) @mock.patch("certbot_apache._internal.configurator.ApacheConfigurator.restart") @mock.patch("certbot_apache._internal.configurator.ApacheConfigurator.add_vhost_id") def test_autohsts_dont_enhance_twice(self, mock_id, _restart): mock_id.return_value = "1234567" - self.config.enable_autohsts(mock.MagicMock(), - ["ocspvhost.com", "ocspvhost.com"]) + self.config.enable_autohsts(mock.MagicMock(), ["ocspvhost.com", "ocspvhost.com"]) self.assertEqual(mock_id.call_count, 1) def test_autohsts_remove_orphaned(self): @@ -168,7 +166,7 @@ class AutoHSTSTest(util.ApacheTest): self.config._autohsts_save_state() self.config.update_autohsts(mock.MagicMock()) - self.assertFalse("orphan_id" in self.config._autohsts) + self.assertNotIn("orphan_id", self.config._autohsts) # Make sure it's removed from the pluginstorage file as well self.config._autohsts = None self.config._autohsts_fetch_state() @@ -181,9 +179,8 @@ class AutoHSTSTest(util.ApacheTest): self.config._autohsts_save_state() 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( - "VirtualHost with id orphan_id was not" in mock_log.call_args[0][0]) + self.assertIs(mock_log.called, True) + self.assertIn("VirtualHost with id orphan_id was not", mock_log.call_args[0][0]) if __name__ == "__main__": diff --git a/certbot-apache/tests/centos6_test.py b/certbot-apache/tests/centos6_test.py index 017e2f09f..85f1333e9 100644 --- a/certbot-apache/tests/centos6_test.py +++ b/certbot-apache/tests/centos6_test.py @@ -48,8 +48,7 @@ class CentOS6Tests(util.ApacheTest): self.temp_dir, "centos6_apache/apache") def test_get_parser(self): - self.assertTrue(isinstance(self.config.parser, - override_centos.CentOSParser)) + self.assertIsInstance(self.config.parser, override_centos.CentOSParser) def test_get_virtual_hosts(self): """Make sure all vhosts are being properly found.""" @@ -72,9 +71,9 @@ class CentOS6Tests(util.ApacheTest): "LoadModule", "ssl_module", exclude=False) self.assertEqual(len(ssl_loadmods), 1) # Make sure the LoadModule ssl_module is in ssl.conf (default) - self.assertTrue("ssl.conf" in ssl_loadmods[0]) + self.assertIn("ssl.conf", ssl_loadmods[0]) # ...and that it's not inside of - self.assertFalse("IfModule" in ssl_loadmods[0]) + self.assertNotIn("IfModule", ssl_loadmods[0]) # Get the example vhost self.config.assoc["test.example.com"] = self.vh_truth[0] @@ -95,7 +94,7 @@ class CentOS6Tests(util.ApacheTest): # ...and both of them should be wrapped in # lm[:-17] strips off /directive/arg[1] from the path. ifmod_args = self.config.parser.get_all_args(lm[:-17]) - self.assertTrue("!mod_ssl.c" in ifmod_args) + self.assertIn("!mod_ssl.c", ifmod_args) @mock.patch("certbot_apache._internal.configurator.display_util.notify") def test_loadmod_multiple(self, unused_mock_notify): @@ -107,7 +106,7 @@ class CentOS6Tests(util.ApacheTest): pre_loadmods = self.config.parser.find_dir( "LoadModule", "ssl_module", exclude=False) # LoadModules are not within IfModule blocks - self.assertFalse(any("ifmodule" in m.lower() for m in pre_loadmods)) + self.assertIs(any("ifmodule" in m.lower() for m in pre_loadmods), False) self.config.assoc["test.example.com"] = self.vh_truth[0] self.config.deploy_cert( "random.demo", "example/cert.pem", "example/key.pem", @@ -116,7 +115,9 @@ class CentOS6Tests(util.ApacheTest): "LoadModule", "ssl_module", exclude=False) for mod in post_loadmods: - self.assertTrue(self.config.parser.not_modssl_ifmodule(mod)) #pylint: disable=no-member + with self.subTest(mod=mod): + # pylint: disable=no-member + self.assertIs(self.config.parser.not_modssl_ifmodule(mod), True) @mock.patch("certbot_apache._internal.configurator.display_util.notify") def test_loadmod_rootconf_exists(self, unused_mock_notify): @@ -207,20 +208,20 @@ class CentOS6Tests(util.ApacheTest): post_loadmods = self.config.parser.find_dir("LoadModule", "ssl_module", exclude=False) - self.assertFalse(post_loadmods) + self.assertEqual(post_loadmods, []) def test_no_ifmod_search_false(self): #pylint: disable=no-member - self.assertFalse(self.config.parser.not_modssl_ifmodule( + self.assertIs(self.config.parser.not_modssl_ifmodule( "/path/does/not/include/ifmod" - )) - self.assertFalse(self.config.parser.not_modssl_ifmodule( + ), False) + self.assertIs(self.config.parser.not_modssl_ifmodule( "" - )) - self.assertFalse(self.config.parser.not_modssl_ifmodule( + ), False) + self.assertIs(self.config.parser.not_modssl_ifmodule( "/path/includes/IfModule/but/no/arguments" - )) + ), False) if __name__ == "__main__": diff --git a/certbot-apache/tests/centos_test.py b/certbot-apache/tests/centos_test.py index a92c37979..c9a820466 100644 --- a/certbot-apache/tests/centos_test.py +++ b/certbot-apache/tests/centos_test.py @@ -34,6 +34,7 @@ def get_vh_truth(temp_dir, config_name): ] return vh_truth + class FedoraRestartTest(util.ApacheTest): """Tests for Fedora specific self-signed certificate override""" @@ -140,8 +141,8 @@ class MultipleVhostsTestCentOS(util.ApacheTest): self.assertEqual(mock_get.call_count, 3) self.assertEqual(len(self.config.parser.modules), 4) self.assertEqual(len(self.config.parser.variables), 2) - self.assertTrue("TEST2" in self.config.parser.variables) - self.assertTrue("mod_another.c" in self.config.parser.modules) + self.assertIn("TEST2", self.config.parser.variables) + self.assertIn("mod_another.c", self.config.parser.modules) def test_get_virtual_hosts(self): """Make sure all vhosts are being properly found.""" @@ -172,11 +173,11 @@ class MultipleVhostsTestCentOS(util.ApacheTest): mock_osi.return_value = ("centos", "7") self.config.parser.update_runtime_variables() - self.assertTrue("mock_define" in self.config.parser.variables) - self.assertTrue("mock_define_too" in self.config.parser.variables) - self.assertTrue("mock_value" in self.config.parser.variables) + self.assertIn("mock_define", self.config.parser.variables) + self.assertIn("mock_define_too", self.config.parser.variables) + self.assertIn("mock_value", self.config.parser.variables) self.assertEqual("TRUE", self.config.parser.variables["mock_value"]) - self.assertTrue("MOCK_NOSEP" in self.config.parser.variables) + self.assertIn("MOCK_NOSEP", self.config.parser.variables) self.assertEqual("NOSEP_VAL", self.config.parser.variables["NOSEP_TWO"]) @mock.patch("certbot_apache._internal.configurator.util.run_script") diff --git a/certbot-apache/tests/complex_parsing_test.py b/certbot-apache/tests/complex_parsing_test.py index e36bd85d1..973af302a 100644 --- a/certbot-apache/tests/complex_parsing_test.py +++ b/certbot-apache/tests/complex_parsing_test.py @@ -11,8 +11,7 @@ class ComplexParserTest(util.ParserTest): """Apache Parser Test.""" def setUp(self): # pylint: disable=arguments-differ - super().setUp( - "complex_parsing", "complex_parsing") + super().setUp("complex_parsing", "complex_parsing") self.setup_variables() # This needs to happen after due to setup_variables not being run @@ -78,12 +77,12 @@ class ComplexParserTest(util.ParserTest): def test_load_modules(self): """If only first is found, there is bad variable parsing.""" - self.assertTrue("status_module" in self.parser.modules) - self.assertTrue("mod_status.c" in self.parser.modules) + self.assertIn("status_module", self.parser.modules) + self.assertIn("mod_status.c", self.parser.modules) # This is in an IfDefine - self.assertTrue("ssl_module" in self.parser.modules) - self.assertTrue("mod_ssl.c" in self.parser.modules) + self.assertIn("ssl_module", self.parser.modules) + self.assertIn("mod_ssl.c", self.parser.modules) def verify_fnmatch(self, arg, hit=True): """Test if Include was correctly parsed.""" diff --git a/certbot-apache/tests/configurator_reverter_test.py b/certbot-apache/tests/configurator_reverter_test.py index d8f5ddd05..72b8fe2bd 100644 --- a/certbot-apache/tests/configurator_reverter_test.py +++ b/certbot-apache/tests/configurator_reverter_test.py @@ -14,15 +14,13 @@ import util class ConfiguratorReverterTest(util.ApacheTest): """Test for ApacheConfigurator reverter methods""" - def setUp(self): # pylint: disable=arguments-differ super().setUp() self.config = util.get_apache_configurator( self.config_path, self.vhost_path, self.config_dir, self.work_dir) - self.vh_truth = util.get_vh_truth( - self.temp_dir, "debian_apache_2_4/multiple_vhosts") + self.vh_truth = util.get_vh_truth(self.temp_dir, "debian_apache_2_4/multiple_vhosts") def tearDown(self): shutil.rmtree(self.config_dir) @@ -30,17 +28,13 @@ class ConfiguratorReverterTest(util.ApacheTest): shutil.rmtree(self.temp_dir) def test_bad_save_checkpoint(self): - self.config.reverter.add_to_checkpoint = mock.Mock( - side_effect=errors.ReverterError) - self.config.parser.add_dir( - self.vh_truth[0].path, "Test", "bad_save_ckpt") + self.config.reverter.add_to_checkpoint = mock.Mock(side_effect=errors.ReverterError) + self.config.parser.add_dir(self.vh_truth[0].path, "Test", "bad_save_ckpt") self.assertRaises(errors.PluginError, self.config.save) def test_bad_save_finalize_checkpoint(self): - self.config.reverter.finalize_checkpoint = mock.Mock( - side_effect=errors.ReverterError) - self.config.parser.add_dir( - self.vh_truth[0].path, "Test", "bad_save_ckpt") + self.config.reverter.finalize_checkpoint = mock.Mock(side_effect=errors.ReverterError) + self.config.parser.add_dir(self.vh_truth[0].path, "Test", "bad_save_ckpt") self.assertRaises(errors.PluginError, self.config.save, "Title") def test_finalize_save(self): @@ -72,8 +66,7 @@ class ConfiguratorReverterTest(util.ApacheTest): self.assertEqual(mock_load.call_count, 1) def test_rollback_error(self): - self.config.reverter.rollback_checkpoints = mock.Mock( - side_effect=errors.ReverterError) + self.config.reverter.rollback_checkpoints = mock.Mock(side_effect=errors.ReverterError) self.assertRaises(errors.PluginError, self.config.rollback_checkpoints) def test_recovery_routine_reload(self): diff --git a/certbot-apache/tests/configurator_test.py b/certbot-apache/tests/configurator_test.py index 84f9e2053..f9d9ac1fe 100644 --- a/certbot-apache/tests/configurator_test.py +++ b/certbot-apache/tests/configurator_test.py @@ -83,8 +83,8 @@ class MultipleVhostsTest(util.ApacheTest): self.config.prepare() except errors.PluginError as err: err_msg = str(err) - self.assertTrue("lock" in err_msg) - self.assertTrue(self.config.conf("server-root") in err_msg) + self.assertIn("lock", err_msg) + self.assertIn(self.config.conf("server-root"), err_msg) else: # pragma: no cover self.fail("Exception wasn't raised!") @@ -116,7 +116,8 @@ class MultipleVhostsTest(util.ApacheTest): # Make sure that all (and only) the expected values exist self.assertEqual(len(mock_add.call_args_list), len(found)) for e in exp: - self.assertTrue(e in found) + with self.subTest(e=e): + self.assertIn(e, found) del os.environ["CERTBOT_DOCS"] @@ -130,11 +131,10 @@ class MultipleVhostsTest(util.ApacheTest): from certbot_apache._internal.configurator import ApacheConfigurator parameters = set(ApacheConfigurator.OS_DEFAULTS.__dict__.keys()) for cls in OVERRIDE_CLASSES.values(): - self.assertTrue(parameters.issubset(set(cls.OS_DEFAULTS.__dict__.keys()))) + self.assertIs(parameters.issubset(set(cls.OS_DEFAULTS.__dict__.keys())), True) def test_constant(self): - self.assertTrue("debian_apache_2_4/multiple_vhosts/apache" in - self.config.options.server_root) + self.assertIn("debian_apache_2_4/multiple_vhosts/apache", self.config.options.server_root) @certbot_util.patch_display_util() def test_get_all_names(self, mock_getutility): @@ -162,9 +162,9 @@ class MultipleVhostsTest(util.ApacheTest): names = self.config.get_all_names() self.assertEqual(len(names), 9) - self.assertTrue("zombo.com" in names) - self.assertTrue("google.com" in names) - self.assertTrue("certbot.demo" in names) + self.assertIn("zombo.com", names) + self.assertIn("google.com", names) + self.assertIn("certbot.demo", names) def test_get_bad_path(self): self.assertEqual(apache_util.get_file_path(None), None) @@ -188,16 +188,14 @@ class MultipleVhostsTest(util.ApacheTest): True, False) # pylint: disable=protected-access self.config._add_servernames(ssl_vh1) - self.assertTrue( - self.config._add_servername_alias("oy_vey", ssl_vh1) is None) + self.assertIsNone(self.config._add_servername_alias("oy_vey", ssl_vh1)) def test_add_servernames_alias(self): self.config.parser.add_dir( self.vh_truth[2].path, "ServerAlias", ["*.le.co"]) # pylint: disable=protected-access self.config._add_servernames(self.vh_truth[2]) - self.assertEqual( - self.vh_truth[2].get_names(), {"*.le.co", "ip-172-30-0-17"}) + self.assertEqual(self.vh_truth[2].get_names(), {"*.le.co", "ip-172-30-0-17"}) def test_get_virtual_hosts(self): """Make sure all vhosts are being properly found.""" @@ -246,8 +244,8 @@ class MultipleVhostsTest(util.ApacheTest): self.vh_truth[0].get_names(), chosen_vhost.get_names()) # Make sure we go from HTTP -> HTTPS - self.assertFalse(self.vh_truth[0].ssl) - self.assertTrue(chosen_vhost.ssl) + self.assertIs(self.vh_truth[0].ssl, False) + self.assertIs(chosen_vhost.ssl, True) @mock.patch("certbot_apache._internal.configurator.ApacheConfigurator._find_best_vhost") @mock.patch("certbot_apache._internal.parser.ApacheParser.add_dir") @@ -256,7 +254,7 @@ class MultipleVhostsTest(util.ApacheTest): ret_vh.enabled = False mock_find.return_value = self.vh_truth[8] self.config.choose_vhost("whatever.com") - self.assertTrue(mock_add.called) + self.assertIs(mock_add.called, True) @mock.patch("certbot_apache._internal.display_ops.select_vhost") def test_choose_vhost_select_vhost_with_temp(self, mock_select): @@ -291,23 +289,17 @@ class MultipleVhostsTest(util.ApacheTest): def test_findbest_continues_on_short_domain(self): # pylint: disable=protected-access - chosen_vhost = self.config._find_best_vhost("purple.com") - self.assertEqual(None, chosen_vhost) + self.assertIsNone(self.config._find_best_vhost("purple.com")) def test_findbest_continues_on_long_domain(self): # pylint: disable=protected-access - chosen_vhost = self.config._find_best_vhost("green.red.purple.com") - self.assertEqual(None, chosen_vhost) + self.assertIsNone(self.config._find_best_vhost("green.red.purple.com")) def test_find_best_vhost(self): # pylint: disable=protected-access - self.assertEqual( - self.vh_truth[3], self.config._find_best_vhost("certbot.demo")) - self.assertEqual( - self.vh_truth[0], - self.config._find_best_vhost("encryption-example.demo")) - self.assertEqual( - self.config._find_best_vhost("does-not-exist.com"), None) + self.assertEqual(self.vh_truth[3], self.config._find_best_vhost("certbot.demo")) + self.assertEqual(self.vh_truth[0], self.config._find_best_vhost("encryption-example.demo")) + self.assertEqual(self.config._find_best_vhost("does-not-exist.com"), None) def test_find_best_vhost_variety(self): # pylint: disable=protected-access @@ -345,11 +337,11 @@ class MultipleVhostsTest(util.ApacheTest): self.config.parser.modules["mod_ssl.c"] = None self.config.parser.modules["socache_shmcb_module"] = None - self.assertFalse(ssl_vhost.enabled) + self.assertIs(ssl_vhost.enabled, False) 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) + self.assertIs(ssl_vhost.enabled, True) def test_no_duplicate_include(self): def mock_find_dir(directive, argument, _): @@ -366,7 +358,7 @@ class MultipleVhostsTest(util.ApacheTest): if a[0][1] == "Include" and a[0][2] == self.config.mod_ssl_conf: tried_to_add = True # Include should be added, find_dir is not patched, and returns falsy - self.assertTrue(tried_to_add) + self.assertIs(tried_to_add, True) self.config.parser.find_dir = mock_find_dir mock_add.reset_mock() @@ -395,20 +387,16 @@ class MultipleVhostsTest(util.ApacheTest): f_args.append(self.config.parser.get_arg(d)) return f_args # Verify that the dummy directives do not exist - self.assertFalse( - "insert_cert_file_path" in find_args(vhostpath, - "SSLCertificateFile")) - self.assertFalse( - "insert_key_file_path" in find_args(vhostpath, - "SSLCertificateKeyFile")) + self.assertNotIn( + "insert_cert_file_path", find_args(vhostpath, "SSLCertificateFile")) + self.assertNotIn( + "insert_key_file_path", find_args(vhostpath, "SSLCertificateKeyFile")) orig_add_dummy(vhostpath) # Verify that the dummy directives exist - self.assertTrue( - "insert_cert_file_path" in find_args(vhostpath, - "SSLCertificateFile")) - self.assertTrue( - "insert_key_file_path" in find_args(vhostpath, - "SSLCertificateKeyFile")) + self.assertIn( + "insert_cert_file_path", find_args(vhostpath, "SSLCertificateFile")) + self.assertIn( + "insert_key_file_path", find_args(vhostpath, "SSLCertificateKeyFile")) # pylint: disable=protected-access self.config._add_dummy_ssl_directives = mock_add_dummy_ssl @@ -420,8 +408,8 @@ class MultipleVhostsTest(util.ApacheTest): self.config.save() # Verify ssl_module was enabled. - self.assertTrue(self.vh_truth[1].enabled) - self.assertTrue("ssl_module" in self.config.parser.modules) + self.assertIs(self.vh_truth[1].enabled, True) + self.assertIn("ssl_module", self.config.parser.modules) loc_cert = self.config.parser.find_dir( "sslcertificatefile", "example/cert.pem", self.vh_truth[1].path) @@ -457,17 +445,15 @@ class MultipleVhostsTest(util.ApacheTest): def test_is_name_vhost(self): addr = obj.Addr.fromstring("*:80") - self.assertTrue(self.config.is_name_vhost(addr)) + self.assertIs(self.config.is_name_vhost(addr), True) self.config.version = (2, 2) - self.assertFalse(self.config.is_name_vhost(addr)) + self.assertEqual(self.config.is_name_vhost(addr), []) def test_add_name_vhost(self): self.config.add_name_vhost(obj.Addr.fromstring("*:443")) self.config.add_name_vhost(obj.Addr.fromstring("*:80")) - self.assertTrue(self.config.parser.find_dir( - "NameVirtualHost", "*:443", exclude=False)) - self.assertTrue(self.config.parser.find_dir( - "NameVirtualHost", "*:80")) + self.assertTrue(self.config.parser.find_dir("NameVirtualHost", "*:443", exclude=False)) + self.assertTrue(self.config.parser.find_dir("NameVirtualHost", "*:80")) def test_add_listen_80(self): mock_find = mock.Mock() @@ -476,8 +462,8 @@ class MultipleVhostsTest(util.ApacheTest): self.config.parser.find_dir = mock_find self.config.parser.add_dir = mock_add_dir self.config.ensure_listen("80") - self.assertTrue(mock_add_dir.called) - self.assertTrue(mock_find.called) + self.assertIs(mock_add_dir.called, True) + self.assertIs(mock_find.called, True) self.assertEqual(mock_add_dir.call_args[0][1], "Listen") self.assertEqual(mock_add_dir.call_args[0][2], "80") @@ -502,13 +488,13 @@ class MultipleVhostsTest(util.ApacheTest): # Test self.config.ensure_listen("8080") self.assertEqual(mock_add_dir.call_count, 3) - self.assertTrue(mock_add_dir.called) + self.assertIs(mock_add_dir.called, True) self.assertEqual(mock_add_dir.call_args[0][1], "Listen") call_found = False for mock_call in mock_add_dir.mock_calls: if mock_call[1][2] == ['1.2.3.4:8080']: call_found = True - self.assertTrue(call_found) + self.assertIs(call_found, True) @mock.patch("certbot_apache._internal.parser.ApacheParser.reset_modules") def test_prepare_server_https(self, mock_reset): @@ -631,8 +617,8 @@ class MultipleVhostsTest(util.ApacheTest): def test_make_vhost_ssl_nonsymlink(self): ssl_vhost_slink = self.config.make_vhost_ssl(self.vh_truth[8]) - self.assertTrue(ssl_vhost_slink.ssl) - self.assertTrue(ssl_vhost_slink.enabled) + self.assertIs(ssl_vhost_slink.ssl, True) + self.assertIs(ssl_vhost_slink.enabled, True) self.assertEqual(ssl_vhost_slink.name, "nonsym.link") def test_make_vhost_ssl_nonexistent_vhost_path(self): @@ -653,8 +639,8 @@ class MultipleVhostsTest(util.ApacheTest): self.assertEqual(len(ssl_vhost.addrs), 1) self.assertEqual({obj.Addr.fromstring("*:443")}, ssl_vhost.addrs) self.assertEqual(ssl_vhost.name, "encryption-example.demo") - self.assertTrue(ssl_vhost.ssl) - self.assertFalse(ssl_vhost.enabled) + self.assertIs(ssl_vhost.ssl, True) + self.assertIs(ssl_vhost.enabled, False) self.assertEqual(self.config.is_name_vhost(self.vh_truth[0]), self.config.is_name_vhost(ssl_vhost)) @@ -733,15 +719,14 @@ class MultipleVhostsTest(util.ApacheTest): def test_get_ssl_vhost_path(self): # pylint: disable=protected-access - self.assertTrue( - self.config._get_ssl_vhost_path("example_path").endswith(".conf")) + self.assertIs(self.config._get_ssl_vhost_path("example_path").endswith(".conf"), True) def test_add_name_vhost_if_necessary(self): # pylint: disable=protected-access self.config.add_name_vhost = mock.Mock() self.config.version = (2, 2) self.config._add_name_vhost_if_necessary(self.vh_truth[0]) - self.assertTrue(self.config.add_name_vhost.called) + self.assertIs(self.config.add_name_vhost.called, True) new_addrs = set() for addr in self.vh_truth[0].addrs: @@ -780,9 +765,9 @@ class MultipleVhostsTest(util.ApacheTest): for i, achall in enumerate(achalls): self.config.cleanup([achall]) if i == len(achalls) - 1: - self.assertTrue(mock_restart.called) + self.assertIs(mock_restart.called, True) else: - self.assertFalse(mock_restart.called) + self.assertIs(mock_restart.called, False) @mock.patch("certbot_apache._internal.configurator.ApacheConfigurator.restart") @mock.patch("certbot_apache._internal.apache_util._get_runtime_cfg") @@ -795,10 +780,10 @@ class MultipleVhostsTest(util.ApacheTest): self.config._chall_out.add(achall) # pylint: disable=protected-access self.config.cleanup([achalls[-1]]) - self.assertFalse(mock_restart.called) + self.assertIs(mock_restart.called, False) self.config.cleanup(achalls) - self.assertTrue(mock_restart.called) + self.assertIs(mock_restart.called, True) @mock.patch("certbot.util.run_script") def test_get_version(self, mock_script): @@ -847,18 +832,18 @@ class MultipleVhostsTest(util.ApacheTest): self.assertTrue(self.config.more_info()) def test_get_chall_pref(self): - self.assertTrue(isinstance(self.config.get_chall_pref(""), list)) + self.assertIsInstance(self.config.get_chall_pref(""), list) def test_install_ssl_options_conf(self): path = os.path.join(self.work_dir, "test_it") other_path = os.path.join(self.work_dir, "other_test_it") self.config.install_ssl_options_conf(path, other_path) - self.assertTrue(os.path.isfile(path)) - self.assertTrue(os.path.isfile(other_path)) + self.assertIs(os.path.isfile(path), True) + self.assertIs(os.path.isfile(other_path), True) # TEST ENHANCEMENTS def test_supported_enhancements(self): - self.assertTrue(isinstance(self.config.supported_enhancements(), list)) + self.assertIsInstance(self.config.supported_enhancements(), list) def test_find_http_vhost_without_ancestor(self): # pylint: disable=protected-access @@ -897,16 +882,16 @@ class MultipleVhostsTest(util.ApacheTest): self.assertRaises(errors.PluginError, self.config.enhance, "certbot.demo", "redirect") # Check that correct logger.warning was printed - self.assertTrue("not able to find" in mock_log.call_args[0][0]) - self.assertTrue("\"redirect\"" in mock_log.call_args[0][0]) + self.assertIn("not able to find", mock_log.call_args[0][0]) + self.assertIn("\"redirect\"", mock_log.call_args[0][0]) mock_log.reset_mock() self.assertRaises(errors.PluginError, self.config.enhance, "certbot.demo", "ensure-http-header", "Test") # Check that correct logger.warning was printed - self.assertTrue("not able to find" in mock_log.call_args[0][0]) - self.assertTrue("Test" in mock_log.call_args[0][0]) + self.assertIn("not able to find", mock_log.call_args[0][0]) + self.assertIn("Test", mock_log.call_args[0][0]) @mock.patch("certbot.util.exe_exists") def test_ocsp_stapling(self, mock_exe): @@ -984,7 +969,7 @@ class MultipleVhostsTest(util.ApacheTest): # pylint: disable=protected-access http_vh = self.config._get_http_vhost(ssl_vh) - self.assertFalse(http_vh.ssl) + self.assertIs(http_vh.ssl, False) @mock.patch("certbot.util.run_script") @mock.patch("certbot.util.exe_exists") @@ -1039,7 +1024,7 @@ class MultipleVhostsTest(util.ApacheTest): self.config.enhance("certbot.demo", "ensure-http-header", "Upgrade-Insecure-Requests") - self.assertTrue("headers_module" in self.config.parser.modules) + self.assertIn("headers_module", self.config.parser.modules) # Get the ssl vhost for certbot.demo ssl_vhost = self.config.assoc["certbot.demo"] @@ -1091,8 +1076,8 @@ class MultipleVhostsTest(util.ApacheTest): self.assertEqual(len(rw_rule), 3) # [:-3] to remove the vhost index number - self.assertTrue(rw_engine[0].startswith(self.vh_truth[3].path[:-3])) - self.assertTrue(rw_rule[0].startswith(self.vh_truth[3].path[:-3])) + self.assertIs(rw_engine[0].startswith(self.vh_truth[3].path[:-3]), True) + self.assertIs(rw_rule[0].startswith(self.vh_truth[3].path[:-3]), True) def test_rewrite_rule_exists(self): # Skip the enable mod @@ -1101,7 +1086,7 @@ class MultipleVhostsTest(util.ApacheTest): self.config.parser.add_dir( self.vh_truth[3].path, "RewriteRule", ["Unknown"]) # pylint: disable=protected-access - self.assertTrue(self.config._is_rewrite_exists(self.vh_truth[3])) + self.assertIs(self.config._is_rewrite_exists(self.vh_truth[3]), True) def test_rewrite_engine_exists(self): # Skip the enable mod @@ -1141,10 +1126,10 @@ class MultipleVhostsTest(util.ApacheTest): # three args to rw_rule + 1 arg for the pre existing rewrite self.assertEqual(len(rw_rule), 5) # [:-3] to remove the vhost index number - self.assertTrue(rw_engine[0].startswith(self.vh_truth[3].path[:-3])) - self.assertTrue(rw_rule[0].startswith(self.vh_truth[3].path[:-3])) + self.assertIs(rw_engine[0].startswith(self.vh_truth[3].path[:-3]), True) + self.assertIs(rw_rule[0].startswith(self.vh_truth[3].path[:-3]), True) - self.assertTrue("rewrite_module" in self.config.parser.modules) + self.assertIn("rewrite_module", self.config.parser.modules) @mock.patch("certbot.util.run_script") @mock.patch("certbot.util.exe_exists") @@ -1202,7 +1187,7 @@ class MultipleVhostsTest(util.ApacheTest): "ApacheConfigurator._verify_no_certbot_redirect") with mock.patch(verify_no_redirect) as mock_verify: self.config.enhance("green.blue.purple.com", "redirect") - self.assertFalse(mock_verify.called) + self.assertIs(mock_verify.called, False) def test_redirect_from_previous_run(self): # Skip the enable mod @@ -1243,16 +1228,16 @@ class MultipleVhostsTest(util.ApacheTest): def test_sift_rewrite_rule(self): # pylint: disable=protected-access small_quoted_target = "RewriteRule ^ \"http://\"" - self.assertFalse(self.config._sift_rewrite_rule(small_quoted_target)) + self.assertIs(self.config._sift_rewrite_rule(small_quoted_target), False) https_target = "RewriteRule ^ https://satoshi" - self.assertTrue(self.config._sift_rewrite_rule(https_target)) + self.assertIs(self.config._sift_rewrite_rule(https_target), True) normal_target = "RewriteRule ^/(.*) http://www.a.com:1234/$1 [L,R]" - self.assertFalse(self.config._sift_rewrite_rule(normal_target)) + self.assertIs(self.config._sift_rewrite_rule(normal_target), False) not_rewriterule = "NotRewriteRule ^ ..." - self.assertFalse(self.config._sift_rewrite_rule(not_rewriterule)) + self.assertIs(self.config._sift_rewrite_rule(not_rewriterule), False) def get_key_and_achalls(self): """Return testing achallenges.""" @@ -1281,15 +1266,13 @@ class MultipleVhostsTest(util.ApacheTest): vhost = self.vh_truth[0] vhost.enabled = False vhost.filep = inc_path - self.assertFalse(self.config.parser.find_dir("Include", inc_path)) - self.assertFalse( - os.path.dirname(inc_path) in self.config.parser.existing_paths) + self.assertEqual(self.config.parser.find_dir("Include", inc_path), []) + self.assertNotIn(os.path.dirname(inc_path), self.config.parser.existing_paths) self.config.enable_site(vhost) - self.assertTrue(self.config.parser.find_dir("Include", inc_path)) - self.assertTrue( - os.path.dirname(inc_path) in self.config.parser.existing_paths) - self.assertTrue( - os.path.basename(inc_path) in self.config.parser.existing_paths[ + self.assertGreaterEqual(len(self.config.parser.find_dir("Include", inc_path)), 1) + self.assertIn(os.path.dirname(inc_path), self.config.parser.existing_paths) + self.assertIn( + os.path.basename(inc_path), self.config.parser.existing_paths[ os.path.dirname(inc_path)]) @mock.patch('certbot_apache._internal.configurator.display_util.notify') @@ -1312,7 +1295,7 @@ class MultipleVhostsTest(util.ApacheTest): "example/cert.pem", "example/key.pem", "example/cert_chain.pem") # Test that we actually called add_include - self.assertTrue(mock_add.called) + self.assertIs(mock_add.called, True) shutil.rmtree(tmp_path) def test_deploy_cert_no_mod_ssl(self): @@ -1331,7 +1314,7 @@ class MultipleVhostsTest(util.ApacheTest): ret_vh.enabled = True self.config.enable_site(ret_vh) # Make sure that we return early - self.assertFalse(mock_parsed.called) + self.assertIs(mock_parsed.called, False) def test_enable_mod_unsupported(self): self.assertRaises(errors.MisconfigurationError, @@ -1352,7 +1335,7 @@ class MultipleVhostsTest(util.ApacheTest): # And the actual returned values self.assertEqual(len(vhs), 1) self.assertEqual(vhs[0].name, "certbot.demo") - self.assertTrue(vhs[0].ssl) + self.assertIs(vhs[0].ssl, True) self.assertNotEqual(vhs[0], self.vh_truth[3]) @@ -1364,7 +1347,7 @@ class MultipleVhostsTest(util.ApacheTest): mock_select_vhs.return_value = [self.vh_truth[1]] vhs = self.config._choose_vhosts_wildcard("*.certbot.demo", create_ssl=False) - self.assertFalse(mock_makessl.called) + self.assertIs(mock_makessl.called, False) self.assertEqual(vhs[0], self.vh_truth[1]) @mock.patch("certbot_apache._internal.configurator.ApacheConfigurator._vhosts_for_wildcard") @@ -1381,14 +1364,13 @@ class MultipleVhostsTest(util.ApacheTest): self.assertEqual(mock_select_vhs.call_args[0][0][0], self.vh_truth[7]) self.assertEqual(len(mock_select_vhs.call_args_list), 1) # Ensure that make_vhost_ssl was not called, vhost.ssl == true - self.assertFalse(mock_makessl.called) + self.assertIs(mock_makessl.called, False) # And the actual returned values self.assertEqual(len(vhs), 1) - self.assertTrue(vhs[0].ssl) + self.assertIs(vhs[0].ssl, True) self.assertEqual(vhs[0], self.vh_truth[7]) - @mock.patch('certbot_apache._internal.configurator.display_util.notify') def test_deploy_cert_wildcard(self, unused_mock_notify): # pylint: disable=protected-access @@ -1399,7 +1381,7 @@ class MultipleVhostsTest(util.ApacheTest): with mock.patch(mock_d) as mock_dep: self.config.deploy_cert("*.wildcard.example.org", "/tmp/path", "/tmp/path", "/tmp/path", "/tmp/path") - self.assertTrue(mock_dep.called) + self.assertIs(mock_dep.called, True) self.assertEqual(len(mock_dep.call_args_list), 1) self.assertEqual(self.vh_truth[7], mock_dep.call_args_list[0][0][0]) @@ -1421,7 +1403,7 @@ class MultipleVhostsTest(util.ApacheTest): self.config._wildcard_vhosts["*.certbot.demo"] = [self.vh_truth[3]] self.config.enhance("*.certbot.demo", "ensure-http-header", "Upgrade-Insecure-Requests") - self.assertFalse(mock_choose.called) + self.assertIs(mock_choose.called, False) @mock.patch("certbot_apache._internal.configurator.ApacheConfigurator._choose_vhosts_wildcard") def test_enhance_wildcard_no_install(self, mock_choose): @@ -1431,7 +1413,7 @@ class MultipleVhostsTest(util.ApacheTest): self.config.parser.modules["headers_module"] = None self.config.enhance("*.certbot.demo", "ensure-http-header", "Upgrade-Insecure-Requests") - self.assertTrue(mock_choose.called) + self.assertIs(mock_choose.called, True) def test_add_vhost_id(self): for vh in [self.vh_truth[0], self.vh_truth[1], self.vh_truth[2]]: @@ -1510,7 +1492,8 @@ class AugeasVhostsTest(util.ApacheTest): names = ( "an.example.net", "another.example.net", "an.other.example.net") for name in names: - self.assertFalse(name in self.config.choose_vhost(name).aliases) + with self.subTest(name=name): + self.assertNotIn(name, self.config.choose_vhost(name).aliases) @mock.patch("certbot_apache._internal.obj.VirtualHost.conflicts") def test_choose_vhost_without_matching_wildcard(self, mock_conflicts): @@ -1518,7 +1501,7 @@ class AugeasVhostsTest(util.ApacheTest): mock_path = "certbot_apache._internal.display_ops.select_vhost" with mock.patch(mock_path, lambda _, vhosts: vhosts[0]): for name in ("a.example.net", "other.example.net"): - self.assertTrue(name in self.config.choose_vhost(name).aliases) + self.assertIn(name, self.config.choose_vhost(name).aliases) @mock.patch("certbot_apache._internal.obj.VirtualHost.conflicts") def test_choose_vhost_wildcard_not_found(self, mock_conflicts): @@ -1551,6 +1534,7 @@ class AugeasVhostsTest(util.ApacheTest): self.assertRaises(errors.PluginError, self.config.make_vhost_ssl, broken_vhost) + class MultiVhostsTest(util.ApacheTest): """Test configuration with multiple virtualhosts in a single file.""" # pylint: disable=protected-access @@ -1559,9 +1543,7 @@ 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().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, @@ -1582,9 +1564,8 @@ class MultiVhostsTest(util.ApacheTest): self.assertEqual(len(ssl_vhost.addrs), 1) self.assertEqual({obj.Addr.fromstring("*:443")}, ssl_vhost.addrs) self.assertEqual(ssl_vhost.name, "banana.vomit.com") - self.assertTrue(ssl_vhost.ssl) - self.assertFalse(ssl_vhost.enabled) - + self.assertIs(ssl_vhost.ssl, True) + self.assertIs(ssl_vhost.enabled, False) self.assertEqual(self.config.is_name_vhost(self.vh_truth[1]), self.config.is_name_vhost(ssl_vhost)) @@ -1616,8 +1597,7 @@ class MultiVhostsTest(util.ApacheTest): ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[4]) - self.assertTrue(self.config.parser.find_dir( - "RewriteEngine", "on", ssl_vhost.path, False)) + self.assertTrue(self.config.parser.find_dir("RewriteEngine", "on", ssl_vhost.path, False)) with open(ssl_vhost.filep) as the_file: conf_text = the_file.read() @@ -1625,8 +1605,8 @@ class MultiVhostsTest(util.ApacheTest): "\"https://new.example.com/docs/$1\" [R,L]") uncommented_rewrite_rule = ("RewriteRule \"^/docs/(.+)\" " "\"http://new.example.com/docs/$1\" [R,L]") - self.assertTrue(commented_rewrite_rule in conf_text) - self.assertTrue(uncommented_rewrite_rule in conf_text) + self.assertIn(commented_rewrite_rule, conf_text) + self.assertIn(uncommented_rewrite_rule, conf_text) self.assertEqual(mock_notify.call_count, 1) self.assertIn("Some rewrite rules", mock_notify.call_args[0][0]) @@ -1650,12 +1630,12 @@ class MultiVhostsTest(util.ApacheTest): "https://%{SERVER_NAME}%{REQUEST_URI} " "[L,NE,R=permanent]") - self.assertTrue(not_commented_cond1 in conf_line_set) - self.assertTrue(not_commented_rewrite_rule in conf_line_set) + self.assertIn(not_commented_cond1, conf_line_set) + self.assertIn(not_commented_rewrite_rule, conf_line_set) - 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) + self.assertIn(commented_cond1, conf_line_set) + self.assertIn(commented_cond2, conf_line_set) + self.assertIn(commented_rewrite_rule, conf_line_set) self.assertEqual(mock_notify.call_count, 1) self.assertIn("Some rewrite rules", mock_notify.call_args[0][0]) @@ -1677,7 +1657,7 @@ class InstallSslOptionsConfTest(util.ApacheTest): return crypto_util.sha256sum(self.config.pick_apache_config()) def _assert_current_file(self): - self.assertTrue(os.path.isfile(self.config.mod_ssl_conf)) + self.assertIs(os.path.isfile(self.config.mod_ssl_conf), True) self.assertEqual(crypto_util.sha256sum(self.config.mod_ssl_conf), self._current_ssl_options_hash()) @@ -1685,7 +1665,7 @@ class InstallSslOptionsConfTest(util.ApacheTest): # prepare should have placed a file there self._assert_current_file() os.remove(self.config.mod_ssl_conf) - self.assertFalse(os.path.isfile(self.config.mod_ssl_conf)) + self.assertIs(os.path.isfile(self.config.mod_ssl_conf), False) self._call() self._assert_current_file() @@ -1707,8 +1687,8 @@ class InstallSslOptionsConfTest(util.ApacheTest): mod_ssl_conf.write("a new line for the wrong hash\n") with mock.patch("certbot.plugins.common.logger") as mock_logger: self._call() - self.assertFalse(mock_logger.warning.called) - self.assertTrue(os.path.isfile(self.config.mod_ssl_conf)) + self.assertIs(mock_logger.warning.called, False) + self.assertIs(os.path.isfile(self.config.mod_ssl_conf), True) self.assertEqual(crypto_util.sha256sum( self.config.pick_apache_config()), self._current_ssl_options_hash()) @@ -1731,7 +1711,7 @@ class InstallSslOptionsConfTest(util.ApacheTest): # only print warning once with mock.patch("certbot.plugins.common.logger") as mock_logger: self._call() - self.assertFalse(mock_logger.warning.called) + self.assertIs(mock_logger.warning.called, False) def test_ssl_config_files_hash_in_all_hashes(self): """ @@ -1747,12 +1727,14 @@ class InstallSslOptionsConfTest(util.ApacheTest): "certbot_apache", os.path.join("_internal", "tls_configs")) all_files = [os.path.join(tls_configs_dir, name) for name in os.listdir(tls_configs_dir) if name.endswith('options-ssl-apache.conf')] - self.assertTrue(all_files) + self.assertGreaterEqual(len(all_files), 1) for one_file in all_files: file_hash = crypto_util.sha256sum(one_file) - self.assertTrue(file_hash in ALL_SSL_OPTIONS_HASHES, - "Constants.ALL_SSL_OPTIONS_HASHES must be appended with the sha256 " - "hash of {0} when it is updated.".format(one_file)) + self.assertIn( + file_hash, ALL_SSL_OPTIONS_HASHES, + f"Constants.ALL_SSL_OPTIONS_HASHES must be appended with the sha256 " + f"hash of {one_file} when it is updated." + ) def test_openssl_version(self): self.config._openssl_version = None @@ -1786,14 +1768,14 @@ class InstallSslOptionsConfTest(util.ApacheTest): def test_current_version(self): self.config.version = (2, 4, 10) self.config._openssl_version = '1.0.2m' - self.assertTrue('old' in self.config.pick_apache_config()) + self.assertIn('old', self.config.pick_apache_config()) self.config.version = (2, 4, 11) self.config._openssl_version = '1.0.2m' - self.assertTrue('current' in self.config.pick_apache_config()) + self.assertIn('current', self.config.pick_apache_config()) self.config._openssl_version = '1.0.2a' - self.assertTrue('old' in self.config.pick_apache_config()) + self.assertIn('old', self.config.pick_apache_config()) def test_openssl_version_warns(self): self.config._openssl_version = '1.0.2a' @@ -1802,14 +1784,14 @@ class InstallSslOptionsConfTest(util.ApacheTest): self.config._openssl_version = None with mock.patch("certbot_apache._internal.configurator.logger.warning") as mock_log: self.assertEqual(self.config.openssl_version(), None) - self.assertTrue("Could not find ssl_module" in mock_log.call_args[0][0]) + self.assertIn("Could not find ssl_module", mock_log.call_args[0][0]) # When no ssl_module is present at all self.config._openssl_version = None - self.assertTrue("ssl_module" not in self.config.parser.modules) + self.assertNotIn("ssl_module", self.config.parser.modules) with mock.patch("certbot_apache._internal.configurator.logger.warning") as mock_log: self.assertEqual(self.config.openssl_version(), None) - self.assertTrue("Could not find ssl_module" in mock_log.call_args[0][0]) + self.assertIn("Could not find ssl_module", mock_log.call_args[0][0]) # When ssl_module is statically linked but --apache-bin not provided self.config._openssl_version = None @@ -1817,13 +1799,13 @@ class InstallSslOptionsConfTest(util.ApacheTest): 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) - self.assertTrue("ssl_module is statically linked but" in mock_log.call_args[0][0]) + self.assertIn("ssl_module is statically linked but", mock_log.call_args[0][0]) self.config.parser.modules['ssl_module'] = "/fake/path" with mock.patch("certbot_apache._internal.configurator.logger.warning") as mock_log: # Check that correct logger.warning was printed self.assertEqual(self.config.openssl_version(), None) - self.assertTrue("Unable to read" in mock_log.call_args[0][0]) + self.assertIn("Unable to read", mock_log.call_args[0][0]) contents_missing_openssl = b"these contents won't match the regex" with mock.patch("certbot_apache._internal.configurator." @@ -1832,7 +1814,7 @@ class InstallSslOptionsConfTest(util.ApacheTest): with mock.patch("certbot_apache._internal.configurator.logger.warning") as mock_log: # Check that correct logger.warning was printed self.assertEqual(self.config.openssl_version(), None) - self.assertTrue("Could not find OpenSSL" in mock_log.call_args[0][0]) + self.assertIn("Could not find OpenSSL", mock_log.call_args[0][0]) def test_open_module_file(self): mock_open = mock.mock_open(read_data="testing 12 3") diff --git a/certbot-apache/tests/debian_test.py b/certbot-apache/tests/debian_test.py index 35425223b..2bbf40312 100644 --- a/certbot-apache/tests/debian_test.py +++ b/certbot-apache/tests/debian_test.py @@ -45,8 +45,7 @@ class MultipleVhostsTestDebian(util.ApacheTest): def test_enable_mod_unsupported_dirs(self): shutil.rmtree(os.path.join(self.config.parser.root, "mods-enabled")) - self.assertRaises( - errors.NotSupportedError, self.config.enable_mod, "ssl") + self.assertRaises(errors.NotSupportedError, self.config.enable_mod, "ssl") @mock.patch("certbot.util.run_script") @mock.patch("certbot.util.exe_exists") @@ -58,29 +57,29 @@ class MultipleVhostsTestDebian(util.ApacheTest): mock_exe_exists.return_value = True self.config.enable_mod("ssl") - self.assertTrue("ssl_module" in self.config.parser.modules) - self.assertTrue("mod_ssl.c" in self.config.parser.modules) + self.assertIn("ssl_module", self.config.parser.modules) + self.assertIn("mod_ssl.c", self.config.parser.modules) - self.assertTrue(mock_run_script.called) + self.assertIs(mock_run_script.called, True) def test_deploy_cert_enable_new_vhost(self): # Create ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[0]) self.config.parser.modules["ssl_module"] = None self.config.parser.modules["mod_ssl.c"] = None - self.assertFalse(ssl_vhost.enabled) + self.assertIs(ssl_vhost.enabled, False) 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) + self.assertIs(ssl_vhost.enabled, True) # Make sure that we don't error out if symlink already exists ssl_vhost.enabled = False - self.assertFalse(ssl_vhost.enabled) + self.assertIs(ssl_vhost.enabled, False) 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) + self.assertIs(ssl_vhost.enabled, True) def test_enable_site_failure(self): self.config.parser.root = "/tmp/nonexistent" @@ -110,8 +109,8 @@ class MultipleVhostsTestDebian(util.ApacheTest): self.config.save() # Verify ssl_module was enabled. - self.assertTrue(self.vh_truth[1].enabled) - self.assertTrue("ssl_module" in self.config.parser.modules) + self.assertIs(self.vh_truth[1].enabled, True) + self.assertIn("ssl_module", self.config.parser.modules) loc_cert = self.config.parser.find_dir( "sslcertificatefile", "example/fullchain.pem", @@ -170,7 +169,7 @@ class MultipleVhostsTestDebian(util.ApacheTest): # This will create an ssl vhost for certbot.demo self.config.choose_vhost("certbot.demo") self.config.enhance("certbot.demo", "staple-ocsp") - self.assertTrue("socache_shmcb_module" in self.config.parser.modules) + self.assertIn("socache_shmcb_module", self.config.parser.modules) @mock.patch("certbot.util.run_script") @mock.patch("certbot.util.exe_exists") @@ -183,7 +182,7 @@ class MultipleVhostsTestDebian(util.ApacheTest): self.config.choose_vhost("certbot.demo") self.config.enhance("certbot.demo", "ensure-http-header", "Strict-Transport-Security") - self.assertTrue("headers_module" in self.config.parser.modules) + self.assertIn("headers_module", self.config.parser.modules) @mock.patch("certbot.util.run_script") @mock.patch("certbot.util.exe_exists") @@ -194,10 +193,10 @@ class MultipleVhostsTestDebian(util.ApacheTest): # This will create an ssl vhost for certbot.demo self.config.choose_vhost("certbot.demo") self.config.enhance("certbot.demo", "redirect") - self.assertTrue("rewrite_module" in self.config.parser.modules) + self.assertIn("rewrite_module", self.config.parser.modules) def test_enable_site_already_enabled(self): - self.assertTrue(self.vh_truth[1].enabled) + self.assertIs(self.vh_truth[1].enabled, True) self.config.enable_site(self.vh_truth[1]) def test_enable_site_call_parent(self): @@ -207,7 +206,7 @@ class MultipleVhostsTestDebian(util.ApacheTest): vh = self.vh_truth[0] vh.enabled = False self.config.enable_site(vh) - self.assertTrue(e_s.called) + self.assertIs(e_s.called, True) @mock.patch("certbot.util.exe_exists") def test_enable_mod_no_disable(self, mock_exe_exists): diff --git a/certbot-apache/tests/display_ops_test.py b/certbot-apache/tests/display_ops_test.py index fc3ab821d..50ab6bfc7 100644 --- a/certbot-apache/tests/display_ops_test.py +++ b/certbot-apache/tests/display_ops_test.py @@ -23,7 +23,7 @@ class SelectVhostMultiTest(unittest.TestCase): self.base_dir, "debian_apache_2_4/multiple_vhosts") def test_select_no_input(self): - self.assertFalse(select_vhost_multiple([])) + self.assertEqual(len(select_vhost_multiple([])), 0) @certbot_util.patch_display_util() def test_select_correct(self, mock_util): @@ -33,15 +33,15 @@ class SelectVhostMultiTest(unittest.TestCase): vhs = select_vhost_multiple([self.vhosts[3], self.vhosts[2], self.vhosts[1]]) - self.assertTrue(self.vhosts[2] in vhs) - self.assertTrue(self.vhosts[3] in vhs) - self.assertFalse(self.vhosts[1] in vhs) + self.assertIn(self.vhosts[2], vhs) + self.assertIn(self.vhosts[3], vhs) + self.assertNotIn(self.vhosts[1], vhs) @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) + self.assertEqual(vhs, []) class SelectVhostTest(unittest.TestCase): @@ -68,7 +68,7 @@ class SelectVhostTest(unittest.TestCase): try: self._call(self.vhosts) except errors.MissingCommandlineFlag as e: - self.assertTrue("vhost ambiguity" in str(e)) + self.assertIn("vhost ambiguity", str(e)) @certbot_util.patch_display_util() def test_more_info_cancel(self, mock_util): @@ -76,10 +76,10 @@ class SelectVhostTest(unittest.TestCase): (display_util.CANCEL, -1), ] - self.assertEqual(None, self._call(self.vhosts)) + self.assertIsNone(self._call(self.vhosts)) def test_no_vhosts(self): - self.assertEqual(self._call([]), None) + self.assertIsNone(self._call([])) @mock.patch("certbot_apache._internal.display_ops.display_util") @mock.patch("certbot_apache._internal.display_ops.logger") diff --git a/certbot-apache/tests/dualnode_test.py b/certbot-apache/tests/dualnode_test.py index ef7f8b2d8..83a5729a5 100644 --- a/certbot-apache/tests/dualnode_test.py +++ b/certbot-apache/tests/dualnode_test.py @@ -53,20 +53,20 @@ class DualParserNodeTest(unittest.TestCase): # pylint: disable=too-many-public- primary=self.block.secondary, secondary=self.block.primary) # Switched around - self.assertTrue(cnode.primary is self.comment.secondary) - self.assertTrue(cnode.secondary is self.comment.primary) - self.assertTrue(dnode.primary is self.directive.secondary) - self.assertTrue(dnode.secondary is self.directive.primary) - self.assertTrue(bnode.primary is self.block.secondary) - self.assertTrue(bnode.secondary is self.block.primary) + self.assertEqual(cnode.primary, self.comment.secondary) + self.assertEqual(cnode.secondary, self.comment.primary) + self.assertEqual(dnode.primary, self.directive.secondary) + self.assertEqual(dnode.secondary, self.directive.primary) + self.assertEqual(bnode.primary, self.block.secondary) + self.assertEqual(bnode.secondary, self.block.primary) def test_set_params(self): params = ("first", "second") self.directive.primary.set_parameters = mock.Mock() self.directive.secondary.set_parameters = mock.Mock() self.directive.set_parameters(params) - self.assertTrue(self.directive.primary.set_parameters.called) - self.assertTrue(self.directive.secondary.set_parameters.called) + self.assertIs(self.directive.primary.set_parameters.called, True) + self.assertIs(self.directive.secondary.set_parameters.called, True) def test_set_parameters(self): pparams = mock.MagicMock() @@ -76,8 +76,8 @@ class DualParserNodeTest(unittest.TestCase): # pylint: disable=too-many-public- self.directive.primary.set_parameters = pparams self.directive.secondary.set_parameters = sparams self.directive.set_parameters(("param", "seq")) - self.assertTrue(pparams.called) - self.assertTrue(sparams.called) + self.assertIs(pparams.called, True) + self.assertIs(sparams.called, True) def test_delete_child(self): pdel = mock.MagicMock() @@ -85,8 +85,8 @@ class DualParserNodeTest(unittest.TestCase): # pylint: disable=too-many-public- self.block.primary.delete_child = pdel self.block.secondary.delete_child = sdel self.block.delete_child(self.comment) - self.assertTrue(pdel.called) - self.assertTrue(sdel.called) + self.assertIs(pdel.called, True) + self.assertIs(sdel.called, True) def test_unsaved_files(self): puns = mock.MagicMock() @@ -96,8 +96,8 @@ class DualParserNodeTest(unittest.TestCase): # pylint: disable=too-many-public- self.block.primary.unsaved_files = puns self.block.secondary.unsaved_files = suns self.block.unsaved_files() - self.assertTrue(puns.called) - self.assertTrue(suns.called) + self.assertIs(puns.called, True) + self.assertIs(suns.called, True) def test_getattr_equality(self): self.directive.primary.variableexception = "value" @@ -140,8 +140,8 @@ class DualParserNodeTest(unittest.TestCase): # pylint: disable=too-many-public- self.block.primary.add_child_block = mock_first self.block.secondary.add_child_block = mock_second self.block.add_child_block("Block") - self.assertTrue(mock_first.called) - self.assertTrue(mock_second.called) + self.assertIs(mock_first.called, True) + self.assertIs(mock_second.called, True) def test_add_child_directive(self): mock_first = mock.MagicMock(return_value=self.directive.primary) @@ -149,8 +149,8 @@ class DualParserNodeTest(unittest.TestCase): # pylint: disable=too-many-public- self.block.primary.add_child_directive = mock_first self.block.secondary.add_child_directive = mock_second self.block.add_child_directive("Directive") - self.assertTrue(mock_first.called) - self.assertTrue(mock_second.called) + self.assertIs(mock_first.called, True) + self.assertIs(mock_second.called, True) def test_add_child_comment(self): mock_first = mock.MagicMock(return_value=self.comment.primary) @@ -158,8 +158,8 @@ class DualParserNodeTest(unittest.TestCase): # pylint: disable=too-many-public- self.block.primary.add_child_comment = mock_first self.block.secondary.add_child_comment = mock_second self.block.add_child_comment("Comment") - self.assertTrue(mock_first.called) - self.assertTrue(mock_second.called) + self.assertIs(mock_first.called, True) + self.assertIs(mock_second.called, True) def test_find_comments(self): pri_comments = [augeasparser.AugeasCommentNode(comment="some comment", @@ -183,9 +183,9 @@ class DualParserNodeTest(unittest.TestCase): # pylint: disable=too-many-public- # Check that every comment response is represented in the list of # DualParserNode instances. for p in p_dcoms: - self.assertTrue(p in p_coms) + self.assertIn(p, p_coms) for s in s_dcoms: - self.assertTrue(s in s_coms) + self.assertIn(s, s_coms) def test_find_blocks_first_passing(self): youshallnotpass = [augeasparser.AugeasBlockNode(name="notpassing", @@ -207,8 +207,8 @@ class DualParserNodeTest(unittest.TestCase): # pylint: disable=too-many-public- assertions.assertEqual(block.primary, block.secondary) except AssertionError: # pragma: no cover self.fail("Assertion should have passed") - self.assertTrue(assertions.isPassDirective(block.primary)) - self.assertFalse(assertions.isPassDirective(block.secondary)) + self.assertIs(assertions.isPassDirective(block.primary), True) + self.assertIs(assertions.isPassDirective(block.secondary), False) def test_find_blocks_second_passing(self): youshallnotpass = [augeasparser.AugeasBlockNode(name="notpassing", @@ -230,8 +230,8 @@ class DualParserNodeTest(unittest.TestCase): # pylint: disable=too-many-public- assertions.assertEqual(block.primary, block.secondary) except AssertionError: # pragma: no cover self.fail("Assertion should have passed") - self.assertFalse(assertions.isPassDirective(block.primary)) - self.assertTrue(assertions.isPassDirective(block.secondary)) + self.assertIs(assertions.isPassDirective(block.primary), False) + self.assertIs(assertions.isPassDirective(block.secondary), True) def test_find_dirs_first_passing(self): notpassing = [augeasparser.AugeasDirectiveNode(name="notpassing", @@ -253,8 +253,8 @@ class DualParserNodeTest(unittest.TestCase): # pylint: disable=too-many-public- assertions.assertEqual(directive.primary, directive.secondary) except AssertionError: # pragma: no cover self.fail("Assertion should have passed") - self.assertTrue(assertions.isPassDirective(directive.primary)) - self.assertFalse(assertions.isPassDirective(directive.secondary)) + self.assertIs(assertions.isPassDirective(directive.primary), True) + self.assertIs(assertions.isPassDirective(directive.secondary), False) def test_find_dirs_second_passing(self): notpassing = [augeasparser.AugeasDirectiveNode(name="notpassing", @@ -276,8 +276,8 @@ class DualParserNodeTest(unittest.TestCase): # pylint: disable=too-many-public- assertions.assertEqual(directive.primary, directive.secondary) except AssertionError: # pragma: no cover self.fail("Assertion should have passed") - self.assertFalse(assertions.isPassDirective(directive.primary)) - self.assertTrue(assertions.isPassDirective(directive.secondary)) + self.assertIs(assertions.isPassDirective(directive.primary), False) + self.assertIs(assertions.isPassDirective(directive.secondary), True) def test_find_coms_first_passing(self): notpassing = [augeasparser.AugeasCommentNode(comment="notpassing", @@ -299,8 +299,8 @@ class DualParserNodeTest(unittest.TestCase): # pylint: disable=too-many-public- assertions.assertEqual(comment.primary, comment.secondary) except AssertionError: # pragma: no cover self.fail("Assertion should have passed") - self.assertTrue(assertions.isPassComment(comment.primary)) - self.assertFalse(assertions.isPassComment(comment.secondary)) + self.assertIs(assertions.isPassComment(comment.primary), True) + self.assertIs(assertions.isPassComment(comment.secondary), False) def test_find_coms_second_passing(self): notpassing = [augeasparser.AugeasCommentNode(comment="notpassing", @@ -322,8 +322,8 @@ class DualParserNodeTest(unittest.TestCase): # pylint: disable=too-many-public- assertions.assertEqual(comment.primary, comment.secondary) except AssertionError: # pragma: no cover self.fail("Assertion should have passed") - self.assertFalse(assertions.isPassComment(comment.primary)) - self.assertTrue(assertions.isPassComment(comment.secondary)) + self.assertIs(assertions.isPassComment(comment.primary), False) + self.assertIs(assertions.isPassComment(comment.secondary), True) def test_find_blocks_no_pass_equal(self): notpassing1 = [augeasparser.AugeasBlockNode(name="notpassing", @@ -341,8 +341,9 @@ class DualParserNodeTest(unittest.TestCase): # pylint: disable=too-many-public- blocks = self.block.find_blocks("anything") for block in blocks: - self.assertEqual(block.primary, block.secondary) - self.assertTrue(block.primary is not block.secondary) + with self.subTest(block=block): + self.assertEqual(block.primary, block.secondary) + self.assertIsNot(block.primary, block.secondary) def test_find_dirs_no_pass_equal(self): notpassing1 = [augeasparser.AugeasDirectiveNode(name="notpassing", @@ -360,8 +361,9 @@ class DualParserNodeTest(unittest.TestCase): # pylint: disable=too-many-public- directives = self.block.find_directives("anything") for directive in directives: - self.assertEqual(directive.primary, directive.secondary) - self.assertTrue(directive.primary is not directive.secondary) + with self.subTest(directive=directive): + self.assertEqual(directive.primary, directive.secondary) + self.assertIsNot(directive.primary, directive.secondary) def test_find_comments_no_pass_equal(self): notpassing1 = [augeasparser.AugeasCommentNode(comment="notpassing", @@ -379,8 +381,9 @@ class DualParserNodeTest(unittest.TestCase): # pylint: disable=too-many-public- comments = self.block.find_comments("anything") for comment in comments: - self.assertEqual(comment.primary, comment.secondary) - self.assertTrue(comment.primary is not comment.secondary) + with self.subTest(comment=comment): + self.assertEqual(comment.primary, comment.secondary) + self.assertIsNot(comment.primary, comment.secondary) def test_find_blocks_no_pass_notequal(self): notpassing1 = [augeasparser.AugeasBlockNode(name="notpassing", @@ -424,8 +427,8 @@ class DualParserNodeTest(unittest.TestCase): # pylint: disable=too-many-public- self.block.primary.parsed_paths = mock_p self.block.secondary.parsed_paths = mock_s self.block.parsed_paths() - self.assertTrue(mock_p.called) - self.assertTrue(mock_s.called) + self.assertIs(mock_p.called, True) + self.assertIs(mock_s.called, True) def test_parsed_paths_error(self): mock_p = mock.MagicMock(return_value=['/path/file.conf']) @@ -441,5 +444,5 @@ class DualParserNodeTest(unittest.TestCase): # pylint: disable=too-many-public- self.block.primary.find_ancestors = primarymock self.block.secondary.find_ancestors = secondarymock self.block.find_ancestors("anything") - self.assertTrue(primarymock.called) - self.assertTrue(secondarymock.called) + self.assertIs(primarymock.called, True) + self.assertIs(secondarymock.called, True) diff --git a/certbot-apache/tests/fedora_test.py b/certbot-apache/tests/fedora_test.py index d48559cee..fca3c4ba4 100644 --- a/certbot-apache/tests/fedora_test.py +++ b/certbot-apache/tests/fedora_test.py @@ -134,8 +134,8 @@ class MultipleVhostsTestFedora(util.ApacheTest): self.assertEqual(mock_get.call_count, 3) self.assertEqual(len(self.config.parser.modules), 4) self.assertEqual(len(self.config.parser.variables), 2) - self.assertTrue("TEST2" in self.config.parser.variables) - self.assertTrue("mod_another.c" in self.config.parser.modules) + self.assertIn("TEST2", self.config.parser.variables) + self.assertIn("mod_another.c", self.config.parser.modules) @mock.patch("certbot_apache._internal.configurator.util.run_script") def test_get_version(self, mock_run_script): @@ -172,11 +172,11 @@ class MultipleVhostsTestFedora(util.ApacheTest): mock_osi.return_value = ("fedora", "29") self.config.parser.update_runtime_variables() - self.assertTrue("mock_define" in self.config.parser.variables) - self.assertTrue("mock_define_too" in self.config.parser.variables) - self.assertTrue("mock_value" in self.config.parser.variables) + self.assertIn("mock_define", self.config.parser.variables) + self.assertIn("mock_define_too", self.config.parser.variables) + self.assertIn("mock_value", self.config.parser.variables) self.assertEqual("TRUE", self.config.parser.variables["mock_value"]) - self.assertTrue("MOCK_NOSEP" in self.config.parser.variables) + self.assertIn("MOCK_NOSEP", self.config.parser.variables) self.assertEqual("NOSEP_VAL", self.config.parser.variables["NOSEP_TWO"]) @mock.patch("certbot_apache._internal.configurator.util.run_script") diff --git a/certbot-apache/tests/gentoo_test.py b/certbot-apache/tests/gentoo_test.py index cf39ff5cb..25f9e929b 100644 --- a/certbot-apache/tests/gentoo_test.py +++ b/certbot-apache/tests/gentoo_test.py @@ -63,8 +63,7 @@ class MultipleVhostsTestGentoo(util.ApacheTest): self.temp_dir, "gentoo_apache/apache") def test_get_parser(self): - self.assertTrue(isinstance(self.config.parser, - override_gentoo.GentooParser)) + self.assertIsInstance(self.config.parser, override_gentoo.GentooParser) def test_get_virtual_hosts(self): """Make sure all vhosts are being properly found.""" @@ -91,7 +90,7 @@ class MultipleVhostsTestGentoo(util.ApacheTest): with mock.patch("certbot_apache._internal.override_gentoo.GentooParser.update_modules"): self.config.parser.update_runtime_variables() for define in defines: - self.assertTrue(define in self.config.parser.variables) + self.assertIn(define, self.config.parser.variables) @mock.patch("certbot_apache._internal.apache_util.parse_from_subprocess") def test_no_binary_configdump(self, mock_subprocess): @@ -101,11 +100,11 @@ class MultipleVhostsTestGentoo(util.ApacheTest): with mock.patch("certbot_apache._internal.override_gentoo.GentooParser.update_modules"): self.config.parser.update_runtime_variables() self.config.parser.reset_modules() - self.assertFalse(mock_subprocess.called) + self.assertIs(mock_subprocess.called, False) self.config.parser.update_runtime_variables() self.config.parser.reset_modules() - self.assertTrue(mock_subprocess.called) + self.assertIs(mock_subprocess.called, True) @mock.patch("certbot_apache._internal.apache_util._get_runtime_cfg") def test_opportunistic_httpd_runtime_parsing(self, mock_get): @@ -129,7 +128,7 @@ class MultipleVhostsTestGentoo(util.ApacheTest): self.assertEqual(mock_get.call_count, 1) self.assertEqual(len(self.config.parser.modules), 4) - self.assertTrue("mod_another.c" in self.config.parser.modules) + self.assertIn("mod_another.c", self.config.parser.modules) @mock.patch("certbot_apache._internal.configurator.util.run_script") def test_alt_restart_works(self, mock_run_script): diff --git a/certbot-apache/tests/http_01_test.py b/certbot-apache/tests/http_01_test.py index 1ce47ed1a..9085f68dc 100644 --- a/certbot-apache/tests/http_01_test.py +++ b/certbot-apache/tests/http_01_test.py @@ -51,7 +51,7 @@ class ApacheHttp01Test(util.ApacheTest): self.http = ApacheHttp01(self.config) def test_empty_perform(self): - self.assertFalse(self.http.perform()) + self.assertEqual(len(self.http.perform()), 0) @mock.patch("certbot_apache._internal.configurator.ApacheConfigurator.enable_mod") def test_enable_modules_apache_2_2(self, mock_enmod): @@ -77,7 +77,7 @@ class ApacheHttp01Test(util.ApacheTest): self.http.prepare_http01_modules() - self.assertTrue(mock_enmod.called) + self.assertIs(mock_enmod.called, True) calls = mock_enmod.call_args_list other_calls = [] for call in calls: @@ -186,7 +186,7 @@ class ApacheHttp01Test(util.ApacheTest): def common_perform_test(self, achalls, vhosts): """Tests perform with the given achalls.""" challenge_dir = self.http.challenge_dir - self.assertFalse(os.path.exists(challenge_dir)) + self.assertIs(os.path.exists(challenge_dir), False) for achall in achalls: self.http.add_chall(achall) @@ -194,8 +194,8 @@ class ApacheHttp01Test(util.ApacheTest): achall.response(self.account_key) for achall in achalls] self.assertEqual(self.http.perform(), expected_response) - self.assertTrue(os.path.isdir(self.http.challenge_dir)) - self.assertTrue(filesystem.has_min_permissions(self.http.challenge_dir, 0o755)) + self.assertIs(os.path.isdir(self.http.challenge_dir), True) + self.assertIs(filesystem.has_min_permissions(self.http.challenge_dir, 0o755), True) self._test_challenge_conf() for achall in achalls: @@ -211,7 +211,7 @@ class ApacheHttp01Test(util.ApacheTest): vhost.path) self.assertEqual(len(matches), 1) - self.assertTrue(os.path.exists(challenge_dir)) + self.assertIs(os.path.exists(challenge_dir), True) @mock.patch("certbot_apache._internal.http_01.filesystem.makedirs") def test_failed_makedirs(self, mock_makedirs): @@ -226,20 +226,20 @@ class ApacheHttp01Test(util.ApacheTest): with open(self.http.challenge_conf_post) as f: post_conf_contents = f.read() - self.assertTrue("RewriteEngine on" in pre_conf_contents) - self.assertTrue("RewriteRule" in pre_conf_contents) + self.assertIn("RewriteEngine on", pre_conf_contents) + self.assertIn("RewriteRule", pre_conf_contents) - self.assertTrue(self.http.challenge_dir in post_conf_contents) + self.assertIn(self.http.challenge_dir, post_conf_contents) if self.config.version < (2, 4): - self.assertTrue("Allow from all" in post_conf_contents) + self.assertIn("Allow from all", post_conf_contents) else: - self.assertTrue("Require all granted" in post_conf_contents) + self.assertIn("Require all granted", post_conf_contents) def _test_challenge_file(self, achall): name = os.path.join(self.http.challenge_dir, achall.chall.encode("token")) validation = achall.validation(self.account_key) - self.assertTrue(filesystem.has_min_permissions(name, 0o644)) + self.assertIs(filesystem.has_min_permissions(name, 0o644), True) with open(name, 'rb') as f: self.assertEqual(f.read(), validation.encode()) diff --git a/certbot-apache/tests/obj_test.py b/certbot-apache/tests/obj_test.py index 8aae3ad45..411ec21e9 100644 --- a/certbot-apache/tests/obj_test.py +++ b/certbot-apache/tests/obj_test.py @@ -44,15 +44,14 @@ class VirtualHostTest(unittest.TestCase): "fp", "vhp", {Addr.fromstring("*:443"), Addr.fromstring("1.2.3.4:443")}, False, False) - self.assertTrue(complex_vh.conflicts([self.addr1])) - self.assertTrue(complex_vh.conflicts([self.addr2])) - self.assertFalse(complex_vh.conflicts([self.addr_default])) + self.assertIs(complex_vh.conflicts([self.addr1]), True) + self.assertIs(complex_vh.conflicts([self.addr2]), True) + self.assertIs(complex_vh.conflicts([self.addr_default]), False) - self.assertTrue(self.vhost1.conflicts([self.addr2])) - self.assertFalse(self.vhost1.conflicts([self.addr_default])) + self.assertIs(self.vhost1.conflicts([self.addr2]), True) + self.assertIs(self.vhost1.conflicts([self.addr_default]), False) - self.assertFalse(self.vhost2.conflicts([self.addr1, - self.addr_default])) + self.assertIs(self.vhost2.conflicts([self.addr1, self.addr_default]), False) def test_same_server(self): from certbot_apache._internal.obj import VirtualHost @@ -67,12 +66,12 @@ class VirtualHostTest(unittest.TestCase): "fp", "vhp", {self.addr2, self.addr_default}, False, False, None) - self.assertTrue(self.vhost1.same_server(self.vhost2)) - self.assertTrue(no_name1.same_server(no_name2)) + self.assertIs(self.vhost1.same_server(self.vhost2), True) + self.assertIs(no_name1.same_server(no_name2), True) - self.assertFalse(self.vhost1.same_server(no_name1)) - self.assertFalse(no_name1.same_server(no_name3)) - self.assertFalse(no_name1.same_server(no_name4)) + self.assertIs(self.vhost1.same_server(no_name1), False) + self.assertIs(no_name1.same_server(no_name3), False) + self.assertIs(no_name1.same_server(no_name4), False) class AddrTest(unittest.TestCase): @@ -88,9 +87,9 @@ class AddrTest(unittest.TestCase): self.addr_default = Addr.fromstring("_default_:443") def test_wildcard(self): - self.assertFalse(self.addr.is_wildcard()) - self.assertTrue(self.addr1.is_wildcard()) - self.assertTrue(self.addr2.is_wildcard()) + self.assertIs(self.addr.is_wildcard(), False) + self.assertIs(self.addr1.is_wildcard(), True) + self.assertIs(self.addr2.is_wildcard(), True) def test_get_sni_addr(self): from certbot_apache._internal.obj import Addr @@ -103,29 +102,29 @@ class AddrTest(unittest.TestCase): def test_conflicts(self): # Note: Defined IP is more important than defined port in match - self.assertTrue(self.addr.conflicts(self.addr1)) - self.assertTrue(self.addr.conflicts(self.addr2)) - self.assertTrue(self.addr.conflicts(self.addr_defined)) - self.assertFalse(self.addr.conflicts(self.addr_default)) + self.assertIs(self.addr.conflicts(self.addr1), True) + self.assertIs(self.addr.conflicts(self.addr2), True) + self.assertIs(self.addr.conflicts(self.addr_defined), True) + self.assertIs(self.addr.conflicts(self.addr_default), False) - self.assertFalse(self.addr1.conflicts(self.addr)) - self.assertTrue(self.addr1.conflicts(self.addr_defined)) - self.assertFalse(self.addr1.conflicts(self.addr_default)) + self.assertIs(self.addr1.conflicts(self.addr), False) + self.assertIs(self.addr1.conflicts(self.addr_defined), True) + self.assertIs(self.addr1.conflicts(self.addr_default), False) - self.assertFalse(self.addr_defined.conflicts(self.addr1)) - self.assertFalse(self.addr_defined.conflicts(self.addr2)) - self.assertFalse(self.addr_defined.conflicts(self.addr)) - self.assertFalse(self.addr_defined.conflicts(self.addr_default)) + self.assertIs(self.addr_defined.conflicts(self.addr1), False) + self.assertIs(self.addr_defined.conflicts(self.addr2), False) + self.assertIs(self.addr_defined.conflicts(self.addr), False) + self.assertIs(self.addr_defined.conflicts(self.addr_default), False) - self.assertTrue(self.addr_default.conflicts(self.addr)) - self.assertTrue(self.addr_default.conflicts(self.addr1)) - self.assertTrue(self.addr_default.conflicts(self.addr_defined)) + self.assertIs(self.addr_default.conflicts(self.addr), True) + self.assertIs(self.addr_default.conflicts(self.addr1), True) + self.assertIs(self.addr_default.conflicts(self.addr_defined), True) # Self test - self.assertTrue(self.addr.conflicts(self.addr)) - self.assertTrue(self.addr1.conflicts(self.addr1)) + self.assertIs(self.addr.conflicts(self.addr), True) + self.assertIs(self.addr1.conflicts(self.addr1), True) # This is a tricky one... - self.assertTrue(self.addr1.conflicts(self.addr2)) + self.assertIs(self.addr1.conflicts(self.addr2), True) def test_equal(self): self.assertEqual(self.addr1, self.addr2) diff --git a/certbot-apache/tests/parser_test.py b/certbot-apache/tests/parser_test.py index 61855fdb7..5166d9e0e 100644 --- a/certbot-apache/tests/parser_test.py +++ b/certbot-apache/tests/parser_test.py @@ -42,7 +42,7 @@ class BasicParserTest(util.ParserTest): self.assertEqual(self.parser.check_aug_version(), ["something"]) self.parser.aug.match.side_effect = RuntimeError - self.assertFalse(self.parser.check_aug_version()) + self.assertIs(self.parser.check_aug_version(), False) def test_find_config_root_no_root(self): # pylint: disable=protected-access @@ -80,8 +80,7 @@ class BasicParserTest(util.ParserTest): aug_default = "/files" + self.parser.loc["default"] self.parser.add_dir(aug_default, "AddDirective", "test") - self.assertTrue( - self.parser.find_dir("AddDirective", "test", aug_default)) + self.assertTrue(self.parser.find_dir("AddDirective", "test", aug_default)) self.parser.add_dir(aug_default, "AddList", ["1", "2", "3", "4"]) matches = self.parser.find_dir("AddList", None, aug_default) @@ -94,12 +93,9 @@ class BasicParserTest(util.ParserTest): "AddDirectiveBeginning", "testBegin") - self.assertTrue( - self.parser.find_dir("AddDirectiveBeginning", "testBegin", aug_default)) + self.assertTrue(self.parser.find_dir("AddDirectiveBeginning", "testBegin", aug_default)) - self.assertEqual( - self.parser.aug.get(aug_default+"/directive[1]"), - "AddDirectiveBeginning") + self.assertEqual(self.parser.aug.get(aug_default+"/directive[1]"), "AddDirectiveBeginning") self.parser.add_dir_beginning(aug_default, "AddList", ["1", "2", "3", "4"]) matches = self.parser.find_dir("AddList", None, aug_default) for i, match in enumerate(matches): @@ -108,11 +104,13 @@ class BasicParserTest(util.ParserTest): 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)) + self.assertGreater( + len(self.parser.find_dir("AddDirectiveBeginning", "testBegin", conf)), + 0 + ) def test_empty_arg(self): - self.assertEqual(None, - self.parser.get_arg("/files/whatever/nonexistent")) + self.assertIsNone(self.parser.get_arg("/files/whatever/nonexistent")) def test_add_dir_to_ifmodssl(self): """test add_dir_to_ifmodssl. @@ -131,7 +129,7 @@ class BasicParserTest(util.ParserTest): matches = self.parser.find_dir("FakeDirective", "123") self.assertEqual(len(matches), 1) - self.assertTrue("IfModule" in matches[0]) + self.assertIn("IfModule", matches[0]) def test_add_dir_to_ifmodssl_multiple(self): from certbot_apache._internal.parser import get_aug_path @@ -145,7 +143,7 @@ class BasicParserTest(util.ParserTest): matches = self.parser.find_dir("FakeDirective") self.assertEqual(len(matches), 3) - self.assertTrue("IfModule" in matches[0]) + self.assertIn("IfModule", matches[0]) def test_get_aug_path(self): from certbot_apache._internal.parser import get_aug_path @@ -170,7 +168,7 @@ class BasicParserTest(util.ParserTest): with mock.patch("certbot_apache._internal.parser.logger") as mock_logger: self.parser.parse_modules() # Make sure that we got None return value and logged the file - self.assertTrue(mock_logger.debug.called) + self.assertIs(mock_logger.debug.called, True) @mock.patch("certbot_apache._internal.parser.ApacheParser.find_dir") @mock.patch("certbot_apache._internal.apache_util._get_runtime_cfg") @@ -328,7 +326,7 @@ class BasicParserTest(util.ParserTest): self.parser.add_comment(get_aug_path(self.parser.loc["name"]), "123456") comm = self.parser.find_comments("123456") self.assertEqual(len(comm), 1) - self.assertTrue(self.parser.loc["name"] in comm[0]) + self.assertIn(self.parser.loc["name"], comm[0]) class ParserInitTest(util.ApacheTest): diff --git a/certbot-apache/tests/parsernode_configurator_test.py b/certbot-apache/tests/parsernode_configurator_test.py index 411871a43..ebeda3c37 100644 --- a/certbot-apache/tests/parsernode_configurator_test.py +++ b/certbot-apache/tests/parsernode_configurator_test.py @@ -32,7 +32,7 @@ class ConfiguratorParserNodeTest(util.ApacheTest): # pylint: disable=too-many-p self.config.USE_PARSERNODE = True vhosts = self.config.get_virtual_hosts() # Legacy get_virtual_hosts() do not set the node - self.assertTrue(vhosts[0].node is not None) + self.assertIsNotNone(vhosts[0].node) def test_parsernode_get_vhosts_mismatch(self): vhosts = self.config.get_virtual_hosts_v2() diff --git a/certbot-apache/tests/util.py b/certbot-apache/tests/util.py index b9e7d2ea0..f0c76e693 100644 --- a/certbot-apache/tests/util.py +++ b/certbot-apache/tests/util.py @@ -1,6 +1,5 @@ """Common utilities for certbot_apache.""" import shutil -import sys import unittest import augeas @@ -14,7 +13,6 @@ except ImportError: # pragma: no cover from certbot.compat import os 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 From 97a09dee19273a6c43aa71edc04ecfae3e761314 Mon Sep 17 00:00:00 2001 From: Adrien Ferrand Date: Tue, 4 Jan 2022 10:24:24 +0100 Subject: [PATCH 2/7] Revert "Remove win2016 from Azure devops pipelines. (#9145)" (#9158) This reverts commit dc66c879283c6c8b27dbcdd5d5faa4fe271e3e8f. --- .../templates/jobs/packaging-jobs.yml | 18 ++++++++++-------- .../templates/jobs/standard-tests-jobs.yml | 6 +++--- .../templates/stages/changelog-stage.yml | 2 +- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/.azure-pipelines/templates/jobs/packaging-jobs.yml b/.azure-pipelines/templates/jobs/packaging-jobs.yml index c1bba62f5..cfd817c8b 100644 --- a/.azure-pipelines/templates/jobs/packaging-jobs.yml +++ b/.azure-pipelines/templates/jobs/packaging-jobs.yml @@ -54,12 +54,8 @@ jobs: done displayName: Run integration tests for Docker images - job: installer_build - strategy: - matrix: - win-2019: - vmImage: windows-2019 - win-2022: - vmImage: windows-2022 + pool: + vmImage: vs2017-win2016 steps: - task: UsePythonVersion@0 inputs: @@ -91,11 +87,17 @@ jobs: matrix: win2019: imageName: windows-2019 - win2022: - imageName: windows-2022 + win2016: + imageName: vs2017-win2016 pool: vmImage: $(imageName) steps: + - powershell: | + if ($PSVersionTable.PSVersion.Major -ne 5) { + throw "Powershell version is not 5.x" + } + condition: eq(variables['imageName'], 'vs2017-win2016') + displayName: Check Powershell 5.x is used in vs2017-win2016 - task: UsePythonVersion@0 inputs: versionSpec: 3.9 diff --git a/.azure-pipelines/templates/jobs/standard-tests-jobs.yml b/.azure-pipelines/templates/jobs/standard-tests-jobs.yml index 0aa0e6248..18416fcbb 100644 --- a/.azure-pipelines/templates/jobs/standard-tests-jobs.yml +++ b/.azure-pipelines/templates/jobs/standard-tests-jobs.yml @@ -13,15 +13,15 @@ jobs: PYTHON_VERSION: 3.10 TOXENV: py310-cover windows-py36: - IMAGE_NAME: windows-2019 + IMAGE_NAME: vs2017-win2016 PYTHON_VERSION: 3.6 TOXENV: py36-win windows-py39-cover: - IMAGE_NAME: windows-2019 + IMAGE_NAME: vs2017-win2016 PYTHON_VERSION: 3.9 TOXENV: py39-cover-win windows-integration-certbot: - IMAGE_NAME: windows-2019 + IMAGE_NAME: vs2017-win2016 PYTHON_VERSION: 3.9 TOXENV: integration-certbot linux-oldest-tests-1: diff --git a/.azure-pipelines/templates/stages/changelog-stage.yml b/.azure-pipelines/templates/stages/changelog-stage.yml index fc8251ac4..7d089f8d4 100644 --- a/.azure-pipelines/templates/stages/changelog-stage.yml +++ b/.azure-pipelines/templates/stages/changelog-stage.yml @@ -3,7 +3,7 @@ stages: jobs: - job: prepare pool: - vmImage: win2019 + vmImage: vs2017-win2016 steps: # If we change the output filename from `release_notes.md`, it should also be changed in tools/create_github_release.py - bash: | From ed7964b424345b5d8ad98335be8f98d5c21059a6 Mon Sep 17 00:00:00 2001 From: Mads Jensen Date: Tue, 4 Jan 2022 23:59:58 +0100 Subject: [PATCH 3/7] Improve assertions in nginx and DNS plugin tests. (#9157) * Improve assertions in nginx and DNS plugin tests. * Use assertIs for asserting is True/False. --- certbot-dns-google/tests/dns_google_test.py | 15 +++--- certbot-dns-linode/tests/dns_linode_test.py | 4 ++ certbot-dns-rfc2136/tests/dns_rfc2136_test.py | 5 +- certbot-nginx/tests/configurator_test.py | 46 ++++++++++--------- certbot-nginx/tests/display_ops_test.py | 9 ++-- certbot-nginx/tests/nginxparser_test.py | 10 ++-- certbot-nginx/tests/obj_test.py | 37 +++++++-------- certbot-nginx/tests/parser_obj_test.py | 11 +++-- certbot-nginx/tests/parser_test.py | 8 ++-- certbot/tests/display/obj_test.py | 3 +- 10 files changed, 82 insertions(+), 66 deletions(-) diff --git a/certbot-dns-google/tests/dns_google_test.py b/certbot-dns-google/tests/dns_google_test.py index dc5bc1bf1..b6f63a937 100644 --- a/certbot-dns-google/tests/dns_google_test.py +++ b/certbot-dns-google/tests/dns_google_test.py @@ -184,9 +184,9 @@ class GoogleClientTest(unittest.TestCase): with mock.patch(mock_get_rrs) as mock_rrs: mock_rrs.return_value = {"rrdatas": ["sample-txt-contents"], "ttl": self.record_ttl} client.add_txt_record(DOMAIN, self.record_name, self.record_content, self.record_ttl) - self.assertTrue(changes.create.called) + self.assertIs(changes.create.called, True) deletions = changes.create.call_args_list[0][1]["body"]["deletions"][0] - self.assertTrue("sample-txt-contents" in deletions["rrdatas"]) + self.assertIn("sample-txt-contents", deletions["rrdatas"]) self.assertEqual(self.record_ttl, deletions["ttl"]) @mock.patch('oauth2client.service_account.ServiceAccountCredentials.from_json_keyfile_name') @@ -201,9 +201,9 @@ class GoogleClientTest(unittest.TestCase): custom_ttl = 300 mock_rrs.return_value = {"rrdatas": ["sample-txt-contents"], "ttl": custom_ttl} client.add_txt_record(DOMAIN, self.record_name, self.record_content, self.record_ttl) - self.assertTrue(changes.create.called) + self.assertIs(changes.create.called, True) deletions = changes.create.call_args_list[0][1]["body"]["deletions"][0] - self.assertTrue("sample-txt-contents" in deletions["rrdatas"]) + self.assertIn("sample-txt-contents", deletions["rrdatas"]) self.assertEqual(custom_ttl, deletions["ttl"]) #otherwise HTTP 412 @mock.patch('oauth2client.service_account.ServiceAccountCredentials.from_json_keyfile_name') @@ -214,7 +214,7 @@ class GoogleClientTest(unittest.TestCase): [{'managedZones': [{'id': self.zone}]}]) client.add_txt_record(DOMAIN, "_acme-challenge.example.org", "example-txt-contents", self.record_ttl) - self.assertFalse(changes.create.called) + self.assertIs(changes.create.called, False) @mock.patch('oauth2client.service_account.ServiceAccountCredentials.from_json_keyfile_name') @mock.patch('certbot_dns_google._internal.dns_google.open', @@ -357,7 +357,7 @@ class GoogleClientTest(unittest.TestCase): client, unused_changes = self._setUp_client_with_mock( [{'managedZones': [{'id': self.zone}]}]) not_found = client.get_existing_txt_rrset(self.zone, "nonexistent.tld") - self.assertEqual(not_found, None) + self.assertIsNone(not_found) @mock.patch('oauth2client.service_account.ServiceAccountCredentials.from_json_keyfile_name') @mock.patch('certbot_dns_google._internal.dns_google.open', @@ -367,7 +367,7 @@ class GoogleClientTest(unittest.TestCase): [{'managedZones': [{'id': self.zone}]}], API_ERROR) # Record name mocked in setUp found = client.get_existing_txt_rrset(self.zone, "_acme-challenge.example.org") - self.assertEqual(found, None) + self.assertIsNone(found) @mock.patch('oauth2client.service_account.ServiceAccountCredentials.from_json_keyfile_name') @mock.patch('certbot_dns_google._internal.dns_google.open', @@ -411,5 +411,6 @@ class DummyResponse: def __init__(self): self.status = 200 + if __name__ == "__main__": unittest.main() # pragma: no cover diff --git a/certbot-dns-linode/tests/dns_linode_test.py b/certbot-dns-linode/tests/dns_linode_test.py index 9861d2ab0..d0d6ceb03 100644 --- a/certbot-dns-linode/tests/dns_linode_test.py +++ b/certbot-dns-linode/tests/dns_linode_test.py @@ -18,6 +18,7 @@ TOKEN = 'a-token' TOKEN_V3 = '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ64' TOKEN_V4 = '0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef' + class AuthenticatorTest(test_util.TempDirTestCase, dns_test_common_lexicon.BaseLexiconAuthenticatorTest): @@ -119,6 +120,7 @@ class AuthenticatorTest(test_util.TempDirTestCase, auth._setup_credentials() self.assertRaises(errors.PluginError, auth._get_linode_client) + class LinodeLexiconClientTest(unittest.TestCase, dns_test_common_lexicon.BaseLexiconClientTest): DOMAIN_NOT_FOUND = Exception('Domain not found') @@ -131,6 +133,7 @@ class LinodeLexiconClientTest(unittest.TestCase, dns_test_common_lexicon.BaseLex self.provider_mock = mock.MagicMock() self.client.provider = self.provider_mock + class Linode4LexiconClientTest(unittest.TestCase, dns_test_common_lexicon.BaseLexiconClientTest): DOMAIN_NOT_FOUND = Exception('Domain not found') @@ -143,5 +146,6 @@ class Linode4LexiconClientTest(unittest.TestCase, dns_test_common_lexicon.BaseLe self.provider_mock = mock.MagicMock() self.client.provider = self.provider_mock + if __name__ == "__main__": unittest.main() # pragma: no cover diff --git a/certbot-dns-rfc2136/tests/dns_rfc2136_test.py b/certbot-dns-rfc2136/tests/dns_rfc2136_test.py index 72ea6d9a4..d0434aef5 100644 --- a/certbot-dns-rfc2136/tests/dns_rfc2136_test.py +++ b/certbot-dns-rfc2136/tests/dns_rfc2136_test.py @@ -23,6 +23,7 @@ SECRET = 'SSB3b25kZXIgd2hvIHdpbGwgYm90aGVyIHRvIGRlY29kZSB0aGlzIHRleHQK' VALID_CONFIG = {"rfc2136_server": SERVER, "rfc2136_name": NAME, "rfc2136_secret": SECRET} TIMEOUT = 45 + class AuthenticatorTest(test_util.TempDirTestCase, dns_test_common.BaseAuthenticatorTest): def setUp(self): @@ -113,7 +114,7 @@ class RFC2136ClientTest(unittest.TestCase): self.rfc2136_client.add_txt_record("bar", "baz", 42) query_mock.assert_called_with(mock.ANY, SERVER, TIMEOUT, PORT) - self.assertTrue("bar. 42 IN TXT \"baz\"" in str(query_mock.call_args[0][0])) + self.assertIn('bar. 42 IN TXT "baz"', str(query_mock.call_args[0][0])) @mock.patch("dns.query.tcp") def test_add_txt_record_wraps_errors(self, query_mock): @@ -146,7 +147,7 @@ class RFC2136ClientTest(unittest.TestCase): self.rfc2136_client.del_txt_record("bar", "baz") query_mock.assert_called_with(mock.ANY, SERVER, TIMEOUT, PORT) - self.assertTrue("bar. 0 NONE TXT \"baz\"" in str(query_mock.call_args[0][0])) + self.assertIn('bar. 0 NONE TXT "baz"', str(query_mock.call_args[0][0])) @mock.patch("dns.query.tcp") def test_del_txt_record_wraps_errors(self, query_mock): diff --git a/certbot-nginx/tests/configurator_test.py b/certbot-nginx/tests/configurator_test.py index e667d5375..a182f789a 100644 --- a/certbot-nginx/tests/configurator_test.py +++ b/certbot-nginx/tests/configurator_test.py @@ -24,7 +24,6 @@ import test_util as util class NginxConfiguratorTest(util.NginxTest): """Test a semi complex vhost configuration.""" - def setUp(self): super().setUp() @@ -79,8 +78,8 @@ class NginxConfiguratorTest(util.NginxTest): self.config.prepare() except errors.PluginError as err: err_msg = str(err) - self.assertTrue("lock" in err_msg) - self.assertTrue(self.config.conf("server-root") in err_msg) + self.assertIn("lock", err_msg) + self.assertIn(self.config.conf("server-root"), err_msg) else: # pragma: no cover self.fail("Exception wasn't raised!") @@ -192,8 +191,9 @@ class NginxConfiguratorTest(util.NginxTest): '69.255.225.155'] for name in bad_results: - self.assertRaises(errors.MisconfigurationError, - self.config.choose_vhosts, name) + with self.subTest(name=name): + self.assertRaises(errors.MisconfigurationError, + self.config.choose_vhosts, name) def test_ipv6only(self): # ipv6_info: (ipv6_active, ipv6only_present) @@ -215,7 +215,7 @@ class NginxConfiguratorTest(util.NginxTest): self.assertFalse(addr.ipv6only) def test_more_info(self): - self.assertTrue('nginx.conf' in self.config.more_info()) + self.assertIn('nginx.conf', self.config.more_info()) def test_deploy_cert_requires_fullchain_path(self): self.config.version = (1, 3, 1) @@ -547,7 +547,7 @@ class NginxConfiguratorTest(util.NginxTest): self.config.enhance("www.example.com", "redirect") generated_conf = self.config.parser.parsed[example_conf] - self.assertTrue(util.contains_at_depth(generated_conf, expected, 2)) + self.assertIs(util.contains_at_depth(generated_conf, expected, 2), True) # Test that we successfully add a redirect when there is # no listen directive @@ -557,7 +557,7 @@ class NginxConfiguratorTest(util.NginxTest): expected = UnspacedList(_redirect_block_for_domain("migration.com"))[0] generated_conf = self.config.parser.parsed[migration_conf] - self.assertTrue(util.contains_at_depth(generated_conf, expected, 2)) + self.assertIs(util.contains_at_depth(generated_conf, expected, 2), True) def test_split_for_redirect(self): example_conf = self.config.parser.abs_path('sites-enabled/example.com') @@ -627,7 +627,7 @@ class NginxConfiguratorTest(util.NginxTest): "Strict-Transport-Security") expected = ['add_header', 'Strict-Transport-Security', '"max-age=31536000"', 'always'] generated_conf = self.config.parser.parsed[example_conf] - self.assertTrue(util.contains_at_depth(generated_conf, expected, 2)) + self.assertIs(util.contains_at_depth(generated_conf, expected, 2), True) def test_multiple_headers_hsts(self): headers_conf = self.config.parser.abs_path('sites-enabled/headers.com') @@ -635,7 +635,7 @@ class NginxConfiguratorTest(util.NginxTest): "Strict-Transport-Security") expected = ['add_header', 'Strict-Transport-Security', '"max-age=31536000"', 'always'] generated_conf = self.config.parser.parsed[headers_conf] - self.assertTrue(util.contains_at_depth(generated_conf, expected, 2)) + self.assertIs(util.contains_at_depth(generated_conf, expected, 2), True) def test_http_header_hsts_twice(self): self.config.enhance("www.example.com", "ensure-http-header", @@ -645,7 +645,6 @@ class NginxConfiguratorTest(util.NginxTest): self.config.enhance, "www.example.com", "ensure-http-header", "Strict-Transport-Security") - @mock.patch('certbot_nginx._internal.obj.VirtualHost.contains_list') def test_certbot_redirect_exists(self, mock_contains_list): # Test that we add no redirect statement if there is already a @@ -867,7 +866,7 @@ class NginxConfiguratorTest(util.NginxTest): vhs = self.config._choose_vhosts_wildcard("*.com", prefer_ssl=True) # Check that the dialog was called with migration.com - self.assertTrue(vhost in mock_select_vhs.call_args[0][0]) + self.assertIn(vhost, mock_select_vhs.call_args[0][0]) # And the actual returned values self.assertEqual(len(vhs), 1) @@ -883,7 +882,7 @@ class NginxConfiguratorTest(util.NginxTest): vhs = self.config._choose_vhosts_wildcard("*.com", prefer_ssl=False) # Check that the dialog was called with migration.com - self.assertTrue(vhost in mock_select_vhs.call_args[0][0]) + self.assertIn(vhost, mock_select_vhs.call_args[0][0]) # And the actual returned values self.assertEqual(len(vhs), 1) @@ -928,7 +927,7 @@ class NginxConfiguratorTest(util.NginxTest): if 'summer.com' in x.names][0] mock_dialog.return_value = [vhost] self.config.enhance("*.com", "staple-ocsp", "example/chain.pem") - self.assertTrue(mock_dialog.called) + self.assertIs(mock_dialog.called, True) @mock.patch("certbot_nginx._internal.display_ops.select_vhost_multiple") def test_enhance_wildcard_double_redirect(self, mock_dialog): @@ -1042,7 +1041,8 @@ class InstallSslOptionsConfTest(util.NginxTest): f.write("hashofanoldversion") with mock.patch("certbot.plugins.common.logger") as mock_logger: self._call() - self.assertEqual(mock_logger.warning.call_args[0][0], + self.assertEqual( + mock_logger.warning.call_args[0][0], "%s has been manually modified; updated file " "saved to %s. We recommend updating %s for security purposes.") self.assertEqual(crypto_util.sha256sum(self.config.mod_ssl_conf_src), @@ -1054,9 +1054,11 @@ class InstallSslOptionsConfTest(util.NginxTest): def test_current_file_hash_in_all_hashes(self): from certbot_nginx._internal.constants import ALL_SSL_OPTIONS_HASHES - self.assertTrue(self._current_ssl_options_hash() in ALL_SSL_OPTIONS_HASHES, + self.assertIn( + self._current_ssl_options_hash(), ALL_SSL_OPTIONS_HASHES, "Constants.ALL_SSL_OPTIONS_HASHES must be appended" - " with the sha256 hash of self.config.mod_ssl_conf when it is updated.") + " with the sha256 hash of self.config.mod_ssl_conf when it is updated." + ) def test_ssl_config_files_hash_in_all_hashes(self): """ @@ -1077,9 +1079,11 @@ class InstallSslOptionsConfTest(util.NginxTest): self.assertTrue(all_files) for one_file in all_files: file_hash = crypto_util.sha256sum(one_file) - self.assertTrue(file_hash in ALL_SSL_OPTIONS_HASHES, - "Constants.ALL_SSL_OPTIONS_HASHES must be appended with the sha256 " - "hash of {0} when it is updated.".format(one_file)) + self.assertIn( + file_hash, ALL_SSL_OPTIONS_HASHES, + f"Constants.ALL_SSL_OPTIONS_HASHES must be appended with the sha256 " + f"hash of {one_file} when it is updated." + ) def test_nginx_version_uses_correct_config(self): self.config.version = (1, 5, 8) @@ -1127,7 +1131,7 @@ class DetermineDefaultServerRootTest(certbot_test_util.ConfigTestCase): self.assertIn("/usr/local/etc/nginx", server_root) self.assertIn("/etc/nginx", server_root) else: - self.assertTrue(server_root in ("/etc/nginx", "/usr/local/etc/nginx")) + self.assertIn(server_root, ("/etc/nginx", "/usr/local/etc/nginx")) if __name__ == "__main__": diff --git a/certbot-nginx/tests/display_ops_test.py b/certbot-nginx/tests/display_ops_test.py index 7480a1109..19f51e7e8 100644 --- a/certbot-nginx/tests/display_ops_test.py +++ b/certbot-nginx/tests/display_ops_test.py @@ -27,15 +27,16 @@ class SelectVhostMultiTest(util.NginxTest): vhs = select_vhost_multiple([self.vhosts[3], self.vhosts[2], self.vhosts[1]]) - self.assertTrue(self.vhosts[2] in vhs) - self.assertTrue(self.vhosts[3] in vhs) - self.assertFalse(self.vhosts[1] in vhs) + self.assertIn(self.vhosts[2], vhs) + self.assertIn(self.vhosts[3], vhs) + self.assertNotIn(self.vhosts[1], vhs) @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) + self.assertEqual(len(vhs), 0) + self.assertEqual(vhs, []) if __name__ == "__main__": diff --git a/certbot-nginx/tests/nginxparser_test.py b/certbot-nginx/tests/nginxparser_test.py index 8f7d5accf..d8f0a5909 100644 --- a/certbot-nginx/tests/nginxparser_test.py +++ b/certbot-nginx/tests/nginxparser_test.py @@ -432,15 +432,15 @@ class TestUnspacedList(unittest.TestCase): self.assertEqual(ul3, ["some", "things", "why", "did", "whether"]) def test_is_dirty(self): - self.assertEqual(False, self.ul2.is_dirty()) + self.assertIs(self.ul2.is_dirty(), False) ul3 = UnspacedList([]) ul3.append(self.ul) - self.assertEqual(False, self.ul.is_dirty()) - self.assertEqual(True, ul3.is_dirty()) + self.assertIs(self.ul.is_dirty(), False) + self.assertIs(ul3.is_dirty(), True) ul4 = UnspacedList([[1], [2, 3, 4]]) - self.assertEqual(False, ul4.is_dirty()) + self.assertIs(ul4.is_dirty(), False) ul4[1][2] = 5 - self.assertEqual(True, ul4.is_dirty()) + self.assertIs(ul4.is_dirty(), True) if __name__ == '__main__': diff --git a/certbot-nginx/tests/obj_test.py b/certbot-nginx/tests/obj_test.py index f385649ed..de82e5682 100644 --- a/certbot-nginx/tests/obj_test.py +++ b/certbot-nginx/tests/obj_test.py @@ -19,37 +19,37 @@ class AddrTest(unittest.TestCase): def test_fromstring(self): self.assertEqual(self.addr1.get_addr(), "192.168.1.1") self.assertEqual(self.addr1.get_port(), "") - self.assertFalse(self.addr1.ssl) - self.assertFalse(self.addr1.default) + self.assertIs(self.addr1.ssl, False) + self.assertIs(self.addr1.default, False) self.assertEqual(self.addr2.get_addr(), "192.168.1.1") self.assertEqual(self.addr2.get_port(), "*") - self.assertTrue(self.addr2.ssl) - self.assertFalse(self.addr2.default) + self.assertIs(self.addr2.ssl, True) + self.assertIs(self.addr2.default, False) self.assertEqual(self.addr3.get_addr(), "192.168.1.1") self.assertEqual(self.addr3.get_port(), "80") - self.assertFalse(self.addr3.ssl) - self.assertFalse(self.addr3.default) + self.assertIs(self.addr3.ssl, False) + self.assertIs(self.addr3.default, False) self.assertEqual(self.addr4.get_addr(), "*") self.assertEqual(self.addr4.get_port(), "80") - self.assertTrue(self.addr4.ssl) - self.assertTrue(self.addr4.default) + self.assertIs(self.addr4.ssl, True) + self.assertIs(self.addr4.default, True) self.assertEqual(self.addr5.get_addr(), "myhost") self.assertEqual(self.addr5.get_port(), "") - self.assertFalse(self.addr5.ssl) - self.assertFalse(self.addr5.default) + self.assertIs(self.addr5.ssl, False) + self.assertIs(self.addr5.default, False) self.assertEqual(self.addr6.get_addr(), "") self.assertEqual(self.addr6.get_port(), "80") - self.assertFalse(self.addr6.ssl) - self.assertTrue(self.addr6.default) + self.assertIs(self.addr6.ssl, False) + self.assertIs(self.addr6.default, True) - self.assertTrue(self.addr8.default) + self.assertIs(self.addr8.default, True) - self.assertEqual(None, self.addr7) + self.assertIsNone(self.addr7) def test_str(self): self.assertEqual(str(self.addr1), "192.168.1.1") @@ -177,10 +177,10 @@ class VirtualHostTest(unittest.TestCase): self.assertEqual(stringified, str(self.vhost1)) def test_has_header(self): - self.assertTrue(self.vhost_has_hsts.has_header('Strict-Transport-Security')) - self.assertFalse(self.vhost_has_hsts.has_header('Bogus-Header')) - self.assertFalse(self.vhost1.has_header('Strict-Transport-Security')) - self.assertFalse(self.vhost1.has_header('Bogus-Header')) + self.assertIs(self.vhost_has_hsts.has_header('Strict-Transport-Security'), True) + self.assertIs(self.vhost_has_hsts.has_header('Bogus-Header'), False) + self.assertIs(self.vhost1.has_header('Strict-Transport-Security'), False) + self.assertIs(self.vhost1.has_header('Bogus-Header'), False) def test_contains_list(self): from certbot_nginx._internal.obj import VirtualHost @@ -224,5 +224,6 @@ class VirtualHostTest(unittest.TestCase): self.assertTrue(vhost_haystack.contains_list(test_needle)) self.assertFalse(vhost_bad_haystack.contains_list(test_needle)) + if __name__ == "__main__": unittest.main() # pragma: no cover diff --git a/certbot-nginx/tests/parser_obj_test.py b/certbot-nginx/tests/parser_obj_test.py index 8262c5f52..4d1f25277 100644 --- a/certbot-nginx/tests/parser_obj_test.py +++ b/certbot-nginx/tests/parser_obj_test.py @@ -35,8 +35,8 @@ class CommentHelpersTest(unittest.TestCase): self.assertTrue(_is_certbot_comment(comment)) self.assertEqual(comment.dump(), COMMENT_BLOCK) self.assertEqual(comment.dump(True), [' '] + COMMENT_BLOCK) - self.assertEqual(_certbot_comment(None, 2).dump(True), - [' '] + COMMENT_BLOCK) + self.assertEqual(_certbot_comment(None, 2).dump(True), [' '] + COMMENT_BLOCK) + class ParsingHooksTest(unittest.TestCase): def test_is_sentence(self): @@ -94,6 +94,7 @@ class ParsingHooksTest(unittest.TestCase): parse_raw([], add_spaces=True) fake_parser1.parse.called_with([None, True]) + class SentenceTest(unittest.TestCase): def setUp(self): from certbot_nginx._internal.parser_obj import Sentence @@ -140,6 +141,7 @@ class SentenceTest(unittest.TestCase): self.sentence.parse(['\n\t \n', 'tabs']) self.assertEqual(self.sentence.get_tabs(), '') + class BlockTest(unittest.TestCase): def setUp(self): from certbot_nginx._internal.parser_obj import Block @@ -244,8 +246,8 @@ class StatementsTest(unittest.TestCase): def test_parse_hides_trailing_whitespace(self): self.statements.parse(self.raw + ['\n\n ']) - self.assertTrue(isinstance(self.statements.dump()[-1], list)) - self.assertTrue(self.statements.dump(True)[-1].isspace()) + self.assertIsInstance(self.statements.dump()[-1], list) + self.assertIs(self.statements.dump(True)[-1].isspace(), True) self.assertEqual(self.statements.dump(True)[-1], '\n\n ') def test_iterate(self): @@ -254,5 +256,6 @@ class StatementsTest(unittest.TestCase): for i, elem in enumerate(self.statements.iterate(match=lambda x: 'sentence' in x)): self.assertEqual(expected[i], elem.dump()) + if __name__ == "__main__": unittest.main() # pragma: no cover diff --git a/certbot-nginx/tests/parser_test.py b/certbot-nginx/tests/parser_test.py index 9047cb446..7d5b5e669 100644 --- a/certbot-nginx/tests/parser_test.py +++ b/certbot-nginx/tests/parser_test.py @@ -203,8 +203,7 @@ class NginxParserTest(util.NginxTest): mock_vhost.raw = [['listen', '80'], ['ssl', 'on'], ['server_name', '*.www.foo.com', '*.www.example.com']] - self.assertTrue(nparser.has_ssl_on_directive(mock_vhost)) - + self.assertIs(nparser.has_ssl_on_directive(mock_vhost), True) def test_remove_server_directives(self): nparser = parser.NginxParser(self.config_path) @@ -452,7 +451,7 @@ class NginxParserTest(util.NginxTest): nparser.filedump(ext='') # check properties of new vhost - self.assertFalse(next(iter(new_vhost.addrs)).default) + self.assertIs(next(iter(new_vhost.addrs)).default, False) self.assertNotEqual(new_vhost.path, default.path) # check that things are written to file correctly @@ -461,7 +460,7 @@ class NginxParserTest(util.NginxTest): new_defaults = [x for x in new_vhosts if 'default' in x.filep] self.assertEqual(len(new_defaults), 2) new_vhost_parsed = new_defaults[1] - self.assertFalse(next(iter(new_vhost_parsed.addrs)).default) + self.assertIs(next(iter(new_vhost_parsed.addrs)).default, False) self.assertEqual(next(iter(default.names)), next(iter(new_vhost_parsed.names))) self.assertEqual(len(default.raw), len(new_vhost_parsed.raw)) self.assertTrue(next(iter(default.addrs)).super_eq(next(iter(new_vhost_parsed.addrs)))) @@ -533,5 +532,6 @@ class NginxParserTest(util.NginxTest): for output in log.output )) + if __name__ == "__main__": unittest.main() # pragma: no cover diff --git a/certbot/tests/display/obj_test.py b/certbot/tests/display/obj_test.py index 7e99aa463..f6fe41a68 100644 --- a/certbot/tests/display/obj_test.py +++ b/certbot/tests/display/obj_test.py @@ -293,7 +293,8 @@ class NoninteractiveDisplayTest(unittest.TestCase): self.displayer.notification("message2", pause=False) string = self.mock_stdout.write.call_args[0][0] - self.assertTrue("- - - " in string and ("message2" + os.linesep) in string) + self.assertIn("- - - ", string) + self.assertIn("message2" + os.linesep, string) def test_input(self): d = "an incomputable value" From 7e5e51aeff46d5cda0ee92f64b9ba4b5058f5dd7 Mon Sep 17 00:00:00 2001 From: Mads Jensen Date: Sun, 9 Jan 2022 22:50:44 +0100 Subject: [PATCH 4/7] Use super().__init__ instead of explicitly calling named super-class. (#9166) * Use super().__init__ instead of explicitly calling named super-class. * Fix unittest (typo fix). --- acme/acme/standalone.py | 15 +++++++-------- acme/tests/standalone_test.py | 4 +--- .../certbot_nginx/_internal/nginxparser.py | 2 +- certbot/certbot/_internal/error_handler.py | 3 ++- certbot/tests/plugins/dns_common_test.py | 2 +- 5 files changed, 12 insertions(+), 14 deletions(-) diff --git a/acme/acme/standalone.py b/acme/acme/standalone.py index cd2caa221..5dfb27136 100644 --- a/acme/acme/standalone.py +++ b/acme/acme/standalone.py @@ -34,10 +34,9 @@ class TLSServer(socketserver.TCPServer): else: self.address_family = socket.AF_INET self.certs = kwargs.pop("certs", {}) - self.method = kwargs.pop( - "method", crypto_util._DEFAULT_SSL_METHOD) + self.method = kwargs.pop("method", crypto_util._DEFAULT_SSL_METHOD) self.allow_reuse_address = kwargs.pop("allow_reuse_address", True) - socketserver.TCPServer.__init__(self, *args, **kwargs) + super().__init__(*args, **kwargs) def _wrap_sock(self) -> None: self.socket = crypto_util.SSLSocket( @@ -190,7 +189,7 @@ class HTTPServer(BaseHTTPServer.HTTPServer): self.address_family = socket.AF_INET6 else: self.address_family = socket.AF_INET - BaseHTTPServer.HTTPServer.__init__(self, *args, **kwargs) + super().__init__(*args, **kwargs) class HTTP01Server(HTTPServer, ACMEServerMixin): @@ -198,8 +197,8 @@ class HTTP01Server(HTTPServer, ACMEServerMixin): def __init__(self, server_address: Tuple[str, int], resources: Set[challenges.HTTP01], ipv6: bool = False, timeout: int = 30) -> None: - HTTPServer.__init__( - self, server_address, HTTP01RequestHandler.partial_init( + super().__init__( + server_address, HTTP01RequestHandler.partial_init( simple_http_resources=resources, timeout=timeout), ipv6=ipv6) @@ -208,7 +207,7 @@ class HTTP01DualNetworkedServers(BaseDualNetworkedServers): affect the other.""" def __init__(self, *args: Any, **kwargs: Any) -> None: - BaseDualNetworkedServers.__init__(self, HTTP01Server, *args, **kwargs) + super().__init__(HTTP01Server, *args, **kwargs) class HTTP01RequestHandler(BaseHTTPServer.BaseHTTPRequestHandler): @@ -226,7 +225,7 @@ class HTTP01RequestHandler(BaseHTTPServer.BaseHTTPRequestHandler): def __init__(self, *args: Any, **kwargs: Any) -> None: self.simple_http_resources = kwargs.pop("simple_http_resources", set()) self._timeout = kwargs.pop('timeout', 30) - BaseHTTPServer.BaseHTTPRequestHandler.__init__(self, *args, **kwargs) + super().__init__(*args, **kwargs) self.server: HTTP01Server # In parent class BaseHTTPRequestHandler, 'timeout' is a class-level property but we diff --git a/acme/tests/standalone_test.py b/acme/tests/standalone_test.py index e0aa5aa22..ad5751fcf 100644 --- a/acme/tests/standalone_test.py +++ b/acme/tests/standalone_test.py @@ -165,7 +165,6 @@ class TLSALPN01ServerTest(unittest.TestCase): class BaseDualNetworkedServersTest(unittest.TestCase): """Test for acme.standalone.BaseDualNetworkedServers.""" - class SingleProtocolServer(socketserver.TCPServer): """Server that only serves on a single protocol. FreeBSD has this behavior for AF_INET6.""" def __init__(self, *args, **kwargs): @@ -175,7 +174,7 @@ class BaseDualNetworkedServersTest(unittest.TestCase): kwargs["bind_and_activate"] = False else: self.address_family = socket.AF_INET - socketserver.TCPServer.__init__(self, *args, **kwargs) + super().__init__(*args, **kwargs) if ipv6: # NB: On Windows, socket.IPPROTO_IPV6 constant may be missing. # We use the corresponding value (41) instead. @@ -202,7 +201,6 @@ class BaseDualNetworkedServersTest(unittest.TestCase): self.assertEqual(em.exception.errno, EADDRINUSE) - def test_ports_equal(self): from acme.standalone import BaseDualNetworkedServers servers = BaseDualNetworkedServers( diff --git a/certbot-nginx/certbot_nginx/_internal/nginxparser.py b/certbot-nginx/certbot_nginx/_internal/nginxparser.py index 2aa677c38..03aa88db4 100644 --- a/certbot-nginx/certbot_nginx/_internal/nginxparser.py +++ b/certbot-nginx/certbot_nginx/_internal/nginxparser.py @@ -118,7 +118,7 @@ class UnspacedList(list): # Turn self into a version of the source list that has spaces removed # and all sub-lists also UnspacedList()ed - list.__init__(self, list_source) + super().__init__(list_source) for i, entry in reversed(list(enumerate(self))): if isinstance(entry, list): sublist = UnspacedList(entry) diff --git a/certbot/certbot/_internal/error_handler.py b/certbot/certbot/_internal/error_handler.py index 0e63d02de..24ab46d9d 100644 --- a/certbot/certbot/_internal/error_handler.py +++ b/certbot/certbot/_internal/error_handler.py @@ -167,6 +167,7 @@ class ErrorHandler: logger.debug("Calling signal %s", signum) os.kill(os.getpid(), signum) + class ExitHandler(ErrorHandler): """Context manager for running code that must be cleaned up. @@ -175,5 +176,5 @@ class ExitHandler(ErrorHandler): regular exit. """ def __init__(self, func: Callable[..., Any], *args: Any, **kwargs: Any) -> None: - ErrorHandler.__init__(self, func, *args, **kwargs) + super().__init__(func, *args, **kwargs) self.call_on_regular_exit = True diff --git a/certbot/tests/plugins/dns_common_test.py b/certbot/tests/plugins/dns_common_test.py index 41117f894..f68d36137 100644 --- a/certbot/tests/plugins/dns_common_test.py +++ b/certbot/tests/plugins/dns_common_test.py @@ -133,7 +133,7 @@ class CredentialsConfigurationTest(test_util.TempDirTestCase): def __init__(self, *args, **kwargs): self.reset() - logging.Handler.__init__(self, *args, **kwargs) + super().__init__(*args, **kwargs) def emit(self, record): self.messages[record.levelname.lower()].append(record.getMessage()) From 30b066f08260b73fc26256b5484a180468b9d0a6 Mon Sep 17 00:00:00 2001 From: Mads Jensen Date: Sun, 9 Jan 2022 22:51:06 +0100 Subject: [PATCH 5/7] Remove outdated pylint comments (#9167) * Remove outdated pylint: disable=unused-import annotations. * remove # pylint: disable=ungrouped-imports annotations. * Remove single pylint: disable = unused-argument in DeleteIfAppropriateTest.test_opt_in_deletion. --- certbot-apache/certbot_apache/_internal/http_01.py | 2 +- certbot-nginx/certbot_nginx/_internal/configurator.py | 2 +- certbot/certbot/_internal/account.py | 4 +++- certbot/certbot/ocsp.py | 2 +- certbot/tests/display/completer_test.py | 6 +++--- certbot/tests/main_test.py | 3 +-- certbot/tests/plugins/standalone_test.py | 5 ++--- 7 files changed, 12 insertions(+), 12 deletions(-) diff --git a/certbot-apache/certbot_apache/_internal/http_01.py b/certbot-apache/certbot_apache/_internal/http_01.py index 749a57634..117eec2a5 100644 --- a/certbot-apache/certbot_apache/_internal/http_01.py +++ b/certbot-apache/certbot_apache/_internal/http_01.py @@ -9,7 +9,7 @@ from certbot import errors from certbot.compat import filesystem from certbot.compat import os from certbot.plugins import common -from certbot_apache._internal.obj import VirtualHost # pylint: disable=unused-import +from certbot_apache._internal.obj import VirtualHost from certbot_apache._internal.parser import get_aug_path if TYPE_CHECKING: diff --git a/certbot-nginx/certbot_nginx/_internal/configurator.py b/certbot-nginx/certbot_nginx/_internal/configurator.py index ede184159..46ec7d57e 100644 --- a/certbot-nginx/certbot_nginx/_internal/configurator.py +++ b/certbot-nginx/certbot_nginx/_internal/configurator.py @@ -28,7 +28,7 @@ from certbot_nginx._internal import constants from certbot_nginx._internal import display_ops from certbot_nginx._internal import http_01 from certbot_nginx._internal import nginxparser -from certbot_nginx._internal import obj # pylint: disable=unused-import +from certbot_nginx._internal import obj from certbot_nginx._internal import parser NAME_RANK = 0 diff --git a/certbot/certbot/_internal/account.py b/certbot/certbot/_internal/account.py index 07a869789..041fe9cf5 100644 --- a/certbot/certbot/_internal/account.py +++ b/certbot/certbot/_internal/account.py @@ -20,7 +20,7 @@ import pytz from acme import fields as acme_fields from acme import messages -from acme.client import ClientBase # pylint: disable=unused-import +from acme.client import ClientBase from certbot import configuration from certbot import errors from certbot import interfaces @@ -125,6 +125,7 @@ class AccountMemoryStorage(interfaces.AccountStorage): except KeyError: raise errors.AccountNotFound(account_id) + class RegistrationResourceWithNewAuthzrURI(messages.RegistrationResource): """A backwards-compatible RegistrationResource with a new-authz URI. @@ -136,6 +137,7 @@ class RegistrationResourceWithNewAuthzrURI(messages.RegistrationResource): """ new_authzr_uri = jose.Field('new_authzr_uri') + class AccountFileStorage(interfaces.AccountStorage): """Accounts file storage. diff --git a/certbot/certbot/ocsp.py b/certbot/certbot/ocsp.py index 51d486b6b..2a51fe834 100644 --- a/certbot/certbot/ocsp.py +++ b/certbot/certbot/ocsp.py @@ -22,7 +22,7 @@ from certbot import crypto_util from certbot import errors from certbot import util from certbot.compat.os import getenv -from certbot.interfaces import RenewableCert # pylint: disable=unused-import +from certbot.interfaces import RenewableCert logger = logging.getLogger(__name__) diff --git a/certbot/tests/display/completer_test.py b/certbot/tests/display/completer_test.py index 75b11d1d7..a6ada8b9a 100644 --- a/certbot/tests/display/completer_test.py +++ b/certbot/tests/display/completer_test.py @@ -10,9 +10,9 @@ import string import sys import unittest -from certbot.compat import filesystem # pylint: disable=ungrouped-imports -from certbot.compat import os # pylint: disable=ungrouped-imports -import certbot.tests.util as test_util # pylint: disable=ungrouped-imports +from certbot.compat import filesystem +from certbot.compat import os +import certbot.tests.util as test_util try: import mock diff --git a/certbot/tests/main_test.py b/certbot/tests/main_test.py index 9a67f0853..18c8b8081 100644 --- a/certbot/tests/main_test.py +++ b/certbot/tests/main_test.py @@ -18,7 +18,7 @@ import pytz from certbot import crypto_util, configuration from certbot import errors -from certbot import interfaces # pylint: disable=unused-import +from certbot import interfaces from certbot import util from certbot._internal import account from certbot._internal import cli @@ -529,7 +529,6 @@ class DeleteIfAppropriateTest(test_util.ConfigTestCase): def test_opt_in_deletion(self, mock_get_utility, mock_delete, mock_cert_path_to_lineage, mock_full_archive_dir, mock_match_and_check_overlaps, mock_renewal_file_for_certname): - # pylint: disable = unused-argument config = self.config config.namespace.delete_after_revoke = True config.cert_path = "/some/reasonable/path" diff --git a/certbot/tests/plugins/standalone_test.py b/certbot/tests/plugins/standalone_test.py index 3c990a3f5..2649abae9 100644 --- a/certbot/tests/plugins/standalone_test.py +++ b/certbot/tests/plugins/standalone_test.py @@ -7,10 +7,10 @@ from typing import Tuple import unittest import josepy as jose -import OpenSSL.crypto # pylint: disable=unused-import +import OpenSSL.crypto from acme import challenges -from acme import standalone as acme_standalone # pylint: disable=unused-import +from acme import standalone as acme_standalone from certbot import achallenges from certbot import errors from certbot.tests import acme_util @@ -22,7 +22,6 @@ except ImportError: # pragma: no cover from unittest import mock - class ServerManagerTest(unittest.TestCase): """Tests for certbot._internal.plugins.standalone.ServerManager.""" From 16aad35d31a887dab157f9d4f5e0fe9218d06064 Mon Sep 17 00:00:00 2001 From: Adrien Ferrand Date: Thu, 13 Jan 2022 01:36:51 +0100 Subject: [PATCH 6/7] Fully type certbot-nginx module (#9124) * Work in progress * Fix type * Work in progress * Work in progress * Work in progress * Work in progress * Work in progress * Oups. * Fix typing in UnspacedList * Fix logic * Finish typing * List certbot-nginx as fully typed in tox * Fix lint * Fix checks * Organize imports * Fix typing for Python 3.6 * Fix checks * Fix lint * Update certbot-nginx/certbot_nginx/_internal/configurator.py Co-authored-by: alexzorin * Update certbot-nginx/certbot_nginx/_internal/configurator.py Co-authored-by: alexzorin * Fix signature of deploy_cert regarding the installer interface * Update certbot-nginx/certbot_nginx/_internal/obj.py Co-authored-by: alexzorin * Fix types * Update certbot-nginx/certbot_nginx/_internal/parser.py Co-authored-by: alexzorin * Precise type * Precise _coerce possible inputs/outputs * Fix type * Update certbot-nginx/certbot_nginx/_internal/http_01.py Co-authored-by: ohemorange * Fix type * Remove an undesirable implementation. * Fix type Co-authored-by: alexzorin Co-authored-by: ohemorange --- .../certbot_apache/_internal/configurator.py | 15 +- .../certbot_nginx/_internal/configurator.py | 198 ++++++++++-------- .../certbot_nginx/_internal/constants.py | 6 +- .../certbot_nginx/_internal/display_ops.py | 9 +- .../certbot_nginx/_internal/http_01.py | 48 +++-- .../certbot_nginx/_internal/nginxparser.py | 148 ++++++++----- certbot-nginx/certbot_nginx/_internal/obj.py | 49 +++-- .../certbot_nginx/_internal/parser.py | 184 +++++++++------- .../certbot_nginx/_internal/parser_obj.py | 104 +++++---- certbot/certbot/plugins/common.py | 2 +- tox.ini | 4 +- 11 files changed, 458 insertions(+), 309 deletions(-) diff --git a/certbot-apache/certbot_apache/_internal/configurator.py b/certbot-apache/certbot_apache/_internal/configurator.py index fdeb474ba..1decdc1a0 100644 --- a/certbot-apache/certbot_apache/_internal/configurator.py +++ b/certbot-apache/certbot_apache/_internal/configurator.py @@ -964,7 +964,9 @@ class ApacheConfigurator(common.Configurator): logger.warning("Encountered a problem while parsing file: %s, skipping", path) return None for arg in args: - addrs.add(obj.Addr.fromstring(self.parser.get_arg(arg))) + addr = obj.Addr.fromstring(self.parser.get_arg(arg)) + if addr: + addrs.add(addr) is_ssl = False if self.parser.find_dir("SSLEngine", "on", start=path, exclude=False): @@ -1094,7 +1096,9 @@ class ApacheConfigurator(common.Configurator): """ addrs = set() for param in node.parameters: - addrs.add(obj.Addr.fromstring(param)) + addr = obj.Addr.fromstring(param) + if addr: + addrs.add(addr) is_ssl = False # Exclusion to match the behavior in get_virtual_hosts_v2 @@ -1647,9 +1651,10 @@ class ApacheConfigurator(common.Configurator): for addr in ssl_addr_p: old_addr = obj.Addr.fromstring( str(self.parser.get_arg(addr))) - ssl_addr = old_addr.get_addr_obj("443") - self.parser.aug.set(addr, str(ssl_addr)) - ssl_addrs.add(ssl_addr) + if old_addr: + ssl_addr = old_addr.get_addr_obj("443") + self.parser.aug.set(addr, str(ssl_addr)) + ssl_addrs.add(ssl_addr) return ssl_addrs diff --git a/certbot-nginx/certbot_nginx/_internal/configurator.py b/certbot-nginx/certbot_nginx/_internal/configurator.py index 46ec7d57e..fb819f194 100644 --- a/certbot-nginx/certbot_nginx/_internal/configurator.py +++ b/certbot-nginx/certbot_nginx/_internal/configurator.py @@ -6,30 +6,38 @@ import socket import subprocess import tempfile import time +from typing import Any +from typing import Callable from typing import Dict +from typing import Iterable from typing import List +from typing import Mapping from typing import Optional +from typing import Sequence from typing import Set from typing import Text from typing import Tuple +from typing import Type +from typing import Union -import OpenSSL -import pkg_resources - -from acme import challenges -from acme import crypto_util as acme_crypto_util -from certbot import crypto_util -from certbot import errors -from certbot import util -from certbot.display import util as display_util -from certbot.compat import os -from certbot.plugins import common from certbot_nginx._internal import constants from certbot_nginx._internal import display_ops from certbot_nginx._internal import http_01 from certbot_nginx._internal import nginxparser from certbot_nginx._internal import obj from certbot_nginx._internal import parser +import OpenSSL +import pkg_resources + +from acme import challenges +from acme import crypto_util as acme_crypto_util +from certbot import achallenges +from certbot import crypto_util +from certbot import errors +from certbot import util +from certbot.compat import os +from certbot.display import util as display_util +from certbot.plugins import common NAME_RANK = 0 START_WILDCARD_RANK = 1 @@ -70,7 +78,7 @@ class NginxConfigurator(common.Configurator): SSL_DIRECTIVES = ['ssl_certificate', 'ssl_certificate_key', 'ssl_dhparam'] @classmethod - def add_parser_arguments(cls, add): + def add_parser_arguments(cls, add: Callable[..., None]) -> None: default_server_root = _determine_default_server_root() add("server-root", default=constants.CLI_DEFAULTS["server_root"], help="Nginx server root directory. (default: %s)" % default_server_root) @@ -82,11 +90,11 @@ class NginxConfigurator(common.Configurator): "to apply when reloading.") @property - def nginx_conf(self): + def nginx_conf(self) -> str: """Nginx config file path.""" return os.path.join(self.conf("server_root"), "nginx.conf") - def __init__(self, *args, **kwargs): + def __init__(self, *args: Any, **kwargs: Any) -> None: """Initialize an Nginx Configurator. :param tup version: version of Nginx as a tuple (1, 4, 7) @@ -125,7 +133,7 @@ class NginxConfigurator(common.Configurator): self.parser: parser.NginxParser @property - def mod_ssl_conf_src(self): + def mod_ssl_conf_src(self) -> str: """Full absolute path to SSL configuration file source.""" # Why all this complexity? Well, we want to support Mozilla's intermediate @@ -159,22 +167,23 @@ class NginxConfigurator(common.Configurator): "certbot_nginx", os.path.join("_internal", "tls_configs", config_filename)) @property - def mod_ssl_conf(self): + def mod_ssl_conf(self) -> str: """Full absolute path to SSL configuration file.""" return os.path.join(self.config.config_dir, constants.MOD_SSL_CONF_DEST) @property - def updated_mod_ssl_conf_digest(self): + def updated_mod_ssl_conf_digest(self) -> str: """Full absolute path to digest of updated SSL configuration file.""" return os.path.join(self.config.config_dir, constants.UPDATED_MOD_SSL_CONF_DIGEST) - def install_ssl_options_conf(self, options_ssl, options_ssl_digest): + def install_ssl_options_conf(self, options_ssl: str, options_ssl_digest: str) -> None: """Copy Certbot's SSL options file into the system's config dir if required.""" - return common.install_version_controlled_file(options_ssl, options_ssl_digest, + common.install_version_controlled_file( + options_ssl, options_ssl_digest, self.mod_ssl_conf_src, constants.ALL_SSL_OPTIONS_HASHES) # This is called in determine_authenticator and determine_installer - def prepare(self): + def prepare(self) -> None: """Prepare the authenticator/installer. :raises .errors.NoInstallationError: If Nginx ctl cannot be found @@ -210,8 +219,8 @@ class NginxConfigurator(common.Configurator): raise errors.PluginError('Unable to lock {0}'.format(self.conf('server-root'))) # Entry point in main.py for installing cert - def deploy_cert(self, domain, cert_path, key_path, - chain_path=None, fullchain_path=None): + def deploy_cert(self, domain: str, cert_path: str, key_path: str, chain_path: str, + fullchain_path: str) -> None: """Deploys certificate to specified virtual host. .. note:: Aborts if the vhost is missing ssl_certificate or @@ -234,7 +243,8 @@ class NginxConfigurator(common.Configurator): display_util.notify("Successfully deployed certificate for {} to {}" .format(domain, vhost.filep)) - def _deploy_cert(self, vhost, cert_path, key_path, chain_path, fullchain_path): # pylint: disable=unused-argument + def _deploy_cert(self, vhost: obj.VirtualHost, _cert_path: str, key_path: str, + _chain_path: str, fullchain_path: str) -> None: """ Helper function for deploy_cert() that handles the actual deployment this exists because we might want to do multiple deployments per @@ -244,8 +254,7 @@ class NginxConfigurator(common.Configurator): cert_directives = [['\n ', 'ssl_certificate', ' ', fullchain_path], ['\n ', 'ssl_certificate_key', ' ', key_path]] - self.parser.update_or_add_server_directives(vhost, - cert_directives) + self.parser.update_or_add_server_directives(vhost, cert_directives) logger.info("Deploying Certificate to VirtualHost %s", vhost.filep) self.save_notes += ("Changed vhost at %s with addresses of %s\n" % @@ -254,7 +263,8 @@ class NginxConfigurator(common.Configurator): self.save_notes += "\tssl_certificate %s\n" % fullchain_path self.save_notes += "\tssl_certificate_key %s\n" % key_path - def _choose_vhosts_wildcard(self, domain, prefer_ssl, no_ssl_filter_port=None): + def _choose_vhosts_wildcard(self, domain: str, prefer_ssl: bool, + no_ssl_filter_port: Optional[str] = None) -> List[obj.VirtualHost]: """Prompts user to choose vhosts to install a wildcard certificate for""" if prefer_ssl: vhosts_cache = self._wildcard_vhosts @@ -303,12 +313,13 @@ class NginxConfigurator(common.Configurator): ####################### # Vhost parsing methods ####################### - def _choose_vhost_single(self, target_name): + def _choose_vhost_single(self, target_name: str) -> List[obj.VirtualHost]: matches = self._get_ranked_matches(target_name) vhosts = [x for x in [self._select_best_name_match(matches)] if x is not None] return vhosts - def choose_vhosts(self, target_name, create_if_no_match=False): + def choose_vhosts(self, target_name: str, + create_if_no_match: bool = False) -> List[obj.VirtualHost]: """Chooses a virtual host based on the given domain name. .. note:: This makes the vhost SSL-enabled if it isn't already. Follows @@ -352,7 +363,7 @@ class NginxConfigurator(common.Configurator): return vhosts - def ipv6_info(self, port): + def ipv6_info(self, port: str) -> Tuple[bool, bool]: """Returns tuple of booleans (ipv6_active, ipv6only_present) ipv6_active is true if any server block listens ipv6 address in any port @@ -365,9 +376,6 @@ class NginxConfigurator(common.Configurator): configuration, and existence of ipv6only directive for specified port :rtype: tuple of type (bool, bool) """ - # port should be a string, but it's easy to mess up, so let's - # make sure it is one - port = str(port) vhosts = self.parser.get_vhosts() ipv6_active = False ipv6only_present = False @@ -377,10 +385,10 @@ class NginxConfigurator(common.Configurator): ipv6_active = True if addr.ipv6only and addr.get_port() == port: ipv6only_present = True - return (ipv6_active, ipv6only_present) + return ipv6_active, ipv6only_present - def _vhost_from_duplicated_default(self, domain: str, allow_port_mismatch: bool, port: str - ) -> obj.VirtualHost: + def _vhost_from_duplicated_default(self, domain: str, allow_port_mismatch: bool, + port: str) -> obj.VirtualHost: """if allow_port_mismatch is False, only server blocks with matching ports will be used as a default server block template. """ @@ -395,7 +403,7 @@ class NginxConfigurator(common.Configurator): self._add_server_name_to_vhost(self.new_vhost, domain) return self.new_vhost - def _add_server_name_to_vhost(self, vhost, domain): + def _add_server_name_to_vhost(self, vhost: obj.VirtualHost, domain: str) -> None: vhost.names.add(domain) name_block = [['\n ', 'server_name']] for name in vhost.names: @@ -403,7 +411,8 @@ class NginxConfigurator(common.Configurator): name_block[0].append(name) self.parser.update_or_add_server_directives(vhost, name_block) - def _get_default_vhost(self, domain, allow_port_mismatch, port): + def _get_default_vhost(self, domain: str, allow_port_mismatch: bool, + port: str) -> obj.VirtualHost: """Helper method for _vhost_from_duplicated_default; see argument documentation there""" vhost_list = self.parser.get_vhosts() # if one has default_server set, return that one @@ -424,10 +433,11 @@ class NginxConfigurator(common.Configurator): # TODO: present a list of vhosts for user to choose from - raise errors.MisconfigurationError("Could not automatically find a matching server" - " block for %s. Set the `server_name` directive to use the Nginx installer." % domain) + raise errors.MisconfigurationError("Could not automatically find a matching server " + f"block for {domain}. Set the `server_name` directive " + "to use the Nginx installer.") - def _get_ranked_matches(self, target_name): + def _get_ranked_matches(self, target_name: str) -> List[Dict[str, Any]]: """Returns a ranked list of vhosts that match target_name. The ranking gives preference to SSL vhosts. @@ -440,7 +450,8 @@ class NginxConfigurator(common.Configurator): vhost_list = self.parser.get_vhosts() return self._rank_matches_by_name_and_ssl(vhost_list, target_name) - def _select_best_name_match(self, matches): + def _select_best_name_match(self, + matches: Sequence[Mapping[str, Any]]) -> Optional[obj.VirtualHost]: """Returns the best name match of a ranked list of vhosts. :param list matches: list of dicts containing the vhost, the matching name, @@ -460,7 +471,8 @@ class NginxConfigurator(common.Configurator): # Exact or regex match return matches[0]['vhost'] - def _rank_matches_by_name(self, vhost_list, target_name): + def _rank_matches_by_name(self, vhost_list: Iterable[obj.VirtualHost], + target_name: str) -> List[Dict[str, Any]]: """Returns a ranked list of vhosts from vhost_list that match target_name. This method should always be followed by a call to _select_best_name_match. @@ -497,7 +509,8 @@ class NginxConfigurator(common.Configurator): 'rank': REGEX_RANK}) return sorted(matches, key=lambda x: x['rank']) - def _rank_matches_by_name_and_ssl(self, vhost_list, target_name): + def _rank_matches_by_name_and_ssl(self, vhost_list: Iterable[obj.VirtualHost], + target_name: str) -> List[Dict[str, Any]]: """Returns a ranked list of vhosts from vhost_list that match target_name. The ranking gives preference to SSLishness before name match level. @@ -610,7 +623,7 @@ class NginxConfigurator(common.Configurator): def _vhost_listening_on_port_no_ssl(self, vhost: obj.VirtualHost, port: str) -> bool: return self._vhost_listening(vhost, port, False) - def _get_redirect_ranked_matches(self, target_name, port): + def _get_redirect_ranked_matches(self, target_name: str, port: str) -> List[Dict[str, Any]]: """Gets a ranked list of plaintextish port-listening vhosts matching target_name Filter all hosts for those listening on port without using ssl. @@ -625,14 +638,14 @@ class NginxConfigurator(common.Configurator): """ all_vhosts = self.parser.get_vhosts() - def _vhost_matches(vhost, port): + def _vhost_matches(vhost: obj.VirtualHost, port: str) -> bool: return self._vhost_listening_on_port_no_ssl(vhost, port) matching_vhosts = [vhost for vhost in all_vhosts if _vhost_matches(vhost, port)] return self._rank_matches_by_name(matching_vhosts, target_name) - def get_all_names(self): + def get_all_names(self) -> Set[str]: """Returns all names found in the Nginx Configuration. :returns: All ServerNames, ServerAliases, and reverse DNS entries for @@ -670,7 +683,7 @@ class NginxConfigurator(common.Configurator): return util.get_filtered_names(all_names) - def _get_snakeoil_paths(self): + def _get_snakeoil_paths(self) -> Tuple[str, str]: """Generate invalid certs that let us create ssl directives for Nginx""" # TODO: generate only once tmp_dir = os.path.join(self.config.work_dir, "snakeoil") @@ -688,7 +701,7 @@ class NginxConfigurator(common.Configurator): cert_file.write(cert_pem) return cert_path, le_key.file - def _make_server_ssl(self, vhost): + def _make_server_ssl(self, vhost: obj.VirtualHost) -> None: """Make a server SSL. Make a server SSL by adding new listen and SSL directives. @@ -698,7 +711,7 @@ class NginxConfigurator(common.Configurator): """ https_port = self.config.https_port - ipv6info = self.ipv6_info(https_port) + ipv6info = self.ipv6_info(str(https_port)) ipv6_block = [''] ipv4_block = [''] @@ -745,11 +758,12 @@ class NginxConfigurator(common.Configurator): ################################## # enhancement methods (Installer) ################################## - def supported_enhancements(self): + def supported_enhancements(self) -> List[str]: """Returns currently supported enhancements.""" return ['redirect', 'ensure-http-header', 'staple-ocsp'] - def enhance(self, domain, enhancement, options=None): + def enhance(self, domain: str, enhancement: str, + options: Optional[Union[str, List[str]]] = None) -> None: """Enhance configuration. :param str domain: domain to enhance @@ -761,16 +775,16 @@ class NginxConfigurator(common.Configurator): """ try: - return self._enhance_func[enhancement](domain, options) + self._enhance_func[enhancement](domain, options) except (KeyError, ValueError): raise errors.PluginError( "Unsupported enhancement: {0}".format(enhancement)) - def _has_certbot_redirect(self, vhost, domain): + def _has_certbot_redirect(self, vhost: obj.VirtualHost, domain: str) -> bool: test_redirect_block = _test_block_from_block(_redirect_block_for_domain(domain)) return vhost.contains_list(test_redirect_block) - def _set_http_header(self, domain, header_substring): + def _set_http_header(self, domain: str, header_substring: Union[str, List[str], None]) -> None: """Enables header identified by header_substring on domain. If the vhost is listening plaintextishly, separates out the relevant @@ -784,7 +798,10 @@ class NginxConfigurator(common.Configurator): :raises .errors.PluginError: If no viable HTTPS host can be created or set with header header_substring. """ - if not header_substring in constants.HEADER_ARGS: + if not isinstance(header_substring, str): + raise errors.NotSupportedError("Invalid header_substring type " + f"{type(header_substring)}, expected a str.") + if header_substring not in constants.HEADER_ARGS: raise errors.NotSupportedError( f"{header_substring} is not supported by the nginx plugin.") @@ -808,7 +825,7 @@ class NginxConfigurator(common.Configurator): ['\n']] self.parser.add_server_directives(vhost, header_directives) - def _add_redirect_block(self, vhost, domain): + def _add_redirect_block(self, vhost: obj.VirtualHost, domain: str) -> None: """Add redirect directive to vhost """ redirect_block = _redirect_block_for_domain(domain) @@ -816,7 +833,8 @@ class NginxConfigurator(common.Configurator): self.parser.add_server_directives( vhost, redirect_block, insert_at_top=True) - def _split_block(self, vhost, only_directives=None): + def _split_block(self, vhost: obj.VirtualHost, only_directives: Optional[List[str]] = None + ) -> Tuple[obj.VirtualHost, obj.VirtualHost]: """Splits this "virtual host" (i.e. this nginx server block) into separate HTTP and HTTPS blocks. @@ -829,13 +847,13 @@ class NginxConfigurator(common.Configurator): """ http_vhost = self.parser.duplicate_vhost(vhost, only_directives=only_directives) - def _ssl_match_func(directive): + def _ssl_match_func(directive: str) -> bool: return 'ssl' in directive - def _ssl_config_match_func(directive): + def _ssl_config_match_func(directive: str) -> bool: return self.mod_ssl_conf in directive - def _no_ssl_match_func(directive): + def _no_ssl_match_func(directive: str) -> bool: return 'ssl' not in directive # remove all ssl addresses and related directives from the new block @@ -849,7 +867,8 @@ class NginxConfigurator(common.Configurator): self.parser.remove_server_directives(vhost, 'listen', match_func=_no_ssl_match_func) return http_vhost, vhost - def _enable_redirect(self, domain, unused_options): + def _enable_redirect(self, domain: str, + unused_options: Optional[Union[str, List[str]]]) -> None: """Redirect all equivalent HTTP traffic to ssl_vhost. If the vhost is listening plaintextishly, separate out the @@ -876,7 +895,7 @@ class NginxConfigurator(common.Configurator): for vhost in vhosts: self._enable_redirect_single(domain, vhost) - def _enable_redirect_single(self, domain, vhost): + def _enable_redirect_single(self, domain: str, vhost: obj.VirtualHost) -> None: """Redirect all equivalent HTTP traffic to ssl_vhost. If the vhost is listening plaintextishly, separate out the @@ -905,7 +924,8 @@ class NginxConfigurator(common.Configurator): logger.info("Redirecting all traffic on port %s to ssl in %s", self.DEFAULT_LISTEN_PORT, vhost.filep) - def _enable_ocsp_stapling(self, domain, chain_path): + def _enable_ocsp_stapling(self, domain: str, + chain_path: Optional[Union[str, List[str]]]) -> None: """Include OCSP response in TLS handshake :param str domain: domain to enable OCSP response for @@ -913,11 +933,15 @@ class NginxConfigurator(common.Configurator): :type chain_path: `str` or `None` """ + if not isinstance(chain_path, str) and chain_path is not None: + raise errors.NotSupportedError(f"Invalid chain_path type {type(chain_path)}, " + "expected a str or None.") vhosts = self.choose_vhosts(domain) for vhost in vhosts: self._enable_ocsp_stapling_single(vhost, chain_path) - def _enable_ocsp_stapling_single(self, vhost, chain_path): + def _enable_ocsp_stapling_single(self, vhost: obj.VirtualHost, + chain_path: Optional[str]) -> None: """Include OCSP response in TLS handshake :param str vhost: vhost to enable OCSP response for @@ -957,7 +981,7 @@ class NginxConfigurator(common.Configurator): ###################################### # Nginx server management (Installer) ###################################### - def restart(self): + def restart(self) -> None: """Restarts nginx server. :raises .errors.MisconfigurationError: If either the reload fails. @@ -965,7 +989,7 @@ class NginxConfigurator(common.Configurator): """ nginx_restart(self.conf('ctl'), self.nginx_conf, self.conf('sleep-seconds')) - def config_test(self): + def config_test(self) -> None: """Check the configuration of Nginx for errors. :raises .errors.MisconfigurationError: If config_test fails @@ -976,7 +1000,7 @@ class NginxConfigurator(common.Configurator): except errors.SubprocessError as err: raise errors.MisconfigurationError(str(err)) - def _nginx_version(self): + def _nginx_version(self) -> str: """Return results of nginx -V :returns: version text @@ -1000,7 +1024,7 @@ class NginxConfigurator(common.Configurator): "Unable to run %s -V" % self.conf('ctl')) return text - def get_version(self): + def get_version(self) -> Tuple[int, ...]: """Return version of Nginx Server. Version is returned as tuple. (ie. 2.4.7 = (2, 4, 7)) @@ -1045,7 +1069,7 @@ class NginxConfigurator(common.Configurator): return nginx_version - def _get_openssl_version(self): + def _get_openssl_version(self) -> str: """Return version of OpenSSL linked to Nginx. Version is returned as string. If no version can be found, empty string is returned. @@ -1067,7 +1091,7 @@ class NginxConfigurator(common.Configurator): return "" return matches[0] - def more_info(self): + def more_info(self) -> str: """Human-readable string to help understand the module""" return ( "Configures Nginx to authenticate and install HTTPS.{0}" @@ -1077,7 +1101,8 @@ class NginxConfigurator(common.Configurator): version=".".join(str(i) for i in self.version)) ) - def auth_hint(self, failed_achalls): # pragma: no cover + def auth_hint(self, # pragma: no cover + failed_achalls: Iterable[achallenges.AnnotatedChallenge]) -> str: return ( "The Certificate Authority failed to verify the temporary nginx configuration changes " "made by Certbot. Ensure the listed domains point to this nginx server and that it is " @@ -1087,7 +1112,7 @@ class NginxConfigurator(common.Configurator): ################################################### # Wrapper functions for Reverter class (Installer) ################################################### - def save(self, title=None, temporary=False): + def save(self, title: str = None, temporary: bool = False) -> None: """Saves all changes to the configuration files. :param str title: The title of the save. If a title is given, the @@ -1111,7 +1136,7 @@ class NginxConfigurator(common.Configurator): if title and not temporary: self.finalize_checkpoint(title) - def recovery_routine(self): + def recovery_routine(self) -> None: """Revert all previously modified files. Reverts all modified files that have not been saved as a checkpoint @@ -1123,7 +1148,7 @@ class NginxConfigurator(common.Configurator): self.new_vhost = None self.parser.load() - def revert_challenge_config(self): + def revert_challenge_config(self) -> None: """Used to cleanup challenge configurations. :raises .errors.PluginError: If unable to revert the challenge config. @@ -1133,7 +1158,7 @@ class NginxConfigurator(common.Configurator): self.new_vhost = None self.parser.load() - def rollback_checkpoints(self, rollback=1): + def rollback_checkpoints(self, rollback: int = 1) -> None: """Rollback saved checkpoints. :param int rollback: Number of checkpoints to revert @@ -1149,12 +1174,13 @@ class NginxConfigurator(common.Configurator): ########################################################################### # Challenges Section for Authenticator ########################################################################### - def get_chall_pref(self, unused_domain): + def get_chall_pref(self, unused_domain: str) -> List[Type[challenges.Challenge]]: """Return list of challenge preferences.""" return [challenges.HTTP01] # Entry point in main.py for performing challenges - def perform(self, achalls): + def perform(self, achalls: List[achallenges.AnnotatedChallenge] + ) -> List[challenges.HTTP01Response]: """Perform the configuration related challenge. This function currently assumes all challenges will be fulfilled. @@ -1163,7 +1189,7 @@ class NginxConfigurator(common.Configurator): """ self._chall_out += len(achalls) - responses = [None] * len(achalls) + responses: List[Optional[challenges.HTTP01Response]] = [None] * len(achalls) http_doer = http_01.NginxHttp01(self) for i, achall in enumerate(achalls): @@ -1183,10 +1209,10 @@ class NginxConfigurator(common.Configurator): for i, resp in enumerate(http_response): responses[http_doer.indices[i]] = resp - return responses + return [response for response in responses if response] # called after challenges are performed - def cleanup(self, achalls): + def cleanup(self, achalls: List[achallenges.AnnotatedChallenge]) -> None: """Revert all challenges.""" self._chall_out -= len(achalls) @@ -1196,13 +1222,13 @@ class NginxConfigurator(common.Configurator): self.restart() -def _test_block_from_block(block): +def _test_block_from_block(block: List[Any]) -> List[Any]: test_block = nginxparser.UnspacedList(block) parser.comment_directive(test_block, 0) return test_block[:-1] -def _redirect_block_for_domain(domain): +def _redirect_block_for_domain(domain: str) -> List[Any]: updated_domain = domain match_symbol = '=' if util.is_wildcard_domain(domain): @@ -1218,7 +1244,7 @@ def _redirect_block_for_domain(domain): return redirect_block -def nginx_restart(nginx_ctl, nginx_conf, sleep_duration): +def nginx_restart(nginx_ctl: str, nginx_conf: str, sleep_duration: int) -> None: """Restarts the Nginx Server. .. todo:: Nginx restart is fatal if the configuration references @@ -1263,10 +1289,10 @@ def nginx_restart(nginx_ctl, nginx_conf, sleep_duration): time.sleep(sleep_duration) -def _determine_default_server_root(): +def _determine_default_server_root() -> str: if os.environ.get("CERTBOT_DOCS") == "1": - default_server_root = "%s or %s" % (constants.LINUX_SERVER_ROOT, - constants.FREEBSD_DARWIN_SERVER_ROOT) + default_server_root = (f"{constants.LINUX_SERVER_ROOT} " + f"or {constants.FREEBSD_DARWIN_SERVER_ROOT}") else: default_server_root = constants.CLI_DEFAULTS["server_root"] return default_server_root diff --git a/certbot-nginx/certbot_nginx/_internal/constants.py b/certbot-nginx/certbot_nginx/_internal/constants.py index a0946b0fd..aa242903e 100644 --- a/certbot-nginx/certbot_nginx/_internal/constants.py +++ b/certbot-nginx/certbot_nginx/_internal/constants.py @@ -52,7 +52,8 @@ ALL_SSL_OPTIONS_HASHES = [ ] """SHA256 hashes of the contents of all versions of MOD_SSL_CONF_SRC""" -def os_constant(key): + +def os_constant(key: str) -> Any: # XXX TODO: In the future, this could return different constants # based on what OS we are running under. To see an # approach to how to handle different OSes, see the @@ -61,11 +62,12 @@ def os_constant(key): """ Get a constant value for operating system - :param key: name of cli constant + :param str key: name of cli constant :return: value of constant for active os """ return CLI_DEFAULTS[key] + HSTS_ARGS = ['\"max-age=31536000\"', ' ', 'always'] HEADER_ARGS = {'Strict-Transport-Security': HSTS_ARGS} diff --git a/certbot-nginx/certbot_nginx/_internal/display_ops.py b/certbot-nginx/certbot_nginx/_internal/display_ops.py index 95163a81f..89483c94a 100644 --- a/certbot-nginx/certbot_nginx/_internal/display_ops.py +++ b/certbot-nginx/certbot_nginx/_internal/display_ops.py @@ -1,12 +1,17 @@ """Contains UI methods for Nginx operations.""" import logging +from typing import Iterable +from typing import List +from typing import Optional + +from certbot_nginx._internal.obj import VirtualHost from certbot.display import util as display_util logger = logging.getLogger(__name__) -def select_vhost_multiple(vhosts): +def select_vhost_multiple(vhosts: Optional[Iterable[VirtualHost]]) -> List[VirtualHost]: """Select multiple Vhosts to install the certificate for :param vhosts: Available Nginx VirtualHosts :type vhosts: :class:`list` of type `~obj.Vhost` @@ -28,7 +33,7 @@ def select_vhost_multiple(vhosts): return [] -def _reversemap_vhosts(names, vhosts): +def _reversemap_vhosts(names: Iterable[str], vhosts: Iterable[VirtualHost]) -> List[VirtualHost]: """Helper function for select_vhost_multiple for mapping string representations back to actual vhost objects""" return_vhosts = [] diff --git a/certbot-nginx/certbot_nginx/_internal/http_01.py b/certbot-nginx/certbot_nginx/_internal/http_01.py index 95592fea0..6f61bfb6f 100644 --- a/certbot-nginx/certbot_nginx/_internal/http_01.py +++ b/certbot-nginx/certbot_nginx/_internal/http_01.py @@ -2,17 +2,20 @@ import io import logging +from typing import Any from typing import List from typing import Optional from typing import TYPE_CHECKING +from certbot_nginx._internal import nginxparser +from certbot_nginx._internal.obj import Addr + from acme import challenges -from certbot import achallenges +from acme.challenges import HTTP01Response from certbot import errors +from certbot.achallenges import KeyAuthorizationAnnotatedChallenge from certbot.compat import os from certbot.plugins import common -from certbot_nginx._internal import nginxparser -from certbot_nginx._internal import obj if TYPE_CHECKING: from certbot_nginx._internal.configurator import NginxConfigurator @@ -46,7 +49,7 @@ class NginxHttp01(common.ChallengePerformer): self.challenge_conf = os.path.join( configurator.config.config_dir, "le_http_01_cert_challenge.conf") - def perform(self): + def perform(self) -> List[HTTP01Response]: """Perform a challenge on Nginx. :returns: list of :class:`certbot.acme.challenges.HTTP01Response` @@ -66,7 +69,7 @@ class NginxHttp01(common.ChallengePerformer): return responses - def _mod_config(self): + def _mod_config(self) -> None: """Modifies Nginx config to include server_names_hash_bucket_size directive and server challenge blocks. @@ -113,39 +116,40 @@ class NginxHttp01(common.ChallengePerformer): with io.open(self.challenge_conf, "w", encoding="utf-8") as new_conf: nginxparser.dump(config, new_conf) - def _default_listen_addresses(self): + def _default_listen_addresses(self) -> List[Addr]: """Finds addresses for a challenge block to listen on. :returns: list of :class:`certbot_nginx._internal.obj.Addr` to apply :rtype: list """ - addresses: List[obj.Addr] = [] + addresses: List[Optional[Addr]] = [] default_addr = "%s" % self.configurator.config.http01_port ipv6_addr = "[::]:{0}".format( self.configurator.config.http01_port) port = self.configurator.config.http01_port - ipv6, ipv6only = self.configurator.ipv6_info(port) + ipv6, ipv6only = self.configurator.ipv6_info(str(port)) if ipv6: # If IPv6 is active in Nginx configuration if not ipv6only: # If ipv6only=on is not already present in the config ipv6_addr = ipv6_addr + " ipv6only=on" - addresses = [obj.Addr.fromstring(default_addr), - obj.Addr.fromstring(ipv6_addr)] + addresses = [Addr.fromstring(default_addr), + Addr.fromstring(ipv6_addr)] logger.debug(("Using default addresses %s and %s for authentication."), default_addr, ipv6_addr) else: - addresses = [obj.Addr.fromstring(default_addr)] + addresses = [Addr.fromstring(default_addr)] logger.debug("Using default address %s for authentication.", default_addr) - return addresses - def _get_validation_path(self, achall): + return [address for address in addresses if address] + + def _get_validation_path(self, achall: KeyAuthorizationAnnotatedChallenge) -> str: return os.sep + os.path.join(challenges.HTTP01.URI_ROOT_PATH, achall.chall.encode("token")) - def _make_server_block(self, achall: achallenges.KeyAuthorizationAnnotatedChallenge) -> List: + def _make_server_block(self, achall: KeyAuthorizationAnnotatedChallenge) -> List[Any]: """Creates a server block for a challenge. :param achall: Annotated HTTP-01 challenge @@ -168,7 +172,8 @@ class NginxHttp01(common.ChallengePerformer): # TODO: do we want to return something else if they otherwise access this block? return [['server'], block] - def _location_directive_for_achall(self, achall): + def _location_directive_for_achall(self, achall: KeyAuthorizationAnnotatedChallenge + ) -> List[Any]: validation = achall.validation(achall.account_key) validation_path = self._get_validation_path(achall) @@ -177,9 +182,8 @@ class NginxHttp01(common.ChallengePerformer): ['return', ' ', '200', ' ', validation]]] return location_directive - - def _make_or_mod_server_block(self, achall: achallenges.KeyAuthorizationAnnotatedChallenge - ) -> Optional[List]: + def _make_or_mod_server_block(self, achall: KeyAuthorizationAnnotatedChallenge + ) -> Optional[List[Any]]: """Modifies server blocks to respond to a challenge. Returns a new HTTP server block to add to the configuration if an existing one can't be found. @@ -192,7 +196,7 @@ class NginxHttp01(common.ChallengePerformer): """ http_vhosts, https_vhosts = self.configurator.choose_auth_vhosts(achall.domain) - new_vhost: Optional[list] = None + new_vhost: Optional[List[Any]] = None if not http_vhosts: # Couldn't find either a matching name+port server block # or a port+default_server block, so create a dummy block @@ -205,8 +209,8 @@ class NginxHttp01(common.ChallengePerformer): self.configurator.parser.add_server_directives(vhost, location_directive) rewrite_directive = [['rewrite', ' ', '^(/.well-known/acme-challenge/.*)', - ' ', '$1', ' ', 'break']] - self.configurator.parser.add_server_directives(vhost, - rewrite_directive, insert_at_top=True) + ' ', '$1', ' ', 'break']] + self.configurator.parser.add_server_directives( + vhost, rewrite_directive, insert_at_top=True) return new_vhost diff --git a/certbot-nginx/certbot_nginx/_internal/nginxparser.py b/certbot-nginx/certbot_nginx/_internal/nginxparser.py index 03aa88db4..70a55be3a 100644 --- a/certbot-nginx/certbot_nginx/_internal/nginxparser.py +++ b/certbot-nginx/certbot_nginx/_internal/nginxparser.py @@ -2,14 +2,23 @@ # Forked from https://github.com/fatiherikli/nginxparser (MIT Licensed) import copy import logging +import typing from typing import Any from typing import IO +from typing import Iterable +from typing import Iterator +from typing import List +from typing import overload +from typing import Tuple +from typing import TYPE_CHECKING +from typing import Union from pyparsing import Combine from pyparsing import Forward from pyparsing import Group from pyparsing import Literal from pyparsing import Optional +from pyparsing import ParseResults from pyparsing import QuotedString from pyparsing import Regex from pyparsing import restOfLine @@ -17,6 +26,9 @@ from pyparsing import stringEnd from pyparsing import White from pyparsing import ZeroOrMore +if TYPE_CHECKING: + from typing_extensions import SupportsIndex # typing.SupportsIndex not supported on Python 3.6 + logger = logging.getLogger(__name__) @@ -59,23 +71,24 @@ class RawNginxParser: script = ZeroOrMore(contents) + space + stringEnd script.parseWithTabs().leaveWhitespace() - def __init__(self, source): + def __init__(self, source: str) -> None: self.source = source - def parse(self): + def parse(self) -> ParseResults: """Returns the parsed tree.""" return self.script.parseString(self.source) - def as_list(self): + def as_list(self) -> List[Any]: """Returns the parsed tree as a list.""" return self.parse().asList() + class RawNginxDumper: """A class that dumps nginx configuration from the provided tree.""" - def __init__(self, blocks): + def __init__(self, blocks: List[Any]) -> None: self.blocks = blocks - def __iter__(self, blocks=None): + def __iter__(self, blocks: typing.Optional[List[Any]] = None) -> Iterator[str]: """Iterates the dumped nginx content.""" blocks = blocks or self.blocks for b0 in blocks: @@ -100,7 +113,7 @@ class RawNginxDumper: semicolon = "" yield "".join(item) + semicolon - def __str__(self): + def __str__(self) -> str: """Return the parsed block as a string.""" return ''.join(self) @@ -108,10 +121,10 @@ class RawNginxDumper: spacey = lambda x: (isinstance(x, str) and x.isspace()) or x == '' -class UnspacedList(list): +class UnspacedList(List[Any]): """Wrap a list [of lists], making any whitespace entries magically invisible""" - def __init__(self, list_source): + def __init__(self, list_source: Iterable[Any]) -> None: # ensure our argument is not a generator, and duplicate any sublists self.spaced = copy.deepcopy(list(list_source)) self.dirty = False @@ -122,14 +135,23 @@ class UnspacedList(list): for i, entry in reversed(list(enumerate(self))): if isinstance(entry, list): sublist = UnspacedList(entry) - list.__setitem__(self, i, sublist) + super().__setitem__(i, sublist) self.spaced[i] = sublist.spaced elif spacey(entry): # don't delete comments if "#" not in self[:i]: - list.__delitem__(self, i) + super().__delitem__(i) - def _coerce(self, inbound): + @overload + def _coerce(self, inbound: None) -> Tuple[None, None]: ... + + @overload + def _coerce(self, inbound: str) -> Tuple[str, str]: ... + + @overload + def _coerce(self, inbound: List[Any]) -> Tuple["UnspacedList", List[Any]]: ... + + def _coerce(self, inbound: Any) -> Tuple[Any, Any]: """ Coerce some inbound object to be appropriately usable in this object @@ -138,100 +160,114 @@ class UnspacedList(list): :rtype: tuple """ - if not isinstance(inbound, list): # str or None + if not isinstance(inbound, list): # str or None return inbound, inbound else: if not hasattr(inbound, "spaced"): inbound = UnspacedList(inbound) return inbound, inbound.spaced - def insert(self, i, x): + def insert(self, i: int, x: Any) -> None: + """Insert object before index.""" item, spaced_item = self._coerce(x) slicepos = self._spaced_position(i) if i < len(self) else len(self.spaced) self.spaced.insert(slicepos, spaced_item) if not spacey(item): - list.insert(self, i, item) + super().insert(i, item) self.dirty = True - def append(self, x): + def append(self, x: Any) -> None: + """Append object to the end of the list.""" item, spaced_item = self._coerce(x) self.spaced.append(spaced_item) if not spacey(item): - list.append(self, item) + super().append(item) self.dirty = True - def extend(self, x): + def extend(self, x: Any) -> None: + """Extend list by appending elements from the iterable.""" item, spaced_item = self._coerce(x) self.spaced.extend(spaced_item) - list.extend(self, item) + super().extend(item) self.dirty = True - def __add__(self, other): - l = copy.deepcopy(self) - l.extend(other) - l.dirty = True - return l + def __add__(self, other: List[Any]) -> "UnspacedList": + new_list = copy.deepcopy(self) + new_list.extend(other) + new_list.dirty = True + return new_list - def pop(self, _i=None): + def pop(self, *args: Any, **kwargs: Any) -> None: + """Function pop() is not implemented for UnspacedList""" raise NotImplementedError("UnspacedList.pop() not yet implemented") - def remove(self, _): + + def remove(self, *args: Any, **kwargs: Any) -> None: + """Function remove() is not implemented for UnspacedList""" raise NotImplementedError("UnspacedList.remove() not yet implemented") - def reverse(self): + + def reverse(self) -> None: + """Function reverse() is not implemented for UnspacedList""" raise NotImplementedError("UnspacedList.reverse() not yet implemented") - def sort(self, _cmp=None, _key=None, _Rev=None): + + def sort(self, *_args: Any, **_kwargs: Any) -> None: + """Function sort() is not implemented for UnspacedList""" raise NotImplementedError("UnspacedList.sort() not yet implemented") - def __setslice__(self, _i, _j, _newslice): + + def __setslice__(self, *args: Any, **kwargs: Any) -> None: raise NotImplementedError("Slice operations on UnspacedLists not yet implemented") - def __setitem__(self, i, value): + def __setitem__(self, i: Union["SupportsIndex", slice], value: Any) -> None: if isinstance(i, slice): raise NotImplementedError("Slice operations on UnspacedLists not yet implemented") item, spaced_item = self._coerce(value) self.spaced.__setitem__(self._spaced_position(i), spaced_item) if not spacey(item): - list.__setitem__(self, i, item) + super().__setitem__(i, item) self.dirty = True - def __delitem__(self, i): + def __delitem__(self, i: Union["SupportsIndex", slice]) -> None: + if isinstance(i, slice): + raise NotImplementedError("Slice operations on UnspacedLists not yet implemented") self.spaced.__delitem__(self._spaced_position(i)) - list.__delitem__(self, i) + super().__delitem__(i) self.dirty = True - def __deepcopy__(self, memo): + def __deepcopy__(self, memo: Any) -> "UnspacedList": new_spaced = copy.deepcopy(self.spaced, memo=memo) - l = UnspacedList(new_spaced) - l.dirty = self.dirty - return l + new_list = UnspacedList(new_spaced) + new_list.dirty = self.dirty + return new_list - def is_dirty(self): + def is_dirty(self) -> bool: """Recurse through the parse tree to figure out if any sublists are dirty""" if self.dirty: return True return any((isinstance(x, UnspacedList) and x.is_dirty() for x in self)) - def _spaced_position(self, idx): - "Convert from indexes in the unspaced list to positions in the spaced one" + def _spaced_position(self, idx: "SupportsIndex") -> int: + """Convert from indexes in the unspaced list to positions in the spaced one""" + int_idx = idx.__index__() pos = spaces = 0 # Normalize indexes like list[-1] etc, and save the result - if idx < 0: - idx = len(self) + idx - if not 0 <= idx < len(self): + if int_idx < 0: + int_idx = len(self) + int_idx + if not 0 <= int_idx < len(self): raise IndexError("list index out of range") - idx0 = idx - # Count the number of spaces in the spaced list before idx in the unspaced one - while idx != -1: + int_idx0 = int_idx + # Count the number of spaces in the spaced list before int_idx in the unspaced one + while int_idx != -1: if spacey(self.spaced[pos]): spaces += 1 else: - idx -= 1 + int_idx -= 1 pos += 1 - return idx0 + spaces + return int_idx0 + spaces # Shortcut functions to respect Python's serialization interface # (like pyyaml, picker or json) -def loads(source): +def loads(source: str) -> UnspacedList: """Parses from a string. :param str source: The string to parse @@ -242,34 +278,34 @@ def loads(source): return UnspacedList(RawNginxParser(source).as_list()) -def load(_file): +def load(file_: IO[Any]) -> UnspacedList: """Parses from a file. - :param file _file: The file to parse + :param file file_: The file to parse :returns: The parsed tree :rtype: list """ - return loads(_file.read()) + return loads(file_.read()) def dumps(blocks: UnspacedList) -> str: """Dump to a Unicode string. - :param UnspacedList block: The parsed tree + :param UnspacedList blocks: The parsed tree :rtype: six.text_type """ return str(RawNginxDumper(blocks.spaced)) -def dump(blocks: UnspacedList, _file: IO[Any]) -> None: +def dump(blocks: UnspacedList, file_: IO[Any]) -> None: """Dump to a file. - :param UnspacedList block: The parsed tree - :param IO[Any] _file: The file stream to dump to. It must be opened with + :param UnspacedList blocks: The parsed tree + :param IO[Any] file_: The file stream to dump to. It must be opened with Unicode encoding. :rtype: None """ - _file.write(dumps(blocks)) + file_.write(dumps(blocks)) diff --git a/certbot-nginx/certbot_nginx/_internal/obj.py b/certbot-nginx/certbot_nginx/_internal/obj.py index 44be0e598..1e0dbba1a 100644 --- a/certbot-nginx/certbot_nginx/_internal/obj.py +++ b/certbot-nginx/certbot_nginx/_internal/obj.py @@ -1,10 +1,17 @@ """Module contains classes used by the Nginx Configurator.""" import re +from typing import Any +from typing import List +from typing import Optional +from typing import Sequence +from typing import Set +from typing import Union from certbot.plugins import common ADD_HEADER_DIRECTIVE = 'add_header' + class Addr(common.Addr): r"""Represents an Nginx address, i.e. what comes after the 'listen' directive. @@ -34,7 +41,8 @@ class Addr(common.Addr): UNSPECIFIED_IPV4_ADDRESSES = ('', '*', '0.0.0.0') CANONICAL_UNSPECIFIED_ADDRESS = UNSPECIFIED_IPV4_ADDRESSES[0] - def __init__(self, host, port, ssl, default, ipv6, ipv6only): + def __init__(self, host: str, port: str, ssl: bool, default: bool, + ipv6: bool, ipv6only: bool) -> None: super().__init__((host, port)) self.ssl = ssl self.default = default @@ -43,7 +51,7 @@ class Addr(common.Addr): self.unspecified_address = host in self.UNSPECIFIED_IPV4_ADDRESSES @classmethod - def fromstring(cls, str_addr): + def fromstring(cls, str_addr: str) -> Optional["Addr"]: """Initialize Addr from string.""" parts = str_addr.split(' ') ssl = False @@ -94,7 +102,7 @@ class Addr(common.Addr): return cls(host, port, ssl, default, ipv6, ipv6only) - def to_string(self, include_default=True): + def to_string(self, include_default: bool = True) -> str: """Return string representation of Addr""" parts = '' if self.tup[0] and self.tup[1]: @@ -111,18 +119,18 @@ class Addr(common.Addr): return parts - def __str__(self): + def __str__(self) -> str: return self.to_string() - def __repr__(self): + def __repr__(self) -> str: return "Addr(" + self.__str__() + ")" - def __hash__(self): # pylint: disable=useless-super-delegation + def __hash__(self) -> int: # pylint: disable=useless-super-delegation # Python 3 requires explicit overridden for __hash__ # See certbot-apache/certbot_apache/_internal/obj.py for more information return super().__hash__() - def super_eq(self, other): + def super_eq(self, other: "Addr") -> bool: """Check ip/port equality, with IPv6 support. """ # If both addresses got an unspecified address, then make sure the @@ -134,7 +142,7 @@ class Addr(common.Addr): other.tup[1]), other.ipv6) return super().__eq__(other) - def __eq__(self, other): + def __eq__(self, other: Any) -> bool: if isinstance(other, self.__class__): return (self.super_eq(other) and self.ssl == other.ssl and @@ -158,7 +166,8 @@ class VirtualHost: """ - def __init__(self, filep, addrs, ssl, enabled, names, raw, path): + def __init__(self, filep: str, addrs: Sequence[Addr], ssl: bool, enabled: bool, + names: Set[str], raw: List[Any], path: List[int]) -> None: """Initialize a VH.""" self.filep = filep self.addrs = addrs @@ -168,7 +177,7 @@ class VirtualHost: self.raw = raw self.path = path - def __str__(self): + def __str__(self) -> str: addr_str = ", ".join(str(addr) for addr in sorted(self.addrs, key=str)) # names might be a set, and it has different representations in Python # 2 and 3. Force it to be a list here for consistent outputs @@ -179,10 +188,10 @@ class VirtualHost: "enabled: %s" % (self.filep, addr_str, list(self.names), self.ssl, self.enabled)) - def __repr__(self): + def __repr__(self) -> str: return "VirtualHost(" + self.__str__().replace("\n", ", ") + ")\n" - def __eq__(self, other): + def __eq__(self, other: Any) -> bool: if isinstance(other, self.__class__): return (self.filep == other.filep and sorted(self.addrs, key=str) == sorted(other.addrs, key=str) and @@ -193,12 +202,12 @@ class VirtualHost: return False - def __hash__(self): + def __hash__(self) -> int: return hash((self.filep, tuple(self.path), tuple(self.addrs), tuple(self.names), self.ssl, self.enabled)) - def has_header(self, header_name): + def has_header(self, header_name: str) -> bool: """Determine if this server block has a particular header set. :param str header_name: The name of the header to check for, e.g. 'Strict-Transport-Security' @@ -206,7 +215,7 @@ class VirtualHost: found = _find_directive(self.raw, ADD_HEADER_DIRECTIVE, header_name) return found is not None - def contains_list(self, test): + def contains_list(self, test: List[Any]) -> bool: """Determine if raw server block contains test list at top level """ for i in range(0, len(self.raw) - len(test) + 1): @@ -214,7 +223,7 @@ class VirtualHost: return True return False - def ipv6_enabled(self): + def ipv6_enabled(self) -> bool: """Return true if one or more of the listen directives in vhost supports IPv6""" for a in self.addrs: @@ -222,7 +231,7 @@ class VirtualHost: return True return False - def ipv4_enabled(self): + def ipv4_enabled(self) -> bool: """Return true if one or more of the listen directives in vhost are IPv4 only""" if not self.addrs: @@ -232,7 +241,7 @@ class VirtualHost: return True return False - def display_repr(self): + def display_repr(self) -> str: """Return a representation of VHost to be used in dialog""" return ( "File: {filename}\n" @@ -244,7 +253,9 @@ class VirtualHost: names=", ".join(self.names), https="Yes" if self.ssl else "No")) -def _find_directive(directives, directive_name, match_content=None): + +def _find_directive(directives: Optional[Union[str, List[Any]]], directive_name: str, + match_content: Optional[Any] = None) -> Optional[Any]: """Find a directive of type directive_name in directives. If match_content is given, Searches for `match_content` in the directive arguments. """ diff --git a/certbot-nginx/certbot_nginx/_internal/parser.py b/certbot-nginx/certbot_nginx/_internal/parser.py index 83ec4c923..e60d66eb2 100644 --- a/certbot-nginx/certbot_nginx/_internal/parser.py +++ b/certbot-nginx/certbot_nginx/_internal/parser.py @@ -5,20 +5,26 @@ import glob import io import logging import re +from typing import Any +from typing import Callable +from typing import cast from typing import Dict +from typing import Iterable from typing import List +from typing import Mapping from typing import Optional +from typing import Sequence from typing import Set from typing import Tuple from typing import Union +from certbot_nginx._internal import nginxparser +from certbot_nginx._internal import obj +from certbot_nginx._internal.nginxparser import UnspacedList import pyparsing from certbot import errors from certbot.compat import os -from certbot_nginx._internal import nginxparser -from certbot_nginx._internal import obj -from certbot_nginx._internal.nginxparser import UnspacedList logger = logging.getLogger(__name__) @@ -32,8 +38,8 @@ class NginxParser: """ - def __init__(self, root): - self.parsed: Dict[str, Union[List, nginxparser.UnspacedList]] = {} + def __init__(self, root: str) -> None: + self.parsed: Dict[str, UnspacedList] = {} self.root = os.path.abspath(root) self.config_root = self._find_config_root() @@ -42,14 +48,14 @@ class NginxParser: # not enable sites from there. self.load() - def load(self): + def load(self) -> None: """Loads Nginx files into a parsed tree. """ self.parsed = {} self._parse_recursively(self.config_root) - def _parse_recursively(self, filepath): + def _parse_recursively(self, filepath: str) -> None: """Parses nginx config files recursively by looking at 'include' directives inside 'http' and 'server' blocks. Note that this only reads Nginx files that potentially declare a virtual host. @@ -77,7 +83,7 @@ class NginxParser: if _is_include_directive(server_entry): self._parse_recursively(server_entry[1]) - def abs_path(self, path): + def abs_path(self, path: str) -> str: """Converts a relative path to an absolute path relative to the root. Does nothing for paths that are already absolute. @@ -90,7 +96,7 @@ class NginxParser: return os.path.normpath(os.path.join(self.root, path)) return os.path.normpath(path) - def _build_addr_to_ssl(self): + def _build_addr_to_ssl(self) -> Dict[Tuple[str, str], bool]: """Builds a map from address to whether it listens on ssl in any server block """ servers = self._get_raw_servers() @@ -107,11 +113,11 @@ class NginxParser: addr_to_ssl[addr_tuple] = addr.ssl or addr_to_ssl[addr_tuple] return addr_to_ssl - def _get_raw_servers(self) -> Dict: + def _get_raw_servers(self) -> Dict[str, Union[List[Any], UnspacedList]]: # pylint: disable=cell-var-from-loop """Get a map of unparsed all server blocks """ - servers: Dict[str, Union[List, nginxparser.UnspacedList]] = {} + servers: Dict[str, Union[List[Any], nginxparser.UnspacedList]] = {} for filename, tree in self.parsed.items(): servers[filename] = [] srv = servers[filename] # workaround undefined loop var in lambdas @@ -126,7 +132,7 @@ class NginxParser: servers[filename][i] = (new_server, path) return servers - def get_vhosts(self): + def get_vhosts(self) -> List[obj.VirtualHost]: """Gets list of all 'virtual hosts' found in Nginx configuration. Technically this is a misnomer because Nginx does not have virtual hosts, it has 'server blocks'. @@ -158,7 +164,7 @@ class NginxParser: return vhosts - def _update_vhosts_addrs_ssl(self, vhosts): + def _update_vhosts_addrs_ssl(self, vhosts: Iterable[obj.VirtualHost]) -> None: """Update a list of raw parsed vhosts to include global address sslishness """ addr_to_ssl = self._build_addr_to_ssl() @@ -168,7 +174,7 @@ class NginxParser: if addr.ssl: vhost.ssl = True - def _get_included_directives(self, block): + def _get_included_directives(self, block: UnspacedList) -> UnspacedList: """Returns array with the "include" directives expanded out by concatenating the contents of the included file to the block. @@ -188,7 +194,7 @@ class NginxParser: pass return result - def _parse_files(self, filepath, override=False): + def _parse_files(self, filepath: str, override: bool = False) -> List[UnspacedList]: """Parse files from a glob :param str filepath: Nginx config file path @@ -219,7 +225,7 @@ class NginxParser: logger.warning("Could not parse file: %s due to %s", item, err) return trees - def _find_config_root(self): + def _find_config_root(self) -> str: """Return the Nginx Configuration Root file.""" location = ['nginx.conf'] @@ -230,7 +236,7 @@ class NginxParser: raise errors.NoInstallationError( "Could not find Nginx root configuration file (nginx.conf)") - def filedump(self, ext='tmp', lazy=True): + def filedump(self, ext: str = 'tmp', lazy: bool = True) -> None: """Dumps parsed configurations into files. :param str ext: The file extension to use for the dumped files. If @@ -255,7 +261,7 @@ class NginxParser: except IOError: logger.error("Could not open file for writing: %s", filename) - def parse_server(self, server): + def parse_server(self, server: UnspacedList) -> Dict[str, Any]: """Parses a list of server directives, accounting for global address sslishness. :param list server: list of directives in a server block @@ -266,7 +272,7 @@ class NginxParser: _apply_global_addr_ssl(addr_to_ssl, parsed_server) return parsed_server - def has_ssl_on_directive(self, vhost): + def has_ssl_on_directive(self, vhost: obj.VirtualHost) -> bool: """Does vhost have ssl on for all ports? :param :class:`~certbot_nginx._internal.obj.VirtualHost` vhost: The vhost in question @@ -284,7 +290,8 @@ class NginxParser: return False - def add_server_directives(self, vhost, directives, insert_at_top=False): + def add_server_directives(self, vhost: obj.VirtualHost, directives: List[Any], + insert_at_top: bool = False) -> None: """Add directives to the server block identified by vhost. This method modifies vhost to be fully consistent with the new directives. @@ -305,7 +312,8 @@ class NginxParser: self._modify_server_directives(vhost, functools.partial(_add_directives, directives, insert_at_top)) - def update_or_add_server_directives(self, vhost, directives, insert_at_top=False): + def update_or_add_server_directives(self, vhost: obj.VirtualHost, directives: List[Any], + insert_at_top: bool = False) -> None: """Add or replace directives in the server block identified by vhost. This method modifies vhost to be fully consistent with the new directives. @@ -327,7 +335,8 @@ class NginxParser: self._modify_server_directives(vhost, functools.partial(_update_or_add_directives, directives, insert_at_top)) - def remove_server_directives(self, vhost, directive_name, match_func=None): + def remove_server_directives(self, vhost: obj.VirtualHost, directive_name: str, + match_func: Optional[Callable[[Any], bool]] = None) -> None: """Remove all directives of type directive_name. :param :class:`~certbot_nginx._internal.obj.VirtualHost` vhost: The vhost @@ -339,7 +348,8 @@ class NginxParser: self._modify_server_directives(vhost, functools.partial(_remove_directives, directive_name, match_func)) - def _update_vhost_based_on_new_directives(self, vhost, directives_list): + def _update_vhost_based_on_new_directives(self, vhost: obj.VirtualHost, + directives_list: UnspacedList) -> None: new_server = self._get_included_directives(directives_list) parsed_server = self.parse_server(new_server) vhost.addrs = parsed_server['addrs'] @@ -347,7 +357,8 @@ class NginxParser: vhost.names = parsed_server['names'] vhost.raw = new_server - def _modify_server_directives(self, vhost, block_func): + def _modify_server_directives(self, vhost: obj.VirtualHost, + block_func: Callable[[List[Any]], None]) -> None: filename = vhost.filep try: result = self.parsed[filename] @@ -364,7 +375,7 @@ class NginxParser: def duplicate_vhost(self, vhost_template: obj.VirtualHost, remove_singleton_listen_params: bool = False, - only_directives: Optional[List] = None) -> obj.VirtualHost: + only_directives: Optional[List[Any]] = None) -> obj.VirtualHost: """Duplicate the vhost in the configuration files. :param :class:`~certbot_nginx._internal.obj.VirtualHost` vhost_template: The vhost @@ -417,7 +428,7 @@ class NginxParser: return new_vhost -def _parse_ssl_options(ssl_options): +def _parse_ssl_options(ssl_options: Optional[str]) -> List[UnspacedList]: if ssl_options is not None: try: with io.open(ssl_options, "r", encoding="utf-8") as _file: @@ -429,9 +440,12 @@ def _parse_ssl_options(ssl_options): "Only UTF-8 encoding is supported.", ssl_options) except pyparsing.ParseBaseException as err: logger.warning("Could not parse file: %s due to %s", ssl_options, err) - return [] + return UnspacedList([]) -def _do_for_subarray(entry, condition, func, path=None): + +def _do_for_subarray(entry: List[Any], condition: Callable[[List[Any]], bool], + func: Callable[[List[Any], List[int]], None], + path: Optional[List[int]] = None) -> None: """Executes a function for a subarray of a nested array if it matches the given condition. @@ -450,7 +464,7 @@ def _do_for_subarray(entry, condition, func, path=None): _do_for_subarray(item, condition, func, path + [index]) -def get_best_match(target_name, names): +def get_best_match(target_name: str, names: Iterable[str]) -> Tuple[Optional[str], Optional[str]]: """Finds the best match for target_name out of names using the Nginx name-matching rules (exact > longest wildcard starting with * > longest wildcard ending with * > regex). @@ -479,29 +493,29 @@ def get_best_match(target_name, names): if exact: # There can be more than one exact match; e.g. eff.org, .eff.org match = min(exact, key=len) - return ('exact', match) + return 'exact', match if wildcard_start: # Return the longest wildcard match = max(wildcard_start, key=len) - return ('wildcard_start', match) + return 'wildcard_start', match if wildcard_end: # Return the longest wildcard match = max(wildcard_end, key=len) - return ('wildcard_end', match) + return 'wildcard_end', match if regex: # Just return the first one for now match = regex[0] - return ('regex', match) + return 'regex', match - return (None, None) + return None, None -def _exact_match(target_name, name): +def _exact_match(target_name: str, name: str) -> bool: target_lower = target_name.lower() return name.lower() in (target_lower, '.' + target_lower) -def _wildcard_match(target_name, name, start): +def _wildcard_match(target_name: str, name: str, start: bool) -> bool: # Degenerate case if name == '*': return True @@ -526,7 +540,7 @@ def _wildcard_match(target_name, name, start): return target_name_lower.endswith('.' + name_lower) -def _regex_match(target_name, name): +def _regex_match(target_name: str, name: str) -> bool: # Must start with a tilde if len(name) < 2 or name[0] != '~': return False @@ -534,13 +548,13 @@ def _regex_match(target_name, name): # After tilde is a perl-compatible regex try: regex = re.compile(name[1:]) - return re.match(regex, target_name) + return bool(re.match(regex, target_name)) except re.error: # pragma: no cover # perl-compatible regexes are sometimes not recognized by python return False -def _is_include_directive(entry): +def _is_include_directive(entry: Any) -> bool: """Checks if an nginx parsed entry is an 'include' directive. :param list entry: the parsed entry @@ -552,7 +566,8 @@ def _is_include_directive(entry): len(entry) == 2 and entry[0] == 'include' and isinstance(entry[1], str)) -def _is_ssl_on_directive(entry): + +def _is_ssl_on_directive(entry: Any) -> bool: """Checks if an nginx parsed entry is an 'ssl on' directive. :param list entry: the parsed entry @@ -564,14 +579,18 @@ def _is_ssl_on_directive(entry): len(entry) == 2 and entry[0] == 'ssl' and entry[1] == 'on') -def _add_directives(directives, insert_at_top, block): + +def _add_directives(directives: List[Any], insert_at_top: bool, + block: UnspacedList) -> None: """Adds directives to a config block.""" for directive in directives: _add_directive(block, directive, insert_at_top) if block and '\n' not in block[-1]: # could be " \n " or ["\n"] ! block.append(nginxparser.UnspacedList('\n')) -def _update_or_add_directives(directives, insert_at_top, block): + +def _update_or_add_directives(directives: List[Any], insert_at_top: bool, + block: UnspacedList) -> None: """Adds or replaces directives in a config block.""" for directive in directives: _update_or_add_directive(block, directive, insert_at_top) @@ -584,7 +603,8 @@ REPEATABLE_DIRECTIVES = {'server_name', 'listen', INCLUDE, 'rewrite', 'add_heade COMMENT = ' managed by Certbot' COMMENT_BLOCK = [' ', '#', COMMENT] -def comment_directive(block, location): + +def comment_directive(block: UnspacedList, location: int) -> None: """Add a ``#managed by Certbot`` comment to the end of the line at location. :param list block: The block containing the directive to be commented @@ -603,40 +623,45 @@ def comment_directive(block, location): if next_entry is not None and "\n" not in next_entry: block.insert(location + 2, '\n') -def _comment_out_directive(block, location, include_location): + +def _comment_out_directive(block: UnspacedList, location: int, include_location: str) -> None: """Comment out the line at location, with a note of explanation.""" comment_message = ' duplicated in {0}'.format(include_location) # add the end comment # create a dumpable object out of block[location] (so it includes the ;) directive = block[location] - new_dir_block = nginxparser.UnspacedList([]) # just a wrapper + new_dir_block = nginxparser.UnspacedList([]) # just a wrapper new_dir_block.append(directive) dumped = nginxparser.dumps(new_dir_block) - commented = dumped + ' #' + comment_message # add the comment directly to the one-line string - new_dir = nginxparser.loads(commented) # reload into UnspacedList + commented = dumped + ' #' + comment_message # add the comment directly to the one-line string + new_dir = nginxparser.loads(commented) # reload into UnspacedList # add the beginning comment insert_location = 0 - if new_dir[0].spaced[0] != new_dir[0][0]: # if there's whitespace at the beginning + if new_dir[0].spaced[0] != new_dir[0][0]: # if there's whitespace at the beginning insert_location = 1 - new_dir[0].spaced.insert(insert_location, "# ") # comment out the line - new_dir[0].spaced.append(";") # directly add in the ;, because now dumping won't work properly + new_dir[0].spaced.insert(insert_location, "# ") # comment out the line + new_dir[0].spaced.append(";") # directly add in the ;, because now dumping won't work properly dumped = nginxparser.dumps(new_dir) - new_dir = nginxparser.loads(dumped) # reload into an UnspacedList + new_dir = nginxparser.loads(dumped) # reload into an UnspacedList block[location] = new_dir[0] # set the now-single-line-comment directive back in place -def _find_location(block, directive_name, match_func=None): + +def _find_location(block: UnspacedList, directive_name: str, + match_func: Optional[Callable[[Any], bool]] = None) -> Optional[int]: """Finds the index of the first instance of directive_name in block. If no line exists, use None.""" - return next((index for index, line in enumerate(block) \ - if line and line[0] == directive_name and (match_func is None or match_func(line))), None) + return next((index for index, line in enumerate(block) if ( + line and line[0] == directive_name and (match_func is None or match_func(line)))), None) -def _is_whitespace_or_comment(directive): + +def _is_whitespace_or_comment(directive: Sequence[Any]) -> bool: """Is this directive either a whitespace or comment directive?""" return len(directive) == 0 or directive[0] == '#' -def _add_directive(block, directive, insert_at_top): + +def _add_directive(block: UnspacedList, directive: Sequence[Any], insert_at_top: bool) -> None: if not isinstance(directive, nginxparser.UnspacedList): directive = nginxparser.UnspacedList(directive) if _is_whitespace_or_comment(directive): @@ -653,10 +678,11 @@ def _add_directive(block, directive, insert_at_top): # handle flat include files directive_name = directive[0] - def can_append(loc, dir_name): + + def can_append(loc: Optional[int], dir_name: str) -> bool: """ Can we append this directive to the block? """ return loc is None or (isinstance(dir_name, str) - and dir_name in REPEATABLE_DIRECTIVES) + and dir_name in REPEATABLE_DIRECTIVES) err_fmt = 'tried to insert directive "{0}" but found conflicting "{1}".' @@ -672,10 +698,14 @@ def _add_directive(block, directive, insert_at_top): included_dir_name = included_directive[0] if (not _is_whitespace_or_comment(included_directive) and not can_append(included_dir_loc, included_dir_name)): - if block[included_dir_loc] != included_directive: - raise errors.MisconfigurationError(err_fmt.format(included_directive, - block[included_dir_loc])) - _comment_out_directive(block, included_dir_loc, directive[1]) + + # By construction of can_append(), included_dir_loc cannot be None at that point + resolved_included_dir_loc = cast(int, included_dir_loc) + + if block[resolved_included_dir_loc] != included_directive: + raise errors.MisconfigurationError(err_fmt.format( + included_directive, block[resolved_included_dir_loc])) + _comment_out_directive(block, resolved_included_dir_loc, directive[1]) if can_append(location, directive_name): if insert_at_top: @@ -687,14 +717,22 @@ def _add_directive(block, directive, insert_at_top): else: block.append(directive) comment_directive(block, len(block) - 1) - elif block[location] != directive: - raise errors.MisconfigurationError(err_fmt.format(directive, block[location])) + return -def _update_directive(block, directive, location): + # By construction of can_append(), location cannot be None at that point + resolved_location = cast(int, location) + + if block[resolved_location] != directive: + raise errors.MisconfigurationError(err_fmt.format(directive, block[resolved_location])) + + +def _update_directive(block: UnspacedList, directive: Sequence[Any], location: int) -> None: block[location] = directive comment_directive(block, location) -def _update_or_add_directive(block, directive, insert_at_top): + +def _update_or_add_directive(block: UnspacedList, directive: Sequence[Any], + insert_at_top: bool) -> None: if not isinstance(directive, nginxparser.UnspacedList): directive = nginxparser.UnspacedList(directive) if _is_whitespace_or_comment(directive): @@ -711,10 +749,13 @@ def _update_or_add_directive(block, directive, insert_at_top): _add_directive(block, directive, insert_at_top) -def _is_certbot_comment(directive): + +def _is_certbot_comment(directive: Sequence[Any]) -> bool: return '#' in directive and COMMENT in directive -def _remove_directives(directive_name, match_func, block): + +def _remove_directives(directive_name: str, match_func: Callable[[Any], bool], + block: UnspacedList) -> None: """Removes directives of name directive_name from a config block if match_func matches. """ while True: @@ -726,7 +767,9 @@ def _remove_directives(directive_name, match_func, block): del block[location + 1] del block[location] -def _apply_global_addr_ssl(addr_to_ssl, parsed_server): + +def _apply_global_addr_ssl(addr_to_ssl: Mapping[Tuple[str, str], bool], + parsed_server: Dict[str, Any]) -> None: """Apply global sslishness information to the parsed server block """ for addr in parsed_server['addrs']: @@ -734,7 +777,8 @@ def _apply_global_addr_ssl(addr_to_ssl, parsed_server): if addr.ssl: parsed_server['ssl'] = True -def _parse_server_raw(server): + +def _parse_server_raw(server: UnspacedList) -> Dict[str, Any]: """Parses a list of server directives. :param list server: list of directives in a server block diff --git a/certbot-nginx/certbot_nginx/_internal/parser_obj.py b/certbot-nginx/certbot_nginx/_internal/parser_obj.py index 33ed822c3..0af38a936 100644 --- a/certbot-nginx/certbot_nginx/_internal/parser_obj.py +++ b/certbot-nginx/certbot_nginx/_internal/parser_obj.py @@ -5,7 +5,14 @@ raw lists of tokens from pyparsing. """ import abc import logging +from typing import Any +from typing import Callable +from typing import Iterator from typing import List +from typing import Optional +from typing import Sequence +from typing import Tuple +from typing import Type from certbot import errors @@ -23,24 +30,24 @@ class Parsable: __metaclass__ = abc.ABCMeta - def __init__(self, parent=None): - self._data: List[object] = [] + def __init__(self, parent: Optional["Parsable"] = None): + self._data: List[Any] = [] self._tabs = None self.parent = parent @classmethod - def parsing_hooks(cls): + def parsing_hooks(cls) -> Tuple[Type["Block"], Type["Sentence"], Type["Statements"]]: """Returns object types that this class should be able to `parse` recusrively. The order of the objects indicates the order in which the parser should try to parse each subitem. :returns: A list of Parsable classes. :rtype list: """ - return (Block, Sentence, Statements) + return Block, Sentence, Statements @staticmethod @abc.abstractmethod - def should_parse(lists): + def should_parse(lists: Any) -> bool: """ Returns whether the contents of `lists` can be parsed into this object. :returns: Whether `lists` can be parsed as this object. @@ -49,7 +56,7 @@ class Parsable: raise NotImplementedError() @abc.abstractmethod - def parse(self, raw_list, add_spaces=False): + def parse(self, raw_list: List[Any], add_spaces: bool = False) -> None: """ Loads information into this object from underlying raw_list structure. Each Parsable object might make different assumptions about the structure of raw_list. @@ -64,7 +71,8 @@ class Parsable: raise NotImplementedError() @abc.abstractmethod - def iterate(self, expanded=False, match=None): + def iterate(self, expanded: bool = False, + match: Optional[Callable[["Parsable"], bool]] = None) -> Iterator[Any]: """ Iterates across this object. If this object is a leaf object, only yields itself. If it contains references other parsing objects, and `expanded` is set, this function should first yield itself, then recursively iterate across all of them. @@ -77,7 +85,7 @@ class Parsable: raise NotImplementedError() @abc.abstractmethod - def get_tabs(self): + def get_tabs(self) -> str: """ Guess at the tabbing style of this parsed object, based on whitespace. If this object is a leaf, it deducts the tabbing based on its own contents. @@ -90,7 +98,7 @@ class Parsable: raise NotImplementedError() @abc.abstractmethod - def set_tabs(self, tabs=" "): + def set_tabs(self, tabs: str = " ") -> None: """This tries to set and alter the tabbing of the current object to a desired whitespace string. Primarily meant for objects that were constructed, so they can conform to surrounding whitespace. @@ -99,7 +107,7 @@ class Parsable: """ raise NotImplementedError() - def dump(self, include_spaces=False): + def dump(self, include_spaces: bool = False) -> List[Any]: """ Dumps back to pyparsing-like list tree. The opposite of `parse`. Note: if this object has not been modified, `dump` with `include_spaces=True` @@ -121,17 +129,17 @@ class Statements(Parsable): an extra `_trailing_whitespace` string to keep track of the whitespace that does not precede any more statements. """ - def __init__(self, parent=None): + def __init__(self, parent: Optional[Parsable] = None): super().__init__(parent) self._trailing_whitespace = None # ======== Begin overridden functions @staticmethod - def should_parse(lists): + def should_parse(lists: Any) -> bool: return isinstance(lists, list) - def set_tabs(self, tabs=" "): + def set_tabs(self, tabs: str = " ") -> None: """ Sets the tabbing for this set of statements. Does this by calling `set_tabs` on each of the child statements. @@ -144,7 +152,7 @@ class Statements(Parsable): if self.parent is not None: self._trailing_whitespace = "\n" + self.parent.get_tabs() - def parse(self, raw_list, add_spaces=False): + def parse(self, raw_list: List[Any], add_spaces: bool = False) -> None: """ Parses a list of statements. Expects all elements in `raw_list` to be parseable by `type(self).parsing_hooks`, with an optional whitespace string at the last index of `raw_list`. @@ -157,14 +165,14 @@ class Statements(Parsable): raw_list = raw_list[:-1] self._data = [parse_raw(elem, self, add_spaces) for elem in raw_list] - def get_tabs(self): + def get_tabs(self) -> str: """ Takes a guess at the tabbing of all contained Statements by retrieving the tabbing of the first Statement.""" if self._data: return self._data[0].get_tabs() return "" - def dump(self, include_spaces=False): + def dump(self, include_spaces: bool = False) -> List[Any]: """ Dumps this object by first dumping each statement, then appending its trailing whitespace (if `include_spaces` is set) """ data = super().dump(include_spaces) @@ -172,7 +180,8 @@ class Statements(Parsable): return data + [self._trailing_whitespace] return data - def iterate(self, expanded=False, match=None): + def iterate(self, expanded: bool = False, + match: Optional[Callable[["Parsable"], bool]] = None) -> Iterator[Any]: """ Combines each statement's iterator. """ for elem in self._data: for sub_elem in elem.iterate(expanded, match): @@ -181,7 +190,7 @@ class Statements(Parsable): # ======== End overridden functions -def _space_list(list_): +def _space_list(list_: Sequence[Any]) -> List[str]: """ Inserts whitespace between adjacent non-whitespace tokens. """ spaced_statement: List[str] = [] for i in reversed(range(len(list_))): @@ -197,7 +206,7 @@ class Sentence(Parsable): # ======== Begin overridden functions @staticmethod - def should_parse(lists): + def should_parse(lists: Any) -> bool: """ Returns True if `lists` can be parseable as a `Sentence`-- that is, every element is a string type. @@ -205,38 +214,39 @@ class Sentence(Parsable): :returns: whether this lists is parseable by `Sentence`. """ - return isinstance(lists, list) and len(lists) > 0 and \ - all(isinstance(elem, str) for elem in lists) + return (isinstance(lists, list) and len(lists) > 0 and + all(isinstance(elem, str) for elem in lists)) - def parse(self, raw_list, add_spaces=False): + def parse(self, raw_list: List[Any], add_spaces: bool = False) -> None: """ Parses a list of string types into this object. If add_spaces is set, adds whitespace tokens between adjacent non-whitespace tokens.""" if add_spaces: raw_list = _space_list(raw_list) - if not isinstance(raw_list, list) or \ - any(not isinstance(elem, str) for elem in raw_list): + if (not isinstance(raw_list, list) + or any(not isinstance(elem, str) for elem in raw_list)): raise errors.MisconfigurationError("Sentence parsing expects a list of string types.") self._data = raw_list - def iterate(self, expanded=False, match=None): + def iterate(self, expanded: bool = False, + match: Optional[Callable[[Parsable], bool]] = None) -> Iterator[Any]: """ Simply yields itself. """ if match is None or match(self): yield self - def set_tabs(self, tabs=" "): + def set_tabs(self, tabs: str = " ") -> None: """ Sets the tabbing on this sentence. Inserts a newline and `tabs` at the beginning of `self._data`. """ if self._data[0].isspace(): return self._data.insert(0, "\n" + tabs) - def dump(self, include_spaces=False): + def dump(self, include_spaces: bool = False) -> List[Any]: """ Dumps this sentence. If include_spaces is set, includes whitespace tokens.""" if not include_spaces: return self.words return self._data - def get_tabs(self): + def get_tabs(self) -> str: """ Guesses at the tabbing of this sentence. If the first element is whitespace, returns the whitespace after the rightmost newline in the string. """ first = self._data[0] @@ -248,14 +258,14 @@ class Sentence(Parsable): # ======== End overridden functions @property - def words(self): + def words(self) -> List[str]: """ Iterates over words, but without spaces. Like Unspaced List. """ return [word.strip("\"\'") for word in self._data if not word.isspace()] - def __getitem__(self, index): + def __getitem__(self, index: int) -> str: return self.words[index] - def __contains__(self, word): + def __contains__(self, word: str) -> bool: return word in self.words @@ -270,13 +280,13 @@ class Block(Parsable): names = ["block", " ", "name", " "] contents = [["\n ", "content", " ", "1"], ["\n ", "content", " ", "2"], "\n"] """ - def __init__(self, parent=None): + def __init__(self, parent: Optional[Parsable] = None) -> None: super().__init__(parent) - self.names: Sentence = None - self.contents: Block = None + self.names: Optional[Sentence] = None + self.contents: Optional[Block] = None @staticmethod - def should_parse(lists): + def should_parse(lists: Any) -> bool: """ Returns True if `lists` can be parseable as a `Block`-- that is, it's got a length of 2, the first element is a `Sentence` and the second can be a `Statements`. @@ -287,13 +297,14 @@ class Block(Parsable): return isinstance(lists, list) and len(lists) == 2 and \ Sentence.should_parse(lists[0]) and isinstance(lists[1], list) - def set_tabs(self, tabs=" "): + def set_tabs(self, tabs: str = " ") -> None: """ Sets tabs by setting equivalent tabbing on names, then adding tabbing to contents.""" self.names.set_tabs(tabs) self.contents.set_tabs(tabs + " ") - def iterate(self, expanded=False, match=None): + def iterate(self, expanded: bool = False, + match: Optional[Callable[[Parsable], bool]] = None) -> Iterator[Any]: """ Iterator over self, and if expanded is set, over its contents. """ if match is None or match(self): yield self @@ -301,7 +312,7 @@ class Block(Parsable): for elem in self.contents.iterate(expanded, match): yield elem - def parse(self, raw_list, add_spaces=False): + def parse(self, raw_list: List[Any], add_spaces: bool = False) -> None: """ Parses a list that resembles a block. The assumptions that this routine makes are: @@ -323,11 +334,12 @@ class Block(Parsable): self.contents.parse(raw_list[1], add_spaces) self._data = [self.names, self.contents] - def get_tabs(self): + def get_tabs(self) -> str: """ Guesses tabbing by retrieving tabbing guess of self.names. """ return self.names.get_tabs() -def _is_comment(parsed_obj): + +def _is_comment(parsed_obj: Parsable) -> bool: """ Checks whether parsed_obj is a comment. :param .Parsable parsed_obj: @@ -339,7 +351,8 @@ def _is_comment(parsed_obj): return False return parsed_obj.words[0] == "#" -def _is_certbot_comment(parsed_obj): + +def _is_certbot_comment(parsed_obj: Parsable) -> bool: """ Checks whether parsed_obj is a "managed by Certbot" comment. :param .Parsable parsed_obj: @@ -356,7 +369,8 @@ def _is_certbot_comment(parsed_obj): return False return True -def _certbot_comment(parent, preceding_spaces=4): + +def _certbot_comment(parent: Parsable, preceding_spaces: int = 4) -> Sentence: """ A "Managed by Certbot" comment. :param int preceding_spaces: Number of spaces between the end of the previous statement and the comment. @@ -367,7 +381,8 @@ def _certbot_comment(parent, preceding_spaces=4): result.parse([" " * preceding_spaces] + COMMENT_BLOCK) return result -def _choose_parser(parent, list_): + +def _choose_parser(parent: Parsable, list_: Any) -> Parsable: """ Choose a parser from type(parent).parsing_hooks, depending on whichever hook returns True first. """ hooks = Parsable.parsing_hooks() @@ -379,7 +394,8 @@ def _choose_parser(parent, list_): raise errors.MisconfigurationError( "None of the parsing hooks succeeded, so we don't know how to parse this set of lists.") -def parse_raw(lists_, parent=None, add_spaces=False): + +def parse_raw(lists_: Any, parent: Optional[Parsable] = None, add_spaces: bool = False) -> Parsable: """ Primary parsing factory function. :param list lists_: raw lists from pyparsing to parse. diff --git a/certbot/certbot/plugins/common.py b/certbot/certbot/plugins/common.py index 99d059371..cc4ace4fa 100644 --- a/certbot/certbot/plugins/common.py +++ b/certbot/certbot/plugins/common.py @@ -256,7 +256,7 @@ class Addr: self.ipv6 = ipv6 @classmethod - def fromstring(cls, str_addr: str) -> 'Addr': + def fromstring(cls, str_addr: str) -> Optional['Addr']: """Initialize Addr from string.""" if str_addr.startswith('['): # ipv6 addresses starts with [ diff --git a/tox.ini b/tox.ini index 08e4ddf91..652c80fa1 100644 --- a/tox.ini +++ b/tox.ini @@ -20,8 +20,8 @@ install_and_test = python {toxinidir}/tools/install_and_test.py dns_packages = certbot-dns-cloudflare certbot-dns-cloudxns certbot-dns-digitalocean certbot-dns-dnsimple certbot-dns-dnsmadeeasy certbot-dns-gehirn certbot-dns-google certbot-dns-linode certbot-dns-luadns certbot-dns-nsone certbot-dns-ovh certbot-dns-rfc2136 certbot-dns-route53 certbot-dns-sakuracloud win_all_packages = acme[test] certbot[test] {[base]dns_packages} certbot-nginx all_packages = {[base]win_all_packages} certbot-apache -fully_typed_source_paths = acme/acme certbot/certbot certbot-ci/certbot_integration_tests certbot-ci/snap_integration_tests certbot-ci/windows_installer_integration_tests certbot-compatibility-test/certbot_compatibility_test certbot-dns-cloudflare/certbot_dns_cloudflare certbot-dns-cloudxns/certbot_dns_cloudxns certbot-dns-digitalocean/certbot_dns_digitalocean certbot-dns-dnsimple/certbot_dns_dnsimple certbot-dns-dnsmadeeasy/certbot_dns_dnsmadeeasy certbot-dns-gehirn/certbot_dns_gehirn certbot-dns-google/certbot_dns_google certbot-dns-linode/certbot_dns_linode certbot-dns-luadns/certbot_dns_luadns certbot-dns-nsone/certbot_dns_nsone certbot-dns-ovh/certbot_dns_ovh certbot-dns-rfc2136/certbot_dns_rfc2136 certbot-dns-route53/certbot_dns_route53 certbot-dns-sakuracloud/certbot_dns_sakuracloud tests/lock_test.py -partially_typed_source_paths = certbot-apache/certbot_apache certbot-nginx/certbot_nginx +fully_typed_source_paths = acme/acme certbot/certbot certbot-ci/certbot_integration_tests certbot-ci/snap_integration_tests certbot-ci/windows_installer_integration_tests certbot-compatibility-test/certbot_compatibility_test certbot-dns-cloudflare/certbot_dns_cloudflare certbot-dns-cloudxns/certbot_dns_cloudxns certbot-dns-digitalocean/certbot_dns_digitalocean certbot-dns-dnsimple/certbot_dns_dnsimple certbot-dns-dnsmadeeasy/certbot_dns_dnsmadeeasy certbot-dns-gehirn/certbot_dns_gehirn certbot-dns-google/certbot_dns_google certbot-dns-linode/certbot_dns_linode certbot-dns-luadns/certbot_dns_luadns certbot-dns-nsone/certbot_dns_nsone certbot-dns-ovh/certbot_dns_ovh certbot-dns-rfc2136/certbot_dns_rfc2136 certbot-dns-route53/certbot_dns_route53 certbot-dns-sakuracloud/certbot_dns_sakuracloud certbot-nginx/certbot_nginx tests/lock_test.py +partially_typed_source_paths = certbot-apache/certbot_apache [testenv] passenv = From afc5be5abeeee60560113c5eda0abfeaf1e04be8 Mon Sep 17 00:00:00 2001 From: kevgrig Date: Tue, 18 Jan 2022 14:20:25 -0800 Subject: [PATCH 7/7] Add wildcard example (#9164) * Add wildcard example * Update wildcard example --- .../certbot_dns_digitalocean/__init__.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/certbot-dns-digitalocean/certbot_dns_digitalocean/__init__.py b/certbot-dns-digitalocean/certbot_dns_digitalocean/__init__.py index 2cb7a92de..8fd509196 100644 --- a/certbot-dns-digitalocean/certbot_dns_digitalocean/__init__.py +++ b/certbot-dns-digitalocean/certbot_dns_digitalocean/__init__.py @@ -77,6 +77,15 @@ Examples -d example.com \\ -d www.example.com +.. code-block:: bash + :caption: To acquire a wildcard certificate for ``*.example.com`` + + certbot certonly \\ + --dns-digitalocean \\ + --dns-digitalocean-credentials ~/.secrets/certbot/digitalocean.ini \\ + -d example.com \\ + -d '*.example.com' + .. code-block:: bash :caption: To acquire a certificate for ``example.com``, waiting 60 seconds for DNS propagation