More specific validators

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

@@ -84,12 +84,3 @@ func validateHeaderName(name string) error {
 	}
 	return nil
 }
-
-// GenericValidator validates values for generic cases in the nginx conf.
-type GenericValidator struct{}
-
-// ValidateEscapedStringNoVarExpansion ensures that no invalid characters are included in the string value that
-// could lead to unwanted nginx behavior.
-func (GenericValidator) ValidateEscapedStringNoVarExpansion(value string) error {
-	return validateEscapedStringNoVarExpansion(value, nil)
-}

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

@@ -71,24 +71,3 @@ func TestValidateValidHeaderName(t *testing.T) {
 		strings.Repeat("very-long-header", 16)+"1",
 	)
 }
-
-func TestGenericValidator_ValidateEscapedStringNoVarExpansion(t *testing.T) {
-	validator := GenericValidator{}
-
-	testValidValuesForSimpleValidator(
-		t,
-		validator.ValidateEscapedStringNoVarExpansion,
-		`test`,
-		`test test`,
-		`\"`,
-		`\\`,
-	)
-
-	testInvalidValuesForSimpleValidator(
-		t,
-		validator.ValidateEscapedStringNoVarExpansion,
-		`\`,
-		`test"test`,
-		`$test`,
-	)
-}

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

@@ -0,0 +1,83 @@
+package validation
+
+import (
+	"errors"
+	"regexp"
+
+	k8svalidation "k8s.io/apimachinery/pkg/util/validation"
+)
+
+// GenericValidator validates values for generic cases in the nginx conf.
+type GenericValidator struct{}
+
+// ValidateEscapedStringNoVarExpansion ensures that no invalid characters are included in the string value that
+// could lead to unwanted nginx behavior.
+func (GenericValidator) ValidateEscapedStringNoVarExpansion(value string) error {
+	return validateEscapedStringNoVarExpansion(value, nil)
+}
+
+const (
+	alphaNumericStringFmt    = `[a-zA-Z0-9_-]+`
+	alphaNumericStringErrMsg = "must contain only alphanumeric characters or '-' or '_'"
+)
+
+var alphaNumericStringFmtRegexp = regexp.MustCompile("^" + alphaNumericStringFmt + "$")
+
+// ValidateServiceName validates a k8s service name that can only use alphanumeric characters.
+func (GenericValidator) ValidateServiceName(name string) error {
+	if !alphaNumericStringFmtRegexp.MatchString(name) {
+		examples := []string{
+			"svc1",
+			"svc-1",
+			"svc_1",
+		}
+
+		return errors.New(k8svalidation.RegexError(alphaNumericStringErrMsg, alphaNumericStringFmt, examples...))
+	}
+
+	return nil
+}
+
+const (
+	durationStringFmt    = `\d{1,4}(ms|s)?`
+	durationStringErrMsg = "must contain a number followed by 'ms' or 's'"
+)
+
+var durationStringFmtRegexp = regexp.MustCompile("^" + durationStringFmt + "$")
+
+// ValidateNginxDuration validates a duration string that nginx can understand.
+func (GenericValidator) ValidateNginxDuration(duration string) error {
+	if !durationStringFmtRegexp.MatchString(duration) {
+		examples := []string{
+			"5ms",
+			"10s",
+		}
+
+		return errors.New(k8svalidation.RegexError(durationStringFmt, durationStringErrMsg, examples...))
+	}
+
+	return nil
+}
+
+const (
+	//nolint:lll
+	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})?`
+	endpointStringErrMsg = "must be an alphanumeric hostname with optional http scheme and optional port"
+)
+
+var endpointStringFmtRegexp = regexp.MustCompile("^" + endpointStringFmt + "$")
+
+// ValidateEndpoint validates an alphanumeric endpoint, with optional http scheme and port.
+func (GenericValidator) ValidateEndpoint(endpoint string) error {
+	if !endpointStringFmtRegexp.MatchString(endpoint) {
+		examples := []string{
+			"my-endpoint",
+			"my.endpoint:5678",
+			"http://my-endpoint",
+		}
+
+		return errors.New(k8svalidation.RegexError(endpointStringFmt, endpointStringErrMsg, examples...))
+	}
+
+	return nil
+}

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

@@ -0,0 +1,86 @@
+package validation
+
+import "testing"
+
+func TestGenericValidator_ValidateEscapedStringNoVarExpansion(t *testing.T) {
+	validator := GenericValidator{}
+
+	testValidValuesForSimpleValidator(
+		t,
+		validator.ValidateEscapedStringNoVarExpansion,
+		`test`,
+		`test test`,
+		`\"`,
+		`\\`,
+	)
+
+	testInvalidValuesForSimpleValidator(
+		t,
+		validator.ValidateEscapedStringNoVarExpansion,
+		`\`,
+		`test"test`,
+		`$test`,
+	)
+}
+
+func TestValidateServiceName(t *testing.T) {
+	validator := GenericValidator{}
+
+	testValidValuesForSimpleValidator(
+		t,
+		validator.ValidateServiceName,
+		`test`,
+		`Test-test`,
+		`test_Test`,
+		`test123`,
+	)
+
+	testInvalidValuesForSimpleValidator(
+		t,
+		validator.ValidateServiceName,
+		`test#$%`,
+		`test test`,
+		`test.test`,
+	)
+}
+
+func TestValidateNginxDuration(t *testing.T) {
+	validator := GenericValidator{}
+
+	testValidValuesForSimpleValidator(
+		t,
+		validator.ValidateNginxDuration,
+		`5ms`,
+		`10s`,
+		`123ms`,
+	)
+
+	testInvalidValuesForSimpleValidator(
+		t,
+		validator.ValidateNginxDuration,
+		`test`,
+		`12345`,
+		`5m`,
+	)
+}
+
+func TestValidateEndpoint(t *testing.T) {
+	validator := GenericValidator{}
+
+	testValidValuesForSimpleValidator(
+		t,
+		validator.ValidateEndpoint,
+		`http://my-endpoint:5678`,
+		`my.endpoint`,
+		`myendpoint:123`,
+		`my-endpoint123:456`,
+	)
+
+	testInvalidValuesForSimpleValidator(
+		t,
+		validator.ValidateEndpoint,
+		`https://my-endpoint`,
+		`my_endpoint`,
+		`my$endpoint`,
+	)
+}

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

