[Apache v2] Implement ApacheConfigurator get_virtual_hosts()
diff --git a/certbot-apache/certbot_apache/apache_util.py b/certbot-apache/certbot_apache/apache_util.py
index 70febc9495b..085ccddc8fa 100644
--- a/certbot-apache/certbot_apache/apache_util.py
+++ b/certbot-apache/certbot_apache/apache_util.py
@@ -1,5 +1,6 @@
 """ Utility functions for certbot-apache plugin """
 import binascii
+import fnmatch
 import logging
 import re
 import subprocess
@@ -114,6 +115,22 @@ def unique_id():
     return binascii.hexlify(os.urandom(16)).decode("utf-8")
 
 
+def included_in_paths(filepath, paths):
+    """
+    Returns true if the filepath is included in the list of paths
+    that may contain full paths or wildcard paths that need to be
+    expanded.
+
+    :param str filepath: Filepath to check
+    :params list paths: List of paths to check against
+
+    :returns: True if included
+    :rtype: bool
+    """
+
+    return any([fnmatch.fnmatch(filepath, path) for path in paths])
+
+
 def parse_defines(apachectl):
     """
     Gets Defines from httpd process and returns a dictionary of
diff --git a/certbot-apache/certbot_apache/assertions.py b/certbot-apache/certbot_apache/assertions.py
index c7a61f44679..1a5ce20965d 100644
--- a/certbot-apache/certbot_apache/assertions.py
+++ b/certbot-apache/certbot_apache/assertions.py
@@ -60,6 +60,8 @@ def assertEqualDirective(first, second):
 
 def isPass(value): # pragma: no cover
     """Checks if the value is set to PASS"""
+    if isinstance(value, bool):
+        return True
     return PASS in value
 
 def isPassDirective(block):
@@ -105,6 +107,26 @@ def assertEqualSimple(first, second):
     if not isPass(first) and not isPass(second):
         assert first == second
 
+def isEqualVirtualHost(first, second):
+    """
+    Checks that two VirtualHost objects are similar. There are some built
+    in differences with the implementations: VirtualHost created by ParserNode
+    implementation doesn't have "path" defined, as it was used for Augeas path
+    and that cannot obviously be used in the future. Similarly the legacy
+    version lacks "node" variable, that has a reference to the BlockNode for the
+    VirtualHost.
+    """
+    return (
+        first.name == second.name and
+        first.aliases == second.aliases and
+        first.filep == second.filep and
+        first.addrs == second.addrs and
+        first.ssl == second.ssl and
+        first.enabled == second.enabled and
+        first.modmacro == second.modmacro and
+        first.ancestor == second.ancestor
+    )
+
 def assertEqualPathsList(first, second):  # pragma: no cover
     """
     Checks that the two lists of file paths match. This assertion allows for wildcard
diff --git a/certbot-apache/certbot_apache/augeasparser.py b/certbot-apache/certbot_apache/augeasparser.py
index d2771c9d2df..1c6ce6675f9 100644
--- a/certbot-apache/certbot_apache/augeasparser.py
+++ b/certbot-apache/certbot_apache/augeasparser.py
@@ -115,7 +115,8 @@ def find_ancestors(self, name):
         while True:
             # Get the path of ancestor node
             parent = parent.rpartition("/")[0]
-            if not parent:
+            # Root of the tree
+            if not parent or parent == "/files":
                 break
             anc = self._create_blocknode(parent)
             if anc.name.lower() == name.lower():
@@ -134,7 +135,13 @@ def _create_blocknode(self, path):
         name = self._aug_get_name(path)
         metadata = {"augeasparser": self.parser, "augeaspath": path}
 
+        # Check if the file was included from the root config or initial state
+        enabled = self.parser.parsed_in_original(
+            apache_util.get_file_path(path)
+        )
+
         return AugeasBlockNode(name=name,
+                               enabled=enabled,
                                ancestor=assertions.PASS,
                                filepath=apache_util.get_file_path(path),
                                metadata=metadata)
@@ -265,10 +272,15 @@ def add_child_block(self, name, parameters=None, position=None):  # pragma: no c
 
         # Create the new block
         self.parser.aug.insert(insertpath, name, before)
+        # Check if the file was included from the root config or initial state
+        enabled = self.parser.parsed_in_original(
+            apache_util.get_file_path(realpath)
+        )
 
         # Parameters will be set at the initialization of the new object
         new_block = AugeasBlockNode(name=name,
                                     parameters=parameters,
+                                    enabled=enabled,
                                     ancestor=assertions.PASS,
                                     filepath=apache_util.get_file_path(realpath),
                                     metadata=new_metadata)
@@ -291,9 +303,14 @@ def add_child_directive(self, name, parameters=None, position=None):  # pragma:
         self.parser.aug.insert(insertpath, "directive", before)
         # Set the directive key
         self.parser.aug.set(realpath, name)
