openstack/nova
Code Issues Proposed changes

Support Identity v3 when connecting to Ironic

Browse Source 
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
(317d9d8f13) 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
This commit is contained in:
Clenimar Filemon 2016-03-31 14:46:43 -03:00
parent c37af5653c
commit 2ea2399ec3
5 changed files with 143 additions and 93 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

69
nova/conf/ironic.py
View File

@ -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')
}

79
nova/tests/unit/virt/ironic/test_client_wrapper.py
View File

@ -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):

67
nova/virt/ironic/client_wrapper.py
View File

@ -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:

15
releasenotes/notes/deprecate-old-auth-parameters-948d70045335b312.yaml Normal file
View File

@ -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`).

6
releasenotes/notes/remove-auth-admin-token-support-1b59ae7739b06bc2.yaml Normal file
View File

@ -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.