From: Simon Glass <simon.glass@canonical.com> Add check-gitlab command to verify GitLab permissions for the configured token. This helps diagnose issues like 403 Forbidden errors when trying to create merge requests. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/pickman/README.rst | 7 +++ tools/pickman/__main__.py | 5 +++ tools/pickman/control.py | 40 +++++++++++++++++ tools/pickman/ftest.py | 53 ++++++++++++++++++++++ tools/pickman/gitlab_api.py | 88 +++++++++++++++++++++++++++++++++++++ 5 files changed, 193 insertions(+) diff --git a/tools/pickman/README.rst b/tools/pickman/README.rst index ce520f09a45..ff3ca3d58c3 100644 --- a/tools/pickman/README.rst +++ b/tools/pickman/README.rst @@ -60,6 +60,13 @@ This shows: master branch (ci/master) - The last common commit between the two branches +To check GitLab permissions for the configured token:: + + ./tools/pickman/pickman check-gitlab + +This verifies that the GitLab token has the required permissions to push +branches and create merge requests. Use ``-r`` to specify a different remote. + To show the next set of commits to cherry-pick from a source branch:: ./tools/pickman/pickman next-set us/next diff --git a/tools/pickman/__main__.py b/tools/pickman/__main__.py index 1258a0835c2..8a56976b872 100755 --- a/tools/pickman/__main__.py +++ b/tools/pickman/__main__.py @@ -48,6 +48,11 @@ def parse_args(argv): apply_cmd.add_argument('-t', '--target', default='master', help='Target branch for MR (default: master)') + check_gl = subparsers.add_parser('check-gitlab', + help='Check GitLab permissions') + check_gl.add_argument('-r', '--remote', default='ci', + help='Git remote (default: ci)') + commit_src = subparsers.add_parser('commit-source', help='Update database with last commit') commit_src.add_argument('source', help='Source branch name') diff --git a/tools/pickman/control.py b/tools/pickman/control.py index d9d326a0d21..b07ef703ba2 100644 --- a/tools/pickman/control.py +++ b/tools/pickman/control.py @@ -3,6 +3,7 @@ # Copyright 2025 Canonical Ltd. # Written by Simon Glass <simon.glass@canonical.com> # +# pylint: disable=too-many-lines """Control module for pickman - handles the main logic.""" from collections import namedtuple @@ -147,6 +148,44 @@ def do_compare(args, dbs): # pylint: disable=unused-argument return 0 +def do_check_gitlab(args, dbs): # pylint: disable=unused-argument + """Check GitLab permissions for the configured token + + Args: + args (Namespace): Parsed arguments with 'remote' attribute + dbs (Database): Database instance (unused) + + Returns: + int: 0 on success with sufficient permissions, 1 otherwise + """ + remote = args.remote + + perms = gitlab_api.check_permissions(remote) + if not perms: + return 1 + + tout.info(f"GitLab permission check for remote '{remote}':") + tout.info(f" Host: {perms.host}") + tout.info(f" Project: {perms.project}") + tout.info(f" User: {perms.user}") + tout.info(f" Access level: {perms.access_name}") + tout.info('') + tout.info('Permissions:') + tout.info(f" Push branches: {'Yes' if perms.can_push else 'No'}") + tout.info(f" Create MRs: {'Yes' if perms.can_create_mr else 'No'}") + tout.info(f" Merge MRs: {'Yes' if perms.can_merge else 'No'}") + + if not perms.can_create_mr: + tout.warning('') + tout.warning('Insufficient permissions to create merge requests!') + tout.warning('The user needs at least Developer access level.') + return 1 + + tout.info('') + tout.info('All required permissions are available.') + return 0 + + def get_next_commits(dbs, source): """Get the next set of commits to cherry-pick from a source @@ -940,6 +979,7 @@ def do_test(args, dbs): # pylint: disable=unused-argument COMMANDS = { 'add-source': do_add_source, 'apply': do_apply, + 'check-gitlab': do_check_gitlab, 'commit-source': do_commit_source, 'compare': do_compare, 'count-merges': do_count_merges, diff --git a/tools/pickman/ftest.py b/tools/pickman/ftest.py index 66e087e6625..769813122fb 100644 --- a/tools/pickman/ftest.py +++ b/tools/pickman/ftest.py @@ -1480,6 +1480,59 @@ class TestConfigFile(unittest.TestCase): self.assertEqual(value, 'value1') +class TestCheckPermissions(unittest.TestCase): + """Tests for check_permissions function.""" + + @mock.patch.object(gitlab_api, 'get_remote_url') + @mock.patch.object(gitlab_api, 'get_token') + @mock.patch.object(gitlab_api, 'AVAILABLE', True) + def test_check_permissions_developer(self, mock_token, mock_url): + """Test checking permissions for a developer.""" + mock_token.return_value = 'test-token' + mock_url.return_value = 'git@gitlab.com:group/project.git' + + mock_user = mock.MagicMock() + mock_user.username = 'testuser' + mock_user.id = 123 + + mock_member = mock.MagicMock() + mock_member.access_level = 30 # Developer + + mock_project = mock.MagicMock() + mock_project.members.get.return_value = mock_member + + mock_glab = mock.MagicMock() + mock_glab.user = mock_user + mock_glab.projects.get.return_value = mock_project + + with mock.patch('gitlab.Gitlab', return_value=mock_glab): + perms = gitlab_api.check_permissions('origin') + + self.assertIsNotNone(perms) + self.assertEqual(perms.user, 'testuser') + self.assertEqual(perms.access_level, 30) + self.assertEqual(perms.access_name, 'Developer') + self.assertTrue(perms.can_push) + self.assertTrue(perms.can_create_mr) + self.assertFalse(perms.can_merge) + + @mock.patch.object(gitlab_api, 'AVAILABLE', False) + def test_check_permissions_not_available(self): + """Test check_permissions when gitlab not available.""" + with terminal.capture(): + perms = gitlab_api.check_permissions('origin') + self.assertIsNone(perms) + + @mock.patch.object(gitlab_api, 'get_token') + @mock.patch.object(gitlab_api, 'AVAILABLE', True) + def test_check_permissions_no_token(self, mock_token): + """Test check_permissions when no token set.""" + mock_token.return_value = None + with terminal.capture(): + perms = gitlab_api.check_permissions('origin') + self.assertIsNone(perms) + + class TestUpdateMrDescription(unittest.TestCase): """Tests for update_mr_description function.""" diff --git a/tools/pickman/gitlab_api.py b/tools/pickman/gitlab_api.py index d2297f40c93..0db251bd9b8 100644 --- a/tools/pickman/gitlab_api.py +++ b/tools/pickman/gitlab_api.py @@ -427,3 +427,91 @@ def push_and_create_mr(remote, branch, target, title, desc=''): tout.info(f'Merge request created: {mr_url}') return mr_url + + +# Access level constants from GitLab +ACCESS_LEVELS = { + 0: 'No access', + 5: 'Minimal access', + 10: 'Guest', + 20: 'Reporter', + 30: 'Developer', + 40: 'Maintainer', + 50: 'Owner', +} + +# Permission info returned by check_permissions() +PermissionInfo = namedtuple('PermissionInfo', [ + 'user', 'user_id', 'access_level', 'access_name', + 'can_push', 'can_create_mr', 'can_merge', 'project', 'host' +]) + + +def check_permissions(remote): # pylint: disable=too-many-return-statements + """Check GitLab permissions for the current token + + Args: + remote (str): Remote name + + Returns: + PermissionInfo: Permission info, or None on failure + """ + if not check_available(): + return None + + token = get_token() + if not token: + tout.error('No GitLab token configured') + tout.error('Set token in ~/.config/pickman.conf or GITLAB_TOKEN env var') + return None + + remote_url = get_remote_url(remote) + host, proj_path = parse_url(remote_url) + + if not host or not proj_path: + tout.error(f"Could not parse GitLab URL from remote '{remote}'") + return None + + try: + glab = gitlab.Gitlab(f'https://{host}', private_token=token) + glab.auth() + user = glab.user + + project = glab.projects.get(proj_path) + + # Get user's access level in this project + access_level = 0 + try: + # Try to get the member directly + member = project.members.get(user.id) + access_level = member.access_level + except gitlab.exceptions.GitlabGetError: + # User might have inherited access from a group + try: + member = project.members_all.get(user.id) + access_level = member.access_level + except gitlab.exceptions.GitlabGetError: + pass + + access_name = ACCESS_LEVELS.get(access_level, f'Unknown ({access_level})') + + return PermissionInfo( + user=user.username, + user_id=user.id, + access_level=access_level, + access_name=access_name, + can_push=access_level >= 30, # Developer or higher + can_create_mr=access_level >= 30, # Developer or higher + can_merge=access_level >= 40, # Maintainer or higher + project=proj_path, + host=host, + ) + except gitlab.exceptions.GitlabAuthenticationError as exc: + tout.error(f'Authentication failed: {exc}') + return None + except gitlab.exceptions.GitlabGetError as exc: + tout.error(f'Could not access project: {exc}') + return None + except gitlab.exceptions.GitlabError as exc: + tout.error(f'GitLab API error: {exc}') + return None -- 2.43.0