From 9512a496310bc4e68cb1cc667f59eaa0ef7282b6 Mon Sep 17 00:00:00 2001 From: mprahl Date: Apr 04 2019 13:12:27 +0000 Subject: Allow whitelisted buildrequires with xmd.mbs.disttag_marking set to influnece the disttag --- diff --git a/module_build_service/config.py b/module_build_service/config.py index 52bc0e5..07b4ead 100644 --- a/module_build_service/config.py +++ b/module_build_service/config.py @@ -541,7 +541,14 @@ class Config(object): 'desc': 'The Greenwave decision context that whose messages should ' 'be handled by MBS. By default, MBS handles Greenwave ' 'messages for OSCI.', - } + }, + 'allowed_disttag_marking_module_names': { + 'type': list, + 'default': [], + 'desc': ('List of modules that are allowed to influence the RPM disttag when ' + 'buildrequired. These modules can set xmd.mbs.disttag_marking to do so. MBS ' + 'will use this list order to determine which modules take precedence.') + }, } def __init__(self, conf_section_obj): diff --git a/module_build_service/mmd_resolver.py b/module_build_service/mmd_resolver.py index 0b0ae52..49f03ac 100644 --- a/module_build_service/mmd_resolver.py +++ b/module_build_service/mmd_resolver.py @@ -262,13 +262,14 @@ class MMDResolver(object): :return: Dict with module name as a key and new stream as a value. """ overrides = {} - if "mbs" in mmd.get_xmd().keys(): + xmd = mmd.get_xmd() + if "mbs" in xmd.keys() and "buildrequires" in xmd["mbs"].keys(): for base_module_name in conf.base_module_names: - if base_module_name not in mmd.get_xmd()["mbs"]["buildrequires"].keys(): + if base_module_name not in xmd["mbs"]["buildrequires"].keys(): continue - if "stream" not in mmd.get_xmd()["mbs"]["buildrequires"][base_module_name].keys(): + if "stream" not in xmd["mbs"]["buildrequires"][base_module_name].keys(): continue - stream = mmd.get_xmd()["mbs"]["buildrequires"][base_module_name]["stream"] + stream = xmd["mbs"]["buildrequires"][base_module_name]["stream"] overrides[base_module_name] = stream return overrides diff --git a/module_build_service/utils/general.py b/module_build_service/utils/general.py index b8504ac..ef01fc9 100644 --- a/module_build_service/utils/general.py +++ b/module_build_service/utils/general.py @@ -240,36 +240,39 @@ def get_rpm_release(module_build): .format(module_build.id)) buildrequires = None - # Determine which base module is buildrequired and its marking in the disttag - base_module_marking = '' + # Determine which buildrequired module will influence the disttag + br_module_marking = '' # If the buildrequires are recorded in the xmd then we can try to find the base module that # is buildrequired if buildrequires: - # Looping through all the base modules in conf.base_module_names instead of looping through - # all the buildrequires guarantees the order in conf.base_module_names is preserved for - # which base module is used as the marking - for base_module in conf.base_module_names: - bm_in_xmd = buildrequires.get(base_module) + # Looping through all the non-base modules that are allowed to set the disttag_marking + # and the base modules to see what the disttag marking should be. Doing it this way + # preserves the order in the configurations. + for module in conf.allowed_disttag_marking_module_names + conf.base_module_names: + module_in_xmd = buildrequires.get(module) - if not bm_in_xmd: + if not module_in_xmd: continue with models.make_session(conf) as session: - base_module_obj = models.ModuleBuild.get_build_from_nsvc( - session, base_module, bm_in_xmd['stream'], bm_in_xmd['version'], - bm_in_xmd['context']) - if not base_module_obj: + module_obj = models.ModuleBuild.get_build_from_nsvc( + session, module, module_in_xmd['stream'], module_in_xmd['version'], + module_in_xmd['context']) + if not module_obj: continue - # Default to using the base module's stream, but if the base module has - # disttag_marking set in the xmd, use that instead try: - marking = base_module_obj.mmd().get_xmd()['mbs']['disttag_marking'] + marking = module_obj.mmd().get_xmd()['mbs']['disttag_marking'] # We must check for a KeyError because a Variant object doesn't support the `get` # method except KeyError: - marking = base_module_obj.stream - base_module_marking = marking + '+' + if module not in conf.base_module_names: + continue + # If we've made it past all the modules in + # conf.allowed_disttag_marking_module_names, and the base module doesn't have + # the disttag_marking set, then default to the stream of the first base module + marking = module_obj.stream + br_module_marking = marking + '+' break else: log.warning('Module build {0} does not buildrequire a base module ({1})' @@ -280,7 +283,7 @@ def get_rpm_release(module_build): return '{prefix}{base_module_marking}{index}+{dist_hash}'.format( prefix=prefix, - base_module_marking=base_module_marking, + base_module_marking=br_module_marking, index=index, dist_hash=dist_hash, ) diff --git a/module_build_service/utils/submit.py b/module_build_service/utils/submit.py index d416835..0d718b7 100644 --- a/module_build_service/utils/submit.py +++ b/module_build_service/utils/submit.py @@ -299,12 +299,16 @@ def validate_mmd(mmd): "Custom module repositories aren't allowed. " "%r bears repository %r" % (modname, mod.get_repository())) - if 'mbs' in mmd.get_xmd(): - raise ValidationError('The "mbs" xmd field is reserved for MBS') + name = mmd.get_name() + xmd = mmd.get_xmd() + if 'mbs' in xmd: + allowed_to_mark_disttag = name in conf.allowed_disttag_marking_module_names + if not (xmd['mbs'].keys() == ['disttag_marking'] and allowed_to_mark_disttag): + raise ValidationError('The "mbs" xmd field is reserved for MBS') - if mmd.get_name() in conf.base_module_names: + if name in conf.base_module_names: raise ValidationError( - 'You cannot build a module named "{}" since it is a base module'.format(mmd.get_name())) + 'You cannot build a module named "{}" since it is a base module'.format(name)) def merge_included_mmd(mmd, included_mmd): diff --git a/tests/staged_data/build_metadata_module.yaml b/tests/staged_data/build_metadata_module.yaml index f4b3b4c..519576b 100644 --- a/tests/staged_data/build_metadata_module.yaml +++ b/tests/staged_data/build_metadata_module.yaml @@ -26,3 +26,4 @@ data: commit: virtual requires: {} mse: true + disttag_marking: product12 diff --git a/tests/staged_data/build_metadata_module_not_processed.yaml b/tests/staged_data/build_metadata_module_not_processed.yaml new file mode 100644 index 0000000..59ec530 --- /dev/null +++ b/tests/staged_data/build_metadata_module_not_processed.yaml @@ -0,0 +1,17 @@ +document: modulemd +version: 1 +data: + description: A metadata-only module build for product X + name: build + license: + module: [MIT] + stream: product1.2 + summary: A metadata-only module build for a product X + dependencies: + buildrequires: + platform: f28 + requires: + platform: f28 + xmd: + mbs: + disttag_marking: 'product12' diff --git a/tests/test_scheduler/test_module_wait.py b/tests/test_scheduler/test_module_wait.py index 56ea992..dd3eedd 100644 --- a/tests/test_scheduler/test_module_wait.py +++ b/tests/test_scheduler/test_module_wait.py @@ -211,7 +211,7 @@ class TestModuleWait: @patch('module_build_service.resolver.DBResolver') @patch('module_build_service.resolver.GenericResolver') @patch("module_build_service.config.Config.base_module_names", - new_callable=mock.PropertyMock, return_value=set(["base-runtime", "platform"])) + new_callable=mock.PropertyMock, return_value=["base-runtime", "platform"]) def test_set_cg_build_koji_tag( self, cfg, generic_resolver, resolver, create_builder, dbg, koji_cg_tag_build, expected_cg_koji_build_tag): diff --git a/tests/test_utils/test_utils.py b/tests/test_utils/test_utils.py index 3041d1c..620d7de 100644 --- a/tests/test_utils/test_utils.py +++ b/tests/test_utils/test_utils.py @@ -405,6 +405,40 @@ class TestUtils: release = module_build_service.utils.get_rpm_release(build_one) assert release == 'module+fedora28+2+814cfa39' + @patch('module_build_service.config.Config.allowed_disttag_marking_module_names', + new_callable=mock.PropertyMock, return_value=['build']) + def test_get_rpm_release_metadata_br_stream_override(self, mock_admmn): + """ + Test that when a module buildrequires a module in conf.allowed_disttag_marking_module_names, + and that module has the xmd.mbs.disttag_marking field set, it should influence the disttag. + """ + scheduler_init_data(1) + mmd_path = path.abspath(path.join( + __file__, path.pardir, path.pardir, 'staged_data', 'build_metadata_module.yaml')) + metadata_mmd = module_build_service.utils.load_mmd(mmd_path, True) + module_build_service.utils.import_mmd(db.session, metadata_mmd) + + build_one = models.ModuleBuild.query.get(2) + mmd = build_one.mmd() + dep = mmd.get_dependencies()[0] + dep.add_buildrequires('build', ['product1.2']) + mmd.set_dependencies((dep, )) + xmd = glib.from_variant_dict(mmd.get_xmd()) + xmd['mbs']['buildrequires']['build'] = { + 'filtered_rpms': [], + 'ref': 'virtual', + 'stream': 'product1.2', + 'version': '1', + 'context': '00000000', + } + mmd.set_xmd(glib.dict_values(xmd)) + build_one.modulemd = to_text_type(mmd.dumps()) + db.session.add(build_one) + db.session.commit() + + release = module_build_service.utils.get_rpm_release(build_one) + assert release == 'module+product12+2+814cfa39' + def test_get_rpm_release_mse_scratch(self): init_data(contexts=True, scratch=True) build_one = models.ModuleBuild.query.get(2) diff --git a/tests/test_views/test_views.py b/tests/test_views/test_views.py index 34c2232..273870d 100644 --- a/tests/test_views/test_views.py +++ b/tests/test_views/test_views.py @@ -56,7 +56,7 @@ base_dir = dirname(dirname(__file__)) class FakeSCM(object): def __init__(self, mocked_scm, name, mmd_filenames, commit=None, checkout_raise=False, - get_latest_raise=False): + get_latest_raise=False, branch='master'): """ Adds default testing checkout, get_latest and name methods to mocked_scm SCM class. @@ -91,7 +91,7 @@ class FakeSCM(object): else: self.mocked_scm.return_value.get_latest = self.get_latest self.mocked_scm.return_value.repository_root = "https://src.stg.fedoraproject.org/modules/" - self.mocked_scm.return_value.branch = 'master' + self.mocked_scm.return_value.branch = branch self.mocked_scm.return_value.sourcedir = self.sourcedir self.mocked_scm.return_value.get_module_yaml = self.get_module_yaml @@ -2025,3 +2025,26 @@ class TestViews: rv = self.client.post(post_url, data=json.dumps({'branch': 'master', 'scmurl': scm_url})) assert rv.status_code == 201 + + @patch('module_build_service.auth.get_user', return_value=user) + @patch('module_build_service.scm.SCM') + @patch('module_build_service.config.Config.allowed_disttag_marking_module_names', + new_callable=PropertyMock, return_value=['build']) + def test_submit_build_with_disttag_marking_in_xmd( + self, mocked_admmn, mocked_scm, mocked_get_user): + """ + Test that white-listed modules may set the disttag_marking in xmd.mbs. + """ + FakeSCM(mocked_scm, 'build', 'build_metadata_module_not_processed.yaml', + '620ec77321b2ea7b0d67d82992dda3e1d67055b4', branch='product1.2') + + post_url = '/module-build-service/2/module-builds/' + scm_url = ('https://src.stg.fedoraproject.org/modules/testmodule.git?#68931c90de214d9d13fe' + 'efbd35246a81b6cb8d49') + + rv = self.client.post( + post_url, data=json.dumps({'branch': 'product1.2', 'scmurl': scm_url})) + assert rv.status_code == 201 + data = json.loads(rv.data)[0] + mmd = Modulemd.Module().new_from_string(data['modulemd']) + assert mmd.get_xmd()['mbs']['disttag_marking'] == 'product12'