Skip to content

Commit 54b4f45

Browse files
stephenfinosfrickler
stephenfin
authored and
osfrickler
committed
identity: Don't pass unset options when creating user
In change I06f3848812bce60c65909f1311f36b70eba427d4, we migrated the 'user *' commands from keystoneclient to SDK. One side effect of this is that we are no longer able to rely on keystoneclient's 'filter_none' helper method that filters out parameters that are set to None. As such, we now need to do this ourselves. Eventually, it would be nice if SDK provided such functionality itself. The same change also introduced a bug where the '--domain' argument was being used to lookup a project rather than the '--project-domain' argument. This is also corrected. Change-Id: I1204ca611a74d134c879467d6c2b73f16e043213 Signed-off-by: Stephen Finucane <stephenfin@redhat.com> Closes-bug: #2080600 (cherry picked from commit 033793a)
1 parent 08c8445 commit 54b4f45

File tree

2 files changed

+39
-105
lines changed

2 files changed

+39
-105
lines changed

‎openstackclient/identity/v3/user.py‎

Lines changed: 35 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -249,26 +249,44 @@ def get_parser(self, prog_name):
249249
def take_action(self, parsed_args):
250250
identity_client = self.app.client_manager.sdk_connection.identity
251251

252+
kwargs = {}
253+
252254
domain_id = None
253255
if parsed_args.domain:
254256
domain_id = identity_client.find_domain(
255-
name_or_id=parsed_args.domain,
257+
parsed_args.domain,
256258
ignore_missing=False,
257259
).id
260+
kwargs['domain_id'] = domain_id
258261

259-
project_id = None
260262
if parsed_args.project:
261-
project_id = identity_client.find_project(
262-
name_or_id=parsed_args.project,
263+
project_domain_id = None
264+
if parsed_args.project_domain:
265+
project_domain_id = identity_client.find_domain(
266+
parsed_args.project_domain,
267+
ignore_missing=False,
268+
).id
269+
kwargs['default_project_id'] = identity_client.find_project(
270+
parsed_args.project,
263271
ignore_missing=False,
264-
domain_id=domain_id,
272+
domain_id=project_domain_id,
265273
).id
266274

275+
if parsed_args.description:
276+
kwargs['description'] = parsed_args.description
277+
278+
if parsed_args.email:
279+
kwargs['email'] = parsed_args.email
280+
267281
is_enabled = True
268282
if parsed_args.disable:
269283
is_enabled = False
270-
if parsed_args.password_prompt:
271-
parsed_args.password = utils.get_password(self.app.stdin)
284+
285+
password = None
286+
if parsed_args.password:
287+
password = parsed_args.password
288+
elif parsed_args.password_prompt:
289+
password = utils.get_password(self.app.stdin)
272290

273291
if not parsed_args.password:
274292
LOG.warning(
@@ -278,24 +296,26 @@ def take_action(self, parsed_args):
278296
)
279297
)
280298
options = _get_options_for_user(identity_client, parsed_args)
299+
if options:
300+
kwargs['options'] = options
281301

282302
try:
283303
user = identity_client.create_user(
284-
default_project_id=project_id,
285-
description=parsed_args.description,
286-
domain_id=domain_id,
287-
email=parsed_args.email,
288304
is_enabled=is_enabled,
289305
name=parsed_args.name,
290-
password=parsed_args.password,
291-
options=options,
306+
password=password,
307+
**kwargs,
292308
)
293309
except sdk_exc.ConflictException:
294310
if parsed_args.or_show:
311+
kwargs = {}
312+
if domain_id:
313+
kwargs['domain_id'] = domain_id
314+
295315
user = identity_client.find_user(
296-
name_or_id=parsed_args.name,
297-
domain_id=domain_id,
316+
parsed_args.name,
298317
ignore_missing=False,
318+
**kwargs,
299319
)
300320
LOG.info(_('Returning existing user %s'), user.name)
301321
else:

‎openstackclient/tests/unit/identity/v3/test_user.py‎

