Skip to content

Commit ddeee1e

Browse files
sjbermansjberman
committed
More specific validators
1 parent db30943 commit ddeee1e

File tree

8 files changed

+417
-53
lines changed

8 files changed

+417
-53
lines changed

‎internal/mode/static/nginx/config/validation/common.go

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -84,12 +84,3 @@ func validateHeaderName(name string) error {
8484
}
8585
return nil
8686
}
87-
88-
// GenericValidator validates values for generic cases in the nginx conf.
89-
type GenericValidator struct{}
90-
91-
// ValidateEscapedStringNoVarExpansion ensures that no invalid characters are included in the string value that
92-
// could lead to unwanted nginx behavior.
93-
func (GenericValidator) ValidateEscapedStringNoVarExpansion(value string) error {
94-
return validateEscapedStringNoVarExpansion(value, nil)
95-
}

‎internal/mode/static/nginx/config/validation/common_test.go

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -71,24 +71,3 @@ func TestValidateValidHeaderName(t *testing.T) {
7171
strings.Repeat("very-long-header", 16)+"1",
7272
)
7373
}
74-
75-
func TestGenericValidator_ValidateEscapedStringNoVarExpansion(t *testing.T) {
76-
validator := GenericValidator{}
77-
78-
testValidValuesForSimpleValidator(
79-
t,
80-
validator.ValidateEscapedStringNoVarExpansion,
81-
`test`,
82-
`test test`,
83-
`\"`,
84-
`\\`,
85-
)
86-
87-
testInvalidValuesForSimpleValidator(
88-
t,
89-
validator.ValidateEscapedStringNoVarExpansion,
90-
`\`,
91-
`test"test`,
92-
`$test`,
93-
)
94-
}

‎internal/mode/static/nginx/config/validation/generic.go

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
package validation
2+
3+
import (
4+
"errors"
5+
"regexp"
6+
7+
k8svalidation "k8s.io/apimachinery/pkg/util/validation"
8+
)
9+
10+
// GenericValidator validates values for generic cases in the nginx conf.
11+
type GenericValidator struct{}
12+
13+
// ValidateEscapedStringNoVarExpansion ensures that no invalid characters are included in the string value that
14+
// could lead to unwanted nginx behavior.
15+
func (GenericValidator) ValidateEscapedStringNoVarExpansion(value string) error {
16+
return validateEscapedStringNoVarExpansion(value, nil)
17+
}
18+
19+
const (
20+
alphaNumericStringFmt = `[a-zA-Z0-9_-]+`
21+
alphaNumericStringErrMsg = "must contain only alphanumeric characters or '-' or '_'"
22+
)
23+
24+
var alphaNumericStringFmtRegexp = regexp.MustCompile("^" + alphaNumericStringFmt + "$")
25+
26+
// ValidateServiceName validates a k8s service name that can only use alphanumeric characters.
27+
func (GenericValidator) ValidateServiceName(name string) error {
28+
if !alphaNumericStringFmtRegexp.MatchString(name) {
29+
examples := []string{
30+
"svc1",
31+
"svc-1",
32+
"svc_1",
33+
}
34+
35+
return errors.New(k8svalidation.RegexError(alphaNumericStringErrMsg, alphaNumericStringFmt, examples...))
36+
}
37+
38+
return nil
39+
}
40+
41+
const (
42+
durationStringFmt = `\d{1,4}(ms|s)?`
43+
durationStringErrMsg = "must contain a number followed by 'ms' or 's'"
44+
)
45+
46+
var durationStringFmtRegexp = regexp.MustCompile("^" + durationStringFmt + "$")
47+
48+
// ValidateNginxDuration validates a duration string that nginx can understand.
49+
func (GenericValidator) ValidateNginxDuration(duration string) error {
50+
if !durationStringFmtRegexp.MatchString(duration) {
51+
examples := []string{
52+
"5ms",
53+
"10s",
54+
}
55+
56+
return errors.New(k8svalidation.RegexError(durationStringFmt, durationStringErrMsg, examples...))
57+
}
58+
59+
return nil
60+
}
61+
62+
const (
63+
//nolint:lll
64+
endpointStringFmt = `(?:http?:\/\/)?[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?(?:\.[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?)*(?::\d{1,5})?`
65+
endpointStringErrMsg = "must be an alphanumeric hostname with optional http scheme and optional port"
66+
)
67+
68+
var endpointStringFmtRegexp = regexp.MustCompile("^" + endpointStringFmt + "$")
69+
70+
// ValidateEndpoint validates an alphanumeric endpoint, with optional http scheme and port.
71+
func (GenericValidator) ValidateEndpoint(endpoint string) error {
72+
if !endpointStringFmtRegexp.MatchString(endpoint) {
73+
examples := []string{
74+
"my-endpoint",
75+
"my.endpoint:5678",
76+
"http://my-endpoint",
77+
}
78+
79+
return errors.New(k8svalidation.RegexError(endpointStringFmt, endpointStringErrMsg, examples...))
80+
}
81+
82+
return nil
83+
}

