Merge branch 'master' into issue_4866

5 files changed, 48 insertions, 47 deletions

diff --git a/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py b/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py
index 3622bee41..ed5cf750e 100644
--- a/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py
+++ b/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py
@@ -79,6 +79,8 @@ def _get_names(config):
                 if line.strip().startswith("server_name"):
                     names = line.partition("server_name")[2].rpartition(";")[0]
                     for n in names.split():
-                        all_names.add(n)
+                        # Filter out wildcards in both all_names and test_names
+                        if not n.startswith("*."):
+                            all_names.add(n)
     non_ip_names = set(n for n in all_names if not util.IP_REGEX.match(n))
     return all_names, non_ip_names
diff --git a/certbot-compatibility-test/certbot_compatibility_test/test_driver.py b/certbot-compatibility-test/certbot_compatibility_test/test_driver.py
index 71100bb27..71a0ba574 100644
--- a/certbot-compatibility-test/certbot_compatibility_test/test_driver.py
+++ b/certbot-compatibility-test/certbot_compatibility_test/test_driver.py
@@ -1,7 +1,6 @@
 """Tests Certbot plugins against different server configurations."""
 import argparse
 import filecmp
-import functools
 import logging
 import os
 import shutil
@@ -64,17 +63,17 @@ def test_authenticator(plugin, config, temp_dir):
                 type(achalls[i]), achalls[i].domain, config)
             success = False
         elif isinstance(responses[i], challenges.TLSSNI01Response):
-            verify = functools.partial(responses[i].simple_verify, achalls[i].chall,
-                                       achalls[i].domain,
-                                       util.JWK.public_key(),
-                                       host="127.0.0.1",
-                                       port=plugin.https_port)
-            if _try_until_true(verify):
+            verified = responses[i].simple_verify(achalls[i].chall,
+                                                  achalls[i].domain,
+                                                  util.JWK.public_key(),
+                                                  host="127.0.0.1",
+                                                  port=plugin.https_port)
+            if verified:
                 logger.info(
                     "tls-sni-01 verification for %s succeeded", achalls[i].domain)
             else:
                 logger.error(
-                    "tls-sni-01 verification for %s in %s failed",
+                    "**** tls-sni-01 verification for %s in %s failed",
                     achalls[i].domain, config)
                 success = False
 
@@ -122,7 +121,7 @@ def test_installer(args, plugin, config, temp_dir):
     if names_match:
         logger.info("get_all_names test succeeded")
     else:
-        logger.error("get_all_names test failed for config %s", config)
+        logger.error("**** get_all_names test failed for config %s", config)
 
     domains = list(plugin.get_testable_domain_names())
     success = test_deploy_cert(plugin, temp_dir, domains)
@@ -147,7 +146,7 @@ def test_deploy_cert(plugin, temp_dir, domains):
             plugin.deploy_cert(domain, cert_path, util.KEY_PATH, cert_path, cert_path)
             plugin.save()  # Needed by the Apache plugin
         except le_errors.Error as error:
-            logger.error("Plugin failed to deploy certificate for %s:", domain)
+            logger.error("**** Plugin failed to deploy certificate for %s:", domain)
             logger.exception(error)
             return False
 
@@ -155,11 +154,12 @@ def test_deploy_cert(plugin, temp_dir, domains):
         return False
 
     success = True
+    time.sleep(3)
     for domain in domains:
-        verify = functools.partial(validator.Validator().certificate, cert,
-                                   domain, "127.0.0.1", plugin.https_port)
-        if not _try_until_true(verify):
-            logger.error("Could not verify certificate for domain %s", domain)
+        verified = validator.Validator().certificate(
+            cert, domain, "127.0.0.1", plugin.https_port)
+        if not verified:
+            logger.error("**** Could not verify certificate for domain %s", domain)
             success = False
 
     if success:
@@ -177,16 +177,21 @@ def test_enhancements(plugin, domains):
                      "enhancements")
         return False
 
-    for domain in domains:
+    domains_and_info = [(domain, []) for domain in domains]
+
+    for domain, info in domains_and_info:
         try:
+            previous_redirect = validator.Validator().any_redirect(
+                "localhost", plugin.http_port, headers={"Host": domain})
+            info.append(previous_redirect)
             plugin.enhance(domain, "redirect")
             plugin.save()  # Needed by the Apache plugin
         except le_errors.PluginError as error:
             # Don't immediately fail because a redirect may already be enabled
-            logger.warning("Plugin failed to enable redirect for %s:", domain)
+            logger.warning("*** Plugin failed to enable redirect for %s:", domain)
             logger.warning("%s", error)
         except le_errors.Error as error:
