Merge remote-tracking branch 'origin/master' into split-renew

letsencrypt-apache/letsencrypt_apache/tests/augeas_configurator_test.py

@@ -20,7 +20,7 @@ class AugeasConfiguratorTest(util.ApacheTest):
             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/two_vhost_80")
+            self.temp_dir, "debian_apache_2_4/multiple_vhosts")
 
     def tearDown(self):
         shutil.rmtree(self.config_dir)

letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py

@@ -20,17 +20,17 @@ from letsencrypt_apache import obj
 from letsencrypt_apache.tests import util
 
 
-class TwoVhost80Test(util.ApacheTest):
+class MultipleVhostsTest(util.ApacheTest):
     """Test two standard well-configured HTTP vhosts."""
 
     def setUp(self):  # pylint: disable=arguments-differ
-        super(TwoVhost80Test, self).setUp()
+        super(MultipleVhostsTest, self).setUp()
 
         self.config = util.get_apache_configurator(
             self.config_path, self.vhost_path, self.config_dir, self.work_dir)
         self.config = self.mock_deploy_cert(self.config)
         self.vh_truth = util.get_vh_truth(
-            self.temp_dir, "debian_apache_2_4/two_vhost_80")
+            self.temp_dir, "debian_apache_2_4/multiple_vhosts")
 
     def mock_deploy_cert(self, config):
         """A test for a mock deploy cert"""

letsencrypt-apache/letsencrypt_apache/tests/display_ops_test.py

@@ -20,7 +20,7 @@ class SelectVhostTest(unittest.TestCase):
         zope.component.provideUtility(display_util.FileDisplay(sys.stdout))
         self.base_dir = "/example_path"
         self.vhosts = util.get_vh_truth(
-            self.base_dir, "debian_apache_2_4/two_vhost_80")
+            self.base_dir, "debian_apache_2_4/multiple_vhosts")
 
     @classmethod
     def _call(cls, vhosts):

letsencrypt-apache/letsencrypt_apache/tests/parser_test.py

@@ -193,7 +193,7 @@ class ParserInitTest(util.ApacheTest):
                         "update_runtime_variables"):
             path = os.path.join(
                 self.temp_dir,
-                "debian_apache_2_4/////two_vhost_80/../two_vhost_80/apache2")
+                "debian_apache_2_4/////multiple_vhosts/../multiple_vhosts/apache2")
 
             parser = ApacheParser(self.aug, path,
                                   "/dummy/vhostpath")

letsencrypt-apache/letsencrypt_apache/tests/util.py

@@ -22,9 +22,9 @@ from letsencrypt_apache import obj
 
 class ApacheTest(unittest.TestCase):  # pylint: disable=too-few-public-methods
 
-    def setUp(self, test_dir="debian_apache_2_4/two_vhost_80",
-              config_root="debian_apache_2_4/two_vhost_80/apache2",
-              vhost_root="debian_apache_2_4/two_vhost_80/apache2/sites-available"):
+    def setUp(self, test_dir="debian_apache_2_4/multiple_vhosts",
+              config_root="debian_apache_2_4/multiple_vhosts/apache2",
+              vhost_root="debian_apache_2_4/multiple_vhosts/apache2/sites-available"):
         # pylint: disable=arguments-differ
         super(ApacheTest, self).setUp()
 
@@ -59,9 +59,9 @@ class ApacheTest(unittest.TestCase):  # pylint: disable=too-few-public-methods
 
 class ParserTest(ApacheTest):  # pytlint: disable=too-few-public-methods
 
-    def setUp(self, test_dir="debian_apache_2_4/two_vhost_80",
-              config_root="debian_apache_2_4/two_vhost_80/apache2",
-              vhost_root="debian_apache_2_4/two_vhost_80/apache2/sites-available"):
+    def setUp(self, test_dir="debian_apache_2_4/multiple_vhosts",
+              config_root="debian_apache_2_4/multiple_vhosts/apache2",
+              vhost_root="debian_apache_2_4/multiple_vhosts/apache2/sites-available"):
         super(ParserTest, self).setUp(test_dir, config_root, vhost_root)
 
         zope.component.provideUtility(display_util.FileDisplay(sys.stdout))
