Address comments in installer

This commit is contained in:
sydneyli 2018-04-27 14:25:22 -07:00
parent f52f345d3c
commit d744602279
7 changed files with 37 additions and 42 deletions

View file

@ -42,6 +42,7 @@ class Installer(plugins_common.Installer):
def add_parser_arguments(cls, add):
add("ctl", default=constants.CLI_DEFAULTS["ctl"],
help="Path to the 'postfix' control program.")
# This directory points to Postfix's configuration directory.
add("config-dir", default=constants.CLI_DEFAULTS["config_dir"],
help="Path to the directory containing the "
"Postfix main.cf file to modify instead of using the "
@ -52,6 +53,7 @@ class Installer(plugins_common.Installer):
help="Name of the policy file that we should write to in config-dir.")
def _verify_setup(self):
# TODO (sydneyli): do this
pass
def __init__(self, *args, **kwargs):
@ -71,8 +73,12 @@ class Installer(plugins_common.Installer):
self.postfix = None
self.policy_file = None
self._enhance_func = {"starttls-policy": self._enable_policy_list}
# Since we only need to enable policy once for all domains,
# keep track of whether this enhancement was already called.
self._starttls_policy_enabled = False
def _ensure_ca_certificates_exist(self):
# TODO (sydneyli): This might block starttls-everywhere
# TODO (sydneyli): Ensure `ca-certificates` is installed correctly, or that
# /etc/ssl/certs/ even has certificates in it, probably via a sanity check using
# `openssl` command?
@ -124,7 +130,8 @@ class Installer(plugins_common.Installer):
"""
if self._get_version() < constants.MINIMUM_VERSION:
raise errors.NotSupportedError('Postfix version is too old')
version_string = '.'.join([str(n) for n in constants.MINIMUM_VERSION])
raise errors.NotSupportedError('Postfix version must be at least %s', version_string)
def _lock_config_dir(self):
"""Stop two Postfix plugins from modifying the config at once.
@ -216,11 +223,14 @@ class Installer(plugins_common.Installer):
check_override=util.report_master_overrides)
def _enable_policy_list(self, domain, options):
if self._starttls_policy_enabled:
return
self._starttls_policy_enabled = True
# pylint: disable=unused-argument
try:
from starttls_policy import policy
except ImportError:
raise ImportError('STARTTLS Everywhere policy Python module not installed!')
raise errors.PluginError('STARTTLS Everywhere policy Python module not installed!')
if options is None:
policy = policy.Config()
else:
@ -232,10 +242,6 @@ class Installer(plugins_common.Installer):
def enhance(self, domain, enhancement, options=None):
"""Raises an exception for request for unsupported enhancement.
:raises .PluginError: this is always raised as no enhancements
are currently supported
"""
try:
func = self._enhance_func[enhancement]
@ -254,7 +260,7 @@ class Installer(plugins_common.Installer):
:rtype: list
"""
return ['starttls-policy'] # TODO(sydneyli): Call this starttls-policy?
return ['starttls-policy']
def save(self, title=None, temporary=False):
"""Creates backups and writes changes to configuration files.

View file

