From 381e072bb79e5e249238353afd53c0b902982f18 Mon Sep 17 00:00:00 2001 From: Peter Hamilton Date: Mon, 28 Nov 2016 13:09:05 -0500 Subject: [PATCH] Updating the server to allow optional policy_path values This change updates the server, updating how it processes config values and allowing it to handle optional or ommitted policy_path values. This fixes a bug where users could not leave the policy_path config file unset, in addition to a bug that forced users to use '/etc/pykmip/policies' as their policy directory. Fixes #210 --- kmip/services/server/config.py | 22 ++++++++----------- kmip/services/server/engine.py | 8 +++---- kmip/services/server/server.py | 4 ++-- .../tests/unit/services/server/test_config.py | 14 ++---------- .../tests/unit/services/server/test_engine.py | 10 ++++----- 5 files changed, 22 insertions(+), 36 deletions(-) diff --git a/kmip/services/server/config.py b/kmip/services/server/config.py index 4687999..25bab5e 100644 --- a/kmip/services/server/config.py +++ b/kmip/services/server/config.py @@ -41,7 +41,9 @@ class KmipServerConfig(object): 'certificate_path', 'key_path', 'ca_path', - 'auth_suite', + 'auth_suite' + ] + self._optional_settings = [ 'policy_path' ] @@ -61,7 +63,7 @@ class KmipServerConfig(object): ConfigurationError: Raised if the setting is not supported or if the setting value is invalid. """ - if setting not in self._expected_settings: + if setting not in self._expected_settings + self._optional_settings: raise exceptions.ConfigurationError( "Setting '{0}' is not supported.".format(setting) ) @@ -116,11 +118,11 @@ class KmipServerConfig(object): ) settings = [x[0] for x in parser.items('server')] - for setting in settings: - if setting not in self._expected_settings: + for s in settings: + if s not in self._expected_settings + self._optional_settings: raise exceptions.ConfigurationError( "Setting '{0}' is not a supported setting. Please " - "remove it from the configuration file.".format(setting) + "remove it from the configuration file.".format(s) ) for setting in self._expected_settings: if setting not in settings: @@ -231,16 +233,10 @@ class KmipServerConfig(object): self.settings['auth_suite'] = value def _set_policy_path(self, value): - if value is None: + if not value: self.settings['policy_path'] = None elif isinstance(value, six.string_types): - if os.path.exists(value): - self.settings['policy_path'] = value - else: - raise exceptions.ConfigurationError( - "The policy path value, if specified, must be a valid " - "string path to a filesystem directory." - ) + self.settings['policy_path'] = value else: raise exceptions.ConfigurationError( "The policy path, if specified, must be a valid string path " diff --git a/kmip/services/server/engine.py b/kmip/services/server/engine.py index 587d509..27e7341 100644 --- a/kmip/services/server/engine.py +++ b/kmip/services/server/engine.py @@ -134,9 +134,9 @@ class KmipEngine(object): def _load_operation_policies(self, policy_path): if (policy_path is None) or (not os.path.isdir(policy_path)): self._logger.warning( - "The specified operation policy directory ({0}) is not " - "valid. No user-defined policies will be loaded".format( - policy_path + "The specified operation policy directory{0} is not " + "valid. No user-defined policies will be loaded.".format( + " (" + policy_path + ")" if policy_path else '' ) ) return dict() @@ -151,7 +151,7 @@ class KmipEngine(object): file_path = os.path.join(policy_path, filename) if os.path.isfile(file_path): self._logger.info( - "Loading user_defined operation policies " + "Loading user-defined operation policies " "from file: {0}".format(file_path) ) diff --git a/kmip/services/server/server.py b/kmip/services/server/server.py index 32b6ea9..6cf11d3 100644 --- a/kmip/services/server/server.py +++ b/kmip/services/server/server.py @@ -51,7 +51,7 @@ class KmipServer(object): auth_suite=None, config_path='/etc/pykmip/server.conf', log_path='/var/log/pykmip/server.log', - policy_path='/etc/pykmip/policies' + policy_path=None ): """ Create a KmipServer. @@ -95,7 +95,7 @@ class KmipServer(object): '/var/log/pykmip/server.log'. policy_path (string): The path to the filesystem directory containing PyKMIP server operation policy JSON files. - Optional, defaults to '/etc/pykmip/policies'. + Optional, defaults to None. """ self._logger = logging.getLogger('kmip.server') self._setup_logging(log_path) diff --git a/kmip/tests/unit/services/server/test_config.py b/kmip/tests/unit/services/server/test_config.py index 19bfb54..f9121dc 100644 --- a/kmip/tests/unit/services/server/test_config.py +++ b/kmip/tests/unit/services/server/test_config.py @@ -513,20 +513,10 @@ class TestKmipServerConfig(testtools.TestCase): # Test that a ConfigurationError is generated when setting the wrong # value. c._logger.reset_mock() - args = (0, ) + args = (1, ) self.assertRaises( exceptions.ConfigurationError, c._set_policy_path, *args ) - self.assertNotEqual(0, c.settings.get('policy_path')) - - args = ('/test/path/policies', ) - with mock.patch('os.path.exists') as os_mock: - os_mock.return_value = False - self.assertRaises( - exceptions.ConfigurationError, - c._set_policy_path, - *args - ) - self.assertNotEqual(0, c.settings.get('policy_path')) + self.assertNotEqual(1, c.settings.get('policy_path')) diff --git a/kmip/tests/unit/services/server/test_engine.py b/kmip/tests/unit/services/server/test_engine.py index 87956e7..23f3624 100644 --- a/kmip/tests/unit/services/server/test_engine.py +++ b/kmip/tests/unit/services/server/test_engine.py @@ -163,7 +163,7 @@ class TestKmipEngine(testtools.TestCase): ) ) e._logger.info.assert_any_call( - "Loading user_defined operation policies from file: {0}".format( + "Loading user-defined operation policies from file: {0}".format( policy_file.name ) ) @@ -203,7 +203,7 @@ class TestKmipEngine(testtools.TestCase): ) ) e._logger.info.assert_any_call( - "Loading user_defined operation policies from file: {0}".format( + "Loading user-defined operation policies from file: {0}".format( policy_file.name ) ) @@ -239,7 +239,7 @@ class TestKmipEngine(testtools.TestCase): ) ) e._logger.info.assert_any_call( - "Loading user_defined operation policies from file: {0}".format( + "Loading user-defined operation policies from file: {0}".format( policy_file.name ) ) @@ -283,12 +283,12 @@ class TestKmipEngine(testtools.TestCase): ) ) e._logger.info.assert_any_call( - "Loading user_defined operation policies from file: {0}".format( + "Loading user-defined operation policies from file: {0}".format( policy_file_a.name ) ) e._logger.info.assert_any_call( - "Loading user_defined operation policies from file: {0}".format( + "Loading user-defined operation policies from file: {0}".format( policy_file_b.name ) )