#1061 Add the ability to override buildrequires and requires when submitting a module build
Merged 6 months ago by mprahl. Opened 6 months ago by mprahl.

file modified
+5

@@ -97,6 +97,11 @@ 

  

  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.

@@ -407,6 +407,54 @@ 

      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 @@ 

      """

      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)

file modified
+30 -4

@@ -30,7 +30,7 @@ 

  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 @@ 

      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 @@ 

                      .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):

@@ -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

@@ -1205,6 +1205,111 @@ 

          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'

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

Hm, I'm not sure if it's generic enough. I would suggest approach based on "search and replace" - in API, you would define what you want to replace with what.

This would have following benefits and fix following problems:

  • In case there are two buildrequires/requires pairs - one buildrequiring platform:f28 and one buildrequiring platform:f29 with different dependencies, you probably don't want to replace platform:f29 with platform:f28.0.0.
  • It would allow the same API to be used to even remove dependency, you could just define "search for platform:f29 and replace it with ~nothing~".
  • Using similar way, you can add new dependencies - "search for ~nothing~ and replace it with platform:f29".

Although I know that the remove/add is not the main use-case right now, it is something modularity-wg would like to use in the future based on @psabata in FACTORY-3084:

it would also be nice if this mechanism allowed, besides overriding existing or adding new modular deps, for dep removal without replacement.

I think it would be nice if REST API in theory supported that. It does not have to be implemented right now in this PR, but it would be great to count with that in API design.

Pull-Request has been merged by mprahl

6 months ago