diff --git a/devstack/lib/ironic b/devstack/lib/ironic index 1e0eda62d0..fb4c9dff7d 100644 --- a/devstack/lib/ironic +++ b/devstack/lib/ironic @@ -1059,9 +1059,13 @@ function configure_ironic_api { cp -p $IRONIC_DIR/etc/ironic/policy.json $IRONIC_POLICY_JSON } -function configure_auth_for { +# configure_client_for() - is used by configure_ironic_conductor. +# Sets options to instantiate clients for other services +# single argument - config section to fill +function configure_client_for { local service_config_section service_config_section=$1 + # keystoneauth auth plugin options iniset $IRONIC_CONF_FILE $service_config_section auth_type password iniset $IRONIC_CONF_FILE $service_config_section auth_url $KEYSTONE_SERVICE_URI iniset $IRONIC_CONF_FILE $service_config_section username ironic @@ -1069,24 +1073,39 @@ function configure_auth_for { iniset $IRONIC_CONF_FILE $service_config_section project_name $SERVICE_PROJECT_NAME iniset $IRONIC_CONF_FILE $service_config_section user_domain_id default iniset $IRONIC_CONF_FILE $service_config_section project_domain_id default + # keystoneauth session options iniset $IRONIC_CONF_FILE $service_config_section cafile $SSL_BUNDLE_FILE } +# TODO(pas-ha) this function is for transition period only, +# after all clients are moved to use keystoneauth adapters, it will be merged +# into configure_client_for function +function configure_adapter_for { + local service_config_section + service_config_section=$1 + # keystoneauth adapter options + # NOTE(pas-ha) relying on defaults for valid_interfaces being "internal,public" in ironic + iniset $IRONIC_CONF_FILE $service_config_section region_name $REGION_NAME +} + # configure_ironic_conductor() - Is used by configure_ironic(). # Sets conductor specific settings. function configure_ironic_conductor { - # set keystone region for all services - iniset $IRONIC_CONF_FILE keystone region_name $REGION_NAME + # NOTE(pas-ha) service_catalog section is used to discover + # ironic API endpoint from keystone catalog + local client_sections="neutron swift glance inspector cinder service_catalog" + for conf_section in $client_sections; do + configure_client_for $conf_section + done - # set keystone auth plugin options for services - configure_auth_for neutron - configure_auth_for swift - configure_auth_for glance - configure_auth_for inspector - configure_auth_for cinder - # this one is needed for lookup of Ironic API endpoint via Keystone - configure_auth_for service_catalog + # TODO(pas-ha) this block is for transition period only, + # after all clients are moved to use keystoneauth adapters, + # it will be deleted + local sections_with_adapter="service_catalog" + for conf_section in $sections_with_adapter; do + configure_adapter_for $conf_section + done cp $IRONIC_DIR/etc/ironic/rootwrap.conf $IRONIC_ROOTWRAP_CONF cp -r $IRONIC_DIR/etc/ironic/rootwrap.d $IRONIC_CONF_DIR @@ -1237,8 +1256,6 @@ function create_ironic_accounts { get_or_create_service "ironic" "baremetal" "Ironic baremetal provisioning service" get_or_create_endpoint "baremetal" \ "$REGION_NAME" \ - "$IRONIC_SERVICE_PROTOCOL://$IRONIC_HOSTPORT" \ - "$IRONIC_SERVICE_PROTOCOL://$IRONIC_HOSTPORT" \ "$IRONIC_SERVICE_PROTOCOL://$IRONIC_HOSTPORT" # Create ironic service user diff --git a/etc/ironic/ironic.conf.sample b/etc/ironic/ironic.conf.sample index 5bfcff9ed5..49fcda310a 100644 --- a/etc/ironic/ironic.conf.sample +++ b/etc/ironic/ironic.conf.sample @@ -1026,10 +1026,15 @@ # Seconds between conductor heart beats. (integer value) #heartbeat_interval = 10 -# URL of Ironic API service. If not set ironic can get the -# current value from the keystone service catalog. If set, the -# value must start with either http:// or https://. (uri -# value) +# DEPRECATED: URL of Ironic API service. If not set ironic can +# get the current value from the keystone service catalog. If +# set, the value must start with either http:// or https://. +# (uri value) +# This option is deprecated for removal. +# Its value may be silently ignored in the future. +# Reason: Use [service_catalog]endpoint_override option +# instead if required to use a specific ironic api address, +# for example in noauth mode. #api_url = # Maximum time (in seconds) since the last check-in of a @@ -3545,12 +3550,28 @@ # Domain name to scope to (string value) #domain_name = +# Always use this endpoint URL for requests for this client. +# (string value) +#endpoint_override = + # Verify HTTPS connections. (boolean value) #insecure = false # PEM encoded client certificate key file (string value) #keyfile = +# The maximum major version of a given API, intended to be +# used as the upper bound of a range with min_version. +# Mutually exclusive with version. (string value) +#max_version = + +# The minimum major version of a given API, intended to be +# used as the lower bound of a range with max_version. +# Mutually exclusive with version. If min_version is given +# with no max_version it is as if max version is "latest". +# (string value) +#min_version = + # User's password (string value) #password = @@ -3568,6 +3589,18 @@ # Deprecated group/name - [service_catalog]/tenant_name #project_name = +# The default region_name for endpoint URL discovery. (string +# value) +#region_name = + +# The default service_name for endpoint URL discovery. (string +# value) +#service_name = + +# The default service_type for endpoint URL discovery. (string +# value) +#service_type = baremetal + # Tenant ID (string value) #tenant_id = @@ -3593,6 +3626,15 @@ # Deprecated group/name - [service_catalog]/user_name #username = +# List of interfaces, in order of preference, for endpoint +# URL. (list value) +#valid_interfaces = internal,public + +# Minimum Major API version within a given Major API version +# for endpoint URL discovery. Mutually exclusive with +# min_version and max_version (string value) +#version = + [snmp] diff --git a/ironic/common/keystone.py b/ironic/common/keystone.py index 706ac45157..bc97ac35ef 100644 --- a/ironic/common/keystone.py +++ b/ironic/common/keystone.py @@ -20,6 +20,7 @@ from oslo_log import log as logging import six from ironic.common import exception +from ironic.conf import auth as auth_conf from ironic.conf import CONF @@ -86,7 +87,22 @@ def get_auth(group, **auth_kwargs): return auth -# NOTE(pas-ha) Used by neutronclient and resolving ironic API only +@ks_exceptions +def get_adapter(group, **adapter_kwargs): + """Loads adapter from options in a configuration file section. + + The adapter_kwargs will be passed directly to keystoneauth1 Adapter + and will override the values loaded from config. + Consult keystoneauth1 docs for available adapter options. + + :param group: name of the config section to load adapter options from + + """ + return kaloading.load_adapter_from_conf_options(CONF, group, + **adapter_kwargs) + + +# NOTE(pas-ha) Used by neutronclient and glanceclient only # FIXME(pas-ha) remove this while moving to kesytoneauth adapters @ks_exceptions def get_service_url(session, **kwargs): @@ -103,7 +119,5 @@ def get_service_url(session, **kwargs): if 'interface' in kwargs: return session.get_endpoint(**kwargs) - try: - return session.get_endpoint(interface='internal', **kwargs) - except kaexception.EndpointNotFound: - return session.get_endpoint(interface='public', **kwargs) + return session.get_endpoint(interface=auth_conf.DEFAULT_VALID_INTERFACES, + **kwargs) diff --git a/ironic/conf/auth.py b/ironic/conf/auth.py index 08956460b4..35b3f492c4 100644 --- a/ironic/conf/auth.py +++ b/ironic/conf/auth.py @@ -15,13 +15,16 @@ import copy from keystoneauth1 import loading as kaloading +from oslo_config import cfg from oslo_log import log LOG = log.getLogger(__name__) +DEFAULT_VALID_INTERFACES = ['internal', 'public'] -def register_auth_opts(conf, group): + +def register_auth_opts(conf, group, service_type=None): """Register session- and auth-related options Registers only basic auth options shared by all auth plugins. @@ -29,9 +32,14 @@ def register_auth_opts(conf, group): """ kaloading.register_session_conf_options(conf, group) kaloading.register_auth_conf_options(conf, group) + if service_type: + kaloading.register_adapter_conf_options(conf, group) + conf.set_default('valid_interfaces', DEFAULT_VALID_INTERFACES, + group=group) + conf.set_default('service_type', service_type, group=group) -def add_auth_opts(options): +def add_auth_opts(options, service_type=None): """Add auth options to sample config As these are dynamically registered at runtime, @@ -55,5 +63,12 @@ def add_auth_opts(options): plugin = kaloading.get_plugin_loader(name) add_options(opts, kaloading.get_auth_plugin_conf_options(plugin)) add_options(opts, kaloading.get_session_conf_options()) + if service_type: + adapter_opts = kaloading.get_adapter_conf_options( + include_deprecated=False) + # adding defaults for valid interfaces + cfg.set_defaults(adapter_opts, service_type=service_type, + valid_interfaces=DEFAULT_VALID_INTERFACES) + add_options(opts, adapter_opts) opts.sort(key=lambda x: x.name) return opts diff --git a/ironic/conf/conductor.py b/ironic/conf/conductor.py index e8d2d73b20..5cdbf72864 100644 --- a/ironic/conf/conductor.py +++ b/ironic/conf/conductor.py @@ -30,6 +30,11 @@ opts = [ help=_('Seconds between conductor heart beats.')), cfg.URIOpt('api_url', schemes=('http', 'https'), + deprecated_for_removal=True, + deprecated_reason=_("Use [service_catalog]endpoint_override " + "option instead if required to use " + "a specific ironic api address, " + "for example in noauth mode."), help=_('URL of Ironic API service. If not set ironic can ' 'get the current value from the keystone service ' 'catalog. If set, the value must start with either ' diff --git a/ironic/conf/service_catalog.py b/ironic/conf/service_catalog.py index 56b9d0a415..7bc89351cf 100644 --- a/ironic/conf/service_catalog.py +++ b/ironic/conf/service_catalog.py @@ -26,8 +26,9 @@ SERVICE_CATALOG_GROUP = cfg.OptGroup( def register_opts(conf): - auth.register_auth_opts(conf, SERVICE_CATALOG_GROUP.name) + auth.register_auth_opts(conf, SERVICE_CATALOG_GROUP.name, + service_type='baremetal') def list_opts(): - return auth.add_auth_opts([]) + return auth.add_auth_opts([], service_type='baremetal') diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index 81336871d9..cdd52d567d 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/ironic/drivers/modules/deploy_utils.py @@ -77,9 +77,7 @@ _IRONIC_SESSION = None def _get_ironic_session(): global _IRONIC_SESSION if not _IRONIC_SESSION: - auth = keystone.get_auth('service_catalog') - _IRONIC_SESSION = keystone.get_session('service_catalog', - auth=auth) + _IRONIC_SESSION = keystone.get_session('service_catalog') return _IRONIC_SESSION @@ -94,18 +92,28 @@ def get_ironic_api_url(): either from config of from Keystone catalog. """ - ironic_api = CONF.conductor.api_url - if not ironic_api: - try: - ironic_session = _get_ironic_session() - ironic_api = keystone.get_service_url(ironic_session) - except (exception.KeystoneFailure, - exception.CatalogNotFound, - exception.KeystoneUnauthorized) as e: - raise exception.InvalidParameterValue(_( - "Couldn't get the URL of the Ironic API service from the " - "configuration file or keystone catalog. Keystone error: " - "%s") % six.text_type(e)) + adapter_opts = {'session': _get_ironic_session()} + # NOTE(pas-ha) force 'none' auth plugin for noauth mode + if CONF.auth_strategy != 'keystone': + CONF.set_override('auth_type', 'none', group='service_catalog') + adapter_opts['auth'] = keystone.get_auth('service_catalog') + + # TODO(pas-ha) remove in Rocky + # NOTE(pas-ha) if both set, the new options win + if CONF.conductor.api_url and not CONF.service_catalog.endpoint_override: + adapter_opts['endpoint_override'] = CONF.conductor.api_url + if CONF.keystone.region_name and not CONF.service_catalog.region_name: + adapter_opts['region_name'] = CONF.keystone.region_name + adapter = keystone.get_adapter('service_catalog', **adapter_opts) + try: + ironic_api = adapter.get_endpoint() + except (exception.KeystoneFailure, + exception.CatalogNotFound, + exception.KeystoneUnauthorized) as e: + raise exception.InvalidParameterValue(_( + "Couldn't get the URL of the Ironic API service from the " + "configuration file or keystone catalog. Keystone error: " + "%s") % six.text_type(e)) # NOTE: we should strip '/' from the end because it might be used in # hardcoded ramdisk script ironic_api = ironic_api.rstrip('/') diff --git a/ironic/tests/unit/common/test_keystone.py b/ironic/tests/unit/common/test_keystone.py index 1b1a420706..31c85baad0 100644 --- a/ironic/tests/unit/common/test_keystone.py +++ b/ironic/tests/unit/common/test_keystone.py @@ -12,7 +12,6 @@ # License for the specific language governing permissions and limitations # under the License. -from keystoneauth1 import exceptions as ksexception from keystoneauth1 import loading as kaloading import mock from oslo_config import cfg @@ -32,7 +31,8 @@ class KeystoneTestCase(base.TestCase): group='keystone') self.test_group = 'test_group' self.cfg_fixture.conf.register_group(cfg.OptGroup(self.test_group)) - ironic_auth.register_auth_opts(self.cfg_fixture.conf, self.test_group) + ironic_auth.register_auth_opts(self.cfg_fixture.conf, self.test_group, + service_type='vikings') self.config(auth_type='password', group=self.test_group) # NOTE(pas-ha) this is due to auth_plugin options @@ -76,20 +76,19 @@ class KeystoneTestCase(base.TestCase): self.assertEqual('spam', keystone.get_service_url(session, **params)) session.get_endpoint.assert_called_once_with(**params) - def test_get_service_url_internal(self): + def test_get_service_url(self): session = mock.Mock() session.get_endpoint.return_value = 'spam' params = {'ham': 'eggs'} self.assertEqual('spam', keystone.get_service_url(session, **params)) - session.get_endpoint.assert_called_once_with(interface='internal', - **params) + session.get_endpoint.assert_called_once_with( + interface=['internal', 'public'], **params) - def test_get_service_url_internal_fail(self): - session = mock.Mock() - session.get_endpoint.side_effect = [ksexception.EndpointNotFound(), - 'spam'] - params = {'ham': 'eggs'} - self.assertEqual('spam', keystone.get_service_url(session, **params)) - session.get_endpoint.assert_has_calls([ - mock.call(interface='internal', **params), - mock.call(interface='public', **params)]) + def test_get_adapter_from_config(self): + self.config(valid_interfaces=['internal', 'public'], + group=self.test_group) + session = keystone.get_session(self.test_group) + adapter = keystone.get_adapter(self.test_group, session=session, + interface='admin') + self.assertEqual('admin', adapter.interface) + self.assertEqual(session, adapter.session) diff --git a/ironic/tests/unit/drivers/modules/test_deploy_utils.py b/ironic/tests/unit/drivers/modules/test_deploy_utils.py index a8e8d05dd0..b844c92489 100644 --- a/ironic/tests/unit/drivers/modules/test_deploy_utils.py +++ b/ironic/tests/unit/drivers/modules/test_deploy_utils.py @@ -1214,38 +1214,42 @@ class OtherFunctionTestCase(db_base.DbTestCase): mock_clean_up_caches.assert_called_once_with(None, 'master_dir', [('uuid', 'path')]) + @mock.patch('ironic.common.keystone.get_auth') @mock.patch.object(utils, '_get_ironic_session') - @mock.patch('ironic.common.keystone.get_service_url') - def test_get_ironic_api_url_from_config(self, mock_get_url, mock_ks): + def test_get_ironic_api_url_from_config(self, mock_ks, mock_auth): mock_sess = mock.Mock() mock_ks.return_value = mock_sess fake_api_url = 'http://foo/' - mock_get_url.side_effect = exception.KeystoneFailure self.config(api_url=fake_api_url, group='conductor') - url = utils.get_ironic_api_url() # also checking for stripped trailing slash - self.assertEqual(fake_api_url[:-1], url) - self.assertFalse(mock_get_url.called) + self.assertEqual(fake_api_url[:-1], utils.get_ironic_api_url()) + @mock.patch('ironic.common.keystone.get_auth') @mock.patch.object(utils, '_get_ironic_session') - @mock.patch('ironic.common.keystone.get_service_url') - def test_get_ironic_api_url_from_keystone(self, mock_get_url, mock_ks): + @mock.patch('ironic.common.keystone.get_adapter') + def test_get_ironic_api_url_from_keystone(self, mock_ka, mock_ks, + mock_auth): mock_sess = mock.Mock() mock_ks.return_value = mock_sess fake_api_url = 'http://foo/' - mock_get_url.return_value = fake_api_url + mock_ka.return_value.get_endpoint.return_value = fake_api_url + # NOTE(pas-ha) endpoint_override is None by default self.config(api_url=None, group='conductor') url = utils.get_ironic_api_url() # also checking for stripped trailing slash self.assertEqual(fake_api_url[:-1], url) - mock_get_url.assert_called_with(mock_sess) + mock_ka.assert_called_with('service_catalog', session=mock_sess, + auth=mock_auth.return_value) + mock_ka.return_value.get_endpoint.assert_called_once_with() + @mock.patch('ironic.common.keystone.get_auth') @mock.patch.object(utils, '_get_ironic_session') - @mock.patch('ironic.common.keystone.get_service_url') - def test_get_ironic_api_url_fail(self, mock_get_url, mock_ks): + @mock.patch('ironic.common.keystone.get_adapter') + def test_get_ironic_api_url_fail(self, mock_ka, mock_ks, mock_auth): mock_sess = mock.Mock() mock_ks.return_value = mock_sess - mock_get_url.side_effect = exception.KeystoneFailure() + mock_ka.return_value.get_endpoint.side_effect = ( + exception.KeystoneFailure()) self.config(api_url=None, group='conductor') self.assertRaises(exception.InvalidParameterValue, utils.get_ironic_api_url) diff --git a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py index 39d37c9d7f..72a4ecc489 100644 --- a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py +++ b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py @@ -521,7 +521,9 @@ class IscsiDeployMethodsTestCase(db_base.DbTestCase): iscsi_deploy.validate, task) mock_get_url.assert_called_once_with() - def test_validate_invalid_root_device_hints(self): + @mock.patch('ironic.drivers.modules.deploy_utils.get_ironic_api_url') + def test_validate_invalid_root_device_hints(self, mock_get_url): + mock_get_url.return_value = 'http://spam.ham/baremetal' with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: task.node.properties['root_device'] = {'size': 'not-int'} diff --git a/releasenotes/notes/keystoneauth-adapter-opts-ca4f68f568e6cf6f.yaml b/releasenotes/notes/keystoneauth-adapter-opts-ca4f68f568e6cf6f.yaml new file mode 100644 index 0000000000..d2cde3f23c --- /dev/null +++ b/releasenotes/notes/keystoneauth-adapter-opts-ca4f68f568e6cf6f.yaml @@ -0,0 +1,41 @@ +--- +upgrade: + - | + To facilitate automatic discovery of services from the keystone catalog, + the configuration file sections for service clients may include these + configuration options: ``service_type``, ``service_name``, + ``valid_interfaces``, ``region_name`` and other keystoneauth Adaper-related + options. + + These options together must uniquely specify an endpoint for a service + registered in the keystone catalog. + Consult the ``keystoneauth`` library documentation for full list of + available options, their meaning and possible values. + + Default values for ``service_type`` are set by ironic to sane defaults + based on required services and their entries in ``service-types-authority``. + + The ``valid_interfaces`` option defaults to ``['internal', 'public']``. + + The ``region_name`` option defaults to ``None`` and must be explicitly set + for multi-regional setup for endpoint discovery to succeed. + + The configuration file sections where these new options are available are: + + - service_catalog + +features: + - | + Options for service endpoint discovery can now be set per-service in + appropriate service configuration file sections: + + - ``[service_catalog]`` for baremetal API discovery + +deprecations: + - | + Configuration option ``[conductor]api_url`` is deprecated + and will be ignored in the Rocky release. + Instead, use ``[service_catalog]endpoint_override`` configuration option + to set specific ironic API address if automatic discovery of ironic API + endpoint from keystone catalog is not desired. + This new option defaults to ``None`` and must be set explicitly if needed.