Skip to content

Commit

Permalink
PTFE-772 generate valid name (#369)
Browse files Browse the repository at this point in the history
Validate the runner group name to ensure a valid runner name when
generating it.
  • Loading branch information
tcarmet authored Aug 23, 2023
1 parent e06e764 commit 998439b
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 12 deletions.
26 changes: 18 additions & 8 deletions runner_manager/models/runner_group.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import logging
import re
from datetime import datetime, timedelta
from typing import Any, List, Optional, Self, Union
from uuid import uuid4
Expand All @@ -12,6 +13,7 @@
from githubkit.webhooks.types import WorkflowJobEvent
from pydantic import BaseModel as PydanticBaseModel
from pydantic import Field as PydanticField
from pydantic import validator
from redis_om import Field, NotFoundError
from typing_extensions import Annotated

Expand All @@ -25,6 +27,8 @@

log = logging.getLogger(__name__)

regex = re.compile(r"[a-z](?:[-a-z0-9]{0,61}[a-z0-9])?|[1-9][0-9]{0,19}")


class BaseRunnerGroup(PydanticBaseModel):
name: str
Expand All @@ -51,7 +55,7 @@ class BaseRunnerGroup(PydanticBaseModel):
class RunnerGroup(BaseModel, BaseRunnerGroup):

id: Optional[int] = Field(index=True, default=None)
name: str = Field(index=True, full_text_search=True)
name: str = Field(index=True, full_text_search=True, max_length=39)
organization: str = Field(index=True, full_text_search=True)
repository: Optional[str] = Field(index=True, full_text_search=True)
max: int = Field(index=True, ge=1, default=20)
Expand All @@ -64,6 +68,16 @@ def __post_init_post_parse__(self):
if self.backend.manager is None:
self.backend.manager = self.manager

@validator("name")
def validate_name(cls, v):
"""Validate group name.
A group name must match the following regex:
'[a-z](?:[-a-z0-9]{0,61}[a-z0-9])?|[1-9][0-9]{0,19}'.
"""
assert regex.fullmatch(v), f"Group name {v} must be match of regex: {regex}"
return v

@property
def runner_labels(self) -> List[RunnerLabel]:
"""Return self.labels as a list of RunnerLabel."""
Expand All @@ -75,16 +89,12 @@ def generate_runner_name(self) -> str:
Returns a string used as the runner name.
- Prefixed by the group name.
- Suffixed by a random uuid.
- Limited by 63 characters.
- Match the following regex:
'[a-z](?:[-a-z0-9]{0,61}[a-z0-9])?|[1-9][0-9]{0,19}'.
- Must be unique and not already exist in the database.
"""

def _generate_name() -> str:
"""Generate a random name."""

return f"{self.name}-{uuid4()}"

name = _generate_name()
name: str = f"{self.name}-{uuid4()}"
return name

def get_runners(self) -> List[Runner]:
Expand Down
10 changes: 7 additions & 3 deletions tests/strategies.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from string import ascii_letters
from string import ascii_lowercase
from typing import Optional

from githubkit.rest.models import Runner as GitHubRunner
Expand Down Expand Up @@ -33,8 +33,12 @@ class Repo(Repository):
license: Optional[License] = None


# Text strategy with only normal characters
Text = st.text(ascii_letters, min_size=10, max_size=10)
# Text strategy with only ascii characters
# that must start with a alphabetic character,
# and only lowercase characters
Text = st.text(ascii_lowercase, min_size=10, max_size=10).filter(
lambda x: x[0].isalpha()
)
Int = st.integers(min_value=1, max_value=100)
UserStrategy = st.builds(
User,
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/jobs/test_workflow_job.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ def test_workflow_job_queued(
assert webhook.organization
runner_group: RunnerGroup = RunnerGroup(
organization=webhook.organization.login,
name=uuid4().hex,
name=f"queued-{uuid4().hex.lower()}",
labels=webhook.workflow_job.labels,
manager=settings.name,
backend={"name": "base"},
Expand Down
27 changes: 27 additions & 0 deletions tests/unit/models/test_runner_group.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from githubkit.rest.models import AuthenticationToken
from githubkit.webhooks.models import WorkflowJobCompleted
from hypothesis import given
from pydantic import ValidationError
from redis_om import Migrator, NotFoundError

from runner_manager import Runner
Expand Down Expand Up @@ -115,3 +116,29 @@ def test_update_from_base(runner_group: RunnerGroup, github: GitHub):
assert basegroup.name != runner_group.name
runner_group.update(**basegroup.dict(exclude_unset=True))
assert basegroup.name == runner_group.name


def test_runner_group_name():
allowed_names = [
"test",
"test-42",
"42",
"a" * 39,
]
forbidden_names = ["TEST", "test 42", "42-test", "a" * 40]
for name in allowed_names:
group = RunnerGroup(
name=name,
organization="test",
backend={"name": "base"},
labels=["test"],
)
assert group.name == name
for name in forbidden_names:
with pytest.raises(ValidationError):
RunnerGroup(
name=name,
organization="test",
backend={"name": "base"},
labels=["test"],
)

0 comments on commit 998439b

Please sign in to comment.