openstack/ironic
Code Issues Proposed changes

Merge "Add configurable Redfish client authentication"

Browse Source
This commit is contained in:
Zuul 2018-12-08 19:50:59 +00:00 committed by Gerrit Code Review
commit f0809594c7
5 changed files with 150 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
doc/source/admin/drivers/redfish.rst
View File

@ -77,6 +77,14 @@ field:
driver will use for verification. To disable
verifying TLS_, set this to False. This is optional.
- ``redfish_auth_type``: Redfish HTTP client authentication method. Can be
"basic", "session" or "auto".
The "auto" mode first tries "session" and falls back
to "basic" if session authentication is not supported
by the Redfish BMC. Default is set in ironic config
as ``[redfish]auth_type``.
The ``openstack baremetal node create`` command can be used to enroll
a node with the ``redfish`` driver. For example:

9
ironic/conf/redfish.py
View File

@ -36,7 +36,14 @@ opts = [
'BMC connections (obtained through Redfish Session '
'Service). This option caps the maximum number of '
'connections to maintain. The value of `0` disables '
'client connection caching completely.'))
'client connection caching completely.')),
cfg.StrOpt('auth_type',
choices=[('basic', _('Use HTTP basic authentication')),
('session', _('Use HTTP session authentication')),
('auto', _('Try HTTP session authentication first, '
'fall back to basic HTTP authentication'))],
default='auto',
help=_('Redfish HTTP client authentication method.'))
]

50
ironic/drivers/modules/redfish/utils.py
View File

