From b61a1498e1309985cb0abf4fa7260ce7a9a0848c Mon Sep 17 00:00:00 2001 From: Jared Steiner Date: Wed, 17 Jun 2026 14:46:19 +1000 Subject: [PATCH 1/2] fix(spawner): Normalize Task names to valid RFC 1123 names Task names were built as "-". For Jira sources item.ID is the issue key (e.g. PROJECT-1234), whose uppercase letters are invalid in Kubernetes object names, so Task creation failed for every Jira-sourced item. GitHub and Cron sources were unaffected because their IDs are already name-safe. Route both task-name construction sites through buildTaskName, which returns already-valid names unchanged and otherwise normalizes to a valid RFC 1123 name. Using it at both the dedup-lookup and creation sites keeps the two derivations identical. The raw issue key is preserved everywhere else it is used (prompt, URL, logs). Co-Authored-By: Claude Opus 4.8 --- cmd/kelos-spawner/main.go | 23 ++++++++- cmd/kelos-spawner/taskname_test.go | 82 ++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 2 deletions(-) create mode 100644 cmd/kelos-spawner/taskname_test.go diff --git a/cmd/kelos-spawner/main.go b/cmd/kelos-spawner/main.go index 7e1f67ff6..06d2f629c 100644 --- a/cmd/kelos-spawner/main.go +++ b/cmd/kelos-spawner/main.go @@ -16,6 +16,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/apimachinery/pkg/util/validation" clientgoscheme "k8s.io/client-go/kubernetes/scheme" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/cache" @@ -301,7 +302,7 @@ func runCycleWithSourceCore(ctx context.Context, cl client.Client, key types.Nam var newItems []source.WorkItem for _, item := range items { - taskName := fmt.Sprintf("%s-%s", ts.Name, item.ID) + taskName := buildTaskName(ts.Name, item.ID) existing, found := existingTaskMap[taskName] if !found { newItems = append(newItems, item) @@ -367,7 +368,7 @@ func runCycleWithSourceCore(ctx context.Context, cl client.Client, key types.Nam break } - taskName := fmt.Sprintf("%s-%s", ts.Name, item.ID) + taskName := buildTaskName(ts.Name, item.ID) templateVars := source.WorkItemToTemplateVars(item) @@ -807,3 +808,21 @@ func parsePollInterval(s string) time.Duration { } return d } + +// buildTaskName combines a spawner name and work item ID into a valid RFC 1123 +// DNS subdomain for use as a Task object name. It normalizes IDs that are not +// name-safe on their own, such as Jira issue keys (e.g. "PROJECT-1234"), whose +// uppercase letters would otherwise be rejected by Kubernetes. IDs that are +// already name-safe are returned unchanged. +func buildTaskName(spawnerName, itemID string) string { + combined := spawnerName + "-" + itemID + + if len(validation.IsDNS1123Subdomain(combined)) == 0 { + return combined + } + + segments := strings.FieldsFunc(strings.ToLower(combined), func(r rune) bool { + return !('a' <= r && r <= 'z' || '0' <= r && r <= '9') + }) + return strings.Join(segments, "-") +} diff --git a/cmd/kelos-spawner/taskname_test.go b/cmd/kelos-spawner/taskname_test.go new file mode 100644 index 000000000..82772298a --- /dev/null +++ b/cmd/kelos-spawner/taskname_test.go @@ -0,0 +1,82 @@ +package main + +import ( + "context" + "testing" + + "k8s.io/apimachinery/pkg/util/validation" + "sigs.k8s.io/controller-runtime/pkg/client" + + kelos "github.com/kelos-dev/kelos/api/v1alpha2" + "github.com/kelos-dev/kelos/internal/source" +) + +func TestBuildTaskName(t *testing.T) { + cases := []struct { + name string + spawner string + id string + want string + }{ + {"jira key is lowercased", "my-spawner", "PROJECT-1234", "my-spawner-project-1234"}, + {"github numeric id unchanged", "spawner", "42", "spawner-42"}, + {"cron timestamp unchanged", "spawner", "20060102-1504", "spawner-20060102-1504"}, + {"invalid characters replaced", "spawner", "PROJ_1", "spawner-proj-1"}, + {"consecutive dashes collapsed", "spawner", "PROJ--1", "spawner-proj-1"}, + {"dots treated as separators", "spawner", "a..b", "spawner-a-b"}, + {"trailing separator trimmed", "spawner", "ABC-", "spawner-abc"}, + {"all-invalid id falls back to spawner", "spawner", "@@@", "spawner"}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := buildTaskName(tc.spawner, tc.id) + if got != tc.want { + t.Errorf("buildTaskName(%q, %q) = %q, want %q", tc.spawner, tc.id, got, tc.want) + } + // The result must be a name the Kubernetes API server accepts. + if errs := validation.IsDNS1123Subdomain(got); len(errs) != 0 { + t.Errorf("buildTaskName(%q, %q) = %q is not a valid RFC 1123 name: %v", tc.spawner, tc.id, got, errs) + } + }) + } +} + +func TestRunCycleWithSource_NormalizesJiraKeyAndDedupes(t *testing.T) { + ts := newTaskSpawner("spawner", "default", nil) + cl, key := setupTest(t, ts) + + src := &fakeSource{ + items: []source.WorkItem{ + {ID: "PROJECT-1234", Number: 1234, Title: "Fix the thing"}, + }, + } + + // First cycle: the Task is created with a normalized name. + if err := runCycleWithSource(context.Background(), cl, key, src); err != nil { + t.Fatalf("first cycle: unexpected error: %v", err) + } + + var taskList kelos.TaskList + if err := cl.List(context.Background(), &taskList, client.InNamespace("default")); err != nil { + t.Fatalf("listing tasks: %v", err) + } + if len(taskList.Items) != 1 { + t.Fatalf("after first cycle: expected 1 task, got %d", len(taskList.Items)) + } + if got, want := taskList.Items[0].Name, "spawner-project-1234"; got != want { + t.Errorf("task name = %q, want %q", got, want) + } + + // Second cycle, same item: must not create a duplicate. This proves the + // dedup lookup name and the creation name are derived identically. + if err := runCycleWithSource(context.Background(), cl, key, src); err != nil { + t.Fatalf("second cycle: unexpected error: %v", err) + } + if err := cl.List(context.Background(), &taskList, client.InNamespace("default")); err != nil { + t.Fatalf("listing tasks: %v", err) + } + if len(taskList.Items) != 1 { + t.Fatalf("after second cycle: expected 1 task (no duplicate), got %d", len(taskList.Items)) + } +} From 821970010beed8523501d5e51b01b1081acdd276 Mon Sep 17 00:00:00 2001 From: Jared Steiner Date: Tue, 23 Jun 2026 12:39:28 +1000 Subject: [PATCH 2/2] fix(spawner): Cap normalized Task names at the RFC 1123 label length buildTaskName could return a name longer than 63 characters, which is invalid where the name is reused as the kelos.dev/task label value and the Job name. Cap the result at DNS1123LabelMaxLength and, when truncating, append a short hash of the full name so distinct names do not collide on a shared prefix during dedup. Add the length check to the fast path too, so an already-valid but over-length name no longer bypasses the cap. Co-Authored-By: Claude Opus 4.8 --- cmd/kelos-spawner/main.go | 29 +++++++++++++++++++++++------ cmd/kelos-spawner/taskname_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 6 deletions(-) diff --git a/cmd/kelos-spawner/main.go b/cmd/kelos-spawner/main.go index 06d2f629c..222079771 100644 --- a/cmd/kelos-spawner/main.go +++ b/cmd/kelos-spawner/main.go @@ -2,6 +2,8 @@ package main import ( "context" + "crypto/sha256" + "encoding/hex" "flag" "fmt" "net/http" @@ -809,20 +811,35 @@ func parsePollInterval(s string) time.Duration { return d } +// taskNameMaxLength bounds generated Task names to a single RFC 1123 label. +// Although a Kubernetes object name may be up to DNS1123SubdomainMaxLength, the +// name is reused as the kelos.dev/task label value and as the Job name, both of +// which are limited to a 63-character label. +const taskNameMaxLength = validation.DNS1123LabelMaxLength + // buildTaskName combines a spawner name and work item ID into a valid RFC 1123 -// DNS subdomain for use as a Task object name. It normalizes IDs that are not -// name-safe on their own, such as Jira issue keys (e.g. "PROJECT-1234"), whose -// uppercase letters would otherwise be rejected by Kubernetes. IDs that are -// already name-safe are returned unchanged. +// name (at most taskNameMaxLength) for use as a Task object name. It normalizes +// IDs that are not name-safe on their own, such as Jira issue keys (e.g. +// "PROJECT-1234"), whose uppercase letters would otherwise be rejected by +// Kubernetes. Names that are already valid and within length are returned +// unchanged. Over-length names are truncated with a short hash suffix so that +// distinct names do not collide on a shared prefix. func buildTaskName(spawnerName, itemID string) string { combined := spawnerName + "-" + itemID - if len(validation.IsDNS1123Subdomain(combined)) == 0 { + if len(combined) <= taskNameMaxLength && len(validation.IsDNS1123Subdomain(combined)) == 0 { return combined } segments := strings.FieldsFunc(strings.ToLower(combined), func(r rune) bool { return !('a' <= r && r <= 'z' || '0' <= r && r <= '9') }) - return strings.Join(segments, "-") + name := strings.Join(segments, "-") + + if len(name) > taskNameMaxLength { + sum := sha256.Sum256([]byte(name)) + suffix := "-" + hex.EncodeToString(sum[:])[:10] + name = strings.TrimRight(name[:taskNameMaxLength-len(suffix)], "-") + suffix + } + return name } diff --git a/cmd/kelos-spawner/taskname_test.go b/cmd/kelos-spawner/taskname_test.go index 82772298a..be4b7b4f2 100644 --- a/cmd/kelos-spawner/taskname_test.go +++ b/cmd/kelos-spawner/taskname_test.go @@ -2,6 +2,7 @@ package main import ( "context" + "strings" "testing" "k8s.io/apimachinery/pkg/util/validation" @@ -42,6 +43,30 @@ func TestBuildTaskName(t *testing.T) { } } +func TestBuildTaskNameLength(t *testing.T) { + // A valid but over-length name (lowercase alphanumerics) must still be + // capped: this exercises both the length guard on the fast path and the + // truncation branch. + got := buildTaskName("spawner", strings.Repeat("a", 100)) + if len(got) > taskNameMaxLength { + t.Errorf("name length = %d, want <= %d (%q)", len(got), taskNameMaxLength, got) + } + if errs := validation.IsDNS1123Subdomain(got); len(errs) != 0 { + t.Errorf("name %q is not a valid RFC 1123 name: %v", got, errs) + } + + // Distinct over-length inputs that share a truncated prefix must not collide, + // otherwise dedup would silently skip one of them. + a := buildTaskName("spawner", strings.Repeat("a", 80)+"-one") + b := buildTaskName("spawner", strings.Repeat("a", 80)+"-two") + if a == b { + t.Errorf("distinct long inputs collided: both produced %q", a) + } + if len(a) > taskNameMaxLength || len(b) > taskNameMaxLength { + t.Errorf("truncated names exceed %d: %q (%d), %q (%d)", taskNameMaxLength, a, len(a), b, len(b)) + } +} + func TestRunCycleWithSource_NormalizesJiraKeyAndDedupes(t *testing.T) { ts := newTaskSpawner("spawner", "default", nil) cl, key := setupTest(t, ts)