fix: enforce connection limit value (#4458)

* fix: enforce connection limit value Signed-off-by: Guy Daich <guy.daich@sap.com> * rm omitempty Signed-off-by: Guy Daich <guy.daich@sap.com> * fix cel Signed-off-by: Guy Daich <guy.daich@sap.com> * remove validation Signed-off-by: Guy Daich <guy.daich@sap.com> --------- Signed-off-by: Guy Daich <guy.daich@sap.com> Co-authored-by: zirain <zirain2009@gmail.com>
envoyproxy · Oct 22, 2024 · 31ff8ff · 31ff8ff
1 parent 04fc944
commit 31ff8ff
Show file tree

Hide file tree

Showing 5 changed files with 53 additions and 7 deletions.
diff --git 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
@@ -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
@@ -782,7 +782,7 @@ _Appears in:_
 
 | Field | Type | Required | Description |
 | ---   | ---  | ---      | ---         |
-| `value` | _integer_ |  true  | Value of the maximum concurrent connections limit.<br />When the limit is reached, incoming connections will be closed after the CloseDelay duration.<br />Default: unlimited. |
+| `value` | _integer_ |  true  | Value of the maximum concurrent connections limit.<br />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<br />once the limit value is reached.<br />Default: none. |
 
 

diff --git 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.<br />When the limit is reached, incoming connections will be closed after the CloseDelay duration.<br />Default: unlimited. |
+| `value` | _integer_ |  true  | Value of the maximum concurrent connections limit.<br />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<br />once the limit value is reached.<br />Default: none. |
 
 

diff --git 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])?$', <nil>: 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) {