From 1f37c646f766b71f8bfd9305b46262416c889d88 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Wed, 5 May 2021 20:32:48 -0400 Subject: [PATCH 1/4] add allow_dashes_in_svc config option --- module_build_service/common/config.py | 5 +++++ module_build_service/common/submit.py | 13 ++++++++++--- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/module_build_service/common/config.py b/module_build_service/common/config.py index 35f22b07..91bbb33e 100644 --- a/module_build_service/common/config.py +++ b/module_build_service/common/config.py @@ -754,6 +754,11 @@ class Config(object): ], "desc": "The list Python paths for the Celery application to import.", }, + "allow_dashes_in_svc": { + "type": bool, + "default": False, + "desc": "Whether to allow dashes in stream, version, and context values.", + }, } def __init__(self, conf_section_obj): diff --git a/module_build_service/common/submit.py b/module_build_service/common/submit.py index c93338c9..b0cf72ed 100644 --- a/module_build_service/common/submit.py +++ b/module_build_service/common/submit.py @@ -84,11 +84,18 @@ def fetch_mmd(url, branch=None, allow_local_url=False, whitelist_url=False, mand # If the stream was set in the modulemd, make sure it matches what the repo # branch is - if stream_or_packager.get_stream_name() and stream_or_packager.get_stream_name() != scm.branch: + stream_override = stream_or_packager.get_stream_name() + scm_stream = scm.branch + if not conf.allow_dashes_in_svc: + scm_stream = scm_stream.replace('-', '_') + if stream_override and '-' in stream_override: + raise ValidationError('Dashes are not allowed in the stream value') + if stream_override and stream_override != scm_stream: 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(stream_or_packager.get_stream_name(), scm.branch) + 'The stream "{0}" that is stored in the modulemd does not match the stream ' + 'determined by the branch "{1}"' + .format(stream_override, scm_stream) ) else: # Set the module stream From 76e30dd8ad493510fb71bcfe2fe364a43158ca3e Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Wed, 5 May 2021 22:35:15 -0400 Subject: [PATCH 2/4] enforce allow_dashes_in_svc at module creation --- module_build_service/web/submit.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/module_build_service/web/submit.py b/module_build_service/web/submit.py index 86dd6e76..963454e9 100644 --- a/module_build_service/web/submit.py +++ b/module_build_service/web/submit.py @@ -710,6 +710,15 @@ def submit_module_build(db_session, username, stream_or_packager, params, module module.context = mmd.get_context() module.context += context_suffix + + if not conf.allow_dashes_in_svc: + if '-' in module.stream: + raise ValidationError('Dashes not allowed in stream') + if '-' in module.version: + raise ValidationError('Dashes not allowed in version') + if '-' in module.context: + raise ValidationError('Dashes not allowed in context') + db_session.commit() notify_on_module_state_change( From 53000c0783d93801bf5a81a50d56acf602a137f7 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Thu, 6 May 2021 11:53:55 -0400 Subject: [PATCH 3/4] fix unit test --- tests/test_web/test_views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_web/test_views.py b/tests/test_web/test_views.py index 31a82ce1..8a3cdb44 100644 --- a/tests/test_web/test_views.py +++ b/tests/test_web/test_views.py @@ -1348,7 +1348,7 @@ class TestSubmitBuild: assert data["status"] == 400 assert data["message"] == ( 'The stream "wrong_stream" that is stored in the modulemd ' - 'does not match the branch "master"' + 'does not match the stream determined by the branch "master"' ) assert data["error"] == "Bad Request" From d6185e806eb31fe6b29443920639f691de6285cb Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Thu, 6 May 2021 13:43:30 -0400 Subject: [PATCH 4/4] new unit test --- .../staged_data/testmodule-dashed-stream.yaml | 38 ++++++++++++++++++ tests/test_web/test_views.py | 40 +++++++++++++++++++ 2 files changed, 78 insertions(+) create mode 100644 tests/staged_data/testmodule-dashed-stream.yaml diff --git a/tests/staged_data/testmodule-dashed-stream.yaml b/tests/staged_data/testmodule-dashed-stream.yaml new file mode 100644 index 00000000..e5c750fd --- /dev/null +++ b/tests/staged_data/testmodule-dashed-stream.yaml @@ -0,0 +1,38 @@ +document: modulemd +version: 1 +data: + summary: A test module in all its beautiful beauty + stream: some-stream + 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: f28 + requires: + platform: f28 + 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. + buildorder: 10 + ref: master diff --git a/tests/test_web/test_views.py b/tests/test_web/test_views.py index 8a3cdb44..fbd9e6ac 100644 --- a/tests/test_web/test_views.py +++ b/tests/test_web/test_views.py @@ -1352,6 +1352,46 @@ class TestSubmitBuild: ) assert data["error"] == "Bad Request" + @pytest.mark.parametrize("dashes_allowed", (True, False)) + @pytest.mark.parametrize("yaml_file", ("testmodule.yaml", "testmodule-dashed-stream.yaml")) + @patch("module_build_service.web.auth.get_user", return_value=user) + @patch("module_build_service.common.scm.SCM") + def test_submit_build_dashes_in_stream(self, mocked_scm, mocked_get_user, yaml_file, + dashes_allowed): + FakeSCM( + mocked_scm, + "testmodule", + yaml_file, + "620ec77321b2ea7b0d67d82992dda3e1d67055b4", + ) + + with patch.object(mbs_config.Config, "allow_dashes_in_svc", + new_callable=PropertyMock) as da, \ + patch.object(mbs_config.Config, "allow_stream_override_from_scm", + new_callable=PropertyMock) as asofs: + da.return_value = dashes_allowed + asofs.return_value = True + rv = self.client.post( + "/module-build-service/1/module-builds/", + data=json.dumps({ + "branch": "master", + "scmurl": "https://src.stg.fedoraproject.org/modules/" + "testmodule.git?#68931c90de214d9d13feefbd35246a81b6cb8d49", + }), + ) + data = json.loads(rv.data) + + if not dashes_allowed and 'dash' in yaml_file: + assert data["status"] == 400 + assert data["message"] == "Dashes are not allowed in the stream value" + assert data["error"] == "Bad Request" + else: + assert data["name"] == "testmodule" + if 'dash' in yaml_file: + assert data["stream"] == "some-stream" + else: + assert data["stream"] == "master" + @patch("module_build_service.web.auth.get_user", return_value=user) def test_submit_build_set_owner(self, mocked_get_user): data = {