diff --git a/changelogs/fragments/6516-gitlab-user-sshkey-update-mode.yml b/changelogs/fragments/6516-gitlab-user-sshkey-update-mode.yml deleted file mode 100644 index dc5ea98f98..0000000000 --- a/changelogs/fragments/6516-gitlab-user-sshkey-update-mode.yml +++ /dev/null @@ -1,2 +0,0 @@ -minor_changes: - - gitlab_user - add the ``sshkey_update_mode`` option to control whether same-name SSH keys are preserved, updated when unique, or deduplicated before replacement (https://github.com/ansible-collections/community.general/issues/6516, https://github.com/ansible-collections/community.general/pull/11996). diff --git a/plugins/modules/gitlab_user.py b/plugins/modules/gitlab_user.py index ae88c3a5ae..b0701ca684 100644 --- a/plugins/modules/gitlab_user.py +++ b/plugins/modules/gitlab_user.py @@ -75,20 +75,9 @@ options: sshkey_expires_at: description: - The expiration date of the SSH public key in ISO 8601 format C(YYYY-MM-DDTHH:MM:SSZ). - - This is only used when creating or replacing SSH public keys. + - This is only used when adding new SSH public keys. type: str version_added: 3.1.0 - sshkey_update_mode: - description: - - Controls how existing SSH keys with the same O(sshkey_name) are handled. - - Use V(create) to preserve the historical behavior and leave existing keys with the same name unchanged. - - Use V(update) to replace an existing key only when exactly one key with the same name exists and its key material differs. - - Use V(deduplicate) to delete all keys with the same name before creating the requested key. - - SSH key comments are ignored when comparing key material. - type: str - choices: ["create", "update", "deduplicate"] - default: create - version_added: 13.0.0 group: description: - ID or Full path of parent group in the form of group/name. @@ -172,18 +161,6 @@ EXAMPLES = r""" group: super_group/mon_group access_level: owner -- name: "Replace a GitLab SSH key when one matching name exists" - community.general.gitlab_user: - api_url: https://gitlab.example.com/ - api_token: "{{ access_token }}" - name: My Name - username: myusername - email: me@example.com - sshkey_name: MySSH - sshkey_file: ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAI... - sshkey_update_mode: update - state: present - - name: "Create GitLab User using external identity provider" community.general.gitlab_user: api_url: https://gitlab.example.com/ @@ -332,7 +309,6 @@ class GitLabUser: "name": options["sshkey_name"], "file": options["sshkey_file"], "expires_at": options["sshkey_expires_at"], - "update_mode": options["sshkey_update_mode"], }, ) changed = changed or key_changed @@ -370,15 +346,8 @@ class GitLabUser: @param sshkey_name Name of the ssh key """ - @staticmethod - def normalize_ssh_key(ssh_key): - return " ".join(ssh_key.split(None, 2)[:2]) - - def find_ssh_keys(self, user, sshkey_name): - return [key for key in user.keys.list(**list_all_kwargs) if key.title == sshkey_name] - def ssh_key_exists(self, user, sshkey_name): - return bool(self.find_ssh_keys(user, sshkey_name)) + return any(k.title == sshkey_name for k in user.keys.list(**list_all_kwargs)) """ @param user User object @@ -386,44 +355,22 @@ class GitLabUser: """ def add_ssh_key_to_user(self, user, sshkey): - user_keys = self.find_ssh_keys(user, sshkey["name"]) - desired_key = self.normalize_ssh_key(sshkey["file"]) - update_mode = sshkey["update_mode"] - matching_keys = [key for key in user_keys if self.normalize_ssh_key(key.key) == desired_key] + if not self.ssh_key_exists(user, sshkey["name"]): + if self._module.check_mode: + return True - if update_mode == "create": - if user_keys: - return False - elif update_mode == "update": - if len(user_keys) > 1: - self._module.warn( - f"Found multiple SSH keys named '{sshkey['name']}'. Skipping update because sshkey_update_mode=update requires a single matching key." - ) - return False - if matching_keys: - return False - elif len(user_keys) == 1 and matching_keys: - return False - - if self._module.check_mode: + try: + parameter = { + "title": sshkey["name"], + "key": sshkey["file"], + } + if sshkey["expires_at"] is not None: + parameter["expires_at"] = sshkey["expires_at"] + user.keys.create(parameter) + except gitlab.exceptions.GitlabCreateError as e: + self._module.fail_json(msg=f"Failed to assign sshkey to user: {e}") return True - - try: - for key in user_keys: - key.delete() - - parameter = { - "title": sshkey["name"], - "key": sshkey["file"], - } - if sshkey["expires_at"] is not None: - parameter["expires_at"] = sshkey["expires_at"] - user.keys.create(parameter) - except gitlab.exceptions.GitlabDeleteError as e: - self._module.fail_json(msg=f"Failed to update sshkey for user: {e}") - except gitlab.exceptions.GitlabCreateError as e: - self._module.fail_json(msg=f"Failed to assign sshkey to user: {e}") - return True + return False """ @param group Group object @@ -649,7 +596,6 @@ def main(): sshkey_name=dict(type="str"), sshkey_file=dict(type="str", no_log=False), sshkey_expires_at=dict(type="str", no_log=False), - sshkey_update_mode=dict(type="str", default="create", choices=["create", "update", "deduplicate"]), group=dict(type="str"), access_level=dict( type="str", default="guest", choices=["developer", "guest", "maintainer", "master", "owner", "reporter"] @@ -691,7 +637,6 @@ def main(): user_sshkey_name = module.params["sshkey_name"] user_sshkey_file = module.params["sshkey_file"] user_sshkey_expires_at = module.params["sshkey_expires_at"] - user_sshkey_update_mode = module.params["sshkey_update_mode"] group_path = module.params["group"] access_level = module.params["access_level"] confirm = module.params["confirm"] @@ -739,7 +684,6 @@ def main(): "sshkey_name": user_sshkey_name, "sshkey_file": user_sshkey_file, "sshkey_expires_at": user_sshkey_expires_at, - "sshkey_update_mode": user_sshkey_update_mode, "group_path": group_path, "access_level": access_level, "confirm": confirm, diff --git a/tests/integration/targets/gitlab_user/defaults/main.yml b/tests/integration/targets/gitlab_user/defaults/main.yml index e123e4a903..c7a4a5dd34 100644 --- a/tests/integration/targets/gitlab_user/defaults/main.yml +++ b/tests/integration/targets/gitlab_user/defaults/main.yml @@ -8,5 +8,4 @@ gitlab_user_pass: Secr3tPassw00rd gitlab_user_email: root@localhost gitlab_sshkey_name: ansibletest gitlab_sshkey_file: ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQDI8GIMlrirf+zsvBpxnF0daykP6YEJ5wytZXhDGD2dZXg9Tln0KUSDgreT3FDgoabjlOmG1L/nhu6ML76WCsmc/wnVMlXlDlQpVJSQ2PCxGNs9WRW7Y/Pk6t9KtV/VSYr0LaPgLEU8VkffSUBJezbKa1cssjb4CmRRqcePRNYpgCXdK05TEgFvmXl9qIM8Domf1ak1PlbyMmi/MytzHmnVFzxgUKv5c0Mr+vguCi131gPdh3QSf5AHPLEoO9LcMfu2IO1zvl61wYfsJ0Wn2Fncw+tJQfUin0ffTFgUIsGqki04/YjXyWynjSwQf5Jym4BYM0i2zlDUyRxs4/Tfp4yvJFik42ambzjLK6poq+iCpQReeYih9WZUaZwUQe7zYWhTOuoV7ydsk8+kDRMPidF9K5zWkQnglGrOzdbTqnhxNpwHCg2eSRJ49kPYLOH76g8P7IQvl+zluG0o8Nndir1WcYil4D4CCBskM8WbmrElZH1CRyP/NQMNIf4hFMItTjk= ansible@ansible -gitlab_sshkey_file_updated: ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIPIaTZwcBi8bgM/DrGJ7RqK7bQDdEeXQPBZGgSF1owY/ ansible-updated@ansible gitlab_sshkey_expires_at: 2030-01-01T00:00:00.000Z diff --git a/tests/integration/targets/gitlab_user/tasks/sshkey.yml b/tests/integration/targets/gitlab_user/tasks/sshkey.yml index 6153c8ef21..bba724d5ee 100644 --- a/tests/integration/targets/gitlab_user/tasks/sshkey.yml +++ b/tests/integration/targets/gitlab_user/tasks/sshkey.yml @@ -46,46 +46,6 @@ that: - gitlab_user_sshkey_again is not changed -- name: Update gitlab user ssh key when key material changes - gitlab_user: - api_url: "{{ gitlab_host }}" - api_token: "{{ gitlab_login_token }}" - email: "{{ gitlab_user_email }}" - name: "{{ gitlab_user }}" - username: "{{ gitlab_user }}" - password: "{{ gitlab_user_pass }}" - validate_certs: false - sshkey_name: "{{ gitlab_sshkey_name }}" - sshkey_file: "{{ gitlab_sshkey_file_updated }}" - sshkey_update_mode: update - state: present - register: gitlab_user_sshkey_updated - -- name: Check ssh key has been updated - assert: - that: - - gitlab_user_sshkey_updated is changed - -- name: Update gitlab user ssh key again - gitlab_user: - api_url: "{{ gitlab_host }}" - api_token: "{{ gitlab_login_token }}" - email: "{{ gitlab_user_email }}" - name: "{{ gitlab_user }}" - username: "{{ gitlab_user }}" - password: "{{ gitlab_user_pass }}" - validate_certs: false - sshkey_name: "{{ gitlab_sshkey_name }}" - sshkey_file: "{{ gitlab_sshkey_file_updated }}" - sshkey_update_mode: update - state: present - register: gitlab_user_sshkey_updated_again - -- name: Check updated ssh key is idempotent - assert: - that: - - gitlab_user_sshkey_updated_again is not changed - - name: Add expires_at to an already created gitlab user with ssh key gitlab_user: api_url: "{{ gitlab_host }}" @@ -96,9 +56,8 @@ password: "{{ gitlab_user_pass }}" validate_certs: false sshkey_name: "{{ gitlab_sshkey_name }}" - sshkey_file: "{{ gitlab_sshkey_file_updated }}" + sshkey_file: "{{ gitlab_sshkey_file }}" sshkey_expires_at: "{{ gitlab_sshkey_expires_at }}" - sshkey_update_mode: update state: present register: gitlab_user_created_user_sshkey_expires_at diff --git a/tests/unit/plugins/modules/gitlab.py b/tests/unit/plugins/modules/gitlab.py index 18d70c3403..87eb9ef94c 100644 --- a/tests/unit/plugins/modules/gitlab.py +++ b/tests/unit/plugins/modules/gitlab.py @@ -4,10 +4,7 @@ from __future__ import annotations -import json import unittest -from typing import Any -from unittest.mock import Mock import gitlab from httmock import ( @@ -20,7 +17,6 @@ class FakeAnsibleModule: def __init__(self, module_params=None): self.check_mode = False self.params = module_params if module_params else {} - self.warn = Mock() def fail_json(self, **args): pass @@ -44,10 +40,6 @@ def python_gitlab_version_match_requirement(): return "2.3.0" -deleted_user_key_ids: list[str] = [] -created_user_keys: list[dict[str, Any]] = [] - - """ USER API """ @@ -148,8 +140,6 @@ def resp_get_user_keys(url, request): @urlmatch(scheme="http", netloc="localhost", path="/api/v4/users/1/keys", method="post") def resp_create_user_keys(url, request): - if request.body: - created_user_keys.append(json.loads(request.body)) headers = {"content-type": "application/json"} content = ( '{"id": 1, "title": "Private key",' @@ -164,36 +154,6 @@ def resp_create_user_keys(url, request): return response(201, content, headers, None, 5, request) -@urlmatch(scheme="http", netloc="localhost", path=r"/api/v4/users/1/keys/[0-9]+", method="delete") -def resp_delete_user_key(url, request): - deleted_user_key_ids.append(url.path.rsplit("/", 1)[1]) - headers = {"content-type": "application/json"} - content = "{}" - content = content.encode("utf-8") - return response(204, content, headers, None, 5, request) - - -@urlmatch(scheme="http", netloc="localhost", path="/api/v4/users/1/keys", method="get") -def resp_get_user_duplicate_keys(url, request): - headers = {"content-type": "application/json"} - content = ( - '[{"id": 1, "title": "Public key",' - '"key": "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDA1YotVDm2mAyk2tPt4E7AHm01sS6JZmcU' - "dRuSuA5zszUJzYPPUSRAX3BCgTqLqYx//UuVncK7YqLVSbbwjKR2Ez5lISgCnVfLVEXzwhv+" - "xawxKWmI7hJ5S0tOv6MJ+IxyTa4xcKwJTwB86z22n9fVOQeJTR2dSOH1WJrf0PvRk+KVNY2j" - "TiGHTi9AIjLnyD/jWRpOgtdfkLRc8EzAWrWlgNmH2WOKBw6za0az6XoG75obUdFVdW3qcD0x" - 'c809OHLi7FDf+E7U4wiZJCFuUizMeXyuK/SkaE1aee4Qp5R4dxTR4TP9M1XAYkf+kF0W9srZ+mhF069XD/zhUPJsvwEF",' - '"created_at": "2014-08-01T14:47:39.080Z"},' - '{"id": 3, "title": "Public key",' - '"key": "ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJejgt4596' - "k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4soW6NUlfDzpvZK2H5E7eQa" - 'SeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0=",' - '"created_at": "2014-08-01T14:47:39.080Z"}]' - ) - content = content.encode("utf-8") - return response(200, content, headers, None, 5, request) - - """ GROUP API """ diff --git a/tests/unit/plugins/modules/test_gitlab_user.py b/tests/unit/plugins/modules/test_gitlab_user.py index 5ad13a7d2c..9fb02f2b70 100644 --- a/tests/unit/plugins/modules/test_gitlab_user.py +++ b/tests/unit/plugins/modules/test_gitlab_user.py @@ -22,18 +22,14 @@ try: from .gitlab import ( GitlabModuleTestCase, - created_user_keys, - deleted_user_key_ids, resp_add_member, resp_create_user, resp_create_user_keys, resp_delete_user, - resp_delete_user_key, resp_find_user, resp_get_group, resp_get_member, resp_get_user, - resp_get_user_duplicate_keys, resp_get_user_keys, resp_update_member, ) @@ -41,16 +37,12 @@ except ImportError: pytestmark.append(pytest.mark.skip("Could not load gitlab module required for testing")) # Need to set these to something so that we don't fail when parsing GitlabModuleTestCase = object # type: ignore - created_user_keys = [] - deleted_user_key_ids = [] resp_find_user = _dummy resp_get_user = _dummy resp_get_user_keys = _dummy resp_create_user_keys = _dummy - resp_delete_user_key = _dummy resp_create_user = _dummy resp_delete_user = _dummy - resp_get_user_duplicate_keys = _dummy resp_get_member = _dummy resp_get_group = _dummy resp_add_member = _dummy @@ -161,7 +153,6 @@ class TestGitlabUser(GitlabModuleTestCase): "jgt4596k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4" "soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0=", "expires_at": "", - "update_mode": "create", }, ) self.assertEqual(rvalue, False) @@ -176,138 +167,10 @@ class TestGitlabUser(GitlabModuleTestCase): "TiGHTi9AIjLnyD/jWRpOgtdfkLRc8EzAWrWlgNmH2WOKBw6za0az6XoG75obUdFVdW3qcD0x" "c809OHLi7FDf+E7U4wiZJCFuUizMeXyuK/SkaE1aee4Qp5R4dxTR4TP9M1XAYkf+kF0W9srZ+mhF069XD/zhUPJsvwEF", "expires_at": "2027-01-01", - "update_mode": "create", }, ) self.assertEqual(rvalue, True) - @with_httmock(resp_get_user) - @with_httmock(resp_get_user_keys) - def test_sshkey_comment_change_is_ignored(self): - user = self.gitlab_instance.users.get(1) - - rvalue = self.moduleUtil.add_ssh_key_to_user( - user, - { - "name": "Public key", - "file": "ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJe" - "jgt4596k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4" - "soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0= desired-comment", - "expires_at": None, - "update_mode": "update", - }, - ) - - self.assertEqual(rvalue, False) - - @with_httmock(resp_get_user) - @with_httmock(resp_delete_user_key) - @with_httmock(resp_create_user_keys) - @with_httmock(resp_get_user_keys) - def test_update_sshkey_when_key_changes(self): - user = self.gitlab_instance.users.get(1) - - rvalue = self.moduleUtil.add_ssh_key_to_user( - user, - { - "name": "Public key", - "file": "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDA1YotVDm2mAyk2tPt4E7AHm01sS6JZmcU" - "dRuSuA5zszUJzYPPUSRAX3BCgTqLqYx//UuVncK7YqLVSbbwjKR2Ez5lISgCnVfLVEXzwhv+" - "xawxKWmI7hJ5S0tOv6MJ+IxyTa4xcKwJTwB86z22n9fVOQeJTR2dSOH1WJrf0PvRk+KVNY2j" - "TiGHTi9AIjLnyD/jWRpOgtdfkLRc8EzAWrWlgNmH2WOKBw6za0az6XoG75obUdFVdW3qcD0x" - "c809OHLi7FDf+E7U4wiZJCFuUizMeXyuK/SkaE1aee4Qp5R4dxTR4TP9M1XAYkf+kF0W9srZ+mhF069XD/zhUPJsvwEF", - "expires_at": "2027-01-01", - "update_mode": "update", - }, - ) - - self.assertEqual(rvalue, True) - - @with_httmock(resp_get_user) - @with_httmock(resp_get_user_keys) - def test_create_sshkey_with_existing_same_name_does_not_change(self): - user = self.gitlab_instance.users.get(1) - - rvalue = self.moduleUtil.add_ssh_key_to_user( - user, - { - "name": "Public key", - "file": "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDA1YotVDm2mAyk2tPt4E7AHm01sS6JZmcU" - "dRuSuA5zszUJzYPPUSRAX3BCgTqLqYx//UuVncK7YqLVSbbwjKR2Ez5lISgCnVfLVEXzwhv+" - "xawxKWmI7hJ5S0tOv6MJ+IxyTa4xcKwJTwB86z22n9fVOQeJTR2dSOH1WJrf0PvRk+KVNY2j" - "TiGHTi9AIjLnyD/jWRpOgtdfkLRc8EzAWrWlgNmH2WOKBw6za0az6XoG75obUdFVdW3qcD0x" - "c809OHLi7FDf+E7U4wiZJCFuUizMeXyuK/SkaE1aee4Qp5R4dxTR4TP9M1XAYkf+kF0W9srZ+mhF069XD/zhUPJsvwEF", - "expires_at": None, - "update_mode": "create", - }, - ) - - self.assertEqual(rvalue, False) - - @with_httmock(resp_get_user) - @with_httmock(resp_get_user_duplicate_keys) - def test_update_sshkey_with_multiple_same_name_keys_warns(self): - user = self.gitlab_instance.users.get(1) - - rvalue = self.moduleUtil.add_ssh_key_to_user( - user, - { - "name": "Public key", - "file": "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDA1YotVDm2mAyk2tPt4E7AHm01sS6JZmcU" - "dRuSuA5zszUJzYPPUSRAX3BCgTqLqYx//UuVncK7YqLVSbbwjKR2Ez5lISgCnVfLVEXzwhv+" - "xawxKWmI7hJ5S0tOv6MJ+IxyTa4xcKwJTwB86z22n9fVOQeJTR2dSOH1WJrf0PvRk+KVNY2j" - "TiGHTi9AIjLnyD/jWRpOgtdfkLRc8EzAWrWlgNmH2WOKBw6za0az6XoG75obUdFVdW3qcD0x" - "c809OHLi7FDf+E7U4wiZJCFuUizMeXyuK/SkaE1aee4Qp5R4dxTR4TP9M1XAYkf+kF0W9srZ+mhF069XD/zhUPJsvwEF", - "expires_at": None, - "update_mode": "update", - }, - ) - - self.assertEqual(rvalue, False) - self.mock_module.warn.assert_called_once_with( - "Found multiple SSH keys named 'Public key'. Skipping update because sshkey_update_mode=update requires a single matching key." - ) - - @with_httmock(resp_get_user) - @with_httmock(resp_delete_user_key) - @with_httmock(resp_create_user_keys) - @with_httmock(resp_get_user_duplicate_keys) - def test_deduplicate_sshkey_deletes_all_same_name_keys(self): - user = self.gitlab_instance.users.get(1) - deleted_user_key_ids.clear() - created_user_keys.clear() - - rvalue = self.moduleUtil.add_ssh_key_to_user( - user, - { - "name": "Public key", - "file": "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDA1YotVDm2mAyk2tPt4E7AHm01sS6JZmcU" - "dRuSuA5zszUJzYPPUSRAX3BCgTqLqYx//UuVncK7YqLVSbbwjKR2Ez5lISgCnVfLVEXzwhv+" - "xawxKWmI7hJ5S0tOv6MJ+IxyTa4xcKwJTwB86z22n9fVOQeJTR2dSOH1WJrf0PvRk+KVNY2j" - "TiGHTi9AIjLnyD/jWRpOgtdfkLRc8EzAWrWlgNmH2WOKBw6za0az6XoG75obUdFVdW3qcD0x" - "c809OHLi7FDf+E7U4wiZJCFuUizMeXyuK/SkaE1aee4Qp5R4dxTR4TP9M1XAYkf+kF0W9srZ+mhF069XD/zhUPJsvwEF", - "expires_at": "2027-01-01", - "update_mode": "deduplicate", - }, - ) - - self.assertEqual(rvalue, True) - self.assertEqual(deleted_user_key_ids, ["1", "3"]) - self.assertEqual( - created_user_keys, - [ - { - "title": "Public key", - "key": "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDA1YotVDm2mAyk2tPt4E7AHm01sS6JZmcU" - "dRuSuA5zszUJzYPPUSRAX3BCgTqLqYx//UuVncK7YqLVSbbwjKR2Ez5lISgCnVfLVEXzwhv+" - "xawxKWmI7hJ5S0tOv6MJ+IxyTa4xcKwJTwB86z22n9fVOQeJTR2dSOH1WJrf0PvRk+KVNY2j" - "TiGHTi9AIjLnyD/jWRpOgtdfkLRc8EzAWrWlgNmH2WOKBw6za0az6XoG75obUdFVdW3qcD0x" - "c809OHLi7FDf+E7U4wiZJCFuUizMeXyuK/SkaE1aee4Qp5R4dxTR4TP9M1XAYkf+kF0W9srZ+mhF069XD/zhUPJsvwEF", - "expires_at": "2027-01-01", - }, - ], - ) - @with_httmock(resp_get_group) @with_httmock(resp_get_member) def test_find_member(self):