refactor(go.d/dyncfg): split job-name validation per domain (#22247)
Ilya Mashchenko committed
Apr 22, 2026 at 11:55 UTC
531d5493837417706e3489332c24bac76d552532
12 files changed
+100
-29
src/go/plugin/agent/discovery/sd/dyncfg.go
+5
-1
@@ -83,6 +83,10 @@ func (cb *sdCallbacks) ExtractKey(fn dyncfg.Function) (key, name string, ok bool
83
return dt + ":" + name, name, true
84
}
85
86
+func (cb *sdCallbacks) ValidateJobName(name string) error {
87
+ return dyncfg.JobNameRuleAllowDots(name)
88
+}
89
+
90
func (cb *sdCallbacks) ParseAndValidate(fn dyncfg.Function, name string) (sdConfig, error) {
91
dt, _, _ := cb.sd.extractDiscovererAndName(fn.ID())
92
if _, err := parseDyncfgPayload(fn.Payload(), dt, name, cb.sd.configDefaults, cb.sd.discovererRegistry(), true); err != nil {
@@ -251,7 +255,7 @@ func (d *ServiceDiscovery) dyncfgCmdTest(fn dyncfg.Function) {
255
if !isJob {
256
name = dyncfgTemplateJobName(fn)
257
}
254
- if err := dyncfg.ValidateJobName(name); err != nil {
258
+ if err := dyncfg.JobNameRuleAllowDots(name); err != nil {
259
d.Warningf("dyncfg: test: unacceptable job name '%s' for '%s': %v", name, dt, err)
260
d.dyncfgApi.SendCodef(fn, 400, "Unacceptable job name '%s': %v.", name, err)
261
return
src/go/plugin/agent/jobmgr/dyncfg_collector_callbacks.go
+4
@@ -58,6 +58,10 @@ func (cb *collectorCallbacks) ExtractKey(fn dyncfg.Function) (key, name string,
58
return key, jn, true
59
}
60
61
+func (cb *collectorCallbacks) ValidateJobName(name string) error {
62
+ return dyncfg.JobNameRuleStrict(name)
63
+}
64
+
65
func (cb *collectorCallbacks) ParseAndValidate(fn dyncfg.Function, name string) (confgroup.Config, error) {
66
mn, ok := cb.mgr.extractModuleName(fn.ID())
67
if !ok {
src/go/plugin/agent/jobmgr/dyncfg_collector_cmds.go
+1
-1
@@ -114,7 +114,7 @@ func (m *Manager) dyncfgCmdTest(fn dyncfg.Function) {
114
115
m.Infof("dyncfg: %s: %s/%s job by user '%s'", cmd, mn, jn, fn.User())
116
117
- if err := dyncfg.ValidateJobName(jn); err != nil {
117
+ if err := dyncfg.JobNameRuleStrict(jn); err != nil {
118
m.Warningf("dyncfg: %s: module %s: unacceptable job name '%s': %v", cmd, mn, jn, err)
119
m.dyncfgResponder.SendCodef(fn, 400, "Unacceptable job name '%s': %v.", jn, err)
120
return
src/go/plugin/agent/jobmgr/dyncfg_collector_test.go
+4
@@ -874,6 +874,10 @@ func (cb *collectorSeqTestCallbacks) ExtractKey(fn dyncfg.Function) (key, name s
874
return cb.mgr.collectorCallbacks.ExtractKey(fn)
875
}
876
877
+func (cb *collectorSeqTestCallbacks) ValidateJobName(name string) error {
878
+ return dyncfg.JobNameRuleStrict(name)
879
+}
880
+
881
func (cb *collectorSeqTestCallbacks) ParseAndValidate(fn dyncfg.Function, _ string) (confgroup.Config, error) {
882
cfg, ok := cb.parsed[fn.Command()]
883
if !ok {
src/go/plugin/agent/jobmgr/secretsctl/callbacks.go
+4
@@ -132,6 +132,10 @@ func (cb *secretStoreCallbacks) ExtractKey(fn dyncfg.Function) (key, name string
132
return key, name, true
133
}
134
135
+func (cb *secretStoreCallbacks) ValidateJobName(name string) error {
136
+ return dyncfg.JobNameRuleAllowDots(name)
137
+}
138
+
139
func (cb *secretStoreCallbacks) ParseAndValidate(fn dyncfg.Function, name string) (secretstore.Config, error) {
140
var kind secretstore.StoreKind
141
if fn.Command() == dyncfg.CommandAdd {
src/go/plugin/agent/jobmgr/secretsctl/dyncfg.go
+1
-1
@@ -60,7 +60,7 @@ func (c *Controller) dyncfgCmdAdd(fn dyncfg.Function) {
60
c.api.SendCodef(fn, 400, "%v", err)
61
return
62
}
63
- if err := dyncfg.ValidateJobName(name); err != nil {
63
+ if err := dyncfg.JobNameRuleAllowDots(name); err != nil {
64
c.api.SendCodef(fn, 400, "invalid job name '%s': %v.", name, err)
65
return
66
}
src/go/plugin/agent/jobmgr/secretstore_deps.go
+1
-1
@@ -289,7 +289,7 @@ func extractSecretStoreKeysFromString(value string, seen map[string]struct{}) {
289
if !kind.IsValid() {
290
continue
291
}
292
- if err := dyncfg.ValidateJobName(name); err != nil {
292
+ if err := dyncfg.JobNameRuleAllowDots(name); err != nil {
293
continue
294
}
295
seen[secretstore.StoreKey(kind, name)] = struct{}{}
src/go/plugin/agent/jobmgr/vnodectl/dyncfg.go
+1
-1
@@ -111,7 +111,7 @@ func (c *Controller) dyncfgCmdAdd(fn dyncfg.Function) {
111
c.api.SendCodef(fn, 400, "Missing vnode name.")
112
return
113
}
114
- if err := dyncfg.ValidateJobName(name); err != nil {
114
+ if err := dyncfg.JobNameRuleAllowDots(name); err != nil {
115
c.Warningf("dyncfg: %s: unacceptable vnode name '%s': %v", dyncfg.CommandAdd, name, err)
116
c.api.SendCodef(fn, 400, "Unacceptable vnode name '%s': %v.", name, err)
117
return
src/go/plugin/agent/secrets/secretstore/helpers.go
+1
-1
@@ -5,5 +5,5 @@ package secretstore
5
import "github.com/netdata/netdata/go/plugins/plugin/framework/dyncfg"
6
7
func validateStoreName(name string) error {
8
- return dyncfg.ValidateJobName(name)
8
+ return dyncfg.JobNameRuleAllowDots(name)
9
}
src/go/plugin/framework/dyncfg/handler.go
+5
-1
@@ -21,6 +21,10 @@ type Callbacks[C Config] interface {
21
// Includes all validation (including heavy checks like module instantiation).
22
ParseAndValidate(fn Function, name string) (C, error)
23
24
+ // ValidateJobName enforces the domain's job-name policy. Called before
25
+ // ParseAndValidate so cheap name-format rejections happen without parsing payload.
26
+ ValidateJobName(name string) error
27
+
28
// Start creates a work unit and starts it. Owns the full start lifecycle
29
// including pre-start cleanup and post-fail retry scheduling.
30
// Return CodedError to override EnableFailCode.
@@ -437,7 +441,7 @@ func (h *Handler[C]) CmdAdd(fn Function) {
441
return
442
}
443
440
- if err := ValidateJobName(name); err != nil {
444
+ if err := h.cb.ValidateJobName(name); err != nil {
445
h.api.SendCodef(fn, 400, "invalid job name '%s': %v.", name, err)
446
return
447
}
src/go/plugin/framework/dyncfg/handler_test.go
+48
-17
@@ -73,6 +73,10 @@ func (m *mockCallbacks) ParseAndValidate(fn Function, name string) (testConfig,
73
return testConfig{uid: "dyncfg:" + name, key: name, sourceType: "dyncfg", source: "test"}, nil
74
}
75
76
+func (m *mockCallbacks) ValidateJobName(name string) error {
77
+ return JobNameRuleStrict(name)
78
+}
79
+
80
func (m *mockCallbacks) Start(cfg testConfig) error {
81
m.startCalls = append(m.startCalls, cfg)
82
if m.startFn != nil {
@@ -1187,31 +1191,58 @@ func TestNotifyJobCreate_SupportedCommands(t *testing.T) {
1191
}
1192
}
1193
1190
-// --- ValidateJobName Tests ---
1194
+// --- Job-name rule tests ---
1195
1192
-func TestValidateJobName(t *testing.T) {
1193
- tests := []struct {
1194
- name string
1196
+func TestJobNameRuleStrict(t *testing.T) {
1197
+ tests := map[string]struct {
1198
input string
1199
wantErr bool
1200
}{
1198
- {"valid", "my_job", false},
1199
- {"valid with numbers", "job123", false},
1200
- {"valid with dashes", "my-job", false},
1201
- {"space", "my job", true},
1202
- {"tab", "my\tjob", true},
1203
- {"dot", "my.job", true},
1204
- {"colon", "my:job", true},
1205
- {"empty", "", false},
1201
+ "valid": {input: "my_job"},
1202
+ "valid with numbers": {input: "job123"},
1203
+ "valid with dashes": {input: "my-job"},
1204
+ "space": {input: "my job", wantErr: true},
1205
+ "tab": {input: "my\tjob", wantErr: true},
1206
+ "dot": {input: "my.job", wantErr: true},
1207
+ "colon": {input: "my:job", wantErr: true},
1208
+ "empty": {input: ""},
1209
}
1210
1208
- for _, tt := range tests {
1209
- t.Run(tt.name, func(t *testing.T) {
1210
- err := ValidateJobName(tt.input)
1211
+ for name, tt := range tests {
1212
+ t.Run(name, func(t *testing.T) {
1213
+ err := JobNameRuleStrict(tt.input)
1214
+ if tt.wantErr {
1215
+ assert.Error(t, err, fmt.Sprintf("JobNameRuleStrict(%q) should fail", tt.input))
1216
+ } else {
1217
+ assert.NoError(t, err, fmt.Sprintf("JobNameRuleStrict(%q) should pass", tt.input))
1218
+ }
1219
+ })
1220
+ }
1221
+}
1222
+
1223
+func TestJobNameRuleAllowDots(t *testing.T) {
1224
+ tests := map[string]struct {
1225
+ input string
1226
+ wantErr bool
1227
+ }{
1228
+ "valid": {input: "my_job"},
1229
+ "valid with numbers": {input: "job123"},
1230
+ "valid with dashes": {input: "my-job"},
1231
+ "dotted name": {input: "my.job"},
1232
+ "fqdn": {input: "host.example.com"},
1233
+ "space": {input: "my job", wantErr: true},
1234
+ "tab": {input: "my\tjob", wantErr: true},
1235
+ "colon": {input: "my:job", wantErr: true},
1236
+ "empty": {input: ""},
1237
+ }
1238
+
1239
+ for name, tt := range tests {
1240
+ t.Run(name, func(t *testing.T) {
1241
+ err := JobNameRuleAllowDots(tt.input)
1242
if tt.wantErr {
1212
- assert.Error(t, err, fmt.Sprintf("ValidateJobName(%q) should fail", tt.input))
1243
+ assert.Error(t, err, fmt.Sprintf("JobNameRuleAllowDots(%q) should fail", tt.input))
1244
} else {
1214
- assert.NoError(t, err, fmt.Sprintf("ValidateJobName(%q) should pass", tt.input))
1245
+ assert.NoError(t, err, fmt.Sprintf("JobNameRuleAllowDots(%q) should pass", tt.input))
1246
}
1247
})
1248
}
src/go/plugin/framework/dyncfg/validate.go
+25
-5
@@ -8,14 +8,34 @@ import (
8
"unicode"
9
)
10
11
-// ValidateJobName checks that a job name contains no spaces, dots, or colons.
12
-func ValidateJobName(jobName string) error {
13
- for _, r := range jobName {
11
+// JobNameRuleStrict rejects spaces, dots, and colons.
12
+// Use for collector job names, which must not conflict with dyncfg template/job
13
+// ID separators (':') or module hierarchy ('.').
14
+func JobNameRuleStrict(name string) error {
15
+ if err := rejectSpacesAndColons(name); err != nil {
16
+ return err
17
+ }
18
+ for _, r := range name {
19
+ if r == '.' {
20
+ return fmt.Errorf("contains '%c'", r)
21
+ }
22
+ }
23
+ return nil
24
+}
25
+
26
+// JobNameRuleAllowDots rejects spaces and colons but allows dots.
27
+// Use for service discovery, vnode, and secretstore names where dotted identifiers
28
+// are legitimate (e.g. hostnames, FQDNs).
29
+func JobNameRuleAllowDots(name string) error {
30
+ return rejectSpacesAndColons(name)
31
+}
32
+
33
+func rejectSpacesAndColons(name string) error {
34
+ for _, r := range name {
35
if unicode.IsSpace(r) {
36
return errors.New("contains spaces")
37
}
17
- switch r {
18
- case '.', ':':
38
+ if r == ':' {
39
return fmt.Errorf("contains '%c'", r)
40
}
41
}