Skip to content

Commit

Permalink
Improve k8s pipes pod name sanitization (#24252)
Browse files Browse the repository at this point in the history
Summary:
Restrict additional characters in addition to just underscores (most
notably, capital letters, but other non-alphanumeric characters as
well).

Test Plan: New test case

## Summary & Motivation

## How I Tested These Changes

## Changelog [New | Bug | Docs]

> Replace this message with a changelog entry, or `NOCHANGELOG`
  • Loading branch information
gibsondan authored Sep 5, 2024
1 parent 8dbdbe9 commit 1760f78
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 2 deletions.
3 changes: 2 additions & 1 deletion python_modules/libraries/dagster-k8s/dagster_k8s/pipes.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import os
import random
import re
import string
from contextlib import contextmanager
from pathlib import Path
Expand Down Expand Up @@ -43,7 +44,7 @@


def get_pod_name(run_id: str, op_name: str):
clean_op_name = op_name.replace("_", "-")
clean_op_name = re.sub("[^a-z0-9-]", "", op_name.lower().replace("_", "-"))
suffix = "".join(random.choice(string.digits) for i in range(10))
return f"dagster-{run_id[:18]}-{clean_op_name[:20]}-{suffix}"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,13 @@

import pytest
from dagster._core.errors import DagsterInvariantViolationError
from dagster_k8s.pipes import _DEV_NULL_MESSAGE_WRITER, _detect_current_namespace, build_pod_body
from dagster._core.utils import make_new_run_id
from dagster_k8s.pipes import (
_DEV_NULL_MESSAGE_WRITER,
_detect_current_namespace,
build_pod_body,
get_pod_name,
)
from dagster_pipes import DAGSTER_PIPES_CONTEXT_ENV_VAR, DAGSTER_PIPES_MESSAGES_ENV_VAR


Expand Down Expand Up @@ -446,3 +452,11 @@ def test_pipes_client_namespace_autodetection_from_secret(tmpdir, kubeconfig_dum
namespace_secret_path.write_text("my-namespace-from-secret")
got = _detect_current_namespace(kubeconfig_with_namespace, namespace_secret_path)
assert got == "my-namespace-from-secret"


def test_pipes_pod_name_sanitization():
capital_op_name = "WHY_ARE&_YOU!_YELLING_AND_WHY_IS_THIS_OP_NAME_SO_LONG"
run_id = make_new_run_id()
capital_pod_name = get_pod_name(run_id, capital_op_name)
assert capital_pod_name.startswith(f"dagster-{run_id[:18]}-why-are-you-yelling--")
assert len(capital_pod_name) <= 63

0 comments on commit 1760f78

Please sign in to comment.