diff --git a/changelogs/fragments/6516-gitlab-user-sshkey-update-mode.yml b/changelogs/fragments/6516-gitlab-user-sshkey-update-mode.yml new file mode 100644 index 0000000000..dc5ea98f98 --- /dev/null +++ b/changelogs/fragments/6516-gitlab-user-sshkey-update-mode.yml @@ -0,0 +1,2 @@ +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 f069d575bb..3dfb8efdfc 100644 --- a/plugins/modules/gitlab_user.py +++ b/plugins/modules/gitlab_user.py @@ -75,9 +75,20 @@ 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 adding new SSH public keys. + - This is only used when creating or replacing 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. @@ -161,6 +172,18 @@ 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/ @@ -309,6 +332,7 @@ 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 @@ -346,8 +370,15 @@ 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 any(k.title == sshkey_name for k in user.keys.list(**list_all_kwargs)) + return bool(self.find_ssh_keys(user, sshkey_name)) """ @param user User object @@ -355,22 +386,44 @@ class GitLabUser: """ def add_ssh_key_to_user(self, user, sshkey): - if not self.ssh_key_exists(user, sshkey["name"]): - if self._module.check_mode: - return True + 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] - 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}") + 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: return True - return False + + 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 """ @param group Group object @@ -596,6 +649,7 @@ 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"] @@ -637,6 +691,7 @@ 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"] @@ -684,6 +739,7 @@ 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 c7a4a5dd34..e123e4a903 100644 --- a/tests/integration/targets/gitlab_user/defaults/main.yml +++ b/tests/integration/targets/gitlab_user/defaults/main.yml @@ -8,4 +8,5 @@ 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 bba724d5ee..6153c8ef21 100644 --- a/tests/integration/targets/gitlab_user/tasks/sshkey.yml +++ b/tests/integration/targets/gitlab_user/tasks/sshkey.yml @@ -46,6 +46,46 @@ 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 }}" @@ -56,8 +96,9 @@ password: "{{ gitlab_user_pass }}" validate_certs: false sshkey_name: "{{ gitlab_sshkey_name }}" - sshkey_file: "{{ gitlab_sshkey_file }}" + sshkey_file: "{{ gitlab_sshkey_file_updated }}" 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 87eb9ef94c..18d70c3403 100644 --- a/tests/unit/plugins/modules/gitlab.py +++ b/tests/unit/plugins/modules/gitlab.py @@ -4,7 +4,10 @@ from __future__ import annotations +import json import unittest +from typing import Any +from unittest.mock import Mock import gitlab from httmock import ( @@ -17,6 +20,7 @@ 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 @@ -40,6 +44,10 @@ 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 """ @@ -140,6 +148,8 @@ 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",' @@ -154,6 +164,36 @@ 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 9fb02f2b70..5ad13a7d2c 100644 --- a/tests/unit/plugins/modules/test_gitlab_user.py +++ b/tests/unit/plugins/modules/test_gitlab_user.py @@ -22,14 +22,18 @@ 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, ) @@ -37,12 +41,16 @@ 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 @@ -153,6 +161,7 @@ class TestGitlabUser(GitlabModuleTestCase): "jgt4596k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4" "soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0=", "expires_at": "", + "update_mode": "create", }, ) self.assertEqual(rvalue, False) @@ -167,10 +176,138 @@ 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):