diff --git a/docs/VIRTUAL_MODULES.rst b/docs/VIRTUAL_MODULES.rst index b1d5d8d5..e66e2228 100644 --- a/docs/VIRTUAL_MODULES.rst +++ b/docs/VIRTUAL_MODULES.rst @@ -67,7 +67,8 @@ Custom fields in xmd: - ``mse`` - this is an internal identifier used by MBS to know if this is a legacy module build prior to module stream expansion. This should always be ``TRUE``. - ``koji_tag`` - this defines the Koji tag with the RPMs that are part of this module. For base - modules this will likely be a tag representing a buildroot. + modules this will likely be a tag representing a buildroot. If this is a metadata-only module, + then this can be left unset. - ``virtual_streams`` - the list of streams which groups multiple modules together. For more information on this field, see the ``Virtual Streams`` section below. - ``disttag_marking`` - if this module is a base module, then MBS will use the stream of the base diff --git a/module_build_service/resolver/DBResolver.py b/module_build_service/resolver/DBResolver.py index 9e17fd9f..80a8e916 100644 --- a/module_build_service/resolver/DBResolver.py +++ b/module_build_service/resolver/DBResolver.py @@ -245,6 +245,11 @@ class DBResolver(GenericResolver): if not build: raise RuntimeError( 'Buildrequired module %s %r does not exist in MBS db' % (br_name, details)) + + # If the buildrequire is a meta-data only module with no Koji tag set, then just + # skip it + if build.koji_tag is None: + continue module_tags.setdefault(build.koji_tag, []) module_tags[build.koji_tag].append(build.mmd()) diff --git a/module_build_service/resolver/MBSResolver.py b/module_build_service/resolver/MBSResolver.py index efa260b2..c538796f 100644 --- a/module_build_service/resolver/MBSResolver.py +++ b/module_build_service/resolver/MBSResolver.py @@ -294,6 +294,10 @@ class MBSResolver(GenericResolver): db.session, name, details['stream']) if local_modules: for m in local_modules: + # If the buildrequire is a meta-data only module with no Koji tag set, then just + # skip it + if m.koji_tag is None: + continue module_tags[m.koji_tag] = m.mmd() continue @@ -305,6 +309,10 @@ class MBSResolver(GenericResolver): for m in modules: if m["koji_tag"] in module_tags: continue + # If the buildrequire is a meta-data only module with no Koji tag set, then just + # skip it + if m["koji_tag"] is None: + continue module_tags.setdefault(m["koji_tag"], []) module_tags[m["koji_tag"]].append(self.extract_modulemd(m["modulemd"])) diff --git a/module_build_service/utils/general.py b/module_build_service/utils/general.py index 6fdadfd3..b8504acd 100644 --- a/module_build_service/utils/general.py +++ b/module_build_service/utils/general.py @@ -364,14 +364,29 @@ def import_mmd(session, mmd): log.error(msg) raise UnprocessableEntity(msg) - # Get the koji_tag. - try: - xmd = mmd.get_xmd() - koji_tag = xmd["mbs"]["koji_tag"] - except KeyError: - msg = "'koji_tag' is not set in xmd['mbs'] for module {}".format(nsvc) - log.error(msg) - raise UnprocessableEntity(msg) + if len(mmd.get_dependencies()) > 1: + raise UnprocessableEntity( + "The imported module's dependencies list should contain just one element") + + xmd = glib.from_variant_dict(mmd.get_xmd()) + # Set some defaults in xmd["mbs"] if they're not provided by the user + if "mbs" not in xmd: + xmd["mbs"] = {"mse": True} + + if mmd.get_dependencies(): + brs = set(mmd.get_dependencies()[0].get_buildrequires().keys()) + xmd_brs = set(xmd["mbs"].get("buildrequires", {}).keys()) + if brs - xmd_brs: + raise UnprocessableEntity( + 'The imported module buildrequires other modules, but the metadata in the ' + 'xmd["mbs"]["buildrequires"] dictionary is missing entries') + elif "buildrequires" not in xmd["mbs"]: + xmd["mbs"]["buildrequires"] = {} + mmd.set_xmd(glib.dict_values(xmd)) + + koji_tag = xmd['mbs'].get('koji_tag') + if koji_tag is None: + log.warning("'koji_tag' is not set in xmd['mbs'] for module {}".format(nsvc)) # Get the ModuleBuild from DB. build = models.ModuleBuild.get_build_from_nsvc( @@ -397,6 +412,11 @@ def import_mmd(session, mmd): build.time_completed = datetime.utcnow() if build.name in conf.base_module_names: build.stream_version = models.ModuleBuild.get_stream_version(stream) + + # Record the base modules this module buildrequires + for base_module in build.get_buildrequired_base_modules(): + build.buildrequires.append(base_module) + session.add(build) session.commit() msg = "Module {} imported".format(nsvc) diff --git a/tests/scm_data/mariadb/objects/34/a463979786199c1b41ca21eb309cf3ce5ce789 b/tests/scm_data/mariadb/objects/34/a463979786199c1b41ca21eb309cf3ce5ce789 new file mode 100644 index 00000000..7ab09b50 Binary files /dev/null and b/tests/scm_data/mariadb/objects/34/a463979786199c1b41ca21eb309cf3ce5ce789 differ diff --git a/tests/scm_data/mariadb/objects/48/83b004d723a15cdc266c0280fe26c424ae10db b/tests/scm_data/mariadb/objects/48/83b004d723a15cdc266c0280fe26c424ae10db new file mode 100644 index 00000000..2f1a5664 Binary files /dev/null and b/tests/scm_data/mariadb/objects/48/83b004d723a15cdc266c0280fe26c424ae10db differ diff --git a/tests/scm_data/mariadb/objects/60/5345608fcc41d4e297cf062c653ebfdfe52476 b/tests/scm_data/mariadb/objects/60/5345608fcc41d4e297cf062c653ebfdfe52476 new file mode 100644 index 00000000..46ee1aad --- /dev/null +++ b/tests/scm_data/mariadb/objects/60/5345608fcc41d4e297cf062c653ebfdfe52476 @@ -0,0 +1,2 @@ +x]j1S)*jNCEy`xu`mM'?&yd$#Xp& +r`fH&bƒ"ɋH!+eon7ڲ4_6 3 Lp3 jF;.Ge˿Q;D 3G \ No newline at end of file diff --git a/tests/scm_data/mariadb/objects/6c/d6a18e15a4f23a1eaa7040e7f4da9be1aed36d b/tests/scm_data/mariadb/objects/6c/d6a18e15a4f23a1eaa7040e7f4da9be1aed36d new file mode 100644 index 00000000..ca45778f Binary files /dev/null and b/tests/scm_data/mariadb/objects/6c/d6a18e15a4f23a1eaa7040e7f4da9be1aed36d differ diff --git a/tests/scm_data/mariadb/objects/6e/a27c8576449ad5a12c1b6d27b50ff32c854ee5 b/tests/scm_data/mariadb/objects/6e/a27c8576449ad5a12c1b6d27b50ff32c854ee5 new file mode 100644 index 00000000..208590dc Binary files /dev/null and b/tests/scm_data/mariadb/objects/6e/a27c8576449ad5a12c1b6d27b50ff32c854ee5 differ diff --git a/tests/scm_data/mariadb/objects/7f/96fc61223e4bc4c93c7c286cca98f6efaa3716 b/tests/scm_data/mariadb/objects/7f/96fc61223e4bc4c93c7c286cca98f6efaa3716 new file mode 100644 index 00000000..327495b6 --- /dev/null +++ b/tests/scm_data/mariadb/objects/7f/96fc61223e4bc4c93c7c286cca98f6efaa3716 @@ -0,0 +1 @@ +xR0L\,:*\*i Ŋ\ZP!2$ׄF; 5;nYdBH79/ zxy>8Bqww\<F¾fM@i@ #*$N99bc%':C*cJ22fә3]3v痯jR|"0I~ Q*Wn gIaH>R[dw֑ mU:RlcZ'g ;._ ]]Jn/鷲^]RZe,:$0/&I(#ܗ,vp+f2x&=F-!d%*%|0a0cfؿ_R*zPhm. +#Wfm%RjN=@ѣ0 ta5$#?c=:|mv]n+|vK98@{xh8澝]nbO1&IM^vDk \ No newline at end of file diff --git a/tests/scm_data/mariadb/objects/fb/9351bd6f2439a73d78de0822fefe64410f905c b/tests/scm_data/mariadb/objects/fb/9351bd6f2439a73d78de0822fefe64410f905c new file mode 100644 index 00000000..0b94117d Binary files /dev/null and b/tests/scm_data/mariadb/objects/fb/9351bd6f2439a73d78de0822fefe64410f905c differ diff --git a/tests/scm_data/mariadb/objects/fc/399d16e46feb3af72a692b316a241c21bae1d0 b/tests/scm_data/mariadb/objects/fc/399d16e46feb3af72a692b316a241c21bae1d0 new file mode 100644 index 00000000..c8580fac Binary files /dev/null and b/tests/scm_data/mariadb/objects/fc/399d16e46feb3af72a692b316a241c21bae1d0 differ diff --git a/tests/scm_data/mariadb/refs/heads/master b/tests/scm_data/mariadb/refs/heads/master new file mode 100644 index 00000000..dbbdd382 --- /dev/null +++ b/tests/scm_data/mariadb/refs/heads/master @@ -0,0 +1 @@ +e3ca243c664440cdb8b704b0d5c32ed433bee4da diff --git a/tests/staged_data/build_metadata_module.yaml b/tests/staged_data/build_metadata_module.yaml new file mode 100644 index 00000000..f4b3b4c0 --- /dev/null +++ b/tests/staged_data/build_metadata_module.yaml @@ -0,0 +1,28 @@ +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 + version: 1 + context: 00000000 + dependencies: + buildrequires: + platform: f28 + requires: + platform: f28 + xmd: + mbs: + buildrequires: + platform: + filtered_rpms: [] + ref: virtual + stream: f28 + version: '3' + context: '00000000' + commit: virtual + requires: {} + mse: true diff --git a/tests/staged_data/testmodule_br_metadata_module.yaml b/tests/staged_data/testmodule_br_metadata_module.yaml new file mode 100644 index 00000000..3cba79f3 --- /dev/null +++ b/tests/staged_data/testmodule_br_metadata_module.yaml @@ -0,0 +1,38 @@ +document: modulemd +version: 1 +data: + summary: A test module in all its beautiful beauty + description: >- + This module demonstrates how to write simple modulemd files And + can be used for testing the build and release pipeline. ’ + license: + module: [ MIT ] + dependencies: + buildrequires: + platform: f28 + build: product1.2 + requires: + platform: f28 + references: + community: https://docs.pagure.org/modularity/ + documentation: https://fedoraproject.org/wiki/Fedora_Packaging_Guidelines_for_Modules + profiles: + default: + rpms: + - tangerine + api: + rpms: + - perl-Tangerine + - tangerine + components: + rpms: + perl-List-Compare: + rationale: A dependency of tangerine. + ref: master + perl-Tangerine: + rationale: Provides API for this module and is a dependency of tangerine. + ref: master + tangerine: + rationale: Provides API for this module. + buildorder: 10 + ref: master diff --git a/tests/test_build/test_build.py b/tests/test_build/test_build.py index 41cffc0e..eea8ccda 100644 --- a/tests/test_build/test_build.py +++ b/tests/test_build/test_build.py @@ -106,6 +106,7 @@ class FakeModuleBuilder(GenericBuilder): on_finalize_cb = None on_buildroot_add_artifacts_cb = None on_tag_artifacts_cb = None + on_buildroot_add_repos_cb = None @module_build_service.utils.validate_koji_tag('tag_name') def __init__(self, owner, module, config, tag_name, components): @@ -122,6 +123,7 @@ class FakeModuleBuilder(GenericBuilder): FakeModuleBuilder.on_finalize_cb = None FakeModuleBuilder.on_buildroot_add_artifacts_cb = None FakeModuleBuilder.on_tag_artifacts_cb = None + FakeModuleBuilder.on_buildroot_add_repos_cb = None FakeModuleBuilder.DEFAULT_GROUPS = None FakeModuleBuilder.backend = 'test' @@ -172,7 +174,8 @@ class FakeModuleBuilder(GenericBuilder): self._send_repo_done() def buildroot_add_repos(self, dependencies): - pass + if FakeModuleBuilder.on_buildroot_add_repos_cb: + FakeModuleBuilder.on_buildroot_add_repos_cb(self, dependencies) def tag_artifacts(self, artifacts, dest_tag=True): if FakeModuleBuilder.on_tag_artifacts_cb: @@ -1372,6 +1375,43 @@ class TestBuild: module = db.session.query(models.ModuleBuild).get(module_build_id) assert module.state == models.BUILD_STATES['build'] + @patch('module_build_service.auth.get_user', return_value=user) + @patch('module_build_service.scm.SCM') + def test_submit_br_metadata_only_module( + self, mocked_scm, mocked_get_user, conf_system, dbg, hmsc): + """ + Test that when a build is submitted with a buildrequire without a Koji tag, + MBS doesn't supply it as a dependency to the builder. + """ + metadata_mmd = module_build_service.utils.load_mmd( + path.join(base_dir, 'staged_data', 'build_metadata_module.yaml'), True) + module_build_service.utils.import_mmd(db.session, metadata_mmd) + + FakeSCM(mocked_scm, 'testmodule', 'testmodule_br_metadata_module.yaml', + '620ec77321b2ea7b0d67d82992dda3e1d67055b4') + post_url = '/module-build-service/1/module-builds/' + post_data = { + 'branch': 'master', + 'scmurl': 'https://src.stg.fedoraproject.org/modules/' + 'testmodule.git?#620ec77321b2ea7b0d67d82992dda3e1d67055b4', + } + rv = self.client.post(post_url, data=json.dumps(post_data)) + assert rv.status_code == 201 + + data = json.loads(rv.data) + module_build_id = data['id'] + + def on_buildroot_add_repos_cb(cls, dependencies): + # Make sure that the metadata module is not present since it doesn't have a Koji tag + assert dependencies.keys() == ['module-f28-build'] + + FakeModuleBuilder.on_buildroot_add_repos_cb = on_buildroot_add_repos_cb + stop = module_build_service.scheduler.make_simple_stop_condition(db.session) + module_build_service.scheduler.main([], stop) + + module = db.session.query(models.ModuleBuild).get(module_build_id) + assert module.state == models.BUILD_STATES['ready'] + @patch("module_build_service.config.Config.system", new_callable=PropertyMock, return_value="testlocal") diff --git a/tests/test_utils/test_utils.py b/tests/test_utils/test_utils.py index b3ddfdd3..9f5db09e 100644 --- a/tests/test_utils/test_utils.py +++ b/tests/test_utils/test_utils.py @@ -321,6 +321,32 @@ class TestUtils: assert mmd_context == models.DEFAULT_MODULE_CONTEXT assert build.context == models.DEFAULT_MODULE_CONTEXT + def test_import_mmd_multiple_dependencies(self): + mmd = Modulemd.Module().new_from_file( + path.join(BASE_DIR, '..', 'staged_data', 'formatted_testmodule.yaml')) + mmd.upgrade() + mmd.add_dependencies(mmd.get_dependencies()[0]) + + expected_error = 'The imported module\'s dependencies list should contain just one element' + with pytest.raises(UnprocessableEntity) as e: + module_build_service.utils.import_mmd(db.session, mmd) + assert str(e.value) == expected_error + + def test_import_mmd_no_xmd_buildrequires(self): + mmd = Modulemd.Module().new_from_file( + path.join(BASE_DIR, '..', 'staged_data', 'formatted_testmodule.yaml')) + mmd.upgrade() + xmd = glib.from_variant_dict(mmd.get_xmd()) + del xmd['mbs']['buildrequires'] + mmd.set_xmd(glib.dict_values(xmd)) + + expected_error = ( + 'The imported module buildrequires other modules, but the metadata in the ' + 'xmd["mbs"]["buildrequires"] dictionary is missing entries') + with pytest.raises(UnprocessableEntity) as e: + module_build_service.utils.import_mmd(db.session, mmd) + assert str(e.value) == expected_error + @pytest.mark.parametrize('stream, disttag_marking, error_msg', ( ('f28', None, None), ('f28', 'fedora28', None), diff --git a/tests/test_views/test_views.py b/tests/test_views/test_views.py index 7449d8ed..7993fdb8 100644 --- a/tests/test_views/test_views.py +++ b/tests/test_views/test_views.py @@ -1575,7 +1575,7 @@ class TestViews: rv = self.client.post( post_url, data=json.dumps({'scmurl': 'file://' + scm_base_dir + ( - '/mariadb?#7cf8fb26db8dbfea075eb5f898cc053139960250')})) + '/mariadb?#e9742ed681f82e3ef5281fc652b4e68a3826cea6')})) data = json.loads(rv.data) assert 'Module mariadb:10.2:20180724000000:00000000 imported' in data['messages'] @@ -1605,7 +1605,7 @@ class TestViews: rv = self.client.post( post_url, data=json.dumps({'scmurl': 'file://' + scm_base_dir + ( - '/mariadb?#1a43ea22cd32f235c2f119de1727a37902a49f20')})) + '/mariadb?#8b43f38cdafdd773e7d0308e758105bf9f0f67a8')})) data = json.loads(rv.data) assert 'Module mariadb:10.2:20180724065109:00000000 imported' in data['messages'] @@ -1651,29 +1651,13 @@ class TestViews: rv = self.client.post( post_url, data=json.dumps({'scmurl': 'file://' + scm_base_dir + ( - '/mariadb?#cb7cf7069059141e0797ad2cf5a559fb673ef43d')})) + '/mariadb?#f7c5c7218c9a197d7fd245eeb4eee3d7abffd75d')})) data = json.loads(rv.data) assert data['error'] == 'Unprocessable Entity' assert re.match(r'The modulemd .* is invalid\. Please verify the syntax is correct', data['message']) - @pytest.mark.parametrize('api_version', [1, 2]) - @patch('module_build_service.auth.get_user', return_value=import_module_user) - @patch.object(module_build_service.config.Config, 'scmurls', - new_callable=PropertyMock, return_value=['file://']) - def test_import_build_scm_missing_koji_tag(self, mocked_scmurls, mocked_get_user, - api_version): - post_url = '/module-build-service/{0}/import-module/'.format(api_version) - rv = self.client.post( - post_url, - data=json.dumps({'scmurl': 'file://' + scm_base_dir + ( - '/mariadb?#9ab5fdeba83eb3382413ee8bc06299344ef4477d')})) - data = json.loads(rv.data) - - assert data['error'] == 'Unprocessable Entity' - assert data['message'].startswith('\'koji_tag\' is not set in xmd[\'mbs\'] for module') - def test_buildrequires_is_included_in_json_output(self): # Inject xmd/mbs/buildrequires into an existing module build for # assertion later.