@ -5,6 +5,8 @@ from certbot import errors
from certbot_postfix import util
# TODO (sydneyli): tox-ify and make sure this runs in Python 3.
class ConfigMain(util.PostfixUtilBase):
"""A parser for Postfix's main.cf file."""
@ -36,28 +38,28 @@ class ConfigMain(util.PostfixUtilBase):
self._master_db[param_name].append((service, value))
def get_default(self, name):
"""Retrieves default value of parameter |name| from postfix parameters.
"""Retrieves default value of parameter `name` from postfix parameters.
:param str name: The name of the parameter to fetch.
:rtype str: The default value of parameter |name|.
:rtype str: The default value of parameter `name`.
"""
out = self._get_output(['-d', name])
_, value = next(_parse_main_output(out), (None, None))
return value
def get(self, name):
"""Retrieves working value of parameter |name| from postfix parameters.
"""Retrieves working value of parameter `name` from postfix parameters.
:param str name: The name of the parameter to fetch.
:rtype str: The value of parameter |name|.
:rtype str: The value of parameter `name`.
"""
if name in self._updated:
return self._updated[name]
return self._db[name]
def get_master_overrides(self, name):
"""Retrieves list of overrides for parameter |name| in postfix's Master config
"""Retrieves list of overrides for parameter `name` in postfix's Master config
file.
:returns: List of tuples (service, value), meaning that parameter |name|
is overridden as |value| for |service|.
:returns: List of tuples (service, value), meaning that parameter `name`
is overridden as `value` for `service`.
:rtype `list` of `tuple` of `str:
"""
if name in self._master_db:
@ -65,33 +67,26 @@ class ConfigMain(util.PostfixUtilBase):
return None
def set(self, name, value, check_override=None):
"""Sets parameter |name| to |value|.
If |name| is overridden by a particular service in `master.cf`, calls
`check_override` on |name|, and the set of overrides.
"""Sets parameter `name` to `value`.
If `name` is overridden by a particular service in `master.cf`, calls
`check_override` on `name`, and the set of overrides.
Note that this function does not flush these parameter values to main.cf;
To do that, use |flush|.
To do that, use `flush`.
:param str name: The name of the parameter to set.
:param str value: The value of the parameter.
"""
if name not in self._db:
return # TODO: error here
return
# Check to see if this parameter is overridden by master.
# TODO: comment the below
overrides = self.get_master_overrides(name)
if check_override is not None and overrides is not None:
check_override(name, overrides)
# We've updated this name before.
if name in self._updated:
if value == self._updated[name]:
return
if value == self._db[name]:
del self._updated[name]
return
# We haven't updated this name before.
else:
# If we're just setting the default value, ignore.
if value != self._db[name]:
self._updated[name] = value
if value != self._db[name]:
self._updated[name] = value
elif name in self._updated:
del self._updated[name]
def flush(self):
"""Flushes all parameter changes made using "self.set" to "main.cf".

View file

@ -11,7 +11,6 @@ from certbot import errors
from certbot.tests import util as certbot_test_util
class InstallerTest(certbot_test_util.ConfigTestCase):
# pylint: disable=too-many-public-methods
def setUp(self):
super(InstallerTest, self).setUp()

View file

@ -8,6 +8,9 @@ import string
import tempfile
import unittest
# TODO (sydneyli): Mock out calls to postconf
# TODO (sydneyli): inherit certbot.tests.util.TempDirTestCase
def _rand_str(n):
"""Returns a random string with length n, for use as a temporary directory."""
return ''.join([random.choice(string.lowercase) for _ in xrange(n)])

View file

@ -29,14 +29,6 @@ class PostfixUtilBase(object):
self._set_base_command(config_dir)
self.config_dir = None
def update_dir(self, config_dir):
"""Updates the directory of the configuration files for Postfix.
:param str config_dir: The path containing the Postfix configuration files.
"""
self.config_dir = config_dir
self._set_base_command(config_dir)
def _set_base_command(self, config_dir):
self._base_command = [self.executable]
if config_dir is not None:

View file

@ -481,7 +481,7 @@ class Client(object):
if enhancement_name in supported:
if config_name == "redirect" and config_value is None:
config_value = enhancements.ask(enhancement_name)
if config_name == "starttls_policy" and len(config_value) > 0:
if config_name == "starttls_policy" and config_value is not None:
option = config_value
if config_value:
self.apply_enhancement(domains, enhancement_name, option)

View file

@ -60,7 +60,7 @@ CLI_DEFAULTS = dict(
hsts=None,
uir=None,
staple=None,
starttls_policy="",
starttls_policy=None,
strict_permissions=False,
pref_challs=[],
validate_hooks=True,