From 234e1b376727bffaad1ea8dc2a06e607bdebeb5c Mon Sep 17 00:00:00 2001 From: Sam Date: Thu, 5 Nov 2020 18:40:36 +0100 Subject: [PATCH] fix(service): correctly set project_name slug on project create (#1691) --- conftest.py | 4 +- renku/core/models/git.py | 7 ++- renku/core/utils/scm.py | 14 ++++- renku/service/serializers/cache.py | 4 ++ renku/service/serializers/templates.py | 14 +++-- .../test_templates_create_project.py | 61 ++++++++++++++++++- tests/service/views/test_templates_views.py | 4 +- 7 files changed, 94 insertions(+), 14 deletions(-) diff --git a/conftest.py b/conftest.py index f66e7293a7..36e76316ec 100644 --- a/conftest.py +++ b/conftest.py @@ -1036,7 +1036,7 @@ def svc_client_with_templates(svc_client, mock_redis, authentication_headers, te def svc_client_templates_creation(svc_client_with_templates): """Setup and teardown steps for templates tests.""" from renku.core.utils.requests import retry - from renku.core.utils.scm import strip_and_lower + from renku.core.utils.scm import normalize_to_ascii svc_client, authentication_headers, template = svc_client_with_templates parameters = [] @@ -1055,7 +1055,7 @@ def svc_client_templates_creation(svc_client_with_templates): # clenup by invoking the GitLab delete API # TODO: consider using the project delete endpoint once implemented def remove_project(): - project_slug = "{0}/{1}".format(payload["project_namespace"], strip_and_lower(payload["project_name"])) + project_slug = "{0}/{1}".format(payload["project_namespace"], normalize_to_ascii(payload["project_name"])) project_slug_encoded = urllib.parse.quote(project_slug, safe="") project_delete_url = "{0}/api/v4/projects/{1}".format(payload["project_repository"], project_slug_encoded) diff --git a/renku/core/models/git.py b/renku/core/models/git.py index fcb5814dae..abf5b7ce03 100644 --- a/renku/core/models/git.py +++ b/renku/core/models/git.py @@ -105,13 +105,16 @@ def __attrs_post_init__(self): @classmethod def parse(cls, href): - """Derive basic informations.""" + """Derive URI components.""" + if not href.isascii(): + raise UnicodeError(f"`{href}` is not a valid Git remote") + for regex in _REPOSITORY_URLS: matches = re.search(regex, href) if matches: return cls(href=href, regex=regex, **matches.groupdict()) else: - raise errors.ConfigurationError("`{href}` is not a valid Git remote.".format(href=href)) + raise errors.ConfigurationError(f"`{href}` is not a valid Git remote") @property def image(self): diff --git a/renku/core/utils/scm.py b/renku/core/utils/scm.py index 3b6558870c..e8c14e2c3d 100644 --- a/renku/core/utils/scm.py +++ b/renku/core/utils/scm.py @@ -19,9 +19,19 @@ import re -def strip_and_lower(input): +def normalize_to_ascii(input_string, sep="-"): """Adjust chars to make the input compatible as scm source.""" - return re.sub(r"\s", r"-", input.strip()).lower() + return ( + sep.join( + [ + component + for component in re.sub(r"[^a-zA-Z0-9_.-]+", " ", input_string).split(" ") + if component and component.isascii() + ] + ) + .lower() + .strip(sep) + ) def git_unicode_unescape(s, encoding="utf-8"): diff --git a/renku/service/serializers/cache.py b/renku/service/serializers/cache.py index 013fabeed7..563c05ff90 100644 --- a/renku/service/serializers/cache.py +++ b/renku/service/serializers/cache.py @@ -124,6 +124,8 @@ def validate_git_url(self, value): """Validates git url.""" try: GitURL.parse(value) + except UnicodeError as e: + raise ValidationError("`git_url` contains unsupported characters") from e except ConfigurationError as e: raise ValidationError("Invalid `git_url`") from e @@ -144,6 +146,8 @@ def set_owner_name(self, data, **kwargs): """Set owner and name fields.""" try: git_url = GitURL.parse(data["git_url"]) + except UnicodeError as e: + raise ValidationError("`git_url` contains unsupported characters") from e except ConfigurationError as e: raise ValidationError("Invalid `git_url`") from e diff --git a/renku/service/serializers/templates.py b/renku/service/serializers/templates.py index e10ae4058c..53397d2f04 100644 --- a/renku/service/serializers/templates.py +++ b/renku/service/serializers/templates.py @@ -23,7 +23,7 @@ from renku.core.errors import ConfigurationError from renku.core.models.git import GitURL -from renku.core.utils.scm import strip_and_lower +from renku.core.utils.scm import normalize_to_ascii from renku.service.config import TEMPLATE_CLONE_DEPTH_DEFAULT from renku.service.serializers.cache import ProjectCloneContext from renku.service.serializers.rpc import JsonRPCResponse @@ -40,7 +40,6 @@ class ManifestTemplatesRequest(ProjectCloneContext): def set_git_url(self, data, **kwargs): """Set git_url field.""" data["git_url"] = data["url"] - return data @@ -68,8 +67,15 @@ class ProjectTemplateRequest(ManifestTemplatesRequest): @pre_load() def create_new_project_url(self, data, **kwargs): """Set owner and name fields.""" - project_name_stripped = strip_and_lower(data["project_name"]) - new_project_url = f"{data['project_repository']}/{data['project_namespace']}/{project_name_stripped}" + try: + project_name_stripped = normalize_to_ascii(data["project_name"]) + new_project_url = f"{data['project_repository']}/{data['project_namespace']}/{project_name_stripped}" + _ = GitURL.parse(new_project_url) + except UnicodeError as e: + raise ValidationError("`git_url` contains unsupported characters") from e + except ConfigurationError as e: + raise ValidationError("Invalid `git_url`") from e + project_slug = f"{data['project_namespace']}/{project_name_stripped}" data["new_project_url"] = new_project_url data["project_name_stripped"] = project_name_stripped diff --git a/tests/service/controllers/test_templates_create_project.py b/tests/service/controllers/test_templates_create_project.py index 492854c99e..8e8ac5c0ad 100644 --- a/tests/service/controllers/test_templates_create_project.py +++ b/tests/service/controllers/test_templates_create_project.py @@ -16,7 +16,10 @@ # See the License for the specific language governing permissions and # limitations under the License. """Renku service templates create project controller tests.""" -from renku.core.utils.scm import strip_and_lower +import pytest +from marshmallow import ValidationError + +from renku.core.utils.scm import normalize_to_ascii def test_template_create_project_ctrl(ctrl_init, svc_client_templates_creation, mocker): @@ -85,6 +88,60 @@ def test_template_create_project_ctrl(ctrl_init, svc_client_templates_creation, assert payload["project_namespace"] == received_metadata["__namespace__"] assert payload["project_repository"] == received_metadata["__repository__"] - project_name = strip_and_lower(payload["project_name"]) + project_name = normalize_to_ascii(payload["project_name"]) assert project_name == received_metadata["__sanitized_project_name__"] assert f"{payload['project_namespace']}/{project_name}" == received_metadata["__project_slug__"] + + +@pytest.mark.parametrize( + "project_name,expected_name", + [ + ("Test renku-core /é", "test-renku-core"), + ("Test renku-core é", "test-renku-core"), + ("Test é renku-core ", "test-renku-core"), + ("é Test é renku-core ", "test-renku-core"), + ("Test/renku-core", "test-renku-core"), + ("Test 😁", "test"), + ("invalid wörd", "invalid-w-rd"), + ("invalid wörd and another invalid wórd", "invalid-w-rd-and-another-invalid-w-rd"), + ("João", "jo-o"), + ("'My Input String", "my-input-string"), + ("My Input String", "my-input-string"), + (" a new project ", "a-new-project"), + ("test!_pro-ject~", "test-_pro-ject"), + ("test!!!!_pro-ject~", "test-_pro-ject"), + ("Test:-)", "test"), + ("-Test:-)-", "test"), + ], +) +def test_project_name_handler(project_name, expected_name, ctrl_init, svc_client_templates_creation, mocker): + """Test template create project controller correct set of project name.""" + from renku.service.controllers.templates_create_project import TemplatesCreateProjectCtrl + + cache, user_data = ctrl_init + svc_client, headers, payload, rm_remote = svc_client_templates_creation + payload["project_name"] = project_name + + ctrl = TemplatesCreateProjectCtrl(cache, user_data, payload) + mocker.patch.object(ctrl, "new_project_push", return_value=None) + response = ctrl.to_response() + + # Check response. + assert {"result"} == response.json.keys() + assert {"url", "namespace", "name"} == response.json["result"].keys() + assert expected_name == response.json["result"]["name"] + + +@pytest.mark.parametrize("project_name", ["здрасти"]) +def test_except_project_name_handler(project_name, ctrl_init, svc_client_templates_creation, mocker): + """Test template create project controller exception raised.""" + from renku.service.controllers.templates_create_project import TemplatesCreateProjectCtrl + + cache, user_data = ctrl_init + svc_client, headers, payload, rm_remote = svc_client_templates_creation + payload["project_name"] = project_name + + with pytest.raises(ValidationError) as exc_info: + TemplatesCreateProjectCtrl(cache, user_data, payload) + + assert "Invalid `git_url`" in str(exc_info.value) diff --git a/tests/service/views/test_templates_views.py b/tests/service/views/test_templates_views.py index 7165116dcc..a833e977a6 100644 --- a/tests/service/views/test_templates_views.py +++ b/tests/service/views/test_templates_views.py @@ -25,7 +25,7 @@ from flaky import flaky from renku.core.commands.init import fetch_template_from_git, read_template_manifest -from renku.core.utils.scm import strip_and_lower +from renku.core.utils.scm import normalize_to_ascii from renku.service.config import RENKU_EXCEPTION_ERROR_CODE @@ -109,7 +109,7 @@ def test_create_project_from_template(svc_client_templates_creation): assert response assert {"result"} == set(response.json.keys()) - stripped_name = strip_and_lower(payload["project_name"]) + stripped_name = normalize_to_ascii(payload["project_name"]) assert stripped_name == response.json["result"]["name"] expected_url = "{0}/{1}/{2}".format(payload["project_repository"], payload["project_namespace"], stripped_name,) assert expected_url == response.json["result"]["url"]