diff --git a/changelogs/fragments/1724-various-fixes-for-updating-existing-gitlab-user.yml b/changelogs/fragments/1724-various-fixes-for-updating-existing-gitlab-user.yml
new file mode 100644
index 0000000000..eab67e0f47
--- /dev/null
+++ b/changelogs/fragments/1724-various-fixes-for-updating-existing-gitlab-user.yml
@@ -0,0 +1,2 @@
+bugfixes:
+ - gitlab_user - make updates to the ``isadmin``, ``password`` and ``confirm`` options of an already existing GitLab user work (https://github.com/ansible-collections/community.general/pull/1724).
diff --git a/plugins/modules/source_control/gitlab/gitlab_user.py b/plugins/modules/source_control/gitlab/gitlab_user.py
index 458a4734b2..1e8ee65a67 100644
--- a/plugins/modules/source_control/gitlab/gitlab_user.py
+++ b/plugins/modules/source_control/gitlab/gitlab_user.py
@@ -205,6 +205,7 @@ class GitLabUser(object):
'''
def createOrUpdateUser(self, username, options):
changed = False
+ potentionally_changed = False
# Because we have already call userExists in main()
if self.userObject is None:
@@ -218,11 +219,36 @@ class GitLabUser(object):
'external': options['external']})
changed = True
else:
- changed, user = self.updateUser(self.userObject, {
- 'name': options['name'],
- 'email': options['email'],
- 'is_admin': options['isadmin'],
- 'external': options['external']})
+ changed, user = self.updateUser(
+ self.userObject, {
+ # add "normal" parameters here, put uncheckable
+ # params in the dict below
+ 'name': {'value': options['name']},
+ 'email': {'value': options['email']},
+
+ # note: for some attributes like this one the key
+ # from reading back from server is unfortunately
+ # different to the one needed for pushing/writing,
+ # in that case use the optional setter key
+ 'is_admin': {
+ 'value': options['isadmin'], 'setter': 'admin'
+ },
+ 'external': {'value': options['external']},
+ },
+ {
+ # put "uncheckable" params here, this means params
+ # which the gitlab does accept for setting but does
+ # not return any information about it
+ 'skip_reconfirmation': {'value': not options['confirm']},
+ 'password': {'value': options['password']},
+ }
+ )
+
+ # note: as we unfortunately have some uncheckable parameters
+ # where it is not possible to determine if the update
+ # changed something or not, we must assume here that a
+ # changed happend and that an user object update is needed
+ potentionally_changed = True
# Assign ssh keys
if options['sshkey_name'] and options['sshkey_file']:
@@ -237,14 +263,15 @@ class GitLabUser(object):
changed = changed or group_changed
self.userObject = user
- if changed:
- if self._module.check_mode:
- self._module.exit_json(changed=True, msg="Successfully created or updated the user %s" % username)
-
+ if (changed or potentionally_changed) and not self._module.check_mode:
try:
user.save()
except Exception as e:
self._module.fail_json(msg="Failed to update user: %s " % to_native(e))
+
+ if changed:
+ if self._module.check_mode:
+ self._module.exit_json(changed=True, msg="Successfully created or updated the user %s" % username)
return True
else:
return False
@@ -348,15 +375,23 @@ class GitLabUser(object):
@param user User object
@param arguments User attributes
'''
- def updateUser(self, user, arguments):
+ def updateUser(self, user, arguments, uncheckable_args):
changed = False
for arg_key, arg_value in arguments.items():
- if arguments[arg_key] is not None:
- if getattr(user, arg_key) != arguments[arg_key]:
- setattr(user, arg_key, arguments[arg_key])
+ av = arg_value['value']
+
+ if av is not None:
+ if getattr(user, arg_key) != av:
+ setattr(user, arg_value.get('setter', arg_key), av)
changed = True
+ for arg_key, arg_value in uncheckable_args.items():
+ av = arg_value['value']
+
+ if av is not None:
+ setattr(user, arg_value.get('setter', arg_key), av)
+
return (changed, user)
'''
diff --git a/tests/integration/targets/gitlab_user/tasks/main.yml b/tests/integration/targets/gitlab_user/tasks/main.yml
index 6408a27f37..6cbcd14c34 100644
--- a/tests/integration/targets/gitlab_user/tasks/main.yml
+++ b/tests/integration/targets/gitlab_user/tasks/main.yml
@@ -10,25 +10,25 @@
- name: Clean up gitlab user
gitlab_user:
- server_url: "{{ gitlab_host }}"
+ api_url: "{{ gitlab_host }}"
name: ansible_test_user
username: ansible_test_user
password: Secr3tPassw00rd
email: root@localhost
validate_certs: false
- login_token: "{{ gitlab_login_token }}"
+ api_token: "{{ gitlab_login_token }}"
state: absent
- name: Create gitlab user
gitlab_user:
- server_url: "{{ gitlab_host }}"
+ api_url: "{{ gitlab_host }}"
email: "{{ gitlab_user_email }}"
name: "{{ gitlab_user }}"
username: "{{ gitlab_user }}"
password: "{{ gitlab_user_pass }}"
validate_certs: False
- login_token: "{{ gitlab_login_token }}"
+ api_token: "{{ gitlab_login_token }}"
state: present
register: gitlab_user_state
@@ -39,13 +39,13 @@
- name: Create gitlab user again
gitlab_user:
- server_url: "{{ gitlab_host }}"
+ api_url: "{{ gitlab_host }}"
email: root@localhost
name: ansible_test_user
username: ansible_test_user
password: Secr3tPassw00rd
validate_certs: False
- login_token: "{{ gitlab_login_token }}"
+ api_token: "{{ gitlab_login_token }}"
state: present
register: gitlab_user_state_again
@@ -53,3 +53,198 @@
assert:
that:
- gitlab_user_state_again is not changed
+ - gitlab_user_state_again.user.is_admin == False
+
+
+- name: Update User Test => Make User Admin
+ gitlab_user:
+ api_url: "{{ gitlab_host }}"
+ email: "{{ gitlab_user_email }}"
+ name: "{{ gitlab_user }}"
+ username: "{{ gitlab_user }}"
+ isadmin: true
+ validate_certs: False
+ api_token: "{{ gitlab_login_token }}"
+ state: present
+ register: gitlab_user_state
+
+- name: Check if user is admin now
+ assert:
+ that:
+ - gitlab_user_state is changed
+ - gitlab_user_state.user.is_admin == True
+
+- name: Update User Test => Make User Admin (Again)
+ gitlab_user:
+ api_url: "{{ gitlab_host }}"
+ email: "{{ gitlab_user_email }}"
+ name: "{{ gitlab_user }}"
+ username: "{{ gitlab_user }}"
+ isadmin: true
+ validate_certs: False
+ api_token: "{{ gitlab_login_token }}"
+ state: present
+ register: gitlab_user_state
+
+- name: Check state is not changed
+ assert:
+ that:
+ - gitlab_user_state is not changed
+ - gitlab_user_state.user.is_admin == True
+
+- name: Update User Test => Remove Admin Rights
+ gitlab_user:
+ api_url: "{{ gitlab_host }}"
+ email: "{{ gitlab_user_email }}"
+ name: "{{ gitlab_user }}"
+ username: "{{ gitlab_user }}"
+ isadmin: false
+ validate_certs: False
+ api_token: "{{ gitlab_login_token }}"
+ state: present
+ register: gitlab_user_state
+
+- name: Check if user is not admin anymore
+ assert:
+ that:
+ - gitlab_user_state is changed
+ - gitlab_user_state.user.is_admin == False
+
+
+- name: Update User Test => Try Changing Mail without Confirmation Skipping
+ gitlab_user:
+ api_url: "{{ gitlab_host }}"
+ email: foo@bar.baz
+ name: "{{ gitlab_user }}"
+ username: "{{ gitlab_user }}"
+ confirm: True
+ validate_certs: False
+ api_token: "{{ gitlab_login_token }}"
+ state: present
+ register: gitlab_user_state
+
+- name: Check that eMail is unchanged (Only works with confirmation skipping)
+ assert:
+ that:
+ - gitlab_user_state is changed
+ - gitlab_user_state.user.email == gitlab_user_email
+
+- name: Update User Test => Change Mail with Confirmation Skip
+ gitlab_user:
+ api_url: "{{ gitlab_host }}"
+ email: foo@bar.baz
+ name: "{{ gitlab_user }}"
+ username: "{{ gitlab_user }}"
+ confirm: false
+ validate_certs: False
+ api_token: "{{ gitlab_login_token }}"
+ state: present
+ register: gitlab_user_state
+
+- name: Check that mail has changed now
+ assert:
+ that:
+ - gitlab_user_state is changed
+ - gitlab_user_state.user.email == 'foo@bar.baz'
+
+- name: Update User Test => Change Mail with Confirmation Skip (Again)
+ gitlab_user:
+ api_url: "{{ gitlab_host }}"
+ email: foo@bar.baz
+ name: "{{ gitlab_user }}"
+ username: "{{ gitlab_user }}"
+ confirm: false
+ validate_certs: False
+ api_token: "{{ gitlab_login_token }}"
+ state: present
+ register: gitlab_user_state
+
+- name: Check state is not changed
+ assert:
+ that:
+ - gitlab_user_state is not changed
+ - gitlab_user_state.user.email == 'foo@bar.baz'
+
+- name: Update User Test => Revert to original Mail Address
+ gitlab_user:
+ api_url: "{{ gitlab_host }}"
+ email: "{{ gitlab_user_email }}"
+ name: "{{ gitlab_user }}"
+ username: "{{ gitlab_user }}"
+ confirm: false
+ validate_certs: False
+ api_token: "{{ gitlab_login_token }}"
+ state: present
+ register: gitlab_user_state
+
+- name: Check that reverting mail back to original has worked
+ assert:
+ that:
+ - gitlab_user_state is changed
+ - gitlab_user_state.user.email == gitlab_user_email
+
+
+- name: Update User Test => Change User Password
+ gitlab_user:
+ api_url: "{{ gitlab_host }}"
+ validate_certs: False
+
+ # note: the only way to check if a password really is what it is expected
+ # to be is to use it for login, so we use it here instead of the
+ # default token assuming that a user can always change its own password
+ api_username: "{{ gitlab_user }}"
+ api_password: "{{ gitlab_user_pass }}"
+
+ email: "{{ gitlab_user_email }}"
+ name: "{{ gitlab_user }}"
+ username: "{{ gitlab_user }}"
+ password: new-super-password
+ state: present
+ register: gitlab_user_state
+
+- name: Check PW setting return state
+ assert:
+ that:
+ # note: there is no way to determine if a password has changed or
+ # not, so it can only be always yellow or always green, we
+ # decided for always green for now
+ - gitlab_user_state is not changed
+
+- name: Update User Test => Reset User Password
+ gitlab_user:
+ api_url: "{{ gitlab_host }}"
+ validate_certs: False
+
+ api_username: "{{ gitlab_user }}"
+ api_password: new-super-password
+
+ email: "{{ gitlab_user_email }}"
+ name: "{{ gitlab_user }}"
+ username: "{{ gitlab_user }}"
+ password: "{{ gitlab_user_pass }}"
+ state: present
+ register: gitlab_user_state
+
+- name: Check PW setting return state (Again)
+ assert:
+ that:
+ - gitlab_user_state is not changed
+
+- name: Update User Test => Check that password was reset
+ gitlab_user:
+ api_url: "{{ gitlab_host }}"
+ validate_certs: False
+
+ api_username: "{{ gitlab_user }}"
+ api_password: "{{ gitlab_user_pass }}"
+
+ email: "{{ gitlab_user_email }}"
+ name: "{{ gitlab_user }}"
+ username: "{{ gitlab_user }}"
+ state: present
+ register: gitlab_user_state
+
+- name: Check PW setting return state (Reset)
+ assert:
+ that:
+ - gitlab_user_state is not changed
diff --git a/tests/unit/plugins/modules/source_control/gitlab/test_gitlab_user.py b/tests/unit/plugins/modules/source_control/gitlab/test_gitlab_user.py
index f78f0efb71..4a47654a8c 100644
--- a/tests/unit/plugins/modules/source_control/gitlab/test_gitlab_user.py
+++ b/tests/unit/plugins/modules/source_control/gitlab/test_gitlab_user.py
@@ -88,16 +88,33 @@ class TestGitlabUser(GitlabModuleTestCase):
@with_httmock(resp_get_user)
def test_update_user(self):
user = self.gitlab_instance.users.get(1)
- changed, newUser = self.moduleUtil.updateUser(user, {'name': "Jack Smith", "is_admin": "true"})
+
+ changed, newUser = self.moduleUtil.updateUser(
+ user,
+ {'name': {'value': "Jack Smith"}, "is_admin": {'value': "true", 'setter': 'admin'}}, {}
+ )
self.assertEqual(changed, True)
self.assertEqual(newUser.name, "Jack Smith")
- self.assertEqual(newUser.is_admin, "true")
+ self.assertEqual(newUser.admin, "true")
- changed, newUser = self.moduleUtil.updateUser(user, {'name': "Jack Smith"})
+ changed, newUser = self.moduleUtil.updateUser(user, {'name': {'value': "Jack Smith"}}, {})
self.assertEqual(changed, False)
+ changed, newUser = self.moduleUtil.updateUser(
+ user,
+ {}, {
+ 'skip_reconfirmation': {'value': True},
+ 'password': {'value': 'super_secret-super_secret'},
+ }
+ )
+
+ # note: uncheckable parameters dont set changed state
+ self.assertEqual(changed, False)
+ self.assertEqual(newUser.skip_reconfirmation, True)
+ self.assertEqual(newUser.password, 'super_secret-super_secret')
+
@with_httmock(resp_find_user)
@with_httmock(resp_delete_user)
def test_delete_user(self):