From bdf24d2bed1976ae5f8938d46709b1cdef424b4f Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Wed, 13 Nov 2019 00:19:02 +0200 Subject: [PATCH 1/3] Implement add_child_directive (#7517) --- certbot-apache/certbot_apache/augeasparser.py | 24 +++++++++++++++---- .../certbot_apache/tests/augeasnode_test.py | 22 ++++++++++++++++- 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/certbot-apache/certbot_apache/augeasparser.py b/certbot-apache/certbot_apache/augeasparser.py index e340519ce..5fd5247c1 100644 --- a/certbot-apache/certbot_apache/augeasparser.py +++ b/certbot-apache/certbot_apache/augeasparser.py @@ -221,12 +221,26 @@ class AugeasBlockNode(AugeasDirectiveNode): # 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_metadata = {"augeasparser": self.parser, "augeaspath": assertions.PASS} - new_dir = AugeasDirectiveNode(name=assertions.PASS, - ancestor=self, - filepath=assertions.PASS, + + if not parameters: + raise errors.PluginError("Directive requires parameters and none were set.") + + insertpath, realpath, before = self._aug_resolve_child_position( + "directive", + position + ) + new_metadata = {"augeasparser": self.parser, "augeaspath": realpath} + + # Create the new directive + self.parser.aug.insert(insertpath, "directive", before) + # Set the directive key + self.parser.aug.set(realpath, name) + + new_dir = AugeasDirectiveNode(name=name, + parameters=parameters, + ancestor=assertions.PASS, + filepath=apache_util.get_file_path(realpath), metadata=new_metadata) - self.children += (new_dir,) return new_dir def add_child_comment(self, comment="", position=None): # pylint: disable=unused-argument diff --git a/certbot-apache/certbot_apache/tests/augeasnode_test.py b/certbot-apache/certbot_apache/tests/augeasnode_test.py index 8846c3c07..76f46c14d 100644 --- a/certbot-apache/certbot_apache/tests/augeasnode_test.py +++ b/certbot-apache/certbot_apache/tests/augeasnode_test.py @@ -9,7 +9,7 @@ from certbot_apache import assertions from certbot_apache.tests import util -class AugeasParserNodeTest(util.ApacheTest): +class AugeasParserNodeTest(util.ApacheTest): # pylint: disable=too-many-public-methods """Test AugeasParserNode using available test configurations""" def setUp(self): # pylint: disable=arguments-differ @@ -216,6 +216,7 @@ class AugeasParserNodeTest(util.ApacheTest): AugeasBlockNode, **parameters ) + def test_node_init_error_missing_augeaspath(self): from certbot_apache.augeasparser import AugeasBlockNode parameters = { @@ -231,3 +232,22 @@ class AugeasParserNodeTest(util.ApacheTest): AugeasBlockNode, **parameters ) + + def test_add_child_directive(self): + self.config.parser_root.primary.add_child_directive( + "ThisWasAdded", + ["with", "parameters"], + position=0 + ) + dirs = self.config.parser_root.find_directives("ThisWasAdded") + 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]")) + + def test_add_child_directive_exception(self): + self.assertRaises( + errors.PluginError, + self.config.parser_root.primary.add_child_directive, + "ThisRaisesErrorBecauseMissingParameters" + ) From d14eec9ecf89da33dacb4fbac618a226e54526b0 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Wed, 13 Nov 2019 00:19:21 +0200 Subject: [PATCH 2/3] [Apache v2] Implement save() and unsaved_files() (#7520) * Implement save() and unsaved_files() * Linter fix --- certbot-apache/certbot_apache/augeasparser.py | 8 ++++---- .../certbot_apache/tests/augeasnode_test.py | 12 ++++++++++++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/certbot-apache/certbot_apache/augeasparser.py b/certbot-apache/certbot_apache/augeasparser.py index 5fd5247c1..a1d72fe52 100644 --- a/certbot-apache/certbot_apache/augeasparser.py +++ b/certbot-apache/certbot_apache/augeasparser.py @@ -97,8 +97,8 @@ class AugeasParserNode(interfaces.ParserNode): except KeyError: raise errors.PluginError("Augeas path is required") - def save(self, msg): # pragma: no cover - pass + def save(self, msg): + self.parser.save(msg) class AugeasCommentNode(AugeasParserNode): @@ -304,9 +304,9 @@ class AugeasBlockNode(AugeasDirectiveNode): """Deletes a ParserNode from the sequence of children""" pass - def unsaved_files(self): # pragma: no cover + def unsaved_files(self): """Returns a list of unsaved filepaths""" - return [assertions.PASS] + return self.parser.unsaved_files() def _create_commentnode(self, path): """Helper function to create a CommentNode from Augeas path""" diff --git a/certbot-apache/certbot_apache/tests/augeasnode_test.py b/certbot-apache/certbot_apache/tests/augeasnode_test.py index 76f46c14d..39736ee17 100644 --- a/certbot-apache/certbot_apache/tests/augeasnode_test.py +++ b/certbot-apache/certbot_apache/tests/augeasnode_test.py @@ -20,6 +20,18 @@ class AugeasParserNodeTest(util.ApacheTest): # pylint: disable=too-many-public- 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.parser.ApacheParser.save') as mock_save: + self.config.parser_root.save("A save message") + self.assertTrue(mock_save.called) + self.assertEqual(mock_save.call_args[0][0], "A save message") + + def test_unsaved_files(self): + with mock.patch('certbot_apache.parser.ApacheParser.unsaved_files') as mock_uf: + mock_uf.return_value = ["first", "second"] + files = self.config.parser_root.unsaved_files() + self.assertEqual(files, ["first", "second"]) + def test_get_block_node_name(self): from certbot_apache.augeasparser import AugeasBlockNode block = AugeasBlockNode( From 517ff5cb1930e21b1de0d8c5343cb99c622e321f Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Wed, 13 Nov 2019 00:19:35 +0200 Subject: [PATCH 3/3] [Apache v2] Implement delete_child() (#7521) * Implement delete_child * Fix linter --- certbot-apache/certbot_apache/augeasparser.py | 15 ++++++++++++--- .../certbot_apache/tests/augeasnode_test.py | 18 ++++++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/certbot-apache/certbot_apache/augeasparser.py b/certbot-apache/certbot_apache/augeasparser.py index a1d72fe52..47b8602ab 100644 --- a/certbot-apache/certbot_apache/augeasparser.py +++ b/certbot-apache/certbot_apache/augeasparser.py @@ -300,9 +300,18 @@ class AugeasBlockNode(AugeasDirectiveNode): return nodes - def delete_child(self, child): # pragma: no cover - """Deletes a ParserNode from the sequence of children""" - pass + def delete_child(self, child): + """ + Deletes a ParserNode from the sequence of children, and raises an + exception if it's unable to do so. + :param AugeasParserNode: child: A node to delete. + """ + if not self.parser.aug.remove(child.metadata["augeaspath"]): + + raise errors.PluginError( + ("Could not delete child node, the Augeas path: {} doesn't " + + "seem to exist.").format(child.metadata["augeaspath"]) + ) def unsaved_files(self): """Returns a list of unsaved filepaths""" diff --git a/certbot-apache/certbot_apache/tests/augeasnode_test.py b/certbot-apache/certbot_apache/tests/augeasnode_test.py index 39736ee17..a86b618b2 100644 --- a/certbot-apache/certbot_apache/tests/augeasnode_test.py +++ b/certbot-apache/certbot_apache/tests/augeasnode_test.py @@ -144,6 +144,24 @@ class AugeasParserNodeTest(util.ApacheTest): # pylint: disable=too-many-public- found = True self.assertTrue(found) + def test_delete_child(self): + listens = self.config.parser_root.find_directives("Listen") + self.assertEqual(len(listens), 1) + self.config.parser_root.primary.delete_child(listens[0]) + + 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" + + self.assertRaises( + errors.PluginError, + self.config.parser_root.delete_child, + listen + ) + def test_add_child_block(self): nb = self.config.parser_root.primary.add_child_block( "NewBlock",