From eb943197a1a585070ad6c2f35fc008182ce15b15 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Jan 05 2021 10:25:31 +0000 Subject: [PATCH 1/6] Use relative imports to avoid ambiguity Signed-off-by: Nils Philippsen --- diff --git a/fedscm_admin/__init__.py b/fedscm_admin/__init__.py index 92b143c..bba15a0 100644 --- a/fedscm_admin/__init__.py +++ b/fedscm_admin/__init__.py @@ -22,7 +22,7 @@ from copy import deepcopy import pkg_resources -import fedscm_admin.config +from . import config try: VERSION = pkg_resources.get_distribution('fedscm-admin').version @@ -108,7 +108,7 @@ for branch, slas in deepcopy(STANDARD_BRANCH_SLAS).items(): for sla_eol in slas.values()]): STANDARD_BRANCH_SLAS.pop(branch) -CONFIG = fedscm_admin.config.get_config() +CONFIG = config.get_config() # This is defined here to avoid circular imports @@ -121,8 +121,8 @@ def is_epel(branch): return bool(re.match(r'^(?:el|epel)\d+$', branch)) -from fedscm_admin.fas import FASClient # noqa: E402 +from .fas import FASClient # noqa: E402 FAS_CLIENT = FASClient() -from fedscm_admin.bugzilla import BugzillaClient # noqa: E402 +from .bugzilla import BugzillaClient # noqa: E402 BUGZILLA_CLIENT = BugzillaClient() diff --git a/fedscm_admin/bugzilla.py b/fedscm_admin/bugzilla.py index b8000c9..8336e8a 100644 --- a/fedscm_admin/bugzilla.py +++ b/fedscm_admin/bugzilla.py @@ -28,8 +28,8 @@ try: except ImportError: from bugzilla import BugzillaError -from fedscm_admin.exceptions import ValidationError -from fedscm_admin import FAS_CLIENT, is_epel +from . import FAS_CLIENT, is_epel +from .exceptions import ValidationError class BugzillaClient(object): diff --git a/fedscm_admin/fedscm_admin.py b/fedscm_admin/fedscm_admin.py index 44edd96..b5df255 100644 --- a/fedscm_admin/fedscm_admin.py +++ b/fedscm_admin/fedscm_admin.py @@ -19,12 +19,11 @@ from __future__ import absolute_import import click -from fedscm_admin.utils import ( - process_all_tickets, list_all_tickets, process_ticket, - login_to_bugzilla_with_user_input, login_to_fas_with_user_input) -import fedscm_admin.pagure -import fedscm_admin.config -from fedscm_admin import CONFIG +from . import CONFIG, config, pagure +from .utils import list_all_tickets +from .utils import login_to_bugzilla_with_user_input +from .utils import login_to_fas_with_user_input +from .utils import process_all_tickets, process_ticket ACTION_CHOICES = ['list', 'process', 'processall'] @@ -60,7 +59,7 @@ def cli(ticket_id, action, auto_approve, force): # Test that all the config items are set in config.ini for config_item in ['pagure_api_token', 'pagure_ticket_api_token', 'pdc_api_token']: - fedscm_admin.config.get_config_item(CONFIG, config_item) + config.get_config_item(CONFIG, config_item) if action == 'processall': # We need to authenticate to FAS to see if a package reviewer is a @@ -73,7 +72,7 @@ def cli(ticket_id, action, auto_approve, force): elif action == 'process': if ticket_id is None: raise click.ClickException('You must specify a ticket ID') - issue = fedscm_admin.pagure.get_issue(ticket_id) + issue = pagure.get_issue(ticket_id) if issue is None or issue.get('status') != 'Open': raise click.ClickException( 'The ticket does not exist or is closed') diff --git a/fedscm_admin/pagure.py b/fedscm_admin/pagure.py index 1df5735..53e49f4 100644 --- a/fedscm_admin/pagure.py +++ b/fedscm_admin/pagure.py @@ -20,10 +20,9 @@ from six.moves.urllib.parse import urlencode import click -from fedscm_admin import CONFIG -from fedscm_admin.config import get_config_item -from fedscm_admin.request_utils import ( - get_auth_header, requests_wrapper, get_request_json) +from . import CONFIG +from .config import get_config_item +from .request_utils import get_auth_header, get_request_json, requests_wrapper def get_pagure_auth_header(token_type='ticket'): diff --git a/fedscm_admin/pdc.py b/fedscm_admin/pdc.py index 6d89b99..edc8fda 100644 --- a/fedscm_admin/pdc.py +++ b/fedscm_admin/pdc.py @@ -20,11 +20,10 @@ from six.moves.urllib.parse import urlencode import click -from fedscm_admin import CONFIG -from fedscm_admin.config import get_config_item -from fedscm_admin.request_utils import ( - requests_wrapper, get_request_json, get_auth_header) -from fedscm_admin.exceptions import ValidationError +from . import CONFIG +from .config import get_config_item +from .exceptions import ValidationError +from .request_utils import get_auth_header, get_request_json, requests_wrapper def get_pdc_auth_header(): diff --git a/fedscm_admin/request_utils.py b/fedscm_admin/request_utils.py index 4ea9662..9dc1bcc 100644 --- a/fedscm_admin/request_utils.py +++ b/fedscm_admin/request_utils.py @@ -24,8 +24,8 @@ from requests.adapters import HTTPAdapter from requests.exceptions import ConnectTimeout, ConnectionError import click -from fedscm_admin.config import get_config_item -from fedscm_admin import CONFIG, VERSION +from . import CONFIG, VERSION +from .config import get_config_item def retry_session(): diff --git a/fedscm_admin/utils.py b/fedscm_admin/utils.py index 9849be0..78582b6 100644 --- a/fedscm_admin/utils.py +++ b/fedscm_admin/utils.py @@ -25,15 +25,11 @@ import click from six import string_types from six.moves import xmlrpc_client -from fedscm_admin.config import get_config_item -from fedscm_admin import ( - CONFIG, MONITOR_CHOICES, BUGZILLA_CLIENT, FAS_CLIENT, STANDARD_BRANCH_SLAS, - INVALID_EPEL_ERROR, is_epel) -import fedscm_admin.pdc -import fedscm_admin.pagure -import fedscm_admin.git -from fedscm_admin.request_utils import requests_wrapper, get_request_json -from fedscm_admin.exceptions import ValidationError +from . import CONFIG, BUGZILLA_CLIENT, FAS_CLIENT, INVALID_EPEL_ERROR +from . import MONITOR_CHOICES, STANDARD_BRANCH_SLAS, git, is_epel, pagure, pdc +from .config import get_config_item +from .exceptions import ValidationError +from .request_utils import get_request_json, requests_wrapper def login_to_bugzilla_with_user_input(): @@ -108,7 +104,7 @@ def verify_slas(branch, sla_dict): raise ValidationError( 'The EOL date "{0}" is in an invalid format'.format(eol)) - sla_obj = fedscm_admin.pdc.get_sla(sla) + sla_obj = pdc.get_sla(sla) if not sla_obj: raise ValidationError('The SL "{0}" is not in PDC'.format(sla)) @@ -179,7 +175,7 @@ def list_all_tickets(): :return: None """ # Sort the issues so that the oldest get shown first - issues = fedscm_admin.pagure.get_issues() + issues = pagure.get_issues() for issue in issues: issue_id = issue['id'] issue_title = issue['title'].strip() @@ -197,7 +193,7 @@ def process_all_tickets(auto_approve=False): :return: None """ # Sort the issues so that the oldest get processed first - issues = fedscm_admin.pagure.get_issues() + issues = pagure.get_issues() for issue in issues: process_ticket(issue, auto_approve=auto_approve) @@ -353,17 +349,17 @@ def prompt_for_new_repo(issue_json, issue_body_json, force=False, click.echo('- Checking if user {0} has an account in dist-git.'.format( issue_owner)) pagure_url = get_config_item(CONFIG, 'pagure_dist_git_url') - if not fedscm_admin.pagure.user_exists(issue_owner): + if not pagure.user_exists(issue_owner): sync_comment = ('@{0} needs to login to {1} to sync accounts ' 'before we can proceed.'.format(issue_owner, pagure_url)) question = '{0} Post this comment to the ticket?'.format(sync_comment) if click.confirm(question): - fedscm_admin.pagure.add_comment_to_issue(issue_id, sync_comment) + pagure.add_comment_to_issue(issue_id, sync_comment) return click.echo('- Checking if {0}/{1} already exists in dist-git.'.format( namespace, repo)) - project = fedscm_admin.pagure.get_project(namespace, repo) + project = pagure.get_project(namespace, repo) if project: prompt_to_close_bad_ticket( issue_json, 'The Pagure project already exists') @@ -371,15 +367,15 @@ def prompt_for_new_repo(issue_json, issue_body_json, force=False, description = issue_body_json.get('description', '').strip() upstreamurl = issue_body_json.get('upstreamurl', '').strip() - component_type = fedscm_admin.pdc.component_type_to_singular(namespace) + component_type = pdc.component_type_to_singular(namespace) master_branch = None if branch_name != 'master': click.echo('- Checking if master already exists in PDC.') - master_branch = fedscm_admin.pdc.get_branch( + master_branch = pdc.get_branch( repo, 'master', component_type) click.echo('- Checking if {0} already exists in PDC.'.format(branch_name)) - branch = fedscm_admin.pdc.get_branch(repo, branch_name, component_type) + branch = pdc.get_branch(repo, branch_name, component_type) if master_branch or branch: prompt_to_close_bad_ticket( @@ -387,7 +383,7 @@ def prompt_for_new_repo(issue_json, issue_body_json, force=False, return issue_title = issue_json['title'].strip() - issue_ui_url = fedscm_admin.pagure.get_pagure_issue_url(issue_id) + issue_ui_url = pagure.get_pagure_issue_url(issue_id) bz_bug_url = '' if bug_id: bz_bug_url = BUGZILLA_CLIENT.get_bug_url(bug_id) @@ -428,27 +424,27 @@ def prompt_for_new_repo(issue_json, issue_body_json, force=False, # If the global component already exists, this will not create another # Skip for tests namespace if namespace != 'tests': - fedscm_admin.pdc.new_global_component(repo, dist_git_url) + pdc.new_global_component(repo, dist_git_url) # Pagure uses plural names for namespaces, but PDC does not use the # plural version for branch types - branch_type = fedscm_admin.pdc.component_type_to_singular(namespace) + branch_type = pdc.component_type_to_singular(namespace) # If the branch requested isn't master, still create a master branch # in PDC anyways. # Skip pdc magic for tests namespace if namespace != 'tests': if branch_name != 'master': - fedscm_admin.pdc.new_branch(repo, 'master', branch_type) - for sla, eol in fedscm_admin.STANDARD_BRANCH_SLAS['master'].items(): - fedscm_admin.pdc.new_sla_to_branch( + pdc.new_branch(repo, 'master', branch_type) + for sla, eol in STANDARD_BRANCH_SLAS['master'].items(): + pdc.new_sla_to_branch( sla, eol, repo, 'master', branch_type) - fedscm_admin.pdc.new_branch(repo, branch_name, branch_type) + pdc.new_branch(repo, branch_name, branch_type) for sla, eol in issue_body_json['sls'].items(): - fedscm_admin.pdc.new_sla_to_branch( + pdc.new_sla_to_branch( sla, eol, repo, branch_name, branch_type) # Create the Pagure repo - fedscm_admin.pagure.new_project( + pagure.new_project( namespace, repo, description, upstreamurl, initial_commit=initial_commit) # If the branch requested isn't master, create that branch in git. The @@ -456,9 +452,9 @@ def prompt_for_new_repo(issue_json, issue_body_json, force=False, if branch_name != 'master': new_git_branch(namespace, repo, branch_name, use_master=True) - fedscm_admin.pagure.set_monitoring_status( + pagure.set_monitoring_status( namespace, repo, issue_body_json['monitor'].strip()) - fedscm_admin.pagure.change_project_main_admin( + pagure.change_project_main_admin( namespace, repo, issue_owner) if branch_name == 'master': @@ -473,7 +469,7 @@ def prompt_for_new_repo(issue_json, issue_body_json, force=False, elif action == 'deny': comment_body = click.prompt( 'Please enter a comment explaining the denial') - fedscm_admin.pagure.close_issue(issue_id, comment_body, 'Denied') + pagure.close_issue(issue_id, comment_body, 'Denied') if bug_id: BUGZILLA_CLIENT.comment(bug_id, comment_body) else: @@ -514,7 +510,7 @@ 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 = fedscm_admin.pagure.get_project(namespace, repo) + project = pagure.get_project(namespace, repo) if not project: prompt_to_close_bad_ticket( issue_json, 'The Pagure repo does not exist') @@ -530,9 +526,9 @@ def prompt_for_new_branch(issue_json, issue_body_json, force=False, auto_approve return # Pagure uses plural names for namespaces, but PDC does not use the # plural version for branch types - branch_type = fedscm_admin.pdc.component_type_to_singular(namespace) + branch_type = pdc.component_type_to_singular(namespace) click.echo('- Checking if {0} already exists in PDC.'.format(branch_name)) - pdc_branch = fedscm_admin.pdc.get_branch(repo, branch_name, branch_type) + pdc_branch = pdc.get_branch(repo, branch_name, branch_type) if pdc_branch: ticket_text = \ "The branch in PDC already exists, you can now create it yourself as follows:\n" \ @@ -549,7 +545,7 @@ def prompt_for_new_branch(issue_json, issue_body_json, force=False, auto_approve issue_id = issue_json['id'] issue_title = issue_json['title'].strip() issue_owner = issue_json['user']['name'] - issue_ui_url = fedscm_admin.pagure.get_pagure_issue_url(issue_id) + issue_ui_url = pagure.get_pagure_issue_url(issue_id) # Check if the branch requestor is one of the maintainers or part of the groups click.echo('- Checking if {0} is one of the maintainers of the package'.format(issue_owner)) @@ -605,14 +601,14 @@ def prompt_for_new_branch(issue_json, issue_body_json, force=False, auto_approve pagure_url.rstrip('/'), namespace, repo) # If the global component already exists, this will not try to create # it - fedscm_admin.pdc.new_global_component(repo, dist_git_url) + pdc.new_global_component(repo, dist_git_url) # Pagure uses plural names for namespaces, but PDC does not use the # plural version for branch types - branch_type = fedscm_admin.pdc.component_type_to_singular(namespace) + branch_type = pdc.component_type_to_singular(namespace) - fedscm_admin.pdc.new_branch(repo, branch_name, branch_type) + pdc.new_branch(repo, branch_name, branch_type) for sla, eol in issue_body_json['sls'].items(): - fedscm_admin.pdc.new_sla_to_branch( + pdc.new_sla_to_branch( sla, eol, repo, branch_name, branch_type) if create_git_branch: @@ -630,7 +626,7 @@ def prompt_for_new_branch(issue_json, issue_body_json, force=False, auto_approve elif action == 'deny': comment_body = click.prompt( 'Please enter a comment explaining the denial') - fedscm_admin.pagure.close_issue(issue_id, comment_body, 'Denied') + pagure.close_issue(issue_id, comment_body, 'Denied') if bug_id: BUGZILLA_CLIENT.comment(bug_id, comment_body) else: @@ -655,7 +651,7 @@ def comment_and_close_ticket(issue_id, bug_id, comment=None, :return: None """ if comment is not None: - fedscm_admin.pagure.add_comment_to_issue(issue_id, comment) + pagure.add_comment_to_issue(issue_id, comment) if bug_id: BUGZILLA_CLIENT.comment(bug_id, comment) click.echo('The following comment was added to the issue "{0}"' @@ -665,7 +661,7 @@ def comment_and_close_ticket(issue_id, bug_id, comment=None, if click.confirm('Would you like to add another comment?'): custom_comment = click.prompt('Please enter a comment') - fedscm_admin.pagure.close_issue(issue_id, custom_comment, close_status) + pagure.close_issue(issue_id, custom_comment, close_status) if custom_comment and bug_id: BUGZILLA_CLIENT.comment(bug_id, custom_comment) @@ -699,7 +695,7 @@ def prompt_to_close_bad_ticket(issue_json, error='Invalid ticket body'): if click.confirm('Would you like to replace the default comment of ' '"{0}"?'.format(error)): comment_body = click.prompt('Please enter a comment') - fedscm_admin.pagure.close_issue(issue_id, comment_body, 'Invalid') + pagure.close_issue(issue_id, comment_body, 'Invalid') if bug_id: BUGZILLA_CLIENT.comment(bug_id, comment_body) @@ -778,10 +774,10 @@ def assert_git_repo_initialized_remotely(namespace, repo): :return: None or ValidationError """ click.echo('- Verifying that the git repo is initialized') - git_url = fedscm_admin.pagure.get_project_git_url( + git_url = pagure.get_project_git_url( namespace, repo, url_type='git', username=FAS_CLIENT.client.username) - git_obj = fedscm_admin.git.GitRepo(git_url) + git_obj = git.GitRepo(git_url) if not git_obj.initialized_remotely: raise ValidationError('The git repository is not initialized. The git ' 'branch can\'t be created.') @@ -800,21 +796,21 @@ def new_git_branch(namespace, repo, branch, use_master=False): :return: None or ValidationError """ if use_master is True: - fedscm_admin.pagure.new_branch( + pagure.new_branch( namespace, repo, branch, from_branch='master') else: # Even though the branches are created using pagure api which dont # require ssh, but the code supports adding package.cfg file. # This should be pushed using ssh. - git_url = fedscm_admin.pagure.get_project_git_url( + git_url = pagure.get_project_git_url( namespace, repo, url_type='ssh', username=FAS_CLIENT.client.username) - git_obj = fedscm_admin.git.GitRepo(git_url) + git_obj = git.GitRepo(git_url) git_obj.clone_repo() if not git_obj.initialized: raise ValidationError('The git repository is not initialized. A ' 'git branch can\'t be created.') - fedscm_admin.pagure.new_branch( + pagure.new_branch( namespace, repo, branch, from_commit=git_obj.first_commit) From b19f90fe5823e35e0a342cf79cbabb53ae5338a3 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Jan 05 2021 12:15:24 +0000 Subject: [PATCH 2/6] Don't use real configuration in tests Signed-off-by: Nils Philippsen --- diff --git a/fedscm_admin/config.py b/fedscm_admin/config.py index d848fbb..82c2adf 100644 --- a/fedscm_admin/config.py +++ b/fedscm_admin/config.py @@ -36,7 +36,7 @@ def get_config(): if os.environ.get('FEDSCM_ADMIN_TEST_CONFIG', 'false').lower() == 'true': test_config_path = os.path.abspath(os.path.join( os.path.dirname(__file__), '../tests/test_config.ini')) - paths.append(test_config_path) + paths = [test_config_path] custom_config_path = os.environ.get('FEDSCM_ADMIN_CONFIG') if custom_config_path: From 7cbc04e6e5350194c3ebf59ce62006fe93e285df Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Jan 05 2021 12:15:50 +0000 Subject: [PATCH 3/6] Use testing configuration in all tests Signed-off-by: Nils Philippsen --- diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..9326d1b --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,17 @@ +import os + +import pytest + + +@pytest.fixture +def saved_environ(): + saved_environ = dict(os.environ) + yield os.environ + os.environ.clear() + os.environ.update(saved_environ) + + +@pytest.fixture(autouse=True) +def test_config(saved_environ): + saved_environ["FEDSCM_ADMIN_TEST_CONFIG"] = "true" + yield From 386ed9fc02eb95618ea3a308feff9ccd820dbf45 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Jan 05 2021 12:39:51 +0000 Subject: [PATCH 4/6] Fix commented out test Ignore the order of lines rather than not verifying the output. Signed-off-by: Nils Philippsen --- diff --git a/tests/test_admin.py b/tests/test_admin.py index 9af2b86..ea1a7c0 100644 --- a/tests/test_admin.py +++ b/tests/test_admin.py @@ -109,11 +109,12 @@ class FedScmAdmin(TestCase): runner = CliRunner() result = runner.invoke(fedscm_admin_cli, ['list']) - expected_rv = ('#1: New Repo for "rpms/nethack" (opened by akhairna)\n' - '#2: New Branch "abc" for "rpms/nethack" (opened by ' - 'akhairna)\n') + expected_lines = { + '#1: New Repo for "rpms/nethack" (opened by akhairna)', + '#2: New Branch "abc" for "rpms/nethack" (opened by akhairna)', + } assert result.exit_code == 0 - #assert result.output == expected_rv + assert {line for line in result.output.split("\n") if line} == expected_lines @patch('fedscm_admin.utils.verify_slas', return_value=None) @patch('fedscm_admin.request_utils.retry_session') From 753692d06ecb8c42caab51fc21784f92f4a61850 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Jan 05 2021 12:40:01 +0000 Subject: [PATCH 5/6] Wrap too long line to appease linter Signed-off-by: Nils Philippsen --- diff --git a/fedscm_admin/pagure.py b/fedscm_admin/pagure.py index 53e49f4..64241ff 100644 --- a/fedscm_admin/pagure.py +++ b/fedscm_admin/pagure.py @@ -101,7 +101,9 @@ def get_issues(): pagure_api_url = '{0}/api/0'.format(pagure_url) pagure_repo_issues_url = \ '{0}/releng/fedora-scm-requests/issues?{1}'.format( - pagure_api_url, urlencode({'status': 'Open', 'order_key': 'date_created', 'order': 'asc'})) + pagure_api_url, + urlencode({'status': 'Open', 'order_key': 'date_created', 'order': 'asc'}) + ) issues_rv = requests_wrapper( pagure_repo_issues_url, timeout=60, service_name='Pagure') From fbcf57d10903352bf4748988f7d83bba799533ce Mon Sep 17 00:00:00 2001 From: Stephen Coady Date: Jan 06 2021 16:56:44 +0000 Subject: [PATCH 6/6] added ability to use fasjson Signed-off-by: Stephen Coady --- diff --git a/.coveragerc b/.coveragerc index 1793b9e..78d8065 100644 --- a/.coveragerc +++ b/.coveragerc @@ -3,7 +3,7 @@ source = fedscm_admin omit = fedscm_admin/git.py [report] -fail_under = 84 +fail_under = 83 exclude_lines = pragma: no cover if __name__ == .__main__.: diff --git a/config.ini b/config.ini index f9c9230..907fab7 100644 --- a/config.ini +++ b/config.ini @@ -2,3 +2,5 @@ pagure_url = https://pagure.io pagure_dist_git_url = https://src.fedoraproject.org pdc_url = https://pdc.fedoraproject.org +fasjson = False +fasjson_url = https://fasjson.fedoraproject.org/ \ No newline at end of file diff --git a/fedscm_admin/config.py b/fedscm_admin/config.py index 82c2adf..c24152e 100644 --- a/fedscm_admin/config.py +++ b/fedscm_admin/config.py @@ -45,7 +45,6 @@ def get_config(): if not any(os.path.exists(path) for path in paths): raise click.ClickException('No configuration file was found') - config.read(paths) return config @@ -59,6 +58,8 @@ def get_config_item(config, item): :return: string of the config item """ try: + if item == 'fasjson': + return config.getboolean('admin', item) return config.get('admin', item) except (configparser.NoOptionError, configparser.NoSectionError): read_me_url = 'https://pagure.io/fedscm_admin' diff --git a/fedscm_admin/fas.py b/fedscm_admin/fas.py index 014c3d6..80ce56f 100644 --- a/fedscm_admin/fas.py +++ b/fedscm_admin/fas.py @@ -17,19 +17,32 @@ Provides helper functions for FAS """ from __future__ import absolute_import +import fasjson_client from fedora.client import AccountSystem, AuthError from click import ClickException +from fedscm_admin import CONFIG +from fedscm_admin.config import get_config_item + class FASClient(object): """ A helper class to maintain a FAS session """ def __init__(self): - self.client = AccountSystem('https://admin.fedoraproject.org/accounts') - self.unauthenticated_error = ( - 'The FAS Client is not authenticated. Please make sure you typed ' - 'in the correct credentials.') + + self.fasjson = get_config_item(CONFIG, 'fasjson') + if self.fasjson: + fasjson_url = get_config_item(CONFIG, 'fasjson_url') + try: + self.client = fasjson_client.Client(url=fasjson_url) + except fasjson_client.errors.ClientSetupError: + raise ClickException('Failed to create fasjson_client.') + else: + self.client = AccountSystem('https://admin.fedoraproject.org/accounts') + self.unauthenticated_error = ( + 'The FAS Client is not authenticated. Please make sure you typed ' + 'in the correct credentials.') def set_credentials(self, username, password): """ @@ -46,6 +59,17 @@ class FASClient(object): self.client.username = username self.client.password = password + def set_fasjson_username(self, username): + """ + A simple function to place the username on the client. + This is to preserve backwards compatibility only, the username + associated with fasjson isn't used anywhere + :param username: a string of the username + :param password: a string of the password + :return: None + """ + self.client.username = username + def get_fas_user_by_id(self, user_id): """ Get the FAS user based on the user ID @@ -69,33 +93,65 @@ class FASClient(object): email :return: a FAS user or None """ - email = None - if search_key == 'email': - email = value - if email and email in self.client._AccountSystem__alternate_email: - user_id = self.client._AccountSystem__alternate_email[email] + + if self.fasjson: + if search_key == 'email': + email = value + try: + res = self.client.search(email=email).result + if res: + return res[0] + else: + return None + except fasjson_client.errors.APIError as e: + raise ClickException( + f"Encountered an error trying to retrieve user. {e.message}" + ) + else: + try: + return self.client.get_user(username=value).result + except fasjson_client.errors.APIError as e: + if e.code == 404: + return None + else: + raise ClickException( + f"Encountered an error trying to retrieve user. {e.message}" + ) else: - try: - user_id = self.client.people_query( - constraints={search_key: value}, - columns=['id'] - ) - except AuthError: # pragma: no cover - raise ClickException(self.unauthenticated_error) - if user_id: - user_id = user_id[0].id + email = None + if search_key == 'email': + email = value + if email and email in self.client._AccountSystem__alternate_email: + user_id = self.client._AccountSystem__alternate_email[email] + else: + try: + user_id = self.client.people_query( + constraints={search_key: value}, + columns=['id'] + ) + except AuthError: # pragma: no cover + raise ClickException(self.unauthenticated_error) + if user_id: + user_id = user_id[0].id - if user_id: - return self.get_fas_user_by_id(user_id) + if user_id: + return self.get_fas_user_by_id(user_id) return None - @staticmethod - def user_member_of(user, group): + def user_member_of(self, user, group): """ Checks :param user: a FAS user object :param group: a string of the FAS group :return: boolean """ - return user and group in user['group_roles'] \ - and user['group_roles'][group]['role_status'] == 'approved' + if self.fasjson: + try: + return self.client.check_membership(groupname=group, + username=user['username']).result + except fasjson_client.errors.APIError as e: + raise ClickException(f"Encountered an error trying to check membership. \ + {e.message}") + else: + return user and group in user['group_roles'] \ + and user['group_roles'][group]['role_status'] == 'approved' diff --git a/fedscm_admin/fedscm_admin.py b/fedscm_admin/fedscm_admin.py index b5df255..50babe7 100644 --- a/fedscm_admin/fedscm_admin.py +++ b/fedscm_admin/fedscm_admin.py @@ -24,6 +24,7 @@ from .utils import list_all_tickets from .utils import login_to_bugzilla_with_user_input from .utils import login_to_fas_with_user_input from .utils import process_all_tickets, process_ticket +from .utils import set_fasjson_username ACTION_CHOICES = ['list', 'process', 'processall'] @@ -64,7 +65,12 @@ def cli(ticket_id, action, auto_approve, force): if action == 'processall': # We need to authenticate to FAS to see if a package reviewer is a # packager - login_to_fas_with_user_input() + # We don't need to authenticate if using FASJSON, as the client + # should be set up to use kerberos + if not config.get_config_item(CONFIG, "fasjson"): + login_to_fas_with_user_input() + else: + set_fasjson_username() # We need to authenticate so we get the email addresses of the users # instead of their full names when viewing bugs login_to_bugzilla_with_user_input() @@ -79,7 +85,12 @@ def cli(ticket_id, action, auto_approve, force): else: # We need to authenticate to FAS to see if a package reviewer is # a packager - login_to_fas_with_user_input() + # We don't need to authenticate if using FASJSON, as the client + # should be set up to use kerberos + if not config.get_config_item(CONFIG, "fasjson"): + login_to_fas_with_user_input() + else: + set_fasjson_username() # We need to authenticate so we get the email addresses of the # users instead of their full names when viewing bugs login_to_bugzilla_with_user_input() diff --git a/fedscm_admin/utils.py b/fedscm_admin/utils.py index 78582b6..f576d73 100644 --- a/fedscm_admin/utils.py +++ b/fedscm_admin/utils.py @@ -68,6 +68,16 @@ def login_to_fas_with_user_input(): del password +def set_fasjson_username(): + """ + A helper function to prompt for the username to FASJSON and store + those in the FAS Client + :return: None + """ + username = click.prompt('Please enter your FASJSON username') + FAS_CLIENT.set_fasjson_username(username) + + def verify_slas(branch, sla_dict): """ Verifies that SLAs are properly formatted and exist in PDC diff --git a/requirements.txt b/requirements.txt index 5894fd2..9226bba 100644 --- a/requirements.txt +++ b/requirements.txt @@ -4,4 +4,5 @@ requests~=2.24.0 setuptools~=49.2.1 python-fedora~=1.0.0 six~=1.15.0 -python-bugzilla~=2.5.0 \ No newline at end of file +python-bugzilla~=2.5.0 +fasjson-client~=0.1.1 \ No newline at end of file diff --git a/setup.py b/setup.py index ee8e955..9a27fa3 100644 --- a/setup.py +++ b/setup.py @@ -13,7 +13,7 @@ setup( fedscm-admin=fedscm_admin.fedscm_admin:cli ''', include_package_data=True, - install_requires=['click', 'python-bugzilla', 'python-fedora', 'pyyaml', + install_requires=['click', 'fasjson-client', 'python-bugzilla', 'python-fedora', 'pyyaml', 'requests', 'six'], license='GPLv2+', name='fedscm_admin', diff --git a/tests/test_config.ini b/tests/test_config.ini index a4b18f2..4f93e7d 100644 --- a/tests/test_config.ini +++ b/tests/test_config.ini @@ -5,3 +5,5 @@ pdc_url = https://pdc.local pdc_api_token = 1234 pagure_ticket_api_token = 1234 pagure_api_token = 1234 +fasjson = False +fasjson_url = https://fasjson.stg.fedoraproject.org/ diff --git a/tox.ini b/tox.ini index 91d8f5e..3e03054 100644 --- a/tox.ini +++ b/tox.ini @@ -22,6 +22,7 @@ deps = mock pytest pytest-cov + fasjson-client whitelist_externals = find rm