@@ -116,7 +116,7 @@ def get_apache_configurator(
 
 def get_vh_truth(temp_dir, config_name):
     """Return the ground truth for the specified directory."""
-    if config_name == "debian_apache_2_4/two_vhost_80":
+    if config_name == "debian_apache_2_4/multiple_vhosts":
         prefix = os.path.join(
             temp_dir, config_name, "apache2/sites-available")
         aug_pre = "/files" + prefix

letsencrypt/cli.py

@@ -137,7 +137,7 @@ def set_configurator(previously, now):
     :param str previously: previously identified request for the installer/authenticator
     :param str requested: the request currently being processed
     """
-    if now is None:
+    if not now:
         # we're not actually setting anything
         return previously
     if previously:

letsencrypt/client.py

@@ -146,8 +146,7 @@ def perform_registration(acme, config):
     try:
         return acme.register(messages.NewRegistration.from_data(email=config.email))
     except messages.Error as e:
-        err = repr(e)
-        if "MX record" in err or "Validation of contact mailto" in err:
+        if e.typ == "urn:acme:error:invalidEmail":
             config.namespace.email = display_ops.get_email(more=True, invalid=True)
             return perform_registration(acme, config)
         else:

letsencrypt/plugins/webroot.py

@@ -152,5 +152,8 @@ to serve all files under specified web root ({0})."""
                     if exc.errno == errno.ENOTEMPTY:
                         logger.debug("Challenges cleaned up but %s not empty",
                                      root_path)
+                    elif exc.errno == errno.EACCES:
+                        logger.debug("Challenges cleaned up but no permissions for %s",
+                                     root_path)
                     else:
                         raise

letsencrypt/plugins/webroot_test.py

@@ -158,7 +158,7 @@ class AuthenticatorTest(unittest.TestCase):
         os.rmdir(leftover_path)
 
     @mock.patch('os.rmdir')
-    def test_cleanup_oserror(self, mock_rmdir):
+    def test_cleanup_permission_denied(self, mock_rmdir):
         self.auth.prepare()
         self.auth.perform([self.achall])
 
@@ -166,10 +166,22 @@ class AuthenticatorTest(unittest.TestCase):
         os_error.errno = errno.EACCES
         mock_rmdir.side_effect = os_error
 
+        self.auth.cleanup([self.achall])
+        self.assertFalse(os.path.exists(self.validation_path))
+        self.assertTrue(os.path.exists(self.root_challenge_path))
+
+    @mock.patch('os.rmdir')
+    def test_cleanup_oserror(self, mock_rmdir):
+        self.auth.prepare()
+        self.auth.perform([self.achall])
+
+        os_error = OSError()
+        os_error.errno = errno.ENOENT
+        mock_rmdir.side_effect = os_error
+
         self.assertRaises(OSError, self.auth.cleanup, [self.achall])
         self.assertFalse(os.path.exists(self.validation_path))
         self.assertTrue(os.path.exists(self.root_challenge_path))
 
-
 if __name__ == "__main__":
     unittest.main()  # pragma: no cover

letsencrypt/tests/cli_test.py

@@ -52,6 +52,9 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
 
     def tearDown(self):
         shutil.rmtree(self.tmp_dir)
+        # Reset globals in cli
+        # pylint: disable=protected-access
+        cli._parser = cli._set_by_cli.detector = None
 
     def _call(self, args):
         "Run the cli with output streams and actual client mocked out"
@@ -135,7 +138,6 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         out = self._help_output(['-h'])
         self.assertTrue(cli.usage_strings(plugins)[0] in out)
 
-
     def _cli_missing_flag(self, args, message):
         "Ensure that a particular error raises a missing cli flag error containing message"
         exc = None
@@ -682,8 +684,8 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         with open(os.path.join(renewer_configs_dir, 'test.conf'), 'w') as f:
             f.write("My contents don't matter")
 
-    def _test_renew_common(self, renewalparams=None, error_expected=False,
-                           names=None, assert_oc_called=None):
+    def _test_renew_common(self, renewalparams=None, names=None,
+                           assert_oc_called=None, **kwargs):
         self._make_dummy_renewal_config()
         with mock.patch('letsencrypt.storage.RenewableCert') as mock_rc:
             mock_lineage = mock.MagicMock()
@@ -694,8 +696,9 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
                 mock_lineage.names.return_value = names
             mock_rc.return_value = mock_lineage
             with mock.patch('letsencrypt.main.obtain_cert') as mock_obtain_cert:
-                self._test_renewal_common(True, None, error_expected=error_expected,
-                                          args=['renew'], should_renew=False)
+                kwargs.setdefault('args', ['renew'])
+                self._test_renewal_common(True, None, should_renew=False, **kwargs)
+
             if assert_oc_called is not None:
                 if assert_oc_called:
                     self.assertTrue(mock_obtain_cert.called)
@@ -718,7 +721,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
     def test_renew_with_nonetype_http01(self):
         renewalparams = {'authenticator': 'webroot',
                          'http01_port': 'None'}
-        self._test_renew_common(renewalparams=renewalparams, error_expected=False,
+        self._test_renew_common(renewalparams=renewalparams,
                                 assert_oc_called=True)
 
     def test_renew_with_bad_domain(self):
@@ -727,6 +730,12 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         self._test_renew_common(renewalparams=renewalparams, error_expected=True,
                                 names=names, assert_oc_called=False)
 
+    def test_renew_with_configurator(self):
+        renewalparams = {'authenticator': 'webroot'}
+        self._test_renew_common(
+            renewalparams=renewalparams, assert_oc_called=True,
+            args='renew --configurator apache'.split())
+
     def test_renew_plugin_config_restoration(self):
         renewalparams = {'authenticator': 'webroot',
                          'webroot_path': 'None',

letsencrypt/tests/client_test.py

@@ -63,8 +63,8 @@ class RegisterTest(unittest.TestCase):
     @mock.patch("letsencrypt.client.display_ops.get_email")
     def test_email_retry(self, _rep, mock_get_email):
         from acme import messages
-        msg = "Validation of contact mailto:sousaphone@improbablylongggstring.tld failed"
-        mx_err = messages.Error(detail=msg, typ="malformed", title="title")
+        msg = "DNS problem: NXDOMAIN looking up MX for example.com"
+        mx_err = messages.Error(detail=msg, typ="urn:acme:error:invalidEmail")
         with mock.patch("letsencrypt.client.acme_client.Client") as mock_client:
             mock_client().register.side_effect = [mx_err, mock.MagicMock()]
             self._call()