Don't be unnecessarily inefficient when finding a cert by name. (#4128)

* Make lineage_for_certname and domains_for_certname O(1)

* update tests
This commit is contained in:
Erica Portnoy 2017-01-27 15:16:19 -08:00 committed by Brad Warren
parent caa7e4e3f0
commit a1b1ae25ae
2 changed files with 25 additions and 37 deletions

View file

@ -95,27 +95,23 @@ def delete(config):
# Public Helpers
###################
def lineage_for_certname(config, certname):
def lineage_for_certname(cli_config, certname):
"""Find a lineage object with name certname."""
def update_cert_for_name_match(candidate_lineage, rv):
"""Return cert if it has name certname, else return rv
"""
matching_lineage_name_cert = rv
if candidate_lineage.lineagename == certname:
matching_lineage_name_cert = candidate_lineage
return matching_lineage_name_cert
return _search_lineages(config, update_cert_for_name_match, None)
configs_dir = cli_config.renewal_configs_dir
# Verify the directory is there
util.make_or_verify_dir(configs_dir, mode=0o755, uid=os.geteuid())
renewal_file = storage.renewal_file_for_certname(cli_config, certname)
try:
return storage.RenewableCert(renewal_file, cli_config)
except (errors.CertStorageError, IOError):
logger.debug("Renewal conf file %s is broken.", renewal_file)
logger.debug("Traceback was:\n%s", traceback.format_exc())
return None
def domains_for_certname(config, certname):
"""Find the domains in the cert with name certname."""
def update_domains_for_name_match(candidate_lineage, rv):
"""Return domains if certname matches, else return rv
"""
matching_domains = rv
if candidate_lineage.lineagename == certname:
matching_domains = candidate_lineage.names()
return matching_domains
return _search_lineages(config, update_domains_for_name_match, None)
lineage = lineage_for_certname(config, certname)
return lineage.names() if lineage else None
def find_duplicative_certs(config, domains):
"""Find existing certs that duplicate the request."""

View file

@ -268,11 +268,11 @@ class LineageForCertnameTest(BaseCertManagerTest):
"""Tests for certbot.cert_manager.lineage_for_certname"""
@mock.patch('certbot.util.make_or_verify_dir')
@mock.patch('certbot.storage.renewal_conf_files')
@mock.patch('certbot.storage.renewal_file_for_certname')
@mock.patch('certbot.storage.RenewableCert')
def test_found_match(self, mock_renewable_cert, mock_renewal_conf_files,
def test_found_match(self, mock_renewable_cert, mock_renewal_conf_file,
mock_make_or_verify_dir):
mock_renewal_conf_files.return_value = ["somefile.conf"]
mock_renewal_conf_file.return_value = "somefile.conf"
mock_match = mock.Mock(lineagename="example.com")
mock_renewable_cert.return_value = mock_match
from certbot import cert_manager
@ -281,13 +281,10 @@ class LineageForCertnameTest(BaseCertManagerTest):
self.assertTrue(mock_make_or_verify_dir.called)
@mock.patch('certbot.util.make_or_verify_dir')
@mock.patch('certbot.storage.renewal_conf_files')
@mock.patch('certbot.storage.RenewableCert')
def test_no_match(self, mock_renewable_cert, mock_renewal_conf_files,
@mock.patch('certbot.storage.renewal_file_for_certname')
def test_no_match(self, mock_renewal_conf_file,
mock_make_or_verify_dir):
mock_renewal_conf_files.return_value = ["somefile.conf"]
mock_match = mock.Mock(lineagename="other.com")
mock_renewable_cert.return_value = mock_match
mock_renewal_conf_file.return_value = "other.com.conf"
from certbot import cert_manager
self.assertEqual(cert_manager.lineage_for_certname(self.cli_config, "example.com"),
None)
@ -298,11 +295,11 @@ class DomainsForCertnameTest(BaseCertManagerTest):
"""Tests for certbot.cert_manager.domains_for_certname"""
@mock.patch('certbot.util.make_or_verify_dir')
@mock.patch('certbot.storage.renewal_conf_files')
@mock.patch('certbot.storage.renewal_file_for_certname')
@mock.patch('certbot.storage.RenewableCert')
def test_found_match(self, mock_renewable_cert, mock_renewal_conf_files,
def test_found_match(self, mock_renewable_cert, mock_renewal_conf_file,
mock_make_or_verify_dir):
mock_renewal_conf_files.return_value = ["somefile.conf"]
mock_renewal_conf_file.return_value = "somefile.conf"
mock_match = mock.Mock(lineagename="example.com")
domains = ["example.com", "example.org"]
mock_match.names.return_value = domains
@ -313,15 +310,10 @@ class DomainsForCertnameTest(BaseCertManagerTest):
self.assertTrue(mock_make_or_verify_dir.called)
@mock.patch('certbot.util.make_or_verify_dir')
@mock.patch('certbot.storage.renewal_conf_files')
@mock.patch('certbot.storage.RenewableCert')
def test_no_match(self, mock_renewable_cert, mock_renewal_conf_files,
@mock.patch('certbot.storage.renewal_file_for_certname')
def test_no_match(self, mock_renewal_conf_file,
mock_make_or_verify_dir):
mock_renewal_conf_files.return_value = ["somefile.conf"]
mock_match = mock.Mock(lineagename="example.com")
domains = ["example.com", "example.org"]
mock_match.names.return_value = domains
mock_renewable_cert.return_value = mock_match
mock_renewal_conf_file.return_value = "somefile.conf"
from certbot import cert_manager
self.assertEqual(cert_manager.domains_for_certname(self.cli_config, "other.com"),
None)