Lines changed: 4 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -91,11 +91,6 @@ def test_user_create_no_options(self):
9191
# Set expected values
9292
kwargs = {
9393
'name': self.user.name,
94-
'default_project_id': None,
95-
'description': None,
96-
'domain_id': None,
97-
'email': None,
98-
'options': {},
9994
'is_enabled': True,
10095
'password': None,
10196
}
@@ -127,11 +122,6 @@ def test_user_create_password(self):
127122
# Set expected values
128123
kwargs = {
129124
'name': self.user.name,
130-
'default_project_id': None,
131-
'description': None,
132-
'domain_id': None,
133-
'email': None,
134-
'options': {},
135125
'is_enabled': True,
136126
'password': 'secret',
137127
}
@@ -165,11 +155,6 @@ def test_user_create_password_prompt(self):
165155
# Set expected values
166156
kwargs = {
167157
'name': self.user.name,
168-
'default_project_id': None,
169-
'description': None,
170-
'domain_id': None,
171-
'email': None,
172-
'options': {},
173158
'is_enabled': True,
174159
'password': 'abc123',
175160
}
@@ -200,12 +185,8 @@ def test_user_create_email(self):
200185
# Set expected values
201186
kwargs = {
202187
'name': self.user.name,
203-
'default_project_id': None,
204-
'description': None,
205-
'domain_id': None,
206188
'email': 'barney@example.com',
207189
'is_enabled': True,
208-
'options': {},
209190
'password': None,
210191
}
211192
self.identity_sdk_client.create_user.assert_called_with(**kwargs)
@@ -236,11 +217,7 @@ def test_user_create_project(self):
236217
kwargs = {
237218
'name': self.user.name,
238219
'default_project_id': self.project.id,
239-
'description': None,
240-
'domain_id': None,
241-
'email': None,
242220
'is_enabled': True,
243-
'options': {},
244221
'password': None,
245222
}
246223
self.identity_sdk_client.create_user.assert_called_with(**kwargs)
@@ -284,14 +261,13 @@ def test_user_create_project_domain(self):
284261
kwargs = {
285262
'name': self.user.name,
286263
'default_project_id': self.project.id,
287-
'description': None,
288-
'domain_id': None,
289-
'email': None,
290-
'options': {},
291264
'is_enabled': True,
292265
'password': None,
293266
}
294-
self.identity_sdk_client.create_user.assert_called_with(**kwargs)
267+
self.identity_sdk_client.create_user.assert_called_once_with(**kwargs)
268+
self.identity_sdk_client.find_domain.assert_called_once_with(
269+
self.project.domain_id, ignore_missing=False
270+
)
295271

