Skip to content

fix: lowercase CleanStringForDNS output for RFC 1123 compliance#228

Draft
aarontsharp wants to merge 1 commit intotemporalio:mainfrom
aarontsharp:fix/clean-string-for-dns-lowercase
Draft

fix: lowercase CleanStringForDNS output for RFC 1123 compliance#228
aarontsharp wants to merge 1 commit intotemporalio:mainfrom
aarontsharp:fix/clean-string-for-dns-lowercase

Conversation

@aarontsharp
Copy link

@aarontsharp aarontsharp commented Mar 12, 2026

Summary

CleanStringForDNS strips invalid characters but does not lowercase the result. When an image tag contains uppercase characters (e.g. myimage:master--HEAD), ComputeVersionedDeploymentName produces a deployment name that violates RFC 1123 DNS label rules, causing Kubernetes to reject the Deployment.

This adds strings.ToLower() to CleanStringForDNS so deployment names are always valid DNS labels.

Why CleanStringForDNS and not cleanBuildID?

The build ID is also used as a Kubernetes label value and as the Temporal worker build ID. Labels allow uppercase ([a-zA-Z0-9._-]), and Temporal build IDs just need to be ASCII. Lowercasing only in CleanStringForDNS keeps the fix scoped to where DNS compliance is required (resource names) without affecting label values or Temporal version matching.

Kubernetes resource names must conform to RFC 1123 DNS labels, which
require lowercase characters. CleanStringForDNS strips invalid characters
but does not lowercase the result, causing deployment creation to fail
when the image tag contains uppercase characters (e.g. "master--HEAD").

This adds strings.ToLower() to CleanStringForDNS so that
ComputeVersionedDeploymentName always produces valid DNS labels.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants