Address small comments on postconf and util

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.

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

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:

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():

certbot-postfix/setup.py

@@ -7,6 +7,7 @@ version = '0.24.0.dev0'
 install_requires = [
     'certbot>0.23.0',
     'setuptools',
+    'six',
     'zope.interface',
 ]