From 2ea2399ec3e4b976beadfbcd1cab78b94382eca3 Mon Sep 17 00:00:00 2001 From: Clenimar Filemon Date: Thu, 31 Mar 2016 14:46:43 -0300 Subject: [PATCH] Support Identity v3 when connecting to Ironic This patch makes Nova: a) support Identity v3 params when creating an Ironiccient by creating a v3Password auth plugin and a Session; b) deprecate auth parameters admin_tenant_name, admin_username admin_password and admin_url; c) remove support to admin_auth_token auth parameter [1]. [1] admin_auth_token was deprecated (317d9d8f13e8a34af189504ae1258d315154cc82) in favour of admin_username and admin_password (which are deprecated now in favour of username and password). More info at Keystone release notes (see Deprecation Notes and Security Issues): http://docs.openstack.org/releasenotes/keystone/mitaka.html#deprecation-notes Change-Id: Id837d26bb21c158de0504627e488c0692aef1e24 Closes-Bug: #1582045 --- nova/conf/ironic.py | 69 ++++++++-------- .../unit/virt/ironic/test_client_wrapper.py | 79 ++++++++++--------- nova/virt/ironic/client_wrapper.py | 67 +++++++++++----- ...-old-auth-parameters-948d70045335b312.yaml | 15 ++++ ...-admin-token-support-1b59ae7739b06bc2.yaml | 6 ++ 5 files changed, 143 insertions(+), 93 deletions(-) create mode 100644 releasenotes/notes/deprecate-old-auth-parameters-948d70045335b312.yaml create mode 100644 releasenotes/notes/remove-auth-admin-token-support-1b59ae7739b06bc2.yaml diff --git a/nova/conf/ironic.py b/nova/conf/ironic.py index 05b92cca8c01..5c4c0a14af8b 100644 --- a/nova/conf/ironic.py +++ b/nova/conf/ironic.py @@ -13,6 +13,7 @@ # License for the specific language governing permissions and limitations # under the License. +from keystoneauth1 import loading as ks_loading from oslo_config import cfg ironic_group = cfg.OptGroup( @@ -21,56 +22,48 @@ ironic_group = cfg.OptGroup( help=""" Configuration options for Ironic driver (Bare Metal). If using the Ironic driver following options must be set: -* admin_url -* admin_tenant_name -* admin_username -* admin_password -* api_endpoint +* auth_type +* auth_url +* project_name +* username +* password +* project_domain_id or project_domain_name +* user_domain_id or user_domain_name + +Please note that if you are using Identity v2 API (deprecated), +you don't need to provide domain information, since domains are +a v3 concept. """) +# FIXME(clenimar): The following deprecated auth options are kept for backwards +# compatibility. Please remove them as soon as we drop its support: +# `admin_username`, `admin_password`, `admin_url` and `admin_tenant_name`. ironic_options = [ cfg.StrOpt( - # TODO(raj_singh): Get this value from keystone service catalog 'api_endpoint', sample_default='http://ironic.example.org:6385/', - help='URL for the Ironic API endpoint'), + help='URL override for the Ironic API endpoint.'), cfg.StrOpt( 'admin_username', - help='Ironic keystone admin username'), + deprecated_for_removal=True, + help='Ironic keystone admin name. ' + 'Use ``username`` instead.'), cfg.StrOpt( 'admin_password', secret=True, - help='Ironic keystone admin password'), - cfg.StrOpt( - 'admin_auth_token', - secret=True, deprecated_for_removal=True, - help=""" -Ironic keystone auth token. This option is deprecated and -admin_username, admin_password and admin_tenant_name options -are used for authorization. -"""), + help='Ironic keystone admin password. ' + 'Use ``password`` instead.'), cfg.StrOpt( - # TODO(raj_singh): Change this option admin_url->auth_url to make it - # consistent with other clients (Neutron, Cinder). It requires lot - # of work in Ironic client to make this happen. 'admin_url', - help='Keystone public API endpoint'), - cfg.StrOpt( - 'cafile', - default=None, - help=""" -Path to the PEM encoded Certificate Authority file to be used when verifying -HTTPs connections with the Ironic driver. By default this option is not used. - -Possible values: - -* None - Default -* Path to the CA file -"""), + deprecated_for_removal=True, + help='Keystone public API endpoint. ' + 'Use ``auth_url`` instead.'), cfg.StrOpt( 'admin_tenant_name', - help='Ironic keystone tenant name'), + deprecated_for_removal=True, + help='Ironic keystone tenant name. ' + 'Use ``project_name`` instead.'), cfg.IntOpt( 'api_max_retries', # TODO(raj_singh): Change this default to some sensible number @@ -101,7 +94,13 @@ Related options: def register_opts(conf): conf.register_group(ironic_group) conf.register_opts(ironic_options, group=ironic_group) + ks_loading.register_auth_conf_options(conf, group=ironic_group.name) + ks_loading.register_session_conf_options(conf, group=ironic_group.name) def list_opts(): - return {ironic_group: ironic_options} + return {ironic_group: ironic_options + + ks_loading.get_session_conf_options() + + ks_loading.get_auth_common_conf_options() + + ks_loading.get_auth_plugin_conf_options('v3password') + } diff --git a/nova/tests/unit/virt/ironic/test_client_wrapper.py b/nova/tests/unit/virt/ironic/test_client_wrapper.py index 5941a79871df..35364bcff6e9 100644 --- a/nova/tests/unit/virt/ironic/test_client_wrapper.py +++ b/nova/tests/unit/virt/ironic/test_client_wrapper.py @@ -15,15 +15,17 @@ from ironicclient import client as ironic_client from ironicclient import exc as ironic_exception +import keystoneauth1.session import mock from oslo_config import cfg +import nova.conf from nova import exception from nova import test from nova.tests.unit.virt.ironic import utils as ironic_utils from nova.virt.ironic import client_wrapper -CONF = cfg.CONF +CONF = nova.conf.CONF FAKE_CLIENT = ironic_utils.FakeClient() @@ -59,53 +61,52 @@ class IronicClientWrapperTestCase(test.NoDBTestCase): mock_multi_getattr.return_value.assert_called_once_with( 'test', associated=True) + @mock.patch.object(keystoneauth1.session, 'Session') @mock.patch.object(ironic_client, 'get_client') - def test__get_client_no_auth_token(self, mock_ir_cli): - self.flags(admin_auth_token=None, group='ironic') + def test__get_client_session(self, mock_ir_cli, mock_session): + """An Ironicclient is called with a keystoneauth1 Session""" + mock_session.return_value = 'session' ironicclient = client_wrapper.IronicClientWrapper() # dummy call to have _get_client() called ironicclient.call("node.list") - expected = {'os_username': CONF.ironic.admin_username, - 'os_password': CONF.ironic.admin_password, - 'os_auth_url': CONF.ironic.admin_url, - 'os_tenant_name': CONF.ironic.admin_tenant_name, - 'os_service_type': 'baremetal', - 'os_endpoint_type': 'public', - 'ironic_url': CONF.ironic.api_endpoint, - 'max_retries': CONF.ironic.api_max_retries, - 'retry_interval': CONF.ironic.api_retry_interval, - 'os_ironic_api_version': '1.21'} - mock_ir_cli.assert_called_once_with(1, **expected) - - @mock.patch.object(ironic_client, 'get_client') - def test__get_client_with_auth_token(self, mock_ir_cli): - self.flags(admin_auth_token='fake-token', group='ironic') - ironicclient = client_wrapper.IronicClientWrapper() - # dummy call to have _get_client() called - ironicclient.call("node.list") - expected = {'os_auth_token': 'fake-token', - 'ironic_url': CONF.ironic.api_endpoint, - 'max_retries': CONF.ironic.api_max_retries, - 'retry_interval': CONF.ironic.api_retry_interval, - 'os_ironic_api_version': '1.21'} - mock_ir_cli.assert_called_once_with(1, **expected) - - @mock.patch.object(ironic_client, 'get_client') - def test__get_client_cafile(self, mock_ir_cli): - self.flags(admin_auth_token='fake-token', group='ironic') - self.flags(cafile='fake-cafile', group='ironic') - ironicclient = client_wrapper.IronicClientWrapper() - # dummy call to have _get_client() called - ironicclient.call("node.list") - expected = {'os_auth_token': 'fake-token', - 'ironic_url': CONF.ironic.api_endpoint, + expected = {'session': 'session', 'max_retries': CONF.ironic.api_max_retries, 'retry_interval': CONF.ironic.api_retry_interval, 'os_ironic_api_version': '1.21', - 'os_cacert': 'fake-cafile', - 'ca_file': 'fake-cafile'} + 'ironic_url': None} mock_ir_cli.assert_called_once_with(1, **expected) + @mock.patch.object(keystoneauth1.session, 'Session') + @mock.patch.object(keystoneauth1.identity, 'V2Password') + @mock.patch.object(ironic_client, 'get_client') + def test__get_session_legacy(self, mock_ir_cli, mock_plugin, mock_session): + """Create a keystoneauth1 Session with a v2Password auth plugin.""" + mock_plugin.return_value = 'v2password' + ironicclient = client_wrapper.IronicClientWrapper() + # dummy call to have _get_client() called + ironicclient.call("node.list") + expected = {'auth': 'v2password', + 'timeout': CONF.ironic.timeout, + 'cert': CONF.ironic.certfile, + 'verify': True} + mock_session.assert_called_once_with(**expected) + + @mock.patch.object(keystoneauth1.identity, 'V2Password') + @mock.patch.object(keystoneauth1.loading, 'load_auth_from_conf_options') + def test__get_auth_plugin_legacy(self, mock_loader, mock_v2password): + """The plugin loader fails to load an auth plugin from proper + parameters, returning None. Take the legacy path and load a v2Password + plugin from deprecated, legacy auth parameters. + """ + ironicclient = client_wrapper.IronicClientWrapper() + mock_loader.return_value = None + ironicclient._get_auth_plugin() + auth = {'auth_url': CONF.ironic.admin_url, + 'username': CONF.ironic.admin_username, + 'password': CONF.ironic.admin_password, + 'tenant_name': CONF.ironic.admin_tenant_name} + mock_v2password.assert_called_once_with(**auth) + @mock.patch.object(client_wrapper.IronicClientWrapper, '_multi_getattr') @mock.patch.object(client_wrapper.IronicClientWrapper, '_get_client') def test_call_fail_exception(self, mock_get_client, mock_multi_getattr): diff --git a/nova/virt/ironic/client_wrapper.py b/nova/virt/ironic/client_wrapper.py index 7640525f67b8..99b55fc80242 100644 --- a/nova/virt/ironic/client_wrapper.py +++ b/nova/virt/ironic/client_wrapper.py @@ -15,19 +15,24 @@ # License for the specific language governing permissions and limitations # under the License. -from oslo_config import cfg +from keystoneauth1 import identity +from keystoneauth1 import loading as ks_loading from oslo_log import log as logging from oslo_utils import importutils +import nova.conf from nova import exception from nova.i18n import _ +from nova.i18n import _LW LOG = logging.getLogger(__name__) -CONF = cfg.CONF +CONF = nova.conf.CONF ironic = None +IRONIC_GROUP = nova.conf.ironic.ironic_group + # The API version required by the Ironic driver IRONIC_API_VERSION = (1, 21) @@ -57,6 +62,33 @@ class IronicClientWrapper(object): """Tell the wrapper to invalidate the cached ironic-client.""" self._cached_client = None + def _get_auth_plugin(self): + """Load an auth plugin from CONF options.""" + # If an auth plugin name is defined in `auth_type` option of [ironic] + # group, register its options and load it. + auth_plugin = ks_loading.load_auth_from_conf_options(CONF, + IRONIC_GROUP.name) + + # If no plugin name is defined, load a v2Password plugin from the + # deprecated, legacy auth options in [ironic] group. + if auth_plugin is None: + LOG.warning(_LW("Couldn't find adequate authentication options " + "under the [ironic] group of nova.conf. Falling " + "to legacy auth options: admin_username, " + "admin_password, admin_tenant_name and admin_url. " + "Please note that these options are deprecated " + "and won't be supported anymore in a future " + "release.")) + legacy_auth = { + 'username': CONF.ironic.admin_username, + 'password': CONF.ironic.admin_password, + 'tenant_name': CONF.ironic.admin_tenant_name, + 'auth_url': CONF.ironic.admin_url + } + auth_plugin = identity.V2Password(**legacy_auth) + + return auth_plugin + def _get_client(self, retry_on_conflict=True): max_retries = CONF.ironic.api_max_retries if retry_on_conflict else 1 retry_interval = (CONF.ironic.api_retry_interval @@ -67,30 +99,27 @@ class IronicClientWrapper(object): if retry_on_conflict and self._cached_client is not None: return self._cached_client - auth_token = CONF.ironic.admin_auth_token - if auth_token is None: - kwargs = {'os_username': CONF.ironic.admin_username, - 'os_password': CONF.ironic.admin_password, - 'os_auth_url': CONF.ironic.admin_url, - 'os_tenant_name': CONF.ironic.admin_tenant_name, - 'os_service_type': 'baremetal', - 'os_endpoint_type': 'public', - 'ironic_url': CONF.ironic.api_endpoint} - else: - kwargs = {'os_auth_token': auth_token, - 'ironic_url': CONF.ironic.api_endpoint} + auth_plugin = self._get_auth_plugin() - if CONF.ironic.cafile: - kwargs['os_cacert'] = CONF.ironic.cafile - # Set the old option for compat with old clients - kwargs['ca_file'] = CONF.ironic.cafile + sess = ks_loading.load_session_from_conf_options(CONF, + IRONIC_GROUP.name, + auth=auth_plugin) # Retries for Conflict exception + kwargs = {} kwargs['max_retries'] = max_retries kwargs['retry_interval'] = retry_interval kwargs['os_ironic_api_version'] = '%d.%d' % IRONIC_API_VERSION + + # NOTE(clenimar): by default, the endpoint is taken from the service + # catalog. Use `api_endpoint` if you want to override it. + ironic_url = (CONF.ironic.api_endpoint + if CONF.ironic.api_endpoint else None) + try: - cli = ironic.client.get_client(IRONIC_API_VERSION[0], **kwargs) + cli = ironic.client.get_client(IRONIC_API_VERSION[0], + ironic_url=ironic_url, + session=sess, **kwargs) # Cache the client so we don't have to reconstruct and # reauthenticate it every time we need it. if retry_on_conflict: diff --git a/releasenotes/notes/deprecate-old-auth-parameters-948d70045335b312.yaml b/releasenotes/notes/deprecate-old-auth-parameters-948d70045335b312.yaml new file mode 100644 index 000000000000..188c9ad5e78c --- /dev/null +++ b/releasenotes/notes/deprecate-old-auth-parameters-948d70045335b312.yaml @@ -0,0 +1,15 @@ +--- +deprecations: + - The auth parameters `admin_username`, `admin_password`, + `admin_tenant_name` and `admin_url` of the [ironic] config + option group are now deprecated and will be removed in a + future release. Using these parameters will log a warning. + Please use `username`, `password`, `project_id` (or + `project_name`) and `auth_url` instead. If you are using + Keystone v3 API, please note that the name uniqueness for + project and user only holds inside the same hierarchy level, + so you must also specify domain information for user (i.e. + `user_domain_id` or `user_domain_name`) and for project, if + you are using `project_name` (i.e. `project_domain_id` or + `project_domain_name`). + diff --git a/releasenotes/notes/remove-auth-admin-token-support-1b59ae7739b06bc2.yaml b/releasenotes/notes/remove-auth-admin-token-support-1b59ae7739b06bc2.yaml new file mode 100644 index 000000000000..9d64f0562750 --- /dev/null +++ b/releasenotes/notes/remove-auth-admin-token-support-1b59ae7739b06bc2.yaml @@ -0,0 +1,6 @@ +--- +upgrade: + - The deprecated auth parameter `admin_auth_token` + was removed from the [ironic] config option group. + The use of `admin_auth_token` is insecure compared + to the use of a proper username/password.