From da51bebcd9ed1d62797a9aa2a500885f5d16f2ec Mon Sep 17 00:00:00 2001 From: Merlin Mathesius Date: Mon, 9 Mar 2020 15:01:58 -0500 Subject: [PATCH] New 'module_stream' optional parameter - Implement new optional parameter module_stream to allow a scratch module build's stream name to be set from the command line when also submitting a YAML modulemd file. - Validate that module_name and module_stream parameters can only be specified along with a YAML modulemd file. - Add tests to verify that module_stream sets the stream name correctly. - Add tests to verify that module_name and module_stream are only allowed along with a YAML modulemd file. Signed-off-by: Merlin Mathesius --- README.rst | 2 + module_build_service/web/views.py | 19 +++++- tests/test_web/test_views.py | 102 +++++++++++++++++++++++++++++- 3 files changed, 118 insertions(+), 5 deletions(-) diff --git a/README.rst b/README.rst index 6e3b5983..ccc0ac85 100644 --- a/README.rst +++ b/README.rst @@ -124,6 +124,8 @@ Options: ``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. + - ``module_stream`` - a string to use as the stream name if a scratch build is requested and the + YAML modulemd is submitted using the ``modulemd`` parameter. - ``rebuild_strategy`` - a string of the desired rebuild strategy (affects what components get rebuilt). For the available options, please look at the "Rebuild Strategies" section below. - ``require_overrides`` - the requires to override the modulemd with. The overrides must be to diff --git a/module_build_service/web/views.py b/module_build_service/web/views.py index 0488a1da..d5ab4d42 100644 --- a/module_build_service/web/views.py +++ b/module_build_service/web/views.py @@ -346,6 +346,7 @@ class BaseHandler(object): "buildrequire_overrides", "modulemd", "module_name", + "module_stream", "owner", "rebuild_strategy", "reuse_components_from", @@ -481,6 +482,17 @@ class SCMHandler(BaseHandler): log.error("Missing branch") raise ValidationError("Missing branch") + if "module_name" in self.data: + log.error("Module name override is only allowed when a YAML file is submitted") + raise ValidationError( + "Module name override is only allowed when a YAML file is submitted" + ) + if "module_stream" in self.data: + log.error("Stream name override is only allowed when a YAML file is submitted") + raise ValidationError( + "Stream name override is only allowed when a YAML file is submitted" + ) + if not skip_optional_params: self.validate_optional_params() @@ -507,12 +519,13 @@ class YAMLFileHandler(BaseHandler): def post(self): if "modulemd" in self.data: handle = BytesIO(self.data["modulemd"].encode("utf-8")) - if self.data.get("module_name"): - handle.filename = self.data["module_name"] else: handle = request.files["yaml"] + if self.data.get("module_name"): + handle.filename = self.data["module_name"] + stream_name = self.data.get("module_stream", None) return submit_module_build_from_yaml( - db.session, self.username, handle, self.data) + db.session, self.username, handle, self.data, stream=stream_name) def _dict_from_request(request): diff --git a/tests/test_web/test_views.py b/tests/test_web/test_views.py index f3b3d887..5d963b50 100644 --- a/tests/test_web/test_views.py +++ b/tests/test_web/test_views.py @@ -2075,6 +2075,60 @@ class TestViews: assert br_modulea == buildrequires.get("modulea") assert br_moduleb == buildrequires.get("moduleb") + @pytest.mark.parametrize("api_version", [1, 2]) + @patch("module_build_service.web.auth.get_user", return_value=user) + @patch("module_build_service.common.scm.SCM") + def test_submit_build_module_name_override_not_allowed( + self, mocked_scm, mocked_get_user, api_version + ): + FakeSCM( + mocked_scm, "testmodule", "testmodule.yaml", "620ec77321b2ea7b0d67d82992dda3e1d67055b4") + + post_url = "/module-build-service/{0}/module-builds/".format(api_version) + rv = self.client.post( + post_url, + data=json.dumps({ + "branch": "master", + "scmurl": "https://src.stg.fedoraproject.org/modules/" + "testmodule.git?#68931c90de214d9d13feefbd35246a81b6cb8d49", + "module_name": "altname", + }), + ) + # module name is allowed only when a modulemd file is submitted + assert rv.status_code == 400 + result = json.loads(rv.data) + assert result["error"] == "Bad Request" + assert result["message"] == ( + "Module name override is only allowed when a YAML file is submitted" + ) + + @pytest.mark.parametrize("api_version", [1, 2]) + @patch("module_build_service.web.auth.get_user", return_value=user) + @patch("module_build_service.common.scm.SCM") + def test_submit_build_stream_name_override_not_allowed( + self, mocked_scm, mocked_get_user, api_version + ): + FakeSCM( + mocked_scm, "testmodule", "testmodule.yaml", "620ec77321b2ea7b0d67d82992dda3e1d67055b4") + + post_url = "/module-build-service/{0}/module-builds/".format(api_version) + rv = self.client.post( + post_url, + data=json.dumps({ + "branch": "master", + "scmurl": "https://src.stg.fedoraproject.org/modules/" + "testmodule.git?#68931c90de214d9d13feefbd35246a81b6cb8d49", + "module_stream": "altstream", + }), + ) + # stream name override is allowed only when a modulemd file is submitted + assert rv.status_code == 400 + result = json.loads(rv.data) + assert result["error"] == "Bad Request" + assert result["message"] == ( + "Stream name override is only allowed when a YAML file is submitted" + ) + @pytest.mark.parametrize("api_version", [1, 2]) @patch("module_build_service.web.auth.get_user", return_value=user) @patch("module_build_service.common.scm.SCM") @@ -2169,6 +2223,45 @@ class TestViews: assert rv.status_code == expected_error["status"] @pytest.mark.parametrize("api_version", [1, 2]) + @pytest.mark.parametrize("mod_stream", [None, "alternate"]) + @patch("module_build_service.web.auth.get_user", return_value=user) + @patch( + "module_build_service.common.config.Config.yaml_submit_allowed", + new_callable=PropertyMock, + return_value=True, + ) + def test_submit_build_with_mmd( + self, mocked_allow_yaml, mocked_get_user, mod_stream, api_version + ): + modulemd = read_staged_data("testmodule") + + post_data = { + "branch": "master", + "modulemd": modulemd, + "module_name": str(splitext(basename(staged_data_filename("testmodule")))[0]), + } + if mod_stream: + post_data["module_stream"] = mod_stream + expected_stream = mod_stream + else: + expected_stream = "master" + post_url = "/module-build-service/{0}/module-builds/".format(api_version) + 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] + + assert data["name"] == "testmodule" + assert data["scratch"] is False + assert data["stream"] == expected_stream + # Assertions for other "testmodule" attributes are done in + # test_submit_scratch_build_with_mmd() + + @pytest.mark.parametrize("api_version", [1, 2]) + @pytest.mark.parametrize("mod_stream", [None, "alternate"]) @patch("module_build_service.web.auth.get_user", return_value=user) @patch( "module_build_service.common.config.Config.modules_allow_scratch", @@ -2181,7 +2274,7 @@ class TestViews: return_value=True, ) def test_submit_scratch_build_with_mmd( - self, mocked_allow_yaml, mocked_allow_scratch, mocked_get_user, api_version + self, mocked_allow_yaml, mocked_allow_scratch, mocked_get_user, mod_stream, api_version ): modulemd = read_staged_data("testmodule") @@ -2191,6 +2284,11 @@ class TestViews: "modulemd": modulemd, "module_name": str(splitext(basename(staged_data_filename("testmodule")))[0]), } + if mod_stream: + post_data["module_stream"] = mod_stream + expected_stream = mod_stream + else: + expected_stream = "master" post_url = "/module-build-service/{0}/module-builds/".format(api_version) rv = self.client.post(post_url, data=json.dumps(post_data)) data = json.loads(rv.data) @@ -2212,7 +2310,7 @@ class TestViews: assert data["time_submitted"] is not None assert data["time_modified"] is not None assert data["time_completed"] is None - assert data["stream"] == "master" + assert data["stream"] == expected_stream assert data["owner"] == "Homer J. Simpson" assert data["id"] == 8 assert data["rebuild_strategy"] == "changed-and-after"