From 9a8ab4d559413b4d0609f826b970655af488bb27 Mon Sep 17 00:00:00 2001 From: Semir Patel Date: Thu, 10 Nov 2022 16:23:26 -0600 Subject: [PATCH] Inline IngressServiceConfig for DRY validatino --- .../api/v1alpha1/ingressgateway_types.go | 24 +-------- .../api/v1alpha1/ingressgateway_types_test.go | 50 +++++++++++-------- .../api/v1alpha1/zz_generated.deepcopy.go | 16 +----- 3 files changed, 33 insertions(+), 57 deletions(-) diff --git a/control-plane/api/v1alpha1/ingressgateway_types.go b/control-plane/api/v1alpha1/ingressgateway_types.go index 06e07b353b..c94b6e1458 100644 --- a/control-plane/api/v1alpha1/ingressgateway_types.go +++ b/control-plane/api/v1alpha1/ingressgateway_types.go @@ -162,17 +162,7 @@ type IngressService struct { RequestHeaders *HTTPHeaderModifiers `json:"requestHeaders,omitempty"` ResponseHeaders *HTTPHeaderModifiers `json:"responseHeaders,omitempty"` - // The maximum number of connections a service instance - // will be allowed to establish against the given upstream. Use this to limit - // HTTP/1.1 traffic, since HTTP/1.1 has a request per connection. - MaxConnections *uint32 `json:"maxConnections,omitempty"` - // The maximum number of requests that will be queued - // while waiting for a connection to be established. - MaxPendingRequests *uint32 `json:"maxPendingRequests,omitempty"` - // The maximum number of concurrent requests that - // will be allowed at a single point in time. Use this to limit HTTP/2 traffic, - // since HTTP/2 has many requests per connection. - MaxConcurrentRequests *uint32 `json:"maxConcurrentRequests,omitempty"` + IngressServiceConfig `json:",inline"` } func (in *IngressGateway) GetObjectMeta() metav1.ObjectMeta { @@ -442,17 +432,7 @@ func (in IngressListener) validate(path *field.Path, consulMeta common.ConsulMet "hosts must be empty if protocol is \"tcp\"")) } - if svc.MaxConnections != nil && *svc.MaxConnections <= 0 { - errs = append(errs, field.Invalid(path.Child("maxconnections"), *svc.MaxConnections, "MaxConnections must be > 0")) - } - - if svc.MaxConcurrentRequests != nil && *svc.MaxConcurrentRequests <= 0 { - errs = append(errs, field.Invalid(path.Child("maxconcurrentrequests"), *svc.MaxConcurrentRequests, "MaxConcurrentRequests must be > 0")) - } - - if svc.MaxPendingRequests != nil && *svc.MaxPendingRequests <= 0 { - errs = append(errs, field.Invalid(path.Child("maxpendingrequests"), *svc.MaxPendingRequests, "MaxPendingRequests must be > 0")) - } + errs = append(errs, svc.IngressServiceConfig.validate(path)...) } return errs } diff --git a/control-plane/api/v1alpha1/ingressgateway_types_test.go b/control-plane/api/v1alpha1/ingressgateway_types_test.go index 4f40bb9536..4942d38e11 100644 --- a/control-plane/api/v1alpha1/ingressgateway_types_test.go +++ b/control-plane/api/v1alpha1/ingressgateway_types_test.go @@ -84,13 +84,15 @@ func TestIngressGateway_MatchesConsul(t *testing.T) { }, Services: []IngressService{ { - Name: "name1", - Hosts: []string{"host1_1", "host1_2"}, - Namespace: "ns1", - Partition: "default", - MaxConnections: &maxConnections, - MaxPendingRequests: &maxPendingRequests, - MaxConcurrentRequests: &maxConcurrentRequests, + Name: "name1", + Hosts: []string{"host1_1", "host1_2"}, + Namespace: "ns1", + Partition: "default", + IngressServiceConfig: IngressServiceConfig{ + MaxConnections: &maxConnections, + MaxPendingRequests: &maxPendingRequests, + MaxConcurrentRequests: &maxConcurrentRequests, + }, TLS: &GatewayServiceTLSConfig{ SDS: &GatewayTLSSDSConfig{ ClusterName: "cluster1", @@ -344,13 +346,15 @@ func TestIngressGateway_ToConsul(t *testing.T) { }, Services: []IngressService{ { - Name: "name1", - Hosts: []string{"host1_1", "host1_2"}, - Namespace: "ns1", - Partition: "default", - MaxConnections: &maxConnections, - MaxPendingRequests: &maxPendingRequests, - MaxConcurrentRequests: &maxConcurrentRequests, + Name: "name1", + Hosts: []string{"host1_1", "host1_2"}, + Namespace: "ns1", + Partition: "default", + IngressServiceConfig: IngressServiceConfig{ + MaxConnections: &maxConnections, + MaxPendingRequests: &maxPendingRequests, + MaxConcurrentRequests: &maxConcurrentRequests, + }, TLS: &GatewayServiceTLSConfig{ SDS: &GatewayTLSSDSConfig{ ClusterName: "cluster1", @@ -893,8 +897,10 @@ func TestIngressGateway_Validate(t *testing.T) { Protocol: "http", Services: []IngressService{ { - Name: "svc1", - MaxConnections: &zero, + Name: "svc1", + IngressServiceConfig: IngressServiceConfig{ + MaxConnections: &zero, + }, }, }, }, @@ -916,8 +922,10 @@ func TestIngressGateway_Validate(t *testing.T) { Protocol: "http", Services: []IngressService{ { - Name: "svc1", - MaxConcurrentRequests: &zero, + Name: "svc1", + IngressServiceConfig: IngressServiceConfig{ + MaxConcurrentRequests: &zero, + }, }, }, }, @@ -939,8 +947,10 @@ func TestIngressGateway_Validate(t *testing.T) { Protocol: "http", Services: []IngressService{ { - Name: "svc1", - MaxPendingRequests: &zero, + Name: "svc1", + IngressServiceConfig: IngressServiceConfig{ + MaxPendingRequests: &zero, + }, }, }, }, diff --git a/control-plane/api/v1alpha1/zz_generated.deepcopy.go b/control-plane/api/v1alpha1/zz_generated.deepcopy.go index 37cab374e5..a51bf63d0d 100644 --- a/control-plane/api/v1alpha1/zz_generated.deepcopy.go +++ b/control-plane/api/v1alpha1/zz_generated.deepcopy.go @@ -450,21 +450,7 @@ func (in *IngressService) DeepCopyInto(out *IngressService) { *out = new(HTTPHeaderModifiers) (*in).DeepCopyInto(*out) } - if in.MaxConnections != nil { - in, out := &in.MaxConnections, &out.MaxConnections - *out = new(uint32) - **out = **in - } - if in.MaxPendingRequests != nil { - in, out := &in.MaxPendingRequests, &out.MaxPendingRequests - *out = new(uint32) - **out = **in - } - if in.MaxConcurrentRequests != nil { - in, out := &in.MaxConcurrentRequests, &out.MaxConcurrentRequests - *out = new(uint32) - **out = **in - } + in.IngressServiceConfig.DeepCopyInto(&out.IngressServiceConfig) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new IngressService.