PR#1700: Block dashes in stream/version/context by default

Merges #1700
https://pagure.io/fm-orchestrator/pull-request/1700

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

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

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(

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

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 = {