‎internal/mode/static/nginx/config/validation/generic_test.go

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
package validation
2+
3+
import "testing"
4+
5+
func TestGenericValidator_ValidateEscapedStringNoVarExpansion(t *testing.T) {
6+
validator := GenericValidator{}
7+
8+
testValidValuesForSimpleValidator(
9+
t,
10+
validator.ValidateEscapedStringNoVarExpansion,
11+
`test`,
12+
`test test`,
13+
`\"`,
14+
`\\`,
15+
)
16+
17+
testInvalidValuesForSimpleValidator(
18+
t,
19+
validator.ValidateEscapedStringNoVarExpansion,
20+
`\`,
21+
`test"test`,
22+
`$test`,
23+
)
24+
}
25+
26+
func TestValidateServiceName(t *testing.T) {
27+
validator := GenericValidator{}
28+
29+
testValidValuesForSimpleValidator(
30+
t,
31+
validator.ValidateServiceName,
32+
`test`,
33+
`Test-test`,
34+
`test_Test`,
35+
`test123`,
36+
)
37+
38+
testInvalidValuesForSimpleValidator(
39+
t,
40+
validator.ValidateServiceName,
41+
`test#$%`,
42+
`test test`,
43+
`test.test`,
44+
)
45+
}
46+
47+
func TestValidateNginxDuration(t *testing.T) {
48+
validator := GenericValidator{}
49+
50+
testValidValuesForSimpleValidator(
51+
t,
52+
validator.ValidateNginxDuration,
53+
`5ms`,
54+
`10s`,
55+
`123ms`,
56+
)
57+
58+
testInvalidValuesForSimpleValidator(
59+
t,
60+
validator.ValidateNginxDuration,
61+
`test`,
62+
`12345`,
63+
`5m`,
64+
)
65+
}
66+
67+
func TestValidateEndpoint(t *testing.T) {
68+
validator := GenericValidator{}
69+
70+
testValidValuesForSimpleValidator(
71+
t,
72+
validator.ValidateEndpoint,
73+
`http://my-endpoint:5678`,
74+
`my.endpoint`,
75+
`myendpoint:123`,
76+
`my-endpoint123:456`,
77+
)
78+
79+
testInvalidValuesForSimpleValidator(
80+
t,
81+
validator.ValidateEndpoint,
82+
`https://my-endpoint`,
83+
`my_endpoint`,
84+
`my$endpoint`,
85+
)
86+
}

‎internal/mode/static/state/graph/nginxproxy.go

