Skip to content

Commit 77143d4

Browse files
zhaochaozhaochao
committed
Migrate to keystoneauth identity cli opts.
Use keystoneauth1 to parse keystone authentication arguments. Previously these arguments are parsed in the different service clients seperately. Use keystoneauth1 instead will make this consistent across projects and less error-prone. This change is inspired by NovaClient. Co-Authored-By: Morgan Fainberg <morgan.fainberg@gmail.com> Co-Authored-By: David Hu <david.hu@hp.com> Co-Authored-By: Monty Taylor <mordred@inaugust.com> Closes-Bug: #1734945 Change-Id: I3c5141eeddd3747ff542e95b04e4848470ad9508 Signed-off-by: Zhao Chao <zhaochao1984@gmail.com>
1 parent 36a2f4b commit 77143d4

2 files changed

Lines changed: 92 additions & 133 deletions

File tree

‎cinderclient/shell.py‎

Lines changed: 76 additions & 133 deletions
Original file line numberDiff line numberDiff line change
@@ -138,23 +138,6 @@ def get_base_parser(self):
138138
default=False),
139139
help=_('Shows debugging output.'))
140140

141-
parser.add_argument('--os-auth-system',
142-
metavar='<os-auth-system>',
143-
dest='os_auth_type',
144-
default=(utils.env('OS_AUTH_TYPE') or
145-
utils.env('OS_AUTH_SYSTEM')),
146-
help=_('DEPRECATED! Use --os-auth-type. '
147-
'Defaults to env[OS_AUTH_SYSTEM].'))
148-
parser.add_argument('--os_auth_system',
149-
help=argparse.SUPPRESS)
150-
parser.add_argument('--os-auth-type',
151-
metavar='<os-auth-type>',
152-
dest='os_auth_type',
153-
default=(utils.env('OS_AUTH_TYPE') or
154-
utils.env('OS_AUTH_SYSTEM')),
155-
help=_('Defaults to env[OS_AUTH_TYPE].'))
156-
parser.add_argument('--os_auth_type',
157-
help=argparse.SUPPRESS)
158141
parser.add_argument('--service-type',
159142
metavar='<service-type>',
160143
help=_('Service type. '
@@ -255,11 +238,16 @@ def get_base_parser(self):
255238
return parser
256239

257240
def _append_global_identity_args(self, parser):
258-
# FIXME(bklei): these are global identity (Keystone) arguments which
259-
# should be consistent and shared by all service clients. Therefore,
260-
# they should be provided by python-keystoneclient. We will need to
261-
# refactor this code once this functionality is available in
262-
# python-keystoneclient.
241+
loading.register_session_argparse_arguments(parser)
242+
243+
# Use "password" auth plugin as default and keep the explicit
244+
# "--os-token" arguments below for backward compatibility.
245+
default_auth_plugin = 'password'
246+
247+
# Passing [] to loading.register_auth_argparse_arguments to avoid
248+
# the auth_type being overriden by the command line.
249+
loading.register_auth_argparse_arguments(
250+
parser, [], default=default_auth_plugin)
263251

264252
parser.add_argument(
265253
'--os-auth-strategy', metavar='<auth-strategy>',
@@ -271,128 +259,88 @@ def _append_global_identity_args(self, parser):
271259
'--os_auth_strategy',
272260
help=argparse.SUPPRESS)
273261

274-
parser.add_argument('--os-username',
275-
metavar='<auth-user-name>',
276-
default=utils.env('OS_USERNAME',
277-
'CINDER_USERNAME'),
278-
help=_('OpenStack user name. '
279-
'Default=env[OS_USERNAME].'))
262+
# Change os_auth_type default value defined by
263+
# register_auth_argparse_arguments to be backward compatible
264+
# with OS_AUTH_SYSTEM.
265+
env_plugin = utils.env('OS_AUTH_TYPE',
266+
'OS_AUTH_PLUGIN',
267+
'OS_AUTH_SYSTEM')
268+
parser.set_defaults(os_auth_type=env_plugin)
269+
parser.add_argument('--os_auth_type',
270+
help=argparse.SUPPRESS)
271+
272+
parser.add_argument('--os-auth-system',
273+
metavar='<os-auth-system>',
274+
dest='os_auth_type',
275+
default=env_plugin,
276+
help=_('DEPRECATED! Use --os-auth-type. '
277+
'Defaults to env[OS_AUTH_SYSTEM].'))
278+
parser.add_argument('--os_auth_system',
279+
help=argparse.SUPPRESS)
280+
281+
parser.set_defaults(os_username=utils.env('OS_USERNAME',
282+
'CINDER_USERNAME'))
280283
parser.add_argument('--os_username',
281284
help=argparse.SUPPRESS)
282285

283-
parser.add_argument('--os-password',
284-
metavar='<auth-password>',
285-
default=utils.env('OS_PASSWORD',
286-
'CINDER_PASSWORD'),
287-
help=_('Password for OpenStack user. '
288-
'Default=env[OS_PASSWORD].'))
286+
parser.set_defaults(os_password=utils.env('OS_PASSWORD',
287+
'CINDER_PASSWORD'))
289288
parser.add_argument('--os_password',
290289
help=argparse.SUPPRESS)
291290

292-
parser.add_argument('--os-tenant-name',
293-
metavar='<auth-tenant-name>',
294-
default=utils.env('OS_TENANT_NAME',
295-
'OS_PROJECT_NAME',
296-
'CINDER_PROJECT_ID'),
297-
help=_('Tenant name. '
298-
'Default=env[OS_TENANT_NAME].'))
291+
# tenant_name is deprecated by project_name in keystoneauth
292+
parser.set_defaults(os_project_name=utils.env('OS_PROJECT_NAME',
293+
'OS_TENANT_NAME',
294+
'CINDER_PROJECT_ID'))
299295
parser.add_argument('--os_tenant_name',
296+
dest='os_project_name',
300297
help=argparse.SUPPRESS)
298+
parser.add_argument(
299+
'--os_project_name',
300+
help=argparse.SUPPRESS)
301301

302-
parser.add_argument('--os-tenant-id',
303-
metavar='<auth-tenant-id>',
304-
default=utils.env('OS_TENANT_ID',
305-
'OS_PROJECT_ID',
306-
'CINDER_TENANT_ID'),
307-
help=_('ID for the tenant. '
308-
'Default=env[OS_TENANT_ID].'))
302+
# tenant_id is deprecated by project_id in keystoneauth
303+
parser.set_defaults(os_project_id=utils.env('OS_PROJECT_ID',
304+
'OS_TENANT_ID',
305+
'CINDER_TENANT_ID'))
309306
parser.add_argument('--os_tenant_id',
307+
dest='os_project_id',
310308
help=argparse.SUPPRESS)
309+
parser.add_argument(
310+
'--os_project_id',
311+
help=argparse.SUPPRESS)
311312

312-
parser.add_argument('--os-auth-url',
313-
metavar='<auth-url>',
314-
default=utils.env('OS_AUTH_URL',
315-
'CINDER_URL'),
316-
help=_('URL for the authentication service. '
317-
'Default=env[OS_AUTH_URL].'))
313+
parser.set_defaults(os_auth_url=utils.env('OS_AUTH_URL',
314+
'CINDER_URL'))
318315
parser.add_argument('--os_auth_url',
319316
help=argparse.SUPPRESS)
320317

321-
parser.add_argument(
322-
'--os-user-id', metavar='<auth-user-id>',
323-
default=utils.env('OS_USER_ID'),
324-
help=_('Authentication user ID (Env: OS_USER_ID).'))
325-
318+
parser.set_defaults(os_user_id=utils.env('OS_USER_ID'))
326319
parser.add_argument(
327320
'--os_user_id',
328321
help=argparse.SUPPRESS)
329322

330-
parser.add_argument(
331-
'--os-user-domain-id',
332-
metavar='<auth-user-domain-id>',
333-
default=utils.env('OS_USER_DOMAIN_ID'),
334-
help=_('OpenStack user domain ID. '
335-
'Defaults to env[OS_USER_DOMAIN_ID].'))
336-
323+
parser.set_defaults(
324+
os_user_domain_id=utils.env('OS_USER_DOMAIN_ID'))
337325
parser.add_argument(
338326
'--os_user_domain_id',
339327
help=argparse.SUPPRESS)
340328

341-
parser.add_argument(
342-
'--os-user-domain-name',
343-
metavar='<auth-user-domain-name>',
344-
default=utils.env('OS_USER_DOMAIN_NAME'),
345-
help=_('OpenStack user domain name. '
346-
'Defaults to env[OS_USER_DOMAIN_NAME].'))
347-
329+
parser.set_defaults(
330+
os_user_domain_name=utils.env('OS_USER_DOMAIN_NAME'))
348331
parser.add_argument(
349332
'--os_user_domain_name',
350333
help=argparse.SUPPRESS)
351334

352-
parser.add_argument(
353-
'--os-project-id',
354-
metavar='<auth-project-id>',
355-
default=utils.env('OS_PROJECT_ID'),
356-
help=_('Another way to specify tenant ID. '
357-
'This option is mutually exclusive with '
358-
' --os-tenant-id. '
359-
'Defaults to env[OS_PROJECT_ID].'))
335+
parser.set_defaults(
336+
os_project_domain_id=utils.env('OS_PROJECT_DOMAIN_ID'))
360337

361-
parser.add_argument(
362-
'--os_project_id',
363-
help=argparse.SUPPRESS)
364-
365-
parser.add_argument(
366-
'--os-project-name',
367-
metavar='<auth-project-name>',
368-
default=utils.env('OS_PROJECT_NAME'),
369-
help=_('Another way to specify tenant name. '
370-
'This option is mutually exclusive with '
371-
' --os-tenant-name. '
372-
'Defaults to env[OS_PROJECT_NAME].'))
373-
374-
parser.add_argument(
375-
'--os_project_name',
376-
help=argparse.SUPPRESS)
377-
378-
parser.add_argument(
379-
'--os-project-domain-id',
380-
metavar='<auth-project-domain-id>',
381-
default=utils.env('OS_PROJECT_DOMAIN_ID'),
382-
help=_('Defaults to env[OS_PROJECT_DOMAIN_ID].'))
338+
parser.set_defaults(
339+
os_project_domain_name=utils.env('OS_PROJECT_DOMAIN_NAME'))
383340

384-
parser.add_argument(
385-
'--os-project-domain-name',
386-
metavar='<auth-project-domain-name>',
387-
default=utils.env('OS_PROJECT_DOMAIN_NAME'),
388-
help=_('Defaults to env[OS_PROJECT_DOMAIN_NAME].'))
389-
390-
parser.add_argument('--os-region-name',
391-
metavar='<region-name>',
392-
default=utils.env('OS_REGION_NAME',
393-
'CINDER_REGION_NAME'),
394-
help=_('Region name. '
395-
'Default=env[OS_REGION_NAME].'))
341+
parser.set_defaults(
342+
os_region_name=utils.env('OS_REGION_NAME',
343+
'CINDER_REGION_NAME'))
396344
parser.add_argument('--os_region_name',
397345
help=argparse.SUPPRESS)
398346

@@ -412,8 +360,6 @@ def _append_global_identity_args(self, parser):
412360
'--os_url',
413361
help=argparse.SUPPRESS)
414362

415-
# Register the CLI arguments that have moved to the session object.
416-
loading.register_session_argparse_arguments(parser)
417363
parser.set_defaults(insecure=utils.env('CINDERCLIENT_INSECURE',
418364
default=False))
419365

@@ -646,13 +592,13 @@ def main(self, argv):
646592
self.do_bash_completion(args)
647593
return 0
648594

649-
(os_username, os_password, os_tenant_name, os_auth_url,
650-
os_region_name, os_tenant_id, endpoint_type,
595+
(os_username, os_password, os_project_name, os_auth_url,
596+
os_region_name, os_project_id, endpoint_type,
651597
service_type, service_name, volume_service_name, os_endpoint,
652598
cacert, os_auth_type) = (
653599
args.os_username, args.os_password,
654-
args.os_tenant_name, args.os_auth_url,
655-
args.os_region_name, args.os_tenant_id,
600+
args.os_project_name, args.os_auth_url,
601+
args.os_region_name, args.os_project_id,
656602
args.os_endpoint_type,
657603
args.service_type, args.service_name,
658604
args.volume_service_name,
@@ -675,12 +621,11 @@ def main(self, argv):
675621
# for os_username or os_password but for compatibility it is not.
676622

677623
# V3 stuff
678-
project_info_provided = ((self.options.os_tenant_name or
679-
self.options.os_tenant_id) or
680-
(self.options.os_project_name and
624+
project_info_provided = ((self.options.os_project_name and
681625
(self.options.os_project_domain_name or
682626
self.options.os_project_domain_id)) or
683-
self.options.os_project_id)
627+
self.options.os_project_id or
628+
self.options.os_project_name)
684629

685630
# NOTE(e0ne): if auth_session exists it means auth plugin created
686631
# session and we don't need to check for password and other
@@ -752,9 +697,9 @@ def main(self, argv):
752697

753698
self.cs = client.Client(
754699
api_version, os_username,
755-
os_password, os_tenant_name, os_auth_url,
700+
os_password, os_project_name, os_auth_url,
756701
region_name=os_region_name,
757-
tenant_id=os_tenant_id,
702+
tenant_id=os_project_id,
758703
endpoint_type=endpoint_type,
759704
extensions=self.extensions,
760705
service_type=service_type,
@@ -852,9 +797,8 @@ def get_v2_auth(self, v2_auth_url):
852797

853798
username = self.options.os_username
854799
password = self.options.os_password
855-
tenant_id = self.options.os_tenant_id or self.options.os_project_id
856-
tenant_name = (self.options.os_tenant_name or
857-
self.options.os_project_name)
800+
tenant_id = self.options.os_project_id
801+
tenant_name = self.options.os_project_name
858802

859803
return v2_auth.Password(
860804
v2_auth_url,
@@ -870,9 +814,8 @@ def get_v3_auth(self, v3_auth_url):
870814
user_domain_name = self.options.os_user_domain_name
871815
user_domain_id = self.options.os_user_domain_id
872816
password = self.options.os_password
873-
project_id = self.options.os_project_id or self.options.os_tenant_id
874-
project_name = (self.options.os_project_name or
875-
self.options.os_tenant_name)
817+
project_id = self.options.os_project_id
818+
project_name = self.options.os_project_name
876819
project_domain_name = self.options.os_project_domain_name
877820
project_domain_id = self.options.os_project_domain_id
878821

‎cinderclient/tests/unit/test_shell.py‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import fixtures
2020
import keystoneauth1.exceptions as ks_exc
2121
from keystoneauth1.exceptions import DiscoveryFailure
22+
from keystoneauth1.identity.generic.password import Password as ks_password
2223
from keystoneauth1 import session
2324
import mock
2425
import requests_mock
@@ -92,6 +93,21 @@ def test_auth_system_env(self):
9293
args, __ = _shell.get_base_parser().parse_known_args([])
9394
self.assertEqual('noauth', args.os_auth_type)
9495

96+
@mock.patch.object(cinderclient.shell.OpenStackCinderShell,
97+
'_get_keystone_session')
98+
@mock.patch.object(cinderclient.client.SessionClient, 'authenticate',
99+
side_effect=RuntimeError())
100+
def test_password_auth_type(self, mock_authenticate,
101+
mock_get_session):
102+
self.make_env(include={'OS_AUTH_TYPE': 'password'})
103+
_shell = shell.OpenStackCinderShell()
104+
105+
# We crash the command after Client instantiation because this test
106+
# focuses only keystoneauth1 indentity cli opts parsing.
107+
self.assertRaises(RuntimeError, _shell.main, ['list'])
108+
self.assertIsInstance(_shell.cs.client.session.auth,
109+
ks_password)
110+
95111
def test_help_unknown_command(self):
96112
self.assertRaises(exceptions.CommandError, self.shell, 'help foofoo')
97113

0 commit comments

Comments
 (0)