From be53a785d30527719db1f2bdb6859337eb1fb510 Mon Sep 17 00:00:00 2001 From: gnaponie Date: Sep 02 2019 08:04:58 +0000 Subject: Remove git archive code SHA1 hashes seem not to allowed when invoking git-archive. Since we need the "rev" field, this method is not the best approach for us. Removing this code. ref: FACTORY-4302 Signed-off-by: gnaponie --- diff --git a/Dockerfile b/Dockerfile index 871dd0b..b5c9dd6 100644 --- a/Dockerfile +++ b/Dockerfile @@ -10,7 +10,6 @@ ARG cacert_url=undefined WORKDIR /src RUN dnf -y install \ - git-core \ python3-dogpile-cache \ python3-fedmsg \ python3-flask \ diff --git a/Vagrantfile b/Vagrantfile index 044524a..a8fc358 100644 --- a/Vagrantfile +++ b/Vagrantfile @@ -5,7 +5,6 @@ $script = <<-'SCRIPT' set -e dnf -y install \ - git-core \ postgresql-server \ postgresql-contrib \ python3-gunicorn \ diff --git a/docs/policies.rst b/docs/policies.rst index a435107..247a3b9 100644 --- a/docs/policies.rst +++ b/docs/policies.rst @@ -263,18 +263,9 @@ 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 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: +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: .. code-block:: console diff --git a/greenwave/app_factory.py b/greenwave/app_factory.py index 18db278..882f243 100644 --- a/greenwave/app_factory.py +++ b/greenwave/app_factory.py @@ -1,8 +1,6 @@ # SPDX-License-Identifier: GPL-2.0+ import logging -import shutil - from flask import Flask from greenwave.api_v1 import api from greenwave.monitor import monitor_api @@ -17,15 +15,12 @@ log = logging.getLogger(__name__) def _can_use_remote_rule(config): - # 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')) + # Ensure that the required config settings are set + return (bool( + config.get('DIST_GIT_BASE_URL') and + config.get('DIST_GIT_URL_TEMPLATE') and + config.get('KOJI_BASE_URL')) + ) def _has_remote_rule(policies): @@ -50,10 +45,10 @@ 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 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.' + "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." ) # register error handlers diff --git a/greenwave/resources.py b/greenwave/resources.py index 195e74a..eda8380 100644 --- a/greenwave/resources.py +++ b/greenwave/resources.py @@ -9,9 +9,6 @@ waiverdb, etc..). import logging import re import json -from io import BytesIO -import tarfile -import subprocess import socket from urllib.parse import urlparse @@ -172,18 +169,7 @@ 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. """ + """ Retrieve cached gating.yaml content for a given rev from the dist-git web UI. """ data = { "DIST_GIT_BASE_URL": (current_app.config['DIST_GIT_BASE_URL'].rstrip('/') + ('/' if pkg_namespace else '')), @@ -199,7 +185,7 @@ def _retrieve_yaml_remote_rule_web(rev, pkg_name, pkg_namespace): return None if response.status_code != 200: - raise BadGateway(_retrieve_gating_yaml_error) + raise BadGateway('Error occurred looking for gating.yaml file in the dist-git repo.') # gating.yaml found... response = requests_session.request('GET', url, @@ -209,38 +195,6 @@ def _retrieve_yaml_remote_rule_web(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'] - # Retry thrice if TimeoutExpired exception is raised - MAX_RETRY = 3 - for _ in range(MAX_RETRY): - try: - git_archive = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) - output, error_output = git_archive.communicate(timeout=30) - break - except subprocess.TimeoutExpired: - 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_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. def retrieve_decision(greenwave_url, data): timeout = current_app.config['REQUESTS_TIMEOUT'] diff --git a/greenwave/tests/test_app_factory.py b/greenwave/tests/test_app_factory.py index ec832da..1915a1d 100644 --- a/greenwave/tests/test_app_factory.py +++ b/greenwave/tests/test_app_factory.py @@ -37,10 +37,8 @@ def test_remote_rules_misconfigured(mock_load_policies): create_app(config) -@mock.patch('shutil.which') -def test_can_use_remote_rule_http(mock_which): +def test_can_use_remote_rule_http(): """ 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', @@ -48,27 +46,10 @@ def test_can_use_remote_rule_http(mock_which): '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', }, { diff --git a/greenwave/tests/test_retrieve_gating_yaml.py b/greenwave/tests/test_retrieve_gating_yaml.py index 0705957..031198c 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 import pytest @@ -98,66 +96,6 @@ def test_retrieve_yaml_remote_rule_no_namespace(): 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') - - @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