diff --git a/ironicclient/client.py b/ironicclient/client.py index 0c4b11159..b785fafa3 100644 --- a/ironicclient/client.py +++ b/ironicclient/client.py @@ -10,146 +10,120 @@ # License for the specific language governing permissions and limitations # under the License. +import logging + from keystoneauth1 import loading as kaloading from oslo_utils import importutils from ironicclient.common.i18n import _ from ironicclient import exc +LOG = logging.getLogger(__name__) -def get_client(api_version, os_auth_token=None, ironic_url=None, - os_username=None, os_password=None, os_auth_url=None, - os_project_id=None, os_project_name=None, os_tenant_id=None, - os_tenant_name=None, os_region_name=None, - os_user_domain_id=None, os_user_domain_name=None, - os_project_domain_id=None, os_project_domain_name=None, - os_service_type=None, os_endpoint_type=None, - insecure=None, timeout=None, os_cacert=None, ca_file=None, - os_cert=None, cert_file=None, os_key=None, key_file=None, - os_ironic_api_version=None, max_retries=None, - retry_interval=None, session=None, **ignored_kwargs): + +# TODO(vdrok): remove in Stein +def convert_keystoneauth_opts(kwargs): + old_to_new_names = { + ('os_auth_token',): 'token', + ('os_username',): 'username', + ('os_password',): 'password', + ('os_auth_url',): 'auth_url', + ('os_project_id',): 'project_id', + ('os_project_name',): 'project_name', + ('os_tenant_id',): 'tenant_id', + ('os_tenant_name',): 'tenant_name', + ('os_region_name',): 'region_name', + ('os_user_domain_id',): 'user_domain_id', + ('os_user_domain_name',): 'user_domain_name', + ('os_project_domain_id',): 'project_domain_id', + ('os_project_domain_name',): 'project_domain_name', + ('os_service_type',): 'service_type', + ('os_endpoint_type',): 'interface', + ('ironic_url',): 'endpoint', + ('os_cacert', 'ca_file'): 'cafile', + ('os_cert', 'cert_file'): 'certfile', + ('os_key', 'key_file'): 'keyfile' + } + for olds, new in old_to_new_names.items(): + for old in olds: + if kwargs.get(old): + LOG.warning('The argument "%s" passed to get_client is ' + 'deprecated and will be removed in Stein release, ' + 'please use "%s" instead.', old, new) + kwargs.setdefault(new, kwargs[old]) + + +def get_client(api_version, auth_type=None, os_ironic_api_version=None, + max_retries=None, retry_interval=None, **kwargs): """Get an authenticated client, based on the credentials. :param api_version: the API version to use. Valid value: '1'. - :param os_auth_token: pre-existing token to re-use - :param ironic_url: ironic API endpoint - :param os_username: name of a user - :param os_password: user's password - :param os_auth_url: endpoint to authenticate against - :param os_tenant_name: name of a tenant (deprecated in favour of - os_project_name) - :param os_tenant_id: ID of a tenant (deprecated in favour of - os_project_id) - :param os_project_name: name of a project - :param os_project_id: ID of a project - :param os_region_name: name of a keystone region - :param os_user_domain_name: name of a domain the user belongs to - :param os_user_domain_id: ID of a domain the user belongs to - :param os_project_domain_name: name of a domain the project belongs to - :param os_project_domain_id: ID of a domain the project belongs to - :param os_service_type: the type of service to lookup the endpoint for - :param os_endpoint_type: the type (exposure) of the endpoint - :param insecure: allow insecure SSL (no cert verification) - :param timeout: allows customization of the timeout for client HTTP - requests - :param os_cacert: path to cacert file - :param ca_file: path to cacert file, deprecated in favour of os_cacert - :param os_cert: path to cert file - :param cert_file: path to cert file, deprecated in favour of os_cert - :param os_key: path to key file - :param key_file: path to key file, deprecated in favour of os_key - :param os_ironic_api_version: ironic API version to use or a list of - available API versions to attempt to negotiate. + :param auth_type: type of keystoneauth auth plugin loader to use. + :param os_ironic_api_version: ironic API version to use. :param max_retries: Maximum number of retries in case of conflict error :param retry_interval: Amount of time (in seconds) between retries in case - of conflict error - :param session: Keystone session to use - :param ignored_kwargs: all the other params that are passed. Left for - backwards compatibility. They are ignored. + of conflict error. + :param kwargs: all the other params that are passed to keystoneauth. """ # TODO(TheJulia): At some point, we should consider possibly noting # the "latest" flag for os_ironic_api_version to cause the client to # auto-negotiate to the greatest available version, however we do not # have the ability yet for a caller to cap the version, and will hold # off doing so until then. - os_service_type = os_service_type or 'baremetal' - os_endpoint_type = os_endpoint_type or 'publicURL' - project_id = (os_project_id or os_tenant_id) - project_name = (os_project_name or os_tenant_name) - kwargs = { + convert_keystoneauth_opts(kwargs) + if auth_type is None: + if 'endpoint' in kwargs: + if 'token' in kwargs: + auth_type = 'admin_token' + else: + auth_type = 'none' + elif 'token' in kwargs and 'auth_url' in kwargs: + auth_type = 'token' + else: + auth_type = 'password' + session = kwargs.get('session') + if not session: + loader = kaloading.get_plugin_loader(auth_type) + loader_options = loader.get_options() + # option.name looks like 'project-name', while dest will be the actual + # argument name to which the value will be passed to (project_name) + auth_options = [o.dest for o in loader_options] + # Include deprecated names as well + auth_options.extend([d.dest for o in loader_options + for d in o.deprecated]) + auth_kwargs = {k: v for (k, v) in kwargs.items() if k in auth_options} + auth_plugin = loader.load_from_options(**auth_kwargs) + # Let keystoneauth do the necessary parameter conversions + session_loader = kaloading.session.Session() + session_opts = {k: v for (k, v) in kwargs.items() if k in + [o.dest for o in session_loader.get_conf_options()]} + session = session_loader.load_from_options(auth=auth_plugin, + **session_opts) + + endpoint = kwargs.get('endpoint') + if not endpoint: + try: + # endpoint will be used to get hostname + # and port that will be used for API version caching. + endpoint = session.get_endpoint( + service_type=kwargs.get('service_type') or 'baremetal', + interface=kwargs.get('interface') or 'publicURL', + region_name=kwargs.get('region_name') + ) + except Exception as e: + raise exc.AmbiguousAuthSystem( + _('Must provide Keystone credentials or user-defined ' + 'endpoint, error was: %s') % e) + + ironicclient_kwargs = { 'os_ironic_api_version': os_ironic_api_version, 'max_retries': max_retries, 'retry_interval': retry_interval, + 'session': session, + 'endpoint_override': endpoint } - endpoint = ironic_url - cacert = os_cacert or ca_file - cert = os_cert or cert_file - key = os_key or key_file - if os_auth_token and endpoint: - kwargs.update({ - 'token': os_auth_token, - 'insecure': insecure, - 'ca_file': cacert, - 'cert_file': cert, - 'key_file': key, - 'timeout': timeout, - }) - elif os_auth_url: - auth_type = 'password' - auth_kwargs = { - 'auth_url': os_auth_url, - 'project_id': project_id, - 'project_name': project_name, - 'user_domain_id': os_user_domain_id, - 'user_domain_name': os_user_domain_name, - 'project_domain_id': os_project_domain_id, - 'project_domain_name': os_project_domain_name, - } - if os_username and os_password: - auth_kwargs.update({ - 'username': os_username, - 'password': os_password, - }) - elif os_auth_token: - auth_type = 'token' - auth_kwargs.update({ - 'token': os_auth_token, - }) - # Create new session only if it was not passed in - if not session: - loader = kaloading.get_plugin_loader(auth_type) - auth_plugin = loader.load_from_options(**auth_kwargs) - # Let keystoneauth do the necessary parameter conversions - session = kaloading.session.Session().load_from_options( - auth=auth_plugin, insecure=insecure, cacert=cacert, - cert=cert, key=key, timeout=timeout, - ) - exception_msg = _('Must provide Keystone credentials or user-defined ' - 'endpoint and token') - if not endpoint: - if session: - try: - # Pass the endpoint, it will be used to get hostname - # and port that will be used for API version caching. It will - # be also set as endpoint_override. - endpoint = session.get_endpoint( - service_type=os_service_type, - interface=os_endpoint_type, - region_name=os_region_name - ) - except Exception as e: - raise exc.AmbiguousAuthSystem( - _('%(message)s, error was: %(error)s') % - {'message': exception_msg, 'error': e}) - else: - # Neither session, nor valid auth parameters provided - raise exc.AmbiguousAuthSystem(exception_msg) - - # Always pass the session - kwargs['session'] = session - - return Client(api_version, endpoint, **kwargs) + return Client(api_version, **ironicclient_kwargs) def Client(version, *args, **kwargs): diff --git a/ironicclient/common/http.py b/ironicclient/common/http.py index 1d293a202..fbba7c0e4 100644 --- a/ironicclient/common/http.py +++ b/ironicclient/common/http.py @@ -81,11 +81,11 @@ def _extract_error_json(body): return error_json -def get_server(endpoint): - """Extract and return the server & port that we're connecting to.""" - if endpoint is None: +def get_server(url): + """Extract and return the server & port.""" + if url is None: return None, None - parts = urlparse.urlparse(endpoint) + parts = urlparse.urlparse(url) return parts.hostname, str(parts.port) @@ -205,7 +205,10 @@ class VersionNegotiationMixin(object): LOG.debug('Negotiated API version is %s', negotiated_ver) # Cache the negotiated version for this server - host, port = get_server(self.endpoint) + # TODO(vdrok): get rid of self.endpoint attribute in Stein + endpoint_override = (getattr(self, 'endpoint_override', None) or + getattr(self, 'endpoint', None)) + host, port = get_server(endpoint_override) filecache.save_data(host=host, port=port, data=negotiated_ver) return negotiated_ver @@ -266,6 +269,8 @@ def with_retries(func): class HTTPClient(VersionNegotiationMixin): def __init__(self, endpoint, **kwargs): + LOG.warning('HTTPClient class is deprecated and will be removed ' + 'in Stein release, please use SessionClient instead.') self.endpoint = endpoint self.endpoint_trimmed = _trim_endpoint_api_version(endpoint) self.auth_token = kwargs.get('token') @@ -556,13 +561,19 @@ class SessionClient(VersionNegotiationMixin, adapter.LegacyJsonAdapter): api_version_select_state, max_retries, retry_interval, - endpoint, + endpoint=None, **kwargs): self.os_ironic_api_version = os_ironic_api_version self.api_version_select_state = api_version_select_state self.conflict_max_retries = max_retries self.conflict_retry_interval = retry_interval - self.endpoint = endpoint + # TODO(vdrok): remove this conditional in Stein + if endpoint and not kwargs.get('endpoint_override'): + LOG.warning('Passing "endpoint" argument to SessionClient ' + 'constructor is deprecated, such possibility will be ' + 'removed in Stein. Please use "endpoint_override" ' + 'instead.') + self.endpoint = endpoint super(SessionClient, self).__init__(**kwargs) @@ -662,8 +673,7 @@ class SessionClient(VersionNegotiationMixin, adapter.LegacyJsonAdapter): return self._http_request(url, method, **kwargs) -def _construct_http_client(endpoint=None, - session=None, +def _construct_http_client(session=None, token=None, auth_ref=None, os_ironic_api_version=DEFAULT_VER, @@ -679,8 +689,8 @@ def _construct_http_client(endpoint=None, if session: kwargs.setdefault('service_type', 'baremetal') kwargs.setdefault('user_agent', 'python-ironicclient') - kwargs.setdefault('interface', kwargs.pop('endpoint_type', None)) - kwargs.setdefault('endpoint_override', endpoint) + kwargs.setdefault('interface', kwargs.pop('endpoint_type', + 'publicURL')) ignored = {'token': token, 'auth_ref': auth_ref, @@ -702,10 +712,11 @@ def _construct_http_client(endpoint=None, api_version_select_state=api_version_select_state, max_retries=max_retries, retry_interval=retry_interval, - endpoint=endpoint, **kwargs) else: + endpoint = None if kwargs: + endpoint = kwargs.pop('endpoint_override', None) LOG.warning('The following arguments are being ignored when ' 'constructing the client: %s'), ', '.join(kwargs) diff --git a/ironicclient/osc/plugin.py b/ironicclient/osc/plugin.py index 848aa37e8..832578e94 100644 --- a/ironicclient/osc/plugin.py +++ b/ironicclient/osc/plugin.py @@ -75,7 +75,7 @@ def make_client(instance): region_name=instance._region_name, # NOTE(vdrok): This will be set as endpoint_override, and the Client # class will be able to do the version stripping if needed - endpoint=instance.get_endpoint_for_service_type( + endpoint_override=instance.get_endpoint_for_service_type( API_NAME, interface=instance.interface, region_name=instance._region_name ) diff --git a/ironicclient/shell.py b/ironicclient/shell.py index b1a08cb0b..8343216e8 100644 --- a/ironicclient/shell.py +++ b/ironicclient/shell.py @@ -98,59 +98,75 @@ class IronicShell(object): help=_('DEPRECATED! Use --os-cacert.')) parser.add_argument('--os-username', + dest='username', default=cliutils.env('OS_USERNAME'), help=_('Defaults to env[OS_USERNAME]')) parser.add_argument('--os_username', + dest='username', help=argparse.SUPPRESS) parser.add_argument('--os-password', + dest='password', default=cliutils.env('OS_PASSWORD'), help=_('Defaults to env[OS_PASSWORD]')) parser.add_argument('--os_password', + dest='password', help=argparse.SUPPRESS) parser.add_argument('--os-tenant-id', + dest='tenant_id', default=cliutils.env('OS_TENANT_ID'), help=_('Defaults to env[OS_TENANT_ID]')) parser.add_argument('--os_tenant_id', + dest='tenant_id', help=argparse.SUPPRESS) parser.add_argument('--os-tenant-name', + dest='tenant_name', default=cliutils.env('OS_TENANT_NAME'), help=_('Defaults to env[OS_TENANT_NAME]')) parser.add_argument('--os_tenant_name', + dest='tenant_name', help=argparse.SUPPRESS) parser.add_argument('--os-auth-url', + dest='auth_url', default=cliutils.env('OS_AUTH_URL'), help=_('Defaults to env[OS_AUTH_URL]')) parser.add_argument('--os_auth_url', + dest='auth_url', help=argparse.SUPPRESS) parser.add_argument('--os-region-name', + dest='region_name', default=cliutils.env('OS_REGION_NAME'), help=_('Defaults to env[OS_REGION_NAME]')) parser.add_argument('--os_region_name', + dest='region_name', help=argparse.SUPPRESS) parser.add_argument('--os-auth-token', + dest='token', default=cliutils.env('OS_AUTH_TOKEN'), help=_('Defaults to env[OS_AUTH_TOKEN]')) parser.add_argument('--os_auth_token', + dest='token', help=argparse.SUPPRESS) parser.add_argument('--ironic-url', + dest='endpoint', default=cliutils.env('IRONIC_URL'), help=_('Defaults to env[IRONIC_URL]')) parser.add_argument('--ironic_url', + dest='endpoint', help=argparse.SUPPRESS) parser.add_argument('--ironic-api-version', @@ -164,15 +180,17 @@ class IronicShell(object): help=argparse.SUPPRESS) parser.add_argument('--os-service-type', + dest='service_type', default=cliutils.env('OS_SERVICE_TYPE'), help=_('Defaults to env[OS_SERVICE_TYPE] or ' '"baremetal"')) parser.add_argument('--os_service_type', + dest='service_type', help=argparse.SUPPRESS) parser.add_argument('--os-endpoint', - dest='ironic_url', + dest='endpoint', default=cliutils.env('OS_SERVICE_ENDPOINT'), help=_('Specify an endpoint to use instead of ' 'retrieving one from the service catalog ' @@ -180,26 +198,31 @@ class IronicShell(object): 'Defaults to env[OS_SERVICE_ENDPOINT].')) parser.add_argument('--os_endpoint', - dest='ironic_url', + dest='endpoint', help=argparse.SUPPRESS) parser.add_argument('--os-endpoint-type', + dest='interface', default=cliutils.env('OS_ENDPOINT_TYPE'), help=_('Defaults to env[OS_ENDPOINT_TYPE] or ' '"publicURL"')) parser.add_argument('--os_endpoint_type', + dest='interface', help=argparse.SUPPRESS) parser.add_argument('--os-user-domain-id', + dest='user_domain_id', default=cliutils.env('OS_USER_DOMAIN_ID'), help=_('Defaults to env[OS_USER_DOMAIN_ID].')) parser.add_argument('--os-user-domain-name', + dest='user_domain_name', default=cliutils.env('OS_USER_DOMAIN_NAME'), help=_('Defaults to env[OS_USER_DOMAIN_NAME].')) parser.add_argument('--os-project-id', + dest='project_id', default=cliutils.env('OS_PROJECT_ID'), help=_('Another way to specify tenant ID. ' 'This option is mutually exclusive with ' @@ -207,6 +230,7 @@ class IronicShell(object): 'Defaults to env[OS_PROJECT_ID].')) parser.add_argument('--os-project-name', + dest='project_name', default=cliutils.env('OS_PROJECT_NAME'), help=_('Another way to specify tenant name. ' 'This option is mutually exclusive with ' @@ -214,10 +238,12 @@ class IronicShell(object): 'Defaults to env[OS_PROJECT_NAME].')) parser.add_argument('--os-project-domain-id', + dest='project_domain_id', default=cliutils.env('OS_PROJECT_DOMAIN_ID'), help=_('Defaults to env[OS_PROJECT_DOMAIN_ID].')) parser.add_argument('--os-project-domain-name', + dest='project_domain_name', default=cliutils.env('OS_PROJECT_DOMAIN_NAME'), help=_('Defaults to env[OS_PROJECT_DOMAIN_NAME].')) @@ -354,38 +380,39 @@ class IronicShell(object): self.do_bash_completion() return 0 - if not (args.os_auth_token and (args.ironic_url or args.os_auth_url)): - if not args.os_username: + # Assume password auth if it does not seem like none, admin_token or + # token auth + if not args.endpoint and not (args.token and args.auth_url): + if not args.username: raise exc.CommandError(_("You must provide a username via " "either --os-username or via " "env[OS_USERNAME]")) - if not args.os_password: + if not args.password: # No password, If we've got a tty, try prompting for it if hasattr(sys.stdin, 'isatty') and sys.stdin.isatty(): # Check for Ctl-D try: - args.os_password = getpass.getpass( + args.password = getpass.getpass( 'OpenStack Password: ') except EOFError: pass # No password because we didn't have a tty or the # user Ctl-D when prompted. - if not args.os_password: + if not args.password: raise exc.CommandError(_("You must provide a password via " "either --os-password, " "env[OS_PASSWORD], " "or prompted response")) - if not (args.os_tenant_id or args.os_tenant_name or - args.os_project_id or args.os_project_name): + if not (args.tenant_id or args.tenant_name or + args.project_id or args.project_name): raise exc.CommandError( _("You must provide a project name or" " project id via --os-project-name, --os-project-id," - " env[OS_PROJECT_ID] or env[OS_PROJECT_NAME]. You may" - " use os-project and os-tenant interchangeably.")) + " env[OS_PROJECT_ID] or env[OS_PROJECT_NAME].")) - if not args.os_auth_url: + if not args.auth_url: raise exc.CommandError(_("You must provide an auth url via " "either --os-auth-url or via " "env[OS_AUTH_URL]")) @@ -397,17 +424,29 @@ class IronicShell(object): raise exc.CommandError(_("You must provide value >= 1 for " "--retry-interval")) client_args = ( - 'os_auth_token', 'ironic_url', 'os_username', 'os_password', - 'os_auth_url', 'os_project_id', 'os_project_name', 'os_tenant_id', - 'os_tenant_name', 'os_region_name', 'os_user_domain_id', - 'os_user_domain_name', 'os_project_domain_id', - 'os_project_domain_name', 'os_service_type', 'os_endpoint_type', - 'os_cacert', 'os_cert', 'os_key', 'max_retries', 'retry_interval', - 'timeout', 'insecure' + 'token', 'endpoint', 'username', 'password', 'auth_url', + 'project_id', 'project_name', 'tenant_id', 'tenant_name', + 'region_name', 'user_domain_id', 'user_domain_name', + 'project_domain_id', 'project_domain_name', 'service_type', + 'interface', 'max_retries', 'retry_interval', 'timeout', 'insecure' ) kwargs = {} for key in client_args: - kwargs[key] = getattr(args, key) + value = getattr(args, key) + # NOTE(vdrok): check for both None and ''. If the default value + # for option is set using cliutils.env function, default empty + # value is ''. If the default is not set explicitly, it is None. + if value not in (None, ''): + kwargs[key] = value + # NOTE(vdrok): this is to workaround the fact that these options are + # named differently in keystoneauth, depending on whether they are + # provided through CLI or loaded from conf options, here we unify them. + for cli_ssl_opt, conf_ssl_opt in [ + ('os_cacert', 'cafile'), ('os_cert', 'certfile'), + ('os_key', 'keyfile')]: + value = getattr(args, cli_ssl_opt) + if value not in (None, ''): + kwargs[conf_ssl_opt] = value kwargs['os_ironic_api_version'] = os_ironic_api_version client = ironicclient.client.get_client(api_major_version, **kwargs) if options.ironic_api_version in ('1', 'latest'): diff --git a/ironicclient/tests/unit/common/test_http.py b/ironicclient/tests/unit/common/test_http.py index be0fd6adc..8d4adb3d0 100644 --- a/ironicclient/tests/unit/common/test_http.py +++ b/ironicclient/tests/unit/common/test_http.py @@ -327,6 +327,11 @@ class VersionNegotiationMixinTest(utils.BaseTestCase): class HttpClientTest(utils.BaseTestCase): + @mock.patch.object(http.LOG, 'warning', autospec=True) + def test_http_client_deprecation(self, log_mock): + http.HTTPClient('http://localhost') + self.assertIn('deprecated', log_mock.call_args[0][0]) + def test_url_generation_trailing_slash_in_base(self): client = http.HTTPClient('http://localhost/') url = client._make_connection_url('/v1/resources') @@ -594,6 +599,13 @@ class HttpClientTest(utils.BaseTestCase): class SessionClientTest(utils.BaseTestCase): + @mock.patch.object(http.LOG, 'warning', autospec=True) + def test_session_client_endpoint_deprecation(self, log_mock): + http.SessionClient(os_ironic_api_version=1, session=mock.Mock(), + api_version_select_state='user', max_retries=5, + retry_interval=5, endpoint='abc') + self.assertIn('deprecated', log_mock.call_args[0][0]) + def test_server_exception_empty_body(self): error_body = _get_error_body() diff --git a/ironicclient/tests/unit/osc/test_plugin.py b/ironicclient/tests/unit/osc/test_plugin.py index f4cff9952..728c7f83d 100644 --- a/ironicclient/tests/unit/osc/test_plugin.py +++ b/ironicclient/tests/unit/osc/test_plugin.py @@ -34,7 +34,7 @@ class MakeClientTest(testtools.TestCase): allow_api_version_downgrade=False, session=instance.session, region_name=instance._region_name, - endpoint='endpoint') + endpoint_override='endpoint') instance.get_endpoint_for_service_type.assert_called_once_with( 'baremetal', region_name=instance._region_name, interface=instance.interface) @@ -54,7 +54,7 @@ class MakeClientTest(testtools.TestCase): allow_api_version_downgrade=True, session=instance.session, region_name=instance._region_name, - endpoint='endpoint') + endpoint_override='endpoint') instance.get_endpoint_for_service_type.assert_called_once_with( 'baremetal', region_name=instance._region_name, interface=instance.interface) @@ -72,7 +72,7 @@ class MakeClientTest(testtools.TestCase): allow_api_version_downgrade=True, session=instance.session, region_name=instance._region_name, - endpoint='endpoint') + endpoint_override='endpoint') instance.get_endpoint_for_service_type.assert_called_once_with( 'baremetal', region_name=instance._region_name, interface=instance.interface) diff --git a/ironicclient/tests/unit/test_client.py b/ironicclient/tests/unit/test_client.py index 17b51d569..c7ddafb15 100644 --- a/ironicclient/tests/unit/test_client.py +++ b/ironicclient/tests/unit/test_client.py @@ -12,7 +12,9 @@ import mock +from keystoneauth1 import identity from keystoneauth1 import loading as kaloading +from keystoneauth1 import token_endpoint from ironicclient import client as iroclient from ironicclient.common import filecache @@ -24,39 +26,42 @@ from ironicclient.v1 import client as v1 class ClientTest(utils.BaseTestCase): - def test_get_client_with_auth_token_ironic_url(self): - kwargs = { - 'ironic_url': 'http://ironic.example.org:6385/', - 'os_auth_token': 'USER_AUTH_TOKEN', - } - client = iroclient.get_client('1', **kwargs) - - self.assertEqual('USER_AUTH_TOKEN', client.http_client.auth_token) - self.assertEqual('http://ironic.example.org:6385/', - client.http_client.endpoint) - + @mock.patch.object(iroclient.LOG, 'warning', autospec=True) @mock.patch.object(filecache, 'retrieve_data', autospec=True) @mock.patch.object(kaloading.session, 'Session', autospec=True) @mock.patch.object(kaloading, 'get_plugin_loader', autospec=True) def _test_get_client(self, mock_ks_loader, mock_ks_session, - mock_retrieve_data, version=None, - auth='password', **kwargs): + mock_retrieve_data, warn_mock, version=None, + auth='password', warn_mock_call_count=0, **kwargs): session = mock_ks_session.return_value.load_from_options.return_value session.get_endpoint.return_value = 'http://localhost:6385/v1/f14b4123' + + class Opt(object): + def __init__(self, name): + self.dest = name + + session_loader_options = [ + Opt('insecure'), Opt('cafile'), Opt('certfile'), Opt('keyfile'), + Opt('timeout')] + mock_ks_session.return_value.get_conf_options.return_value = ( + session_loader_options) mock_ks_loader.return_value.load_from_options.return_value = 'auth' mock_retrieve_data.return_value = version client = iroclient.get_client('1', **kwargs) + self.assertEqual(warn_mock_call_count, warn_mock.call_count) + iroclient.convert_keystoneauth_opts(kwargs) mock_ks_loader.assert_called_once_with(auth) + session_opts = {k: v for (k, v) in kwargs.items() if k in + [o.dest for o in session_loader_options]} mock_ks_session.return_value.load_from_options.assert_called_once_with( - auth='auth', timeout=kwargs.get('timeout'), - insecure=kwargs.get('insecure'), cert=kwargs.get('cert'), - cacert=kwargs.get('cacert'), key=kwargs.get('key')) - session.get_endpoint.assert_called_once_with( - service_type=kwargs.get('os_service_type') or 'baremetal', - interface=kwargs.get('os_endpoint_type') or 'publicURL', - region_name=kwargs.get('os_region_name')) + auth='auth', **session_opts) + if not {'endpoint', 'ironic_url'}.intersection(kwargs): + session.get_endpoint.assert_called_once_with( + service_type=kwargs.get('service_type') or 'baremetal', + interface=kwargs.get('interface') or 'publicURL', + region_name=kwargs.get('region_name')) if 'os_ironic_api_version' in kwargs: # NOTE(TheJulia): This does not test the negotiation logic # as a request must be triggered in order for any verison @@ -71,6 +76,35 @@ class ClientTest(utils.BaseTestCase): port='6385') self.assertEqual(version or v1.DEFAULT_VER, client.http_client.os_ironic_api_version) + return client + + def test_get_client_only_ironic_url(self): + kwargs = {'ironic_url': 'http://localhost:6385/v1'} + client = self._test_get_client(auth='none', + warn_mock_call_count=1, **kwargs) + self.assertIsInstance(client.http_client, http.SessionClient) + self.assertEqual('http://localhost:6385/v1', + client.http_client.endpoint_override) + + def test_get_client_only_endpoint(self): + kwargs = {'endpoint': 'http://localhost:6385/v1'} + client = self._test_get_client(auth='none', **kwargs) + self.assertIsInstance(client.http_client, http.SessionClient) + self.assertEqual('http://localhost:6385/v1', + client.http_client.endpoint_override) + + def test_get_client_with_auth_token_ironic_url(self): + kwargs = { + 'ironic_url': 'http://localhost:6385/v1', + 'os_auth_token': 'USER_AUTH_TOKEN', + } + + client = self._test_get_client(auth='admin_token', + warn_mock_call_count=2, **kwargs) + + self.assertIsInstance(client.http_client, http.SessionClient) + self.assertEqual('http://localhost:6385/v1', + client.http_client.endpoint_override) def test_get_client_no_auth_token(self): kwargs = { @@ -80,7 +114,7 @@ class ClientTest(utils.BaseTestCase): 'os_auth_url': 'http://localhost:35357/v2.0', 'os_auth_token': '', } - self._test_get_client(**kwargs) + self._test_get_client(warn_mock_call_count=4, **kwargs) def test_get_client_service_and_endpoint_type_defaults(self): kwargs = { @@ -92,7 +126,7 @@ class ClientTest(utils.BaseTestCase): 'os_service_type': '', 'os_endpoint_type': '' } - self._test_get_client(**kwargs) + self._test_get_client(warn_mock_call_count=4, **kwargs) def test_get_client_with_region_no_auth_token(self): kwargs = { @@ -103,20 +137,7 @@ class ClientTest(utils.BaseTestCase): 'os_auth_url': 'http://localhost:35357/v2.0', 'os_auth_token': '', } - self._test_get_client(**kwargs) - - def test_get_client_no_url(self): - kwargs = { - 'os_project_name': 'PROJECT_NAME', - 'os_username': 'USERNAME', - 'os_password': 'PASSWORD', - 'os_auth_url': '', - } - self.assertRaises(exc.AmbiguousAuthSystem, iroclient.get_client, - '1', **kwargs) - # test the alias as well to ensure backwards compatibility - self.assertRaises(exc.AmbigiousAuthSystem, iroclient.get_client, - '1', **kwargs) + self._test_get_client(warn_mock_call_count=5, **kwargs) def test_get_client_incorrect_auth_params(self): kwargs = { @@ -125,37 +146,35 @@ class ClientTest(utils.BaseTestCase): 'os_auth_url': 'http://localhost:35357/v2.0', } self.assertRaises(exc.AmbiguousAuthSystem, iroclient.get_client, - '1', **kwargs) + '1', warn_mock_call_count=3, **kwargs) def test_get_client_with_api_version_latest(self): kwargs = { - 'os_project_name': 'PROJECT_NAME', - 'os_username': 'USERNAME', - 'os_password': 'PASSWORD', - 'os_auth_url': 'http://localhost:35357/v2.0', - 'os_auth_token': '', + 'project_name': 'PROJECT_NAME', + 'username': 'USERNAME', + 'password': 'PASSWORD', + 'auth_url': 'http://localhost:35357/v2.0', 'os_ironic_api_version': "latest", } self._test_get_client(**kwargs) def test_get_client_with_api_version_list(self): kwargs = { - 'os_project_name': 'PROJECT_NAME', - 'os_username': 'USERNAME', - 'os_password': 'PASSWORD', - 'os_auth_url': 'http://localhost:35357/v2.0', - 'os_auth_token': '', + 'project_name': 'PROJECT_NAME', + 'username': 'USERNAME', + 'password': 'PASSWORD', + 'auth_url': 'http://localhost:35357/v2.0', + 'auth_token': '', 'os_ironic_api_version': ['1.1', '1.99'], } self._test_get_client(**kwargs) def test_get_client_with_api_version_numeric(self): kwargs = { - 'os_project_name': 'PROJECT_NAME', - 'os_username': 'USERNAME', - 'os_password': 'PASSWORD', - 'os_auth_url': 'http://localhost:35357/v2.0', - 'os_auth_token': '', + 'project_name': 'PROJECT_NAME', + 'username': 'USERNAME', + 'password': 'PASSWORD', + 'auth_url': 'http://localhost:35357/v2.0', 'os_ironic_api_version': "1.4", } self._test_get_client(**kwargs) @@ -165,26 +184,25 @@ class ClientTest(utils.BaseTestCase): # Make sure we don't coincidentally succeed self.assertNotEqual(v1.DEFAULT_VER, version) kwargs = { - 'os_project_name': 'PROJECT_NAME', - 'os_username': 'USERNAME', - 'os_password': 'PASSWORD', - 'os_auth_url': 'http://localhost:35357/v2.0', - 'os_auth_token': '', + 'project_name': 'PROJECT_NAME', + 'username': 'USERNAME', + 'password': 'PASSWORD', + 'auth_url': 'http://localhost:35357/v2.0', } self._test_get_client(version=version, **kwargs) def test_get_client_with_auth_token(self): kwargs = { - 'os_auth_url': 'http://localhost:35357/v2.0', - 'os_auth_token': 'USER_AUTH_TOKEN', + 'auth_url': 'http://localhost:35357/v2.0', + 'token': 'USER_AUTH_TOKEN', } self._test_get_client(auth='token', **kwargs) def test_get_client_with_region_name_auth_token(self): kwargs = { - 'os_auth_url': 'http://localhost:35357/v2.0', - 'os_region_name': 'REGIONONE', - 'os_auth_token': 'USER_AUTH_TOKEN', + 'auth_url': 'http://localhost:35357/v2.0', + 'region_name': 'REGIONONE', + 'token': 'USER_AUTH_TOKEN', } self._test_get_client(auth='token', **kwargs) @@ -209,46 +227,60 @@ class ClientTest(utils.BaseTestCase): '1', **kwargs) @mock.patch.object(kaloading.session, 'Session', autospec=True) - @mock.patch.object(kaloading, 'get_plugin_loader', autospec=True) def _test_loader_arguments_passed_correctly( - self, mock_ks_loader, mock_ks_session, - passed_kwargs, expected_kwargs): + self, mock_ks_session, passed_kwargs, expected_kwargs, + loader_class): session = mock_ks_session.return_value.load_from_options.return_value session.get_endpoint.return_value = 'http://localhost:6385/v1/f14b4123' - mock_ks_loader.return_value.load_from_options.return_value = 'auth' - iroclient.get_client('1', **passed_kwargs) + with mock.patch.object(loader_class, '__init__', + autospec=True) as init_mock: + init_mock.return_value = None + iroclient.get_client('1', **passed_kwargs) + iroclient.convert_keystoneauth_opts(passed_kwargs) - mock_ks_loader.return_value.load_from_options.assert_called_once_with( - **expected_kwargs) + init_mock.assert_called_once_with(mock.ANY, **expected_kwargs) + session_opts = {k: v for (k, v) in passed_kwargs.items() if k in + ['insecure', 'cacert', 'cert', 'key', 'timeout']} mock_ks_session.return_value.load_from_options.assert_called_once_with( - auth='auth', timeout=passed_kwargs.get('timeout'), - insecure=passed_kwargs.get('insecure'), - cert=passed_kwargs.get('cert'), - cacert=passed_kwargs.get('cacert'), key=passed_kwargs.get('key')) - session.get_endpoint.assert_called_once_with( - service_type=passed_kwargs.get('os_service_type') or 'baremetal', - interface=passed_kwargs.get('os_endpoint_type') or 'publicURL', - region_name=passed_kwargs.get('os_region_name')) + auth=mock.ANY, **session_opts) + if 'ironic_url' not in passed_kwargs: + service_type = passed_kwargs.get('service_type') or 'baremetal' + interface = passed_kwargs.get('interface') or 'publicURL' + session.get_endpoint.assert_called_once_with( + service_type=service_type, interface=interface, + region_name=passed_kwargs.get('region_name')) + + def test_loader_arguments_admin_token(self): + passed_kwargs = { + 'ironic_url': 'http://localhost:6385/v1', + 'os_auth_token': 'USER_AUTH_TOKEN', + } + expected_kwargs = { + 'endpoint': 'http://localhost:6385/v1', + 'token': 'USER_AUTH_TOKEN' + } + self._test_loader_arguments_passed_correctly( + passed_kwargs=passed_kwargs, expected_kwargs=expected_kwargs, + loader_class=token_endpoint.Token + ) def test_loader_arguments_token(self): passed_kwargs = { 'os_auth_url': 'http://localhost:35357/v3', 'os_region_name': 'REGIONONE', 'os_auth_token': 'USER_AUTH_TOKEN', + 'os_project_name': 'admin' } expected_kwargs = { 'auth_url': 'http://localhost:35357/v3', - 'project_id': None, - 'project_name': None, - 'user_domain_id': None, - 'user_domain_name': None, - 'project_domain_id': None, - 'project_domain_name': None, + 'project_name': 'admin', 'token': 'USER_AUTH_TOKEN' } self._test_loader_arguments_passed_correctly( - passed_kwargs=passed_kwargs, expected_kwargs=expected_kwargs) + passed_kwargs=passed_kwargs, expected_kwargs=expected_kwargs, + loader_class=identity.Token + ) def test_loader_arguments_password_tenant_name(self): passed_kwargs = { @@ -262,17 +294,16 @@ class ClientTest(utils.BaseTestCase): } expected_kwargs = { 'auth_url': 'http://localhost:35357/v3', - 'project_id': None, 'project_name': 'PROJECT', 'user_domain_id': 'DEFAULT', - 'user_domain_name': None, 'project_domain_id': 'DEFAULT', - 'project_domain_name': None, 'username': 'user', 'password': '1234' } self._test_loader_arguments_passed_correctly( - passed_kwargs=passed_kwargs, expected_kwargs=expected_kwargs) + passed_kwargs=passed_kwargs, expected_kwargs=expected_kwargs, + loader_class=identity.Password + ) def test_loader_arguments_password_project_id(self): passed_kwargs = { @@ -287,47 +318,35 @@ class ClientTest(utils.BaseTestCase): expected_kwargs = { 'auth_url': 'http://localhost:35357/v3', 'project_id': '1000', - 'project_name': None, - 'user_domain_id': None, 'user_domain_name': 'domain1', - 'project_domain_id': None, 'project_domain_name': 'domain1', 'username': 'user', 'password': '1234' } self._test_loader_arguments_passed_correctly( - passed_kwargs=passed_kwargs, expected_kwargs=expected_kwargs) + passed_kwargs=passed_kwargs, expected_kwargs=expected_kwargs, + loader_class=identity.Password + ) @mock.patch.object(iroclient, 'Client', autospec=True) @mock.patch.object(kaloading.session, 'Session', autospec=True) def test_correct_arguments_passed_to_client_constructor_noauth_mode( self, mock_ks_session, mock_client): + session = mock_ks_session.return_value.load_from_options.return_value kwargs = { 'ironic_url': 'http://ironic.example.org:6385/', 'os_auth_token': 'USER_AUTH_TOKEN', 'os_ironic_api_version': 'latest', - 'insecure': True, - 'max_retries': 10, - 'retry_interval': 10, - 'os_cacert': 'data' } iroclient.get_client('1', **kwargs) mock_client.assert_called_once_with( - '1', 'http://ironic.example.org:6385/', - **{ - 'os_ironic_api_version': 'latest', - 'max_retries': 10, - 'retry_interval': 10, - 'token': 'USER_AUTH_TOKEN', - 'insecure': True, - 'ca_file': 'data', - 'cert_file': None, - 'key_file': None, - 'timeout': None, - 'session': None - } + '1', **{'os_ironic_api_version': 'latest', + 'max_retries': None, + 'retry_interval': None, + 'session': session, + 'endpoint_override': 'http://ironic.example.org:6385/'} ) - self.assertFalse(mock_ks_session.called) + self.assertFalse(session.get_endpoint.called) @mock.patch.object(iroclient, 'Client', autospec=True) @mock.patch.object(kaloading.session, 'Session', autospec=True) @@ -345,13 +364,11 @@ class ClientTest(utils.BaseTestCase): } iroclient.get_client('1', **kwargs) mock_client.assert_called_once_with( - '1', session.get_endpoint.return_value, - **{ - 'os_ironic_api_version': None, - 'max_retries': None, - 'retry_interval': None, - 'session': session, - } + '1', **{'os_ironic_api_version': None, + 'max_retries': None, + 'retry_interval': None, + 'session': session, + 'endpoint_override': session.get_endpoint.return_value} ) @mock.patch.object(iroclient, 'Client', autospec=True) @@ -364,13 +381,11 @@ class ClientTest(utils.BaseTestCase): } iroclient.get_client('1', **kwargs) mock_client.assert_called_once_with( - '1', session.get_endpoint.return_value, - **{ - 'os_ironic_api_version': None, - 'max_retries': None, - 'retry_interval': None, - 'session': session, - } + '1', **{'os_ironic_api_version': None, + 'max_retries': None, + 'retry_interval': None, + 'session': session, + 'endpoint_override': session.get_endpoint.return_value} ) self.assertFalse(mock_ks_session.called) diff --git a/ironicclient/tests/unit/test_shell.py b/ironicclient/tests/unit/test_shell.py index 672c4f29d..74d0084ac 100644 --- a/ironicclient/tests/unit/test_shell.py +++ b/ironicclient/tests/unit/test_shell.py @@ -39,6 +39,13 @@ FAKE_ENV = {'OS_USERNAME': 'username', 'OS_PROJECT_NAME': 'project_name', 'OS_AUTH_URL': V2_URL} +FAKE_ENV_WITH_SSL = FAKE_ENV.copy() +FAKE_ENV_WITH_SSL.update({ + 'OS_CACERT': 'cacert', + 'OS_CERT': 'cert', + 'OS_KEY': 'key', +}) + FAKE_ENV_KEYSTONE_V2 = { 'OS_USERNAME': 'username', 'OS_PASSWORD': 'password', @@ -162,16 +169,10 @@ class ShellTest(utils.BaseTestCase): self.assertRaises(keystone_exc.ConnectFailure, self.shell, 'node-list') expected_kwargs = { - 'ironic_url': '', 'os_auth_url': FAKE_ENV['OS_AUTH_URL'], - 'os_tenant_id': '', 'os_tenant_name': '', - 'os_username': FAKE_ENV['OS_USERNAME'], 'os_user_domain_id': '', - 'os_user_domain_name': '', 'os_password': FAKE_ENV['OS_PASSWORD'], - 'os_auth_token': '', 'os_project_id': '', - 'os_project_name': FAKE_ENV['OS_PROJECT_NAME'], - 'os_project_domain_id': '', - 'os_project_domain_name': '', 'os_region_name': '', - 'os_service_type': '', 'os_endpoint_type': '', 'os_cacert': None, - 'os_cert': None, 'os_key': None, + 'auth_url': FAKE_ENV['OS_AUTH_URL'], + 'username': FAKE_ENV['OS_USERNAME'], + 'password': FAKE_ENV['OS_PASSWORD'], + 'project_name': FAKE_ENV['OS_PROJECT_NAME'], 'max_retries': http.DEFAULT_MAX_RETRIES, 'retry_interval': http.DEFAULT_RETRY_INTERVAL, 'os_ironic_api_version': ironic_shell.LATEST_VERSION, @@ -181,6 +182,32 @@ class ShellTest(utils.BaseTestCase): # Make sure we are actually prompted. mock_getpass.assert_called_with('OpenStack Password: ') + @mock.patch.object(client, 'get_client', autospec=True, + side_effect=keystone_exc.ConnectFailure) + @mock.patch('sys.stdin', side_effect=mock.MagicMock, autospec=True) + @mock.patch('getpass.getpass', return_value='password', autospec=True) + def test_password(self, mock_getpass, mock_stdin, mock_client): + self.make_env(environ_dict=FAKE_ENV_WITH_SSL) + # We will get a ConnectFailure because there is no keystone. + self.assertRaises(keystone_exc.ConnectFailure, + self.shell, 'node-list') + expected_kwargs = { + 'auth_url': FAKE_ENV_WITH_SSL['OS_AUTH_URL'], + 'username': FAKE_ENV_WITH_SSL['OS_USERNAME'], + 'password': FAKE_ENV_WITH_SSL['OS_PASSWORD'], + 'project_name': FAKE_ENV_WITH_SSL['OS_PROJECT_NAME'], + 'cafile': FAKE_ENV_WITH_SSL['OS_CACERT'], + 'certfile': FAKE_ENV_WITH_SSL['OS_CERT'], + 'keyfile': FAKE_ENV_WITH_SSL['OS_KEY'], + 'max_retries': http.DEFAULT_MAX_RETRIES, + 'retry_interval': http.DEFAULT_RETRY_INTERVAL, + 'timeout': 600, + 'os_ironic_api_version': ironic_shell.LATEST_VERSION, + 'insecure': False + } + mock_client.assert_called_once_with(1, **expected_kwargs) + self.assertFalse(mock_getpass.called) + @mock.patch.object(client, 'get_client', autospec=True, side_effect=keystone_exc.ConnectFailure) @mock.patch('getpass.getpass', return_value='password', autospec=True) @@ -190,23 +217,49 @@ class ShellTest(utils.BaseTestCase): self.assertRaises(keystone_exc.ConnectFailure, self.shell, 'node-list') expected_kwargs = { - 'ironic_url': '', - 'os_auth_url': FAKE_ENV_KEYSTONE_V2_TOKEN['OS_AUTH_URL'], - 'os_tenant_id': '', - 'os_tenant_name': '', - 'os_username': '', 'os_user_domain_id': '', - 'os_user_domain_name': '', 'os_password': '', - 'os_auth_token': FAKE_ENV_KEYSTONE_V2_TOKEN['OS_AUTH_TOKEN'], - 'os_project_id': '', - 'os_project_name': FAKE_ENV_KEYSTONE_V2_TOKEN['OS_PROJECT_NAME'], - 'os_project_domain_id': '', 'os_project_domain_name': '', - 'os_region_name': '', 'os_service_type': '', - 'os_endpoint_type': '', 'os_cacert': None, 'os_cert': None, - 'os_key': None, 'max_retries': http.DEFAULT_MAX_RETRIES, + 'auth_url': FAKE_ENV_KEYSTONE_V2_TOKEN['OS_AUTH_URL'], + 'token': FAKE_ENV_KEYSTONE_V2_TOKEN['OS_AUTH_TOKEN'], + 'project_name': FAKE_ENV_KEYSTONE_V2_TOKEN['OS_PROJECT_NAME'], + 'max_retries': http.DEFAULT_MAX_RETRIES, + 'retry_interval': http.DEFAULT_RETRY_INTERVAL, + 'timeout': 600, + 'os_ironic_api_version': ironic_shell.LATEST_VERSION, + 'insecure': False + } + mock_client.assert_called_once_with(1, **expected_kwargs) + self.assertFalse(mock_getpass.called) + + @mock.patch.object(client, 'get_client', autospec=True) + @mock.patch('getpass.getpass', return_value='password', autospec=True) + def test_admin_token_auth(self, mock_getpass, mock_client): + self.make_env(environ_dict={ + 'IRONIC_URL': 'http://192.168.1.1/v1', + 'OS_AUTH_TOKEN': FAKE_ENV_KEYSTONE_V2_TOKEN['OS_AUTH_TOKEN']}) + expected_kwargs = { + 'endpoint': 'http://192.168.1.1/v1', + 'token': FAKE_ENV_KEYSTONE_V2_TOKEN['OS_AUTH_TOKEN'], + 'max_retries': http.DEFAULT_MAX_RETRIES, + 'retry_interval': http.DEFAULT_RETRY_INTERVAL, + 'timeout': 600, + 'os_ironic_api_version': ironic_shell.LATEST_VERSION, + 'insecure': False + } + self.shell('node-list') + mock_client.assert_called_once_with(1, **expected_kwargs) + self.assertFalse(mock_getpass.called) + + @mock.patch.object(client, 'get_client', autospec=True) + @mock.patch('getpass.getpass', return_value='password', autospec=True) + def test_none_auth(self, mock_getpass, mock_client): + self.make_env(environ_dict={'IRONIC_URL': 'http://192.168.1.1/v1'}) + expected_kwargs = { + 'endpoint': 'http://192.168.1.1/v1', + 'max_retries': http.DEFAULT_MAX_RETRIES, 'retry_interval': http.DEFAULT_RETRY_INTERVAL, 'os_ironic_api_version': ironic_shell.LATEST_VERSION, 'timeout': 600, 'insecure': False } + self.shell('node-list') mock_client.assert_called_once_with(1, **expected_kwargs) self.assertFalse(mock_getpass.called) @@ -274,16 +327,10 @@ class ShellTest(utils.BaseTestCase): self.assertRaises(keystone_exc.ConnectFailure, self.shell, 'node-list') expected_kwargs = { - 'ironic_url': '', 'os_auth_url': FAKE_ENV['OS_AUTH_URL'], - 'os_tenant_id': '', 'os_tenant_name': '', - 'os_username': FAKE_ENV['OS_USERNAME'], 'os_user_domain_id': '', - 'os_user_domain_name': '', 'os_password': FAKE_ENV['OS_PASSWORD'], - 'os_auth_token': '', 'os_project_id': '', - 'os_project_name': FAKE_ENV['OS_PROJECT_NAME'], - 'os_project_domain_id': '', - 'os_project_domain_name': '', 'os_region_name': '', - 'os_service_type': '', 'os_endpoint_type': '', 'os_cacert': None, - 'os_cert': None, 'os_key': None, + 'auth_url': FAKE_ENV['OS_AUTH_URL'], + 'username': FAKE_ENV['OS_USERNAME'], + 'password': FAKE_ENV['OS_PASSWORD'], + 'project_name': FAKE_ENV['OS_PROJECT_NAME'], 'max_retries': http.DEFAULT_MAX_RETRIES, 'retry_interval': http.DEFAULT_RETRY_INTERVAL, 'os_ironic_api_version': '1.10', @@ -300,16 +347,10 @@ class ShellTest(utils.BaseTestCase): self.assertRaises(keystone_exc.ConnectFailure, self.shell, 'node-list') expected_kwargs = { - 'ironic_url': '', 'os_auth_url': FAKE_ENV['OS_AUTH_URL'], - 'os_tenant_id': '', 'os_tenant_name': '', - 'os_username': FAKE_ENV['OS_USERNAME'], 'os_user_domain_id': '', - 'os_user_domain_name': '', 'os_password': FAKE_ENV['OS_PASSWORD'], - 'os_auth_token': '', 'os_project_id': '', - 'os_project_name': FAKE_ENV['OS_PROJECT_NAME'], - 'os_project_domain_id': '', - 'os_project_domain_name': '', 'os_region_name': '', - 'os_service_type': '', 'os_endpoint_type': '', 'os_cacert': None, - 'os_cert': None, 'os_key': None, + 'auth_url': FAKE_ENV['OS_AUTH_URL'], + 'username': FAKE_ENV['OS_USERNAME'], + 'password': FAKE_ENV['OS_PASSWORD'], + 'project_name': FAKE_ENV['OS_PROJECT_NAME'], 'max_retries': http.DEFAULT_MAX_RETRIES, 'retry_interval': http.DEFAULT_RETRY_INTERVAL, 'os_ironic_api_version': ironic_shell.LATEST_VERSION, @@ -326,16 +367,10 @@ class ShellTest(utils.BaseTestCase): self.assertRaises(keystone_exc.ConnectFailure, self.shell, '--ironic-api-version 1.11 node-list') expected_kwargs = { - 'ironic_url': '', 'os_auth_url': FAKE_ENV['OS_AUTH_URL'], - 'os_tenant_id': '', 'os_tenant_name': '', - 'os_username': FAKE_ENV['OS_USERNAME'], 'os_user_domain_id': '', - 'os_user_domain_name': '', 'os_password': FAKE_ENV['OS_PASSWORD'], - 'os_auth_token': '', 'os_project_id': '', - 'os_project_name': FAKE_ENV['OS_PROJECT_NAME'], - 'os_project_domain_id': '', - 'os_project_domain_name': '', 'os_region_name': '', - 'os_service_type': '', 'os_endpoint_type': '', 'os_cacert': None, - 'os_cert': None, 'os_key': None, + 'auth_url': FAKE_ENV['OS_AUTH_URL'], + 'username': FAKE_ENV['OS_USERNAME'], + 'password': FAKE_ENV['OS_PASSWORD'], + 'project_name': FAKE_ENV['OS_PROJECT_NAME'], 'max_retries': http.DEFAULT_MAX_RETRIES, 'retry_interval': http.DEFAULT_RETRY_INTERVAL, 'os_ironic_api_version': '1.11', @@ -352,16 +387,10 @@ class ShellTest(utils.BaseTestCase): self.assertRaises(keystone_exc.ConnectFailure, self.shell, '--ironic-api-version 1 node-list') expected_kwargs = { - 'ironic_url': '', 'os_auth_url': FAKE_ENV['OS_AUTH_URL'], - 'os_tenant_id': '', 'os_tenant_name': '', - 'os_username': FAKE_ENV['OS_USERNAME'], 'os_user_domain_id': '', - 'os_user_domain_name': '', 'os_password': FAKE_ENV['OS_PASSWORD'], - 'os_auth_token': '', 'os_project_id': '', - 'os_project_name': FAKE_ENV['OS_PROJECT_NAME'], - 'os_project_domain_id': '', - 'os_project_domain_name': '', 'os_region_name': '', - 'os_service_type': '', 'os_endpoint_type': '', 'os_cacert': None, - 'os_cert': None, 'os_key': None, + 'auth_url': FAKE_ENV['OS_AUTH_URL'], + 'username': FAKE_ENV['OS_USERNAME'], + 'password': FAKE_ENV['OS_PASSWORD'], + 'project_name': FAKE_ENV['OS_PROJECT_NAME'], 'max_retries': http.DEFAULT_MAX_RETRIES, 'retry_interval': http.DEFAULT_RETRY_INTERVAL, 'os_ironic_api_version': ironic_shell.LATEST_VERSION, diff --git a/ironicclient/tests/unit/v1/test_client.py b/ironicclient/tests/unit/v1/test_client.py index 33f4de329..40414d2a5 100644 --- a/ironicclient/tests/unit/v1/test_client.py +++ b/ironicclient/tests/unit/v1/test_client.py @@ -31,7 +31,7 @@ class ClientTest(utils.BaseTestCase): os_ironic_api_version=os_ironic_api_version) http_client_mock.assert_called_once_with( - endpoint, token=token, + endpoint_override=endpoint, token=token, os_ironic_api_version=os_ironic_api_version, api_version_select_state='user') @@ -45,7 +45,7 @@ class ClientTest(utils.BaseTestCase): allow_api_version_downgrade=True) http_client_mock.assert_called_once_with( - endpoint, token=token, + token=token, endpoint_override=endpoint, os_ironic_api_version=os_ironic_api_version, api_version_select_state='default') @@ -70,7 +70,7 @@ class ClientTest(utils.BaseTestCase): cache_mock.assert_called_once_with(host='ironic', port='6385') http_client_mock.assert_called_once_with( - endpoint, token=token, + endpoint_override=endpoint, token=token, os_ironic_api_version=os_ironic_api_version, api_version_select_state='cached') @@ -84,7 +84,7 @@ class ClientTest(utils.BaseTestCase): cache_mock.assert_called_once_with(host='ironic', port='6385') http_client_mock.assert_called_once_with( - endpoint, token=token, + endpoint_override=endpoint, token=token, os_ironic_api_version=client.DEFAULT_VER, api_version_select_state='default') @@ -92,8 +92,7 @@ class ClientTest(utils.BaseTestCase): self.assertRaises(exc.EndpointException, client.Client, session='fake_session', - insecure=True, - endpoint_override='http://ironic:6385') + insecure=True) def test_client_initialized_managers(self, http_client_mock): cl = client.Client('http://ironic:6385', token='safe_token', @@ -113,7 +112,7 @@ class ClientTest(utils.BaseTestCase): cl.negotiate_api_version() http_client_mock.assert_called_once_with( - endpoint, api_version_select_state='user', + api_version_select_state='user', endpoint_override=endpoint, os_ironic_api_version='latest', token=token) # TODO(TheJulia): We should verify that negotiate_version # is being called in the client and returns a version, diff --git a/ironicclient/v1/client.py b/ironicclient/v1/client.py index 975355702..d7dd55381 100644 --- a/ironicclient/v1/client.py +++ b/ironicclient/v1/client.py @@ -13,6 +13,8 @@ # License for the specific language governing permissions and limitations # under the License. +import logging + from ironicclient.common import filecache from ironicclient.common import http from ironicclient.common.http import DEFAULT_VER @@ -26,20 +28,29 @@ from ironicclient.v1 import portgroup from ironicclient.v1 import volume_connector from ironicclient.v1 import volume_target +LOG = logging.getLogger(__name__) + class Client(object): """Client for the Ironic v1 API. :param string endpoint: A user-supplied endpoint URL for the ironic - service. + service. DEPRECATED, use endpoint_override instead. + :param string endpoint_override: A user-supplied endpoint URL for the + ironic service. :param function token: Provides token for authentication. :param integer timeout: Allows customization of the timeout for client http requests. (optional) """ - def __init__(self, endpoint=None, *args, **kwargs): + def __init__(self, endpoint=None, endpoint_override=None, *args, **kwargs): """Initialize a new client for the Ironic v1 API.""" allow_downgrade = kwargs.pop('allow_api_version_downgrade', False) + if endpoint_override is None and endpoint is not None: + LOG.warning('Passing "endpoint" parameter to Client constructor ' + 'is deprecated, and it will be removed in Stein ' + 'release. Please use "endpoint_override" instead.') + endpoint_override = endpoint if kwargs.get('os_ironic_api_version'): # TODO(TheJulia): We should sanity check os_ironic_api_version # against our maximum suported version, so the client fails @@ -58,14 +69,14 @@ class Client(object): else: kwargs['api_version_select_state'] = "user" else: - if not endpoint: + if not endpoint_override: raise exc.EndpointException( - _("Must provide 'endpoint' if os_ironic_api_version " - "isn't specified")) + _("Must provide 'endpoint_override' if " + "'os_ironic_api_version' isn't specified")) # If the user didn't specify a version, use a cached version if # one has been stored - host, netport = http.get_server(endpoint) + host, netport = http.get_server(endpoint_override) saved_version = filecache.retrieve_data(host=host, port=netport) if saved_version: kwargs['api_version_select_state'] = "cached" @@ -74,8 +85,8 @@ class Client(object): kwargs['api_version_select_state'] = "default" kwargs['os_ironic_api_version'] = DEFAULT_VER - self.http_client = http._construct_http_client( - endpoint, *args, **kwargs) + kwargs['endpoint_override'] = endpoint_override + self.http_client = http._construct_http_client(*args, **kwargs) self.chassis = chassis.ChassisManager(self.http_client) self.node = node.NodeManager(self.http_client) diff --git a/lower-constraints.txt b/lower-constraints.txt index 43ecb45dc..59a058681 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -46,7 +46,7 @@ openstacksdk==0.11.2 os-client-config==1.28.0 os-service-types==1.2.0 os-testr==1.0.0 -osc-lib==1.8.0 +osc-lib==1.10.0 oslo.concurrency==3.25.0 oslo.config==5.2.0 oslo.context==2.19.2 diff --git a/releasenotes/notes/deprecate-http-client-8d664e5ec50ec403.yaml b/releasenotes/notes/deprecate-http-client-8d664e5ec50ec403.yaml new file mode 100644 index 000000000..c38ae7c09 --- /dev/null +++ b/releasenotes/notes/deprecate-http-client-8d664e5ec50ec403.yaml @@ -0,0 +1,73 @@ +--- +features: + - | + The client now supports ``none`` authorization method, which should be + used if the Identity service is not present in the deployment that the + client talks to. To use it: + + - openstack baremetal CLI -- supported starting with ``osc-lib`` version + ``1.10.0``, by providing ``--os-auth-type none`` and ``--os-endpoint`` + argument to ``openstack`` command + + - ironic CLI -- just specify the ``--ironic-url`` or ``--os-endpoint`` + argument in the ``ironic`` command (or set the corresponding environment + variable) + + - python API -- specify the ``endpoint_override`` argument to the + ``client.get_client()`` method (in addition to the required + ``api_version``) +deprecations: + - | + ``common.http.HTTPClient`` class is deprecated and will be removed in + the Stein release. If you initialize the ironic client via + ``v1.client.Client`` class directly, please pass the `keystoneauth + `_ session to the Client + constructor, so that ``common.http.SessionClient`` is used instead. + - | + As part of standardizing argument naming to the one used by `keystoneauth + `_, the following + arguments to ``client.get_client`` method are deprecated and will be + removed in Stein release: + + * ``os_auth_token``: use ``token`` instead + + * ``os_username``: use ``username`` instead + + * ``os_password``: use ``password`` instead + + * ``os_auth_url``: use ``auth_url`` instead + + * ``os_project_id``: use ``project_id`` instead + + * ``os_project_name``: use ``project_name`` instead + + * ``os_tenant_id``: use ``tenant_id`` instead + + * ``os_tenant_name``: use ``tenant_name`` instead + + * ``os_region_name``: use ``region_name`` instead + + * ``os_user_domain_id``: use ``user_domain_id`` instead + + * ``os_user_domain_name``: use ``user_domain_name`` instead + + * ``os_project_domain_id``: use ``project_domain_id`` instead + + * ``os_project_domain_name``: use ``project_domain_name`` instead + + * ``os_service_type``: use ``service_type`` instead + + * ``os_endpoint_type``: use ``interface`` instead + + * ``ironic_url``: use ``endpoint`` instead + + * ``os_cacert``, ``ca_file``: use ``cafile`` instead + + * ``os_cert``, ``cert_file``: use ``certfile`` instead + + * ``os_key``, ``key_file``: use ``keyfile`` instead + - | + The ``endpoint`` argument to the ``v1.client.Client`` constructor is + deprecated and will be removed in Stein release. Instead, please use the + standard `keystoneauth `_ + argument name ``endpoint_override``. diff --git a/requirements.txt b/requirements.txt index 9277e3263..918958573 100644 --- a/requirements.txt +++ b/requirements.txt @@ -6,7 +6,7 @@ appdirs>=1.3.0 # MIT License dogpile.cache>=0.6.2 # BSD jsonschema<3.0.0,>=2.6.0 # MIT keystoneauth1>=3.4.0 # Apache-2.0 -osc-lib>=1.8.0 # Apache-2.0 +osc-lib>=1.10.0 # Apache-2.0 oslo.i18n>=3.15.3 # Apache-2.0 oslo.serialization!=2.19.1,>=2.18.0 # Apache-2.0 oslo.utils>=3.33.0 # Apache-2.0