From 546d054d5febf716adeacaf967ca67043ab10c63 Mon Sep 17 00:00:00 2001 From: Lukas Holecek Date: Feb 10 2020 13:51:27 +0000 Subject: Simplify remote rules configuration Signed-off-by: Lukas Holecek --- diff --git a/docs/policies.rst b/docs/policies.rst index f41c0f8..e58e67b 100644 --- a/docs/policies.rst +++ b/docs/policies.rst @@ -256,32 +256,30 @@ Greenwave will check if a remote rule file exists, if it does, it pulls it down, loads it, and uses it to additionally evaluate the subject of the decision. -Greenwave requires these configuration parameters ``KOJI_BASE_URL`` and -``REMOTE_RULE_POLICIES``. +Option ``KOJI_BASE_URL`` is required. It contains Koji XML-RPC URL and is used +to get namespace (rpms, container, etc.), name of the build artifact and +revision of the source dist-git commit. These values are used in URL templates +described below as ``{pkg_namespace}``, ``{pkg_name}`` and ``{rev}`` +respectively. -``REMOTE_RULE_POLICIES`` is a map, where the key is the subject type. There could be -a default pattern "*" used when no subject type matched. Old parameter ``DIST_GIT_URL_TEMPLATE`` -is used if there is no default subject type, but please note that it is obsolete -and should not be used in new configurations. Each subject should contain a map of parameters -depending on a retrieval mechanism. - -Greenwave has two mechanisms to retrieve the remote rule file: ``git archive`` is -using for side tags rules and using a git front-end. ``GIT_URL`` and ``GIT_PATH_TEMPLATE`` -should be set for ``git archive`` mechanism, ``HTTP_URL_TEMPLATE`` -should be set if you are going to use a git front-end. - -Below is an example configuration where ``git archive`` is being used for "brew-build-group" -subject type and HTTP is being used for other: +Option ``REMOTE_RULE_POLICIES`` contains mapping/dict from subject type to +remote rule URL template. Parameter ``DIST_GIT_URL_TEMPLATE`` options is used +for all other subject types not defined in the mapping that support remote +rule. .. code-block:: console REMOTE_RULE_POLICIES = { - 'brew-build-group': { - 'GIT_URL': 'git@gitlab.cee.redhat.com:devops/greenwave-policies/side-tags.git', - 'GIT_PATH_TEMPLATE': '{pkg_namespace}/{pkg_name}.yaml' - }, - '*': { - 'HTTP_URL_TEMPLATE': 'https://src.fedoraproject.org/{pkg_namespace}/{pkg_name}/raw/{rev}/f/gating.yaml' - } + 'koji_build': 'https://src.fedoraproject.org/{pkg_namespace}/{pkg_name}/raw/{rev}/f/gating.yaml', + 'brew-build-group': 'https://git.example.com/devops/greenwave-policies/side-tags/raw/master/{pkg_namespace}/{pkg_name}.yaml', } KOJI_BASE_URL = 'https://koji.fedoraproject.org/kojihub' + +For backwards compatibility, option ``DIST_GIT_BASE_URL`` can be also used in +URL templates. + +.. code-block:: console + + DIST_GIT_BASE_URL = 'https://src.stg.fedoraproject.org' + DIST_GIT_URL_TEMPLATE = '{DIST_GIT_BASE_URL}/{pkg_namespace}/{pkg_name}/raw/{rev}/f/gating.yaml' + KOJI_BASE_URL = 'https://koji.stg.fedoraproject.org/kojihub' diff --git a/greenwave/app_factory.py b/greenwave/app_factory.py index d253689..d103588 100644 --- a/greenwave/app_factory.py +++ b/greenwave/app_factory.py @@ -6,7 +6,7 @@ from flask import Flask from greenwave.api_v1 import api from greenwave.monitor import monitor_api from greenwave.utils import json_error, load_config, sha1_mangle_key -from greenwave.policies import load_policies, RemoteRule +from greenwave.policies import load_policies from greenwave.subjects.subject_type import load_subject_types from dogpile.cache import make_region @@ -16,31 +16,6 @@ from werkzeug.exceptions import default_exceptions log = logging.getLogger(__name__) -def _can_use_remote_rule(config): - if not config.get('KOJI_BASE_URL'): - return False - - if config.get('DIST_GIT_URL_TEMPLATE'): - return True - - if config.get('REMOTE_RULE_POLICIES'): - return all( - (conf_item.get('GIT_URL') and conf_item.get('GIT_PATH_TEMPLATE')) or - conf_item.get('HTTP_URL_TEMPLATE') - for conf_item in config.get('REMOTE_RULE_POLICIES').values() - ) - - return False - - -def _has_remote_rule(policies): - return any( - isinstance(rule, RemoteRule) - for policy in policies - for rule in policy.rules - ) - - # applicaiton factory http://flask.pocoo.org/docs/0.12/patterns/appfactories/ def create_app(config_obj=None): app = Flask(__name__) @@ -57,19 +32,6 @@ def create_app(config_obj=None): log.debug("config: Loading subject types from %r", subject_types_dir) app.config['subject_types'] = load_subject_types(subject_types_dir) - if app.config.get('DIST_GIT_URL_TEMPLATE') and app.config.get('DIST_GIT_BASE_URL'): - app.config['DIST_GIT_URL_TEMPLATE'] = app.config['DIST_GIT_URL_TEMPLATE'].replace( - '{DIST_GIT_BASE_URL}', app.config['DIST_GIT_BASE_URL'] - ) - - if not _can_use_remote_rule(app.config) and _has_remote_rule(app.config['policies']): - raise RuntimeError( - 'If you want to apply a RemoteRule, you must have "KOJI_BASE_URL" and ' - '"DIST_GIT_URL_TEMPLATE" or "REMOTE_RULE_POLICIES" or both set in your configuration. ' - 'Each field in "REMOTE_RULE_POLICIES" map have to contain either ' - '"GIT_URL"/"GIT_PATH_TEMPLATE" or "HTTP_URL_TEMPLATE".' - ) - # register error handlers for code in default_exceptions.keys(): app.register_error_handler(code, json_error) diff --git a/greenwave/config.py b/greenwave/config.py index 207839c..41eaabd 100644 --- a/greenwave/config.py +++ b/greenwave/config.py @@ -23,14 +23,12 @@ class Config(object): WAIVERDB_API_URL = 'https://waiverdb.fedoraproject.org/api/v1.0' # Remote rule configuration - # NOTE: DIST_GIT_URL_TEMPLATE is obsolete and used here only for - # backward compatibility. They maybe removed in future versions. Use REMOTE_RULE_POLICIES['*'] - # instead - DIST_GIT_URL_TEMPLATE = \ - 'https://src.fedoraproject.org/{pkg_namespace}{pkg_name}/raw/{rev}/f/gating.yaml' + DIST_GIT_BASE_URL = 'https://src.fedoraproject.org/' + DIST_GIT_URL_TEMPLATE = '{DIST_GIT_BASE_URL}{pkg_namespace}/{pkg_name}/raw/{rev}/f/gating.yaml' REMOTE_RULE_GIT_TIMEOUT = 30 REMOTE_RULE_GIT_MAX_RETRY = 3 KOJI_BASE_URL = 'https://koji.fedoraproject.org/kojihub' + # Options for outbound HTTP requests made by python-requests REQUESTS_TIMEOUT = (6.1, 15) REQUESTS_VERIFY = True diff --git a/greenwave/policies.py b/greenwave/policies.py index 3285934..58c35ff 100644 --- a/greenwave/policies.py +++ b/greenwave/policies.py @@ -433,13 +433,13 @@ class RemoteRule(Rule): 'required': SafeYAMLBool(optional=True, default=False), } - def _get_config_urls(self, rr_config, subject): - if subject in rr_config: - return rr_config[subject] - if '*' in rr_config: - return rr_config['*'] - raise RuntimeError(f'Cannot use a remote rule for {subject} subject ' - f'as it has not been configured') + def _url_template_for(self, subject): + rr_policies = current_app.config.get('REMOTE_RULE_POLICIES', {}) + url = rr_policies.get(subject, {}) + if url: + return url + + return current_app.config.get('DIST_GIT_URL_TEMPLATE') def _get_sub_policies(self, policy, subject): if not subject.supports_remote_rule: @@ -458,14 +458,11 @@ class RemoteRule(Rule): # remote rule file URL if pkg_namespace == 'containers': pkg_name = re.sub('-container$', '', pkg_name) - rr_policies_conf = current_app.config.get('REMOTE_RULE_POLICIES', {}) - if not rr_policies_conf or '*' not in rr_policies_conf: - rr_policies_conf['*'] = { - 'HTTP_URL_TEMPLATE': current_app.config['DIST_GIT_URL_TEMPLATE'] - } - cur_subject_config = self._get_config_urls(rr_policies_conf, policy.subject_type) + + url_template = self._url_template_for(policy.subject_type) + base_url = current_app.config.get('DIST_GIT_BASE_URL') response = greenwave.resources.retrieve_yaml_remote_rule( - rev, pkg_name, pkg_namespace, cur_subject_config + rev, pkg_name, pkg_namespace, url_template, base_url ) if response is None: diff --git a/greenwave/resources.py b/greenwave/resources.py index 67bb862..1034e28 100644 --- a/greenwave/resources.py +++ b/greenwave/resources.py @@ -8,9 +8,6 @@ waiverdb, etc..). import logging import re -from io import BytesIO -import tarfile -import subprocess import socket from urllib.parse import urlparse @@ -167,27 +164,13 @@ def retrieve_scm_from_koji_build(nvr, build, koji_url): @cached -def retrieve_yaml_remote_rule(rev, pkg_name, pkg_namespace, rr_config): - """ Retrieve a remote rule file content from the given repo""" - if rr_config.get('GIT_URL') and rr_config.get('GIT_PATH_TEMPLATE'): - return _retrieve_yaml_remote_rule_git_archive( - pkg_name, pkg_namespace, rr_config['GIT_URL'], rr_config['GIT_PATH_TEMPLATE'] - ) - else: - return _retrieve_yaml_remote_rule_web( - rev, pkg_name, pkg_namespace, rr_config['HTTP_URL_TEMPLATE'] - ) - - -_retrieve_remote_rule_error = 'Error occurred while retrieving a remote rule file from the repo.' - - -def _retrieve_yaml_remote_rule_web(rev, pkg_name, pkg_namespace, url_template): +def retrieve_yaml_remote_rule(rev, pkg_name, pkg_namespace, url_template, base_url=''): """ Retrieve a remote rule file content from the git web UI. """ data = { + "DIST_GIT_BASE_URL": (base_url.rstrip('/') + ('/' if pkg_namespace else '')), "pkg_namespace": pkg_namespace + ('/' if pkg_namespace else ''), "pkg_name": pkg_name, - "rev": rev + "rev": rev, } url = url_template.format(**data) response = requests_session.request('HEAD', url) @@ -195,7 +178,7 @@ def _retrieve_yaml_remote_rule_web(rev, pkg_name, pkg_namespace, url_template): return None if response.status_code != 200: - raise BadGateway(_retrieve_remote_rule_error) + raise BadGateway('Error occurred while retrieving a remote rule file from the repo.') # remote rule file found... response = requests_session.request('GET', url) @@ -203,41 +186,6 @@ def _retrieve_yaml_remote_rule_web(rev, pkg_name, pkg_namespace, url_template): return response.content -def _retrieve_yaml_remote_rule_git_archive(pkg_name, pkg_namespace, git_url, path_template): - """ Retrieve a remote rule file content from a git repo using git archive. """ - git_path = path_template.format(pkg_name=pkg_name, pkg_namespace=pkg_namespace) - cmd = ['git', 'archive', f'--remote={git_url}', 'master', git_path] - # Retry thrice if TimeoutExpired exception is raised - MAX_RETRY = current_app.config.get('REMOTE_RULE_GIT_MAX_RETRY', 3) - git_archive = None - for _ in range(MAX_RETRY): - try: - git_archive = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) - output, error_output = git_archive.communicate( - timeout=current_app.config.get('REMOTE_RULE_GIT_TIMEOUT', 30) - ) - break - except subprocess.TimeoutExpired: - if git_archive: - git_archive.kill() - continue - - if git_archive.returncode != 0: - error_output = error_output.decode('utf-8') - if 'path not found' in error_output: - return None - - cmd_str = ' '.join(cmd) - log.error('The following exception occurred while running "%s": %s', cmd_str, error_output) - raise BadGateway(_retrieve_remote_rule_error) - - # Convert the output to a file-like object with BytesIO, then tar can read it - # in memory rather than writing it to a file first - remote_rule_archive = tarfile.open(fileobj=BytesIO(output)) - remote_rule_content = remote_rule_archive.extractfile(git_path).read().decode('utf-8') - return remote_rule_content - - # NOTE - not cached. def retrieve_decision(greenwave_url, data): response = requests_session.post(greenwave_url, json=data) diff --git a/greenwave/tests/test_app_factory.py b/greenwave/tests/test_app_factory.py index 621cc1a..ee8a30d 100644 --- a/greenwave/tests/test_app_factory.py +++ b/greenwave/tests/test_app_factory.py @@ -5,18 +5,14 @@ import pytest from textwrap import dedent -from greenwave.app_factory import create_app, _can_use_remote_rule +from greenwave.app_factory import create_app from greenwave.policies import Policy from greenwave.config import TestingConfig +from greenwave.subjects.factory import create_subject -@mock.patch('greenwave.policies.load_policies') -def test_remote_rules_misconfigured(mock_load_policies): - """ - The application shouldn't start if RemoteRule is in policy configuration - but if cannot be used because dist-git or koji URL is not configured. - """ - +@pytest.fixture +def mock_load_policies(): policies = Policy.safe_load_all(dedent(""" --- !Policy id: test_policy @@ -26,85 +22,45 @@ def test_remote_rules_misconfigured(mock_load_policies): rules: - !RemoteRule {} """)) - mock_load_policies.return_value = policies - - config = TestingConfig() - config.DIST_GIT_URL_TEMPLATE = '' - config.REMOTE_RULE_POLICIES = {} - expected_error = 'If you want to apply a RemoteRule' + with mock.patch('greenwave.app_factory.load_policies') as mocked: + mocked.return_value = policies + yield mocked - with pytest.raises(RuntimeError, match=expected_error): - create_app(config) - -@mock.patch('greenwave.policies.load_policies') -def test_remote_rules_base_url(mock_load_policies): +@mock.patch('greenwave.resources.requests_session') +@mock.patch('greenwave.resources.retrieve_scm_from_koji') +def test_remote_rules_base_url(mock_retrieve_scm_from_koji, mock_session, mock_load_policies): """ - The application shouldn't start if RemoteRule is in policy configuration - but if cannot be used because dist-git or koji URL is not configured. + Test DIST_GIT_BASE_URL and DIST_GIT_URL_TEMPLATE options. """ - policies = Policy.safe_load_all(dedent(""" - --- !Policy - id: test_policy - product_versions: [fedora-rawhide] - decision_context: another_test_context - subject_type: koji_build - rules: - - !RemoteRule {} - """)) - mock_load_policies.return_value = policies + mock_retrieve_scm_from_koji.return_value = ('rpms', 'nethack', 'c3c47') config = TestingConfig() - config.DIST_GIT_BASE_URL = 'http://localhost.localdomain/' - config.DIST_GIT_URL_TEMPLATE = '{DIST_GIT_BASE_URL}{other_params}/blablabla/gating.yaml' - config.REMOTE_RULE_POLICIES = {} + config.DIST_GIT_BASE_URL = 'https://example.com/' + config.DIST_GIT_URL_TEMPLATE = '{DIST_GIT_BASE_URL}{pkg_namespace}/{pkg_name}/{rev}/gating.yaml' app = create_app(config) - assert app.config['DIST_GIT_URL_TEMPLATE'] == ( - 'http://localhost.localdomain/{other_params}/blablabla/gating.yaml' - ) - - -def test_can_use_remote_rule_http_fallback(): - """ Test that _can_use_remote_rule verifies the configuration properly if HTTP is used. """ - config = { - 'KOJI_BASE_URL': 'https://koji.domain.local/kojihub', - 'DIST_GIT_URL_TEMPLATE': - 'https://dist-git.domain.local/{pkg_namespace}{pkg_name}/raw/{rev}/f/gating.yaml' - } - assert _can_use_remote_rule(config) is True - - -def test_can_use_remote_rule_http(): - """ Test that _can_use_remote_rule verifies the configuration properly if HTTP is used. """ - config = { - 'KOJI_BASE_URL': 'https://koji.domain.local/kojihub', - 'REMOTE_RULE_POLICIES': { - '*': { - 'HTTP_URL_TEMPLATE': 'https://src.fedoraproject.org/{pkg_namespace}{pkg_name}/' - 'raw/{rev}/f/gating.yaml' - } + with app.app_context(): + response = mock.MagicMock() + response.status_code = 404 + mock_session.request.return_value = response + policy = app.config['policies'][0] + remote_rule = policy.rules[0] + subject = create_subject('koji_build', 'nethack-1.2.3-1.el9000') + + assert remote_rule._get_sub_policies(policy, subject) is None + expected_call1 = mock.call( + 'HEAD', 'https://example.com/rpms//nethack/c3c47/gating.yaml') + assert mock_session.request.mock_calls == [expected_call1] + + app.config['REMOTE_RULE_POLICIES'] = { + 'koji_build': 'https://example2.com/{pkg_namespace}/{pkg_name}.yaml' } - } - assert _can_use_remote_rule(config) is True - -@pytest.mark.parametrize('config', ( - { - 'REMOTE_RULE_POLICIES': { - 'brew-build-group': { - 'GIT_URL': 'git@gitlab.cee.redhat.com:devops/greenwave-policies/side-tags.git', - 'GIT_PATH_TEMPLATE': '{pkg_namespace}/{pkg_name}.yaml' - } - }, - }, - { - 'KOJI_BASE_URL': 'https://koji.domain.local/kojihub' - } -)) -def test_can_use_remote_rule_missing_config(config): - """ Test that _can_use_remote_rule will return False if a configuration is missing. """ - assert _can_use_remote_rule(config) is False + assert remote_rule._get_sub_policies(policy, subject) is None + expected_call2 = mock.call( + 'HEAD', 'https://example2.com/rpms//nethack.yaml') + assert mock_session.request.mock_calls == [expected_call1, expected_call2] diff --git a/greenwave/tests/test_retrieve_gating_yaml.py b/greenwave/tests/test_retrieve_gating_yaml.py index 75a68ea..4692e26 100644 --- a/greenwave/tests/test_retrieve_gating_yaml.py +++ b/greenwave/tests/test_retrieve_gating_yaml.py @@ -1,7 +1,5 @@ # SPDX-License-Identifier: GPL-2.0+ -import subprocess -import io import socket from requests.exceptions import ConnectionError, HTTPError @@ -123,11 +121,13 @@ def test_retrieve_yaml_remote_rule_no_namespace(): response.status_code = 404 session.request.return_value = response retrieve_yaml_remote_rule( - "deadbeaf", "pkg", "", app.config['REMOTE_RULE_POLICIES']['*'] + "abcdef", "deadbeaf", "pkg", + app.config['DIST_GIT_URL_TEMPLATE'], + app.config['DIST_GIT_BASE_URL'] ) expected_call = mock.call( - 'HEAD', 'https://src.fedoraproject.org/pkg/raw/deadbeaf/f/gating.yaml') + 'HEAD', 'https://src.fedoraproject.org/pkg//deadbeaf/raw/abcdef/f/gating.yaml') assert session.request.mock_calls == [expected_call] @@ -143,78 +143,17 @@ def test_retrieve_yaml_remote_rule_connection_error(): with pytest.raises(HTTPError) as excinfo: retrieve_yaml_remote_rule( - "deadbeaf", "pkg", "", app.config['REMOTE_RULE_POLICIES']['*'] + "abcdef", "deadbeaf", "pkg", + app.config['DIST_GIT_URL_TEMPLATE'], + app.config['DIST_GIT_BASE_URL'] ) assert str(excinfo.value) == ( '502 Server Error: Something went terribly wrong... for url: ' - 'https://src.fedoraproject.org/pkg/raw/deadbeaf/f/gating.yaml' + 'https://src.fedoraproject.org/pkg//deadbeaf/raw/abcdef/f/gating.yaml' ) -@mock.patch('tarfile.open') -@mock.patch('subprocess.Popen') -def test_retrieve_yaml_remote_rule_git_archive(mock_subp, mock_tar): - # Make the git archive call return bytes - mock_subp.return_value.communicate.return_value = (b'tar file', '') - mock_subp.return_value.returncode = 0 - # Make the tar archive, based on the return value of git archive, return a file-like - # object representing the gating.yaml file - mock_tar.return_value.extractfile.return_value = io.BytesIO(b'some gating yaml file') - - app = greenwave.app_factory.create_app() - rr_config = { - 'GIT_URL': 'git://dist-git.domain.local/abc/abc.git', - 'GIT_PATH_TEMPLATE': '{pkg_namespace}/{pkg_name}.yaml' - } - with app.app_context(): - gating_yaml = retrieve_yaml_remote_rule('abcdef', 'python-requests', 'rpms', rr_config) - - assert gating_yaml == 'some gating yaml file' - expected_cmd = [ - 'git', 'archive', '--remote=git://dist-git.domain.local/abc/abc.git', - 'master', 'rpms/python-requests.yaml'] - mock_subp.assert_called_once_with(expected_cmd, stderr=subprocess.PIPE, stdout=subprocess.PIPE) - tar_file = mock_tar.call_args[1]['fileobj'].read() - assert tar_file == b'tar file' - mock_tar.return_value.extractfile.assert_called_once_with('rpms/python-requests.yaml') - - -@mock.patch('subprocess.Popen') -def test_retrieve_yaml_remote_rule_git_archive_no_file(mock_subp): - # Make the git archive command return an error saying the file isn't in the repo - mock_subp.return_value.communicate.return_value = \ - (None, b'remote: fatal: path not found: xxx.yaml') - mock_subp.return_value.returncode = 1 - - app = greenwave.app_factory.create_app() - rr_config = { - 'GIT_URL': 'git://dist-git.domain.local/abc/abc.git', - 'GIT_PATH_TEMPLATE': '{pkg_namespace}/{pkg_name}.yaml' - } - with app.app_context(): - gating_yaml = retrieve_yaml_remote_rule('master', 'python-requests', 'rpms', rr_config) - - assert gating_yaml is None - - -@mock.patch('subprocess.Popen') -def test_retrieve_yaml_remote_rule_git_archive_error(mock_subp): - # Make the git archive command return an error - mock_subp.return_value.communicate.return_value = (None, b'remote: fatal: some error') - mock_subp.return_value.returncode = 1 - - app = greenwave.app_factory.create_app() - rr_config = { - 'GIT_URL': 'git://dist-git.domain.local/abc/abc.git', - 'GIT_PATH_TEMPLATE': '{pkg_namespace}/{pkg_name}.yaml' - } - expected_error = 'Error occurred while retrieving a remote rule file from the repo.' - with pytest.raises(BadGateway, match=expected_error): - with app.app_context(): - retrieve_yaml_remote_rule('master', 'python-requests', 'rpms', rr_config) - - @mock.patch('greenwave.resources.xmlrpc.client.ServerProxy') def test_retrieve_scm_from_koji_build_socket_error(mock_xmlrpc_client): mock_auth_server = mock_xmlrpc_client.return_value