Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SS-1206 Site dir option should have path validation #259

Merged
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion apps/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
from enum import Enum
from typing import Optional

from django.core.exceptions import ObjectDoesNotExist
import regex as re
from django.core.exceptions import ObjectDoesNotExist, ValidationError
Comment on lines +5 to +6
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isort was not applied here

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from django.db import transaction

from apps.types_.subdomain import SubdomainCandidateName, SubdomainTuple
Expand Down Expand Up @@ -371,3 +372,25 @@ def save_instance_and_related_data(instance, form):
instance.set_k8s_values()
instance.url = get_URI(instance)
instance.save(update_fields=["k8s_values", "url"])


def validate_path_k8s_label_compatible(candidate: str):
anondo1969 marked this conversation as resolved.
Show resolved Hide resolved
"""
Validates to be compatible with k8s labels specification.
See: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set
The RegexValidator will raise a ValidationError if the input does not match the regular expression.
It is up to the caller to handle the raised exception if desired.
"""
error_message = (
"Please provide a valid path. "
"It can be empty. "
"Otherwise, it must be 63 characters or less. "
" It must begin and end with an alphanumeric character (a-z, or 0-9, or A-Z)."
" It could contain dashes ( - ), underscores ( _ ), dots ( . ), "
"and alphanumerics."
)

pattern = r"^(?:[a-zA-Z0-9](?:[a-zA-Z0-9._-]{0,61}[a-zA-Z0-9])?)?$"

if not re.match(pattern, candidate):
raise ValidationError(error_message)
20 changes: 20 additions & 0 deletions apps/migrations/0019_alter_shinyinstance_shiny_site_dir.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Generated by Django 5.1.1 on 2024-11-27 12:47

import apps.models.app_types.shiny
from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
("apps", "0018_customappinstance_default_url_subpath"),
]

operations = [
migrations.AlterField(
model_name="shinyinstance",
name="shiny_site_dir",
field=models.CharField(
blank=True, default="", max_length=255, validators=[apps.helpers.validate_path_k8s_label_compatible]
),
),
]
5 changes: 4 additions & 1 deletion apps/models/app_types/shiny.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from django.db import models

from apps.helpers import validate_path_k8s_label_compatible
from apps.models import (
AppInstanceManager,
BaseAppInstance,
Expand Down Expand Up @@ -35,7 +36,9 @@ class ShinyInstance(BaseAppInstance, SocialMixin, LogsEnabledMixin):
container_waittime = models.IntegerField(default=20000)
heartbeat_timeout = models.IntegerField(default=60000)
heartbeat_rate = models.IntegerField(default=10000)
shiny_site_dir = models.CharField(max_length=255, default="", blank=True)
shiny_site_dir = models.CharField(
validators=[validate_path_k8s_label_compatible], max_length=255, default="", blank=True
)

# The following three settings control the pre-init and seats behaviour (see documentation)
# These settings override the Helm chart default values
Expand Down
69 changes: 69 additions & 0 deletions apps/tests/test_forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from django.test import TestCase

from apps.forms import CustomAppForm
from apps.helpers import validate_path_k8s_label_compatible
from apps.models import Apps, AppStatus, Subdomain, VolumeInstance
from apps.models.app_types.custom.custom import validate_default_url_subpath
from projects.models import Flavor, Project
Expand Down Expand Up @@ -240,3 +241,71 @@ def test_invalid_default_url_subpath(invalid_default_url_subpath):
valid_check = False

assert not valid_check


invalid_shiny_site_dir_list = [
"-invalidStart", # Starts with a non-alphanumeric character
"invalidEnd-", # Ends with a non-alphanumeric character
".dotStart", # Starts with a dot
"dotEnd.", # Ends with a dot
"_underscoreStart", # Starts with an underscore
"underscoreEnd_", # Ends with an underscore
"label with spaces", # Contains spaces
"label@value", # Contains an invalid character (@)
"too_long_label_with_more_than_sixty_three_characters__1234567890", # Exceeds 63 characters
"just-dashes-", # Ends with a dash
"123#", # Contains an invalid character (#)
".....", # Only contains dots
"-a", # Starts with a dash
"_a_", # Starts and ends with underscores
" ", # Contains only whitespace
]

valid_shiny_site_dir_list = [
"", # Empty string is allowed
"a", # Single alphanumeric character
"validLabel", # Alphanumeric characters only
"label-123", # Contains a dash
"label_with_underscores", # Contains underscores
"label.with.dots", # Contains dots
"abc-def_ghi.jkl", # Contains all allowed special characters
"label1", # Ends with a number
"1stLabel", # Starts with a number
"example-label", # Simple example with a dash
"nested.label.value", # Dots between words
"underscore_ending_label", # Underscore in the middle
"valid-value-0123", # Contains numbers and special characters
"long-valid-label-abcdefg-hijklmn-opqrstuv-wxyz", # Long but within 63 characters
"labelvalue123456789", # Combination of letters and numbers
"consecutive-dashes--allowed", # Contains consecutive dashes
"consecutive_underscores__allowed", # Contains consecutive underscores
"dots..in..between", # Contains consecutive dots
"mixed__--..label", # Contains a mix of consecutive allowed characters
"label---with---many---dashes", # Multiple consecutive dashes in different parts
"label..with..dots", # Multiple consecutive dots in between
"valid_--..mix_12", # Combination of numbers, letters, and allowed characters
"simple.label-value_1-2-3", # Mixed with numbers, dots, underscores, and dashes
"complex__label..value--mixed", # A complex mix with all allowed characters in a consecutive manner
]


@pytest.mark.parametrize("valid_shiny_site_dir", valid_shiny_site_dir_list)
def test_valid_shiny_site_dir(valid_shiny_site_dir):
valid_check = True
try:
validate_path_k8s_label_compatible(valid_shiny_site_dir)
except ValidationError:
valid_check = False

assert valid_check


@pytest.mark.parametrize("invalid_shiny_site_dir", invalid_shiny_site_dir_list)
def test_invalid_shiny_site_dir(invalid_shiny_site_dir):
valid_check = True
try:
validate_path_k8s_label_compatible(invalid_shiny_site_dir)
except ValidationError:
valid_check = False

assert not valid_check
1 change: 1 addition & 0 deletions cypress/e2e/ui-tests/test-deploy-app.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,7 @@ describe("Test deploying app", () => {
})

// This test is skipped because it will only work against a Serve instance running on our cluster. should be switched on for the e2e tests against remote.
// We need to add a test here for validating Site-dir option. See SS-1206 for details
it.skip("can deploy a shiny app", { defaultCommandTimeout: defaultCmdTimeoutMs }, () => {
// Names of objects to create
const project_name = "e2e-deploy-app-test"
Expand Down
Loading