From 34071ac112a22c6160ecb044e88818fc2d4ad3cb Mon Sep 17 00:00:00 2001 From: Miroslav Vadkerti Date: Feb 24 2018 07:51:00 +0000 Subject: Add requests-tests-repo command This patch adds a command for adding new repository in the tests namespace. The only required parameter is the module name and the description of the repository. An example of issue filed by the new command: https://stg.pagure.io/releng/fedora-scm-requests/issue/47 Resolves: https://pagure.io/fedpkg/issue/176 Signed-off-by: Miroslav Vadkerti --- diff --git a/fedpkg/cli.py b/fedpkg/cli.py index a203b91..cfb9af2 100644 --- a/fedpkg/cli.py +++ b/fedpkg/cli.py @@ -27,7 +27,8 @@ from pyrpkg import rpkgError from fedpkg.bugzilla import BugzillaClient from fedpkg.utils import ( get_release_branches, sl_list_to_dict, verify_sls, new_pagure_issue, - get_pagure_token, is_epel, assert_valid_epel_package) + get_pagure_token, is_epel, assert_valid_epel_package, + assert_new_tests_repo, get_dist_git_url) RELEASE_BRANCH_REGEX = r'^(f\d+|el\d+|epel\d+)$' @@ -56,6 +57,7 @@ class fedpkgClient(cliClient): self.register_retire() self.register_update() self.register_request_repo() + self.register_request_tests_repo() self.register_request_branch() # Target registry goes here @@ -128,6 +130,42 @@ the package foo: 'process (specifically, it does not require a Bugzilla bug)') request_repo_parser.set_defaults(command=self.request_repo) + def register_request_tests_repo(self): + help_msg = 'Request a new tests dist-git repository' + pagure_url = urlparse(self.config.get( + '{0}.pagure'.format(self.name), 'url')).netloc + anongiturl = self.config.get(self.name, 'anongiturl', vars={'module': 'any'}) + description = '''Request a new dist-git repository in tests shared namespace + + {2}/projects/tests/* + +For more information about tests shared namespace see + + https://fedoraproject.org/wiki/CI/Share_Test_Code + +Please refer to the request-repo command to see what has to be done before +requesting a repository in the tests namespace. + +Below is a basic example of the command to request a dist-git repository for +the space tests/foo: + + fedpkg --module-name foo request-tests-repo "Description of the repository" + +Note that the space name needs to reflect the intent of the tests and will +undergo a manual review. + +'''.format(self.name, pagure_url, get_dist_git_url(anongiturl)) + + request_tests_repo_parser = self.subparsers.add_parser( + 'request-tests-repo', + formatter_class=argparse.RawDescriptionHelpFormatter, + help=help_msg, + description=description) + request_tests_repo_parser.add_argument( + 'description', + help='Description of the tests repository') + request_tests_repo_parser.set_defaults(command=self.request_tests_repo) + def register_request_branch(self): help_msg = 'Request a new dist-git branch' description = '''Request a new dist-git branch @@ -319,9 +357,20 @@ suggest_reboot=False config=self.config, ) + def request_tests_repo(self): + self._request_repo( + module_name=self.cmd.module_name, + ns='tests', + description=self.args.description, + name=self.name, + config=self.config, + anongiturl=self.cmd.anongiturl + ) + @staticmethod - def _request_repo(module_name, ns, branch, summary, description, - upstreamurl, monitor, bug, exception, name, config): + def _request_repo(module_name, ns, description, name, config, branch=None, + summary=None, upstreamurl=None, monitor=None, bug=None, + exception=None, anongiturl=None): """ Implementation of `request_repo`. Submits a request for a new dist-git repo. @@ -330,12 +379,16 @@ suggest_reboot=False value of `self.cmd.module_name`. :param ns: The repository namespace string, i.e. 'rpms' or 'modules'. Typically takes the value of `self.cmd.ns`. + :param description: A string, the description of the new repo. + Typically takes the value of `self.args.description`. + :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`. :param branch: The git branch string when requesting a repo. Typically 'master'. :param summary: A string, the summary of the new repo. Typically takes the value of `self.args.summary`. - :param description: A string, the description of the new repo. - Typically takes the value of `self.args.description`. :param upstreamurl: A string, the upstreamurl of the new repo. Typically takes the value of `self.args.upstreamurl`. :param monitor: A string, the monitoring flag of the new repo, i.e. @@ -349,16 +402,14 @@ suggest_reboot=False granted the right to waive their package review at the discretion of Release Engineering. Typically takes the value of `self.args.exception`. - :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`. + :param anongiturl: A string with the name of the anonymous git url. + Typically the value of `self.cmd.anongiturl`. :return: None """ # bug is not a required parameter in the event the packager has an # exception, in which case, they may use the --exception flag - if not bug and not exception: + if not bug and not exception and ns != 'tests': raise rpkgError( 'A Bugzilla bug is required on new repository requests') repo_regex = r'^[a-zA-Z0-9_][a-zA-Z0-9-_.+]*$' @@ -371,24 +422,35 @@ suggest_reboot=False .format(module_name)) summary_from_bug = '' - if bug: + if bug and ns != 'tests': bz_url = config.get('{0}.bugzilla'.format(name), 'url') bz_client = BugzillaClient(bz_url) bug_obj = bz_client.get_review_bug(bug, ns, module_name) summary_from_bug = bug_obj.summary.split(' - ', 1)[1].strip() - ticket_body = { - 'action': 'new_repo', - 'branch': branch, - 'bug_id': bug or '', - 'description': description or '', - 'exception': exception, - 'monitor': monitor, - 'namespace': ns, - 'repo': module_name, - 'summary': summary or summary_from_bug, - 'upstreamurl': upstreamurl or '' - } + if ns == 'tests': + # check if tests repository does not exist already + assert_new_tests_repo(module_name, get_dist_git_url(anongiturl)) + + ticket_body = { + 'action': 'new_repo', + 'namespace': 'tests', + 'repo': module_name, + 'description': description, + } + else: + ticket_body = { + 'action': 'new_repo', + 'branch': branch, + 'bug_id': bug or '', + 'description': description or '', + 'exception': exception, + 'monitor': monitor, + 'namespace': ns, + 'repo': module_name, + 'summary': summary or summary_from_bug, + 'upstreamurl': upstreamurl or '' + } ticket_body = json.dumps(ticket_body, indent=True) ticket_body = '```\n{0}\n```'.format(ticket_body) diff --git a/fedpkg/utils.py b/fedpkg/utils.py index d241308..f8a415d 100644 --- a/fedpkg/utils.py +++ b/fedpkg/utils.py @@ -14,7 +14,7 @@ import re import json from datetime import datetime -from six.moves.urllib.parse import urlencode +from six.moves.urllib.parse import urlencode, urlparse from six.moves.configparser import NoSectionError, NoOptionError import requests from requests.exceptions import ConnectionError @@ -243,3 +243,38 @@ def assert_valid_epel_package(name, branch): pkg_arches = set(pkg_info['arch']) if pkg_arches == set(['noarch']) or not (all_arches - pkg_arches): raise rpkgError(error_msg_two) + + +def assert_new_tests_repo(name, dist_git_url): + """ + Asserts that the tests repository name is new. Note that the repository name + can be any arbitrary string, so just check if the repository already exists. + + :param name: a string with the package name + :return: None or rpkgError + """ + + url = '{0}/tests/{1}'.format(dist_git_url, name) + error_msg = ( + 'The connection to dist-git failed' + 'trying to determine if this is a valid new tests ' + ' repository name.') + try: + rv = requests.get(url, timeout=60) + except ConnectionError as error: + error_msg += ' The error was: {0}'.format(str(error)) + raise rpkgError(error_msg) + + if rv.ok: + raise rpkgError("Repository {0} already exists".format(url)) + + +def get_dist_git_url(anongiturl): + """ + Extracts dist-git url from the anongiturl configuration option. + :param anongiturl: The `anongiturl` configuration option value. Typically + takes the argument of `self.cmd.anongiturl` + :return: dist-git url string or rpkgError + """ + parsed_url = urlparse(anongiturl) + return '{0}://{1}'.format(parsed_url.scheme, parsed_url.netloc) diff --git a/test/test_cli.py b/test/test_cli.py index 047b962..7662d44 100644 --- a/test/test_cli.py +++ b/test/test_cli.py @@ -1032,3 +1032,82 @@ https://pagure.stg.example.com/releng/fedora-scm-requests/issue/3""" 'Engineering team.') with six.assertRaisesRegex(self, rpkgError, expected_error): cli.request_branch() + + +class TestRequestTestsRepo(CliTestCase): + """Test the request-tests-repo command""" + + def setUp(self): + super(TestRequestTestsRepo, self).setUp() + + def tearDown(self): + super(TestRequestTestsRepo, self).tearDown() + + def get_cli(self, cli_cmd, name='fedpkg-stage', cfg='fedpkg-stage.conf', + user_cfg='fedpkg-user-stage.conf'): + with patch('sys.argv', new=cli_cmd): + return self.new_cli(name=name, cfg=cfg, user_cfg=user_cfg) + + @patch('requests.post') + @patch('requests.get') + @patch('sys.stdout', new=StringIO()) + def test_request_tests_repo(self, mock_request_get, mock_request_post): + """Tests request-tests-repo""" + + # mock the request.get from assert_new_tests_repo + mock_get_rv = Mock() + mock_get_rv.ok = False + mock_request_get.return_value = mock_get_rv + + # mock the request.post to Pagure + mock_post_rv = Mock() + mock_post_rv.ok = True + mock_post_rv.json.return_value = {'issue': {'id': 1}} + mock_request_post.return_value = mock_post_rv + + cli_cmd = ['fedpkg-stage', '--path', self.cloned_repo_path, + '--module-name', 'foo', 'request-tests-repo', + 'Some description'] + cli = self.get_cli(cli_cmd) + cli.request_tests_repo() + + expected_issue_content = { + 'action': 'new_repo', + 'repo': 'foo', + 'namespace': 'tests', + 'description': 'Some description' + } + + # Get the data that was submitted to Pagure + post_data = mock_request_post.call_args_list[0][1]['data'] + actual_issue_content = json.loads(json.loads( + post_data)['issue_content'].strip('```')) + self.assertEqual(expected_issue_content, actual_issue_content) + + output = sys.stdout.getvalue().strip() + expected_output = ('https://pagure.stg.example.com/releng/' + 'fedora-scm-requests/issue/1') + self.assertEqual(output, expected_output) + + @patch('requests.get') + def test_request_tests_repo_exists(self, mock_request_get): + """Tests request-tests-repo exception if repo already exists""" + + # mock the request.get from assert_new_tests_repo + mock_get_rv = Mock() + mock_get_rv.ok = True + mock_request_get.return_value = mock_get_rv + + cli_cmd = ['fedpkg-stage', '--path', self.cloned_repo_path, + '--module-name', 'foo', 'request-tests-repo', + 'Some description'] + cli = self.get_cli(cli_cmd) + + expected_error = ( + 'Repository git://pkgs.stg.example.com/tests/foo already exists') + + try: + cli.request_tests_repo() + assert False, 'rpkgError not raised' + except rpkgError as error: + self.assertEqual(str(error), expected_error)