diff options
author | Brad Warren <bmw@users.noreply.github.com> | 2016-10-12 03:50:11 +0300 |
---|---|---|
committer | Peter Eckersley <pde@users.noreply.github.com> | 2016-10-12 03:50:11 +0300 |
commit | f5bf66ba36d54a1362c91bee4d4f0c7bc800a055 (patch) | |
tree | f11b4c4c12ec9c9dfc30e644a3fcfcf4fdbfa8ad /acme | |
parent | e1da0efb8aef65e474a9ffa78c9c7fc01afb66f1 (diff) |
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
Diffstat (limited to 'acme')
-rw-r--r-- | acme/acme/challenges.py | 7 | ||||
-rw-r--r-- | acme/acme/challenges_test.py | 27 | ||||
-rw-r--r-- | acme/acme/dns_resolver.py | 19 | ||||
-rw-r--r-- | acme/acme/dns_resolver_test.py | 49 | ||||
-rw-r--r-- | acme/acme/test_util.py | 16 | ||||
-rw-r--r-- | acme/acme/util.py | 18 | ||||
-rw-r--r-- | acme/acme/util_test.py | 18 |
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 |