From f7e7765686235d38b28851a39470d2bcd638e65f Mon Sep 17 00:00:00 2001 From: mprahl Date: Oct 29 2018 21:08:27 +0000 Subject: Add the ability to override buildrequires and requires when submitting a module build This will allow for tooling to automatically override certain buildrequires and requires based on the branch name the modulemd is built form. Addresses FACTORY-3414 --- diff --git a/README.rst b/README.rst index 757dcf2..f8e8cf5 100644 --- a/README.rst +++ b/README.rst @@ -97,6 +97,11 @@ The response, in case of a successful submission, would include the task ID. Options: +- ``buildrequire_overrides`` - the buildrequires to override the modulemd with. The overrides must + be to existing buildrequires on the modulemd. The expected format is + ``{'platform': ['f28', 'f29']}``. +- ``require_overrides`` - the requires to override the modulemd with. The overrides must be to + existing requires on the modulemd. The expected format is ``{'platform': ['f28', 'f29']}``. - ``yaml`` - a string of the input file when submitting a YAML file directly in a ``multipart/form-data`` request. The MBS setting ``YAML_SUBMIT_ALLOWED`` must be set to ``True`` for this to be allowed. diff --git a/module_build_service/utils/submit.py b/module_build_service/utils/submit.py index bd9974c..2a22449 100644 --- a/module_build_service/utils/submit.py +++ b/module_build_service/utils/submit.py @@ -407,6 +407,54 @@ def submit_module_build_from_scm(username, url, branch, allow_local_url=False, return submit_module_build(username, url, mmd, optional_params) +def _apply_dep_overrides(mmd, optional_params): + """ + Apply the dependency override parameters (if specified) on the input modulemd. + + :param Modulemd.Module mmd: the modulemd to apply the overrides on + :param dict optional_params: the optional API parameters passed in by the user + :raises ValidationError: if one of the overrides doesn't apply + """ + dep_overrides = { + 'buildrequires': optional_params.get('buildrequire_overrides', {}), + 'requires': optional_params.get('require_overrides', {}) + } + + unused_dep_overrides = { + 'buildrequires': set(dep_overrides['buildrequires'].keys()), + 'requires': set(dep_overrides['requires'].keys()) + } + + deps = mmd.get_dependencies() + for dep in deps: + for dep_type, overrides in dep_overrides.items(): + overridden = False + # Get the existing streams (e.g. dep.get_buildrequires()) + reqs = getattr(dep, 'get_' + dep_type)() + for name, streams in dep_overrides[dep_type].items(): + if name in reqs: + reqs[name].set(streams) + unused_dep_overrides[dep_type].remove(name) + overridden = True + if overridden: + # Set the overridden streams (e.g. dep.set_buildrequires(reqs)) + getattr(dep, 'set_' + dep_type)(reqs) + + for dep_type in unused_dep_overrides.keys(): + if unused_dep_overrides[dep_type]: + raise ValidationError( + 'The {} overrides for the following modules aren\'t applicable: {}' + .format(dep_type[:-1], ', '.join(sorted(unused_dep_overrides[dep_type])))) + + # Delete the parameters or else they'll get past to ModuleBuild.create + if 'buildrequire_overrides' in optional_params: + del optional_params['buildrequire_overrides'] + if 'require_overrides' in optional_params: + del optional_params['require_overrides'] + + mmd.set_dependencies(deps) + + def submit_module_build(username, url, mmd, optional_params=None): """ Submits new module build. @@ -425,20 +473,19 @@ def submit_module_build(username, url, mmd, optional_params=None): """ import koji # Placed here to avoid py2/py3 conflicts... - # For local builds, we want the user to choose the exact stream using the default_streams - # in case there are multiple streams to choose from and raise an exception otherwise. - if optional_params and "local_build" in optional_params: - raise_if_stream_ambigous = True - del optional_params["local_build"] - else: - raise_if_stream_ambigous = False - - # Get the default_streams if set. - if optional_params and "default_streams" in optional_params: - default_streams = optional_params["default_streams"] - del optional_params["default_streams"] - else: - default_streams = {} + raise_if_stream_ambigous = False + default_streams = {} + if optional_params: + # For local builds, we want the user to choose the exact stream using the default_streams + # in case there are multiple streams to choose from and raise an exception otherwise. + if "local_build" in optional_params: + raise_if_stream_ambigous = True + del optional_params["local_build"] + # Get the default_streams if set. + if "default_streams" in optional_params: + default_streams = optional_params["default_streams"] + del optional_params["default_streams"] + _apply_dep_overrides(mmd, optional_params) validate_mmd(mmd) mmds = generate_expanded_mmds(db.session, mmd, raise_if_stream_ambigous, default_streams) diff --git a/module_build_service/views.py b/module_build_service/views.py index ca2a8c8..44550c7 100644 --- a/module_build_service/views.py +++ b/module_build_service/views.py @@ -30,7 +30,7 @@ import json import module_build_service.auth from flask import request, url_for from flask.views import MethodView -from six import text_type +from six import text_type, string_types from module_build_service import app, conf, log, models, db, version, api_version as max_api_version from module_build_service.utils import ( @@ -316,10 +316,33 @@ class BaseHandler(object): def optional_params(self): return {k: v for k, v in self.data.items() if k not in ["owner", "scmurl", "branch"]} + def _validate_dep_overrides_format(self, key): + """ + Validate any dependency overrides provided to the API. + + :param str key: the override key to validate + :raises ValidationError: when the overrides are an invalid format + """ + if not self.data.get(key): + return + invalid_override_msg = ('The "{}" parameter must be an object with the keys as module ' + 'names and the values as arrays of streams'.format(key)) + if not isinstance(self.data[key], dict): + raise ValidationError(invalid_override_msg) + for streams in self.data[key].values(): + if not isinstance(streams, list): + raise ValidationError(invalid_override_msg) + for stream in streams: + if not isinstance(stream, string_types): + raise ValidationError(invalid_override_msg) + def validate_optional_params(self): - forbidden_params = [k for k in self.data - if k not in models.ModuleBuild.__table__.columns and - k not in ["branch", "rebuild_strategy"]] + module_build_columns = set([col.key for col in models.ModuleBuild.__table__.columns]) + other_params = set([ + 'branch', 'rebuild_strategy', 'buildrequire_overrides', 'require_overrides']) + valid_params = other_params | module_build_columns + + forbidden_params = [k for k in self.data if k not in valid_params] if forbidden_params: raise ValidationError('The request contains unspecified parameters: {}' .format(", ".join(forbidden_params))) @@ -339,6 +362,9 @@ class BaseHandler(object): .format(self.data['rebuild_strategy'], ', '.join(conf.rebuild_strategies_allowed))) + self._validate_dep_overrides_format('buildrequire_overrides') + self._validate_dep_overrides_format('require_overrides') + class SCMHandler(BaseHandler): def __init__(self, request): diff --git a/tests/staged_data/testmodule_platform_f290000.yaml b/tests/staged_data/testmodule_platform_f290000.yaml new file mode 100644 index 0000000..84abc87 --- /dev/null +++ b/tests/staged_data/testmodule_platform_f290000.yaml @@ -0,0 +1,38 @@ +document: modulemd +version: 2 +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: ['f29.0.0'] + requires: + platform: ['f29.0.0'] + 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. + ref: master + buildorder: 10 diff --git a/tests/test_views/test_views.py b/tests/test_views/test_views.py index 9a1a721..1394a7b 100644 --- a/tests/test_views/test_views.py +++ b/tests/test_views/test_views.py @@ -1205,6 +1205,111 @@ class TestViews: res2 = submit('git://some.custom.url.org/modules/testmodule.git?#68931c9') assert res2.status_code == 201 + @pytest.mark.parametrize('br_override_streams, req_override_streams', ( + (['f28'], None), + (['f28'], ['f28']), + )) + @patch('module_build_service.auth.get_user', return_value=user) + @patch('module_build_service.scm.SCM') + def test_submit_build_dep_override( + self, mocked_scm, mocked_get_user, br_override_streams, req_override_streams): + init_data(data_size=1, multiple_stream_versions=True) + FakeSCM(mocked_scm, 'testmodule', 'testmodule_platform_f290000.yaml', + '620ec77321b2ea7b0d67d82992dda3e1d67055b4') + + post_url = '/module-build-service/2/module-builds/' + scm_url = ('git://pkgs.stg.fedoraproject.org/modules/testmodule.git?#68931c90de214d9d13feef' + 'bd35246a81b6cb8d49') + json_input = { + 'branch': 'master', + 'scmurl': scm_url + } + + if br_override_streams: + json_input['buildrequire_overrides'] = {'platform': br_override_streams} + expected_br = set(br_override_streams) + else: + expected_br = set(['f29.0.0']) + + if req_override_streams: + json_input['require_overrides'] = {'platform': req_override_streams} + expected_req = set(req_override_streams) + else: + expected_req = set(['f29.0.0']) + + rv = self.client.post(post_url, data=json.dumps(json_input)) + data = json.loads(rv.data) + + mmd = Modulemd.Module().new_from_string(data[0]['modulemd']) + assert len(mmd.get_dependencies()) == 1 + dep = mmd.get_dependencies()[0] + assert set(dep.get_buildrequires()['platform'].get()) == expected_br + assert set(dep.get_requires()['platform'].get()) == expected_req + + @pytest.mark.parametrize('dep_type', ('buildrequire', 'require')) + @patch('module_build_service.auth.get_user', return_value=user) + @patch('module_build_service.scm.SCM') + def test_submit_build_override_unused(self, mocked_scm, mocked_get_user, dep_type): + FakeSCM(mocked_scm, 'testmodule', 'testmodule_platform_f290000.yaml', + '620ec77321b2ea7b0d67d82992dda3e1d67055b4') + + post_url = '/module-build-service/2/module-builds/' + scm_url = ('git://pkgs.stg.fedoraproject.org/modules/testmodule.git?#68931c90de214d9d13feef' + 'bd35246a81b6cb8d49') + json_input = { + 'branch': 'master', + 'scmurl': scm_url, + } + json_input[dep_type + '_overrides'] = {'nonexistent': ['23'], 'nonexistent2': ['2']} + + rv = self.client.post(post_url, data=json.dumps(json_input)) + data = json.loads(rv.data) + + assert data == { + 'error': 'Bad Request', + 'message': ('The {} overrides for the following modules aren\'t applicable: ' + 'nonexistent, nonexistent2').format(dep_type), + 'status': 400 + } + + @pytest.mark.parametrize('optional_params', ( + {'buildrequire_overrides': {'platform': 'f28'}}, + {'buildrequire_overrides': {'platform': 28}}, + {'buildrequire_overrides': 'platform:f28'}, + {'require_overrides': {'platform': 'f28'}}, + {'require_overrides': {'platform': 28}}, + {'require_overrides': 'platform:f28'} + )) + @patch('module_build_service.auth.get_user', return_value=user) + @patch('module_build_service.scm.SCM') + def test_submit_build_invalid_override(self, mocked_scm, mocked_get_user, optional_params): + FakeSCM(mocked_scm, 'testmodule', 'testmodule_platform_f290000.yaml', + '620ec77321b2ea7b0d67d82992dda3e1d67055b4') + + post_url = '/module-build-service/2/module-builds/' + scm_url = ('git://pkgs.stg.fedoraproject.org/modules/testmodule.git?#68931c90de214d9d13feef' + 'bd35246a81b6cb8d49') + json_input = { + 'branch': 'master', + 'scmurl': scm_url, + } + json_input.update(optional_params) + + rv = self.client.post(post_url, data=json.dumps(json_input)) + data = json.loads(rv.data) + + msg = ('The "{}" parameter must be an object with the keys as module names and the values ' + 'as arrays of streams') + if 'buildrequire_overrides' in optional_params: + msg = msg.format('buildrequire_overrides') + elif 'require_overrides' in optional_params: + msg = msg.format('require_overrides') + assert data == { + 'error': 'Bad Request', + 'message': msg, + 'status': 400 + } + def test_about(self): with patch.object(mbs_config.Config, 'auth_method', new_callable=PropertyMock) as auth: auth.return_value = 'kerberos'