From 5e7fd1058314eb476d51c4a19a390248237e3500 Mon Sep 17 00:00:00 2001 From: Chenxiong Qi Date: Oct 25 2018 09:32:49 +0000 Subject: Check transitive stream collision A transitive stream collision could happen in such a case, for example, modulea buildrequires foo:1 and bar:1, then foo:1 requires baz:1 and bar:1 requires baz:2, which will cause the collision on modules baz:1 and baz:2. This patch checks the collision when submit a module build and each expanded (module stream expansion) module will be check. If a module has collision, error will be raised immediately. Resolves: FACTORY-2612 Signed-off-by: Chenxiong Qi --- diff --git a/module_build_service/__init__.py b/module_build_service/__init__.py index cd5016f..1b5db9b 100644 --- a/module_build_service/__init__.py +++ b/module_build_service/__init__.py @@ -54,7 +54,7 @@ from module_build_service.logger import ( from module_build_service.errors import ( ValidationError, Unauthorized, UnprocessableEntity, Conflict, NotFound, - Forbidden, json_error) + Forbidden, json_error, TransitiveStreamCollision) from module_build_service.config import init_config from module_build_service.proxy import ReverseProxy @@ -153,6 +153,12 @@ def notfound_error(e): return json_error(404, 'Not Found', str(e)) +@app.errorhandler(TransitiveStreamCollision) +def transitivestreamcollision_error(e): + """Flask error handler for TransitiveStreamCollision exceptions""" + return json_error(400, 'Bad Request', str(e)) + + init_logging(conf) log = getLogger(__name__) build_logs = ModuleBuildLogs( diff --git a/module_build_service/errors.py b/module_build_service/errors.py index d2fc4aa..67a964d 100644 --- a/module_build_service/errors.py +++ b/module_build_service/errors.py @@ -62,3 +62,7 @@ def json_error(status, error, message): 'message': message}) response.status_code = status return response + + +class TransitiveStreamCollision(ValueError): + pass diff --git a/module_build_service/utils/submit.py b/module_build_service/utils/submit.py index bd9974c..d0fa5c0 100644 --- a/module_build_service/utils/submit.py +++ b/module_build_service/utils/submit.py @@ -41,7 +41,8 @@ from module_build_service import conf, db, log, models, Modulemd from module_build_service.errors import ( ValidationError, UnprocessableEntity, Forbidden, Conflict) from module_build_service import glib -from .mse import generate_expanded_mmds +from module_build_service.utils import mse +from module_build_service.errors import TransitiveStreamCollision def record_filtered_rpms(mmd): @@ -441,7 +442,7 @@ def submit_module_build(username, url, mmd, optional_params=None): default_streams = {} validate_mmd(mmd) - mmds = generate_expanded_mmds(db.session, mmd, raise_if_stream_ambigous, default_streams) + mmds = mse.generate_expanded_mmds(db.session, mmd, raise_if_stream_ambigous, default_streams) modules = [] for mmd in mmds: @@ -483,6 +484,12 @@ def submit_module_build(username, url, mmd, optional_params=None): module.transition(conf, transition_to, "Resubmitted by %s" % username) log.info("Resumed existing module build in previous state %s" % module.state) else: + required_mmds = mse.get_mmds_required_by_module_recursively( + mmd, + default_streams=default_streams, + raise_if_stream_ambigous=raise_if_stream_ambigous) + check_transitive_stream_collision(required_mmds) + log.debug('Creating new module build') module = models.ModuleBuild.create( db.session, @@ -700,3 +707,26 @@ def load_local_builds(local_build_nsvs, session=None): 'Parsed metadata results for "{0}" don\'t match the directory name' .format(found_build[3])) log.info("Loaded local module build %r", module) + + +def check_transitive_stream_collision(mmds): + """Check if there is transitive stream collision modules through requires path + + Think about a case: modulea buildrequires foo:1 and bar:1, then foo:1 requires baz:1 + and bar:1 requires baz:2. As a result, the collision happens on baz:1 and baz:2. + + :param mmds: list of module metadata, each of which is a result of module + stream expansion. + :type mmds: list[Modulemd.Module] + :raises TransitiveStreamCollision: if collision is detected. + """ + name_streams = {} + for mmd in mmds: + name = mmd.get_name() + stream = mmd.get_stream() + if name in name_streams and name_streams[name] != stream: + raise TransitiveStreamCollision( + 'Transitive stream collision happens on {}:{} and {}:{}' + .format(name, name_streams[name], name, stream)) + else: + name_streams[name] = stream diff --git a/tests/test_utils/test_utils.py b/tests/test_utils/test_utils.py index e3d6f8d..5fa69de 100644 --- a/tests/test_utils/test_utils.py +++ b/tests/test_utils/test_utils.py @@ -18,27 +18,26 @@ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE # SOFTWARE. +import koji +import mock +import module_build_service.scheduler.handlers.components +import module_build_service.scm +import module_build_service.utils +import pytest import tempfile -from os import path, mkdir -from shutil import copyfile, rmtree + from datetime import datetime -from werkzeug.datastructures import FileStorage from mock import patch -import module_build_service.utils -import module_build_service.scm -from module_build_service import models, conf -from module_build_service.errors import ProgrammingError, ValidationError, UnprocessableEntity -from tests import ( - reuse_component_init_data, db, reuse_shared_userspace_init_data, clean_database, init_data, - scheduler_init_data) -import mock -import koji -import pytest -import module_build_service.scheduler.handlers.components from module_build_service.builder.base import GenericBuilder from module_build_service.builder.KojiModuleBuilder import KojiModuleBuilder -from module_build_service import glib, Modulemd -from tests import app +from module_build_service.errors import ProgrammingError, ValidationError, UnprocessableEntity +from module_build_service import glib, Modulemd, utils, models, conf +from os import path, mkdir +from shutil import copyfile, rmtree +from werkzeug.datastructures import FileStorage +from tests import ( + app, db, reuse_component_init_data, reuse_shared_userspace_init_data, + clean_database, init_data, scheduler_init_data, make_module) BASE_DIR = path.abspath(path.dirname(__file__)) @@ -1081,3 +1080,61 @@ class TestLocalBuilds: assert len(local_modules) == 1 assert local_modules[0].koji_tag.endswith( "/module-platform-f28-3/results") + + +class TestCheckTransitiveStreamCollision: + """Test submit.check_transitive_stream_collision""" + + def setup_method(self, test_method): + clean_database() + + def teardown_method(self, test_method): + clean_database() + + @staticmethod + def _generate_collision_modules(): + """Generate test modules + + gtk:1 requires bar:1 which requires platform:f29 + foo:1 requires bar:2 which requires platform:f29 + """ + make_module('gtk:1:1:c2', {'bar': ['1']}, {}) + make_module('foo:1:1:c2', {'bar': ['2']}, {}) + make_module('bar:1:0:c2', {'platform': ['f29']}, {}) + make_module('bar:2:0:c2', {'platform': ['f29']}, {}) + make_module('platform:f29:0:c11', {}, {}) + + @staticmethod + def _generate_modules_without_colliison(): + """Generate test modules + + gtk:1 requires bar:1 which requires platform:f29 + foo:1 requires baz:2 which requires platform:f29 + """ + make_module('gtk:1:1:c2', {'bar': ['1']}, {}) + make_module('foo:1:1:c2', {'baz': ['2']}, {}) + make_module('bar:1:0:c2', {'platform': ['f29']}, {}) + make_module('baz:2:0:c2', {'platform': ['f29']}, {}) + make_module('platform:f29:0:c11', {}, {}) + + @staticmethod + def _get_mmds_required_by_module_recursively(module_build): + mmd = module_build.mmd() + utils.mse.expand_mse_streams(db.session, mmd) + return utils.mse.get_mmds_required_by_module_recursively(mmd) + + def test_raise_error_when_there_is_collision(self): + # app:1 buildrequires gtk:1 and foo:1 + module_build = make_module("app:1:0:c1", {}, {'gtk': '1', 'foo': '1'}) + self._generate_collision_modules() + modules = self._get_mmds_required_by_module_recursively(module_build) + with pytest.raises(utils.submit.TransitiveStreamCollision) as excinfo: + utils.submit.check_transitive_stream_collision(modules) + assert 'bar:1 and bar:2' in str(excinfo.value) + + def test_no_error_raised_when_no_collision(self): + # app:1 buildrequires gtk:1 and foo:1 + module_build = make_module("app:1:0:c1", {}, {'gtk': '1', 'foo': '1'}) + self._generate_modules_without_colliison() + modules = self._get_mmds_required_by_module_recursively(module_build) + utils.submit.check_transitive_stream_collision(modules)