From 4b0836ecf84294d1ca3cdb846d9f1f5f928a2a70 Mon Sep 17 00:00:00 2001 From: Valerij Maljulin Date: Jan 13 2021 11:35:48 +0000 Subject: [PATCH 1/4] Add extension switching (yml/yaml) on 404 error from the remote rule URL JIRA: RHELWF-389 This fixes #570 --- diff --git a/greenwave/resources.py b/greenwave/resources.py index fe1ee7b..e110aa6 100644 --- a/greenwave/resources.py +++ b/greenwave/resources.py @@ -211,7 +211,17 @@ def retrieve_yaml_remote_rule(url): """ Retrieve a remote rule file content from the git web UI. """ response = requests_session.request('HEAD', url) if response.status_code == 404: - return None + log.debug(f'Server returned 404 for {url}. Trying to change the extension (yml<->yaml)') + if url.rfind('.yml') != -1: + url = url.replace('.yml', '.yaml') + response = requests_session.request('HEAD', url) + elif url.rfind('.yaml') != -1: + url = url.replace('.yaml', '.yml') + response = requests_session.request('HEAD', url) + if response.status_code == 404: + log.debug('Changing extension was still unsuccessful!') + return None + log.debug('Extension change was successful!') if response.status_code != 200: raise BadGateway('Error occurred while retrieving a remote rule file from the repo.') diff --git a/greenwave/tests/test_retrieve_gating_yaml.py b/greenwave/tests/test_retrieve_gating_yaml.py index dc9f49e..e63b473 100644 --- a/greenwave/tests/test_retrieve_gating_yaml.py +++ b/greenwave/tests/test_retrieve_gating_yaml.py @@ -133,15 +133,40 @@ def test_retrieve_yaml_remote_rule_no_namespace(): response = mock.MagicMock() response.status_code = 404 session.request.return_value = response - retrieve_yaml_remote_rule( + returned_file = retrieve_yaml_remote_rule( app.config['REMOTE_RULE_POLICIES']['*'].format( rev='deadbeaf', pkg_name='pkg', pkg_namespace='' ) ) - expected_call = mock.call( - 'HEAD', 'https://src.fedoraproject.org/pkg/raw/deadbeaf/f/gating.yaml') - assert session.request.mock_calls == [expected_call] + expected_call1 = mock.call( + 'HEAD', 'https://src.fedoraproject.org/pkg/raw/deadbeaf/f/gating.yaml' + ) + expected_call2 = mock.call( + 'HEAD', 'https://src.fedoraproject.org/pkg/raw/deadbeaf/f/gating.yml' + ) + assert session.request.mock_calls == [expected_call1, expected_call2] + assert returned_file is None + + +def test_retrieve_yaml_remote_rule_change_ext(): + app = greenwave.app_factory.create_app() + with app.app_context(): + with mock.patch('greenwave.resources.requests_session') as session: + response1 = mock.MagicMock() + response2 = mock.MagicMock() + response1.status_code = 404 + response2.status_code = 200 + response2.content = 'ABC' + session.request.side_effect = [response1, response2, response2] + + returned_file = retrieve_yaml_remote_rule('https://xxx/yyy.yml') + + expected_call1 = mock.call('HEAD', 'https://xxx/yyy.yml') + expected_call2 = mock.call('HEAD', 'https://xxx/yyy.yaml') + expected_call3 = mock.call('GET', 'https://xxx/yyy.yaml') + assert session.request.mock_calls == [expected_call1, expected_call2, expected_call3] + assert returned_file == response2.content def test_retrieve_yaml_remote_rule_connection_error(): From a10d699f845bfcfdce0990ded123c58424affe97 Mon Sep 17 00:00:00 2001 From: Valerij Maljulin Date: Jan 13 2021 11:35:48 +0000 Subject: [PATCH 2/4] Remote rule URL can now be a list of URLs --- diff --git a/greenwave/policies.py b/greenwave/policies.py index 96e466a..34a49e2 100644 --- a/greenwave/policies.py +++ b/greenwave/policies.py @@ -449,46 +449,53 @@ class RemoteRule(Rule): return [] rr_policies_conf = current_app.config.get('REMOTE_RULE_POLICIES', {}) - cur_subject_url = ( + cur_subject_urls = ( rr_policies_conf.get(policy.subject_type) or rr_policies_conf.get('*') or current_app.config.get('DIST_GIT_URL_TEMPLATE') ) - if not cur_subject_url: + if not cur_subject_urls: raise RuntimeError(f'Cannot use a remote rule for {subject} subject ' f'as it has not been configured') - response = None - url_params = {} - if '{pkg_name}' in cur_subject_url or '{pkg_namespace}' in cur_subject_url or \ - '{rev}' in cur_subject_url: - try: - pkg_namespace, pkg_name, rev = greenwave.resources.retrieve_scm_from_koji( - subject.identifier - ) - except greenwave.resources.NoSourceException as e: - log.error(e) - return None - - # if the element is actually a container and not a pkg there will be a "-container" - # string at the end of the "pkg_name" and it will not match with the one in the - # remote rule file URL - if pkg_namespace == 'containers': - pkg_name = re.sub('-container$', '', pkg_name) - if pkg_namespace: - pkg_namespace += '/' - url_params.update(rev=rev, pkg_name=pkg_name, pkg_namespace=pkg_namespace) - - if '{subject_id}' in cur_subject_url: - subj_id = subject.identifier - if subj_id.startswith('sha256:'): - subj_id = subj_id[7:] - url_params.update(subject_id=subj_id) - - response = greenwave.resources.retrieve_yaml_remote_rule( - cur_subject_url.format(**url_params) - ) + if not isinstance(cur_subject_urls, list): + cur_subject_urls = [cur_subject_urls] + + for current_url in cur_subject_urls: + response = None + url_params = {} + if '{pkg_name}' in current_url or '{pkg_namespace}' in current_url or \ + '{rev}' in current_url: + try: + pkg_namespace, pkg_name, rev = greenwave.resources.retrieve_scm_from_koji( + subject.identifier + ) + except greenwave.resources.NoSourceException as e: + log.error(e) + return None + + # if the element is actually a container and not a pkg there will be a "-container" + # string at the end of the "pkg_name" and it will not match with the one in the + # remote rule file URL + if pkg_namespace == 'containers': + pkg_name = re.sub('-container$', '', pkg_name) + if pkg_namespace: + pkg_namespace += '/' + url_params.update(rev=rev, pkg_name=pkg_name, pkg_namespace=pkg_namespace) + + if '{subject_id}' in current_url: + subj_id = subject.identifier + if subj_id.startswith('sha256:'): + subj_id = subj_id[7:] + url_params.update(subject_id=subj_id) + + response = greenwave.resources.retrieve_yaml_remote_rule( + current_url.format(**url_params) + ) + + if response is not None: + break if response is None: # greenwave extension file not found From dc39d32b6908c1cf7a9a3280a15e8dd80eca76ad Mon Sep 17 00:00:00 2001 From: Valerij Maljulin Date: Jan 13 2021 11:35:48 +0000 Subject: [PATCH 3/4] Remove extension change code --- diff --git a/greenwave/resources.py b/greenwave/resources.py index e110aa6..66847b7 100644 --- a/greenwave/resources.py +++ b/greenwave/resources.py @@ -211,17 +211,8 @@ def retrieve_yaml_remote_rule(url): """ Retrieve a remote rule file content from the git web UI. """ response = requests_session.request('HEAD', url) if response.status_code == 404: - log.debug(f'Server returned 404 for {url}. Trying to change the extension (yml<->yaml)') - if url.rfind('.yml') != -1: - url = url.replace('.yml', '.yaml') - response = requests_session.request('HEAD', url) - elif url.rfind('.yaml') != -1: - url = url.replace('.yaml', '.yml') - response = requests_session.request('HEAD', url) - if response.status_code == 404: - log.debug('Changing extension was still unsuccessful!') - return None - log.debug('Extension change was successful!') + log.debug(f'Server returned 404 for {url}.') + return None if response.status_code != 200: raise BadGateway('Error occurred while retrieving a remote rule file from the repo.') diff --git a/greenwave/tests/test_retrieve_gating_yaml.py b/greenwave/tests/test_retrieve_gating_yaml.py index e63b473..a68aeeb 100644 --- a/greenwave/tests/test_retrieve_gating_yaml.py +++ b/greenwave/tests/test_retrieve_gating_yaml.py @@ -139,36 +139,12 @@ def test_retrieve_yaml_remote_rule_no_namespace(): ) ) - expected_call1 = mock.call( + assert session.request.mock_calls == [mock.call( 'HEAD', 'https://src.fedoraproject.org/pkg/raw/deadbeaf/f/gating.yaml' - ) - expected_call2 = mock.call( - 'HEAD', 'https://src.fedoraproject.org/pkg/raw/deadbeaf/f/gating.yml' - ) - assert session.request.mock_calls == [expected_call1, expected_call2] + )] assert returned_file is None -def test_retrieve_yaml_remote_rule_change_ext(): - app = greenwave.app_factory.create_app() - with app.app_context(): - with mock.patch('greenwave.resources.requests_session') as session: - response1 = mock.MagicMock() - response2 = mock.MagicMock() - response1.status_code = 404 - response2.status_code = 200 - response2.content = 'ABC' - session.request.side_effect = [response1, response2, response2] - - returned_file = retrieve_yaml_remote_rule('https://xxx/yyy.yml') - - expected_call1 = mock.call('HEAD', 'https://xxx/yyy.yml') - expected_call2 = mock.call('HEAD', 'https://xxx/yyy.yaml') - expected_call3 = mock.call('GET', 'https://xxx/yyy.yaml') - assert session.request.mock_calls == [expected_call1, expected_call2, expected_call3] - assert returned_file == response2.content - - def test_retrieve_yaml_remote_rule_connection_error(): app = greenwave.app_factory.create_app() with app.app_context(): From 6dd62f91bf9960e14e1657886564bb890914a55d Mon Sep 17 00:00:00 2001 From: Valerij Maljulin Date: Jan 13 2021 12:56:58 +0000 Subject: [PATCH 4/4] Test for multiple URLs in REMOTE_RULE_POLICIES --- diff --git a/greenwave/tests/test_policies.py b/greenwave/tests/test_policies.py index 4f0427c..363b680 100644 --- a/greenwave/tests/test_policies.py +++ b/greenwave/tests/test_policies.py @@ -654,6 +654,64 @@ def test_remote_rule_policy_redhat_container_image(tmpdir): assert isinstance(decision[0], TestResultFailed) +def test_get_sub_policies_multiple_urls(tmpdir): + """ Testing the RemoteRule with the koji interaction when on_demand policy is given. + In this case we are just mocking koji """ + + config = TestingConfig() + config.REMOTE_RULE_POLICIES = {'*': [ + 'https://src1.fp.org/{pkg_namespace}{pkg_name}/raw/{rev}/f/gating.yaml', + 'https://src2.fp.org/{pkg_namespace}{pkg_name}/raw/{rev}/f/gating.yaml' + ]} + + app = create_app(config) + + nvr = 'nethack-1.2.3-1.el9000' + subject = create_subject('koji_build', nvr) + + serverside_json = { + 'product_version': 'fedora-26', + 'id': 'taskotron_release_critical_tasks_with_remoterule', + 'subject': [{'item': nvr, 'type': 'koji_build'}], + 'rules': [ + { + 'type': 'RemoteRule', + 'required': True + }, + ], + } + + with app.app_context(): + with mock.patch('greenwave.resources.retrieve_scm_from_koji') as scm: + scm.return_value = ('rpms', 'nethack', 'c3c47a08a66451cb9686c49f040776ed35a0d1bb') + with mock.patch('greenwave.resources.requests_session') as session: + response = mock.MagicMock() + response.status_code = 404 + session.request.side_effect = [response, response] + + policy = OnDemandPolicy.create_from_json(serverside_json) + assert isinstance(policy.rules[0], RemoteRule) + assert policy.rules[0].required + + results = DummyResultsRetriever() + decision = policy.check('fedora-26', subject, results) + expected_call1 = mock.call( + 'HEAD', 'https://src1.fp.org/{0}/{1}/raw/{2}/f/gating.yaml'.format( + *scm.return_value + ) + ) + expected_call2 = mock.call( + 'HEAD', 'https://src2.fp.org/{0}/{1}/raw/{2}/f/gating.yaml'.format( + *scm.return_value + ) + ) + assert session.request.mock_calls == [expected_call1, expected_call2] + assert len(decision) == 1 + assert isinstance(decision[0], MissingRemoteRuleYaml) + assert not decision[0].is_satisfied + assert decision[0].subject.identifier == subject.identifier + + def test_redhat_container_image_subject_type(): nvr = '389-ds-1.4-820181127205924.9edba152' rdb_url = 'http://results.db'