+        # Check if the file was included from the root config or initial state
+        enabled = self.parser.parsed_in_original(
+            apache_util.get_file_path(realpath)
+        )
 
         new_dir = AugeasDirectiveNode(name=name,
                                       parameters=parameters,
+                                      enabled=enabled,
                                       ancestor=assertions.PASS,
                                       filepath=apache_util.get_file_path(realpath),
                                       metadata=new_metadata)
@@ -394,8 +411,14 @@ def parsed_paths(self):
         :returns: list of file paths of files that have been parsed
         """
 
-        parsed_paths = self.parser.aug.match("/augeas/load/Httpd/incl")
-        return [self.parser.aug.get(path) for path in parsed_paths]
+        res_paths = []
+
+        paths = self.parser.existing_paths
+        for directory in paths:
+            for filename in paths[directory]:
+                res_paths.append(os.path.join(directory, filename))
+
+        return res_paths
 
     def _create_commentnode(self, path):
         """Helper function to create a CommentNode from Augeas path"""
@@ -416,10 +439,13 @@ def _create_directivenode(self, path):
         name = self.parser.get_arg(path)
         metadata = {"augeasparser": self.parser, "augeaspath": path}
 
-        # Because of the dynamic nature, and the fact that we're not populating
-        # the complete ParserNode tree, we use the search parent as ancestor
+        # Check if the file was included from the root config or initial state
+        enabled = self.parser.parsed_in_original(
+            apache_util.get_file_path(path)
+        )
         return AugeasDirectiveNode(name=name,
                                    ancestor=assertions.PASS,
+                                   enabled=enabled,
                                    filepath=apache_util.get_file_path(path),
                                    metadata=metadata)
 
diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py
index 9a9dec7a863..d4466cc53dc 100644
--- a/certbot-apache/certbot_apache/configurator.py
+++ b/certbot-apache/certbot_apache/configurator.py
@@ -202,7 +202,11 @@ def __init__(self, *args, **kwargs):
         self._autohsts = {}  # type: Dict[str, Dict[str, Union[int, float]]]
         # Reverter save notes
         self.save_notes = ""
-
+        # Should we use ParserNode implementation instead of the old behavior
+        self.USE_PARSERNODE = False
+        # Saves the list of file paths that were parsed initially, and
+        # not added to parser tree by self.conf("vhost-root") for example.
+        self.parsed_paths = []  # type: List[str]
         # These will be set in the prepare function
         self._prepared = False
         self.parser = None
@@ -261,6 +265,7 @@ def prepare(self):
                    "augeaspath": self.parser.get_root_augpath(),
                    "ac_ast": None}
         self.parser_root = self.get_parsernode_root(pn_meta)
+        self.parsed_paths = self.parser_root.parsed_paths()
 
         # Check for errors in parsing files with Augeas
         self.parser.check_parsing_errors("httpd.aug")
@@ -897,6 +902,29 @@ def _create_vhost(self, path):
         return vhost
 
     def get_virtual_hosts(self):
+        """
+        Temporary wrapper for legacy and ParserNode version for
+        get_virtual_hosts. This should be replaced with the ParserNode
+        implementation when ready.
+        """
+
+        v1_vhosts = self.get_virtual_hosts_v1()
+        v2_vhosts = self.get_virtual_hosts_v2()
+
+        for v1_vh in v1_vhosts:
+            found = False
+            for v2_vh in v2_vhosts:
+                if assertions.isEqualVirtualHost(v1_vh, v2_vh):
+                    found = True
+                    break
+            if not found:
+                raise AssertionError("Equivalent for {} was not found".format(v1_vh.path))
+
+        if self.USE_PARSERNODE:
+            return v2_vhosts
+        return v1_vhosts
+
+    def get_virtual_hosts_v1(self):
         """Returns list of virtual hosts found in the Apache configuration.
 
         :returns: List of :class:`~certbot_apache.obj.VirtualHost`
@@ -949,6 +977,79 @@ def get_virtual_hosts(self):
                     vhs.append(new_vhost)
         return vhs
 
