From 31ff8ffd1dfefa9c0acd7d67a6aee76e4d30abbf Mon Sep 17 00:00:00 2001 From: Guy Daich Date: Tue, 22 Oct 2024 05:32:16 -0500 Subject: [PATCH] fix: enforce connection limit value (#4458) * fix: enforce connection limit value Signed-off-by: Guy Daich * rm omitempty Signed-off-by: Guy Daich * fix cel Signed-off-by: Guy Daich * remove validation Signed-off-by: Guy Daich --------- Signed-off-by: Guy Daich Co-authored-by: zirain --- api/v1alpha1/connection_types.go | 5 +- ...y.envoyproxy.io_clienttrafficpolicies.yaml | 5 +- site/content/en/latest/api/extension_types.md | 2 +- site/content/zh/latest/api/extension_types.md | 2 +- .../clienttrafficpolicy_test.go | 46 +++++++++++++++++++ 5 files changed, 53 insertions(+), 7 deletions(-) diff --git a/api/v1alpha1/connection_types.go b/api/v1alpha1/connection_types.go index 6f27794748b..efb24dc3bb1 100644 --- a/api/v1alpha1/connection_types.go +++ b/api/v1alpha1/connection_types.go @@ -66,10 +66,9 @@ type BackendConnection struct { type ConnectionLimit struct { // Value of the maximum concurrent connections limit. // When the limit is reached, incoming connections will be closed after the CloseDelay duration. - // Default: unlimited. // - // +kubebuilder:validation:Minimum=0 - Value int64 `json:"value,omitempty"` + // +kubebuilder:validation:Minimum=1 + Value int64 `json:"value"` // CloseDelay defines the delay to use before closing connections that are rejected // once the limit value is reached. diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_clienttrafficpolicies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_clienttrafficpolicies.yaml index 582486e706f..3e626f3f88a 100644 --- a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_clienttrafficpolicies.yaml +++ b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_clienttrafficpolicies.yaml @@ -125,10 +125,11 @@ spec: description: |- Value of the maximum concurrent connections limit. When the limit is reached, incoming connections will be closed after the CloseDelay duration. - Default: unlimited. format: int64 - minimum: 0 + minimum: 1 type: integer + required: + - value type: object socketBufferLimit: allOf: diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index 4f562fce611..c183a4f0b8f 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -782,7 +782,7 @@ _Appears in:_ | Field | Type | Required | Description | | --- | --- | --- | --- | -| `value` | _integer_ | true | Value of the maximum concurrent connections limit.
When the limit is reached, incoming connections will be closed after the CloseDelay duration.
Default: unlimited. | +| `value` | _integer_ | true | Value of the maximum concurrent connections limit.
When the limit is reached, incoming connections will be closed after the CloseDelay duration. | | `closeDelay` | _[Duration](https://gateway-api.sigs.k8s.io/reference/spec/#gateway.networking.k8s.io/v1.Duration)_ | false | CloseDelay defines the delay to use before closing connections that are rejected
once the limit value is reached.
Default: none. | diff --git a/site/content/zh/latest/api/extension_types.md b/site/content/zh/latest/api/extension_types.md index 4f562fce611..c183a4f0b8f 100644 --- a/site/content/zh/latest/api/extension_types.md +++ b/site/content/zh/latest/api/extension_types.md @@ -782,7 +782,7 @@ _Appears in:_ | Field | Type | Required | Description | | --- | --- | --- | --- | -| `value` | _integer_ | true | Value of the maximum concurrent connections limit.
When the limit is reached, incoming connections will be closed after the CloseDelay duration.
Default: unlimited. | +| `value` | _integer_ | true | Value of the maximum concurrent connections limit.
When the limit is reached, incoming connections will be closed after the CloseDelay duration. | | `closeDelay` | _[Duration](https://gateway-api.sigs.k8s.io/reference/spec/#gateway.networking.k8s.io/v1.Duration)_ | false | CloseDelay defines the delay to use before closing connections that are rejected
once the limit value is reached.
Default: none. | diff --git a/test/cel-validation/clienttrafficpolicy_test.go b/test/cel-validation/clienttrafficpolicy_test.go index 14dd915d78a..3558d1848f9 100644 --- a/test/cel-validation/clienttrafficpolicy_test.go +++ b/test/cel-validation/clienttrafficpolicy_test.go @@ -314,6 +314,52 @@ func TestClientTrafficPolicyTarget(t *testing.T) { "spec.connection.bufferLimit: Invalid value: \"15m\": spec.connection.bufferLimit in body should match '^[1-9]+[0-9]*([EPTGMK]i|[EPTGMk])?$', : Invalid value: \"\"", }, }, + { + desc: "invalid Connection Limit Empty", + mutate: func(ctp *egv1a1.ClientTrafficPolicy) { + ctp.Spec = egv1a1.ClientTrafficPolicySpec{ + PolicyTargetReferences: egv1a1.PolicyTargetReferences{ + TargetRef: &gwapiv1a2.LocalPolicyTargetReferenceWithSectionName{ + LocalPolicyTargetReference: gwapiv1a2.LocalPolicyTargetReference{ + Group: gwapiv1a2.Group("gateway.networking.k8s.io"), + Kind: gwapiv1a2.Kind("Gateway"), + Name: gwapiv1a2.ObjectName("eg"), + }, + }, + }, + Connection: &egv1a1.ClientConnection{ + ConnectionLimit: &egv1a1.ConnectionLimit{}, + }, + } + }, + wantErrors: []string{ + "spec.connection.connectionLimit.value: Invalid value: 0: spec.connection.connectionLimit.value in body should be greater than or equal to 1", + }, + }, + { + desc: "invalid Connection Limit < 1", + mutate: func(ctp *egv1a1.ClientTrafficPolicy) { + ctp.Spec = egv1a1.ClientTrafficPolicySpec{ + PolicyTargetReferences: egv1a1.PolicyTargetReferences{ + TargetRef: &gwapiv1a2.LocalPolicyTargetReferenceWithSectionName{ + LocalPolicyTargetReference: gwapiv1a2.LocalPolicyTargetReference{ + Group: gwapiv1a2.Group("gateway.networking.k8s.io"), + Kind: gwapiv1a2.Kind("Gateway"), + Name: gwapiv1a2.ObjectName("eg"), + }, + }, + }, + Connection: &egv1a1.ClientConnection{ + ConnectionLimit: &egv1a1.ConnectionLimit{ + Value: -1, // Value: 0 is covered by existence test, as 0 is the nil value. + }, + }, + } + }, + wantErrors: []string{ + "spec.connection.connectionLimit.value: Invalid value: -1: spec.connection.connectionLimit.value in body should be greater than or equal to 1", + }, + }, { desc: "invalid InitialStreamWindowSize format", mutate: func(ctp *egv1a1.ClientTrafficPolicy) {