@@ -44,25 +44,12 @@ func validateNginxProxy(
 	var allErrs field.ErrorList
 	spec := field.NewPath("spec")
 
-	validateStringValue := func(
-		value,
-		valueName string,
-		path *field.Path,
-		validator validation.GenericValidator,
-	) *field.Error {
-		if err := validator.ValidateEscapedStringNoVarExpansion(value); err != nil {
-			return field.Invalid(path.Child(valueName), value, err.Error())
-		}
-
-		return nil
-	}
-
 	telemetry := npCfg.Spec.Telemetry
 	if telemetry != nil {
 		telPath := spec.Child("telemetry")
 		if telemetry.ServiceName != nil {
-			if err := validateStringValue(*telemetry.ServiceName, "serviceName", telPath, validator); err != nil {
-				allErrs = append(allErrs, err)
+			if err := validator.ValidateEscapedStringNoVarExpansion(*telemetry.ServiceName); err != nil {
+				allErrs = append(allErrs, field.Invalid(telPath.Child("serviceName"), *telemetry.ServiceName, err.Error()))
 			}
 		}
 
@@ -71,27 +58,27 @@ func validateNginxProxy(
 			expPath := telPath.Child("exporter")
 
 			if exp.Endpoint != "" {
-				if err := validateStringValue(exp.Endpoint, "endpoint", expPath, validator); err != nil {
-					allErrs = append(allErrs, err)
+				if err := validator.ValidateEscapedStringNoVarExpansion(exp.Endpoint); err != nil {
+					allErrs = append(allErrs, field.Invalid(expPath.Child("endpoint"), exp.Endpoint, err.Error()))
 				}
 			}
 
 			if exp.Interval != nil {
-				if err := validateStringValue(string(*exp.Interval), "interval", expPath, validator); err != nil {
-					allErrs = append(allErrs, err)
+				if err := validator.ValidateEscapedStringNoVarExpansion(string(*exp.Interval)); err != nil {
+					allErrs = append(allErrs, field.Invalid(expPath.Child("interval"), *exp.Interval, err.Error()))
 				}
 			}
 		}
 
 		if telemetry.SpanAttributes != nil {
 			spanAttrPath := telPath.Child("spanAttributes")
 			for _, spanAttr := range telemetry.SpanAttributes {
-				if err := validateStringValue(spanAttr.Key, "key", spanAttrPath, validator); err != nil {
-					allErrs = append(allErrs, err)
+				if err := validator.ValidateEscapedStringNoVarExpansion(spanAttr.Key); err != nil {
+					allErrs = append(allErrs, field.Invalid(spanAttrPath.Child("key"), spanAttr.Key, err.Error()))
 				}
 
-				if err := validateStringValue(spanAttr.Value, "value", spanAttrPath, validator); err != nil {
-					allErrs = append(allErrs, err)
+				if err := validator.ValidateEscapedStringNoVarExpansion(spanAttr.Value); err != nil {
+					allErrs = append(allErrs, field.Invalid(spanAttrPath.Child("value"), spanAttr.Value, err.Error()))
 				}
 			}
 		}

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

@@ -219,13 +219,19 @@ func TestValidateNginxProxy(t *testing.T) {
 	createValidValidator := func() *validationfakes.FakeGenericValidator {
 		v := &validationfakes.FakeGenericValidator{}
 		v.ValidateEscapedStringNoVarExpansionReturns(nil)
+		v.ValidateEndpointReturns(nil)
+		v.ValidateServiceNameReturns(nil)
+		v.ValidateNginxDurationReturns(nil)
 
 		return v
 	}
 
 	createInvalidValidator := func() *validationfakes.FakeGenericValidator {
 		v := &validationfakes.FakeGenericValidator{}
 		v.ValidateEscapedStringNoVarExpansionReturns(errors.New("error"))
+		v.ValidateEndpointReturns(errors.New("error"))
+		v.ValidateServiceNameReturns(errors.New("error"))
+		v.ValidateNginxDurationReturns(errors.New("error"))
 
 		return v
 	}
@@ -244,6 +250,13 @@ func TestValidateNginxProxy(t *testing.T) {
 				Spec: ngfAPI.NginxProxySpec{
 					Telemetry: &ngfAPI.Telemetry{
 						ServiceName: helpers.GetPointer("my-svc"),
+						Exporter: &ngfAPI.TelemetryExporter{
+							Interval: helpers.GetPointer[ngfAPI.Duration]("5ms"),
+							Endpoint: "my-endpoint",
+						},
+						SpanAttributes: []ngfAPI.SpanAttribute{
+							{Key: "key", Value: "value"},
+						},
 					},
 				},
 			},