diff --git a/sky/clouds/kubernetes.py b/sky/clouds/kubernetes.py index 0d8e8983576..9777a28948b 100644 --- a/sky/clouds/kubernetes.py +++ b/sky/clouds/kubernetes.py @@ -403,11 +403,17 @@ def get_current_user_identity(cls) -> Optional[List[str]]: @classmethod def is_label_valid(cls, label_key: str, label_value: str) -> Tuple[bool, Optional[str]]: + # Kubernetes labels can be of the format /: key_regex = re.compile( - r'^(?:[a-z0-9]([-a-z0-9]*[a-z0-9])?\/)?[a-z0-9]([-a-z0-9_.]{0,61}' - r'[a-z0-9])?$') + # Look-ahead to ensure proper domain formatting up to a slash + r'^(?:(?=[a-z0-9]([-a-z0-9.]*[a-z0-9])?\/)' + # Match domain: starts and ends with alphanum up to 253 chars + # including a slash in the domain. + r'[a-z0-9]([-a-z0-9.]{0,251}[a-z0-9])?\/)?' + # Match key: starts and ends with alphanum, upto to 63 chars. + r'[a-z0-9]([-a-z0-9_.]{0,61}[a-z0-9])?$') value_regex = re.compile( - r'^[a-zA-Z0-9]([-a-zA-Z0-9_.]{0,61}[a-zA-Z0-9])?$') + r'^([a-zA-Z0-9]([-a-zA-Z0-9_.]{0,61}[a-zA-Z0-9])?)?$') key_valid = bool(key_regex.match(label_key)) value_valid = bool(value_regex.match(label_value)) error_msg = None diff --git a/tests/unit_tests/test_resources.py b/tests/unit_tests/test_resources.py index fb2825e7688..f9e3ad51630 100644 --- a/tests/unit_tests/test_resources.py +++ b/tests/unit_tests/test_resources.py @@ -1,7 +1,23 @@ +from typing import Dict from unittest.mock import Mock +import pytest + +from sky import clouds from sky.resources import Resources +GLOBAL_VALID_LABELS = { + 'plaintext': 'plainvalue', + 'numbers123': '123', +} + +GLOBAL_INVALID_LABELS = { + 'l' * 129: 'value', # Long key + 'key': 'v' * 257, # Long value + 'spaces in label': 'spaces in value', + '': 'emptykey', +} + def test_get_reservations_available_resources(): mock = Mock() @@ -11,3 +27,62 @@ def test_get_reservations_available_resources(): r.get_reservations_available_resources() mock.get_reservations_available_resources.assert_called_once_with( "instance_type", "region", "zone", set()) + + +def _run_label_test(allowed_labels: Dict[str, str], + invalid_labels: Dict[str, str], cloud: clouds.Cloud): + """Run a test for labels with the given allowed and invalid labels.""" + r_allowed = Resources(cloud=cloud, labels=allowed_labels) # Should pass + assert r_allowed.labels == allowed_labels, ('Allowed labels ' + 'should be the same') + + # Check for each invalid label + for invalid_label, value in invalid_labels.items(): + l = {invalid_label: value} + with pytest.raises(ValueError): + _ = Resources(cloud=cloud, labels=l) + assert False, (f'Resources were initialized with ' + f'invalid label {invalid_label}={value}') + + +def test_gcp_labels_resources(): + allowed_labels = { + **GLOBAL_VALID_LABELS, + } + invalid_labels = { + **GLOBAL_INVALID_LABELS, + 'domain/key': 'value', + '1numericstart': 'value', + } + cloud = clouds.GCP() + _run_label_test(allowed_labels, invalid_labels, cloud) + + +def test_aws_labels_resources(): + allowed_labels = { + **GLOBAL_VALID_LABELS, + } + invalid_labels = { + **GLOBAL_INVALID_LABELS, + 'aws:cannotstartwithaws': 'value', + } + cloud = clouds.AWS() + _run_label_test(allowed_labels, invalid_labels, cloud) + + +def test_kubernetes_labels_resources(): + allowed_labels = { + **GLOBAL_VALID_LABELS, + 'kueue.x-k8s.io/queue-name': 'queue', + 'a' * 253 + '/' + 'k' * 63: 'v' * + 63, # upto 253 chars in domain, 63 in key + 'mylabel': '', # empty label values are allowed by k8s + '1numericstart': 'value', # numeric start is allowed by k8s + } + invalid_labels = { + **GLOBAL_INVALID_LABELS, + 'a' * 254 + '/' + 'k' * 64: 'v' * + 63, # exceed 253 chars in domain, 63 in key + } + cloud = clouds.Kubernetes() + _run_label_test(allowed_labels, invalid_labels, cloud)