Check version requirements on optional dependencies (#3618)

* Add and test activate function to acme.

This function can be used to check if our optional dependencies are
available and they meet our version requirements.

* use activate in dns_resolver

* use activate in dns_available() in challenges_test

* Use activate in dns_resolver_test

* Use activate in certbot.plugins.util_test

* Use acme.util.activate for psutil

* Better testing and handling of missing deps

* Factored out *_available() code into a common function

* Delayed exception caused from using acme.dns_resolver without
  dnspython until the function is called. This makes both
  production and testing code simpler.

* Make a common subclass for already_listening tests

* Simplify mocking of USE_PSUTIL in tests

7 files changed, 106 insertions, 48 deletions

diff --git a/acme/acme/challenges.py b/acme/acme/challenges.py
index 4ebd37bf9..9f9cc05b8 100644
--- a/acme/acme/challenges.py
+++ b/acme/acme/challenges.py
@@ -9,6 +9,7 @@ from cryptography.hazmat.primitives import hashes
 import OpenSSL
 import requests
 
+from acme import dns_resolver
 from acme import errors
 from acme import crypto_util
 from acme import fields
@@ -232,11 +233,11 @@ class DNS01Response(KeyAuthorizationChallengeResponse):
         logger.debug("Verifying %s at %s...", chall.typ, validation_domain_name)
 
         try:
-            from acme import dns_resolver
-        except ImportError:  # pragma: no cover
+            txt_records = dns_resolver.txt_records_for_name(
+                validation_domain_name)
+        except errors.DependencyError:
             raise errors.DependencyError("Local validation for 'dns-01' "
                                          "challenges requires 'dnspython'")
-        txt_records = dns_resolver.txt_records_for_name(validation_domain_name)
         exists = validation in txt_records
         if not exists:
             logger.debug("Key authorization from response (%r) doesn't match "
diff --git a/acme/acme/challenges_test.py b/acme/acme/challenges_test.py
index dfd40ebdb..5ac07abdd 100644
--- a/acme/acme/challenges_test.py
+++ b/acme/acme/challenges_test.py
@@ -10,6 +10,7 @@ from six.moves.urllib import parse as urllib_parse  # pylint: disable=import-err
 from acme import errors
 from acme import jose
 from acme import test_util
+from acme.dns_resolver import DNS_REQUIREMENT
 
 CERT = test_util.load_comparable_cert('cert.pem')
 KEY = jose.JWKRSA(key=test_util.load_rsa_private_key('rsa512_key.pem'))
@@ -76,20 +77,6 @@ class KeyAuthorizationChallengeResponseTest(unittest.TestCase):
         self.assertFalse(response.verify(self.chall, KEY.public_key()))
 
 
-def dns_available():
-    """Checks if dns can be imported.
-
-    :rtype: bool
-    :returns: ``True`` if dns can be imported, otherwise, ``False``
-
-    """
-    try:
-        import dns  # pylint: disable=unused-variable
-    except ImportError:  # pragma: no cover
-        return False
-    return True  # pragma: no cover
-
-
 class DNS01ResponseTest(unittest.TestCase):
     # pylint: disable=too-many-instance-attributes
 
@@ -122,7 +109,13 @@ class DNS01ResponseTest(unittest.TestCase):
         key2 = jose.JWKRSA.load(test_util.load_vector('rsa256_key.pem'))
         self.response.simple_verify(self.chall, "local", key2.public_key())
 
-    @test_util.skip_unless(dns_available(),
+    @mock.patch('acme.dns_resolver.DNS_AVAILABLE', False)
+    def test_simple_verify_without_dns(self):
+        self.assertRaises(
+            errors.DependencyError, self.response.simple_verify,
+            self.chall, 'local', KEY.public_key())
+
+    @test_util.skip_unless(test_util.requirement_available(DNS_REQUIREMENT),
                            "optional dependency dnspython is not available")
     def test_simple_verify_good_validation(self):  # pragma: no cover
         with mock.patch(self.records_for_name_path) as mock_resolver:
@@ -133,7 +126,7 @@ class DNS01ResponseTest(unittest.TestCase):
             mock_resolver.assert_called_once_with(
                 self.chall.validation_domain_name("local"))
 
-    @test_util.skip_unless(dns_available(),
+    @test_util.skip_unless(test_util.requirement_available(DNS_REQUIREMENT),
                            "optional dependency dnspython is not available")
     def test_simple_verify_good_validation_multitxts(self):  # pragma: no cover
         with mock.patch(self.records_for_name_path) as mock_resolver:
@@ -144,7 +137,7 @@ class DNS01ResponseTest(unittest.TestCase):
             mock_resolver.assert_called_once_with(
                 self.chall.validation_domain_name("local"))
 
-    @test_util.skip_unless(dns_available(),
+    @test_util.skip_unless(test_util.requirement_available(DNS_REQUIREMENT),
                            "optional dependency dnspython is not available")
     def test_simple_verify_bad_validation(self):  # pragma: no cover
         with mock.patch(self.records_for_name_path) as mock_resolver:
diff --git a/acme/acme/dns_resolver.py b/acme/acme/dns_resolver.py
index f551c6095..2677d92ad 100644
--- a/acme/acme/dns_resolver.py
+++ b/acme/acme/dns_resolver.py
@@ -3,8 +3,20 @@ Required only for local validation of 'dns-01' challenges.
 """
 import logging
 
-import dns.resolver
-import dns.exception
+from acme import errors
+from acme import util
+
+DNS_REQUIREMENT = 'dnspython>=1.12'
+
+try:
+    util.activate(DNS_REQUIREMENT)
+    # pragma: no cover
+    import dns.exception
+    import dns.resolver
+    DNS_AVAILABLE = True
+except errors.DependencyError:  # pragma: no cover
+    DNS_AVAILABLE = False
+
 
 logger = logging.getLogger(__name__)
 
@@ -18,6 +30,9 @@ def txt_records_for_name(name):
     :rtype: list of unicode
 
     """
+    if not DNS_AVAILABLE:
+        raise errors.DependencyError(
+            '{0} is required to use this function'.format(DNS_REQUIREMENT))
     try:
         dns_response = dns.resolver.query(name, 'TXT')
     except dns.resolver.NXDOMAIN as error:
diff --git a/acme/acme/dns_resolver_test.py b/acme/acme/dns_resolver_test.py
index 03f1b3a93..2e2edd0e7 100644
--- a/acme/acme/dns_resolver_test.py
+++ b/acme/acme/dns_resolver_test.py
@@ -1,17 +1,16 @@
 """Tests for acme.dns_resolver."""
-import sys
 import unittest
 
 import mock
+from six.moves import reload_module  # pylint: disable=import-error
 
+from acme import errors
 from acme import test_util
+from acme.dns_resolver import DNS_REQUIREMENT
 
 
-try:
+if test_util.requirement_available(DNS_REQUIREMENT):
     import dns
-    DNS_AVAILABLE = True  # pragma: no cover
-except ImportError:  # pragma: no cover
-    DNS_AVAILABLE = False
 
 
 def create_txt_response(name, txt_records):
@@ -25,15 +24,18 @@ def create_txt_response(name, txt_records):
     return dns.rrset.from_text_list(name, 60, "IN", "TXT", txt_records)
 
 
-@test_util.skip_unless(DNS_AVAILABLE,
-                       "optional dependency dnspython is not available")
-class DnsResolverTestWithDns(unittest.TestCase):
-    """Tests for acme.dns_resolver when dns is available."""
+class TxtRecordsForNameTest(unittest.TestCase):
+    """Tests for acme.dns_resolver.txt_records_for_name."""
     @classmethod
-    def _call(cls, name):
-        from acme import dns_resolver
-        return dns_resolver.txt_records_for_name(name)
+    def _call(cls, *args, **kwargs):
+        from acme.dns_resolver import txt_records_for_name
+        return txt_records_for_name(*args, **kwargs)
+
 
+@test_util.skip_unless(test_util.requirement_available(DNS_REQUIREMENT),
+                       "optional dependency dnspython is not available")
+class TxtRecordsForNameWithDnsTest(TxtRecordsForNameTest):
+    """Tests for acme.dns_resolver.txt_records_for_name with dns."""
     @mock.patch("acme.dns_resolver.dns.resolver.query")
     def test_txt_records_for_name_with_single_response(self, mock_dns):
         mock_dns.return_value = create_txt_response('name', ['response'])
@@ -56,24 +58,19 @@ class DnsResolverTestWithDns(unittest.TestCase):
         self.assertEquals([], self._call('name'))
 
 
-class DnsResolverTestWithoutDns(unittest.TestCase):
-    """Tests for acme.dns_resolver when dns is unavailable."""
+class TxtRecordsForNameWithoutDnsTest(TxtRecordsForNameTest):
+    """Tests for acme.dns_resolver.txt_records_for_name without dns."""
     def setUp(self):
-        self.dns_module = sys.modules['dns'] if 'dns' in sys.modules else None
-
-        if DNS_AVAILABLE:
-            sys.modules['dns'] = None  # pragma: no cover
+        from acme import dns_resolver
+        dns_resolver.DNS_AVAILABLE = False
 
     def tearDown(self):
-        if self.dns_module is not None:
-            sys.modules['dns'] = self.dns_module  # pragma: no cover
-
-    @classmethod
-    def _import_dns(cls):
-        import dns as failed_dns_import  # pylint: disable=unused-variable
+        from acme import dns_resolver
+        reload_module(dns_resolver)
 
-    def test_import_error_is_raised(self):
-        self.assertRaises(ImportError, self._import_dns)
+    def test_exception_raised(self):
+        self.assertRaises(
+            errors.DependencyError, self._call, "example.org")
 
 
 if __name__ == '__main__':
diff --git a/acme/acme/test_util.py b/acme/acme/test_util.py
index 0f5763682..ba968511f 100644
--- a/acme/acme/test_util.py
+++ b/acme/acme/test_util.py
@@ -11,7 +11,9 @@ from cryptography.hazmat.backends import default_backend
 from cryptography.hazmat.primitives import serialization
 import OpenSSL
 
+from acme import errors
 from acme import jose
+from acme import util
 
 
 def vector_path(*names):
@@ -76,6 +78,20 @@ def load_pyopenssl_private_key(*names):
     return OpenSSL.crypto.load_privatekey(loader, load_vector(*names))
 
 
+def requirement_available(requirement):
+    """Checks if requirement can be imported.
+
+    :rtype: bool
+    :returns: ``True`` iff requirement can be imported
+
+    """
+    try:
+        util.activate(requirement)
+    except errors.DependencyError:  # pragma: no cover
+        return False
+    return True  # pragma: no cover
+
+
 def skip_unless(condition, reason):  # pragma: no cover
     """Skip tests unless a condition holds.
 
diff --git a/acme/acme/util.py b/acme/acme/util.py
index 1fff89a9e..ac445b271 100644
--- a/acme/acme/util.py
+++ b/acme/acme/util.py
@@ -1,7 +1,25 @@
 """ACME utilities."""
+import pkg_resources
 import six
 
+from acme import errors
+
 
 def map_keys(dikt, func):
     """Map dictionary keys."""
     return dict((func(key), value) for key, value in six.iteritems(dikt))
+
+
+def activate(requirement):
+    """Make requirement importable.
+
+    :param str requirement: the distribution and version to activate
+
+    :raises acme.errors.DependencyError: if cannot activate requirement
+
+    """
+    try:
+        for distro in pkg_resources.require(requirement):  # pylint: disable=not-callable
+            distro.activate()
+    except (pkg_resources.DistributionNotFound, pkg_resources.VersionConflict):
+        raise errors.DependencyError('{0} is unavailable'.format(requirement))
diff --git a/acme/acme/util_test.py b/acme/acme/util_test.py
index 00aa8b02d..ba6465409 100644
--- a/acme/acme/util_test.py
+++ b/acme/acme/util_test.py
@@ -1,6 +1,8 @@
 """Tests for acme.util."""
 import unittest
 
+from acme import errors
+
 
 class MapKeysTest(unittest.TestCase):
     """Tests for acme.util.map_keys."""
@@ -12,5 +14,21 @@ class MapKeysTest(unittest.TestCase):
         self.assertEqual({2: 2, 4: 4}, map_keys({1: 2, 3: 4}, lambda x: x + 1))
 
 
+class ActivateTest(unittest.TestCase):
+    """Tests for acme.util.activate."""
+
+    @classmethod
+    def _call(cls, *args, **kwargs):
+        from acme.util import activate
+        return activate(*args, **kwargs)
+
+    def test_failure(self):
+        self.assertRaises(errors.DependencyError, self._call, 'acme>99.0.0')
+
+    def test_success(self):
+        self._call('acme')
+        import acme as unused_acme
+
+
 if __name__ == '__main__':
     unittest.main()  # pragma: no cover