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
This commit is contained in:
Peter Hamilton 2016-11-28 13:09:05 -05:00
parent 023bb8fd04
commit 381e072bb7
5 changed files with 22 additions and 36 deletions

View File

@ -41,7 +41,9 @@ class KmipServerConfig(object):
'certificate_path', 'certificate_path',
'key_path', 'key_path',
'ca_path', 'ca_path',
'auth_suite', 'auth_suite'
]
self._optional_settings = [
'policy_path' 'policy_path'
] ]
@ -61,7 +63,7 @@ class KmipServerConfig(object):
ConfigurationError: Raised if the setting is not supported or if ConfigurationError: Raised if the setting is not supported or if
the setting value is invalid. 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( raise exceptions.ConfigurationError(
"Setting '{0}' is not supported.".format(setting) "Setting '{0}' is not supported.".format(setting)
) )
@ -116,11 +118,11 @@ class KmipServerConfig(object):
) )
settings = [x[0] for x in parser.items('server')] settings = [x[0] for x in parser.items('server')]
for setting in settings: for s in settings:
if setting not in self._expected_settings: if s not in self._expected_settings + self._optional_settings:
raise exceptions.ConfigurationError( raise exceptions.ConfigurationError(
"Setting '{0}' is not a supported setting. Please " "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: for setting in self._expected_settings:
if setting not in settings: if setting not in settings:
@ -231,16 +233,10 @@ class KmipServerConfig(object):
self.settings['auth_suite'] = value self.settings['auth_suite'] = value
def _set_policy_path(self, value): def _set_policy_path(self, value):
if value is None: if not value:
self.settings['policy_path'] = None self.settings['policy_path'] = None
elif isinstance(value, six.string_types): elif isinstance(value, six.string_types):
if os.path.exists(value): self.settings['policy_path'] = 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."
)
else: else:
raise exceptions.ConfigurationError( raise exceptions.ConfigurationError(
"The policy path, if specified, must be a valid string path " "The policy path, if specified, must be a valid string path "

View File

@ -134,9 +134,9 @@ class KmipEngine(object):
def _load_operation_policies(self, policy_path): def _load_operation_policies(self, policy_path):
if (policy_path is None) or (not os.path.isdir(policy_path)): if (policy_path is None) or (not os.path.isdir(policy_path)):
self._logger.warning( self._logger.warning(
"The specified operation policy directory ({0}) is not " "The specified operation policy directory{0} is not "
"valid. No user-defined policies will be loaded".format( "valid. No user-defined policies will be loaded.".format(
policy_path " (" + policy_path + ")" if policy_path else ''
) )
) )
return dict() return dict()
@ -151,7 +151,7 @@ class KmipEngine(object):
file_path = os.path.join(policy_path, filename) file_path = os.path.join(policy_path, filename)
if os.path.isfile(file_path): if os.path.isfile(file_path):
self._logger.info( self._logger.info(
"Loading user_defined operation policies " "Loading user-defined operation policies "
"from file: {0}".format(file_path) "from file: {0}".format(file_path)
) )

View File

@ -51,7 +51,7 @@ class KmipServer(object):
auth_suite=None, auth_suite=None,
config_path='/etc/pykmip/server.conf', config_path='/etc/pykmip/server.conf',
log_path='/var/log/pykmip/server.log', log_path='/var/log/pykmip/server.log',
policy_path='/etc/pykmip/policies' policy_path=None
): ):
""" """
Create a KmipServer. Create a KmipServer.
@ -95,7 +95,7 @@ class KmipServer(object):
'/var/log/pykmip/server.log'. '/var/log/pykmip/server.log'.
policy_path (string): The path to the filesystem directory policy_path (string): The path to the filesystem directory
containing PyKMIP server operation policy JSON files. containing PyKMIP server operation policy JSON files.
Optional, defaults to '/etc/pykmip/policies'. Optional, defaults to None.
""" """
self._logger = logging.getLogger('kmip.server') self._logger = logging.getLogger('kmip.server')
self._setup_logging(log_path) self._setup_logging(log_path)

View File

@ -513,20 +513,10 @@ class TestKmipServerConfig(testtools.TestCase):
# Test that a ConfigurationError is generated when setting the wrong # Test that a ConfigurationError is generated when setting the wrong
# value. # value.
c._logger.reset_mock() c._logger.reset_mock()
args = (0, ) args = (1, )
self.assertRaises( self.assertRaises(
exceptions.ConfigurationError, exceptions.ConfigurationError,
c._set_policy_path, c._set_policy_path,
*args *args
) )
self.assertNotEqual(0, c.settings.get('policy_path')) self.assertNotEqual(1, 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'))

View File

@ -163,7 +163,7 @@ class TestKmipEngine(testtools.TestCase):
) )
) )
e._logger.info.assert_any_call( 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 policy_file.name
) )
) )
@ -203,7 +203,7 @@ class TestKmipEngine(testtools.TestCase):
) )
) )
e._logger.info.assert_any_call( 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 policy_file.name
) )
) )
@ -239,7 +239,7 @@ class TestKmipEngine(testtools.TestCase):
) )
) )
e._logger.info.assert_any_call( 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 policy_file.name
) )
) )
@ -283,12 +283,12 @@ class TestKmipEngine(testtools.TestCase):
) )
) )
e._logger.info.assert_any_call( 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 policy_file_a.name
) )
) )
e._logger.info.assert_any_call( 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 policy_file_b.name
) )
) )