+    def get_virtual_hosts_v2(self):
+        """Returns list of virtual hosts found in the Apache configuration using
+        ParserNode interface.
+        :returns: List of :class:`~certbot_apache.obj.VirtualHost`
+            objects found in configuration
+        :rtype: list
+        """
+
+        vhs = []
+        vhosts = self.parser_root.find_blocks("VirtualHost", exclude=False)
+        for vhblock in vhosts:
+            vhs.append(self._create_vhost_v2(vhblock))
+        return vhs
+
+    def _create_vhost_v2(self, node):
+        """Used by get_virtual_hosts_v2 to create vhost objects using ParserNode
+        interfaces.
+        :param interfaces.BlockNode node: The BlockNode object of VirtualHost block
+        :returns: newly created vhost
+        :rtype: :class:`~certbot_apache.obj.VirtualHost`
+        """
+        addrs = set()
+        for param in node.parameters:
+            addrs.add(obj.Addr.fromstring(param))
+
+        is_ssl = False
+        sslengine = node.find_directives("SSLEngine")
+        if sslengine:
+            for directive in sslengine:
+                if directive.parameters[0].lower() == "on":
+                    is_ssl = True
+                    break
+
+        # "SSLEngine on" might be set outside of <VirtualHost>
+        # Treat vhosts with port 443 as ssl vhosts
+        for addr in addrs:
+            if addr.get_port() == "443":
+                is_ssl = True
+
+        enabled = apache_util.included_in_paths(node.filepath, self.parsed_paths)
+
+        macro = False
+        # Check if the VirtualHost is contained in a mod_macro block
+        if node.find_ancestors("Macro"):
+            macro = True
+        vhost = obj.VirtualHost(
+            node.filepath, None, addrs, is_ssl, enabled, modmacro=macro, node=node
+        )
+        self._populate_vhost_names_v2(vhost)
+        return vhost
+
+    def _populate_vhost_names_v2(self, vhost):
+        """Helper function that populates the VirtualHost names.
+        :param host: In progress vhost whose names will be added
+        :type host: :class:`~certbot_apache.obj.VirtualHost`
+        """
+
+        servername_match = vhost.node.find_directives("ServerName",
+                                                      exclude=False)
+        serveralias_match = vhost.node.find_directives("ServerAlias",
+                                                       exclude=False)
+
+        servername = None
+        if servername_match:
+            servername = servername_match[-1].parameters[-1]
+
+        if not vhost.modmacro:
+            for alias in serveralias_match:
+                for serveralias in alias.parameters:
+                    vhost.aliases.add(serveralias)
+            vhost.name = servername
+
+
     def is_name_vhost(self, target_addr):
         """Returns if vhost is a name based vhost
 
diff --git a/certbot-apache/certbot_apache/obj.py b/certbot-apache/certbot_apache/obj.py
index 22abc85cdef..939251802ce 100644
--- a/certbot-apache/certbot_apache/obj.py
+++ b/certbot-apache/certbot_apache/obj.py
@@ -124,7 +124,7 @@ class VirtualHost(object):  # pylint: disable=too-few-public-methods
     strip_name = re.compile(r"^(?:.+://)?([^ :$]*)")
 
     def __init__(self, filep, path, addrs, ssl, enabled, name=None,
-                 aliases=None, modmacro=False, ancestor=None):
+                 aliases=None, modmacro=False, ancestor=None, node=None):
 
         # pylint: disable=too-many-arguments
         """Initialize a VH."""
@@ -137,6 +137,7 @@ def __init__(self, filep, path, addrs, ssl, enabled, name=None,
         self.enabled = enabled
         self.modmacro = modmacro
         self.ancestor = ancestor
+        self.node = node
 
     def get_names(self):
         """Return a set of all names."""
diff --git a/certbot-apache/certbot_apache/tests/parsernode_configurator_test.py b/certbot-apache/certbot_apache/tests/parsernode_configurator_test.py
new file mode 100644
index 00000000000..97f07d3d299
--- /dev/null
+++ b/certbot-apache/certbot_apache/tests/parsernode_configurator_test.py
@@ -0,0 +1,36 @@
+"""Tests for ApacheConfigurator for AugeasParserNode classes"""
+import unittest
+
+import mock
+
+from certbot_apache.tests import util
+
+
+class ConfiguratorParserNodeTest(util.ApacheTest):  # pylint: disable=too-many-public-methods
+    """Test AugeasParserNode using available test configurations"""
+
+    def setUp(self):  # pylint: disable=arguments-differ
+        super(ConfiguratorParserNodeTest, self).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")
+
+    def test_parsernode_get_vhosts(self):
+        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)
+
+    def test_parsernode_get_vhosts_mismatch(self):
+        vhosts = self.config.get_virtual_hosts_v2()
+        # One of the returned VirtualHost objects differs
+        vhosts[0].name = "IdidntExpectThat"
+        self.config.get_virtual_hosts_v2 = mock.MagicMock(return_value=vhosts)
+        with self.assertRaises(AssertionError):
+            _ = self.config.get_virtual_hosts()
+
+
+if __name__ == "__main__":
+    unittest.main()  # pragma: no cover

This replaces #7398 as there are quite a few changes (mostly to other parts than the actual ApacheConfigurator logic though), so I guessed it'd probably be best to start from scratch. The diff looks a bit messy as this is built on top of #7561 and  #7562 but it'll clear up when those are taken care of.

This PR also implements runtime assertion against the legacy implementation for VirtualHost object creation to ensure that the returned lists match where it matters. The objects created by the old implementation have Augeas paths, and the ones created by the new one have ParserNode references, but those obviously aren't matched.