From 4e7b7ab94e4027b4a9b53bd5b7c1de8291678175 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: May 11 2021 19:21:26 +0000 Subject: PR#1700: Block dashes in stream/version/context by default Merges #1700 https://pagure.io/fm-orchestrator/pull-request/1700 --- diff --git a/module_build_service/common/config.py b/module_build_service/common/config.py index 35f22b0..91bbb33 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 c93338c..b0cf72e 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 diff --git a/module_build_service/web/submit.py b/module_build_service/web/submit.py index 86dd6e7..963454e 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( diff --git a/tests/staged_data/testmodule-dashed-stream.yaml b/tests/staged_data/testmodule-dashed-stream.yaml new file mode 100644 index 0000000..e5c750f --- /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 31a82ce..fbd9e6a 100644 --- a/tests/test_web/test_views.py +++ b/tests/test_web/test_views.py @@ -1348,10 +1348,50 @@ 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" + @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 = {