Lines changed: 10 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -44,25 +44,12 @@ func validateNginxProxy(
4444
var allErrs field.ErrorList
4545
spec := field.NewPath("spec")
4646

47-
validateStringValue := func(
48-
value,
49-
valueName string,
50-
path *field.Path,
51-
validator validation.GenericValidator,
52-
) *field.Error {
53-
if err := validator.ValidateEscapedStringNoVarExpansion(value); err != nil {
54-
return field.Invalid(path.Child(valueName), value, err.Error())
55-
}
56-
57-
return nil
58-
}
59-
6047
telemetry := npCfg.Spec.Telemetry
6148
if telemetry != nil {
6249
telPath := spec.Child("telemetry")
6350
if telemetry.ServiceName != nil {
64-
if err := validateStringValue(*telemetry.ServiceName, "serviceName", telPath, validator); err != nil {
65-
allErrs = append(allErrs, err)
51+
if err := validator.ValidateEscapedStringNoVarExpansion(*telemetry.ServiceName); err != nil {
52+
allErrs = append(allErrs, field.Invalid(telPath.Child("serviceName"), *telemetry.ServiceName, err.Error()))
6653
}
6754
}
6855

@@ -71,27 +58,27 @@ func validateNginxProxy(
7158
expPath := telPath.Child("exporter")
7259

7360
if exp.Endpoint != "" {
74-
if err := validateStringValue(exp.Endpoint, "endpoint", expPath, validator); err != nil {
75-
allErrs = append(allErrs, err)
61+
if err := validator.ValidateEscapedStringNoVarExpansion(exp.Endpoint); err != nil {
62+
allErrs = append(allErrs, field.Invalid(expPath.Child("endpoint"), exp.Endpoint, err.Error()))
7663
}
7764
}
7865

7966
if exp.Interval != nil {
80-
if err := validateStringValue(string(*exp.Interval), "interval", expPath, validator); err != nil {
81-
allErrs = append(allErrs, err)
67+
if err := validator.ValidateEscapedStringNoVarExpansion(string(*exp.Interval)); err != nil {
68+
allErrs = append(allErrs, field.Invalid(expPath.Child("interval"), *exp.Interval, err.Error()))
8269
}
8370
}
8471
}
8572

8673
if telemetry.SpanAttributes != nil {
8774
spanAttrPath := telPath.Child("spanAttributes")
8875
for _, spanAttr := range telemetry.SpanAttributes {
89-
if err := validateStringValue(spanAttr.Key, "key", spanAttrPath, validator); err != nil {
90-
allErrs = append(allErrs, err)
76+
if err := validator.ValidateEscapedStringNoVarExpansion(spanAttr.Key); err != nil {
77+
allErrs = append(allErrs, field.Invalid(spanAttrPath.Child("key"), spanAttr.Key, err.Error()))
9178
}
9279

93-
if err := validateStringValue(spanAttr.Value, "value", spanAttrPath, validator); err != nil {
94-
allErrs = append(allErrs, err)
80+
if err := validator.ValidateEscapedStringNoVarExpansion(spanAttr.Value); err != nil {
81+
allErrs = append(allErrs, field.Invalid(spanAttrPath.Child("value"), spanAttr.Value, err.Error()))
9582
}
9683
}
9784
}

‎internal/mode/static/state/graph/nginxproxy_test.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,13 +219,19 @@ func TestValidateNginxProxy(t *testing.T) {
219219
createValidValidator := func() *validationfakes.FakeGenericValidator {
220220
v := &validationfakes.FakeGenericValidator{}
221221
v.ValidateEscapedStringNoVarExpansionReturns(nil)
222+
v.ValidateEndpointReturns(nil)
223+
v.ValidateServiceNameReturns(nil)
224+
v.ValidateNginxDurationReturns(nil)
222225

223226
return v
224227
}
225228

226229
createInvalidValidator := func() *validationfakes.FakeGenericValidator {
227230
v := &validationfakes.FakeGenericValidator{}
228231
v.ValidateEscapedStringNoVarExpansionReturns(errors.New("error"))
232+
v.ValidateEndpointReturns(errors.New("error"))
233+
v.ValidateServiceNameReturns(errors.New("error"))
234+
v.ValidateNginxDurationReturns(errors.New("error"))
229235

230236
return v
231237
}
@@ -244,6 +250,13 @@ func TestValidateNginxProxy(t *testing.T) {
244250
Spec: ngfAPI.NginxProxySpec{
245251
Telemetry: &ngfAPI.Telemetry{
246252
ServiceName: helpers.GetPointer("my-svc"),
253+
Exporter: &ngfAPI.TelemetryExporter{
254+
Interval: helpers.GetPointer[ngfAPI.Duration]("5ms"),
255+
Endpoint: "my-endpoint",
256+
},
257+
SpanAttributes: []ngfAPI.SpanAttribute{
258+
{Key: "key", Value: "value"},
259+
},
247260
},
248261
},
249262
},

0 commit comments

Comments
 (0)