From 796a3674573bfa3fe78850129d3e0c84f9985b31 Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Aug 15 2019 08:49:03 +0000 Subject: [PATCH 1/2] Add "scratch_build_only_branches" configuration options. The goal here is to define certain branches from which only scratch module builds can be submitted. The main use case is for "private-*" branches which can be created and maintained by anyone, but there must not be production-ready module build created from them. This commit adds new `scratch_build_only_branches` config option to define the list of regexes to match such branches. --- diff --git a/module_build_service/config.py b/module_build_service/config.py index 7cdbbf4..ccab1bf 100644 --- a/module_build_service/config.py +++ b/module_build_service/config.py @@ -640,6 +640,12 @@ class Config(object): "default": False, "desc": "Allow module scratch builds", }, + "scratch_build_only_branches": { + "type": list, + "default": [], + "desc": "The list of regexes used to identify branches from which only the module " + "scratch builds can be built", + }, "product_pages_url": { "type": str, "default": "", diff --git a/module_build_service/utils/submit.py b/module_build_service/utils/submit.py index 614e953..d8544ac 100644 --- a/module_build_service/utils/submit.py +++ b/module_build_service/utils/submit.py @@ -1003,6 +1003,18 @@ def submit_module_build(db_session, username, mmd, params): # append incrementing counter to context context_suffix = "_" + str(len(scrmods) + 1) mmd.set_context(mmd.get_context() + context_suffix) + else: + # In case the branch is defined, check whether user is allowed to submit + # non-scratch build from this branch. Note that the branch is always defined + # for official builds from SCM, because it is requested in views.py. + branch = params.get("branch", None) + if branch: + for regex in conf.scratch_build_only_branches: + branch_search = re.search(regex, branch) + if branch_search: + raise ValidationError( + "Only scratch module builds can be build from this branch." + ) log.debug("Creating new module build") module = models.ModuleBuild.create( diff --git a/tests/test_utils/test_utils.py b/tests/test_utils/test_utils.py index fd11623..78ae51c 100644 --- a/tests/test_utils/test_utils.py +++ b/tests/test_utils/test_utils.py @@ -38,6 +38,7 @@ from tests import ( init_data, scheduler_init_data, make_module_in_db, + make_module, read_staged_data, staged_data_filename) import mock import koji @@ -1051,6 +1052,31 @@ class TestUtils: assert builds[0].siblings(db_session) == [builds[1].id] assert builds[1].siblings(db_session) == [builds[0].id] + @patch("module_build_service.utils.mse.generate_expanded_mmds") + @patch( + "module_build_service.config.Config.scratch_build_only_branches", + new_callable=mock.PropertyMock, + return_value=["^private-.*"], + ) + def test_submit_build_scratch_build_only_branches( + self, cfg, generate_expanded_mmds, db_session): + """ + Tests the "scratch_build_only_branches" config option. + """ + mmd = make_module("foo:stream:0:c1") + generate_expanded_mmds.return_value = [mmd] + # Create a copy of mmd1 without xmd.mbs, since that will cause validate_mmd to fail + mmd_copy = mmd.copy() + mmd_copy.set_xmd({}) + + with pytest.raises(ValidationError, + match="Only scratch module builds can be build from this branch."): + module_build_service.utils.submit_module_build( + db_session, "foo", mmd_copy, {"branch": "private-foo"}) + + module_build_service.utils.submit_module_build( + db_session, "foo", mmd_copy, {"branch": "otherbranch"}) + class DummyModuleBuilder(GenericBuilder): """ From 80fca557af61b0448850488517d007dca986d936 Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Aug 21 2019 11:17:04 +0000 Subject: [PATCH 2/2] Do not check Greenwave gating status for scratch builds. Scratch builds cannot be gated. They stay in the `done` state forever. Therefore it is useless to query Greenwave for its status in the Poller. --- diff --git a/module_build_service/scheduler/producer.py b/module_build_service/scheduler/producer.py index 55932d5..615ada2 100644 --- a/module_build_service/scheduler/producer.py +++ b/module_build_service/scheduler/producer.py @@ -482,7 +482,7 @@ class MBSProducer(PollingProducer): module_builds = ( db_session.query(models.ModuleBuild) - .filter_by(state=models.BUILD_STATES["done"]).all() + .filter_by(state=models.BUILD_STATES["done"], scratch=False).all() ) log.info("Checking Greenwave for %d builds", len(module_builds)) diff --git a/module_build_service/utils/submit.py b/module_build_service/utils/submit.py index d8544ac..1f2a888 100644 --- a/module_build_service/utils/submit.py +++ b/module_build_service/utils/submit.py @@ -1007,7 +1007,7 @@ def submit_module_build(db_session, username, mmd, params): # In case the branch is defined, check whether user is allowed to submit # non-scratch build from this branch. Note that the branch is always defined # for official builds from SCM, because it is requested in views.py. - branch = params.get("branch", None) + branch = params.get("branch") if branch: for regex in conf.scratch_build_only_branches: branch_search = re.search(regex, branch) diff --git a/tests/test_scheduler/test_poller.py b/tests/test_scheduler/test_poller.py index ab667d2..7549923 100644 --- a/tests/test_scheduler/test_poller.py +++ b/tests/test_scheduler/test_poller.py @@ -22,7 +22,7 @@ import re import pytest from mock import patch from module_build_service import models, conf -from tests import clean_database +from tests import clean_database, make_module_in_db import mock import koji from module_build_service.scheduler.producer import MBSProducer @@ -710,8 +710,12 @@ class TestPoller: module_build2 = models.ModuleBuild.get_by_id(db_session, 2) module_build2.state = models.BUILD_STATES["done"] - module_build2 = models.ModuleBuild.get_by_id(db_session, 3) - module_build2.state = models.BUILD_STATES["init"] + module_build3 = models.ModuleBuild.get_by_id(db_session, 3) + module_build3.state = models.BUILD_STATES["init"] + + module_build4 = make_module_in_db("foo:1:1:1", {}, db_session=db_session) + module_build4.state = models.BUILD_STATES["done"] + module_build4.scratch = True db_session.commit() @@ -737,6 +741,10 @@ class TestPoller: assert len(modules) == 1 assert modules[0].id == 1 modules = models.ModuleBuild.by_state(db_session, "done") - assert len(modules) == 1 - assert modules[0].id == 2 - assert re.match("Gating failed.*", modules[0].state_reason) + assert len(modules) == 2 + for module in modules: + assert module.id in [2, 4] + if module.id == 2: + assert re.match("Gating failed.*", module.state_reason) + else: + assert module.state_reason is None