From c847939ef0c649e7323c3374bda16a4b9caac005 Mon Sep 17 00:00:00 2001 From: mprahl Date: Feb 26 2019 14:34:52 +0000 Subject: [PATCH 1/2] Automatically set the TEST environment variable when running the functional tests --- diff --git a/functional-tests/conftest.py b/functional-tests/conftest.py index 32bd120..f658e7c 100644 --- a/functional-tests/conftest.py +++ b/functional-tests/conftest.py @@ -82,6 +82,7 @@ def server_subprocess( raise RuntimeError('{} source tree {} does not exist'.format(name, source_path)) env = dict(os.environ, PYTHONPATH=source_path) + env['TEST'] = 'true' # Write out a config if settings_content is not None: From 533ff6b00816f49bc211d2b03b186fa410a4caac Mon Sep 17 00:00:00 2001 From: mprahl Date: Feb 26 2019 14:34:52 +0000 Subject: [PATCH 2/2] Add the option to use "git archive" to retrieve a gating.yaml file from dist-git This is to address when the dist-git deployment doesn't have a UI that updates in real-time, such as cgit. --- diff --git a/Dockerfile b/Dockerfile index 2a1d61a..4960881 100644 --- a/Dockerfile +++ b/Dockerfile @@ -10,6 +10,7 @@ ARG cacert_url=undefined WORKDIR /src RUN dnf -y install \ + git-core \ python3-dogpile-cache \ python3-fedmsg \ python3-flask \ diff --git a/docs/policies.rst b/docs/policies.rst index 4196b0a..895e4b0 100644 --- a/docs/policies.rst +++ b/docs/policies.rst @@ -257,9 +257,18 @@ Greenwave will check if a gating.yaml 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``, -``DIST_GIT_BASE_URL`` and ``DIST_GIT_URL_TEMPLATE``. Here's the default -for the Fedora instance: +Greenwave has two mechanisms to retrieve the gating.yaml file: ``git archive``, +and using a git front-end. The ``git archive`` mechanism is preferred but +the dist-git server may not support it. + +Below is an example configuration for using the ``git archive`` mechanism: + +.. code-block:: console + + DIST_GIT_BASE_URL = 'git://src.fedoraproject.org' + KOJI_BASE_URL = 'https://koji.fedoraproject.org/kojihub' + +Below is an example configuration for using the git front-end mechanism: .. code-block:: console diff --git a/greenwave/app_factory.py b/greenwave/app_factory.py index 96c30d9..276fe6f 100644 --- a/greenwave/app_factory.py +++ b/greenwave/app_factory.py @@ -1,6 +1,7 @@ # SPDX-License-Identifier: GPL-2.0+ import logging +import shutil from flask import Flask from greenwave.api_v1 import api @@ -15,11 +16,15 @@ log = logging.getLogger(__name__) def _can_use_remote_rule(config): - return ( - config.get('DIST_GIT_BASE_URL') and - config.get('DIST_GIT_URL_TEMPLATE') and - config.get('KOJI_BASE_URL') - ) + # Ensure that the required config settings are set for both retrieval mechanisms + if not config.get('DIST_GIT_BASE_URL') or not config.get('KOJI_BASE_URL'): + return False + + if config['DIST_GIT_BASE_URL'].startswith('git://'): + # Ensure the git CLI is installed + return bool(shutil.which('git')) + else: + return bool(config.get('DIST_GIT_URL_TEMPLATE')) def _has_remote_rule(policies): @@ -44,10 +49,11 @@ def create_app(config_obj=None): 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 need to configure 'DIST_GIT_BASE_URL'," - "'DIST_GIT_URL_TEMPLATE' and KOJI_BASE_URL in " - "your configuration.") + 'If you want to apply a RemoteRule, you must have "DIST_GIT_BASE_URL" and ' + '"KOJI_BASE_URL" set in your configuration. Additionally, if you are using the ' + '"git archive" mechanism, the git CLI needs to be installed. If you are not, ' + 'then you must set "DIST_GIT_URL_TEMPLATE" in your configuration.' + ) # register error handlers for code in default_exceptions.keys(): diff --git a/greenwave/config.py b/greenwave/config.py index d3f0563..b0a0977 100644 --- a/greenwave/config.py +++ b/greenwave/config.py @@ -17,10 +17,11 @@ class Config(object): RESULTSDB_API_URL = 'https://taskotron.fedoraproject.org/resultsdb_api/api/v2.0' WAIVERDB_API_URL = 'https://waiverdb.fedoraproject.org/api/v1.0' - # Options for outbound HTTP requests made by python-requests + # Remote rule configuration 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' 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/resources.py b/greenwave/resources.py index d200618..8248dc1 100644 --- a/greenwave/resources.py +++ b/greenwave/resources.py @@ -11,6 +11,9 @@ import re import json import requests import urllib3.exceptions +from io import BytesIO +import tarfile +import subprocess from urllib.parse import urlparse import xmlrpc.client @@ -175,6 +178,17 @@ def retrieve_scm_from_koji_build(nvr, build, koji_url): @cached def retrieve_yaml_remote_rule(rev, pkg_name, pkg_namespace): """ Retrieve cached gating.yaml content for a given rev. """ + if current_app.config['DIST_GIT_BASE_URL'].startswith('git://'): + return _retrieve_yaml_remote_rule_git_archive(rev, pkg_name, pkg_namespace) + else: + return _retrieve_yaml_remote_rule_web(rev, pkg_name, pkg_namespace) + + +_retrieve_gating_yaml_error = 'Error occurred looking for gating.yaml file in the dist-git repo.' + + +def _retrieve_yaml_remote_rule_web(rev, pkg_name, pkg_namespace): + """ Retrieve the gating.yaml file from the dist-git web UI. """ data = { "DIST_GIT_BASE_URL": (current_app.config['DIST_GIT_BASE_URL'].rstrip('/') + ('/' if pkg_namespace else '')), @@ -190,7 +204,7 @@ def retrieve_yaml_remote_rule(rev, pkg_name, pkg_namespace): return None if response.status_code != 200: - raise BadGateway('Error occurred looking for gating.yaml file in the dist-git repo.') + raise BadGateway(_retrieve_gating_yaml_error) # gating.yaml found... response = requests_session.request('GET', url, @@ -200,6 +214,30 @@ def retrieve_yaml_remote_rule(rev, pkg_name, pkg_namespace): return response.content +def _retrieve_yaml_remote_rule_git_archive(rev, pkg_name, pkg_namespace): + """ Retrieve the gating.yaml file from a dist-git repo using git archive. """ + dist_git_base_url = current_app.config['DIST_GIT_BASE_URL'].rstrip('/') + dist_git_url = f'{dist_git_base_url}/{pkg_namespace}/{pkg_name}' + cmd = ['git', 'archive', f'--remote={dist_git_url}', rev, 'gating.yaml'] + git_archive = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + output, error_output = git_archive.communicate() + + 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_gating_yaml_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 + gating_yaml_archive = tarfile.open(fileobj=BytesIO(output)) + gating_yaml = gating_yaml_archive.extractfile('gating.yaml').read().decode('utf-8') + return gating_yaml + + # NOTE - not cached, for now. @greenwave.utils.retry(wait_on=urllib3.exceptions.NewConnectionError) def retrieve_waivers(product_version, subject_type, subject_identifiers): diff --git a/greenwave/tests/test_app_factory.py b/greenwave/tests/test_app_factory.py index 85591ea..ec832da 100644 --- a/greenwave/tests/test_app_factory.py +++ b/greenwave/tests/test_app_factory.py @@ -5,7 +5,7 @@ import pytest from textwrap import dedent -from greenwave.app_factory import create_app +from greenwave.app_factory import create_app, _can_use_remote_rule from greenwave.policies import Policy from greenwave.config import TestingConfig @@ -31,7 +31,51 @@ def test_remote_rules_misconfigured(mock_load_policies): config = TestingConfig() config.DIST_GIT_BASE_URL = '' - expected_error = 'If you want to apply a RemoteRule you need to configure' + expected_error = 'If you want to apply a RemoteRule' with pytest.raises(RuntimeError, match=expected_error): create_app(config) + + +@mock.patch('shutil.which') +def test_can_use_remote_rule_http(mock_which): + """ Test that _can_use_remote_rule verifies the configuration properly if HTTP is used. """ + mock_which.return_value = None + config = { + 'DIST_GIT_BASE_URL': 'https://dist-git.domain.local', + 'KOJI_BASE_URL': 'https://koji.domain.local/kojihub', + 'DIST_GIT_URL_TEMPLATE': ('{DIST_GIT_BASE_URL}{pkg_namespace}/{pkg_name}/raw/{rev}/f/' + 'gating.yaml') + } + assert _can_use_remote_rule(config) is True + mock_which.assert_not_called() + + +@pytest.mark.parametrize('git_installed', (True, False)) +@mock.patch('shutil.which') +def test_can_use_remote_rule_git_archive(mock_which, git_installed): + """ Test that _can_use_remote_rule checks if git is installed if git archive is used. """ + mock_which.return_value = '/usr/bin/git' if git_installed else None + config = { + 'DIST_GIT_BASE_URL': 'git://dist-git.domain.local', + 'KOJI_BASE_URL': 'https://koji.domain.local/kojihub' + } + assert _can_use_remote_rule(config) is git_installed + mock_which.assert_called_once_with('git') + + +@pytest.mark.parametrize('config', ( + { + 'DIST_GIT_BASE_URL': 'git://dist-git.domain.local', + }, + { + 'DIST_GIT_BASE_URL': 'https://dist-git.domain.local', + }, + { + 'DIST_GIT_BASE_URL': 'https://dist-git.domain.local', + '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 diff --git a/greenwave/tests/test_retrieve_gating_yaml.py b/greenwave/tests/test_retrieve_gating_yaml.py index cdd484d..d2c5c17 100644 --- a/greenwave/tests/test_retrieve_gating_yaml.py +++ b/greenwave/tests/test_retrieve_gating_yaml.py @@ -1,8 +1,10 @@ # SPDX-License-Identifier: GPL-2.0+ +import subprocess +import io + import pytest import mock - from werkzeug.exceptions import BadGateway import greenwave.app_factory @@ -93,3 +95,63 @@ def test_retrieve_yaml_remote_rule_no_namespace(): 'https://src.fedoraproject.org/pkg/raw/deadbeaf/f/gating.yaml', headers={'Content-Type': 'application/json'}, timeout=60) assert session.request.mock_calls == [expected_call] + + +@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() + # Set DIST_GIT_BASE_URL to start with `git://` so that it causes retrieve_yaml_remote_rule + # to use git archive instead of HTTP to get the gating.yaml file + app.config['DIST_GIT_BASE_URL'] = 'git://dist-git.domain.local' + with app.app_context(): + gating_yaml = retrieve_yaml_remote_rule('85e796daaa', 'python-requests', 'rpms') + + assert gating_yaml == 'some gating yaml file' + expected_cmd = [ + 'git', 'archive', '--remote=git://dist-git.domain.local/rpms/python-requests', + '85e796daaa', 'gating.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('gating.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: gating.yaml') + mock_subp.return_value.returncode = 1 + + app = greenwave.app_factory.create_app() + # Set DIST_GIT_BASE_URL to start with `git://` so that it causes retrieve_yaml_remote_rule + # to use git archive instead of HTTP to get the gating.yaml file + app.config['DIST_GIT_BASE_URL'] = 'git://dist-git.domain.local' + with app.app_context(): + gating_yaml = retrieve_yaml_remote_rule('85e796daaa', 'python-requests', 'rpms') + + 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() + # Set DIST_GIT_BASE_URL to start with `git://` so that it causes retrieve_yaml_remote_rule + # to use git archive instead of HTTP to get the gating.yaml file + app.config['DIST_GIT_BASE_URL'] = 'git://dist-git.domain.local' + expected_error = 'Error occurred looking for gating.yaml file in the dist-git repo.' + with pytest.raises(BadGateway, match=expected_error): + with app.app_context(): + retrieve_yaml_remote_rule('85e796daaa', 'python-requests', 'rpms')