Skip to content

Commit 4c446b2

Browse files
ian-floresian-flores
committed
fix: use Sprig arg order for regexReplaceAll in job template
The launcher uses Sprig's regexReplaceAll(regex, source, repl) argument order, not Helm's (regex, repl, source). When piping a value into regexReplaceAll, the piped value becomes the replacement string (not the source), causing label values to be destroyed during sanitization. Switch to explicit args matching Sprig's native order. Also includes: $jobMap conversion for struct/map compatibility, roborev code improvements, and Helm chart CRD sync.
1 parent 7f23406 commit 4c446b2

File tree

11 files changed

+73
-58
lines changed

11 files changed

+73
-58
lines changed

‎api/templates/2.5.0/job.tpl‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,12 @@ spec:
116116
{{- if hasKey $jobMap $rule.field }}
117117
{{- $val := index $jobMap $rule.field }}
118118
{{- if $rule.labelKey }}
119-
{{- /* NOTE: regexReplaceAll uses Helm's arg order (regex, repl, s) — see job_tpl_test.go canary test */ -}}
120-
{{- $labelVal := $val | toString | regexReplaceAll "[^a-zA-Z0-9._-]" "_" | regexReplaceAll "_{2,}" "_" | trunc 63 | regexReplaceAll "[^a-zA-Z0-9]+$" "" | regexReplaceAll "^[^a-zA-Z0-9]+" "" }}
119+
{{- /* regexReplaceAll uses Sprig arg order: (regex, source, replacement). Do NOT pipe into it — the piped value becomes the replacement, not the source. */ -}}
120+
{{- $labelVal := regexReplaceAll "[^a-zA-Z0-9._-]" ($val | toString) "_" }}
121+
{{- $labelVal = regexReplaceAll "_{2,}" $labelVal "_" }}
122+
{{- $labelVal = $labelVal | trunc 63 }}
123+
{{- $labelVal = regexReplaceAll "[^a-zA-Z0-9]+$" $labelVal "" }}
124+
{{- $labelVal = regexReplaceAll "^[^a-zA-Z0-9]+" $labelVal "" }}
121125
{{- if ne $labelVal "" }}
122126
{{ $rule.labelKey }}: {{ $labelVal | quote }}
123127
{{- end }}
@@ -127,7 +131,12 @@ spec:
127131
{{- /* Go validation (ValidateDynamicLabelRules) enforces namePrefix < 53 chars, so $maxSuffix is always > 0. */ -}}
128132
{{- $maxSuffix := int (sub 63 (len $namePrefix)) }}
129133
{{- range $match := $matches }}
130-
{{- $suffix := trimPrefix ($rule.trimPrefix | default "") $match | lower | regexReplaceAll "[^a-zA-Z0-9._-]" "_" | regexReplaceAll "_{2,}" "_" | trunc $maxSuffix | regexReplaceAll "[^a-zA-Z0-9]+$" "" | regexReplaceAll "^[^a-zA-Z0-9]+" "" }}
134+
{{- $suffix := trimPrefix ($rule.trimPrefix | default "") $match | lower }}
135+
{{- $suffix = regexReplaceAll "[^a-zA-Z0-9._-]" $suffix "_" }}
136+
{{- $suffix = regexReplaceAll "_{2,}" $suffix "_" }}
137+
{{- $suffix = $suffix | trunc $maxSuffix }}
138+
{{- $suffix = regexReplaceAll "[^a-zA-Z0-9]+$" $suffix "" }}
139+
{{- $suffix = regexReplaceAll "^[^a-zA-Z0-9]+" $suffix "" }}
131140
{{- $computedKey := printf "%s%s" $rule.labelPrefix $suffix }}
132141
{{- /* Guard: a regex rule could theoretically produce a label whose key matches the
133142
operator's cap-reached annotation key (see annotations block above). Labels and

‎api/templates/job_tpl_test.go‎

Lines changed: 25 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"bytes"
55
"encoding/json"
66
"fmt"
7-
"regexp"
87
"strings"
98
"testing"
109
"text/template"
@@ -53,7 +52,12 @@ posit.team/dynamic-label-cap-reached: "true"
5352
{{- if hasKey $jobMap $rule.field }}
5453
{{- $val := index $jobMap $rule.field }}
5554
{{- if $rule.labelKey }}
56-
{{- $labelVal := $val | toString | regexReplaceAll "[^a-zA-Z0-9._-]" "_" | regexReplaceAll "_{2,}" "_" | trunc 63 | regexReplaceAll "[^a-zA-Z0-9]+$" "" | regexReplaceAll "^[^a-zA-Z0-9]+" "" }}
55+
{{- /* regexReplaceAll uses Sprig arg order: (regex, source, replacement). Do NOT pipe into it. */ -}}
56+
{{- $labelVal := regexReplaceAll "[^a-zA-Z0-9._-]" ($val | toString) "_" }}
57+
{{- $labelVal = regexReplaceAll "_{2,}" $labelVal "_" }}
58+
{{- $labelVal = $labelVal | trunc 63 }}
59+
{{- $labelVal = regexReplaceAll "[^a-zA-Z0-9]+$" $labelVal "" }}
60+
{{- $labelVal = regexReplaceAll "^[^a-zA-Z0-9]+" $labelVal "" }}
5761
{{- if ne $labelVal "" }}
5862
{{ $rule.labelKey }}: {{ $labelVal | quote }}
5963
{{- end }}
@@ -63,7 +67,12 @@ posit.team/dynamic-label-cap-reached: "true"
6367
{{- /* Go validation (ValidateDynamicLabelRules) enforces namePrefix < 53 chars, so $maxSuffix is always > 0. */ -}}
6468
{{- $maxSuffix := int (sub 63 (len $namePrefix)) }}
6569
{{- range $match := $matches }}
66-
{{- $suffix := trimPrefix ($rule.trimPrefix | default "") $match | lower | regexReplaceAll "[^a-zA-Z0-9._-]" "_" | regexReplaceAll "_{2,}" "_" | trunc $maxSuffix | regexReplaceAll "[^a-zA-Z0-9]+$" "" | regexReplaceAll "^[^a-zA-Z0-9]+" "" }}
70+
{{- $suffix := trimPrefix ($rule.trimPrefix | default "") $match | lower }}
71+
{{- $suffix = regexReplaceAll "[^a-zA-Z0-9._-]" $suffix "_" }}
72+
{{- $suffix = regexReplaceAll "_{2,}" $suffix "_" }}
73+
{{- $suffix = $suffix | trunc $maxSuffix }}
74+
{{- $suffix = regexReplaceAll "[^a-zA-Z0-9]+$" $suffix "" }}
75+
{{- $suffix = regexReplaceAll "^[^a-zA-Z0-9]+" $suffix "" }}
6776
{{- $computedKey := printf "%s%s" $rule.labelPrefix $suffix }}
6877
{{- /* must match reservedOperatorAnnotationKey in session_config.go */ -}}
6978
{{- if and (ne $suffix "") (ne $computedKey "posit.team/dynamic-label-cap-reached") }}
@@ -76,8 +85,8 @@ posit.team/dynamic-label-cap-reached: "true"
7685
{{- end }}`
7786

7887
// renderDynamicLabels renders the dynamic labels template block with the given
79-
// session config (templateData) and Job data. Uses Helm-compatible regexReplaceAll
80-
// argument order (regex, repl, s) where the piped value is the source string.
88+
// session config (templateData) and Job data. Uses Sprig's native regexReplaceAll
89+
// argument order (regex, s, repl) — the template uses explicit args, not piping.
8190
func renderDynamicLabels(t *testing.T, templateData map[string]any, jobData map[string]any) string {
8291
t.Helper()
8392

@@ -89,18 +98,8 @@ func renderDynamicLabels(t *testing.T, templateData map[string]any, jobData map[
8998
tmpl := template.New("gotpl")
9099
f := TemplateFuncMap(tmpl)
91100
f = AddOnFuncMap(tmpl, f)
92-
// Override regexReplaceAll to match Helm's pipeline-friendly argument order.
93-
// Sprig: regexReplaceAll(regex, s, repl) — piped value becomes repl.
94-
// Helm: regexReplaceAll(regex, repl, s) — piped value becomes s (source).
95-
// NOTE: If upgrading Helm/Sprig, verify that the production argument order still
96-
// matches this mock — otherwise tests will pass against a stale signature.
97-
// TODO: These Go tests exercise a mocked argument order, not the real Helm rendering
98-
// pipeline. Consider adding an integration test that renders through `helm template`
99-
// to validate the actual end-to-end rendering path.
100-
f["regexReplaceAll"] = func(regex string, repl string, s string) string {
101-
r := regexp.MustCompile(regex)
102-
return r.ReplaceAllString(s, repl)
103-
}
101+
// No regexReplaceAll override needed — the template now uses Sprig's native
102+
// argument order (regex, source, replacement) with explicit args instead of piping.
104103
tmpl.Funcs(f)
105104

106105
_, err = tmpl.Parse(mockDataDefine + "\n" + dynamicLabelsTemplate)
@@ -118,9 +117,9 @@ func renderDynamicLabels(t *testing.T, templateData map[string]any, jobData map[
118117
}
119118

120119
// TestCanary_SprigRegexReplaceAllOrder verifies that Sprig's regexReplaceAll
121-
// still uses (regex, s, repl) order, which differs from Helm's (regex, repl, s).
122-
// Our mock in renderDynamicLabels overrides to Helm's order. If Sprig ever changes
123-
// to match Helm, this canary will fire and the mock override can be removed.
120+
// still uses (regex, s, repl) order. Our template uses explicit args matching
121+
// this order. If Sprig ever changes to Helm's order (regex, repl, s), the
122+
// template's explicit calls would break and need updating.
124123
func TestCanary_SprigRegexReplaceAllOrder(t *testing.T) {
125124
tmpl := template.New("canary")
126125
f := TemplateFuncMap(tmpl)
@@ -130,12 +129,10 @@ func TestCanary_SprigRegexReplaceAllOrder(t *testing.T) {
130129
fn, ok := prodFn.(func(string, string, string) string)
131130
require.True(t, ok, "regexReplaceAll should be func(string, string, string) string")
132131

133-
// With Sprig order (regex, s, repl): fn("h", "world", "hello") → replace "h" in "world" → "world" (no match)
134-
// With Helm order (regex, repl, s): fn("h", "world", "hello") → replace "h" in "hello" → "worldello"
135-
result := fn("h", "world", "hello")
136-
if result == "worldello" {
137-
t.Fatalf("Sprig regexReplaceAll now uses Helm's argument order — remove the mock override in renderDynamicLabels")
138-
}
132+
// With Sprig order (regex, s, repl): fn("h", "hello", "world") → replace "h" in "hello" → "worldello"
133+
// If Sprig changes order, this assertion will fail.
134+
result := fn("h", "hello", "world")
135+
assert.Equal(t, "worldello", result, "Sprig regexReplaceAll order changed — template explicit args need updating")
139136
}
140137

141138
// TestDynamicLabelsTemplate_DriftDetection verifies that the dynamicLabelsTemplate
@@ -156,8 +153,8 @@ func TestDynamicLabelsTemplate_DriftDetection(t *testing.T) {
156153
{"per-rule cap", "slice $matches 0 50"},
157154
{"global cap", "sub 200"},
158155
{"raw match cap", "regexFindAll $rule.match $str 500"},
159-
{"sanitize non-alnum", `regexReplaceAll "[^a-zA-Z0-9._-]" "_"`},
160-
{"collapse underscores", `regexReplaceAll "_{2,}" "_"`},
156+
{"sanitize non-alnum", `regexReplaceAll "[^a-zA-Z0-9._-]"`},
157+
{"collapse underscores", `regexReplaceAll "_{2,}"`},
161158
{"reserved key guard", `posit.team/dynamic-label-cap-reached`},
162159
{"dedup seen dict", "$seen := dict"},
163160
{"global total dict", `$globalTotal := dict "n" 0`},

‎config/crd/bases/core.posit.team_connects.yaml‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1916,9 +1916,10 @@ spec:
19161916
description: |-
19171917
Match is a regex pattern applied to the field value. Each match produces a label.
19181918
For array fields (like "args"), elements are joined with spaces before matching.
1919-
At runtime, at most 50 matches are applied per rule and 200 regex-matched labels
1920-
across all rules; direct-mapping labels (using labelKey) are not counted toward
1921-
the 200 global cap. Excess matches are dropped and a
1919+
At runtime, at most 500 raw regex matches are collected per rule (to bound memory),
1920+
then deduplicated and capped at 50 unique matches per rule and 200 regex-matched
1921+
labels across all rules; direct-mapping labels (using labelKey) are not counted
1922+
toward the 200 global cap. Excess matches are dropped and a
19221923
posit.team/dynamic-label-cap-reached annotation is set on the pod. Rules are
19231924
evaluated in array order, so earlier rules in the dynamicLabels list consume cap
19241925
budget first; later rules may receive fewer or no matches if the global cap is

‎config/crd/bases/core.posit.team_sites.yaml‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2960,9 +2960,10 @@ spec:
29602960
description: |-
29612961
Match is a regex pattern applied to the field value. Each match produces a label.
29622962
For array fields (like "args"), elements are joined with spaces before matching.
2963-
At runtime, at most 50 matches are applied per rule and 200 regex-matched labels
2964-
across all rules; direct-mapping labels (using labelKey) are not counted toward
2965-
the 200 global cap. Excess matches are dropped and a
2963+
At runtime, at most 500 raw regex matches are collected per rule (to bound memory),
2964+
then deduplicated and capped at 50 unique matches per rule and 200 regex-matched
2965+
labels across all rules; direct-mapping labels (using labelKey) are not counted
2966+
toward the 200 global cap. Excess matches are dropped and a
29662967
posit.team/dynamic-label-cap-reached annotation is set on the pod. Rules are
29672968
evaluated in array order, so earlier rules in the dynamicLabels list consume cap
29682969
budget first; later rules may receive fewer or no matches if the global cap is

‎config/crd/bases/core.posit.team_workbenches.yaml‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2147,9 +2147,10 @@ spec:
21472147
description: |-
21482148
Match is a regex pattern applied to the field value. Each match produces a label.
21492149
For array fields (like "args"), elements are joined with spaces before matching.
2150-
At runtime, at most 50 matches are applied per rule and 200 regex-matched labels
2151-
across all rules; direct-mapping labels (using labelKey) are not counted toward
2152-
the 200 global cap. Excess matches are dropped and a
2150+
At runtime, at most 500 raw regex matches are collected per rule (to bound memory),
2151+
then deduplicated and capped at 50 unique matches per rule and 200 regex-matched
2152+
labels across all rules; direct-mapping labels (using labelKey) are not counted
2153+
toward the 200 global cap. Excess matches are dropped and a
21532154
posit.team/dynamic-label-cap-reached annotation is set on the pod. Rules are
21542155
evaluated in array order, so earlier rules in the dynamicLabels list consume cap
21552156
budget first; later rules may receive fewer or no matches if the global cap is

‎dist/chart/templates/crd/core.posit.team_connects.yaml‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1937,9 +1937,10 @@ spec:
19371937
description: |-
19381938
Match is a regex pattern applied to the field value. Each match produces a label.
19391939
For array fields (like "args"), elements are joined with spaces before matching.
1940-
At runtime, at most 50 matches are applied per rule and 200 regex-matched labels
1941-
across all rules; direct-mapping labels (using labelKey) are not counted toward
1942-
the 200 global cap. Excess matches are dropped and a
1940+
At runtime, at most 500 raw regex matches are collected per rule (to bound memory),
1941+
then deduplicated and capped at 50 unique matches per rule and 200 regex-matched
1942+
labels across all rules; direct-mapping labels (using labelKey) are not counted
1943+
toward the 200 global cap. Excess matches are dropped and a
19431944
posit.team/dynamic-label-cap-reached annotation is set on the pod. Rules are
19441945
evaluated in array order, so earlier rules in the dynamicLabels list consume cap
19451946
budget first; later rules may receive fewer or no matches if the global cap is

‎dist/chart/templates/crd/core.posit.team_sites.yaml‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2981,9 +2981,10 @@ spec:
29812981
description: |-
29822982
Match is a regex pattern applied to the field value. Each match produces a label.
29832983
For array fields (like "args"), elements are joined with spaces before matching.
2984-
At runtime, at most 50 matches are applied per rule and 200 regex-matched labels
2985-
across all rules; direct-mapping labels (using labelKey) are not counted toward
2986-
the 200 global cap. Excess matches are dropped and a
2984+
At runtime, at most 500 raw regex matches are collected per rule (to bound memory),
2985+
then deduplicated and capped at 50 unique matches per rule and 200 regex-matched
2986+
labels across all rules; direct-mapping labels (using labelKey) are not counted
2987+
toward the 200 global cap. Excess matches are dropped and a
29872988
posit.team/dynamic-label-cap-reached annotation is set on the pod. Rules are
29882989
evaluated in array order, so earlier rules in the dynamicLabels list consume cap
29892990
budget first; later rules may receive fewer or no matches if the global cap is

‎dist/chart/templates/crd/core.posit.team_workbenches.yaml‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2168,9 +2168,10 @@ spec:
21682168
description: |-
21692169
Match is a regex pattern applied to the field value. Each match produces a label.
21702170
For array fields (like "args"), elements are joined with spaces before matching.
2171-
At runtime, at most 50 matches are applied per rule and 200 regex-matched labels
2172-
across all rules; direct-mapping labels (using labelKey) are not counted toward
2173-
the 200 global cap. Excess matches are dropped and a
2171+
At runtime, at most 500 raw regex matches are collected per rule (to bound memory),
2172+
then deduplicated and capped at 50 unique matches per rule and 200 regex-matched
2173+
labels across all rules; direct-mapping labels (using labelKey) are not counted
2174+
toward the 200 global cap. Excess matches are dropped and a
21742175
posit.team/dynamic-label-cap-reached annotation is set on the pod. Rules are
21752176
evaluated in array order, so earlier rules in the dynamicLabels list consume cap
21762177
budget first; later rules may receive fewer or no matches if the global cap is

‎internal/crdapply/bases/core.posit.team_connects.yaml‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1916,9 +1916,10 @@ spec:
19161916
description: |-
19171917
Match is a regex pattern applied to the field value. Each match produces a label.
19181918
For array fields (like "args"), elements are joined with spaces before matching.
1919-
At runtime, at most 50 matches are applied per rule and 200 regex-matched labels
1920-
across all rules; direct-mapping labels (using labelKey) are not counted toward
1921-
the 200 global cap. Excess matches are dropped and a
1919+
At runtime, at most 500 raw regex matches are collected per rule (to bound memory),
1920+
then deduplicated and capped at 50 unique matches per rule and 200 regex-matched
1921+
labels across all rules; direct-mapping labels (using labelKey) are not counted
1922+
toward the 200 global cap. Excess matches are dropped and a
19221923
posit.team/dynamic-label-cap-reached annotation is set on the pod. Rules are
19231924
evaluated in array order, so earlier rules in the dynamicLabels list consume cap
19241925
budget first; later rules may receive fewer or no matches if the global cap is

‎internal/crdapply/bases/core.posit.team_sites.yaml‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2960,9 +2960,10 @@ spec:
29602960
description: |-
29612961
Match is a regex pattern applied to the field value. Each match produces a label.
29622962
For array fields (like "args"), elements are joined with spaces before matching.
2963-
At runtime, at most 50 matches are applied per rule and 200 regex-matched labels
2964-
across all rules; direct-mapping labels (using labelKey) are not counted toward
2965-
the 200 global cap. Excess matches are dropped and a
2963+
At runtime, at most 500 raw regex matches are collected per rule (to bound memory),
2964+
then deduplicated and capped at 50 unique matches per rule and 200 regex-matched
2965+
labels across all rules; direct-mapping labels (using labelKey) are not counted
2966+
toward the 200 global cap. Excess matches are dropped and a
29662967
posit.team/dynamic-label-cap-reached annotation is set on the pod. Rules are
29672968
evaluated in array order, so earlier rules in the dynamicLabels list consume cap
29682969
budget first; later rules may receive fewer or no matches if the global cap is

0 commit comments

Comments
 (0)