Skip to content

Commit

Permalink
apis: add kubebuilder regex validation for timeout strings (#2913)
Browse files Browse the repository at this point in the history
Adds kubebuilder regex validations for all timeout string API
fields.

Closes #2728

Signed-off-by: Steve Kriss <[email protected]>
  • Loading branch information
skriss authored Sep 17, 2020
1 parent 5c549bb commit 28bed07
Show file tree
Hide file tree
Showing 5 changed files with 102 additions and 0 deletions.
5 changes: 5 additions & 0 deletions apis/projectcontour/v1/httpproxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ type AuthorizationServer struct {
// The string "infinity" is also a valid input and specifies no timeout.
//
// +optional
// +kubebuilder:validation:Pattern=`^(((\d*(\.\d*)?h)|(\d*(\.\d*)?m)|(\d*(\.\d*)?s)|(\d*(\.\d*)?ms)|(\d*(\.\d*)?us)|(\d*(\.\d*)?µs)|(\d*(\.\d*)?ns))+|infinity|infinite)$`
ResponseTimeout string `json:"responseTimeout,omitempty"`

// If FailOpen is true, the client request is forwarded to the upstream service
Expand Down Expand Up @@ -411,13 +412,15 @@ type TimeoutPolicy struct {
// Timeout for receiving a response from the server after processing a request from client.
// If not supplied, Envoy's default value of 15s applies.
// +optional
// +kubebuilder:validation:Pattern=`^(((\d*(\.\d*)?h)|(\d*(\.\d*)?m)|(\d*(\.\d*)?s)|(\d*(\.\d*)?ms)|(\d*(\.\d*)?us)|(\d*(\.\d*)?µs)|(\d*(\.\d*)?ns))+|infinity|infinite)$`
Response string `json:"response,omitempty"`

// Timeout after which, if there are no active requests for this route, the connection between
// Envoy and the backend or Envoy and the external client will be closed.
// If not specified, there is no per-route idle timeout, though a connection manager-wide
// stream_idle_timeout default of 5m still applies.
// +optional
// +kubebuilder:validation:Pattern=`^(((\d*(\.\d*)?h)|(\d*(\.\d*)?m)|(\d*(\.\d*)?s)|(\d*(\.\d*)?ms)|(\d*(\.\d*)?us)|(\d*(\.\d*)?µs)|(\d*(\.\d*)?ns))+|infinity|infinite)$`
Idle string `json:"idle,omitempty"`
}

Expand All @@ -434,6 +437,8 @@ type RetryPolicy struct {
NumRetries int64 `json:"count"`
// PerTryTimeout specifies the timeout per retry attempt.
// Ignored if NumRetries is not supplied.
// +optional
// +kubebuilder:validation:Pattern=`^(((\d*(\.\d*)?h)|(\d*(\.\d*)?m)|(\d*(\.\d*)?s)|(\d*(\.\d*)?ms)|(\d*(\.\d*)?us)|(\d*(\.\d*)?µs)|(\d*(\.\d*)?ns))+|infinity|infinite)$`
PerTryTimeout string `json:"perTryTimeout,omitempty"`
// RetryOn specifies the conditions on which to retry a request.
//
Expand Down
6 changes: 6 additions & 0 deletions examples/contour/01-crds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,11 @@ spec:
properties:
idle:
description: Timeout after which, if there are no active requests for this route, the connection between Envoy and the backend or Envoy and the external client will be closed. If not specified, there is no per-route idle timeout, though a connection manager-wide stream_idle_timeout default of 5m still applies.
pattern: ^(((\d*(\.\d*)?h)|(\d*(\.\d*)?m)|(\d*(\.\d*)?s)|(\d*(\.\d*)?ms)|(\d*(\.\d*)?us)|(\d*(\.\d*)?µs)|(\d*(\.\d*)?ns))+|infinity|infinite)$
type: string
response:
description: Timeout for receiving a response from the server after processing a request from client. If not supplied, Envoy's default value of 15s applies.
pattern: ^(((\d*(\.\d*)?h)|(\d*(\.\d*)?m)|(\d*(\.\d*)?s)|(\d*(\.\d*)?ms)|(\d*(\.\d*)?us)|(\d*(\.\d*)?µs)|(\d*(\.\d*)?ns))+|infinity|infinite)$
type: string
type: object
validation:
Expand Down Expand Up @@ -514,6 +516,7 @@ spec:
type: integer
perTryTimeout:
description: PerTryTimeout specifies the timeout per retry attempt. Ignored if NumRetries is not supplied.
pattern: ^(((\d*(\.\d*)?h)|(\d*(\.\d*)?m)|(\d*(\.\d*)?s)|(\d*(\.\d*)?ms)|(\d*(\.\d*)?us)|(\d*(\.\d*)?µs)|(\d*(\.\d*)?ns))+|infinity|infinite)$
type: string
retriableStatusCodes:
description: "RetriableStatusCodes specifies the HTTP status codes that should be retried. \n This field is only respected when you include `retriable-status-codes` in the `RetryOn` field."
Expand Down Expand Up @@ -649,9 +652,11 @@ spec:
properties:
idle:
description: Timeout after which, if there are no active requests for this route, the connection between Envoy and the backend or Envoy and the external client will be closed. If not specified, there is no per-route idle timeout, though a connection manager-wide stream_idle_timeout default of 5m still applies.
pattern: ^(((\d*(\.\d*)?h)|(\d*(\.\d*)?m)|(\d*(\.\d*)?s)|(\d*(\.\d*)?ms)|(\d*(\.\d*)?us)|(\d*(\.\d*)?µs)|(\d*(\.\d*)?ns))+|infinity|infinite)$
type: string
response:
description: Timeout for receiving a response from the server after processing a request from client. If not supplied, Envoy's default value of 15s applies.
pattern: ^(((\d*(\.\d*)?h)|(\d*(\.\d*)?m)|(\d*(\.\d*)?s)|(\d*(\.\d*)?ms)|(\d*(\.\d*)?us)|(\d*(\.\d*)?µs)|(\d*(\.\d*)?ns))+|infinity|infinite)$
type: string
type: object
required:
Expand Down Expand Up @@ -853,6 +858,7 @@ spec:
type: boolean
responseTimeout:
description: ResponseTimeout configures maximum time to wait for a check response from the authorization server. Timeout durations are expressed in the Go [Duration format](https://godoc.org/time#ParseDuration). Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h". The string "infinity" is also a valid input and specifies no timeout.
pattern: ^(((\d*(\.\d*)?h)|(\d*(\.\d*)?m)|(\d*(\.\d*)?s)|(\d*(\.\d*)?ms)|(\d*(\.\d*)?us)|(\d*(\.\d*)?µs)|(\d*(\.\d*)?ns))+|infinity|infinite)$
type: string
required:
- extensionRef
Expand Down
6 changes: 6 additions & 0 deletions examples/render/contour.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,11 @@ spec:
properties:
idle:
description: Timeout after which, if there are no active requests for this route, the connection between Envoy and the backend or Envoy and the external client will be closed. If not specified, there is no per-route idle timeout, though a connection manager-wide stream_idle_timeout default of 5m still applies.
pattern: ^(((\d*(\.\d*)?h)|(\d*(\.\d*)?m)|(\d*(\.\d*)?s)|(\d*(\.\d*)?ms)|(\d*(\.\d*)?us)|(\d*(\.\d*)?µs)|(\d*(\.\d*)?ns))+|infinity|infinite)$
type: string
response:
description: Timeout for receiving a response from the server after processing a request from client. If not supplied, Envoy's default value of 15s applies.
pattern: ^(((\d*(\.\d*)?h)|(\d*(\.\d*)?m)|(\d*(\.\d*)?s)|(\d*(\.\d*)?ms)|(\d*(\.\d*)?us)|(\d*(\.\d*)?µs)|(\d*(\.\d*)?ns))+|infinity|infinite)$
type: string
type: object
validation:
Expand Down Expand Up @@ -623,6 +625,7 @@ spec:
type: integer
perTryTimeout:
description: PerTryTimeout specifies the timeout per retry attempt. Ignored if NumRetries is not supplied.
pattern: ^(((\d*(\.\d*)?h)|(\d*(\.\d*)?m)|(\d*(\.\d*)?s)|(\d*(\.\d*)?ms)|(\d*(\.\d*)?us)|(\d*(\.\d*)?µs)|(\d*(\.\d*)?ns))+|infinity|infinite)$
type: string
retriableStatusCodes:
description: "RetriableStatusCodes specifies the HTTP status codes that should be retried. \n This field is only respected when you include `retriable-status-codes` in the `RetryOn` field."
Expand Down Expand Up @@ -758,9 +761,11 @@ spec:
properties:
idle:
description: Timeout after which, if there are no active requests for this route, the connection between Envoy and the backend or Envoy and the external client will be closed. If not specified, there is no per-route idle timeout, though a connection manager-wide stream_idle_timeout default of 5m still applies.
pattern: ^(((\d*(\.\d*)?h)|(\d*(\.\d*)?m)|(\d*(\.\d*)?s)|(\d*(\.\d*)?ms)|(\d*(\.\d*)?us)|(\d*(\.\d*)?µs)|(\d*(\.\d*)?ns))+|infinity|infinite)$
type: string
response:
description: Timeout for receiving a response from the server after processing a request from client. If not supplied, Envoy's default value of 15s applies.
pattern: ^(((\d*(\.\d*)?h)|(\d*(\.\d*)?m)|(\d*(\.\d*)?s)|(\d*(\.\d*)?ms)|(\d*(\.\d*)?us)|(\d*(\.\d*)?µs)|(\d*(\.\d*)?ns))+|infinity|infinite)$
type: string
type: object
required:
Expand Down Expand Up @@ -962,6 +967,7 @@ spec:
type: boolean
responseTimeout:
description: ResponseTimeout configures maximum time to wait for a check response from the authorization server. Timeout durations are expressed in the Go [Duration format](https://godoc.org/time#ParseDuration). Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h". The string "infinity" is also a valid input and specifies no timeout.
pattern: ^(((\d*(\.\d*)?h)|(\d*(\.\d*)?m)|(\d*(\.\d*)?s)|(\d*(\.\d*)?ms)|(\d*(\.\d*)?us)|(\d*(\.\d*)?µs)|(\d*(\.\d*)?ns))+|infinity|infinite)$
type: string
required:
- extensionRef
Expand Down
84 changes: 84 additions & 0 deletions internal/timeout/validation_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
// Copyright Project Contour Authors
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package timeout

import (
"regexp"
"testing"

"github.com/stretchr/testify/assert"
)

// TestKubebuilderValidation verifies that the regex used as a kubebuilder validation
// for timeout strings matches the behavior of Parse().
func TestKubebuilderValidation(t *testing.T) {
// keep in sync with kubebuilder annotations in apis/projectcontour/v1/httpproxy.go
regex := regexp.MustCompile(`^(((\d*(\.\d*)?h)|(\d*(\.\d*)?m)|(\d*(\.\d*)?s)|(\d*(\.\d*)?ms)|(\d*(\.\d*)?us)|(\d*(\.\d*)?µs)|(\d*(\.\d*)?ns))+|infinity|infinite)$`)

for tc, valid := range map[string]bool{
// valid duration strings across all allowed units
"1h": true,
"1.h": true,
"1.27h": true,

"1m": true,
"1.m": true,
"1.27m": true,

"1s": true,
"1.s": true,
"1.27s": true,

"1ms": true,
"1.ms": true,
"1.27ms": true,

"1us": true,
"1.us": true,
"1.27us": true,

"1µs": true,
"1.µs": true,
"1.27µs": true,

"1ns": true,
"1.ns": true,
"1.27ns": true,

// valid combinations of units
"1h2.34m1s": true,
"1s2.34h1m7.23s0.21us1.ns": true,

// invalid duration strings
"abc": false,
"1": false,
"9,25s": false,
"disabled": false,

// magic strings
"infinity": true,
"infinite": true,
} {
regexMatches := regex.MatchString(tc)
_, parseErr := Parse(tc)

if valid {
assert.True(t, regexMatches, "input string %q: regex should match but does not", tc)
assert.NoError(t, parseErr, "input string %q: timeout.Parse should succeed but does not", tc)
} else {
assert.False(t, regexMatches, "input string %q: regex should not match but does", tc)
assert.NotNil(t, parseErr, "input string %q: timeout.Parse should return an error but does not", tc)
}
}
}
1 change: 1 addition & 0 deletions site/docs/main/api-reference.html
Original file line number Diff line number Diff line change
Expand Up @@ -1501,6 +1501,7 @@ <h3 id="projectcontour.io/v1.RetryPolicy">RetryPolicy
</em>
</td>
<td>
<em>(Optional)</em>
<p>PerTryTimeout specifies the timeout per retry attempt.
Ignored if NumRetries is not supplied.</p>
</td>
Expand Down

0 comments on commit 28bed07

Please sign in to comment.