296272
self.assertEqual(self.columns, columns)
297273
datalist = (
@@ -328,11 +304,7 @@ def test_user_create_domain(self):
328304
# Set expected values
329305
kwargs = {
330306
'name': self.user.name,
331-
'default_project_id': None,
332-
'description': None,
333307
'domain_id': self.domain.id,
334-
'email': None,
335-
'options': {},
336308
'is_enabled': True,
337309
'password': None,
338310
}
@@ -361,11 +333,6 @@ def test_user_create_enable(self):
361333
# Set expected values
362334
kwargs = {
363335
'name': self.user.name,
364-
'default_project_id': None,
365-
'description': None,
366-
'domain_id': None,
367-
'email': None,
368-
'options': {},
369336
'is_enabled': True,
370337
'password': None,
371338
}
@@ -394,11 +361,6 @@ def test_user_create_disable(self):
394361
# Set expected values
395362
kwargs = {
396363
'name': self.user.name,
397-
'default_project_id': None,
398-
'description': None,
399-
'domain_id': None,
400-
'email': None,
401-
'options': {},
402364
'is_enabled': False,
403365
'password': None,
404366
}
@@ -428,10 +390,6 @@ def test_user_create_ignore_lockout_failure_attempts(self):
428390
# Set expected values
429391
kwargs = {
430392
'name': self.user.name,
431-
'default_project_id': None,
432-
'description': None,
433-
'domain_id': None,
434-
'email': None,
435393
'is_enabled': True,
436394
'options': {'ignore_lockout_failure_attempts': True},
437395
'password': None,
@@ -462,10 +420,6 @@ def test_user_create_no_ignore_lockout_failure_attempts(self):
462420
# Set expected values
463421
kwargs = {
464422
'name': self.user.name,
465-
'default_project_id': None,
466-
'description': None,
467-
'domain_id': None,
468-
'email': None,
469423
'is_enabled': True,
470424
'options': {'ignore_lockout_failure_attempts': False},
471425
'password': None,
@@ -496,10 +450,6 @@ def test_user_create_ignore_password_expiry(self):
496450
# Set expected values
497451
kwargs = {
498452
'name': self.user.name,
499-
'default_project_id': None,
500-
'description': None,
501-
'domain_id': None,
502-
'email': None,
503453
'is_enabled': True,
504454
'options': {'ignore_password_expiry': True},
505455
'password': None,
@@ -530,10 +480,6 @@ def test_user_create_no_ignore_password_expiry(self):
530480
# Set expected values
531481
kwargs = {
532482
'name': self.user.name,
533-
'default_project_id': None,
534-
'description': None,
535-
'domain_id': None,
536-
'email': None,
537483
'is_enabled': True,
538484
'options': {'ignore_password_expiry': False},
539485
'password': None,
@@ -564,10 +510,6 @@ def test_user_create_ignore_change_password_upon_first_use(self):
564510
# Set expected values
565511
kwargs = {
566512
'name': self.user.name,
567-
'default_project_id': None,
568-
'description': None,
569-
'domain_id': None,
570-
'email': None,
571513
'is_enabled': True,
572514
'options': {'ignore_change_password_upon_first_use': True},
573515
'password': None,
@@ -598,10 +540,6 @@ def test_user_create_no_ignore_change_password_upon_first_use(self):
598540
# Set expected values
599541
kwargs = {
600542
'name': self.user.name,
601-
'default_project_id': None,
602-
'description': None,
603-
'domain_id': None,
604-
'email': None,
605543
'is_enabled': True,
606544
'options': {'ignore_change_password_upon_first_use': False},
607545
'password': None,
@@ -632,10 +570,6 @@ def test_user_create_enables_lock_password(self):
632570
# Set expected values
633571
kwargs = {
634572
'name': self.user.name,
635-
'default_project_id': None,
636-
'description': None,
637-
'domain_id': None,
638-
'email': None,
639573
'is_enabled': True,
640574
'options': {'lock_password': True},
641575
'password': None,
@@ -666,10 +600,6 @@ def test_user_create_disables_lock_password(self):
666600
# Set expected values
667601
kwargs = {
668602
'name': self.user.name,
669-
'default_project_id': None,
670-
'description': None,
671-
'domain_id': None,
672-
'email': None,
673603
'is_enabled': True,
674604
'options': {'lock_password': False},
675605
'password': None,
@@ -700,10 +630,6 @@ def test_user_create_enable_multi_factor_auth(self):
700630
# Set expected values
701631
kwargs = {
702632
'name': self.user.name,
703-
'default_project_id': None,
704-
'description': None,
705-
'domain_id': None,
706-
'email': None,
707633
'is_enabled': True,
708634
'options': {'multi_factor_auth_enabled': True},
709635
'password': None,
@@ -734,10 +660,6 @@ def test_user_create_disable_multi_factor_auth(self):
734660
# Set expected values
735661
kwargs = {
736662
'name': self.user.name,
737-
'default_project_id': None,
738-
'description': None,
739-
'domain_id': None,
740-
'email': None,
741663
'is_enabled': True,
742664
'options': {'multi_factor_auth_enabled': False},
743665
'password': None,
@@ -774,10 +696,6 @@ def test_user_create_option_with_multi_factor_auth_rule(self):
774696
# Set expected values
775697
kwargs = {
776698
'name': self.user.name,
777-
'default_project_id': None,
778-
'description': None,
779-
'domain_id': None,
780-
'email': None,
781699
'is_enabled': True,
782700
'options': {
783701
'multi_factor_auth_rules': [["password", "totp"], ["password"]]
@@ -815,10 +733,6 @@ def test_user_create_with_multiple_options(self):
815733
# Set expected values
816734
kwargs = {
817735
'name': self.user.name,
818-
'default_project_id': None,
819-
'description': None,
820-
'domain_id': None,
821-
'email': None,
822736
'is_enabled': True,
823737
'options': {
824738
'ignore_password_expiry': True,

0 commit comments

Comments
 (0)