-            logger.error("An error occurred while enabling redirect for %s:",
+            logger.error("*** An error occurred while enabling redirect for %s:",
                          domain)
             logger.exception(error)
 
@@ -194,12 +199,14 @@ def test_enhancements(plugin, domains):
         return False
 
     success = True
-    for domain in domains:
-        verify = functools.partial(validator.Validator().redirect, "localhost",
-                                   plugin.http_port, headers={"Host": domain})
-        if not _try_until_true(verify):
-            logger.error("Improper redirect for domain %s", domain)
-            success = False
+    for domain, info in domains_and_info:
+        previous_redirect = info[0]
+        if not previous_redirect:
+            verified = validator.Validator().redirect(
+                "localhost", plugin.http_port, headers={"Host": domain})
+            if not verified:
+                logger.error("*** Improper redirect for domain %s", domain)
+                success = False
 
     if success:
         logger.info("Enhancements test succeeded")
@@ -207,17 +214,6 @@ def test_enhancements(plugin, domains):
     return success
 
 
-def _try_until_true(func, max_tries=5, sleep_time=0.5):
-    """Calls func up to max_tries times until it returns True"""
-    for _ in xrange(0, max_tries):
-        if func():
-            return True
-        else:
-            time.sleep(sleep_time)
-
-    return False
-
-
 def _save_and_restart(plugin, title=None):
     """Saves and restart the plugin, returning True if no errors occurred"""
     try:
@@ -225,7 +221,7 @@ def _save_and_restart(plugin, title=None):
         plugin.restart()
         return True
     except le_errors.Error as error:
-        logger.error("Plugin failed to save and restart server:")
+        logger.error("*** Plugin failed to save and restart server:")
         logger.exception(error)
         return False
 
@@ -235,12 +231,12 @@ def test_rollback(plugin, config, backup):
     try:
         plugin.rollback_checkpoints(1337)
     except le_errors.Error as error:
-        logger.error("Plugin raised an exception during rollback:")
+        logger.error("*** Plugin raised an exception during rollback:")
         logger.exception(error)
         return False
 
     if _dirs_are_unequal(config, backup):
-        logger.error("Rollback failed for config `%s`", config)
+        logger.error("*** Rollback failed for config `%s`", config)
         return False
     else:
         logger.info("Rollback succeeded")
diff --git a/certbot-compatibility-test/certbot_compatibility_test/testdata/nginx.tar.gz b/certbot-compatibility-test/certbot_compatibility_test/testdata/nginx.tar.gz
index 4ca5cc977..2f06add17 100644
--- a/certbot-compatibility-test/certbot_compatibility_test/testdata/nginx.tar.gz
+++ b/certbot-compatibility-test/certbot_compatibility_test/testdata/nginx.tar.gz
diff --git a/certbot-compatibility-test/certbot_compatibility_test/validator.py b/certbot-compatibility-test/certbot_compatibility_test/validator.py
index 62dd466a1..0fd6efab5 100644
--- a/certbot-compatibility-test/certbot_compatibility_test/validator.py
+++ b/certbot-compatibility-test/certbot_compatibility_test/validator.py
@@ -45,19 +45,12 @@ class Validator(object):
         else:
             response = requests.get(url, allow_redirects=False)
 
-        if response.status_code not in (301, 303):
-            return False
-
         redirect_location = response.headers.get("location", "")
         # We're checking that the redirect we added behaves correctly.
         # It's okay for some server configuration to redirect to an
         # http URL, as long as it's on some other domain.
         if not redirect_location.startswith("https://"):
-            if not redirect_location.startswith("http://"):
-                return False
-            else:
-                if redirect_location[len("http://"):] == name:
-                    return False
+            return False
 
         if response.status_code != 301:
             logger.error("Server did not redirect with permanent code")
@@ -65,6 +58,16 @@ class Validator(object):
 
         return True
 
+    def any_redirect(self, name, port=80, headers=None):
+        """Test whether webserver redirects."""
+        url = "http://{0}:{1}".format(name, port)
+        if headers:
+            response = requests.get(url, headers=headers, allow_redirects=False)
+        else:
+            response = requests.get(url, allow_redirects=False)
+
+        return response.status_code in xrange(300, 309)
+
     def hsts(self, name):
         """Test for HTTP Strict Transport Security header"""
         headers = requests.get("https://" + name).headers
diff --git a/tox.ini b/tox.ini
index 7e7181528..299b7f20f 100644
--- a/tox.ini
+++ b/tox.ini
@@ -161,7 +161,7 @@ passenv = DOCKER_*
 commands =
     docker build -t certbot-compatibility-test -f certbot-compatibility-test/Dockerfile .
     docker build -t nginx-compat -f certbot-compatibility-test/Dockerfile-nginx .
-    docker run --rm -it nginx-compat -c nginx.tar.gz -vvvv
+    docker run --rm -it nginx-compat -c nginx.tar.gz -vv -aie
 whitelist_externals =
     docker
 passenv = DOCKER_*