diff --git a/doc/source/admin/drivers/redfish.rst b/doc/source/admin/drivers/redfish.rst index bcfb683214..db61d4ccc5 100644 --- a/doc/source/admin/drivers/redfish.rst +++ b/doc/source/admin/drivers/redfish.rst @@ -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: diff --git a/ironic/conf/redfish.py b/ironic/conf/redfish.py index aa57514140..a49f2e5d1b 100644 --- a/ironic/conf/redfish.py +++ b/ironic/conf/redfish.py @@ -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.')) ] diff --git a/ironic/drivers/modules/redfish/utils.py b/ironic/drivers/modules/redfish/utils.py index 5cc8ff45bd..69be609188 100644 --- a/ironic/drivers/modules/redfish/utils.py +++ b/ironic/drivers/modules/redfish/utils.py @@ -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}) diff --git a/ironic/tests/unit/drivers/modules/redfish/test_utils.py b/ironic/tests/unit/drivers/modules/redfish/test_utils.py index daa78d4538..9614c17405 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_utils.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_utils.py @@ -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 + ) diff --git a/releasenotes/notes/add-redfish-auth-type-5fe78071b528e53b.yaml b/releasenotes/notes/add-redfish-auth-type-5fe78071b528e53b.yaml new file mode 100644 index 0000000000..60cc818577 --- /dev/null +++ b/releasenotes/notes/add-redfish-auth-type-5fe78071b528e53b.yaml @@ -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.