PR#61: Allow collaborators to request branches they are allowlisted in - fedscm-admin

fedscm-admin

#61 Allow collaborators to request branches they are allowlisted in

Merged 3 years ago by humaton. Opened 3 years ago by salimma.

salimma/fedscm-admin collaborator-branch into main

Allow collaborators to request branches they are allowlisted in

Michel Alexandre Salim • 3 years ago

86af684

fedscm_admin/pagure.py

file modified

+21 -1

		`@@ -43,7 +43,7 @@`
		`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 @@`
		`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`

fedscm_admin/utils.py

file modified

+15 -8

		`@@ -17,8 +17,9 @@`
		`"""`
		`from __future__ import absolute_import`

		`- import re`
		`+ import fnmatch`
		`import json`
		`+ import re`
		`from datetime import datetime`

		`import click`
		`@@ -520,8 +521,8 @@`
		`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 @@`
		`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`

tests/mock_values.py

file modified

+34

		`@@ -250,6 +250,40 @@`
		`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`

tests/test_admin.py

file modified

+5 -5

		`@@ -520,7 +520,7 @@`
		`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 @@`
		`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 @@`
		`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 @@`
		`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 @@`
		`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),`

salimma commented 3 years ago

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 salimma@fedoraproject.org

zuul commented 3 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

tox : FAILURE in 4m 17s

rebased onto 0cfc765c623e3b9e9c3a73393bd2bf4449b2b581

3 years ago

zuul commented 3 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

tox : FAILURE in 5m 51s

rebased onto 8188eef86ea14607f356fc242feff970d5c0e67d

3 years ago

zuul commented 3 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

tox : FAILURE in 5m 15s

rebased onto 8e0153d1c44f2719e53b683bb0bd4c5a69a1540c

3 years ago

zuul commented 3 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

tox : FAILURE in 5m 24s

salimma commented 3 years ago

@pingou not sure what this invocation error is - I can't repro locally as I only have a generic Pagure API token, and on my local machine 29 tests fail with or without my PR due to insufficient privileges

Would appreciate a pointer on what's wrong here

rebased onto 86af684

3 years ago

zuul commented 3 years ago

Build succeeded.