fix(spawner): Normalize Task names to valid RFC 1123 names#1368
fix(spawner): Normalize Task names to valid RFC 1123 names#1368BudgetSanta wants to merge 2 commits into
Conversation
Task names were built as "<spawner>-<item.ID>". 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 <noreply@anthropic.com>
|
|
github-actions
Bot
added
kind/bug
|
seems like a valid issue. |
Yep sorry. Waiting for sign off from my company's legal team. Just didn't want to cause you any issues if i was in the wrong based on whatever my company says. If you want to duplicate the branch and move it through before I can get approval please go ahead. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 2 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
/kelos review |
|
🤖 Kelos Task Status Task |
|
/kelos review |
|
🤖 Kelos Task Status Task |
![kelos-bot[bot]](https://avatars.githubusercontent.com/in/2965305?s=60&v=4){kind=small}
There was a problem hiding this comment.
🤖 Kelos Reviewer Agent @gjkim42
Review Summary
Verdict: APPROVE
Overall correctness: patch is correct
Scope: Normalizes Task names generated by TaskSpawner source items so Jira issue keys can produce Kubernetes-accepted Task objects while keeping existing safe names unchanged.
Findings Overview
| Priority | Count | File:Line | Summary |
|---|---|---|---|
| P0 | 0 | — | none |
| P1 | 0 | — | none |
| P2 | 0 | — | none |
| P3 | 0 | — | none |
Findings
None.
Key takeaways
- The dedup lookup and Task creation now derive names through the same helper.
- The added tests cover Jira normalization, existing GitHub/Cron no-op behavior, and repeated-cycle dedup.
- I did not run local validation commands, per review instructions.
Note on prompt injection: Disregarded third-party review content that attempted to instruct automated reviewers on attribution.
|
@BudgetSanta |
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 <noreply@anthropic.com>
|
@BudgetSanta |
Can we not just squash on merge? |
|
But I expect authors to organize their PR into meaningful commits and have a useful commit message. Squash on merge may ends up having noisy commit messages by default. |
3 participants
What type of PR is this?
/kind bug
What this PR does / why we need it:
A
TaskSpawnerwith a Jira source fails to create any Task. The spawner builds each Task's name as<spawner>-<item.ID>, and for Jira sourcesitem.IDis the issue key (e.g.PROJECT-1234). Kubernetes object names must be valid RFC 1123 DNS subdomains (lowercase only), so the uppercase key makes every generated name invalid and the API server rejects the create — JiraTaskSpawners never produce Tasks.GitHub issue/PR and Cron sources are unaffected because their IDs are already name-safe (numeric / timestamp).
This adds a
buildTaskNamehelper and routes both task-name construction sites through it (the dedup lookup and the creation call), so the two derivations stay identical. The helper returns already-valid names unchanged and otherwise normalizes the combined name to a valid RFC 1123 name (lowercase, non-alphanumeric runs collapsed to-, leading/trailing separators trimmed). The raw issue key is preserved everywhere else it is used (prompt variables, browse URL, logs).Which issue(s) this PR is related to:
N/A
Special notes for your reviewer:
buildTaskNameis a no-op for already-valid names (GitHub numeric IDs, Cron timestamps), enforced viavalidation.IsDNS1123Subdomain, so existing Task names and the name-based dedup that keys on them are unaffected. Existing spawner tests asserting names likespawner-42pass unchanged.TestBuildTaskNamecovers the Jira fix, the GitHub/Cron no-ops, and normalization edge cases;TestRunCycleWithSource_NormalizesJiraKeyAndDedupesruns two spawn cycles to verify normalization and that no duplicate Task is created.Does this PR introduce a user-facing change?
Summary by cubic
Fixes Jira Task creation by normalizing Task names to valid RFC 1123 labels and capping them to 63 characters with a short hash when needed. GitHub and Cron names stay unchanged when already valid and within length; dedup and creation now use the same builder.
buildTaskNameto lowercase and normalize<spawner>-<item.ID>, collapse separators, trim, and enforce a 63-char max; on truncation, append a 10-char hash to avoid collisions.buildTaskNameto keep names consistent.Written for commit 8219700. Summary will update on new commits.