From a4b94d57c9fc3e63a57243d4051423795e3d47e9 Mon Sep 17 00:00:00 2001 From: amedvede Date: Jul 29 2024 06:29:04 +0000 Subject: feat: added request-unretirement command Signed-off-by: amedvede --- diff --git a/fedpkg/cli.py b/fedpkg/cli.py index d6c7715..75b8c88 100644 --- a/fedpkg/cli.py +++ b/fedpkg/cli.py @@ -21,11 +21,9 @@ import re import shutil import textwrap from datetime import datetime -# Use deprecated pkg_resources if importlib isn't available (python 3.6) -try: - import importlib.metadata -except ImportError: - import pkg_resources + +import pkg_resources +import requests import six from pyrpkg import rpkgError from pyrpkg.cli import cliClient @@ -41,9 +39,11 @@ from fedpkg.utils import (assert_new_tests_repo, assert_valid_epel_package, do_fork, expand_release, get_dist_git_url, get_fedora_release_state, get_pagure_branches, get_release_branches, get_stream_branches, is_epel, - new_pagure_issue, sl_list_to_dict, verify_sls) + new_pagure_issue, sl_list_to_dict, verify_sls, + get_user_groups, get_last_commit_date) RELEASE_BRANCH_REGEX = r'^(f\d+|el\d+|epel\d+)$' +BUGZILLA_URL_REGEX = r"https:\/\/bugzilla\.redhat\.com\/show_bug\.cgi\?id=(\d{7})" LOCAL_PACKAGE_CONFIG = 'package.cfg' BODHI_TEMPLATE = """\ @@ -95,17 +95,10 @@ require_testcases=%(require_testcases)s def check_bodhi_version(): - # Use deprecated pkg_resources if importlib isn't available (python 3.6) try: - try: - importlib.metadata.distribution('bodhi_client') - except importlib.metadata.PackageNotFoundError: - raise rpkgError('bodhi-client < 2.0 is not supported.') - except NameError: - try: - pkg_resources.get_distribution('bodhi_client') - except pkg_resources.DistributionNotFound: - raise rpkgError('bodhi-client < 2.0 is not supported.') + pkg_resources.get_distribution('bodhi_client') + except pkg_resources.DistributionNotFound: + raise rpkgError('bodhi-client < 2.0 is not supported.') class fedpkgClient(cliClient): @@ -136,6 +129,7 @@ class fedpkgClient(cliClient): self.register_request_repo() self.register_request_tests_repo() self.register_request_branch() + self.register_request_unretirement() self.register_do_fork() self.register_override() self.register_set_distgit_token() @@ -487,8 +481,8 @@ class fedpkgClient(cliClient): request_branch_parser.add_argument( '--no-auto-module', default=False, action='store_true', help='If requesting an rpm arbitrary branch, do not ' - 'also request a new matching module. See ' - 'https://pagure.io/fedrepo_req/issue/129' + 'also request a new matching module. See ' + 'https://pagure.io/fedrepo_req/issue/129' ) request_branch_parser.add_argument( '--all-releases', default=False, action='store_true', @@ -496,6 +490,69 @@ class fedpkgClient(cliClient): ) request_branch_parser.set_defaults(command=self.request_branch) + def register_request_unretirement(self): + help_msg = "Request an unretirement of package branch" + # TODO improve description + description = textwrap.dedent(""" + Request an unretirement of package branch + + {0} request-unretirement + + Below are various examples of requesting an unretirement of package branch. + + Request an unretirement of few branches: + + {0} request-unretirement -b rawhide f32 + + Request an unretirement of branch that was retired more than 8 weeks ago + and requires bugzilla review to proceed: + + {0} request-unretirement --bz_url some_url.com + + Request an unretirement of package with different namespace than `rpms`: + + {0} request-unretirement -n test + + Request an unretirement of not actual package: + + {0} request-unretirement --repo name_of_package + """.format(self.name)) + request_unretirement_parser = self.subparsers.add_parser( + "request-unretirement", + formatter_class=argparse.RawTextHelpFormatter, + help=help_msg, + description=description, + ) + request_unretirement_parser.add_argument( + '--repo', + required=False, + dest='repo_name', + metavar='NAME', + help='Repository name to unretire some branches in.' + ) + request_unretirement_parser.add_argument( + '--namespace', + required=False, + dest='repo_ns_name', + default='rpms', + help='Repository namespace name, as a default use `rpm`.', + ) + request_unretirement_parser.add_argument( + "--bz_url", + required=False, + dest="bz_url", + default=None, + help="Bugzilla URL with re-review." + ) + request_unretirement_parser.add_argument( + "-b", "--branches", + required=False, + nargs='+', + dest="branches", + help="Comma-separated list of branches for unretirement." + ) + request_unretirement_parser.set_defaults(command=self.request_unretirement) + def register_do_fork(self): help_msg = 'Create a new fork of the current repository' distgit_section = '{0}.distgit'.format(self.name) @@ -569,8 +626,8 @@ class fedpkgClient(cliClient): 'Updates the fedpkg.distgit API token in ~/.config/rpkg/{0}.conf file.\n\n\ Tokens are of length 64 and contain only uppercase and numerical values.\n\ The new API token can be generated at: \n\ - https://{1}/settings/token/new'\ - .format(self.name, urlparse(distgit_api_base_url).netloc) + https://{1}/settings/token/new' \ + .format(self.name, urlparse(distgit_api_base_url).netloc) parser = self.subparsers.add_parser( 'set-distgit-token', @@ -586,8 +643,8 @@ class fedpkgClient(cliClient): 'Updates the fedpkg.pagure API token in ~/.config/rpkg/{0}.conf file.\n\n\ Tokens are of length 64 and contain only uppercase and numerical values.\n\ The new API token. Can be generated at: \n\ - https://{1}/settings/token/new'\ - .format(self.name, urlparse(pagure_url).netloc) + https://{1}/settings/token/new' \ + .format(self.name, urlparse(pagure_url).netloc) parser = self.subparsers.add_parser( 'set-pagure-token', @@ -1193,7 +1250,7 @@ class fedpkgClient(cliClient): if not (branch or all_releases) and active_branch: branch = active_branch - bodhi_url = config.get('{0}.bodhi'.format(name), 'url') + pdc_url = config.get('{0}.pdc'.format(name), 'url') if branch: if is_epel(branch): assert_valid_epel_package(repo_name, branch) @@ -1206,7 +1263,7 @@ class fedpkgClient(cliClient): 'underscores, and pluses are allowed in {0} branch ' 'names'.format('flatpak' if ns == 'flatpaks' else 'module')) release_branches = list(itertools.chain( - *list(get_release_branches(bodhi_url).values()))) + *list(get_release_branches(pdc_url).values()))) # treat epel*-next the same as epel* release branches next_match = re.match(r'^(epel\d+)-next$', branch) @@ -1217,14 +1274,14 @@ class fedpkgClient(cliClient): # If service levels were provided, verify them if service_levels: sl_dict = sl_list_to_dict(service_levels) - verify_sls(sl_dict) + verify_sls(pdc_url, sl_dict) pagure_section = '{0}.pagure'.format(name) pagure_url = config_get_safely(config, pagure_section, 'url') pagure_token = config_get_safely(config, pagure_section, 'token') if all_releases: release_branches = list(itertools.chain( - *list(get_release_branches(bodhi_url).values()))) + *list(get_release_branches(pdc_url).values()))) branches = [b for b in release_branches if re.match(r'^(f\d+)$', b)] else: @@ -1248,10 +1305,10 @@ class fedpkgClient(cliClient): # check whether the requested branch was already created if b not in get_pagure_branches( - logger=logger, - url=get_dist_git_url(anongiturl), - namespace=ns, - repo_name=repo_name + logger=logger, + url=get_dist_git_url(anongiturl), + namespace=ns, + repo_name=repo_name ): print(new_pagure_issue( logger, pagure_url, pagure_token, ticket_title, ticket_body, @@ -1263,10 +1320,10 @@ class fedpkgClient(cliClient): # For non-standard rpm branch requests, also request a matching new # module repo with a matching branch. auto_module = ( - ns == 'rpms' - and not re.match(RELEASE_BRANCH_REGEX, b) - and not next_match # Dont run auto_module on epel-next requests - and not no_auto_module + ns == 'rpms' + and not re.match(RELEASE_BRANCH_REGEX, b) + and not next_match # Dont run auto_module on epel-next requests + and not no_auto_module ) if auto_module: summary = ('Automatically requested module for ' @@ -1300,6 +1357,133 @@ class fedpkgClient(cliClient): anongiturl=anongiturl, ) + def request_unretirement(self): + if self.args.repo_name: + self.cmd.repo_name = self.args.repo_name + self.cmd.ns = self.args.repo_ns_name + + try: + if not self.args.branches: + active_branch = self.cmd.repo.active_branch.name + branches = [active_branch] + else: + branches = self.args.branches + except rpkgError: + branches = ["rawhide"] + + self._request_unretirement( + logger=self.log, + repo_name=self.cmd.repo_name, + ns=self.cmd.ns, + branches=branches, + bugzila_url=self.args.bz_url, + fas_name=self.cmd.user, + name=self.name, + config=self.config, + ) + + @staticmethod + def _request_unretirement( + logger, repo_name, ns, branches, bugzila_url, fas_name, name, config + ): + """ Implementation of `request_unretirement`. + + Submits a request for a unretretirement of package branch. + + :param logger: A logger object. + :param repo_name: The string of the repo name. + :param ns: The string of pacakge namespace. + :param branches: The list of branches that need to be unretired. + :param bugzila_url: The URL of the bugzilla review. + Typically, the value of `self.args.bz_url`, None if not needed. + :param fas_name: The string of fas name of user. + Typically value is `self.cmd.user`. + :param name: A string representing which section of the config should be + used. Typically, the value of `self.name`. + :param config: A dict containing the configuration, loaded from file. + Typically, the value of `self.config`. + """ + + # TODO: add function description + # TODO: write a check for input parameters + # don't know what should be here yet + + # decide if ticket is require bugzilla url + def get_date_diff(today_date, commit_date): + commit_date = datetime.fromtimestamp(int(commit_date)) + date_diff = today_date - commit_date + return date_diff.days + + distgit_section = '{0}.distgit'.format(name) + distgit_url = config_get_safely(config, distgit_section, 'apibaseurl') + today_date = datetime.now() + bugzilla_need = False + + for branch in branches: + commit_date = get_last_commit_date(distgit_url, ns, repo_name, branch) + date_diff = get_date_diff(today_date, commit_date) + if date_diff > 56: # 56 days is 8 weeks + bugzilla_need = True + if bugzila_url is None: + raise rpkgError( + "Bugzilla url should be provided," + "because last commit was made more than 8 weeks ago." + ) + + # is user in packager group + is_user_a_packager = False + user_groups = get_user_groups(fas_name) + for group in user_groups: + if group['groupname'] == 'packager': + is_user_a_packager = True + break + if not is_user_a_packager: + raise rpkgError("The user should be in `packager` group.") + + # is bug has fedora-review+ flag + def get_bug_id(bugzilla_url): + match = re.search(BUGZILLA_URL_REGEX, bugzila_url) + if match: + bug_id = match.group(1) + return bug_id + else: + raise rpkgError("Something is not right in bugzilla url. " + "Try to make it looks like this: " + "`https://bugzilla.redhat.com/show_bug.cgi?id=2260849`") + + if bugzilla_need: + bug_id = get_bug_id(bugzila_url) + bz_url = config.get('{0}.bugzilla'.format(name), 'url') + bz_client = BugzillaClient(bz_url) + # get_review_bug checks if the bug has fedora_review+ flag + bz_client.get_review_bug(bug_id, ns, repo_name) + + # Ticket creation + pagure_section = '{0}.pagure'.format(name) + pagure_url = config_get_safely(config, pagure_section, 'url') + pagure_token = config_get_safely(config, pagure_section, 'token') + + ticket_body = { + 'name': repo_name, + 'type': ns, + 'branches': branches, + 'review_bugzilla': bugzila_url, + } + ticket_body = json.dumps(ticket_body, indent=True) + ticket_body = '```\n{0}\n```'.format(ticket_body) + ticket_title = 'Unretire {0}'.format(repo_name) + + print( + new_pagure_issue( + logger=logger, + url=pagure_url, + token=pagure_token, + title=ticket_title, + body=ticket_body, + cli_name=name, + ) + ) + def do_distgit_fork(self): """create fork of the distgit repository That includes creating fork itself using API call and then adding @@ -1414,23 +1598,25 @@ class fedpkgClient(cliClient): :raises rpkgError: if branch is a stream branch but it is inactive. """ for branch_info in stream_branches: - if branch_info == name: + if branch_info['name'] != name: + continue + if branch_info['active']: return True + else: + raise rpkgError('Cannot build from stream branch {0} as it is ' + 'inactive.'.format(name)) return False def _build(self, sets=None): if hasattr(self.args, 'chain') or self.args.scratch: return super(fedpkgClient, self)._build(sets) - server_url = self.config.get('{0}.bodhi'.format(self.name), 'url') - distgit_section = '{0}.distgit'.format(self.name) - apibaseurl = config_get_safely(self.config, distgit_section, "apibaseurl") - logger = self.log + server_url = self.config.get('{0}.pdc'.format(self.name), 'url') - stream_branches = get_stream_branches(server_url, self.cmd.repo_name, apibaseurl, logger) + stream_branches = get_stream_branches(server_url, self.cmd.repo_name) self.log.debug( 'Package %s has stream branches: %r', - self.cmd.repo_name, [item for item in stream_branches]) + self.cmd.repo_name, [item['name'] for item in stream_branches]) if not self.is_stream_branch(stream_branches, self.cmd.branch_merge): return super(fedpkgClient, self)._build(sets) @@ -1459,7 +1645,7 @@ class fedpkgClient(cliClient): return task_ids def show_releases_info(self): - server_url = self.config.get('{0}.bodhi'.format(self.name), 'url') + server_url = self.config.get('{0}.pdc'.format(self.name), 'url') releases = get_release_branches(server_url) def _join(ln): diff --git a/fedpkg/utils.py b/fedpkg/utils.py index 6f7d2be..98f2965 100644 --- a/fedpkg/utils.py +++ b/fedpkg/utils.py @@ -13,9 +13,11 @@ import json import re from datetime import datetime, timezone +import tempfile import git import requests +import fasjson_client from pyrpkg import rpkgError from requests.exceptions import ConnectionError from six.moves.configparser import NoOptionError, NoSectionError @@ -73,7 +75,7 @@ def new_pagure_issue(logger, url, token, title, body, cli_name): except ConnectionError as error: error_msg = ('The connection to Pagure failed while trying to ' 'create a new issue. The error was: {0}'.format( - str(error))) + str(error))) raise rpkgError(error_msg) base_error_msg = ('The following error occurred while creating a new ' @@ -93,8 +95,8 @@ def new_pagure_issue(logger, url, token, title, body, cli_name): # show hint for expired token if re.search(r"Invalid or expired token", rv_error, re.IGNORECASE): base_error_msg += '\nFor invalid or expired tokens please ' \ - 'set a new token in your user configuration with:' \ - '\n\n\t{0} set-pagure-token \n'.format(cli_name) + 'set a new token in your user configuration with:' \ + '\n\n\t{0} set-pagure-token \n'.format(cli_name) raise rpkgError(base_error_msg.format(rv_error)) return '{0}/releng/fedora-scm-requests/issue/{1}'.format( @@ -151,8 +153,8 @@ def do_fork(logger, base_url, token, repo_name, namespace, cli_name): # show hint for expired token if re.search(r"Invalid or expired token", rv_error, re.IGNORECASE): base_error_msg += '\nFor invalid or expired tokens please ' \ - 'set a new token in your user configuration with:' \ - '\n\n\t{0} set-distgit-token \n'.format(cli_name) + 'set a new token in your user configuration with:' \ + '\n\n\t{0} set-distgit-token \n'.format(cli_name) raise rpkgError(base_error_msg.format(rv_error)) return True @@ -217,7 +219,7 @@ def get_pagure_branches(logger, url, namespace, repo_name): except ConnectionError as error: error_msg = ('The connection to Pagure failed while getting a list ' 'of branches from Pagure. The error was: {0}'.format( - str(error))) + str(error))) raise rpkgError(error_msg) base_error_msg = ('The following error occurred while getting a list ' @@ -327,17 +329,11 @@ def assert_valid_epel_package(name, branch): # Starting with epel9 and epel9-next, check against CentOS compose metadata. if int(version) >= 9: - # Currently the stream-9 branch resides in the production address path.. - # To remove this block, we need for the stream-9 link to point to production + # Currently we only have a latest symlink. In the future we'll need + # separate latest symlinks that include the major version. # https://bugzilla.redhat.com/show_bug.cgi?id=2005139 - if int(version) == 9: - url = 'https://composes.stream.centos.org/production/' \ - 'latest-CentOS-Stream/compose/metadata/rpms.json' - else: - url = ('https://composes.stream.centos.org/stream-{0}/' - 'production/latest-CentOS-Stream/compose/metadata/rpms.json' - .format(version)) - + url = 'https://composes.stream.centos.org/production/' \ + 'latest-CentOS-Stream/compose/metadata/rpms.json' error_msg = ('The connection to composes.stream.centos.org failed while ' 'trying to determine if this is a valid EPEL package.') try: @@ -440,38 +436,41 @@ def get_dist_git_url(anongiturl): return '{0}://{1}'.format(parsed_url.scheme, parsed_url.netloc) -def get_stream_branches(server_url, package_name, apibaseurl, logger): +def get_stream_branches(server_url, package_name): """Get a package's stream branches - :param str server_url: Bodhi server URL. + :param str server_url: PDC server URL. :param str package_name: package name. Generally for RPM packages, this is the repository name without namespace. - :param str apibaseurl: Distgit url (src.fedoraproject.org) - :param obj logger: Log object - :return: a list of stream branches. Each element in the list is an active - release branch name. - :rtype: list + :return: a list of stream branches. Each element in the list is a dict + containing branch property name and active. + :rtype: list[dict] """ - active_branches = set(query_bodhi(server_url)) - - package_branches = get_pagure_branches(logger, apibaseurl, "rpms", package_name) - - # Stream branches are the intersection between active releases in Bodhi - # (active_branches) and package_branches - intersection = list(set(active_branches) & set(package_branches)) - + query_args = { + 'global_component': package_name, + 'fields': ['name', 'active'], + } + branches = query_pdc( + server_url, 'component-branches', params=query_args) + # When write this method, endpoint component-branches contains not only + # stream branches, but also regular release branches, e.g. rawhide/main, f28. + # Please remember to review the data regularly, there are only stream + # branches, or some new replacement of PDC fixes the issue as well, it + # should be ok to remove if from this list. stream_branches = [] - for item in intersection: - if re.match(r'^(f|el)\d+$', item): + for item in branches: + if item['name'] in ('rawhide', 'main'): + continue + elif re.match(r'^(f|el)\d+$', item['name']): continue # epel7 is regular release branch # epel8 and above should be considered a stream branch to use # package.cfg file in the branch. - elif 'epel7' == item: + elif 'epel7' == item['name']: continue # epel8-next and above branches should be considered as release branches # so that it will use epelX-next-candidate target to build. - elif re.match(r'^epel\d+-next$', item): + elif re.match(r'^epel\d+-next$', item['name']): continue else: stream_branches.append(item) @@ -625,8 +624,22 @@ def disable_monitoring(logger, base_url, token, repo_name, namespace, cli_name): # show hint for expired token if re.search(r"Invalid or expired token", rv_error, re.IGNORECASE): base_error_msg += '\nFor invalid or expired tokens please ' \ - 'set a new token in your user configuration with:' \ - '\n\n\t{0} set-distgit-token \n'.format(cli_name) + 'set a new token in your user configuration with:' \ + '\n\n\t{0} set-distgit-token \n'.format(cli_name) raise rpkgError(base_error_msg.format(rv_error)) logger.info("Monitoring of the project was sucessfully disabled.") + + +def get_user_groups(username): + """ + Gets a list of user groups. + :param username: a string of the username + :return: a list of user groups + """ + c = fasjson_client.Client("https://fasjson.fedoraproject.org/") + try: + user_groups = c.list_user_groups(username=username).result + except Exception as e: + return [] + return user_groups diff --git a/requirements.txt b/requirements.txt index cb3de2c..950e6ce 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,5 +1,6 @@ argcomplete bodhi-client +fasjson-client openidc-client python-bugzilla python-fedora diff --git a/test/test_utils.py b/test/test_utils.py index 7a38740..0e62612 100644 --- a/test/test_utils.py +++ b/test/test_utils.py @@ -12,6 +12,9 @@ import json +import fasjson_client +import git +import tempfile import six from requests.exceptions import ConnectionError from six.moves.configparser import NoOptionError, NoSectionError @@ -65,7 +68,7 @@ class TestUtils(unittest.TestCase): assert False, 'An rpkgError exception was not raised' except rpkgError as e: assert str(e) == \ - 'The SL "bug_fixes/2030-12-01" is in an invalid format' + 'The SL "bug_fixes/2030-12-01" is in an invalid format' def test_verify_sls_invalid_date(self): """Test verify_sls with an SL that is not June 1st or December 1st. An @@ -414,7 +417,7 @@ class TestNewPagureIssue(unittest.TestCase): '123456', 'new package', issue_ticket_body, - 'fedpkg',) + 'fedpkg', ) expected_issue_url = ( '{0}/releng/fedora-scm-requests/issue/1' @@ -577,5 +580,54 @@ class TestGetFedoraReleaseState(unittest.TestCase): config = Mock() config.get.side_effect = NoOptionError('releases_service', 'fedpkg.bodhi') six.assertRaisesRegex(self, rpkgError, r"Could not get release state for Fedora \(F30M\): " - "No option 'releases_service' in section: 'fedpkg.bodhi'.", + "No option 'releases_service' in section: 'fedpkg.bodhi'.", utils.get_fedora_release_state, config, 'fedpkg', 'F30M') + + +class TestGetUserGroups(unittest.TestCase): + """Test get_user_groups""" + + @patch("fasjson_client.Client") + def test_get_user_groups_valid(self, mock_client): + mock_client.return_value.list_user_groups.return_value.result = ['group1', 'group2'] + expected_groups = ['group1', 'group2'] + + result = utils.get_user_groups('valid_username') + + self.assertEqual(result, expected_groups) + + @patch("fasjson_client.Client") + def test_get_user_groups_exception(self, mock_client): + mock_client.return_value.list_user_groups.side_effect = Exception('test') + + result = utils.get_user_groups('invalid_username') + + self.assertEqual(result, []) + + +class TestGetLastCommitDate(unittest.TestCase): + """Test get_last_commit_date""" + + @patch("git.Repo") + @patch("tempfile.TemporaryDirectory") + def test_get_last_commit_date_success(self, mock_temp_dir, mock_git_repo): + commit_date = "1712761897" + mock_repo = Mock() + mock_git_repo.init.return_value = mock_repo + mock_repo.git.show.return_value = commit_date + + last_commit_date = utils.get_last_commit_date( + "url", "ns", "name", "branch") + + self.assertEqual(last_commit_date, commit_date) + + @patch("git.Repo") + @patch("tempfile.TemporaryDirectory") + def test_get_last_commit_date_exception(self, mock_temp_dir, mock_git_repo): + error_msg = ("Unable to get last commit date. Try to check repo name and namespace " + "if it exists.") + mock_git_repo.init.side_effect = git.exc.GitCommandError("some error") + + with self.assertRaises(rpkgError, msg=error_msg): + last_commit_date = utils.get_last_commit_date( + "url", "ns", "name", "branch") diff --git a/tests-requirements.txt b/tests-requirements.txt index 178e74e..78fd411 100644 --- a/tests-requirements.txt +++ b/tests-requirements.txt @@ -3,6 +3,7 @@ coverage<5.0.0 cccolutils gitpython freezegun +fasjson-client rpm < 0.0.3 pytest pytest-cov