Skip to content

Commit 7b7eb9e

Browse files
peterroelantsclaude
andcommitted
fix(kuberay): strip trailing non-alphanumeric chars after truncating K8s label values
`normalize_k8s_label_values` stripped leading/trailing non-alphanumeric characters before truncating to 63 chars. For long inputs (e.g. git branch names used as `dagster/git-branch`-style labels), truncation could then land on `_`, `-`, or `.` and the resulting label would fail K8s's label-value regex `(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?`, causing RayCluster creation to fail with HTTP 422. Swap the order: truncate first, then strip. Add parametrized regression tests covering trailing `_`, `.`, `-`, and a collapsed-hyphen run straddling the 63-char boundary. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 351a227 commit 7b7eb9e

2 files changed

Lines changed: 43 additions & 5 deletions

File tree

src/dagster_ray/kuberay/utils.py

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,13 @@ def normalize_k8s_label_values(labels: dict[str, str]) -> dict[str, str]:
5858
# Collapse multiple consecutive hyphens into a single hyphen
5959
value = re.sub(r"-+", "-", value)
6060

61-
# Remove leading/trailing non-alphanumeric characters
62-
value = _LEADING_TRAILING_NON_ALNUM_PATTERN.sub("", value)
63-
64-
# Truncate to 63 characters
65-
normalized[key] = value[:63]
61+
# Truncate to 63 characters first, then strip leading/trailing
62+
# non-alphanumeric characters. The order matters: stripping before
63+
# truncating leaves the last char exposed to whatever lands at
64+
# position 63 — which can be `_`, `-`, or `.` for long inputs and
65+
# would then violate K8s's label-value regex (must end alphanumeric).
66+
value = value[:63]
67+
normalized[key] = _LEADING_TRAILING_NON_ALNUM_PATTERN.sub("", value)
6668

6769
# replace keys starting with `dagster.io/` with `dagster/`
6870
normalized_with_fixed_dagster_keys = {}

tests/kuberay/test_utils.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
1+
import re
2+
3+
import pytest
4+
15
from dagster_ray.kuberay.utils import normalize_k8s_label_values
26

7+
# From the K8s label-value spec:
8+
# https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set
9+
_K8S_LABEL_VALUE = re.compile(r"^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$")
10+
311

412
def test_normalize_k8s_label_values(snapshot):
513
assert snapshot == normalize_k8s_label_values(
@@ -18,3 +26,31 @@ def test_normalize_k8s_label_values(snapshot):
1826

1927
def test_normalize_k8s_label_values_important_labels():
2028
assert normalize_k8s_label_values({"dagster/run-id": "12345"}) == {"dagster/run-id": "12345"}
29+
30+
31+
@pytest.mark.parametrize(
32+
"value",
33+
[
34+
# Git branch name > 63 chars whose 63rd char is `_`. Regression for
35+
# KubeRay RayCluster creation failing with
36+
# `metadata.labels: Invalid value … must start and end with an
37+
# alphanumeric character` when a PR's long branch name was being
38+
# stuck into a `dagster/git-branch`-style label.
39+
"04-22-datastack_reshape_metaxy_map-entry_columns_on_inputmodel_"
40+
"to_silence_spurious_pydantic_serializer_warnings_in_ray_workers",
41+
# Truncation landing on `.`
42+
"a" * 63 + "." + "b" * 10,
43+
# Truncation landing on `-`
44+
"a" * 63 + "-" + "b" * 10,
45+
# Collapsed-hyphen run exactly at position 63.
46+
"a" * 62 + "--" + "b" * 10,
47+
],
48+
)
49+
def test_normalize_k8s_label_values_truncated_tail_is_alphanumeric(value: str) -> None:
50+
"""Truncation to 63 chars must never leave a trailing non-alphanumeric
51+
character; doing so produces a label value that K8s rejects with HTTP 422."""
52+
out = normalize_k8s_label_values({"k": value})["k"]
53+
54+
assert len(out) <= 63
55+
assert out == "" or out[-1].isalnum(), f"label ends non-alphanumeric: {out!r}"
56+
assert _K8S_LABEL_VALUE.match(out), f"value violates K8s label regex: {out!r}"

0 commit comments

Comments
 (0)