diff --git a/certbot-apache/certbot_apache/_internal/apacheparser.py b/certbot-apache/certbot_apache/_internal/apacheparser.py index 77f4517fe..c7b723ae6 100644 --- a/certbot-apache/certbot_apache/_internal/apacheparser.py +++ b/certbot-apache/certbot_apache/_internal/apacheparser.py @@ -73,7 +73,7 @@ class ApacheDirectiveNode(ApacheParserNode): self.metadata == other.metadata) return False - def set_parameters(self, _parameters): + def set_parameters(self, _parameters): # pragma: no cover """Sets the parameters for DirectiveNode""" return @@ -97,7 +97,8 @@ class ApacheBlockNode(ApacheDirectiveNode): self.metadata == other.metadata) return False - def add_child_block(self, name, parameters=None, position=None): # pylint: disable=unused-argument + # pylint: disable=unused-argument + def add_child_block(self, name, parameters=None, position=None): # pragma: no cover """Adds a new BlockNode to the sequence of children""" new_block = ApacheBlockNode(name=assertions.PASS, parameters=assertions.PASS, @@ -107,7 +108,8 @@ class ApacheBlockNode(ApacheDirectiveNode): self.children += (new_block,) return new_block - def add_child_directive(self, name, parameters=None, position=None): # pylint: disable=unused-argument + # pylint: disable=unused-argument + def add_child_directive(self, name, parameters=None, position=None): # pragma: no cover """Adds a new DirectiveNode to the sequence of children""" new_dir = ApacheDirectiveNode(name=assertions.PASS, parameters=assertions.PASS, @@ -144,7 +146,8 @@ class ApacheBlockNode(ApacheDirectiveNode): filepath=assertions.PASS, metadata=self.metadata)] - def find_comments(self, comment, exact=False): # pylint: disable=unused-argument + # pylint: disable=unused-argument + def find_comments(self, comment, exact=False): # pragma: no cover """Recursive search of DirectiveNodes from the sequence of children""" return [ApacheCommentNode(comment=assertions.PASS, ancestor=self, diff --git a/certbot-apache/certbot_apache/_internal/configurator.py b/certbot-apache/certbot_apache/_internal/configurator.py index fc386404d..86d579954 100644 --- a/certbot-apache/certbot_apache/_internal/configurator.py +++ b/certbot-apache/certbot_apache/_internal/configurator.py @@ -12,6 +12,11 @@ import time import six import zope.component import zope.interface +try: + import apacheconfig + HAS_APACHECONFIG = True +except ImportError: # pragma: no cover + HAS_APACHECONFIG = False from acme import challenges from acme.magic_typing import DefaultDict @@ -430,11 +435,17 @@ class ApacheConfigurator(common.Installer): def get_parsernode_root(self, metadata): """Initializes the ParserNode parser root instance.""" - apache_vars = dict() - apache_vars["defines"] = apache_util.parse_defines(self.option("ctl")) - apache_vars["includes"] = apache_util.parse_includes(self.option("ctl")) - apache_vars["modules"] = apache_util.parse_modules(self.option("ctl")) - metadata["apache_vars"] = apache_vars + if HAS_APACHECONFIG: + apache_vars = dict() + apache_vars["defines"] = apache_util.parse_defines(self.option("ctl")) + apache_vars["includes"] = apache_util.parse_includes(self.option("ctl")) + apache_vars["modules"] = apache_util.parse_modules(self.option("ctl")) + metadata["apache_vars"] = apache_vars + + with open(self.parser.loc["root"]) as f: + with apacheconfig.make_loader(writable=True, + **apacheconfig.flavors.NATIVE_APACHE) as loader: + metadata["ac_ast"] = loader.loads(f.read()) return dualparser.DualBlockNode( name=assertions.PASS, @@ -974,7 +985,7 @@ class ApacheConfigurator(common.Installer): """ v1_vhosts = self.get_virtual_hosts_v1() - if self.USE_PARSERNODE: + if self.USE_PARSERNODE and HAS_APACHECONFIG: v2_vhosts = self.get_virtual_hosts_v2() for v1_vh in v1_vhosts: diff --git a/certbot-apache/setup.py b/certbot-apache/setup.py index 4ec1d0a9c..14300370a 100644 --- a/certbot-apache/setup.py +++ b/certbot-apache/setup.py @@ -19,7 +19,7 @@ install_requires = [ ] dev_extras = [ - 'apacheconfig>=0.3.1', + 'apacheconfig>=0.3.2', ] class PyTest(TestCommand): diff --git a/certbot-apache/tests/augeasnode_test.py b/certbot-apache/tests/augeasnode_test.py index 8417bc283..4468a838e 100644 --- a/certbot-apache/tests/augeasnode_test.py +++ b/certbot-apache/tests/augeasnode_test.py @@ -1,13 +1,25 @@ """Tests for AugeasParserNode classes""" import mock +import os +import unittest import util from certbot import errors from certbot_apache._internal import assertions +from certbot_apache._internal import augeasparser +def _get_augeasnode_mock(filepath): + """ Helper function for mocking out DualNode instance with an AugeasNode """ + def augeasnode_mock(metadata): + return augeasparser.AugeasBlockNode( + name=assertions.PASS, + ancestor=None, + filepath=filepath, + metadata=metadata) + return augeasnode_mock class AugeasParserNodeTest(util.ApacheTest): # pylint: disable=too-many-public-methods """Test AugeasParserNode using available test configurations""" @@ -15,8 +27,11 @@ class AugeasParserNodeTest(util.ApacheTest): # pylint: disable=too-many-public- def setUp(self): # pylint: disable=arguments-differ super(AugeasParserNodeTest, self).setUp() - self.config = util.get_apache_configurator( - self.config_path, self.vhost_path, self.config_dir, self.work_dir, use_parsernode=True) + 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.vh_truth = util.get_vh_truth( self.temp_dir, "debian_apache_2_4/multiple_vhosts") @@ -110,7 +125,7 @@ class AugeasParserNodeTest(util.ApacheTest): # pylint: disable=too-many-public- name=servernames[0].name, parameters=["test", "setting", "these"], ancestor=assertions.PASS, - metadata=servernames[0].primary.metadata + metadata=servernames[0].metadata ) self.assertTrue(mock_set.called) self.assertEqual( @@ -145,26 +160,26 @@ class AugeasParserNodeTest(util.ApacheTest): # pylint: disable=too-many-public- self.assertTrue(found) def test_add_child_comment(self): - newc = self.config.parser_root.primary.add_child_comment("The content") + newc = self.config.parser_root.add_child_comment("The content") comments = self.config.parser_root.find_comments("The content") self.assertEqual(len(comments), 1) self.assertEqual( newc.metadata["augeaspath"], - comments[0].primary.metadata["augeaspath"] + comments[0].metadata["augeaspath"] ) self.assertEqual(newc.comment, comments[0].comment) def test_delete_child(self): - listens = self.config.parser_root.primary.find_directives("Listen") + listens = self.config.parser_root.find_directives("Listen") self.assertEqual(len(listens), 1) - self.config.parser_root.primary.delete_child(listens[0]) + self.config.parser_root.delete_child(listens[0]) - listens = self.config.parser_root.primary.find_directives("Listen") + listens = self.config.parser_root.find_directives("Listen") self.assertEqual(len(listens), 0) def test_delete_child_not_found(self): listen = self.config.parser_root.find_directives("Listen")[0] - listen.primary.metadata["augeaspath"] = "/files/something/nonexistent" + listen.metadata["augeaspath"] = "/files/something/nonexistent" self.assertRaises( errors.PluginError, @@ -177,10 +192,10 @@ class AugeasParserNodeTest(util.ApacheTest): # pylint: disable=too-many-public- "NewBlock", ["first", "second"] ) - rpath, _, directive = nb.primary.metadata["augeaspath"].rpartition("/") + rpath, _, directive = nb.metadata["augeaspath"].rpartition("/") self.assertEqual( rpath, - self.config.parser_root.primary.metadata["augeaspath"] + self.config.parser_root.metadata["augeaspath"] ) self.assertTrue(directive.startswith("NewBlock")) @@ -189,8 +204,8 @@ class AugeasParserNodeTest(util.ApacheTest): # pylint: disable=too-many-public- "Beginning", position=0 ) - parser = self.config.parser_root.primary.parser - root_path = self.config.parser_root.primary.metadata["augeaspath"] + parser = self.config.parser_root.parser + 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")) @@ -199,8 +214,8 @@ class AugeasParserNodeTest(util.ApacheTest): # pylint: disable=too-many-public- self.config.parser_root.add_child_block( "VeryLast", ) - parser = self.config.parser_root.primary.parser - root_path = self.config.parser_root.primary.metadata["augeaspath"] + parser = self.config.parser_root.parser + 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")) @@ -210,8 +225,8 @@ class AugeasParserNodeTest(util.ApacheTest): # pylint: disable=too-many-public- "VeryLastAlt", position=99999 ) - parser = self.config.parser_root.primary.parser - root_path = self.config.parser_root.primary.metadata["augeaspath"] + parser = self.config.parser_root.parser + 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")) @@ -221,15 +236,15 @@ class AugeasParserNodeTest(util.ApacheTest): # pylint: disable=too-many-public- "Middle", position=5 ) - parser = self.config.parser_root.primary.parser - root_path = self.config.parser_root.primary.metadata["augeaspath"] + parser = self.config.parser_root.parser + 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")) def test_add_child_block_existing_name(self): - parser = self.config.parser_root.primary.parser - root_path = self.config.parser_root.primary.metadata["augeaspath"] + parser = self.config.parser_root.parser + root_path = self.config.parser_root.metadata["augeaspath"] # There already exists a single VirtualHost in the base config new_block = parser.aug.match("{}/VirtualHost[2]".format(root_path)) self.assertEqual(len(new_block), 0) @@ -238,7 +253,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.primary.metadata["augeaspath"].endswith("VirtualHost[2]")) + self.assertTrue(vh.metadata["augeaspath"].endswith("VirtualHost[2]")) def test_node_init_error_bad_augeaspath(self): from certbot_apache._internal.augeasparser import AugeasBlockNode @@ -283,7 +298,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].primary.metadata["augeaspath"].endswith("[1]")) + self.assertTrue(dirs[0].metadata["augeaspath"].endswith("[1]")) def test_add_child_directive_exception(self): self.assertRaises( @@ -313,6 +328,6 @@ class AugeasParserNodeTest(util.ApacheTest): # pylint: disable=too-many-public- self.assertTrue(nonmacro_test) def test_find_ancestors_bad_path(self): - self.config.parser_root.primary.metadata["augeaspath"] = "" - ancs = self.config.parser_root.primary.find_ancestors("Anything") + self.config.parser_root.metadata["augeaspath"] = "" + ancs = self.config.parser_root.find_ancestors("Anything") self.assertEqual(len(ancs), 0) diff --git a/certbot-apache/tests/parsernode_configurator_test.py b/certbot-apache/tests/parsernode_configurator_test.py index 67d65995a..afaaa6e43 100644 --- a/certbot-apache/tests/parsernode_configurator_test.py +++ b/certbot-apache/tests/parsernode_configurator_test.py @@ -5,7 +5,14 @@ import mock import util +try: + import apacheconfig + HAS_APACHECONFIG = True +except ImportError: # pragma: no cover + HAS_APACHECONFIG = False + +@unittest.skipIf(not HAS_APACHECONFIG, reason='Tests require apacheconfig dependency') class ConfiguratorParserNodeTest(util.ApacheTest): # pylint: disable=too-many-public-methods """Test AugeasParserNode using available test configurations""" diff --git a/tools/dev_constraints.txt b/tools/dev_constraints.txt index cfa036435..6e692841b 100644 --- a/tools/dev_constraints.txt +++ b/tools/dev_constraints.txt @@ -3,7 +3,7 @@ # Some dev package versions specified here may be overridden by higher level constraints # files during tests (eg. letsencrypt-auto-source/pieces/dependency-requirements.txt). alabaster==0.7.10 -apacheconfig==0.3.1 +apacheconfig==0.3.2 apipkg==1.4 appnope==0.1.0 asn1crypto==0.22.0 diff --git a/tools/oldest_constraints.txt b/tools/oldest_constraints.txt index 85d058796..402f3fef1 100644 --- a/tools/oldest_constraints.txt +++ b/tools/oldest_constraints.txt @@ -39,7 +39,7 @@ pytz==2012rc0 google-api-python-client==1.5.5 # Our setup.py constraints -apacheconfig==0.3.1 +apacheconfig==0.3.2 cloudflare==1.5.1 cryptography==1.2.3 parsedatetime==1.3 diff --git a/tox.ini b/tox.ini index 3903cdf45..8aa4bfbf2 100644 --- a/tox.ini +++ b/tox.ini @@ -120,12 +120,14 @@ setenv = basepython = python2.7 commands = {[base]install_packages} + {[base]pip_install} certbot-apache[dev] python tox.cover.py [testenv:py37-cover] basepython = python3.7 commands = {[base]install_packages} + {[base]pip_install} certbot-apache[dev] python tox.cover.py [testenv:lint]