From e39717660d3e1884adc0695754a0c051f9d8ace2 Mon Sep 17 00:00:00 2001 From: Giulia Naponiello Date: Jan 25 2018 13:23:26 +0000 Subject: CLI provide a way to submit waiver by result_id Provided in the CLI a way to submit a new waiver by result_id for backward compatibility. The CLI takes care of "translating" the result_id in subject/testcase and submit the waiver to waiverdb API. --- diff --git a/tests/test_cli.py b/tests/test_cli.py index 12c4b9a..95fab1f 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -32,6 +32,20 @@ auth_method=OIDC assert result.output == 'Error: The config option "api_url" is required\n' +def test_misconfigured_resultdb_api_url(tmpdir): + p = tmpdir.join('client.conf') + p.write(""" +[waiverdb] +auth_method=dummy +api_url=http://localhost:5004/api/v1.0 + """) + runner = CliRunner() + args = ['-C', p.strpath] + result = runner.invoke(waiverdb_cli, args) + assert result.exit_code == 1 + assert result.output == 'Error: The config option "resultsdb_api_url" is required\n' + + def test_misconfigured_oidc_id_provider(tmpdir): p = tmpdir.join('client.conf') p.write(""" @@ -87,6 +101,7 @@ oidc_id_provider=https://id.stg.fedoraproject.org/openidc/ oidc_client_id=waiverdb oidc_scopes= openid +resultsdb_api_url=http://localhost:5001/api/v2.0 """) runner = CliRunner() args = ['-C', p.strpath] @@ -105,6 +120,7 @@ oidc_id_provider=https://id.stg.fedoraproject.org/openidc/ oidc_client_id=waiverdb oidc_scopes= openid +resultsdb_api_url=http://localhost:5001/api/v2.0 """) runner = CliRunner() args = ['-C', p.strpath, '-p', 'fedora-26'] @@ -123,6 +139,7 @@ oidc_id_provider=https://id.stg.fedoraproject.org/openidc/ oidc_client_id=waiverdb oidc_scopes= openid +resultsdb_api_url=http://localhost:5001/api/v2.0 """) runner = CliRunner() args = ['-C', p.strpath, '-p', 'fedora-26', '-s', 'subject'] @@ -157,6 +174,7 @@ oidc_id_provider=https://id.stg.fedoraproject.org/openidc/ oidc_client_id=waiverdb oidc_scopes= openid +resultsdb_api_url=http://localhost:5001/api/v2.0 """) runner = CliRunner() args = ['-C', p.strpath, '-p', 'Parrot', '-s', '{"subject.test": "test", "s": "t"}', @@ -201,6 +219,7 @@ def test_kerberos_is_enabled(tmpdir): [waiverdb] auth_method=Kerberos api_url=http://localhost:5004/api/v1.0 +resultsdb_api_url=http://localhost:5001/api/v2.0 """) runner = CliRunner() args = ['-C', p.strpath, '-p', 'Parrot', '-s', '{"subject.test": "test", "s": "t"}', @@ -208,3 +227,110 @@ api_url=http://localhost:5004/api/v1.0 result = runner.invoke(waiverdb_cli, args) mock_request.assert_called_once() assert result.output == 'Created waiver 15 for result with subject {"subject.test": "test", "s": "t"} and testcase test.testcase\n' # noqa + + +def test_submit_waiver_with_id(tmpdir): + with patch('requests.request') as mock_request: + mock_rv = Mock() + mock_rv.json.return_value = { + "comment": "It's dead!", + "data": {"item": ["htop-1.0-1.fc22"], "type": ["bodhi_update"]}, + "id": 15, + "product_version": "Parrot", + "subject": {"subject.test": "test", "s": "t"}, + "testcase": {"name": "test.testcase"}, + "timestamp": "2017-010-16T17:42:04.209638", + "username": "foo", + "waived": True + } + mock_request.return_value = mock_rv + p = tmpdir.join('client.conf') + p.write(""" +[waiverdb] +auth_method=dummy +api_url=http://localhost:5004/api/v1.0 +resultsdb_api_url=http://localhost:5001/api/v2.0 + """) + runner = CliRunner() + args = ['-C', p.strpath, '-p', 'Parrot', '-r', '123', + '-c', "It's dead!"] + result = runner.invoke(waiverdb_cli, args) + mock_request.assert_called() + assert result.output == 'Created waiver 15 for result with id 123\n' + + +def test_submit_waiver_with_multiple_ids(tmpdir): + with patch('requests.request') as mock_request: + mock_rv = Mock() + mock_rv.json.return_value = { + "comment": "It's dead!", + "data": {"item": ["htop-1.0-1.fc22"], "type": ["bodhi_update"]}, + "id": 15, + "product_version": "Parrot", + "subject": {"subject.test": "test", "s": "t"}, + "testcase": {"name": "test.testcase"}, + "timestamp": "2017-010-16T17:42:04.209638", + "username": "foo", + "waived": True + } + mock_request.return_value = mock_rv + p = tmpdir.join('client.conf') + p.write(""" +[waiverdb] +auth_method=dummy +api_url=http://localhost:5004/api/v1.0 +resultsdb_api_url=http://localhost:5001/api/v2.0 + """) + runner = CliRunner() + args = ['-C', p.strpath, '-p', 'Parrot', '-r', '123', '-r', '456', + '-c', "It's dead!"] + result = runner.invoke(waiverdb_cli, args) + mock_request.assert_called() + + assert result.output == 'Created waiver 15 for result with id 123\n\ +Created waiver 15 for result with id 456\n' + + +def test_malformed_submission_with_id_and_subject_and_testcase(tmpdir): + runner = CliRunner() + p = tmpdir.join('client.conf') + p.write(""" +[waiverdb] +auth_method=dummy +api_url=http://localhost:5004/api/v1.0 +resultsdb_api_url=http://localhost:5001/api/v2.0 + """) + args = ['-C', p.strpath, '-p', 'Parrot', '-r', '123', '-s', + '{"subject.test": "test", "s": "t"}', '-c', "It's dead!"] + result = runner.invoke(waiverdb_cli, args) + assert result.output == 'Error: Please specify result_id or subject/testcase. Not both\n' + + +def test_submit_waiver_for_original_spec_nvr_result(tmpdir): + with patch('requests.request') as mock_request: + mock_rv = Mock() + mock_rv.json.return_value = { + "comment": "It's dead!", + "original_spec_nvr": "test", + "id": 15, + "product_version": "Parrot", + "subject": {"subject.test": "test", "s": "t"}, + "testcase": {"name": "test.testcase"}, + "timestamp": "2017-010-16T17:42:04.209638", + "username": "foo", + "waived": True + } + mock_request.return_value = mock_rv + p = tmpdir.join('client.conf') + p.write(""" +[waiverdb] +auth_method=dummy +api_url=http://localhost:5004/api/v1.0 +resultsdb_api_url=http://localhost:5001/api/v2.0 + """) + runner = CliRunner() + args = ['-C', p.strpath, '-p', 'Parrot', '-r', '123', + '-c', "It's dead!"] + result = runner.invoke(waiverdb_cli, args) + mock_request.assert_called() + assert result.output == 'Created waiver 15 for result with id 123\n' diff --git a/waiverdb/cli.py b/waiverdb/cli.py index ff417c6..85ecf8d 100644 --- a/waiverdb/cli.py +++ b/waiverdb/cli.py @@ -31,12 +31,34 @@ def validate_config(config): for required_config in required_configs: if not config.has_option('waiverdb', required_config): raise click.ClickException(config_error.format(required_config)) + if not config.has_option('waiverdb', 'resultsdb_api_url'): + raise click.ClickException(config_error.format('resultsdb_api_url')) + + +def check_response(resp, data): + if 'result_id' in data: + msg = 'for result with id {0}'.format(data['result_id']) + else: + msg = 'for result with subject {0} and testcase {1}'.format(json.dumps(data['subject']), + data['testcase']) + if not resp.ok: + try: + error_msg = resp.json()['message'] + except (ValueError, KeyError): + error_msg = resp.text + raise click.ClickException( + 'Failed to create waiver {0}:\n{1}' + .format(msg, error_msg)) + click.echo('Created waiver {0} {1}'.format( + resp.json()['id'], msg)) @click.command(context_settings={'help_option_names': ['-h', '--help']}) @click.option('--config-file', '-C', default='/etc/waiverdb/client.conf', type=click.Path(exists=True), help='Specify a config file to use') +@click.option('--result-id', '-r', multiple=True, type=int, + help='Specify one or more results to be waived') @click.option('--subject', '-s', help='Specify one subject for a result to waive') @click.option('--testcase', '-t', @@ -47,12 +69,16 @@ def validate_config(config): help='Whether or not the result is waived') @click.option('--comment', '-c', help='A comment explaining why the result is waived') -def cli(comment, waived, product_version, testcase, subject, config_file): +def cli(comment, waived, product_version, testcase, subject, result_id, config_file): """ - Creates new waivers against test results. + Creates new waiver against test results. Examples: + waiverdb-cli -r 123 -r 456 -p "fedora-26" -c "It's dead!" + + or + waiverdb-cli -t dist.rpmlint -s '{"item": "python-requests-1.2.3-1.fc26", "type": "koji_build"}' -p "fedora-26" -c "It's dead!" @@ -63,21 +89,54 @@ def cli(comment, waived, product_version, testcase, subject, config_file): config.read(config_file) validate_config(config) + result_ids = result_id if not product_version: raise click.ClickException('Please specify product version') - if not subject: + if result_ids and (subject or testcase): + raise click.ClickException('Please specify result_id or subject/testcase. Not both') + if not result_ids and not subject: raise click.ClickException('Please specify one subject') - if not testcase: + if not result_ids and not testcase: raise click.ClickException('Please specify testcase') auth_method = config.get('waiverdb', 'auth_method') - data = { - 'subject': json.loads(subject), - 'testcase': testcase, - 'waived': waived, - 'product_version': product_version, - 'comment': comment - } + data_list = [] + if result_ids: + for result_id in result_ids: + result = requests.request('GET', '{0}/results/{1}'.format(config.get('waiverdb', + 'resultsdb_api_url'), result_id), + headers={'Content-Type': 'application/json'}, + timeout=60) + if 'original_spec_nvr' in result.json(): + subject = {'original_spec_nvr': result.json()['original_spec_nvr']} + else: + if result.json()['data']['type'][0] == 'koji_build' or \ + result.json()['data']['type'][0] == 'bodhi_update': + SUBJECT_KEYS = ['item', 'type'] + subject = dict([(k, v[0]) for k, v in result.json()['data'].items() + if k in SUBJECT_KEYS]) + else: + raise click.ClickException('It is not possible to submit a waiver by \ + id for this result. Please try again specifying \ + a subject and a testcase.') + + data_list.append({ + 'result_id': result_id, + 'subject': subject, + 'testcase': result.json()['testcase']['name'], + 'waived': waived, + 'product_version': product_version, + 'comment': comment + }) + else: + data_list.append({ + 'subject': json.loads(subject), + 'testcase': testcase, + 'waived': waived, + 'product_version': product_version, + 'comment': comment + }) + api_url = config.get('waiverdb', 'api_url') if auth_method == 'OIDC': # Try to import this now so the user gets immediate feedback if @@ -97,12 +156,15 @@ def cli(comment, waived, product_version, testcase, subject, config_file): config.get('waiverdb', 'oidc_client_id'), oidc_client_secret) scopes = config.get('waiverdb', 'oidc_scopes').strip().splitlines() - resp = oidc.send_request( - scopes=scopes, - url='{0}/waivers/'.format(api_url.rstrip('/')), - data=json.dumps(data), - headers={'Content-Type': 'application/json'}, - timeout=60) + + for data in data_list: + resp = oidc.send_request( + scopes=scopes, + url='{0}/waivers/'.format(api_url.rstrip('/')), + data=json.dumps(data), + headers={'Content-Type': 'application/json'}, + timeout=60) + check_response(resp, data) elif auth_method == 'Kerberos': # Try to import this now so the user gets immediate feedback if # it isn't installed @@ -111,28 +173,22 @@ def cli(comment, waived, product_version, testcase, subject, config_file): except ImportError: raise click.ClickException('python-requests-kerberos needs to be installed') auth = requests_kerberos.HTTPKerberosAuth(mutual_authentication=requests_kerberos.OPTIONAL) - resp = requests.request('POST', '{0}/waivers/'.format(api_url.rstrip('/')), - data=json.dumps(data), auth=auth, - headers={'Content-Type': 'application/json'}, - timeout=60) - if resp.status_code == 401: - raise click.ClickException('WaiverDB authentication using Kerberos failed. ' - 'Make sure you have a valid Kerberos ticket.') + for data in data_list: + resp = requests.request('POST', '{0}/waivers/'.format(api_url.rstrip('/')), + data=json.dumps(data), auth=auth, + headers={'Content-Type': 'application/json'}, + timeout=60) + if resp.status_code == 401: + raise click.ClickException('WaiverDB authentication using Kerberos failed. ' + 'Make sure you have a valid Kerberos ticket.') + check_response(resp, data) elif auth_method == 'dummy': - resp = requests.request('POST', '{0}/waivers/'.format(api_url.rstrip('/')), - data=json.dumps(data), auth=('user', 'pass'), - headers={'Content-Type': 'application/json'}, - timeout=60) - if not resp.ok: - try: - error_msg = resp.json()['message'] - except (ValueError, KeyError): - error_msg = resp.text - raise click.ClickException( - 'Failed to create waiver for result with subject {0} and testcase {1}:\n{2}' - .format(subject, testcase, error_msg)) - click.echo('Created waiver {0} for result with subject {1} and testcase {2}'.format( - resp.json()['id'], subject, testcase)) + for data in data_list: + resp = requests.request('POST', '{0}/waivers/'.format(api_url.rstrip('/')), + data=json.dumps(data), auth=('user', 'pass'), + headers={'Content-Type': 'application/json'}, + timeout=60) + check_response(resp, data) if __name__ == '__main__':