From 7828853e8c593ded6e20233580e08ff4ab7ac7fd Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Mon, 2 Feb 2015 14:14:32 +0000 Subject: [PATCH] Remove "direc" magic, use IConfig instead. f(config.x, config) -> f(config) --- letsencrypt/client/apache/configurator.py | 33 +++------ letsencrypt/client/apache/dvsni.py | 39 +++++----- letsencrypt/client/augeas_configurator.py | 22 ++---- letsencrypt/client/client.py | 31 +++----- letsencrypt/client/client_authenticator.py | 6 +- letsencrypt/client/reverter.py | 73 +++++++++---------- letsencrypt/client/revoker.py | 6 +- letsencrypt/client/tests/apache/dvsni_test.py | 13 ++-- letsencrypt/client/tests/apache/util.py | 19 +++-- .../client/tests/client_authenticator_test.py | 6 +- letsencrypt/client/tests/client_test.py | 2 +- letsencrypt/client/tests/reverter_test.py | 65 +++++++---------- letsencrypt/scripts/main.py | 6 +- 13 files changed, 138 insertions(+), 183 deletions(-) diff --git a/letsencrypt/client/apache/configurator.py b/letsencrypt/client/apache/configurator.py index 1f9ecbc0b..6b7e03f24 100644 --- a/letsencrypt/client/apache/configurator.py +++ b/letsencrypt/client/apache/configurator.py @@ -65,9 +65,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): :ivar config: Configuration. :type config: :class:`~letsencrypt.client.interfaces.IConfig` - :ivar str server_root: Path to Apache root directory - :ivar dict location: Path to various files associated - with the configuration :ivar tup version: version of Apache :ivar list vhosts: All vhosts found in the configuration (:class:`list` of :class:`letsencrypt.client.apache.obj.VirtualHost`) @@ -77,34 +74,22 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ zope.interface.implements(interfaces.IAuthenticator, interfaces.IInstaller) - def __init__(self, config, direc=None, version=None): + def __init__(self, config, version=None): """Initialize an Apache Configurator. - :param dict direc: locations of various config directories - (used mostly for unittesting) :param tup version: version of Apache as a tuple (2, 4, 7) (used mostly for unittesting) """ - self.config = config - server_root = self.config.apache_server_root - ssl_options = self.config.apache_mod_ssl_conf - - if direc is None: - direc = {"backup": self.config.backup_dir, - "temp": self.config.temp_checkpoint_dir, - "progress": self.config.in_progress_dir, - "config": self.config.config_dir, - "work": self.config.work_dir} - - super(ApacheConfigurator, self).__init__(config, direc) - self.direc = direc + super(ApacheConfigurator, self).__init__(config) # Verify that all directories and files exist with proper permissions if os.geteuid() == 0: self.verify_setup() - self.parser = parser.ApacheParser(self.aug, server_root, ssl_options) + self.parser = parser.ApacheParser( + self.aug, self.config.apache_server_root, + self.config.apache_mod_ssl_conf) # Check for errors in parsing files with Augeas self.check_parsing_errors("httpd.aug") @@ -126,7 +111,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): self._prepare_server_https() self.enhance_func = {"redirect": self._enable_redirect} - temp_install(ssl_options) + temp_install(self.config.apache_mod_ssl_conf) def deploy_cert(self, domain, cert, key, cert_chain=None): """Deploys certificate to specified virtual host. @@ -954,9 +939,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ uid = os.geteuid() - le_util.make_or_verify_dir(self.direc["config"], 0o755, uid) - le_util.make_or_verify_dir(self.direc["work"], 0o755, uid) - le_util.make_or_verify_dir(self.direc["backup"], 0o755, uid) + le_util.make_or_verify_dir(self.config.config_dir, 0o755, uid) + le_util.make_or_verify_dir(self.config.work_dir, 0o755, uid) + le_util.make_or_verify_dir(self.config.backup_dir, 0o755, uid) ########################################################################### # Challenges Section diff --git a/letsencrypt/client/apache/dvsni.py b/letsencrypt/client/apache/dvsni.py index 242c0d324..79ea5f5c4 100644 --- a/letsencrypt/client/apache/dvsni.py +++ b/letsencrypt/client/apache/dvsni.py @@ -11,8 +11,8 @@ from letsencrypt.client.apache import parser class ApacheDvsni(object): """Class performs DVSNI challenges within the Apache configurator. - :ivar config: ApacheConfigurator object - :type config: + :ivar configurator: ApacheConfigurator object + :type configurator: :class:`letsencrypt.client.apache.configurator.ApacheConfigurator` :ivar dvsni_chall: Data required for challenges. @@ -33,12 +33,12 @@ class ApacheDvsni(object): :param str challenge_conf: location of the challenge config file """ - def __init__(self, config): - self.config = config + def __init__(self, configurator): + self.configurator = configurator self.dvsni_chall = [] self.indices = [] self.challenge_conf = os.path.join( - config.direc["config"], "le_dvsni_cert_challenge.conf") + configurator.config.config_dir, "le_dvsni_cert_challenge.conf") # self.completed = 0 def add_chall(self, chall, idx=None): @@ -60,12 +60,12 @@ class ApacheDvsni(object): return None # Save any changes to the configuration as a precaution # About to make temporary changes to the config - self.config.save() + self.configurator.save() addresses = [] default_addr = "*:443" for chall in self.dvsni_chall: - vhost = self.config.choose_vhost(chall.domain) + vhost = self.configurator.choose_vhost(chall.domain) if vhost is None: logging.error( "No vhost exists with servername or alias of: %s", @@ -75,7 +75,7 @@ class ApacheDvsni(object): return None # TODO - @jdkasten review this code to make sure it makes sense - self.config.make_server_sni_ready(vhost, default_addr) + self.configurator.make_server_sni_ready(vhost, default_addr) for addr in vhost.addrs: if "_default_" == addr.get_addr(): @@ -95,7 +95,7 @@ class ApacheDvsni(object): self._mod_config(addresses) # Save reversible changes - self.config.save("SNI Challenge", True) + self.configurator.save("SNI Challenge", True) return responses @@ -103,7 +103,7 @@ class ApacheDvsni(object): """Generate and write out challenge certificate.""" cert_path = self.get_cert_file(chall.nonce) # Register the path before you write out the file - self.config.reverter.register_file_creation(True, cert_path) + self.configurator.reverter.register_file_creation(True, cert_path) cert_pem, s_b64 = challenge_util.dvsni_gen_cert( chall.domain, chall.r_b64, chall.nonce, chall.key) @@ -131,8 +131,9 @@ class ApacheDvsni(object): self.dvsni_chall[idx].key.file) config_text += "\n" - self._conf_include_check(self.config.parser.loc["default"]) - self.config.reverter.register_file_creation(True, self.challenge_conf) + self._conf_include_check(self.configurator.parser.loc["default"]) + self.configurator.reverter.register_file_creation( + True, self.challenge_conf) with open(self.challenge_conf, 'w') as new_conf: new_conf.write(config_text) @@ -146,11 +147,12 @@ class ApacheDvsni(object): :param str main_config: file path to main user apache config file """ - if len(self.config.parser.find_dir( + if len(self.configurator.parser.find_dir( parser.case_i("Include"), self.challenge_conf)) == 0: # print "Including challenge virtual host(s)" - self.config.parser.add_dir(parser.get_aug_path(main_config), - "Include", self.challenge_conf) + self.configurator.parser.add_dir( + parser.get_aug_path(main_config), + "Include", self.challenge_conf) def _get_config_text(self, nonce, ip_addrs, dvsni_key_file): """Chocolate virtual server configuration text @@ -172,11 +174,12 @@ class ApacheDvsni(object): "\n" "LimitRequestBody 1048576\n" "\n" - "Include " + self.config.parser.loc["ssl_options"] + "\n" + "Include " + self.configurator.parser.loc["ssl_options"] + "\n" "SSLCertificateFile " + self.get_cert_file(nonce) + "\n" "SSLCertificateKeyFile " + dvsni_key_file + "\n" "\n" - "DocumentRoot " + self.config.direc["config"] + "dvsni_page/\n" + "DocumentRoot " + + self.configurator.config.config_dir + "dvsni_page/\n" "\n\n") def get_cert_file(self, nonce): @@ -188,4 +191,4 @@ class ApacheDvsni(object): :rtype: str """ - return self.config.direc["work"] + nonce + ".crt" + return self.configurator.config.work_dir + nonce + ".crt" diff --git a/letsencrypt/client/augeas_configurator.py b/letsencrypt/client/augeas_configurator.py index 32435dde0..8854fef09 100644 --- a/letsencrypt/client/augeas_configurator.py +++ b/letsencrypt/client/augeas_configurator.py @@ -9,30 +9,20 @@ from letsencrypt.client import reverter class AugeasConfigurator(object): """Base Augeas Configurator class. + :ivar config: Configuration. + :type config: :class:`~letsencrypt.client.interfaces.IConfig` + :ivar aug: Augeas object :type aug: :class:`augeas.Augeas` :ivar str save_notes: Human-readable configuration change notes - :ivar dict direc: dictionary containing save directory paths :ivar reverter: saves and reverts checkpoints :type reverter: :class:`letsencrypt.client.reverter.Reverter` """ - def __init__(self, config, direc=None): - """Initialize Augeas Configurator. - - :param config: Configuration. - :type config: :class:`~letsencrypt.client.interfaces.IConfig` - - :param dict direc: location of save directories - (used mostly for testing) - - """ - if not direc: - direc = {"backup": config.backup_dir, - "temp": config.temp_checkpoint_dir, - "progress": config.in_progress_dir} + def __init__(self, config): + self.config = config # Set Augeas flags to not save backup (we do it ourselves) # Set Augeas to not load anything by default @@ -44,7 +34,7 @@ class AugeasConfigurator(object): # This needs to occur before VirtualHost objects are setup... # because this will change the underlying configuration and potential # vhosts - self.reverter = reverter.Reverter(config, direc) + self.reverter = reverter.Reverter(config) self.reverter.recovery_routine() def check_parsing_errors(self, lens): diff --git a/letsencrypt/client/client.py b/letsencrypt/client/client.py index 4765e71ec..6cc1ad81c 100644 --- a/letsencrypt/client/client.py +++ b/letsencrypt/client/client.py @@ -49,16 +49,15 @@ class Client(object): # Note: form is the type of data, "pem" or "der" CSR = collections.namedtuple("CSR", "file data form") - def __init__(self, server, authkey, dv_auth, installer, config): + def __init__(self, config, authkey, dv_auth, installer): """Initialize a client. - :param str server: CA server to contact :param dv_auth: IAuthenticator that can solve the :const:`letsencrypt.client.constants.DV_CHALLENGES` :type dv_auth: :class:`letsencrypt.client.interfaces.IAuthenticator` """ - self.network = network.Network(server) + self.network = network.Network(config.server) self.authkey = authkey self.installer = installer @@ -66,8 +65,7 @@ class Client(object): self.config = config if dv_auth is not None: - client_auth = client_authenticator.ClientAuthenticator( - server, config) + client_auth = client_authenticator.ClientAuthenticator(config) self.auth_handler = auth_handler.AuthHandler( dv_auth, client_auth, self.network) else: @@ -86,9 +84,6 @@ class Client(object): :rtype: `tuple` of `str` """ - cert_path = self.config.cert_path - chain_path = self.config.chain_path - if self.auth_handler is None: logging.warning("Unable to obtain a certificate, because client " "does not have a valid auth handler.") @@ -110,7 +105,7 @@ class Client(object): # Save Certificate cert_file, chain_file = self.save_certificate( - certificate_dict, cert_path, chain_path) + certificate_dict, self.config.cert_path, self.config.chain_path) self.store_cert_key(cert_file, False) @@ -426,7 +421,7 @@ def determine_installer(config): logging.info("Unable to find a way to install the certificate.") -def rollback(checkpoints, config): +def rollback(config): """Revert configuration the specified number of checkpoints. .. note:: If another installer uses something other than the reverter class @@ -441,8 +436,6 @@ def rollback(checkpoints, config): of future installers. Perhaps the interface should define errors that are thrown for the various functions. - :param int checkpoints: Number of checkpoints to revert. - :param config: Configuration. :type config: :class:`letsencrypt.client.interfaces.IConfig` @@ -451,18 +444,18 @@ def rollback(checkpoints, config): try: installer = determine_installer(config) except errors.LetsEncryptMisconfigurationError: - _misconfigured_rollback(checkpoints, config) + _misconfigured_rollback(config) return # No Errors occurred during init... proceed normally # If installer is None... couldn't find an installer... there shouldn't be # anything to rollback if installer is not None: - installer.rollback_checkpoints(checkpoints) + installer.rollback_checkpoints(config.rollback) installer.restart() -def _misconfigured_rollback(checkpoints, config): +def _misconfigured_rollback(config): """Handles the case where the Installer is misconfigured. :param config: Configuration. @@ -484,7 +477,7 @@ def _misconfigured_rollback(checkpoints, config): # Also... not sure how future installers will handle recovery. rev = reverter.Reverter(config) rev.recovery_routine() - rev.rollback_checkpoints(checkpoints) + rev.rollback_checkpoints(config.rollback) # We should try to restart the server try: @@ -497,11 +490,9 @@ def _misconfigured_rollback(checkpoints, config): "Rollback was unable to solve the misconfiguration issues") -def revoke(server, config): +def revoke(config): """Revoke certificates. - :param str server: ACME server the client wishes to revoke certificates from - :param config: Configuration. :type config: :class:`letsencrypt.client.interfaces.IConfig` @@ -525,7 +516,7 @@ def revoke(server, config): "revocation without a valid installer. This feature should come " "soon.") return - revoc = revoker.Revoker(server, installer, config) + revoc = revoker.Revoker(installer, config) revoc.list_certs_keys() diff --git a/letsencrypt/client/client_authenticator.py b/letsencrypt/client/client_authenticator.py index d5c0aa5bc..2897a5132 100644 --- a/letsencrypt/client/client_authenticator.py +++ b/letsencrypt/client/client_authenticator.py @@ -18,17 +18,15 @@ class ClientAuthenticator(object): zope.interface.implements(interfaces.IAuthenticator) # This will have an installer soon for get_key/cert purposes - def __init__(self, server, config): + def __init__(self, config): """Initialize Client Authenticator. - :param str server: ACME CA Server - :param config: Configuration. :type config: :class:`letsencrypt.client.interfaces.IConfig` """ self.rec_token = recovery_token.RecoveryToken( - server, config.rev_token_dirs) + config.acme_server, config.rev_token_dirs) def get_chall_pref(self, unused_domain): # pylint: disable=no-self-use """Return list of challenge preferences.""" diff --git a/letsencrypt/client/reverter.py b/letsencrypt/client/reverter.py index a67f8f987..3f008fc38 100644 --- a/letsencrypt/client/reverter.py +++ b/letsencrypt/client/reverter.py @@ -13,20 +13,14 @@ from letsencrypt.client import le_util class Reverter(object): - """Reverter Class - save and revert configuration checkpoints.""" + """Reverter Class - save and revert configuration checkpoints. - def __init__(self, config, direc=None): - """Initialize Reverter. + :param config: Configuration. + :type config: :class:`letsencrypt.client.interfaces.IConfig` - :param config: Configuration. - :type config: :class:`letsencrypt.client.interfaces.IConfig` - - """ - if direc is None: - direc = {'backup': config.backup_dir, - 'temp': config.temp_checkpoint_dir, - 'progress': config.in_progress_dir} - self.direc = direc + """ + def __init__(self, config): + self.config = config def revert_temporary_config(self): """Reload users original configuration files after a temporary save. @@ -38,13 +32,13 @@ class Reverter(object): Unable to revert config """ - if os.path.isdir(self.direc['temp']): + if os.path.isdir(self.config.temp_checkpoint_dir): try: - self._recover_checkpoint(self.direc['temp']) + self._recover_checkpoint(self.config.temp_checkpoint_dir) except errors.LetsEncryptReverterError: # We have a partial or incomplete recovery logging.fatal("Incomplete or failed recovery for %s", - self.direc['temp']) + self.config.temp_checkpoint_dir) raise errors.LetsEncryptReverterError( "Unable to revert temporary config") @@ -69,7 +63,7 @@ class Reverter(object): logging.error("Rollback argument must be a positive integer") raise errors.LetsEncryptReverterError("Invalid Input") - backups = os.listdir(self.direc['backup']) + backups = os.listdir(self.config.backup_dir) backups.sort() if len(backups) < rollback: @@ -77,7 +71,7 @@ class Reverter(object): rollback, len(backups)) while rollback > 0 and backups: - cp_dir = os.path.join(self.direc['backup'], backups.pop()) + cp_dir = os.path.join(self.config.backup_dir, backups.pop()) try: self._recover_checkpoint(cp_dir) except errors.LetsEncryptReverterError: @@ -94,7 +88,7 @@ class Reverter(object): .. todo:: Decide on a policy for error handling, OSError IOError... """ - backups = os.listdir(self.direc['backup']) + backups = os.listdir(self.config.backup_dir) backups.sort(reverse=True) if not backups: @@ -108,12 +102,12 @@ class Reverter(object): float(bkup) except ValueError: raise errors.LetsEncryptReverterError( - "Invalid directories in {0}".format(self.direc['backup'])) + "Invalid directories in {0}".format(self.config.backup_dir)) output = [] for bkup in backups: output.append(time.ctime(float(bkup))) - cur_dir = os.path.join(self.direc['backup'], bkup) + cur_dir = os.path.join(self.config.backup_dir, bkup) with open(os.path.join(cur_dir, "CHANGES_SINCE")) as changes_fd: output.append(changes_fd.read()) @@ -142,7 +136,8 @@ class Reverter(object): param str save_notes: notes about changes during the save """ - self._add_to_checkpoint_dir(self.direc['temp'], save_files, save_notes) + self._add_to_checkpoint_dir( + self.config.temp_checkpoint_dir, save_files, save_notes) def add_to_checkpoint(self, save_files, save_notes): """Add files to a permanent checkpoint @@ -154,7 +149,7 @@ class Reverter(object): # Check to make sure we are not overwriting a temp file self._check_tempfile_saves(save_files) self._add_to_checkpoint_dir( - self.direc['progress'], save_files, save_notes) + self.config.in_progress_dir, save_files, save_notes) def _add_to_checkpoint_dir(self, cp_dir, save_files, save_notes): """Add save files to checkpoint directory. @@ -264,13 +259,13 @@ class Reverter(object): protected_files = [] # Get temp modified files - temp_path = os.path.join(self.direc['temp'], "FILEPATHS") + temp_path = os.path.join(self.config.temp_checkpoint_dir, "FILEPATHS") if os.path.isfile(temp_path): with open(temp_path, 'r') as protected_fd: protected_files.extend(protected_fd.read().splitlines()) # Get temp new files - new_path = os.path.join(self.direc['temp'], "NEW_FILES") + new_path = os.path.join(self.config.temp_checkpoint_dir, "NEW_FILES") if os.path.isfile(new_path): with open(new_path, 'r') as protected_fd: protected_files.extend(protected_fd.read().splitlines()) @@ -305,9 +300,9 @@ class Reverter(object): "Forgot to provide files to registration call") if temporary: - cp_dir = self.direc['temp'] + cp_dir = self.config.temp_checkpoint_dir else: - cp_dir = self.direc['progress'] + cp_dir = self.config.in_progress_dir le_util.make_or_verify_dir(cp_dir, 0o755, os.geteuid()) @@ -331,7 +326,7 @@ class Reverter(object): def recovery_routine(self): """Revert all previously modified files. - First, any changes found in self.direc['temp'] are removed, + First, any changes found in IConfig.temp_checkpoint_dir are removed, then IN_PROGRESS changes are removed The order is important. IN_PROGRESS is unable to add files that are already added by a TEMP change. Thus TEMP must be rolled back first because that will be the @@ -339,17 +334,17 @@ class Reverter(object): """ self.revert_temporary_config() - if os.path.isdir(self.direc['progress']): + if os.path.isdir(self.config.in_progress_dir): try: - self._recover_checkpoint(self.direc['progress']) + self._recover_checkpoint(self.config.in_progress_dir) except errors.LetsEncryptReverterError: # We have a partial or incomplete recovery logging.fatal("Incomplete or failed recovery for IN_PROGRESS " "checkpoint - %s", - self.direc['progress']) + self.config.in_progress_dir) raise errors.LetsEncryptReverterError( "Incomplete or failed recovery for IN_PROGRESS checkpoint " - "- %s" % self.direc['progress']) + "- %s" % self.config.in_progress_dir) def _remove_contained_files(self, file_list): # pylint: disable=no-self-use """Erase all files contained within file_list. @@ -392,8 +387,8 @@ class Reverter(object): def finalize_checkpoint(self, title): """Move IN_PROGRESS checkpoint to timestamped checkpoint. - Adds title to self.direc['progress'] CHANGES_SINCE - Move self.direc['progress'] to Backups directory and + Adds title to self.config.in_progress_dir CHANGES_SINCE + Move self.config.in_progress_dir to Backups directory and rename the directory as a timestamp :param str title: Title describing checkpoint @@ -402,14 +397,14 @@ class Reverter(object): """ # Check to make sure an "in progress" directory exists - if not os.path.isdir(self.direc['progress']): + if not os.path.isdir(self.config.in_progress_dir): logging.warning("No IN_PROGRESS checkpoint to finalize") return changes_since_path = os.path.join( - self.direc['progress'], 'CHANGES_SINCE') + self.config.in_progress_dir, 'CHANGES_SINCE') changes_since_tmp_path = os.path.join( - self.direc['progress'], 'CHANGES_SINCE.tmp') + self.config.in_progress_dir, 'CHANGES_SINCE.tmp') try: with open(changes_since_tmp_path, 'w') as changes_tmp: @@ -430,9 +425,9 @@ class Reverter(object): # collisions in the naming convention. cur_time = time.time() for _ in range(10): - final_dir = os.path.join(self.direc['backup'], str(cur_time)) + final_dir = os.path.join(self.config.backup_dir, str(cur_time)) try: - os.rename(self.direc['progress'], final_dir) + os.rename(self.config.in_progress_dir, final_dir) return except OSError: # It is possible if the checkpoints are made extremely quickly @@ -443,6 +438,6 @@ class Reverter(object): # After 10 attempts... something is probably wrong here... logging.error( "Unable to finalize checkpoint, %s -> %s", - self.direc['progress'], final_dir) + self.config.in_progress_dir, final_dir) raise errors.LetsEncryptReverterError( "Unable to finalize checkpoint renaming") diff --git a/letsencrypt/client/revoker.py b/letsencrypt/client/revoker.py index 25c912975..3a194610b 100644 --- a/letsencrypt/client/revoker.py +++ b/letsencrypt/client/revoker.py @@ -17,13 +17,13 @@ from letsencrypt.client import network class Revoker(object): """A revocation class for LE. - :param config: Configuration. + :ivar config: Configuration. :type config: :class:`letsencrypt.client.interfaces.IConfig` """ - def __init__(self, server, installer, config): - self.network = network.Network(server) + def __init__(self, installer, config): + self.network = network.Network(config.acme_server) self.installer = installer self.config = config diff --git a/letsencrypt/client/tests/apache/dvsni_test.py b/letsencrypt/client/tests/apache/dvsni_test.py index e831747a8..848652f67 100644 --- a/letsencrypt/client/tests/apache/dvsni_test.py +++ b/letsencrypt/client/tests/apache/dvsni_test.py @@ -100,7 +100,7 @@ class DvsniPerformTest(util.ApacheTest): # Check to make sure challenge config path is included in apache config. self.assertEqual( - len(self.sni.config.parser.find_dir( + len(self.sni.configurator.parser.find_dir( "Include", self.sni.challenge_conf)), 1) self.assertEqual(len(responses), 1) @@ -125,7 +125,7 @@ class DvsniPerformTest(util.ApacheTest): mock_setup_cert.call_args_list[1], mock.call(self.challs[1])) self.assertEqual( - len(self.sni.config.parser.find_dir( + len(self.sni.configurator.parser.find_dir( "Include", self.sni.challenge_conf)), 1) self.assertEqual(len(responses), 2) @@ -142,16 +142,17 @@ class DvsniPerformTest(util.ApacheTest): ll_addr.append(v_addr1) ll_addr.append(v_addr2) self.sni._mod_config(ll_addr) # pylint: disable=protected-access - self.sni.config.save() + self.sni.configurator.save() - self.sni.config.parser.find_dir("Include", self.sni.challenge_conf) - vh_match = self.sni.config.aug.match( + self.sni.configurator.parser.find_dir( + "Include", self.sni.challenge_conf) + vh_match = self.sni.configurator.aug.match( "/files" + self.sni.challenge_conf + "//VirtualHost") vhs = [] for match in vh_match: # pylint: disable=protected-access - vhs.append(self.sni.config._create_vhost(match)) + vhs.append(self.sni.configurator._create_vhost(match)) self.assertEqual(len(vhs), 2) for vhost in vhs: if vhost.addrs == set(v_addr1): diff --git a/letsencrypt/client/tests/apache/util.py b/letsencrypt/client/tests/apache/util.py index eff556f5d..df981aede 100644 --- a/letsencrypt/client/tests/apache/util.py +++ b/letsencrypt/client/tests/apache/util.py @@ -65,16 +65,15 @@ def get_apache_configurator( # This just states that the ssl module is already loaded mock_popen().communicate.return_value = ("ssl_module", "") config = configurator.ApacheConfigurator( - mock.MagicMock(apache_server_root=config_path, - apache_mod_ssl_conf=ssl_options, - le_vhost_ext="-le-ssl.conf"), - { - "backup": backups, - "temp": os.path.join(work_dir, "temp_checkpoint"), - "progress": os.path.join(backups, "IN_PROGRESS"), - "config": config_dir, - "work": work_dir, - }, + mock.MagicMock( + apache_server_root=config_path, + apache_mod_ssl_conf=ssl_options, + le_vhost_ext="-le-ssl.conf", + backup_dir=backups, + config_dir=config_dir, + temp_checkpoint_dir=os.path.join(work_dir, "temp_checkpoints"), + in_progress_dir=os.path.join(backups, "IN_PROGRESS"), + work_dir=work_dir), version) return config diff --git a/letsencrypt/client/tests/client_authenticator_test.py b/letsencrypt/client/tests/client_authenticator_test.py index 86c1220e3..2f49b07d7 100644 --- a/letsencrypt/client/tests/client_authenticator_test.py +++ b/letsencrypt/client/tests/client_authenticator_test.py @@ -10,7 +10,8 @@ class PerformTest(unittest.TestCase): def setUp(self): from letsencrypt.client.client_authenticator import ClientAuthenticator - self.auth = ClientAuthenticator("demo_server.org", mock.MagicMock()) + self.auth = ClientAuthenticator( + mock.MagicMock(acme_server="demo_server.org")) self.auth.rec_token.perform = mock.MagicMock( name="rec_token_perform", side_effect=gen_client_resp) @@ -50,7 +51,8 @@ class CleanupTest(unittest.TestCase): def setUp(self): from letsencrypt.client.client_authenticator import ClientAuthenticator - self.auth = ClientAuthenticator("demo_server.org", mock.MagicMock()) + self.auth = ClientAuthenticator(mock.MagicMock( + acme_server="demo_server.org")) self.mock_cleanup = mock.MagicMock(name="rec_token_cleanup") self.auth.rec_token.cleanup = self.mock_cleanup diff --git a/letsencrypt/client/tests/client_test.py b/letsencrypt/client/tests/client_test.py index df07d1fa9..6b9235e11 100644 --- a/letsencrypt/client/tests/client_test.py +++ b/letsencrypt/client/tests/client_test.py @@ -14,7 +14,7 @@ class RollbackTest(unittest.TestCase): @classmethod def _call(cls, checkpoints): from letsencrypt.client.client import rollback - rollback(checkpoints, mock.MagicMock()) + rollback(mock.MagicMock(rollback=checkpoints)) @mock.patch("letsencrypt.client.client.determine_installer") def test_no_problems(self, mock_det): diff --git a/letsencrypt/client/tests/reverter_test.py b/letsencrypt/client/tests/reverter_test.py index 1ab4bb70e..d60aa91a6 100644 --- a/letsencrypt/client/tests/reverter_test.py +++ b/letsencrypt/client/tests/reverter_test.py @@ -19,15 +19,14 @@ class ReverterCheckpointLocalTest(unittest.TestCase): # Disable spurious errors... we are trying to test for them logging.disable(logging.CRITICAL) - self.work_dir, self.direc = setup_work_direc() - - self.reverter = Reverter(mock.MagicMock(), self.direc) + self.config = setup_work_direc() + self.reverter = Reverter(self.config) tup = setup_test_files() self.config1, self.config2, self.dir1, self.dir2, self.sets = tup def tearDown(self): - shutil.rmtree(self.work_dir) + shutil.rmtree(self.config.work_dir) shutil.rmtree(self.dir1) shutil.rmtree(self.dir2) @@ -38,13 +37,14 @@ class ReverterCheckpointLocalTest(unittest.TestCase): self.reverter.add_to_temp_checkpoint(self.sets[0], "save1") self.reverter.add_to_temp_checkpoint(self.sets[1], "save2") - self.assertTrue(os.path.isdir(self.reverter.direc['temp'])) - self.assertEqual(get_save_notes(self.direc['temp']), "save1save2") + self.assertTrue(os.path.isdir(self.config.temp_checkpoint_dir)) + self.assertEqual(get_save_notes( + self.config.temp_checkpoint_dir), "save1save2") self.assertFalse(os.path.isfile( - os.path.join(self.direc['temp'], "NEW_FILES"))) + os.path.join(self.config.temp_checkpoint_dir, "NEW_FILES"))) self.assertEqual( - get_filepaths(self.direc['temp']), + get_filepaths(self.config.temp_checkpoint_dir), "{0}\n{1}\n".format(self.config1, self.config2)) def test_add_to_checkpoint_copy_failure(self): @@ -113,7 +113,7 @@ class ReverterCheckpointLocalTest(unittest.TestCase): self.reverter.register_file_creation(True, self.config1) self.reverter.register_file_creation(True, self.config1) - files = get_new_files(self.direc['temp']) + files = get_new_files(self.config.temp_checkpoint_dir) self.assertEqual(len(files), 1) @@ -240,14 +240,14 @@ class TestFullCheckpointsReverter(unittest.TestCase): # Disable spurious errors... logging.disable(logging.CRITICAL) - self.work_dir, self.direc = setup_work_direc() - self.reverter = Reverter(mock.MagicMock(), self.direc) + self.config = setup_work_direc() + self.reverter = Reverter(self.config) tup = setup_test_files() self.config1, self.config2, self.dir1, self.dir2, self.sets = tup def tearDown(self): - shutil.rmtree(self.work_dir) + shutil.rmtree(self.config.work_dir) shutil.rmtree(self.dir1) shutil.rmtree(self.dir2) @@ -269,7 +269,7 @@ class TestFullCheckpointsReverter(unittest.TestCase): config3 = self._setup_three_checkpoints() # Check resulting backup directory - self.assertEqual(len(os.listdir(self.direc['backup'])), 3) + self.assertEqual(len(os.listdir(self.config.backup_dir)), 3) # Check rollbacks # First rollback self.reverter.rollback_checkpoints(1) @@ -285,11 +285,11 @@ class TestFullCheckpointsReverter(unittest.TestCase): self.assertFalse(os.path.isfile(config3)) # One dir left... check title - all_dirs = os.listdir(self.direc['backup']) + all_dirs = os.listdir(self.config.backup_dir) self.assertEqual(len(all_dirs), 1) self.assertTrue( "First Checkpoint" in get_save_notes( - os.path.join(self.direc['backup'], all_dirs[0]))) + os.path.join(self.config.backup_dir, all_dirs[0]))) # Final rollback self.reverter.rollback_checkpoints(1) self.assertEqual(read_in(self.config1), "directive-dir1") @@ -350,7 +350,7 @@ class TestFullCheckpointsReverter(unittest.TestCase): def test_view_config_changes_bad_backups_dir(self): # There shouldn't be any "in progess directories when this is called # It must just be clean checkpoints - os.makedirs(os.path.join(self.direc['backup'], "in_progress")) + os.makedirs(os.path.join(self.config.backup_dir, "in_progress")) self.assertRaises(errors.LetsEncryptReverterError, self.reverter.view_config_changes) @@ -384,29 +384,20 @@ class TestFullCheckpointsReverter(unittest.TestCase): return config3 -class QuickInitReverterTest(unittest.TestCase): - # pylint: disable=too-few-public-methods - """Quick test of init.""" - - def test_init(self): - from letsencrypt.client.reverter import Reverter - config = mock.MagicMock() - rev = Reverter(config) - self.assertEqual(rev.direc['backup'], config.backup_dir) - self.assertEqual(rev.direc['temp'], config.temp_checkpoint_dir) - self.assertEqual(rev.direc['progress'], config.in_progress_dir) - - def setup_work_direc(): - """Setup directories.""" - work_dir = tempfile.mkdtemp("work") - backup = os.path.join(work_dir, "backup") - os.makedirs(backup) - direc = {'backup': backup, - 'temp': os.path.join(work_dir, "temp"), - 'progress': os.path.join(backup, "progress")} + """Setup directories. - return work_dir, direc + :returns: Mocked :class:`letsencrypt.client.interfaces.IConfig` + + """ + work_dir = tempfile.mkdtemp("work") + backup_dir = os.path.join(work_dir, "backup") + os.makedirs(backup_dir) + + return mock.MagicMock( + work_dir=work_dir, backup_dir=backup_dir, + temp_checkpoint_dir=os.path.join(work_dir, "temp"), + in_progress_dir=os.path.join(backup_dir, "in_progress_dir")) def setup_test_files(): diff --git a/letsencrypt/scripts/main.py b/letsencrypt/scripts/main.py index 944ae94ef..b92298a9a 100755 --- a/letsencrypt/scripts/main.py +++ b/letsencrypt/scripts/main.py @@ -125,11 +125,11 @@ def main(): # pylint: disable=too-many-branches sys.exit() if config.revoke: - client.revoke(config.acme_server, config) + client.revoke(config) sys.exit() if config.rollback > 0: - client.rollback(config.rollback, config) + client.rollback(config) sys.exit() if not config.eula: @@ -160,7 +160,7 @@ def main(): # pylint: disable=too-many-branches else: privkey = client.Client.Key(config.privkey[0], config.privkey[1]) - acme = client.Client(config.acme_server, privkey, auth, installer, config) + acme = client.Client(config, privkey, auth, installer) # Validate the key and csr client.validate_key_csr(privkey)