From 7592592167e6e8831d7dd56a3c5ffd55babf9a16 Mon Sep 17 00:00:00 2001 From: Semir Patel Date: Tue, 8 Nov 2022 15:43:20 -0600 Subject: [PATCH 1/5] Add maxConnections and friends to ingressgateway_types.go and regen manifests --- .../consul/templates/crd-ingressgateways.yaml | 22 ++++++++ .../api/v1alpha1/ingressgateway_types.go | 13 +++++ .../api/v1alpha1/zz_generated.deepcopy.go | 50 +++++++++++++++++++ .../consul.hashicorp.com_ingressgateways.yaml | 22 ++++++++ 4 files changed, 107 insertions(+) diff --git a/charts/consul/templates/crd-ingressgateways.yaml b/charts/consul/templates/crd-ingressgateways.yaml index f14789e83d..1a241a9cfb 100644 --- a/charts/consul/templates/crd-ingressgateways.yaml +++ b/charts/consul/templates/crd-ingressgateways.yaml @@ -57,6 +57,19 @@ spec: spec: description: IngressGatewaySpec defines the desired state of IngressGateway. properties: + defaults: + description: Defaults is default configuration for all upstream services + properties: + maxConcurrentRequests: + format: int32 + type: integer + maxConnections: + format: int32 + type: integer + maxPendingRequests: + format: int32 + type: integer + type: object listeners: description: Listeners declares what ports the ingress gateway should listen on, and what services to associated to those ports. @@ -98,6 +111,15 @@ spec: items: type: string type: array + maxConcurrentRequests: + format: int32 + type: integer + maxConnections: + format: int32 + type: integer + maxPendingRequests: + format: int32 + type: integer name: description: "Name declares the service to which traffic should be forwarded. \n This can either be a specific diff --git a/control-plane/api/v1alpha1/ingressgateway_types.go b/control-plane/api/v1alpha1/ingressgateway_types.go index 7251608223..6051c4664b 100644 --- a/control-plane/api/v1alpha1/ingressgateway_types.go +++ b/control-plane/api/v1alpha1/ingressgateway_types.go @@ -57,6 +57,15 @@ type IngressGatewaySpec struct { // Listeners declares what ports the ingress gateway should listen on, and // what services to associated to those ports. Listeners []IngressListener `json:"listeners,omitempty"` + + // Defaults is default configuration for all upstream services + Defaults *IngressServiceConfig `json:"defaults,omitempty"` +} + +type IngressServiceConfig struct { + MaxConnections *uint32 `json:"maxConnections,omitempty"` + MaxPendingRequests *uint32 `json:"maxPendingRequests,omitempty"` + MaxConcurrentRequests *uint32 `json:"maxConcurrentRequests,omitempty"` } type GatewayTLSConfig struct { @@ -144,6 +153,10 @@ type IngressService struct { // Allow HTTP header manipulation to be configured. RequestHeaders *HTTPHeaderModifiers `json:"requestHeaders,omitempty"` ResponseHeaders *HTTPHeaderModifiers `json:"responseHeaders,omitempty"` + + MaxConnections *uint32 `json:"maxConnections,omitempty"` + MaxPendingRequests *uint32 `json:"maxPendingRequests,omitempty"` + MaxConcurrentRequests *uint32 `json:"maxConcurrentRequests,omitempty"` } func (in *IngressGateway) GetObjectMeta() metav1.ObjectMeta { diff --git a/control-plane/api/v1alpha1/zz_generated.deepcopy.go b/control-plane/api/v1alpha1/zz_generated.deepcopy.go index 8e96bdf0c2..37cab374e5 100644 --- a/control-plane/api/v1alpha1/zz_generated.deepcopy.go +++ b/control-plane/api/v1alpha1/zz_generated.deepcopy.go @@ -383,6 +383,11 @@ func (in *IngressGatewaySpec) DeepCopyInto(out *IngressGatewaySpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.Defaults != nil { + in, out := &in.Defaults, &out.Defaults + *out = new(IngressServiceConfig) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new IngressGatewaySpec. @@ -445,6 +450,21 @@ 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 + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new IngressService. @@ -457,6 +477,36 @@ func (in *IngressService) DeepCopy() *IngressService { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *IngressServiceConfig) DeepCopyInto(out *IngressServiceConfig) { + *out = *in + 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 + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new IngressServiceConfig. +func (in *IngressServiceConfig) DeepCopy() *IngressServiceConfig { + if in == nil { + return nil + } + out := new(IngressServiceConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *IntentionDestination) DeepCopyInto(out *IntentionDestination) { *out = *in diff --git a/control-plane/config/crd/bases/consul.hashicorp.com_ingressgateways.yaml b/control-plane/config/crd/bases/consul.hashicorp.com_ingressgateways.yaml index 6378ee4213..9934e9c0d9 100644 --- a/control-plane/config/crd/bases/consul.hashicorp.com_ingressgateways.yaml +++ b/control-plane/config/crd/bases/consul.hashicorp.com_ingressgateways.yaml @@ -50,6 +50,19 @@ spec: spec: description: IngressGatewaySpec defines the desired state of IngressGateway. properties: + defaults: + description: Defaults is default configuration for all upstream services + properties: + maxConcurrentRequests: + format: int32 + type: integer + maxConnections: + format: int32 + type: integer + maxPendingRequests: + format: int32 + type: integer + type: object listeners: description: Listeners declares what ports the ingress gateway should listen on, and what services to associated to those ports. @@ -91,6 +104,15 @@ spec: items: type: string type: array + maxConcurrentRequests: + format: int32 + type: integer + maxConnections: + format: int32 + type: integer + maxPendingRequests: + format: int32 + type: integer name: description: "Name declares the service to which traffic should be forwarded. \n This can either be a specific From 876eb7c20e46e8aa5b88a37322dd7523c2a6dae2 Mon Sep 17 00:00:00 2001 From: Semir Patel Date: Tue, 8 Nov 2022 19:03:58 -0600 Subject: [PATCH 2/5] Validate + tests --- .../api/v1alpha1/ingressgateway_types.go | 34 +++++ .../api/v1alpha1/ingressgateway_types_test.go | 117 ++++++++++++++++++ 2 files changed, 151 insertions(+) diff --git a/control-plane/api/v1alpha1/ingressgateway_types.go b/control-plane/api/v1alpha1/ingressgateway_types.go index 6051c4664b..a2d1fd2da9 100644 --- a/control-plane/api/v1alpha1/ingressgateway_types.go +++ b/control-plane/api/v1alpha1/ingressgateway_types.go @@ -270,6 +270,8 @@ func (in *IngressGateway) Validate(consulMeta common.ConsulMeta) error { errs = append(errs, v.validate(path.Child("listeners").Index(i), consulMeta)...) } + errs = append(errs, in.Spec.Defaults.validate(path.Child("defaults"))...) + if len(errs) > 0 { return apierrors.NewInvalid( schema.GroupKind{Group: ConsulHashicorpGroup, Kind: ingressGatewayKubeKind}, @@ -419,6 +421,38 @@ func (in IngressListener) validate(path *field.Path, consulMeta common.ConsulMet string(asJSON), "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")) + } + } + return errs +} + +func (in *IngressServiceConfig) validate(path *field.Path) field.ErrorList { + if in == nil { + return nil + } + var errs field.ErrorList + + if in.MaxConnections != nil && *in.MaxConnections <= 0 { + errs = append(errs, field.Invalid(path.Child("maxconnections"), *in.MaxConnections, "MaxConnections must be > 0")) + } + + if in.MaxConcurrentRequests != nil && *in.MaxConcurrentRequests <= 0 { + errs = append(errs, field.Invalid(path.Child("maxconcurrentrequests"), *in.MaxConcurrentRequests, "MaxConcurrentRequests must be > 0")) + } + + if in.MaxPendingRequests != nil && *in.MaxPendingRequests <= 0 { + errs = append(errs, field.Invalid(path.Child("maxpendingrequests"), *in.MaxPendingRequests, "MaxPendingRequests must be > 0")) } return errs } diff --git a/control-plane/api/v1alpha1/ingressgateway_types_test.go b/control-plane/api/v1alpha1/ingressgateway_types_test.go index 2585614519..6d70f23c5f 100644 --- a/control-plane/api/v1alpha1/ingressgateway_types_test.go +++ b/control-plane/api/v1alpha1/ingressgateway_types_test.go @@ -471,6 +471,8 @@ func TestIngressGateway_ToConsul(t *testing.T) { } func TestIngressGateway_Validate(t *testing.T) { + zero := uint32(0) + cases := map[string]struct { input *IngressGateway namespacesEnabled bool @@ -785,6 +787,121 @@ func TestIngressGateway_Validate(t *testing.T) { }, partitionEnabled: true, }, + "defaults.maxConnections invalid": { + input: &IngressGateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: IngressGatewaySpec{ + Defaults: &IngressServiceConfig{ + MaxConnections: &zero, + }, + }, + }, + expectedErrMsgs: []string{ + `spec.defaults.maxconnections: Invalid`, + }, + }, + "defaults.maxPendingRequests invalid": { + input: &IngressGateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: IngressGatewaySpec{ + Defaults: &IngressServiceConfig{ + MaxPendingRequests: &zero, + }, + }, + }, + expectedErrMsgs: []string{ + `spec.defaults.maxpendingrequests: Invalid`, + }, + }, + "defaults.maxConcurrentRequests invalid": { + input: &IngressGateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: IngressGatewaySpec{ + Defaults: &IngressServiceConfig{ + MaxConcurrentRequests: &zero, + }, + }, + }, + expectedErrMsgs: []string{ + `spec.defaults.maxconcurrentrequests: Invalid`, + }, + }, + "service.maxConnections invalid": { + input: &IngressGateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: IngressGatewaySpec{ + Listeners: []IngressListener{ + { + Protocol: "http", + Services: []IngressService{ + { + Name: "svc1", + MaxConnections: &zero, + }, + }, + }, + }, + }, + }, + expectedErrMsgs: []string{ + `spec.listeners[0].maxconnections: Invalid`, + }, + }, + "service.maxConcurrentRequests invalid": { + input: &IngressGateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: IngressGatewaySpec{ + Listeners: []IngressListener{ + { + Protocol: "http", + Services: []IngressService{ + { + Name: "svc1", + MaxConcurrentRequests: &zero, + }, + }, + }, + }, + }, + }, + expectedErrMsgs: []string{ + `spec.listeners[0].maxconcurrentrequests: Invalid`, + }, + }, + "service.maxPendingRequests invalid": { + input: &IngressGateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: IngressGatewaySpec{ + Listeners: []IngressListener{ + { + Protocol: "http", + Services: []IngressService{ + { + Name: "svc1", + MaxPendingRequests: &zero, + }, + }, + }, + }, + }, + }, + expectedErrMsgs: []string{ + `spec.listeners[0].maxpendingrequests: Invalid`, + }, + }, + "multiple errors": { input: &IngressGateway{ ObjectMeta: metav1.ObjectMeta{ From 438aae1c90a6e87ab36301fd30975d4682d3d12a Mon Sep 17 00:00:00 2001 From: Semir Patel Date: Wed, 9 Nov 2022 11:10:06 -0600 Subject: [PATCH 3/5] IngressGateway.toConsul() + tests --- .../api/v1alpha1/ingressgateway_types.go | 29 +++++++++---- .../api/v1alpha1/ingressgateway_types_test.go | 41 +++++++++++++++---- 2 files changed, 55 insertions(+), 15 deletions(-) diff --git a/control-plane/api/v1alpha1/ingressgateway_types.go b/control-plane/api/v1alpha1/ingressgateway_types.go index a2d1fd2da9..d213b7eb84 100644 --- a/control-plane/api/v1alpha1/ingressgateway_types.go +++ b/control-plane/api/v1alpha1/ingressgateway_types.go @@ -248,6 +248,7 @@ func (in *IngressGateway) ToConsul(datacenter string) capi.ConfigEntry { TLS: *in.Spec.TLS.toConsul(), Listeners: listeners, Meta: meta(datacenter), + Defaults: in.Spec.Defaults.toConsul(), } } @@ -344,13 +345,16 @@ func (in IngressListener) toConsul() capi.IngressListener { func (in IngressService) toConsul() capi.IngressService { return capi.IngressService{ - Name: in.Name, - Hosts: in.Hosts, - Namespace: in.Namespace, - Partition: in.Partition, - TLS: in.TLS.toConsul(), - RequestHeaders: in.RequestHeaders.toConsul(), - ResponseHeaders: in.ResponseHeaders.toConsul(), + Name: in.Name, + Hosts: in.Hosts, + Namespace: in.Namespace, + Partition: in.Partition, + TLS: in.TLS.toConsul(), + RequestHeaders: in.RequestHeaders.toConsul(), + ResponseHeaders: in.ResponseHeaders.toConsul(), + MaxConnections: in.MaxConnections, + MaxPendingRequests: in.MaxPendingRequests, + MaxConcurrentRequests: in.MaxConcurrentRequests, } } @@ -456,3 +460,14 @@ func (in *IngressServiceConfig) validate(path *field.Path) field.ErrorList { } return errs } + +func (in *IngressServiceConfig) toConsul() *capi.IngressServiceConfig { + if in == nil { + return nil + } + return &capi.IngressServiceConfig{ + MaxConnections: in.MaxConnections, + MaxPendingRequests: in.MaxPendingRequests, + MaxConcurrentRequests: in.MaxConcurrentRequests, + } +} diff --git a/control-plane/api/v1alpha1/ingressgateway_types_test.go b/control-plane/api/v1alpha1/ingressgateway_types_test.go index 6d70f23c5f..c60f487954 100644 --- a/control-plane/api/v1alpha1/ingressgateway_types_test.go +++ b/control-plane/api/v1alpha1/ingressgateway_types_test.go @@ -253,6 +253,15 @@ func TestIngressGateway_MatchesConsul(t *testing.T) { } func TestIngressGateway_ToConsul(t *testing.T) { + + defaultMaxConnections := uint32(100) + defaultMaxPendingRequests := uint32(101) + defaultMaxConcurrentRequests := uint32(102) + + maxConnections := uint32(200) + maxPendingRequests := uint32(201) + maxConcurrentRequests := uint32(202) + cases := map[string]struct { Ours IngressGateway Exp *capi.IngressGatewayConfigEntry @@ -289,6 +298,11 @@ func TestIngressGateway_ToConsul(t *testing.T) { TLSMaxVersion: "TLSv1_1", CipherSuites: []string{"ECDHE-ECDSA-AES128-GCM-SHA256", "AES128-SHA"}, }, + Defaults: &IngressServiceConfig{ + MaxConnections: &defaultMaxConnections, + MaxPendingRequests: &defaultMaxPendingRequests, + MaxConcurrentRequests: &defaultMaxConcurrentRequests, + }, Listeners: []IngressListener{ { Port: 8888, @@ -305,10 +319,13 @@ func TestIngressGateway_ToConsul(t *testing.T) { }, Services: []IngressService{ { - Name: "name1", - Hosts: []string{"host1_1", "host1_2"}, - Namespace: "ns1", - Partition: "default", + Name: "name1", + Hosts: []string{"host1_1", "host1_2"}, + Namespace: "ns1", + Partition: "default", + MaxConnections: &maxConnections, + MaxPendingRequests: &maxPendingRequests, + MaxConcurrentRequests: &maxConcurrentRequests, TLS: &GatewayServiceTLSConfig{ SDS: &GatewayTLSSDSConfig{ ClusterName: "cluster1", @@ -378,6 +395,11 @@ func TestIngressGateway_ToConsul(t *testing.T) { TLSMaxVersion: "TLSv1_1", CipherSuites: []string{"ECDHE-ECDSA-AES128-GCM-SHA256", "AES128-SHA"}, }, + Defaults: &capi.IngressServiceConfig{ + MaxConnections: &defaultMaxConnections, + MaxPendingRequests: &defaultMaxPendingRequests, + MaxConcurrentRequests: &defaultMaxConcurrentRequests, + }, Listeners: []capi.IngressListener{ { Port: 8888, @@ -394,10 +416,13 @@ func TestIngressGateway_ToConsul(t *testing.T) { }, Services: []capi.IngressService{ { - Name: "name1", - Hosts: []string{"host1_1", "host1_2"}, - Namespace: "ns1", - Partition: "default", + Name: "name1", + Hosts: []string{"host1_1", "host1_2"}, + Namespace: "ns1", + Partition: "default", + MaxConnections: &maxConnections, + MaxPendingRequests: &maxPendingRequests, + MaxConcurrentRequests: &maxConcurrentRequests, TLS: &capi.GatewayServiceTLSConfig{ SDS: &capi.GatewayTLSSDSConfig{ ClusterName: "cluster1", From 77fb7c72fabce381ee6914d2ff2e739570f64e70 Mon Sep 17 00:00:00 2001 From: Semir Patel Date: Thu, 10 Nov 2022 10:10:17 -0600 Subject: [PATCH 4/5] Document fields and update MatchesConsul() test --- CHANGELOG.md | 2 + .../consul/templates/crd-ingressgateways.yaml | 19 +++++++++ .../api/v1alpha1/ingressgateway_types.go | 24 +++++++++-- .../api/v1alpha1/ingressgateway_types_test.go | 41 +++++++++++++++---- .../consul.hashicorp.com_ingressgateways.yaml | 19 +++++++++ 5 files changed, 93 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 67c5552f6f..dec1b6119b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,8 @@ FEATURES: * Support merged metrics with consul-dataplane. [[GH-1635](https://github.com/hashicorp/consul-k8s/pull/1635)] * Support transparent proxying when using consul-dataplane. [[GH-1625](https://github.com/hashicorp/consul-k8s/pull/1478),[GH-1632](https://github.com/hashicorp/consul-k8s/pull/1632)] * Enable sync-catalog to only talk to Consul servers. [[GH-1659](https://github.com/hashicorp/consul-k8s/pull/1659)] +* Ingress Gateway + * Add support for MaxConnections, MaxConcurrentRequests, and MaxPendingRequests to Ingress Gateway CRD. [[GH-1691](https://github.com/hashicorp/consul-k8s/pull/1691)] IMPROVEMENTS: * CLI diff --git a/charts/consul/templates/crd-ingressgateways.yaml b/charts/consul/templates/crd-ingressgateways.yaml index 1a241a9cfb..6aeca4a2f7 100644 --- a/charts/consul/templates/crd-ingressgateways.yaml +++ b/charts/consul/templates/crd-ingressgateways.yaml @@ -61,12 +61,21 @@ spec: description: Defaults is default configuration for all upstream services properties: maxConcurrentRequests: + description: 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. format: int32 type: integer maxConnections: + description: 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. format: int32 type: integer maxPendingRequests: + description: The maximum number of requests that will be queued + while waiting for a connection to be established. format: int32 type: integer type: object @@ -112,12 +121,22 @@ spec: type: string type: array maxConcurrentRequests: + description: 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. format: int32 type: integer maxConnections: + description: 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. format: int32 type: integer maxPendingRequests: + description: The maximum number of requests that will + be queued while waiting for a connection to be established. format: int32 type: integer name: diff --git a/control-plane/api/v1alpha1/ingressgateway_types.go b/control-plane/api/v1alpha1/ingressgateway_types.go index d213b7eb84..06e07b353b 100644 --- a/control-plane/api/v1alpha1/ingressgateway_types.go +++ b/control-plane/api/v1alpha1/ingressgateway_types.go @@ -63,8 +63,16 @@ type IngressGatewaySpec struct { } type IngressServiceConfig struct { - MaxConnections *uint32 `json:"maxConnections,omitempty"` - MaxPendingRequests *uint32 `json:"maxPendingRequests,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"` } @@ -154,8 +162,16 @@ type IngressService struct { RequestHeaders *HTTPHeaderModifiers `json:"requestHeaders,omitempty"` ResponseHeaders *HTTPHeaderModifiers `json:"responseHeaders,omitempty"` - MaxConnections *uint32 `json:"maxConnections,omitempty"` - MaxPendingRequests *uint32 `json:"maxPendingRequests,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"` } diff --git a/control-plane/api/v1alpha1/ingressgateway_types_test.go b/control-plane/api/v1alpha1/ingressgateway_types_test.go index c60f487954..4f40bb9536 100644 --- a/control-plane/api/v1alpha1/ingressgateway_types_test.go +++ b/control-plane/api/v1alpha1/ingressgateway_types_test.go @@ -13,6 +13,15 @@ import ( ) func TestIngressGateway_MatchesConsul(t *testing.T) { + + defaultMaxConnections := uint32(100) + defaultMaxPendingRequests := uint32(101) + defaultMaxConcurrentRequests := uint32(102) + + maxConnections := uint32(200) + maxPendingRequests := uint32(201) + maxConcurrentRequests := uint32(202) + cases := map[string]struct { Ours IngressGateway Theirs capi.ConfigEntry @@ -54,6 +63,11 @@ func TestIngressGateway_MatchesConsul(t *testing.T) { TLSMaxVersion: "TLSv1_1", CipherSuites: []string{"ECDHE-ECDSA-AES128-GCM-SHA256", "AES128-SHA"}, }, + Defaults: &IngressServiceConfig{ + MaxConnections: &defaultMaxConnections, + MaxPendingRequests: &defaultMaxPendingRequests, + MaxConcurrentRequests: &defaultMaxConcurrentRequests, + }, Listeners: []IngressListener{ { Port: 8888, @@ -70,10 +84,13 @@ func TestIngressGateway_MatchesConsul(t *testing.T) { }, Services: []IngressService{ { - Name: "name1", - Hosts: []string{"host1_1", "host1_2"}, - Namespace: "ns1", - Partition: "default", + Name: "name1", + Hosts: []string{"host1_1", "host1_2"}, + Namespace: "ns1", + Partition: "default", + MaxConnections: &maxConnections, + MaxPendingRequests: &maxPendingRequests, + MaxConcurrentRequests: &maxConcurrentRequests, TLS: &GatewayServiceTLSConfig{ SDS: &GatewayTLSSDSConfig{ ClusterName: "cluster1", @@ -144,6 +161,11 @@ func TestIngressGateway_MatchesConsul(t *testing.T) { TLSMaxVersion: "TLSv1_1", CipherSuites: []string{"ECDHE-ECDSA-AES128-GCM-SHA256", "AES128-SHA"}, }, + Defaults: &capi.IngressServiceConfig{ + MaxConnections: &defaultMaxConnections, + MaxPendingRequests: &defaultMaxPendingRequests, + MaxConcurrentRequests: &defaultMaxConcurrentRequests, + }, Listeners: []capi.IngressListener{ { Port: 8888, @@ -160,10 +182,13 @@ func TestIngressGateway_MatchesConsul(t *testing.T) { }, Services: []capi.IngressService{ { - Name: "name1", - Hosts: []string{"host1_1", "host1_2"}, - Namespace: "ns1", - Partition: "default", + Name: "name1", + Hosts: []string{"host1_1", "host1_2"}, + Namespace: "ns1", + Partition: "default", + MaxConnections: &maxConnections, + MaxPendingRequests: &maxPendingRequests, + MaxConcurrentRequests: &maxConcurrentRequests, TLS: &capi.GatewayServiceTLSConfig{ SDS: &capi.GatewayTLSSDSConfig{ ClusterName: "cluster1", diff --git a/control-plane/config/crd/bases/consul.hashicorp.com_ingressgateways.yaml b/control-plane/config/crd/bases/consul.hashicorp.com_ingressgateways.yaml index 9934e9c0d9..16ac322090 100644 --- a/control-plane/config/crd/bases/consul.hashicorp.com_ingressgateways.yaml +++ b/control-plane/config/crd/bases/consul.hashicorp.com_ingressgateways.yaml @@ -54,12 +54,21 @@ spec: description: Defaults is default configuration for all upstream services properties: maxConcurrentRequests: + description: 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. format: int32 type: integer maxConnections: + description: 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. format: int32 type: integer maxPendingRequests: + description: The maximum number of requests that will be queued + while waiting for a connection to be established. format: int32 type: integer type: object @@ -105,12 +114,22 @@ spec: type: string type: array maxConcurrentRequests: + description: 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. format: int32 type: integer maxConnections: + description: 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. format: int32 type: integer maxPendingRequests: + description: The maximum number of requests that will + be queued while waiting for a connection to be established. format: int32 type: integer name: From 9a8ab4d559413b4d0609f826b970655af488bb27 Mon Sep 17 00:00:00 2001 From: Semir Patel Date: Thu, 10 Nov 2022 16:23:26 -0600 Subject: [PATCH 5/5] 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.