From 86af684b2d7a8afc67ae6de0debe510e67fda10e Mon Sep 17 00:00:00 2001 From: Michel Alexandre Salim Date: Feb 12 2021 02:30:55 +0000 Subject: Allow collaborators to request branches they are allowlisted in If user or group collaborators are granted access to, e.g. "epel*", they should be able to request branches that match this globbing, e.g. "epel9". Pagure has an API endpoint for reporting contributors in the level of detail we need; add a wrapper in `pagure.py` (`get_project_contributors`) and use it in `utils.py`'s `prompt_for_new_branch` Signed-off-by: Michel Alexandre Salim --- diff --git a/fedscm_admin/pagure.py b/fedscm_admin/pagure.py index 0ac04c0..e97ba7c 100644 --- a/fedscm_admin/pagure.py +++ b/fedscm_admin/pagure.py @@ -43,7 +43,7 @@ def get_pagure_auth_header(token_type='ticket'): def get_project(namespace, repo): """ Get an existing Pagure project - :param namespace: a string representing the namespace to create the project + :param namespace: a string representing the namespace of the project in :param repo: a string of the project/repo name :return: a dictionary representing the project or None @@ -60,6 +60,26 @@ def get_project(namespace, repo): return get_request_json(rv, 'getting a project in Pagure') +def get_project_contributors(namespace, repo): + """ + Get an existing Pagure project's contributors + :param namespace: a string representing the namespace of the project + in + :param repo: a string of the project/repo name + :return: a dictionary representing the project or None + """ + pagure_url = get_config_item(CONFIG, 'pagure_dist_git_url') + pagure_project_url = \ + '{0}/api/0/{1}/{2}/contributors'.format(pagure_url.rstrip('/'), namespace, repo) + + rv = requests_wrapper( + pagure_project_url, timeout=60, service_name='Pagure') + if rv.status_code == 404: + return None + else: + return get_request_json(rv, 'getting project contributors in Pagure') + + def user_exists(username): """ Return True if a given pagure user exists :param username: a string of the user's username diff --git a/fedscm_admin/utils.py b/fedscm_admin/utils.py index 067695f..ab5caf5 100644 --- a/fedscm_admin/utils.py +++ b/fedscm_admin/utils.py @@ -17,8 +17,9 @@ Provides helper functions """ from __future__ import absolute_import -import re +import fnmatch import json +import re from datetime import datetime import click @@ -520,8 +521,8 @@ def prompt_for_new_branch(issue_json, issue_body_json, force=False, auto_approve bug_id = str(issue_body_json.get('bug_id', '')).strip() create_git_branch = issue_body_json.get('create_git_branch', True) - project = pagure.get_project(namespace, repo) - if not project: + contributors = pagure.get_project_contributors(namespace, repo) + if not contributors: prompt_to_close_bad_ticket( issue_json, 'The Pagure repo does not exist') return @@ -564,12 +565,18 @@ def prompt_for_new_branch(issue_json, issue_body_json, force=False, auto_approve click.secho(msg, fg='yellow') else: # Get the list of maintainers of the package - maintainers = set(project['access_users']['owner']) | \ - set(project['access_users']['admin']) | \ - set(project['access_users']['commit']) + maintainers = set(contributors['users']['admin']) | \ + set(contributors['users']['commit']) | \ + set(u['user'] + for u in contributors['users']['collaborators'] + if fnmatch.fnmatch(branch_name, u['branches'])) + # Get the list of FAS groups who can maintain the package - access_groups = set(project['access_groups']['admin'])\ - | set(project['access_groups']['commit']) + access_groups = set(contributors['groups']['admin']) | \ + set(contributors['groups']['commit']) | \ + set(g['user'] + for g in contributors['groups']['collaborators'] + if fnmatch.fnmatch(branch_name, g['branches'])) group_member = False for access_group in access_groups: # Check if the requestor is part of any of the FAS groups who can maintain the package diff --git a/tests/mock_values.py b/tests/mock_values.py index 1312965..12395ec 100644 --- a/tests/mock_values.py +++ b/tests/mock_values.py @@ -250,6 +250,40 @@ def get_mock_pagure_project(exists=False): return mock_rv +def get_mock_pagure_project_contributors(exists=False): + mock_rv = Mock() + mock_rv.ok = exists + if exists: + mock_rv.status_code = 200 + mock_rv.json.return_value = { + 'groups': { + 'admin': [], + 'collaborators': [{ + 'branches': 'epel*', + 'user': 'epel-packagers-sig' + }], + 'commit': ['python-sig'], + 'ticket': [] + }, + 'users': { + 'admin': ['salimma'], + 'collaborators': [{ + 'branches': 'epel8', + 'user': 'ngompa' + }], + 'commit': [], + 'ticket': [] + }, + } + else: + mock_rv.status_code = 404 + mock_rv.json.return_value = { + 'error': 'Project not found', + 'error_code': 'ENOPROJECT' + } + return mock_rv + + def get_mock_pagure_git_urls(is_project=False): mock_rv = Mock() mock_rv.ok = True diff --git a/tests/test_admin.py b/tests/test_admin.py index 69db508..5e48bb9 100644 --- a/tests/test_admin.py +++ b/tests/test_admin.py @@ -520,7 +520,7 @@ class FedScmAdmin(TestCase): mock_session.get.side_effect = [ mock_values.get_mock_issue_rv( 'rawhide', ticket_type='new_branch'), - mock_values.get_mock_pagure_project(exists=True), + mock_values.get_mock_pagure_project_contributors(exists=True), mock_values.get_mock_pagure_git_urls(is_project=True), mock_values.get_mock_pdc_branch('rawhide', exists=False) ] @@ -548,7 +548,7 @@ class FedScmAdmin(TestCase): mock_session.get.side_effect = [ mock_values.get_mock_issue_rv( 'rawhide', ticket_type='new_branch'), - mock_values.get_mock_pagure_project(exists=True), + mock_values.get_mock_pagure_project_contributors(exists=True), mock_values.get_mock_pagure_git_urls(is_project=True), mock_values.get_mock_pdc_branch('rawhide', exists=False) ] @@ -646,7 +646,7 @@ class FedScmAdmin(TestCase): mock_session.get.side_effect = [ mock_values.get_mock_issue_rv( 'epel7', ticket_type='new_branch', repo='kernel'), - mock_values.get_mock_pagure_project(exists=True), + mock_values.get_mock_pagure_project_contributors(exists=True), mock_values.get_mock_pagure_git_urls(), mock_values.get_mock_el_check_rv() ] @@ -778,7 +778,7 @@ class FedScmAdmin(TestCase): mock_values.get_mock_pdc_branch('rawhide', exists=False), mock_values.get_mock_pdc_global_component(exists=True), mock_values.get_mock_pdc_branch(None, exists=False), - mock_values.get_mock_pagure_project(exists=True), + mock_values.get_mock_pagure_project_contributors(exists=True), mock_values.get_mock_pagure_git_urls(is_project=True), mock_values.get_mock_pdc_branch(None, exists=False), mock_values.get_mock_pdc_global_component(exists=True), @@ -824,7 +824,7 @@ class FedScmAdmin(TestCase): mock_session.get.side_effect = [ mock_values.get_mock_issue_rv( branch_name, ticket_type='new_branch'), - mock_values.get_mock_pagure_project(exists=True), + mock_values.get_mock_pagure_project_contributors(exists=True), mock_values.get_mock_pagure_git_urls(is_project=True), mock_values.get_mock_pdc_branch(None, exists=False), mock_values.get_mock_pdc_global_component(exists=True),