diff --git a/certbot-postfix/certbot_postfix/postconf.py b/certbot-postfix/certbot_postfix/postconf.py index 5f2f150bd..a28a1b1e7 100644 --- a/certbot-postfix/certbot_postfix/postconf.py +++ b/certbot-postfix/certbot_postfix/postconf.py @@ -5,8 +5,6 @@ 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.""" @@ -18,11 +16,9 @@ class ConfigMain(util.PostfixUtilBase): # 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.""" + self._read_from_conf() # 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 @@ -30,13 +26,16 @@ class ConfigMain(util.PostfixUtilBase): out = self._get_output() for name, value in _parse_main_output(out): self._db[name] = value - out = self._get_output('-P') # get master parameters + out = self._get_output_master() for name, value in _parse_main_output(out): 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)) + def _get_output_master(self): + return self._get_output('-P') + def get_default(self, name): """Retrieves default value of parameter `name` from postfix parameters. :param str name: The name of the parameter to fetch. @@ -77,15 +76,18 @@ class ConfigMain(util.PostfixUtilBase): :param str value: The value of the parameter. """ if name not in self._db: - return + raise KeyError("Parameter name %s is not a valid Postfix parameter name.", name) # 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) if value != self._db[name]: + # _db contains the "original" state of parameters. We only care about + # writes if they cause a delta from the original state. self._updated[name] = value elif name in self._updated: + # If this write reverts a previously updated parameter back to the + # original DB's state, we don't have to keep track of it in _updated. del self._updated[name] def flush(self): diff --git a/certbot-postfix/certbot_postfix/tests/postconf_test.py b/certbot-postfix/certbot_postfix/tests/postconf_test.py index e60352e9b..05a3e21f5 100644 --- a/certbot-postfix/certbot_postfix/tests/postconf_test.py +++ b/certbot-postfix/certbot_postfix/tests/postconf_test.py @@ -1,5 +1,6 @@ """Tests for certbot_postfix.postconf.""" +import mock import os import pkg_resources import shutil @@ -7,43 +8,68 @@ import unittest from certbot.tests import util as test_util -# TODO (sydneyli): Mock out calls to postconf - -class PostConfTest(test_util.TempDirTestCase): - """Tests for certbot_postfix.util.PostfixUtilBase.""" +class PostConfTest(unittest.TestCase): + """Tests for certbot_postfix.util.PostConf.""" def setUp(self): from certbot_postfix.postconf import ConfigMain super(PostConfTest, self).setUp() - _config_file = pkg_resources.resource_filename("certbot_postfix.tests", - os.path.join("testdata", "small.cf")) - self.config_path = os.path.join(self.tempdir, 'main.cf') - shutil.copyfile(_config_file, self.config_path) - self.config = ConfigMain('postconf', self.tempdir) + with mock.patch('certbot_postfix.util.PostfixUtilBase._get_output') as mock_call: + with mock.patch('certbot_postfix.postconf.ConfigMain._get_output_master') as mock_master_call: + with mock.patch('certbot_postfix.postconf.util.verify_exe_exists') as verify_exe: + verify_exe.return_value = True + mock_call.return_value = ('default_parameter = value\n' + 'extra_param =\n' + 'overridden_by_master = default\n') + mock_master_call.return_value = ( + 'service/type/overridden_by_master = master_value\n' + 'service2/type/overridden_by_master = master_value2' + ) + self.config = ConfigMain('postconf', '') - def test_read_defalut(self): - self.assertEqual(self.config.get_default('smtpd_sasl_auth_enable'), 'no') + @mock.patch('certbot_postfix.util.PostfixUtilBase._get_output') + def test_read_defalut(self, mock_get_output): + mock_get_output.return_value = 'param = default_value' + self.assertEqual(self.config.get_default('param'), 'default_value') - def test_read_write(self): - self.config.set('inet_interfaces', '127.0.0.1') + @mock.patch('certbot_postfix.util.PostfixUtilBase._call') + def test_set(self, mock_call): + self.config.set('extra_param', 'other_value') self.config.flush() - with open(self.config_path) as f: - self.assertTrue('inet_interfaces = 127.0.0.1\n' in f.readlines()) + mock_call.assert_called_with(['-e', 'extra_param=other_value']) - def test_write_revert(self): - self.config.set('postscreen_forbidden_commands', 'dummy_value') + def test_set_bad_param_name(self): + self.assertRaises(KeyError, self.config.set, 'nonexistent_param', 'some_value') + + @mock.patch('certbot_postfix.util.PostfixUtilBase._call') + def test_write_revert(self, mock_call): + self.config.set('default_parameter', 'fake_news') # revert config set - self.config.set('postscreen_forbidden_commands', '$smtpd_forbidden_commands') + self.config.set('default_parameter', 'value') self.config.flush() - with open(self.config_path) as f: - self.assertTrue(not any('postscreen_forbidden_commands' in line \ - for line in f.readlines())) + mock_call.assert_not_called() - def test_write_default(self): - self.config.set('postscreen_forbidden_commands', '$smtpd_forbidden_commands') + @mock.patch('certbot_postfix.util.PostfixUtilBase._call') + def test_write_default(self, mock_call): + self.config.set('default_parameter', 'value') self.config.flush() - with open(self.config_path) as f: - self.assertTrue(not any('postscreen_forbidden_commands' in line \ - for line in f.readlines())) + mock_call.assert_not_called() + + def test_master_overrides(self): + self.assertEqual(self.config.get_master_overrides('overridden_by_master'), + [('service/type', 'master_value'), + ('service2/type', 'master_value2')]) + + def test_set_check_override(self): + expected_overrides = [ + ('service/type', 'master_value'), + ('service2/type', 'master_value2')] + def _check_overrides(name, overrides): + self.assertEqual('overridden_by_master', name) + self.assertEqual(expected_overrides, overrides) + self.config.set('overridden_by_master', 'new_value', check_override=_check_overrides) + self.assertEqual(self.config.get_master_overrides('overridden_by_master'), + [('service/type', 'master_value'), + ('service2/type', 'master_value2')]) if __name__ == '__main__': # pragma: no cover unittest.main()