From a46a4ec4703068ae95cdd0e76dcc5c90c6a4f93c Mon Sep 17 00:00:00 2001 From: Merlin Mathesius Date: Mar 19 2019 11:56:47 +0000 Subject: [PATCH 1/3] Accept modulemd for scratch module builds as a parameter in the submitted JSON. Signed-off-by: Merlin Mathesius --- diff --git a/module_build_service/views.py b/module_build_service/views.py index 50221ac..2e3f481 100644 --- a/module_build_service/views.py +++ b/module_build_service/views.py @@ -31,6 +31,7 @@ import module_build_service.auth from flask import request, url_for from flask.views import MethodView from six import string_types +from io import BytesIO from module_build_service import app, conf, log, models, db, version, api_version as max_api_version from module_build_service.utils import ( @@ -172,10 +173,11 @@ class ModuleBuildAPI(AbstractQueryableBuildAPI): # Additional POST and DELETE handlers for modules follow. @validate_api_version() def post(self, api_version): - if hasattr(request, 'files') and "yaml" in request.files: - handler = YAMLFileHandler(request) + data = _dict_from_request(request) + if "modulemd" in data or (hasattr(request, "files") and "yaml" in request.files): + handler = YAMLFileHandler(request, data) else: - handler = SCMHandler(request) + handler = SCMHandler(request, data) if conf.no_auth is True and handler.username == "anonymous" and "owner" in handler.data: handler.username = handler.data["owner"] @@ -312,16 +314,9 @@ class ImportModuleAPI(MethodView): class BaseHandler(object): - def __init__(self, request): + def __init__(self, request, data=None): self.username, self.groups = module_build_service.auth.get_user(request) - if "multipart/form-data" in request.headers.get("Content-Type", ""): - self.data = request.form.to_dict() - else: - try: - self.data = json.loads(request.get_data().decode("utf-8")) - except Exception: - log.error('Invalid JSON submitted') - raise ValidationError('Invalid JSON submitted') + self.data = data or _dict_from_request(request) # canonicalize and validate scratch option if 'scratch' in self.data and str_to_bool(str(self.data['scratch'])): @@ -342,7 +337,8 @@ class BaseHandler(object): @property def optional_params(self): - return {k: v for k, v in self.data.items() if k not in ["owner", "scmurl", "branch"]} + return {k: v for k, v in self.data.items() if k not in + ["owner", "scmurl", "branch", "modulemd", "module_name"]} def _validate_dep_overrides_format(self, key): """ @@ -367,7 +363,13 @@ class BaseHandler(object): def validate_optional_params(self): module_build_columns = set([col.key for col in models.ModuleBuild.__table__.columns]) other_params = set([ - 'branch', 'rebuild_strategy', 'buildrequire_overrides', 'require_overrides']) + 'branch', + 'buildrequire_overrides', + 'modulemd', + 'module_name', + 'rebuild_strategy', + 'require_overrides', + ]) valid_params = other_params | module_build_columns forbidden_params = [k for k in self.data if k not in valid_params] @@ -427,23 +429,43 @@ class SCMHandler(BaseHandler): class YAMLFileHandler(BaseHandler): - def __init__(self, request): - super(YAMLFileHandler, self).__init__(request) + def __init__(self, request, data=None): + super(YAMLFileHandler, self).__init__(request, data) if not self.data['scratch'] and not conf.yaml_submit_allowed: raise Forbidden("YAML submission is not enabled") def validate(self): - if "yaml" not in request.files: + if ("modulemd" not in self.data and + (not hasattr(request, "files") or "yaml" not in request.files)): log.error('Invalid file submitted') raise ValidationError('Invalid file submitted') self.validate_optional_params() def post(self): - handle = request.files["yaml"] + if "modulemd" in self.data: + handle = BytesIO(self.data["modulemd"].encode("utf-8")) + if "module_name" in self.data and self.data["module_name"]: + handle.filename = self.data["module_name"] + else: + handle.filename = "unnamed" + else: + handle = request.files["yaml"] return submit_module_build_from_yaml(self.username, handle, optional_params=self.optional_params) +def _dict_from_request(request): + if "multipart/form-data" in request.headers.get("Content-Type", ""): + data = request.form.to_dict() + else: + try: + data = json.loads(request.get_data().decode("utf-8")) + except Exception: + log.error('Invalid JSON submitted') + raise ValidationError('Invalid JSON submitted') + return data + + def register_api(): """ Registers the MBS API. """ module_view = ModuleBuildAPI.as_view('module_builds') diff --git a/tests/test_views/test_views.py b/tests/test_views/test_views.py index 535c4bf..cc5260e 100644 --- a/tests/test_views/test_views.py +++ b/tests/test_views/test_views.py @@ -26,10 +26,9 @@ from datetime import datetime import module_build_service.scm from mock import patch, PropertyMock -from werkzeug.datastructures import FileStorage from shutil import copyfile from os import path, mkdir -from os.path import dirname +from os.path import basename, dirname, splitext from requests.utils import quote import hashlib import pytest @@ -1703,8 +1702,8 @@ class TestViews: @patch('module_build_service.scm.SCM') @patch('module_build_service.config.Config.modules_allow_scratch', new_callable=PropertyMock, return_value=True) - def test_submit_scratch_build(self, mocked_allow_scratch, mocked_scm, mocked_get_user, - api_version): + def test_submit_scratch_build( + self, mocked_allow_scratch, mocked_scm, mocked_get_user, api_version): FakeSCM(mocked_scm, 'testmodule', 'testmodule.yaml', '620ec77321b2ea7b0d67d82992dda3e1d67055b4') @@ -1760,9 +1759,8 @@ class TestViews: @patch('module_build_service.scm.SCM') @patch('module_build_service.config.Config.modules_allow_scratch', new_callable=PropertyMock, return_value=False) - def test_submit_scratch_build_not_allowed(self, mocked_allow_scratch, - mocked_scm, mocked_get_user, - api_version): + def test_submit_scratch_build_not_allowed( + self, mocked_allow_scratch, mocked_scm, mocked_get_user, api_version): FakeSCM(mocked_scm, 'testmodule', 'testmodule.yaml', '620ec77321b2ea7b0d67d82992dda3e1d67055b4') @@ -1789,21 +1787,21 @@ class TestViews: new_callable=PropertyMock, return_value=True) @patch('module_build_service.config.Config.yaml_submit_allowed', new_callable=PropertyMock, return_value=True) - def test_submit_scratch_build_with_mmd(self, mocked_allow_yaml, - mocked_allow_scratch, - mocked_get_user, - api_version): + def test_submit_scratch_build_with_mmd( + self, mocked_allow_yaml, mocked_allow_scratch, mocked_get_user, api_version): base_dir = path.abspath(path.dirname(__file__)) mmd_path = path.join(base_dir, '..', 'staged_data', 'testmodule.yaml') post_url = '/module-build-service/{0}/module-builds/'.format(api_version) with open(mmd_path, 'rb') as f: - yaml_file = FileStorage(f) - post_data = { - 'branch': 'master', - 'scratch': True, - 'yaml': yaml_file, - } - rv = self.client.post(post_url, content_type='multipart/form-data', data=post_data) + modulemd = f.read().decode('utf-8') + + post_data = { + 'branch': 'master', + 'scratch': True, + 'modulemd': modulemd, + 'module_name': str(splitext(basename(mmd_path))[0]), + } + rv = self.client.post(post_url, data=json.dumps(post_data)) data = json.loads(rv.data) if api_version >= 2: @@ -1857,13 +1855,22 @@ class TestViews: mmd_path = path.join(base_dir, '..', 'staged_data', 'testmodule.yaml') post_url = '/module-build-service/{0}/module-builds/'.format(api_version) with open(mmd_path, 'rb') as f: - yaml_file = FileStorage(f) - post_data = { - 'branch': 'master', - 'scratch': True, - 'yaml': yaml_file, - } - rv = self.client.post(post_url, content_type='multipart/form-data', data=post_data) + modulemd = f.read().decode('utf-8') + + post_data = { + 'branch': 'master', + 'scratch': True, + 'modulemd': modulemd, + 'module_name': str(splitext(basename(mmd_path))[0]), + } + rv = self.client.post(post_url, data=json.dumps(post_data)) + data = json.loads(rv.data) + + if api_version >= 2: + assert isinstance(data, list) + assert len(data) == 1 + data = data[0] + # this test is the same as the previous except YAML_SUBMIT_ALLOWED is False, # but it should still succeed since yaml is always allowed for scratch builds assert rv.status_code == 201 From aaf9eaa6ec7e7dd2d8d762da10ebd25014a9d187 Mon Sep 17 00:00:00 2001 From: Merlin Mathesius Date: Mar 19 2019 11:56:47 +0000 Subject: [PATCH 2/3] Use log.exception when reporting JSON parsing exceptions. Signed-off-by: Merlin Mathesius --- diff --git a/module_build_service/views.py b/module_build_service/views.py index 2e3f481..8b02215 100644 --- a/module_build_service/views.py +++ b/module_build_service/views.py @@ -200,7 +200,7 @@ class ModuleBuildAPI(AbstractQueryableBuildAPI): try: r = json.loads(request.get_data().decode("utf-8")) except Exception: - log.error('Invalid JSON submitted') + log.exception('Invalid JSON submitted') raise ValidationError('Invalid JSON submitted') if "owner" in r: @@ -461,7 +461,7 @@ def _dict_from_request(request): try: data = json.loads(request.get_data().decode("utf-8")) except Exception: - log.error('Invalid JSON submitted') + log.exception('Invalid JSON submitted') raise ValidationError('Invalid JSON submitted') return data From 7b7b07211e00cb29687c02834547c5159fef03e3 Mon Sep 17 00:00:00 2001 From: Merlin Mathesius Date: Mar 19 2019 11:56:47 +0000 Subject: [PATCH 3/3] Add/improve documentation for scratch module build requests, particularly for the new 'modulemd' and 'module_name' parameters. Signed-off-by: Merlin Mathesius --- diff --git a/README.rst b/README.rst index 2b53dd2..654f157 100644 --- a/README.rst +++ b/README.rst @@ -107,9 +107,15 @@ Options: existing requires on the modulemd. The expected format is ``{'platform': ['f28', 'f29']}``. - ``scratch`` - a boolean indicating if a scratch module build should be performed. Only allowed to be ``True`` if the MBS setting ``MODULES_ALLOW_SCRATCH`` is ``True``. -- ``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. +- ``yaml`` - a string of the input file when submitting a YAML modulemd file directly in a + ``multipart/form-data`` request. Only allowed if ``scratch`` is ``True`` or if the MBS + setting ``YAML_SUBMIT_ALLOWED`` is ``True``. The basename of the file will be used as + the module name. +- ``modulemd`` - a string for submitting a YAML modulemd file as a parameter in the JSON data as + an alternative to sending it in a ``multipart/form-data`` request. Only allowed if + ``scratch`` is ``True`` or if the MBS setting ``YAML_SUBMIT_ALLOWED`` is ``True``. +- ``module_name`` - a string to use as the module name if a scratch build is requested and the + YAML modulemd is submitted using the ``modulemd`` parameter. - ``srpms`` - an optional list of Koji upload URLs of SRPMs to include in a module scratch build. Only allowed if ``scratch`` is ``True``. - ``rebuild_strategy`` - a string of the desired rebuild strategy (affects what components get diff --git a/docs/HOW_MBS_BUILDS_MODULES.rst b/docs/HOW_MBS_BUILDS_MODULES.rst index a15383d..896499e 100644 --- a/docs/HOW_MBS_BUILDS_MODULES.rst +++ b/docs/HOW_MBS_BUILDS_MODULES.rst @@ -19,7 +19,8 @@ This JSON data is handled by ``views.SCMHandler``, which validates the JSON and ``submit_module_build(...)``. Alternatively, if submitting a YAML modulemd file is allowed (MBS setting -``YAML_SUBMIT_ALLOWED`` is ``True``), the user can send a ``multipart/form-data`` +``YAML_SUBMIT_ALLOWED`` is ``True``), the user can include it in the JSON data +(called ``modulemd`` and ``module_name``) or send a ``multipart/form-data`` POST request directly including the contents of a YAML modulemd file (called ``yaml``). In this case, the JSON data and YAML file are handled by ``views.YAMLFileHandler``, which validates the data and calls the @@ -27,7 +28,9 @@ POST request directly including the contents of a YAML modulemd file to ``submit_module_build(...)``. If module scratch builds are allowed (MBS setting ``MODULES_ALLOW_SCRATCH`` is -``True``), the user can also upload one or more source RPMs uploaded to Koji +``True``), the user can request a scratch module build (called ``scratch``). +With a scratch build request, the user can include a YAML modulemd file +(see above) and also upload one or more source RPMs to Koji via calls to Koji's ``session.uploadWrapper(..)``, and supply the list of upload links to MBS (called ``srpms``). Such custom SRPMs will be used to override the git repository source for corresponding components.