From 27ea85be4e84fb679e2480d44c34070ba1879c3e Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Tue, 10 Sep 2024 15:33:40 -0700 Subject: [PATCH 1/4] Add validation change to duration in crd and ngf --- apis/v1alpha1/shared_types.go | 7 ++++--- .../gateway.nginx.org_clientsettingspolicies.yaml | 8 ++++---- config/crd/bases/gateway.nginx.org_nginxproxies.yaml | 2 +- deploy/crds.yaml | 10 +++++----- .../mode/static/nginx/config/validation/generic.go | 6 ++++-- site/content/reference/api.md | 4 ++-- 6 files changed, 20 insertions(+), 17 deletions(-) diff --git a/apis/v1alpha1/shared_types.go b/apis/v1alpha1/shared_types.go index d56ff53593..7d206c769f 100644 --- a/apis/v1alpha1/shared_types.go +++ b/apis/v1alpha1/shared_types.go @@ -1,10 +1,11 @@ package v1alpha1 // Duration is a string value representing a duration in time. -// Duration can be specified in milliseconds (ms) or seconds (s) A value without a suffix is seconds. -// Examples: 120s, 50ms. +// Duration can be specified in milliseconds (ms), seconds (s), minutes (m), hours (h). +// A value without a suffix is seconds. +// Examples: 120s, 50ms, 5m, 1h. // -// +kubebuilder:validation:Pattern=`^\d{1,4}(ms|s)?$` +// +kubebuilder:validation:Pattern=`^[0-9]{1,4}(ms|s|m|h)?$` type Duration string // SpanAttribute is a key value pair to be added to a tracing span. diff --git a/config/crd/bases/gateway.nginx.org_clientsettingspolicies.yaml b/config/crd/bases/gateway.nginx.org_clientsettingspolicies.yaml index f2b3bee672..651dc95d56 100644 --- a/config/crd/bases/gateway.nginx.org_clientsettingspolicies.yaml +++ b/config/crd/bases/gateway.nginx.org_clientsettingspolicies.yaml @@ -70,7 +70,7 @@ spec: If a client does not transmit anything within this time, the request is terminated with the 408 (Request Time-out) error. Default: https://nginx.org/en/docs/http/ngx_http_core_module.html#client_body_timeout. - pattern: ^\d{1,4}(ms|s)?$ + pattern: ^[0-9]{1,4}(ms|s|m|h)?$ type: string type: object keepAlive: @@ -91,7 +91,7 @@ spec: Time defines the maximum time during which requests can be processed through one keep-alive connection. After this time is reached, the connection is closed following the subsequent request processing. Default: https://nginx.org/en/docs/http/ngx_http_core_module.html#keepalive_time. - pattern: ^\d{1,4}(ms|s)?$ + pattern: ^[0-9]{1,4}(ms|s|m|h)?$ type: string timeout: description: Timeout defines the keep-alive timeouts for clients. @@ -99,13 +99,13 @@ spec: header: description: 'Header sets the timeout in the "Keep-Alive: timeout=time" response header field.' - pattern: ^\d{1,4}(ms|s)?$ + pattern: ^[0-9]{1,4}(ms|s|m|h)?$ type: string server: description: |- Server sets the timeout during which a keep-alive client connection will stay open on the server side. Setting this value to 0 disables keep-alive client connections. - pattern: ^\d{1,4}(ms|s)?$ + pattern: ^[0-9]{1,4}(ms|s|m|h)?$ type: string type: object x-kubernetes-validations: diff --git a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml index 19ed93c64b..84172c38cd 100644 --- a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml +++ b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml @@ -156,7 +156,7 @@ spec: description: |- Interval is the maximum interval between two exports. Default: https://nginx.org/en/docs/ngx_otel_module.html#otel_exporter - pattern: ^\d{1,4}(ms|s)?$ + pattern: ^[0-9]{1,4}(ms|s|m|h)?$ type: string required: - endpoint diff --git a/deploy/crds.yaml b/deploy/crds.yaml index ef8ffea772..7ff21fb0b0 100644 --- a/deploy/crds.yaml +++ b/deploy/crds.yaml @@ -69,7 +69,7 @@ spec: If a client does not transmit anything within this time, the request is terminated with the 408 (Request Time-out) error. Default: https://nginx.org/en/docs/http/ngx_http_core_module.html#client_body_timeout. - pattern: ^\d{1,4}(ms|s)?$ + pattern: ^[0-9]{1,4}(ms|s|m|h)?$ type: string type: object keepAlive: @@ -90,7 +90,7 @@ spec: Time defines the maximum time during which requests can be processed through one keep-alive connection. After this time is reached, the connection is closed following the subsequent request processing. Default: https://nginx.org/en/docs/http/ngx_http_core_module.html#keepalive_time. - pattern: ^\d{1,4}(ms|s)?$ + pattern: ^[0-9]{1,4}(ms|s|m|h)?$ type: string timeout: description: Timeout defines the keep-alive timeouts for clients. @@ -98,13 +98,13 @@ spec: header: description: 'Header sets the timeout in the "Keep-Alive: timeout=time" response header field.' - pattern: ^\d{1,4}(ms|s)?$ + pattern: ^[0-9]{1,4}(ms|s|m|h)?$ type: string server: description: |- Server sets the timeout during which a keep-alive client connection will stay open on the server side. Setting this value to 0 disables keep-alive client connections. - pattern: ^\d{1,4}(ms|s)?$ + pattern: ^[0-9]{1,4}(ms|s|m|h)?$ type: string type: object x-kubernetes-validations: @@ -741,7 +741,7 @@ spec: description: |- Interval is the maximum interval between two exports. Default: https://nginx.org/en/docs/ngx_otel_module.html#otel_exporter - pattern: ^\d{1,4}(ms|s)?$ + pattern: ^[0-9]{1,4}(ms|s|m|h)?$ type: string required: - endpoint diff --git a/internal/mode/static/nginx/config/validation/generic.go b/internal/mode/static/nginx/config/validation/generic.go index 73c6f01044..2a263fe57f 100644 --- a/internal/mode/static/nginx/config/validation/generic.go +++ b/internal/mode/static/nginx/config/validation/generic.go @@ -39,8 +39,8 @@ func (GenericValidator) ValidateServiceName(name string) error { } const ( - durationStringFmt = `\d{1,4}(ms|s)?` - durationStringErrMsg = "must contain a number followed by 'ms' or 's'" + durationStringFmt = `^[0-9]{1,4}(ms|s|m|h)?` + durationStringErrMsg = "must contain a four digit number followed by 'ms', 's', 'm', or 'h'" ) var durationStringFmtRegexp = regexp.MustCompile("^" + durationStringFmt + "$") @@ -51,6 +51,8 @@ func (GenericValidator) ValidateNginxDuration(duration string) error { examples := []string{ "5ms", "10s", + "500m", + "1000h", } return errors.New(k8svalidation.RegexError(durationStringFmt, durationStringErrMsg, examples...)) diff --git a/site/content/reference/api.md b/site/content/reference/api.md index d5b191193b..f2145c79e7 100644 --- a/site/content/reference/api.md +++ b/site/content/reference/api.md @@ -812,8 +812,8 @@ Support: Gateway, HTTPRoute, GRPCRoute.

