From 3f91b21a91ca5523211c53e7886ea869b0c910c5 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Mar 10 2021 20:09:31 +0000 Subject: Merge #1686 `Fix mmd.copy calls to support PackagerV3` --- diff --git a/module_build_service/common/submit.py b/module_build_service/common/submit.py index e02f00d..c93338c 100644 --- a/module_build_service/common/submit.py +++ b/module_build_service/common/submit.py @@ -10,6 +10,7 @@ import module_build_service.common.scm from module_build_service.common import conf, log from module_build_service.common.errors import ValidationError from module_build_service.common.utils import load_mmd_file +from module_build_service.common.modulemd import Modulemd def _is_eol_in_pdc(name, stream): @@ -46,7 +47,8 @@ def fetch_mmd(url, branch=None, allow_local_url=False, whitelist_url=False, mand if not whitelist_url and mandatory_checks: scm.verify() cofn = scm.get_module_yaml() - mmd = load_mmd_file(cofn) + # load_mmd_file can return either a ModuleStreamV2 or PackagerV3 object + stream_or_packager = load_mmd_file(cofn) finally: try: if td is not None: @@ -60,38 +62,51 @@ def fetch_mmd(url, branch=None, allow_local_url=False, whitelist_url=False, mand "Module {}:{} is marked as EOL in PDC.".format(scm.name, scm.branch)) if not mandatory_checks: - return mmd, scm + return stream_or_packager, scm # If the name was set in the modulemd, make sure it matches what the scmurl # says it should be - if mmd.get_module_name() and mmd.get_module_name() != scm.name: + if stream_or_packager.get_module_name() and stream_or_packager.get_module_name() != scm.name: if not conf.allow_name_override_from_scm: raise ValidationError( 'The name "{0}" that is stored in the modulemd is not valid' - .format(mmd.get_module_name()) + .format(stream_or_packager.get_module_name()) ) else: # Set the module name - mmd = mmd.copy(scm.name) + if isinstance(stream_or_packager, Modulemd.ModuleStream): + # this is a ModuleStreamV2 object + stream_or_packager = stream_or_packager.copy(scm.name) + else: + # this is a PackagerV3 object + stream_or_packager = stream_or_packager.copy() + stream_or_packager.set_module_name(scm.name) # If the stream was set in the modulemd, make sure it matches what the repo # branch is - if mmd.get_stream_name() and mmd.get_stream_name() != scm.branch: + if stream_or_packager.get_stream_name() and stream_or_packager.get_stream_name() != scm.branch: if not conf.allow_stream_override_from_scm: raise ValidationError( 'The stream "{0}" that is stored in the modulemd does not match the branch "{1}"' - .format(mmd.get_stream_name(), scm.branch) + .format(stream_or_packager.get_stream_name(), scm.branch) ) else: # Set the module stream - mmd = mmd.copy(mmd.get_module_name(), scm.branch) + if isinstance(stream_or_packager, Modulemd.ModuleStream): + # this is a ModuleStreamV2 object + stream_or_packager = stream_or_packager.copy(stream_or_packager.get_module_name(), + scm.branch) + else: + # this is a PackagerV3 object + stream_or_packager = stream_or_packager.copy() + stream_or_packager.set_stream_name(scm.branch) # If the version is in the modulemd, throw an exception since the version # since the version is generated by MBS - if mmd.get_mdversion() == 2 and mmd.get_version(): + if stream_or_packager.get_mdversion() == 2 and stream_or_packager.get_version(): raise ValidationError( 'The version "{0}" is already defined in the modulemd but it shouldn\'t be since the ' - "version is generated based on the commit time".format(mmd.get_version()) + "version is generated based on the commit time".format(stream_or_packager.get_version()) ) - return mmd, scm + return stream_or_packager, scm diff --git a/module_build_service/web/submit.py b/module_build_service/web/submit.py index a11161b..981b20c 100644 --- a/module_build_service/web/submit.py +++ b/module_build_service/web/submit.py @@ -111,28 +111,40 @@ def submit_module_build_from_yaml( db_session, username, handle, params, stream=None, skiptests=False ): yaml_file = to_text_type(handle.read()) - mmd = load_mmd(yaml_file) + # load_mmd can return either a ModuleStreamV2 or PackagerV3 object + # PackagerV3 objects become ModuleStreamV2 objects in submit_module_build + stream_or_packager = load_mmd(yaml_file) if hasattr(handle, "filename"): def_name = str(os.path.splitext(os.path.basename(handle.filename))[0]) - elif not mmd.get_module_name(): + elif not stream_or_packager.get_module_name(): raise ValidationError( "The module's name was not present in the modulemd file. Please use the " '"module_name" parameter' ) - module_name = mmd.get_module_name() or def_name - module_stream = stream or mmd.get_stream_name() or "master" - if module_name != mmd.get_module_name() or module_stream != mmd.get_stream_name(): + module_name = stream_or_packager.get_module_name() or def_name + module_stream = stream or stream_or_packager.get_stream_name() or "master" + if module_name != stream_or_packager.get_module_name() or \ + module_stream != stream_or_packager.get_stream_name(): # This is how you set the name and stream in the modulemd - mmd = mmd.copy(module_name, module_stream) - if skiptests: - buildopts = mmd.get_buildopts() or Modulemd.Buildopts() + if isinstance(stream_or_packager, Modulemd.ModuleStream): + # This is a ModuleStreamV2 object + stream_or_packager = stream_or_packager.copy(module_name, module_stream) + else: + # This is a PackagerV3 object + stream_or_packager = stream_or_packager.copy() + stream_or_packager.set_module_name(module_name) + stream_or_packager.set_stream_name(module_stream) + if skiptests and isinstance(stream_or_packager, Modulemd.ModuleStream): + # PackagerV3 objects do not have buildopts methods + buildopts = stream_or_packager.get_buildopts() or Modulemd.Buildopts() macros = buildopts.get_rpm_macros() or "" buildopts.set_rpm_macros(macros + "\n\n%__spec_check_pre exit 0\n") - mmd.set_buildopts(buildopts) + stream_or_packager.set_buildopts(buildopts) - module_stream_version = provide_module_stream_version_from_mmd(mmd) + module_stream_version = provide_module_stream_version_from_mmd(stream_or_packager) - return submit_module_build(db_session, username, mmd, params, module_stream_version) + return submit_module_build(db_session, username, stream_or_packager, params, + module_stream_version) _url_check_re = re.compile(r"^[^:/]+:.*$") @@ -146,11 +158,12 @@ def submit_module_build_from_scm(db_session, username, params, allow_local_url=F log.info("'{}' is not a valid URL, assuming local path".format(url)) url = os.path.abspath(url) url = "file://" + url - mmd, scm = fetch_mmd(url, branch, allow_local_url) + stream_or_packager, scm = fetch_mmd(url, branch, allow_local_url) module_stream_version = int(scm.version) - return submit_module_build(db_session, username, mmd, params, module_stream_version) + return submit_module_build(db_session, username, stream_or_packager, params, + module_stream_version) def _apply_dep_overrides(mmd, params): @@ -540,13 +553,14 @@ def _process_support_streams(db_session, mmd, params): _modify_buildtime_streams(db_session, mmd, new_streams_func) -def submit_module_build(db_session, username, mmd, params, module_stream_version): +def submit_module_build(db_session, username, stream_or_packager, params, module_stream_version): """ Submits new module build. :param db_session: SQLAlchemy session object. :param str username: Username of the build's owner. - :param Modulemd.ModuleStream mmd: Modulemd defining the build. + :type stream_or_packager: Modulemd.ModuleStream or Modulemd.PackagerV3 + Modulemd.ModuleStream or PackagerV3 object defining the build. :param dict params: the API parameters passed in by the user :rtype: list with ModuleBuild :return: List with submitted module builds. @@ -562,7 +576,8 @@ def submit_module_build(db_session, username, mmd, params, module_stream_version if "default_streams" in params: default_streams = params["default_streams"] - input_mmds, static_context = process_module_context_configuration(mmd) + # PackagerV3 objects become ModuleStreamV2 objects at this point + input_mmds, static_context = process_module_context_configuration(stream_or_packager) for mmd in input_mmds: mmd.set_version(module_stream_version) @@ -712,17 +727,17 @@ def submit_module_build(db_session, username, mmd, params, module_stream_version return modules -def process_module_context_configuration(mmd): +def process_module_context_configuration(stream_or_packager): """ Processes initial module metadata context configurations and creates individual module metadata for each context, if static context configuration is present. - :param Modulemd.ModuleStream packager_mmd: Packager (initial) modulemd which kickstarts - the build. + :type stream_or_packager: Modulemd.ModuleStream or Modulemd.PackagerV3 + Packager (initial) modulemd which kickstarts the build. :rtype: list with ModuleBuild :return: list of generated module metadata from context configurations. """ - mdversion = mmd.get_mdversion() + mdversion = stream_or_packager.get_mdversion() static_context = False # we check what version of the metadata format we are using. @@ -730,7 +745,7 @@ def process_module_context_configuration(mmd): # v3 we always talking about a new build and the static context # will be always True static_context = True - mdindex = mmd.convert_to_index() + mdindex = stream_or_packager.convert_to_index() streams = mdindex.search_streams() for stream in streams: @@ -753,19 +768,20 @@ def process_module_context_configuration(mmd): return streams, static_context else: - xmd = mmd.get_xmd() + xmd = stream_or_packager.get_xmd() # check if we are handling rebuild of a static context module if "mbs" in xmd: # check if it is a static context - if "static_context" in xmd["mbs"] or mmd.is_static_context(): + if "static_context" in xmd["mbs"] or stream_or_packager.is_static_context(): static_context = True - return [mmd], static_context + return [stream_or_packager], static_context # we check if static contexts are enabled by the `contexts` property defined by the user i # as an build option. static_context = "mbs_options" in xmd and "contexts" in xmd["mbs_options"] # if the static context configuration exists we expand it. If not we just return # the mmd unchanged, for futher processing. - streams = generate_mmds_from_static_contexts(mmd) if static_context else [mmd] + streams = generate_mmds_from_static_contexts(stream_or_packager) if static_context \ + else [stream_or_packager] return streams, static_context diff --git a/tests/staged_data/v3/mmd_packager.yaml b/tests/staged_data/v3/mmd_packager.yaml index 693e1e3..de4c46d 100644 --- a/tests/staged_data/v3/mmd_packager.yaml +++ b/tests/staged_data/v3/mmd_packager.yaml @@ -4,7 +4,7 @@ document: modulemd-packager version: 3 data: name: foo - stream: latest + stream: master summary: An example module description: >- A module for the demonstration of the metadata format. Also, diff --git a/tests/test_common/test_submit.py b/tests/test_common/test_submit.py index c4711e3..90f1ea2 100644 --- a/tests/test_common/test_submit.py +++ b/tests/test_common/test_submit.py @@ -4,7 +4,9 @@ from __future__ import absolute_import import mock -from module_build_service.common.submit import _is_eol_in_pdc +from module_build_service.common.modulemd import Modulemd +from module_build_service.common.submit import _is_eol_in_pdc, fetch_mmd +from tests.test_web.test_views import FakeSCM @mock.patch("module_build_service.common.submit.requests") @@ -32,3 +34,31 @@ def test_pdc_eol_check(requests): is_eol = _is_eol_in_pdc("mariadb", "10.1") assert is_eol + + +@mock.patch("module_build_service.common.scm.SCM") +def test_fetch_mmd(mocked_scm): + """ Test behavior for fetch_mmd """ + + FakeSCM( + mocked_scm, + "testmodule", + "testmodule.yaml", + "620ec77321b2ea7b0d67d82992dda3e1d67055b4") + + mmd, scm = fetch_mmd('testurl') + assert isinstance(mmd, Modulemd.ModuleStream) + + +@mock.patch("module_build_service.common.scm.SCM") +def test_fetch_mmd_packager_v3(mocked_scm): + """ Test PackagerV3 behavior for fetch_mmd """ + + FakeSCM( + mocked_scm, + "foo", + "v3/mmd_packager.yaml", + "620ec77321b2ea7b0d67d82992dda3e1d67055b4") + + mmd, scm = fetch_mmd('testurl') + assert not isinstance(mmd, Modulemd.ModuleStream) diff --git a/tests/test_web/test_submit.py b/tests/test_web/test_submit.py index 633b816..a2f548f 100644 --- a/tests/test_web/test_submit.py +++ b/tests/test_web/test_submit.py @@ -300,6 +300,36 @@ class TestUtilsComponentReuse: assert username_arg == username rmtree(module_dir) + @mock.patch("module_build_service.web.submit.submit_module_build") + def test_submit_module_build_from_yaml_packager_v3(self, mock_submit): + """ + Tests local module build from a yaml file with the skiptests option + + Args: + mock_submit (MagickMock): mocked function submit_module_build, which we then + inspect if it was called with correct arguments + """ + module_dir = tempfile.mkdtemp() + modulemd_yaml = read_staged_data("v3/mmd_packager") + modulemd_file_path = path.join(module_dir, "testmodule.yaml") + + username = "test" + stream = "dev" + + with io.open(modulemd_file_path, "w", encoding="utf-8") as fd: + fd.write(modulemd_yaml) + + with open(modulemd_file_path, "rb") as fd: + handle = FileStorage(fd) + submit_module_build_from_yaml( + db_session, username, handle, {}, stream=stream, skiptests=False) + mock_submit_args = mock_submit.call_args[0] + username_arg = mock_submit_args[1] + mmd_arg = mock_submit_args[2] + assert mmd_arg.get_stream_name() == stream + assert username_arg == username + rmtree(module_dir) + @mock.patch("module_build_service.web.submit.generate_expanded_mmds") def test_submit_build_new_mse_build(self, generate_expanded_mmds): """