@ -62,7 +62,11 @@ OPTIONAL_PROPERTIES = {
'ignore verifying the SSL certificate. If it\'s '
'a path the driver will use the specified '
'certificate or one of the certificates in the '
'directory. Defaults to True. Optional')
'directory. Defaults to True. Optional'),
'redfish_auth_type': _('Redfish HTTP client authentication method. Can be '
'"basic", "session" or "auto". If not set, the '
'default value is taken from Ironic '
'configuration as ``[redfish]auth_type`` option.')
}
COMMON_PROPERTIES = REQUIRED_PROPERTIES.copy()
@ -142,16 +146,33 @@ def parse_driver_info(node):
'to a file/directory, not "%(value)s"') % {'value': verify_ca,
'node': node.uuid})
auth_type = driver_info.get('redfish_auth_type', CONF.redfish.auth_type)
if auth_type not in ('basic', 'session', 'auto'):
raise exception.InvalidParameterValue(
_('Invalid value "%(value)s" set in '
'driver_info/redfish_auth_type on node %(node)s. '
'The value should be one of "basic", "session" or "auto".') %
{'value': auth_type, 'node': node.uuid})
return {'address': address,
'system_id': system_id,
'username': driver_info.get('redfish_username'),
'password': driver_info.get('redfish_password'),
'verify_ca': verify_ca,
'auth_type': auth_type,
'node_uuid': node.uuid}
class SessionCache(object):
"""Cache of HTTP sessions credentials"""
AUTH_CLASSES = {}
if sushy:
AUTH_CLASSES.update(
basic=sushy.auth.BasicAuth,
session=sushy.auth.SessionAuth,
auto=sushy.auth.SessionOrBasicAuth
)
_sessions = collections.OrderedDict()
def __init__(self, driver_info):
@ -166,11 +187,19 @@ class SessionCache(object):
return self.__class__._sessions[self._session_key]
except KeyError:
auth_type = self._driver_info['auth_type']
auth_class = self.AUTH_CLASSES[auth_type]
authenticator = auth_class(
username=self._driver_info['username'],
password=self._driver_info['password']
)
conn = sushy.Sushy(
self._driver_info['address'],
username=self._driver_info['username'],
password=self._driver_info['password'],
verify=self._driver_info['verify_ca']
verify=self._driver_info['verify_ca'],
auth=authenticator
)
if CONF.redfish.connection_cache_size:
@ -206,7 +235,6 @@ def get_system(node):
:raises: RedfishError if the System is not registered in Redfish
"""
driver_info = parse_driver_info(node)
address = driver_info['address']
system_id = driver_info['system_id']
@retrying.retry(
@ -229,9 +257,12 @@ def get_system(node):
# retrying on them
except sushy.exceptions.ConnectionError as e:
LOG.warning('For node %(node)s, got a connection error from '
'Redfish at address "%(address)s" when fetching '
'System "%(system)s". Error: %(error)s',
{'system': system_id, 'address': address,
'Redfish at address "%(address)s" using auth type '
'"%(auth_type)s" when fetching System "%(system)s". '
'Error: %(error)s',
{'system': system_id,
'address': driver_info['address'],
'auth_type': driver_info['auth_type'],
'node': node.uuid, 'error': e})
raise exception.RedfishConnectionError(node=node.uuid, error=e)
@ -241,4 +272,5 @@ def get_system(node):
with excutils.save_and_reraise_exception():
LOG.error('Failed to connect to Redfish at %(address)s for '
'node %(node)s. Error: %(error)s',
{'address': address, 'node': node.uuid, 'error': e})
{'address': driver_info['address'],
'node': node.uuid, 'error': e})

96
ironic/tests/unit/drivers/modules/redfish/test_utils.py
View File

@ -51,6 +51,7 @@ class RedfishUtilsTestCase(db_base.DbTestCase):
'username': 'username',
'password': 'password',
'verify_ca': True,
'auth_type': 'auto',
'node_uuid': self.node.uuid
}
@ -140,6 +141,20 @@ class RedfishUtilsTestCase(db_base.DbTestCase):
'The value should be a Boolean',
redfish_utils.parse_driver_info, self.node)
def test_parse_driver_info_valid_auth_type(self):
for value in 'basic', 'session', 'auto':
self.node.driver_info['redfish_auth_type'] = value
response = redfish_utils.parse_driver_info(self.node)
self.parsed_driver_info['auth_type'] = value
self.assertEqual(self.parsed_driver_info, response)
def test_parse_driver_info_invalid_auth_type(self):
for value in 'BasiC', 'SESSION', 'Auto':
self.node.driver_info['redfish_auth_type'] = value
self.assertRaisesRegex(exception.InvalidParameterValue,
'The value should be one of ',
redfish_utils.parse_driver_info, self.node)
@mock.patch.object(sushy, 'Sushy', autospec=True)
@mock.patch('ironic.drivers.modules.redfish.utils.'
'SessionCache._sessions', {})
@ -190,17 +205,6 @@ class RedfishUtilsTestCase(db_base.DbTestCase):
mock_sleep.assert_called_with(
redfish_utils.CONF.redfish.connection_retry_interval)
@mock.patch.object(sushy, 'Sushy', autospec=True)
@mock.patch('ironic.drivers.modules.redfish.utils.'
'SessionCache._sessions', {})
def test_auth_auto(self, mock_sushy):
redfish_utils.get_system(self.node)
mock_sushy.assert_called_with(
self.parsed_driver_info['address'],
username=self.parsed_driver_info['username'],
password=self.parsed_driver_info['password'],
verify=True)
@mock.patch.object(sushy, 'Sushy', autospec=True)
@mock.patch('ironic.drivers.modules.redfish.utils.'
'SessionCache._sessions', {})
@ -226,8 +230,21 @@ class RedfishUtilsTestCase(db_base.DbTestCase):
self.assertEqual(2, mock_sushy.call_count)
@mock.patch.object(sushy, 'Sushy', autospec=True)
@mock.patch('ironic.drivers.modules.redfish.utils.'
'SessionCache.AUTH_CLASSES', autospec=True)
@mock.patch('ironic.drivers.modules.redfish.utils.SessionCache._sessions',
collections.OrderedDict())
def test_ensure_basic_session_caching(self, mock_auth, mock_sushy):
self.node.driver_info['redfish_auth_type'] = 'basic'
mock_session_or_basic_auth = mock_auth['auto']
redfish_utils.get_system(self.node)
mock_sushy.assert_called_with(
mock.ANY, verify=mock.ANY,
auth=mock_session_or_basic_auth.return_value,
)
self.assertEqual(len(redfish_utils.SessionCache._sessions), 1)
@mock.patch.object(sushy, 'Sushy', autospec=True)
def test_expire_old_sessions(self, mock_sushy):
cfg.CONF.set_override('connection_cache_size', 10, 'redfish')
for num in range(20):
@ -238,8 +255,8 @@ class RedfishUtilsTestCase(db_base.DbTestCase):
self.assertEqual(len(redfish_utils.SessionCache._sessions), 10)
@mock.patch.object(sushy, 'Sushy', autospec=True)
@mock.patch('ironic.drivers.modules.redfish.utils.SessionCache._sessions',
collections.OrderedDict())
@mock.patch('ironic.drivers.modules.redfish.utils.'
'SessionCache._sessions', {})
def test_disabled_sessions_cache(self, mock_sushy):
cfg.CONF.set_override('connection_cache_size', 0, 'redfish')
for num in range(2):
@ -248,3 +265,56 @@ class RedfishUtilsTestCase(db_base.DbTestCase):
self.assertEqual(mock_sushy.call_count, 2)
self.assertEqual(len(redfish_utils.SessionCache._sessions), 0)
@mock.patch.object(sushy, 'Sushy', autospec=True)
@mock.patch('ironic.drivers.modules.redfish.utils.'
'SessionCache.AUTH_CLASSES', autospec=True)
@mock.patch('ironic.drivers.modules.redfish.utils.'
'SessionCache._sessions', {})
def test_auth_auto(self, mock_auth, mock_sushy):
redfish_utils.get_system(self.node)
mock_session_or_basic_auth = mock_auth['auto']
mock_session_or_basic_auth.assert_called_with(
username=self.parsed_driver_info['username'],
password=self.parsed_driver_info['password']
)
mock_sushy.assert_called_with(
self.parsed_driver_info['address'],
auth=mock_session_or_basic_auth.return_value,
verify=True)
@mock.patch.object(sushy, 'Sushy', autospec=True)
@mock.patch('ironic.drivers.modules.redfish.utils.'
'SessionCache.AUTH_CLASSES', autospec=True)
@mock.patch('ironic.drivers.modules.redfish.utils.'
'SessionCache._sessions', {})
def test_auth_session(self, mock_auth, mock_sushy):
self.node.driver_info['redfish_auth_type'] = 'session'
mock_session_auth = mock_auth['session']
redfish_utils.get_system(self.node)
mock_session_auth.assert_called_with(
username=self.parsed_driver_info['username'],
password=self.parsed_driver_info['password']
)
mock_sushy.assert_called_with(
mock.ANY, verify=mock.ANY,
auth=mock_session_auth.return_value
)
@mock.patch.object(sushy, 'Sushy', autospec=True)
@mock.patch('ironic.drivers.modules.redfish.utils.'
'SessionCache.AUTH_CLASSES', autospec=True)
@mock.patch('ironic.drivers.modules.redfish.utils.'
'SessionCache._sessions', {})
def test_auth_basic(self, mock_auth, mock_sushy):
self.node.driver_info['redfish_auth_type'] = 'basic'
mock_basic_auth = mock_auth['basic']
redfish_utils.get_system(self.node)
mock_basic_auth.assert_called_with(
username=self.parsed_driver_info['username'],
password=self.parsed_driver_info['password']
)
sushy.Sushy.assert_called_with(
mock.ANY, verify=mock.ANY,
auth=mock_basic_auth.return_value
)

10
releasenotes/notes/add-redfish-auth-type-5fe78071b528e53b.yaml Normal file
View File

@ -0,0 +1,10 @@
---
features:
- |
Adds the ``[redfish]auth_type`` ironic configuration option for the
``redfish`` hardware type that is used to choose one of the following
authentication methods:: ``basic``, ``session`` and ``auto``.
The ``auto`` setting first tries ``session`` method and falls back to
``basic`` if session authentication is not supported by the Redfish BMC.
The default is ``auto``. This configuration option can be overridden
on a per-node basis by the ``driver_info/redfish_auth_type`` option.