Skip to content

Commit a2e0207

Browse files
authored
Sanitize build IDs to match k8s label value requirements (#138)
<!--- Note to EXTERNAL Contributors --> <!-- Thanks for opening a PR! If it is a significant code change, please **make sure there is an open issue** for this. We work best with you when we have accepted the idea first before you code. --> <!--- For ALL Contributors 👇 --> ## What was changed <!-- Describe what has changed in this PR --> Build IDs are now sanitized in order to match k8s label value requirements. ## Why? <!-- Tell your future self why have you made these changes --> This ensures that the value of the label `temporal.io/build-id` applied by the controller to k8s resources can match the build ID registered with Temporal. ## Checklist <!--- add/delete as needed ---> 1. Closes <!-- add issue number here --> N/A 2. How was this tested: <!--- Please describe how you tested your changes/how we can test them --> Unit tests updated 3. Any docs updates needed? <!--- update README if applicable or point out where to update docs.temporal.io --> No
1 parent 571ae6a commit a2e0207

File tree

2 files changed

+20
-3
lines changed

2 files changed

+20
-3
lines changed

internal/k8s/deployments.go

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ func ComputeBuildID(w *temporaliov1alpha1.TemporalWorkerDeployment) string {
117117
shortHashSuffix := ResourceNameSeparator + utils.ComputeHash(&w.Spec.Template, nil, true)
118118
maxImgLen := MaxBuildIdLen - len(shortHashSuffix)
119119
imagePrefix := computeImagePrefix(img, maxImgLen)
120-
return imagePrefix + shortHashSuffix
120+
return cleanBuildID(imagePrefix + shortHashSuffix)
121121
}
122122
}
123123
return utils.ComputeHash(&w.Spec.Template, nil, false)
@@ -165,6 +165,23 @@ func CleanStringForDNS(s string) string {
165165
return re.ReplaceAllString(s, ResourceNameSeparator)
166166
}
167167

168+
// Build ID is used as a label in k8s, and as the build ID for
169+
// the worker in Temporal. That means it needs to conform to both
170+
// system's requirements.
171+
//
172+
// https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set
173+
// Valid label value:
174+
// - must be 63 characters or less (can be empty),
175+
// - unless empty, must begin and end with an alphanumeric character ([a-z0-9A-Z]),
176+
// - could contain dashes (-), underscores (_), dots (.), and alphanumerics between.
177+
//
178+
// Temporal build IDs only need to be ASCII.
179+
func cleanBuildID(s string) string {
180+
// Keep only letters, numbers, dashes, and dots.
181+
re := regexp.MustCompile(`[^a-zA-Z0-9-._]+`)
182+
return re.ReplaceAllString(s, ResourceNameSeparator)
183+
}
184+
168185
// NewDeploymentWithOwnerRef creates a new deployment resource, including owner references
169186
func NewDeploymentWithOwnerRef(
170187
typeMeta *metav1.TypeMeta,

internal/k8s/deployments_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,7 @@ func TestGenerateBuildID(t *testing.T) {
323323
twd := testhelpers.MakeTWDWithImage("", "", namedImg)
324324
return twd, nil // only check 1 result, no need to compare
325325
},
326-
expectedPrefix: "library/busybox",
326+
expectedPrefix: "library-busybox",
327327
expectedHashLen: 4,
328328
expectEquality: false,
329329
},
@@ -334,7 +334,7 @@ func TestGenerateBuildID(t *testing.T) {
334334
twd := testhelpers.MakeTWDWithImage("", "", illegalCharsImg)
335335
return twd, nil // only check 1 result, no need to compare
336336
},
337-
expectedPrefix: "this.is.my_weird/image",
337+
expectedPrefix: "this.is.my_weird-image",
338338
expectedHashLen: 4,
339339
expectEquality: false,
340340
},

0 commit comments

Comments
 (0)