From 033793aa0e96e3b8c8e729ff8fa67a9a37029e55 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Mon, 16 Sep 2024 12:43:04 +0100 Subject: [PATCH] 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 Closes-bug: #2080600 --- openstackclient/identity/v3/user.py | 56 +++++++---- .../tests/unit/identity/v3/test_user.py | 94 +------------------ 2 files changed, 42 insertions(+), 108 deletions(-) diff --git a/openstackclient/identity/v3/user.py b/openstackclient/identity/v3/user.py index 38b9a0ab0c..545b1cda77 100644 --- a/openstackclient/identity/v3/user.py +++ b/openstackclient/identity/v3/user.py @@ -249,26 +249,44 @@ class CreateUser(command.ShowOne): def take_action(self, parsed_args): identity_client = self.app.client_manager.sdk_connection.identity + kwargs = {} + domain_id = None if parsed_args.domain: domain_id = identity_client.find_domain( - name_or_id=parsed_args.domain, + parsed_args.domain, ignore_missing=False, ).id + kwargs['domain_id'] = domain_id + + if parsed_args.project: + project_domain_id = None + if parsed_args.project_domain: + project_domain_id = identity_client.find_domain( + parsed_args.project_domain, + ignore_missing=False, + ).id + kwargs['default_project_id'] = identity_client.find_project( + parsed_args.project, + ignore_missing=False, + domain_id=project_domain_id, + ).id - project_id = None - if parsed_args.project: - project_id = identity_client.find_project( - name_or_id=parsed_args.project, - ignore_missing=False, - domain_id=domain_id, - ).id + if parsed_args.description: + kwargs['description'] = parsed_args.description + + if parsed_args.email: + kwargs['email'] = parsed_args.email is_enabled = True if parsed_args.disable: is_enabled = False - if parsed_args.password_prompt: - parsed_args.password = utils.get_password(self.app.stdin) + + password = None + if parsed_args.password: + password = parsed_args.password + elif parsed_args.password_prompt: + password = utils.get_password(self.app.stdin) if not parsed_args.password: LOG.warning( @@ -278,24 +296,26 @@ class CreateUser(command.ShowOne): ) ) options = _get_options_for_user(identity_client, parsed_args) + if options: + kwargs['options'] = options try: user = identity_client.create_user( - default_project_id=project_id, - description=parsed_args.description, - domain_id=domain_id, - email=parsed_args.email, is_enabled=is_enabled, name=parsed_args.name, - password=parsed_args.password, - options=options, + password=password, + **kwargs, ) except sdk_exc.ConflictException: if parsed_args.or_show: + kwargs = {} + if domain_id: + kwargs['domain_id'] = domain_id + user = identity_client.find_user( - name_or_id=parsed_args.name, - domain_id=domain_id, + parsed_args.name, ignore_missing=False, + **kwargs, ) LOG.info(_('Returning existing user %s'), user.name) else: diff --git a/openstackclient/tests/unit/identity/v3/test_user.py b/openstackclient/tests/unit/identity/v3/test_user.py index 19c1a6dcc7..7f4c2497ec 100644 --- a/openstackclient/tests/unit/identity/v3/test_user.py +++ b/openstackclient/tests/unit/identity/v3/test_user.py @@ -91,11 +91,6 @@ class TestUserCreate(identity_fakes.TestIdentityv3): # Set expected values kwargs = { 'name': self.user.name, - 'default_project_id': None, - 'description': None, - 'domain_id': None, - 'email': None, - 'options': {}, 'is_enabled': True, 'password': None, } @@ -127,11 +122,6 @@ class TestUserCreate(identity_fakes.TestIdentityv3): # Set expected values kwargs = { 'name': self.user.name, - 'default_project_id': None, - 'description': None, - 'domain_id': None, - 'email': None, - 'options': {}, 'is_enabled': True, 'password': 'secret', } @@ -165,11 +155,6 @@ class TestUserCreate(identity_fakes.TestIdentityv3): # Set expected values kwargs = { 'name': self.user.name, - 'default_project_id': None, - 'description': None, - 'domain_id': None, - 'email': None, - 'options': {}, 'is_enabled': True, 'password': 'abc123', } @@ -200,12 +185,8 @@ class TestUserCreate(identity_fakes.TestIdentityv3): # Set expected values kwargs = { 'name': self.user.name, - 'default_project_id': None, - 'description': None, - 'domain_id': None, 'email': 'barney@example.com', 'is_enabled': True, - 'options': {}, 'password': None, } self.identity_sdk_client.create_user.assert_called_with(**kwargs) @@ -236,11 +217,7 @@ class TestUserCreate(identity_fakes.TestIdentityv3): kwargs = { 'name': self.user.name, 'default_project_id': self.project.id, - 'description': None, - 'domain_id': None, - 'email': None, 'is_enabled': True, - 'options': {}, 'password': None, } self.identity_sdk_client.create_user.assert_called_with(**kwargs) @@ -284,14 +261,13 @@ class TestUserCreate(identity_fakes.TestIdentityv3): kwargs = { 'name': self.user.name, 'default_project_id': self.project.id, - 'description': None, - 'domain_id': None, - 'email': None, - 'options': {}, 'is_enabled': True, 'password': None, } - self.identity_sdk_client.create_user.assert_called_with(**kwargs) + self.identity_sdk_client.create_user.assert_called_once_with(**kwargs) + self.identity_sdk_client.find_domain.assert_called_once_with( + self.project.domain_id, ignore_missing=False + ) self.assertEqual(self.columns, columns) datalist = ( @@ -328,11 +304,7 @@ class TestUserCreate(identity_fakes.TestIdentityv3): # Set expected values kwargs = { 'name': self.user.name, - 'default_project_id': None, - 'description': None, 'domain_id': self.domain.id, - 'email': None, - 'options': {}, 'is_enabled': True, 'password': None, } @@ -361,11 +333,6 @@ class TestUserCreate(identity_fakes.TestIdentityv3): # Set expected values kwargs = { 'name': self.user.name, - 'default_project_id': None, - 'description': None, - 'domain_id': None, - 'email': None, - 'options': {}, 'is_enabled': True, 'password': None, } @@ -394,11 +361,6 @@ class TestUserCreate(identity_fakes.TestIdentityv3): # Set expected values kwargs = { 'name': self.user.name, - 'default_project_id': None, - 'description': None, - 'domain_id': None, - 'email': None, - 'options': {}, 'is_enabled': False, 'password': None, } @@ -428,10 +390,6 @@ class TestUserCreate(identity_fakes.TestIdentityv3): # Set expected values kwargs = { 'name': self.user.name, - 'default_project_id': None, - 'description': None, - 'domain_id': None, - 'email': None, 'is_enabled': True, 'options': {'ignore_lockout_failure_attempts': True}, 'password': None, @@ -462,10 +420,6 @@ class TestUserCreate(identity_fakes.TestIdentityv3): # Set expected values kwargs = { 'name': self.user.name, - 'default_project_id': None, - 'description': None, - 'domain_id': None, - 'email': None, 'is_enabled': True, 'options': {'ignore_lockout_failure_attempts': False}, 'password': None, @@ -496,10 +450,6 @@ class TestUserCreate(identity_fakes.TestIdentityv3): # Set expected values kwargs = { 'name': self.user.name, - 'default_project_id': None, - 'description': None, - 'domain_id': None, - 'email': None, 'is_enabled': True, 'options': {'ignore_password_expiry': True}, 'password': None, @@ -530,10 +480,6 @@ class TestUserCreate(identity_fakes.TestIdentityv3): # Set expected values kwargs = { 'name': self.user.name, - 'default_project_id': None, - 'description': None, - 'domain_id': None, - 'email': None, 'is_enabled': True, 'options': {'ignore_password_expiry': False}, 'password': None, @@ -564,10 +510,6 @@ class TestUserCreate(identity_fakes.TestIdentityv3): # Set expected values kwargs = { 'name': self.user.name, - 'default_project_id': None, - 'description': None, - 'domain_id': None, - 'email': None, 'is_enabled': True, 'options': {'ignore_change_password_upon_first_use': True}, 'password': None, @@ -598,10 +540,6 @@ class TestUserCreate(identity_fakes.TestIdentityv3): # Set expected values kwargs = { 'name': self.user.name, - 'default_project_id': None, - 'description': None, - 'domain_id': None, - 'email': None, 'is_enabled': True, 'options': {'ignore_change_password_upon_first_use': False}, 'password': None, @@ -632,10 +570,6 @@ class TestUserCreate(identity_fakes.TestIdentityv3): # Set expected values kwargs = { 'name': self.user.name, - 'default_project_id': None, - 'description': None, - 'domain_id': None, - 'email': None, 'is_enabled': True, 'options': {'lock_password': True}, 'password': None, @@ -666,10 +600,6 @@ class TestUserCreate(identity_fakes.TestIdentityv3): # Set expected values kwargs = { 'name': self.user.name, - 'default_project_id': None, - 'description': None, - 'domain_id': None, - 'email': None, 'is_enabled': True, 'options': {'lock_password': False}, 'password': None, @@ -700,10 +630,6 @@ class TestUserCreate(identity_fakes.TestIdentityv3): # Set expected values kwargs = { 'name': self.user.name, - 'default_project_id': None, - 'description': None, - 'domain_id': None, - 'email': None, 'is_enabled': True, 'options': {'multi_factor_auth_enabled': True}, 'password': None, @@ -734,10 +660,6 @@ class TestUserCreate(identity_fakes.TestIdentityv3): # Set expected values kwargs = { 'name': self.user.name, - 'default_project_id': None, - 'description': None, - 'domain_id': None, - 'email': None, 'is_enabled': True, 'options': {'multi_factor_auth_enabled': False}, 'password': None, @@ -774,10 +696,6 @@ class TestUserCreate(identity_fakes.TestIdentityv3): # Set expected values kwargs = { 'name': self.user.name, - 'default_project_id': None, - 'description': None, - 'domain_id': None, - 'email': None, 'is_enabled': True, 'options': { 'multi_factor_auth_rules': [["password", "totp"], ["password"]] @@ -815,10 +733,6 @@ class TestUserCreate(identity_fakes.TestIdentityv3): # Set expected values kwargs = { 'name': self.user.name, - 'default_project_id': None, - 'description': None, - 'domain_id': None, - 'email': None, 'is_enabled': True, 'options': { 'ignore_password_expiry': True,