Duration is a string value representing a duration in time. -Duration can be specified in milliseconds (ms) or seconds (s) A value without a suffix is seconds. -Examples: 120s, 50ms.

+Duration can be specified in milliseconds (ms), seconds (s), minutes (m), hours (h) A value without a suffix is seconds. +Examples: 120s, 50ms, 5m, 1h.

IPFamilyType (string alias)

ΒΆ From 375d40c53167ccef1948b76889ba7952a01a682b Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Tue, 10 Sep 2024 16:01:58 -0700 Subject: [PATCH 2/4] Adjust wording on error message --- internal/mode/static/nginx/config/validation/generic.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/mode/static/nginx/config/validation/generic.go b/internal/mode/static/nginx/config/validation/generic.go index 2a263fe57f..8342ab4134 100644 --- a/internal/mode/static/nginx/config/validation/generic.go +++ b/internal/mode/static/nginx/config/validation/generic.go @@ -40,7 +40,7 @@ func (GenericValidator) ValidateServiceName(name string) error { const ( durationStringFmt = `^[0-9]{1,4}(ms|s|m|h)?` - durationStringErrMsg = "must contain a four digit number followed by 'ms', 's', 'm', or 'h'" + durationStringErrMsg = "must contain an, at most, four digit number followed by 'ms', 's', 'm', or 'h'" ) var durationStringFmtRegexp = regexp.MustCompile("^" + durationStringFmt + "$") From 9713d805d7c5a0ac571b85c719e08699ca61172d Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Tue, 10 Sep 2024 16:10:09 -0700 Subject: [PATCH 3/4] Update generated files --- site/content/reference/api.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/site/content/reference/api.md b/site/content/reference/api.md index f2145c79e7..bc4062b175 100644 --- a/site/content/reference/api.md +++ b/site/content/reference/api.md @@ -812,7 +812,8 @@ Support: Gateway, HTTPRoute, GRPCRoute.

Duration is a string value representing a duration in time. -Duration can be specified in milliseconds (ms), seconds (s), minutes (m), hours (h) A value without a suffix is seconds. +Duration can be specified in milliseconds (ms), seconds (s), minutes (m), hours (h). +A value without a suffix is seconds. Examples: 120s, 50ms, 5m, 1h.

IPFamilyType From 54d0e298c6ef793eb21e903603a6ffe7facb41b4 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Tue, 10 Sep 2024 16:58:45 -0700 Subject: [PATCH 4/4] Update validation tests --- .../policies/clientsettings/validator_test.go | 20 ++++++++++++------- .../nginx/config/validation/generic_test.go | 4 +++- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/internal/mode/static/nginx/config/policies/clientsettings/validator_test.go b/internal/mode/static/nginx/config/policies/clientsettings/validator_test.go index 1be2eff100..2d1f95686d 100644 --- a/internal/mode/static/nginx/config/policies/clientsettings/validator_test.go +++ b/internal/mode/static/nginx/config/policies/clientsettings/validator_test.go @@ -103,13 +103,19 @@ func TestValidator_Validate(t *testing.T) { return p }), expConditions: []conditions.Condition{ - staticConds.NewPolicyInvalid("[spec.body.timeout: Invalid value: \"invalid\": \\d{1,4}(ms|s)? " + - "(e.g. '5ms', or '10s', regex used for validation is 'must contain a number followed by 'ms' or 's''), " + - "spec.keepAlive.time: Invalid value: \"invalid\": \\d{1,4}(ms|s)? (e.g. '5ms', or '10s', regex used for " + - "validation is 'must contain a number followed by 'ms' or 's''), spec.keepAlive.timeout.server: Invalid value: " + - "\"invalid\": \\d{1,4}(ms|s)? (e.g. '5ms', or '10s', regex used for validation is 'must contain a number " + - "followed by 'ms' or 's''), spec.keepAlive.timeout.header: Invalid value: \"invalid\": \\d{1,4}(ms|s)? " + - "(e.g. '5ms', or '10s', regex used for validation is 'must contain a number followed by 'ms' or 's'')]"), + staticConds.NewPolicyInvalid( + "[spec.body.timeout: Invalid value: \"invalid\": ^[0-9]{1,4}(ms|s|m|h)? " + + "(e.g. '5ms', or '10s', or '500m', or '1000h', regex used for validation is " + + "'must contain an, at most, four digit number followed by 'ms', 's', 'm', or 'h''), " + + "spec.keepAlive.time: Invalid value: \"invalid\": ^[0-9]{1,4}(ms|s|m|h)? " + + "(e.g. '5ms', or '10s', or '500m', or '1000h', regex used for validation is " + + "'must contain an, at most, four digit number followed by 'ms', 's', 'm', or 'h''), " + + "spec.keepAlive.timeout.server: Invalid value: \"invalid\": ^[0-9]{1,4}(ms|s|m|h)? " + + "(e.g. '5ms', or '10s', or '500m', or '1000h', regex used for validation is " + + "'must contain an, at most, four digit number followed by 'ms', 's', 'm', or 'h''), " + + "spec.keepAlive.timeout.header: Invalid value: \"invalid\": ^[0-9]{1,4}(ms|s|m|h)? " + + "(e.g. '5ms', or '10s', or '500m', or '1000h', regex used for validation is " + + "'must contain an, at most, four digit number followed by 'ms', 's', 'm', or 'h'')]"), }, }, { diff --git a/internal/mode/static/nginx/config/validation/generic_test.go b/internal/mode/static/nginx/config/validation/generic_test.go index 6934372c0f..5f57b51c56 100644 --- a/internal/mode/static/nginx/config/validation/generic_test.go +++ b/internal/mode/static/nginx/config/validation/generic_test.go @@ -56,6 +56,8 @@ func TestValidateNginxDuration(t *testing.T) { `5ms`, `10s`, `123ms`, + `5m`, + `2h`, ) testInvalidValuesForSimpleValidator( @@ -63,7 +65,7 @@ func TestValidateNginxDuration(t *testing.T) { validator.ValidateNginxDuration, `test`, `12345`, - `5m`, + `5k`, ) }