From f52f345d3ce8460ff70e1b0d8c176257bb203842 Mon Sep 17 00:00:00 2001 From: sydneyli Date: Fri, 27 Apr 2018 12:18:35 -0700 Subject: [PATCH] Address small comments on postconf and util --- certbot-postfix/certbot_postfix/constants.py | 2 +- certbot-postfix/certbot_postfix/installer.py | 1 - certbot-postfix/certbot_postfix/postconf.py | 41 ++++---------------- certbot-postfix/certbot_postfix/util.py | 9 +++-- certbot-postfix/setup.py | 1 + 5 files changed, 16 insertions(+), 38 deletions(-) diff --git a/certbot-postfix/certbot_postfix/constants.py b/certbot-postfix/certbot_postfix/constants.py index 9cc500249..72454dbdf 100644 --- a/certbot-postfix/certbot_postfix/constants.py +++ b/certbot-postfix/certbot_postfix/constants.py @@ -4,7 +4,7 @@ POLICY_FILENAME = "starttls_everywhere_policy" CA_CERTS_PATH = "/etc/ssl/certs/" -MINIMUM_VERSION = (2, 6,) +MINIMUM_VERSION = (2, 11,) # If the value of a default VAR is a tuple, then the values which # come LATER in the tuple are more strict/more secure. diff --git a/certbot-postfix/certbot_postfix/installer.py b/certbot-postfix/certbot_postfix/installer.py index 808dfcd50..be80b88af 100644 --- a/certbot-postfix/certbot_postfix/installer.py +++ b/certbot-postfix/certbot_postfix/installer.py @@ -54,7 +54,6 @@ class Installer(plugins_common.Installer): def _verify_setup(self): pass - # TODO (sydli): fix version fetching code def __init__(self, *args, **kwargs): super(Installer, self).__init__(*args, **kwargs) # Verify that all directories and files exist with proper permissions diff --git a/certbot-postfix/certbot_postfix/postconf.py b/certbot-postfix/certbot_postfix/postconf.py index 10f689187..d2af43174 100644 --- a/certbot-postfix/certbot_postfix/postconf.py +++ b/certbot-postfix/certbot_postfix/postconf.py @@ -1,5 +1,6 @@ """Classes that wrap the postconf command line utility. """ +import six from certbot import errors from certbot_postfix import util @@ -7,33 +8,29 @@ from certbot_postfix import util class ConfigMain(util.PostfixUtilBase): """A parser for Postfix's main.cf file.""" - _modifiers = None - _db = None - _updated = {} - """An iterable containing additional CLI flags for postconf.""" def __init__(self, executable, config_dir=None): - util.PostfixUtilBase.__init__(self, executable, config_dir) + super(ConfigMain, self).__init__(executable, config_dir) self._db = {} # List of current master.cf overrides from Postfix config. Dictionary # of parameter name => list of tuples (service name, paramter value) # Note: We should never modify master without explicit permission. self._master_db = {} self._read_from_conf() + self._updated = {} + """An iterable containing additional CLI flags for postconf.""" + # TODO (sydneyli): Document the above fields in future documentation commit. + # TODO (sydneyli): Test master.cf functionality in future test commit. def _read_from_conf(self): """Reads initial parameter state from main.cf """ out = self._get_output() for name, value in _parse_main_output(out): - if not value: - value = "" self._db[name] = value out = self._get_output('-P') # get master parameters for name, value in _parse_main_output(out): - service, param_name = name.rsplit("/") - if not value: - value = "" + service, param_name = name.rsplit("/", 1) if param_name not in self._master_db: self._master_db[param_name] = [] self._master_db[param_name].append((service, value)) @@ -103,9 +100,8 @@ class ConfigMain(util.PostfixUtilBase): if len(self._updated) == 0: return args = ['-e'] - for name, value in self._updated.iteritems(): + for name, value in six.iteritems(self._updated): args.append('{0}={1}'.format(name, value)) - #TODO (sydli) bugfix: Reset _updated after flushing :) try: self._get_output(args) except: @@ -114,27 +110,6 @@ class ConfigMain(util.PostfixUtilBase): self._db[name] = value self._updated = {} - def _call(self, extra_args=None): - """Runs Postconf and returns the result. - - If self._modifiers is set, it is provided on the command line to - postconf before any values in extra_args. - - :param list extra_args: additional arguments for the command - - :returns: data written to stdout and stderr - :rtype: `tuple` of `str` - - :raises subprocess.CalledProcessError: if the command fails - - """ - all_extra_args = [] - for args_list in (self._modifiers, extra_args,): - if args_list is not None: - all_extra_args.extend(args_list) - - return super(ConfigMain, self)._call(all_extra_args) - def _parse_main_output(output): """Parses the raw output from Postconf about main.cf. Expects the output to look like: diff --git a/certbot-postfix/certbot_postfix/util.py b/certbot-postfix/certbot_postfix/util.py index 146bf170f..0eb794ff1 100644 --- a/certbot-postfix/certbot_postfix/util.py +++ b/certbot-postfix/certbot_postfix/util.py @@ -78,7 +78,7 @@ class PostfixUtil(PostfixUtilBase): """ def __init__(self, config_dir=None): - PostfixUtilBase.__init__(self, COMMAND, config_dir) + super(PostfixUtil, self).__init__(COMMAND, config_dir) def test(self): """Make sure the configuration is valid. @@ -88,7 +88,8 @@ class PostfixUtil(PostfixUtilBase): try: self._call(["check"]) except subprocess.CalledProcessError as e: - print e + logger.debug("Could not check postfix configuration:\n%s", + e) raise errors.MisconfigurationError( "Postfix failed internal configuration check.") @@ -248,8 +249,10 @@ def _get_formatted_policy_for_domain(address_domain, tls_policy): return entry def write_domainwise_tls_policies(policy, policy_file): - """Writes domainwise tls policies to self.policy_file in a format that Postfix + """Writes domainwise tls policies to policy_file in a format that Postfix can parse. + :param policy: A TLSPolicy object that wraps the STARTTLS Policy List. + :param str policy_file: The filepath to the Postfix tls_policy file that should be written. """ policy_lines = [] for address_domain, tls_policy in policy.policies_iter(): diff --git a/certbot-postfix/setup.py b/certbot-postfix/setup.py index 4075cf023..5377200f5 100644 --- a/certbot-postfix/setup.py +++ b/certbot-postfix/setup.py @@ -7,6 +7,7 @@ version = '0.24.0.dev0' install_requires = [ 'certbot>0.23.0', 'setuptools', + 'six', 'zope.interface', ]