Merge pull request #3033 from certbot/reredirect

Fix warning about already enabled redirect in Apache

certbot-apache/certbot_apache/configurator.py

@@ -124,6 +124,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         self.assoc = dict()
         # Outstanding challenges
         self._chall_out = set()
+        # Maps enhancements to vhosts we've enabled the enhancement for
+        self._enhanced_vhosts = defaultdict(set)
 
         # These will be set in the prepare function
         self.parser = None
@@ -1121,9 +1123,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         :param unused_options: Not currently used
         :type unused_options: Not Available
 
-        :returns: Success, general_vhost (HTTP vhost)
-        :rtype: (bool, :class:`~certbot_apache.obj.VirtualHost`)
-
         :raises .errors.PluginError: If no viable HTTP host can be created or
             used for the redirect.
 
@@ -1146,6 +1145,10 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
                         "redirection")
             self._create_redirect_vhost(ssl_vhost)
         else:
+            if general_vh in self._enhanced_vhosts["redirect"]:
+                logger.debug("Already enabled redirect for this vhost")
+                return
+
             # Check if Certbot redirection already exists
             self._verify_no_certbot_redirect(general_vh)
 
@@ -1181,6 +1184,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
                                 (general_vh.filep, ssl_vhost.filep))
             self.save()
 
+            self._enhanced_vhosts["redirect"].add(general_vh)
             logger.info("Redirecting vhost in %s to ssl vhost in %s",
                         general_vh.filep, ssl_vhost.filep)
 
@@ -1269,6 +1273,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         # Make a new vhost data structure and add it to the lists
         new_vhost = self._create_vhost(parser.get_aug_path(redirect_filepath))
         self.vhosts.append(new_vhost)
+        self._enhanced_vhosts["redirect"].add(new_vhost)
 
         # Finally create documentation for the change
         self.save_notes += ("Created a port 80 vhost, %s, for redirection to "

certbot-apache/certbot_apache/tests/configurator_test.py

@@ -1035,15 +1035,31 @@ class MultipleVhostsTest(util.ApacheTest):
         self.assertRaises(
             errors.PluginError, self.config._enable_redirect, ssl_vh, "")
 
-    def test_redirect_twice(self):
+    def test_redirect_two_domains_one_vhost(self):
         # Skip the enable mod
         self.config.parser.modules.add("rewrite_module")
         self.config.get_version = mock.Mock(return_value=(2, 3, 9))
 
-        self.config.enhance("encryption-example.demo", "redirect")
+        self.config.enhance("red.blue.purple.com", "redirect")
+        verify_no_redirect = ("certbot_apache.configurator."
+                              "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)
+
+    def test_redirect_from_previous_run(self):
+        # Skip the enable mod
+        self.config.parser.modules.add("rewrite_module")
+        self.config.get_version = mock.Mock(return_value=(2, 3, 9))
+
+        self.config.enhance("red.blue.purple.com", "redirect")
+        # Clear state about enabling redirect on this run
+        # pylint: disable=protected-access
+        self.config._enhanced_vhosts["redirect"].clear()
+
         self.assertRaises(
             errors.PluginEnhancementAlreadyPresent,
-            self.config.enhance, "encryption-example.demo", "redirect")
+            self.config.enhance, "green.blue.purple.com", "redirect")
 
     def test_create_own_redirect(self):
         self.config.parser.modules.add("rewrite_module")