Skip to content

Commit

Permalink
[Projects] Restrict project name to DNS Label (RFC 1123) (#611)
Browse files Browse the repository at this point in the history
  • Loading branch information
Hedingber committed Dec 21, 2020
1 parent 0dfb625 commit 4795b6c
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 19 deletions.
10 changes: 4 additions & 6 deletions mlrun/api/utils/projects/leader.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def store_project(
project: mlrun.api.schemas.Project,
):
self._enrich_project(project)
self._validate_project_name(name)
self.validate_project_name(name)
self._validate_body_and_path_names_matches(name, project)
self._run_on_all_followers("store_project", session, name, project)
return self.get_project(session, name)
Expand Down Expand Up @@ -223,9 +223,7 @@ def _ensure_project_synced(
# if it was created prior to 0.6.0, and the version was upgraded
# we do not want to sync these projects since it will anyways fail (Nuclio doesn't allow these names
# as well)
if not self._validate_project_name(
project_name, raise_on_failure=False
):
if not self.validate_project_name(project_name, raise_on_failure=False):
return
for missing_follower in missing_followers:
logger.debug(
Expand Down Expand Up @@ -297,7 +295,7 @@ def _initialize_follower(

def _enrich_and_validate_before_creation(self, project: mlrun.api.schemas.Project):
self._enrich_project(project)
self._validate_project_name(project.metadata.name)
self.validate_project_name(project.metadata.name)

@staticmethod
def _enrich_project(project: mlrun.api.schemas.Project):
Expand All @@ -311,7 +309,7 @@ def _enrich_project_patch(project_patch: dict):
]

@staticmethod
def _validate_project_name(name: str, raise_on_failure: bool = True) -> bool:
def validate_project_name(name: str, raise_on_failure: bool = True) -> bool:
try:
mlrun.utils.helpers.verify_field_regex(
"project.metadata.name", name, mlrun.utils.regex.project_name
Expand Down
12 changes: 12 additions & 0 deletions mlrun/projects/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
)
from ..runtimes.utils import add_code_metadata
import mlrun.api.schemas
import mlrun.api.utils.projects.leader


class ProjectError(Exception):
Expand Down Expand Up @@ -210,6 +211,17 @@ def __init__(self, name=None, created=None, labels=None):
self.created = created
self.labels = labels or {}

@property
def name(self) -> str:
"""Project name"""
return self._name

@name.setter
def name(self, name):
if name:
mlrun.api.utils.projects.leader.Member.validate_project_name(name)
self._name = name


class ProjectSpec(ModelObj):
def __init__(
Expand Down
2 changes: 1 addition & 1 deletion mlrun/utils/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ def verify_field_regex(field_name, field_value, patterns):
for pattern in patterns:
if not re.match(pattern, str(field_value)):
logger.warn(
"Field is malformed. Does not match required pattern)",
"Field is malformed. Does not match required pattern",
field_name=field_name,
field_value=field_value,
pattern=pattern,
Expand Down
14 changes: 13 additions & 1 deletion mlrun/utils/regex.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,18 @@
r"^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$",
]

# DNS Label (RFC 1123) - used by k8s for resource names format
# https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apimachinery/pkg/util/validation/validation.go#L183
dns_1123_label = [
r"^.{0,63}$",
r"^[a-z0-9]([-a-z0-9]*[a-z0-9])?$",
]

run_name = label_value

project_name = dns_1123_subdomain
# A project name have the following restrictions:
# It should be a valid Nuclio Project CRD name which is dns 1123 subdomain
# It should be a valid k8s label value since Nuclio use the project name in labels of resources
# It should be a valid namespace name (cause we plan to map it to one) which is dns 1123 label
# of the 3 restrictions, dns 1123 label is the most strict, so we enforce only it
project_name = dns_1123_label
20 changes: 9 additions & 11 deletions tests/api/utils/projects/test_leader_member.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,33 +193,31 @@ def test_create_and_store_project_failure_invalid_name(
{"name": "asd3", "valid": True},
{"name": "asd-asd", "valid": True},
{"name": "333", "valid": True},
{"name": "3.a-b", "valid": True},
{"name": "5.a-a.5", "valid": True},
{"name": "3-a-b", "valid": True},
{"name": "5-a-a-5", "valid": True},
{
# Invalid because the first letter is -
"name": "-as-123_2.8a",
"name": "-as-123-2-8a",
"valid": False,
},
{
# Invalid because the last letter is .
"name": "as-123_2.8a.",
# Invalid because there is .
"name": "as-123-2.a",
"valid": False,
},
{
# Invalid because A is not allowed
"name": "As-123_2.8Aa",
"name": "As-123-2-8Aa",
"valid": False,
},
{
# Invalid because _ is not allowed
"name": "as-123_2.8Aa",
"name": "as-123_2-8aa",
"valid": False,
},
{
# Invalid because it's more than 253 characters
"name": "azsxdcfvg-azsxdcfvg-azsxdcfvg-azsxdcfvg-azsxdcfvg-azsxdcfvg-azsxdcfvg-azsxdcfvg-azsxdcfvg-"
"azsxdcfvg-azsxdcfvg-azsxdcfvg-azsxdcfvg-azsxdcfvg-azsxdcfvg-azsxdcfvg-azsxdcfvg-azsxdcfvg-azsxdcfvg-"
"azsxdcfvg-azsxdcfvg-azsxdcfvg-azsxdcfvg-azsxdcfvg-azsxdcfvg-azsx",
# Invalid because it's more than 63 characters
"name": "azsxdcfvg-azsxdcfvg-azsxdcfvg-azsxdcfvg-azsxdcfvg-azsxdcfvg-azsx",
"valid": False,
},
]
Expand Down
8 changes: 8 additions & 0 deletions tests/projects/test_project.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import pathlib
import pytest

import deepdiff

import mlrun
import mlrun.errors
import mlrun.projects.project
import tests.conftest

Expand Down Expand Up @@ -60,3 +62,9 @@ def test_create_project_from_file_with_legacy_structure():
)
== {}
)


def test_create_project_with_invalid_name():
invalid_name = "project_name"
with pytest.raises(mlrun.errors.MLRunInvalidArgumentError):
mlrun.projects.project.new_project(invalid_name, init_git=False)

0 comments on commit 4795b6c

Please sign in to comment.