From 8d466d0ebc394f30579841ac9201e82d755730d4 Mon Sep 17 00:00:00 2001 From: cskh Date: Fri, 19 Aug 2022 10:44:23 -0400 Subject: [PATCH 1/7] fix: missing MaxInboundConnections in service-defaults CRD --- .../consul/templates/crd-servicedefaults.yaml | 5 +++ .../api/v1alpha1/servicedefaults_types.go | 33 +++++++++++++------ .../v1alpha1/servicedefaults_types_test.go | 2 ++ .../api/v1alpha1/zz_generated.deepcopy.go | 5 +++ .../consul.hashicorp.com_servicedefaults.yaml | 5 +++ control-plane/go.mod | 3 ++ 6 files changed, 43 insertions(+), 10 deletions(-) diff --git a/charts/consul/templates/crd-servicedefaults.yaml b/charts/consul/templates/crd-servicedefaults.yaml index ea7720e9a2..c1b26417e4 100644 --- a/charts/consul/templates/crd-servicedefaults.yaml +++ b/charts/consul/templates/crd-servicedefaults.yaml @@ -113,6 +113,11 @@ spec: TLS SNI value to be changed to a non-connect value when federating with an external system. type: string + maxInboundConnections: + description: MaxInboundConnections is the maximum number of concurrent + inbound connections to each service instance. Defaults to 0 (unlimited) + if not set. + type: integer meshGateway: description: MeshGateway controls the default mesh gateway configuration for this service. diff --git a/control-plane/api/v1alpha1/servicedefaults_types.go b/control-plane/api/v1alpha1/servicedefaults_types.go index 41c855d75c..9617d987c3 100644 --- a/control-plane/api/v1alpha1/servicedefaults_types.go +++ b/control-plane/api/v1alpha1/servicedefaults_types.go @@ -84,6 +84,9 @@ type ServiceDefaultsSpec struct { // mode. Destinations live outside of Consul's catalog, and because of this, they // do not require an artificial node to be created. Destination *ServiceDefaultsDestination `json:"destination,omitempty"` + // MaxInboundConnections is the maximum number of concurrent inbound connections to + // each service instance. Defaults to 0 (unlimited) if not set. + MaxInboundConnections *int `json:"maxInboundConnections,omitempty"` } type Upstreams struct { @@ -244,17 +247,22 @@ func (in *ServiceDefaults) SyncedConditionStatus() corev1.ConditionStatus { // ToConsul converts the entry into it's Consul equivalent struct. func (in *ServiceDefaults) ToConsul(datacenter string) capi.ConfigEntry { + maxInboundConnections := 0 + if in.Spec.MaxInboundConnections != nil { + maxInboundConnections = *in.Spec.MaxInboundConnections + } return &capi.ServiceConfigEntry{ - Kind: in.ConsulKind(), - Name: in.ConsulName(), - Protocol: in.Spec.Protocol, - MeshGateway: in.Spec.MeshGateway.toConsul(), - Expose: in.Spec.Expose.toConsul(), - ExternalSNI: in.Spec.ExternalSNI, - TransparentProxy: in.Spec.TransparentProxy.toConsul(), - UpstreamConfig: in.Spec.UpstreamConfig.toConsul(), - Destination: in.Spec.Destination.toConsul(), - Meta: meta(datacenter), + Kind: in.ConsulKind(), + Name: in.ConsulName(), + Protocol: in.Spec.Protocol, + MeshGateway: in.Spec.MeshGateway.toConsul(), + Expose: in.Spec.Expose.toConsul(), + ExternalSNI: in.Spec.ExternalSNI, + TransparentProxy: in.Spec.TransparentProxy.toConsul(), + UpstreamConfig: in.Spec.UpstreamConfig.toConsul(), + Destination: in.Spec.Destination.toConsul(), + Meta: meta(datacenter), + MaxInboundConnections: uint64(maxInboundConnections), } } @@ -280,6 +288,11 @@ func (in *ServiceDefaults) Validate(consulMeta common.ConsulMeta) error { if err := in.Spec.Destination.validate(path.Child("destination")); err != nil { allErrs = append(allErrs, err...) } + + if in.Spec.MaxInboundConnections != nil && *in.Spec.MaxInboundConnections < 0 { + allErrs = append(allErrs, field.Invalid(path.Child("maxinboundconnections"), in.Spec.MaxInboundConnections, "MaxInboundConnections must > 0")) + } + allErrs = append(allErrs, in.Spec.UpstreamConfig.validate(path.Child("upstreamConfig"), consulMeta.PartitionsEnabled)...) allErrs = append(allErrs, in.Spec.Expose.validate(path.Child("expose"))...) diff --git a/control-plane/api/v1alpha1/servicedefaults_types_test.go b/control-plane/api/v1alpha1/servicedefaults_types_test.go index a19ea556b3..0eebe77def 100644 --- a/control-plane/api/v1alpha1/servicedefaults_types_test.go +++ b/control-plane/api/v1alpha1/servicedefaults_types_test.go @@ -141,6 +141,7 @@ func TestServiceDefaults_ToConsul(t *testing.T) { Addresses: []string{"api.google.com"}, Port: 443, }, + MaxInboundConnections: intPointer(20), }, }, &capi.ServiceConfigEntry{ @@ -243,6 +244,7 @@ func TestServiceDefaults_ToConsul(t *testing.T) { Addresses: []string{"api.google.com"}, Port: 443, }, + MaxInboundConnections: 20, Meta: map[string]string{ common.SourceKey: common.SourceValue, common.DatacenterKey: "datacenter", diff --git a/control-plane/api/v1alpha1/zz_generated.deepcopy.go b/control-plane/api/v1alpha1/zz_generated.deepcopy.go index 85d64c3c85..c8b1092ba7 100644 --- a/control-plane/api/v1alpha1/zz_generated.deepcopy.go +++ b/control-plane/api/v1alpha1/zz_generated.deepcopy.go @@ -1320,6 +1320,11 @@ func (in *ServiceDefaultsSpec) DeepCopyInto(out *ServiceDefaultsSpec) { *out = new(ServiceDefaultsDestination) (*in).DeepCopyInto(*out) } + if in.MaxInboundConnections != nil { + in, out := &in.MaxInboundConnections, &out.MaxInboundConnections + *out = new(int) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ServiceDefaultsSpec. diff --git a/control-plane/config/crd/bases/consul.hashicorp.com_servicedefaults.yaml b/control-plane/config/crd/bases/consul.hashicorp.com_servicedefaults.yaml index f536c17bed..b4fe662233 100644 --- a/control-plane/config/crd/bases/consul.hashicorp.com_servicedefaults.yaml +++ b/control-plane/config/crd/bases/consul.hashicorp.com_servicedefaults.yaml @@ -106,6 +106,11 @@ spec: TLS SNI value to be changed to a non-connect value when federating with an external system. type: string + maxInboundConnections: + description: MaxInboundConnections is the maximum number of concurrent + inbound connections to each service instance. Defaults to 0 (unlimited) + if not set. + type: integer meshGateway: description: MeshGateway controls the default mesh gateway configuration for this service. diff --git a/control-plane/go.mod b/control-plane/go.mod index 1848c8f435..50c9716a0b 100644 --- a/control-plane/go.mod +++ b/control-plane/go.mod @@ -129,4 +129,7 @@ require ( replace github.com/hashicorp/consul/sdk v0.10.0 => github.com/hashicorp/consul/sdk v0.4.1-0.20220801192236-988e1fd35d51 +// TODO beforing merging: remove the replace +replace github.com/hashicorp/consul/api => ../../consul/api + go 1.18 From 37630117b06d03db957bb4354748d66e16a3cf3b Mon Sep 17 00:00:00 2001 From: cskh Date: Fri, 19 Aug 2022 10:49:21 -0400 Subject: [PATCH 2/7] update changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 650fa52068..1ee10fa3f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,7 @@ ## UNRELEASED +FEATURES: +* MaxInboundConnections in service-defaults CRD + * Add support for MaxInboundConnections on the Service Defaults CRD. [[GH-1437](https://github.com/hashicorp/consul-k8s/pull/1437)] ## 0.47.1 (August 12, 2022) From 0498f8a12aacea173e01504130951833cc6de8e9 Mon Sep 17 00:00:00 2001 From: cskh Date: Mon, 22 Aug 2022 08:34:04 -0400 Subject: [PATCH 3/7] fix test --- control-plane/api/v1alpha1/servicedefaults_types.go | 2 +- control-plane/controller/configentry_controller_test.go | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/control-plane/api/v1alpha1/servicedefaults_types.go b/control-plane/api/v1alpha1/servicedefaults_types.go index 9617d987c3..1bc5493980 100644 --- a/control-plane/api/v1alpha1/servicedefaults_types.go +++ b/control-plane/api/v1alpha1/servicedefaults_types.go @@ -262,7 +262,7 @@ func (in *ServiceDefaults) ToConsul(datacenter string) capi.ConfigEntry { UpstreamConfig: in.Spec.UpstreamConfig.toConsul(), Destination: in.Spec.Destination.toConsul(), Meta: meta(datacenter), - MaxInboundConnections: uint64(maxInboundConnections), + MaxInboundConnections: maxInboundConnections, } } diff --git a/control-plane/controller/configentry_controller_test.go b/control-plane/controller/configentry_controller_test.go index 5b92feedd5..4ca7f2d2ea 100644 --- a/control-plane/controller/configentry_controller_test.go +++ b/control-plane/controller/configentry_controller_test.go @@ -19,6 +19,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/pointer" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -51,7 +52,8 @@ func TestConfigEntryControllers_createsConfigEntry(t *testing.T) { Namespace: kubeNS, }, Spec: v1alpha1.ServiceDefaultsSpec{ - Protocol: "http", + Protocol: "http", + MaxInboundConnections: pointer.Int(100), }, }, reconciler: func(client client.Client, consulClient *capi.Client, logger logr.Logger) testReconciler { @@ -68,6 +70,7 @@ func TestConfigEntryControllers_createsConfigEntry(t *testing.T) { svcDefault, ok := consulEntry.(*capi.ServiceConfigEntry) require.True(t, ok, "cast error") require.Equal(t, "http", svcDefault.Protocol) + require.Equal(t, 100, svcDefault.MaxInboundConnections) }, }, { From 70a4b2483d46f38406e33b6afbe1a8c95f81e401 Mon Sep 17 00:00:00 2001 From: cskh Date: Mon, 22 Aug 2022 17:08:12 -0400 Subject: [PATCH 4/7] update go.mod --- control-plane/go.mod | 5 +---- control-plane/go.sum | 4 ++-- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/control-plane/go.mod b/control-plane/go.mod index 50c9716a0b..bc8edd48ea 100644 --- a/control-plane/go.mod +++ b/control-plane/go.mod @@ -6,7 +6,7 @@ require ( github.com/go-logr/logr v0.4.0 github.com/google/go-cmp v0.5.7 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 - github.com/hashicorp/consul/api v1.14.0 + github.com/hashicorp/consul/api v1.10.1-0.20220822180451-60c82757ea35 github.com/hashicorp/consul/sdk v0.11.0 github.com/hashicorp/go-discover v0.0.0-20200812215701-c4b85f6ed31f github.com/hashicorp/go-hclog v0.16.1 @@ -129,7 +129,4 @@ require ( replace github.com/hashicorp/consul/sdk v0.10.0 => github.com/hashicorp/consul/sdk v0.4.1-0.20220801192236-988e1fd35d51 -// TODO beforing merging: remove the replace -replace github.com/hashicorp/consul/api => ../../consul/api - go 1.18 diff --git a/control-plane/go.sum b/control-plane/go.sum index 03ca166926..8be9787ae3 100644 --- a/control-plane/go.sum +++ b/control-plane/go.sum @@ -296,8 +296,8 @@ github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0/go.mod h1:8NvIoxWQoOIhqOTXgf github.com/grpc-ecosystem/grpc-gateway v1.9.0/go.mod h1:vNeuVxBJEsws4ogUvrchl83t/GYV9WGTSLVdBhOQFDY= github.com/grpc-ecosystem/grpc-gateway v1.16.0/go.mod h1:BDjrQk3hbvj6Nolgz8mAMFbcEtjT1g+wF4CSlocrBnw= github.com/hashicorp/consul/api v1.1.0/go.mod h1:VmuI/Lkw1nC05EYQWNKwWGbkg+FbDBtguAZLlVdkD9Q= -github.com/hashicorp/consul/api v1.14.0 h1:Y64GIJ8hYTu+tuGekwO4G4ardXoiCivX9wv1iP/kihk= -github.com/hashicorp/consul/api v1.14.0/go.mod h1:bcaw5CSZ7NE9qfOfKCI1xb7ZKjzu/MyvQkCLTfqLqxQ= +github.com/hashicorp/consul/api v1.10.1-0.20220822180451-60c82757ea35 h1:csNww5qBHaFqsX1eMEKVvmJ4dhqcXWj0sCkbccsSsHc= +github.com/hashicorp/consul/api v1.10.1-0.20220822180451-60c82757ea35/go.mod h1:bcaw5CSZ7NE9qfOfKCI1xb7ZKjzu/MyvQkCLTfqLqxQ= github.com/hashicorp/consul/sdk v0.1.1/go.mod h1:VKf9jXwCTEY1QZP2MOLRhb5i/I/ssyNV1vwHyQBF0x8= github.com/hashicorp/consul/sdk v0.4.1-0.20220801192236-988e1fd35d51/go.mod h1:yPkX5Q6CsxTFMjQQDJwzeNmUUF5NUGGbrDsv9wTb8cw= github.com/hashicorp/consul/sdk v0.11.0 h1:HRzj8YSCln2yGgCumN5CL8lYlD3gBurnervJRJAZyC4= From e2c8b714b23dac582c779cc2b1535f4b1fa6c51d Mon Sep 17 00:00:00 2001 From: cskh Date: Tue, 23 Aug 2022 12:48:33 -0400 Subject: [PATCH 5/7] change type to int --- control-plane/api/v1alpha1/servicedefaults_types.go | 12 ++++-------- .../api/v1alpha1/servicedefaults_types_test.go | 2 +- control-plane/api/v1alpha1/zz_generated.deepcopy.go | 5 ----- .../bases/consul.hashicorp.com_servicedefaults.yaml | 4 ++-- 4 files changed, 7 insertions(+), 16 deletions(-) diff --git a/control-plane/api/v1alpha1/servicedefaults_types.go b/control-plane/api/v1alpha1/servicedefaults_types.go index 1bc5493980..c0bf553f65 100644 --- a/control-plane/api/v1alpha1/servicedefaults_types.go +++ b/control-plane/api/v1alpha1/servicedefaults_types.go @@ -85,8 +85,8 @@ type ServiceDefaultsSpec struct { // do not require an artificial node to be created. Destination *ServiceDefaultsDestination `json:"destination,omitempty"` // MaxInboundConnections is the maximum number of concurrent inbound connections to - // each service instance. Defaults to 0 (unlimited) if not set. - MaxInboundConnections *int `json:"maxInboundConnections,omitempty"` + // each service instance. Defaults to 0 (using consul's default) if not set. + MaxInboundConnections int `json:"maxInboundConnections,omitempty"` } type Upstreams struct { @@ -247,10 +247,6 @@ func (in *ServiceDefaults) SyncedConditionStatus() corev1.ConditionStatus { // ToConsul converts the entry into it's Consul equivalent struct. func (in *ServiceDefaults) ToConsul(datacenter string) capi.ConfigEntry { - maxInboundConnections := 0 - if in.Spec.MaxInboundConnections != nil { - maxInboundConnections = *in.Spec.MaxInboundConnections - } return &capi.ServiceConfigEntry{ Kind: in.ConsulKind(), Name: in.ConsulName(), @@ -262,7 +258,7 @@ func (in *ServiceDefaults) ToConsul(datacenter string) capi.ConfigEntry { UpstreamConfig: in.Spec.UpstreamConfig.toConsul(), Destination: in.Spec.Destination.toConsul(), Meta: meta(datacenter), - MaxInboundConnections: maxInboundConnections, + MaxInboundConnections: in.Spec.MaxInboundConnections, } } @@ -289,7 +285,7 @@ func (in *ServiceDefaults) Validate(consulMeta common.ConsulMeta) error { allErrs = append(allErrs, err...) } - if in.Spec.MaxInboundConnections != nil && *in.Spec.MaxInboundConnections < 0 { + if in.Spec.MaxInboundConnections < 0 { allErrs = append(allErrs, field.Invalid(path.Child("maxinboundconnections"), in.Spec.MaxInboundConnections, "MaxInboundConnections must > 0")) } diff --git a/control-plane/api/v1alpha1/servicedefaults_types_test.go b/control-plane/api/v1alpha1/servicedefaults_types_test.go index 0eebe77def..6c15c00c48 100644 --- a/control-plane/api/v1alpha1/servicedefaults_types_test.go +++ b/control-plane/api/v1alpha1/servicedefaults_types_test.go @@ -141,7 +141,7 @@ func TestServiceDefaults_ToConsul(t *testing.T) { Addresses: []string{"api.google.com"}, Port: 443, }, - MaxInboundConnections: intPointer(20), + MaxInboundConnections: 20, }, }, &capi.ServiceConfigEntry{ diff --git a/control-plane/api/v1alpha1/zz_generated.deepcopy.go b/control-plane/api/v1alpha1/zz_generated.deepcopy.go index c8b1092ba7..85d64c3c85 100644 --- a/control-plane/api/v1alpha1/zz_generated.deepcopy.go +++ b/control-plane/api/v1alpha1/zz_generated.deepcopy.go @@ -1320,11 +1320,6 @@ func (in *ServiceDefaultsSpec) DeepCopyInto(out *ServiceDefaultsSpec) { *out = new(ServiceDefaultsDestination) (*in).DeepCopyInto(*out) } - if in.MaxInboundConnections != nil { - in, out := &in.MaxInboundConnections, &out.MaxInboundConnections - *out = new(int) - **out = **in - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ServiceDefaultsSpec. diff --git a/control-plane/config/crd/bases/consul.hashicorp.com_servicedefaults.yaml b/control-plane/config/crd/bases/consul.hashicorp.com_servicedefaults.yaml index b4fe662233..67f33517ba 100644 --- a/control-plane/config/crd/bases/consul.hashicorp.com_servicedefaults.yaml +++ b/control-plane/config/crd/bases/consul.hashicorp.com_servicedefaults.yaml @@ -108,8 +108,8 @@ spec: type: string maxInboundConnections: description: MaxInboundConnections is the maximum number of concurrent - inbound connections to each service instance. Defaults to 0 (unlimited) - if not set. + inbound connections to each service instance. Defaults to 0 (using + consul's default) if not set. type: integer meshGateway: description: MeshGateway controls the default mesh gateway configuration From cc5fcb324a2d7cfc8461332f605d04f4bf0622d3 Mon Sep 17 00:00:00 2001 From: cskh Date: Tue, 23 Aug 2022 12:51:59 -0400 Subject: [PATCH 6/7] fix test --- charts/consul/templates/crd-servicedefaults.yaml | 4 ++-- control-plane/controller/configentry_controller_test.go | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/charts/consul/templates/crd-servicedefaults.yaml b/charts/consul/templates/crd-servicedefaults.yaml index c1b26417e4..f0ba18a616 100644 --- a/charts/consul/templates/crd-servicedefaults.yaml +++ b/charts/consul/templates/crd-servicedefaults.yaml @@ -115,8 +115,8 @@ spec: type: string maxInboundConnections: description: MaxInboundConnections is the maximum number of concurrent - inbound connections to each service instance. Defaults to 0 (unlimited) - if not set. + inbound connections to each service instance. Defaults to 0 (using + consul's default) if not set. type: integer meshGateway: description: MeshGateway controls the default mesh gateway configuration diff --git a/control-plane/controller/configentry_controller_test.go b/control-plane/controller/configentry_controller_test.go index 4ca7f2d2ea..5a26d9abd6 100644 --- a/control-plane/controller/configentry_controller_test.go +++ b/control-plane/controller/configentry_controller_test.go @@ -19,7 +19,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" - "k8s.io/utils/pointer" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -53,7 +52,7 @@ func TestConfigEntryControllers_createsConfigEntry(t *testing.T) { }, Spec: v1alpha1.ServiceDefaultsSpec{ Protocol: "http", - MaxInboundConnections: pointer.Int(100), + MaxInboundConnections: 100, }, }, reconciler: func(client client.Client, consulClient *capi.Client, logger logr.Logger) testReconciler { From 2c98957e3b17419ce5fee39459a578d4bc09048a Mon Sep 17 00:00:00 2001 From: Ashwin Venkatesh Date: Tue, 23 Aug 2022 13:40:29 -0400 Subject: [PATCH 7/7] Apply suggestions from code review Co-authored-by: Thomas Eckert --- control-plane/api/v1alpha1/servicedefaults_types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/control-plane/api/v1alpha1/servicedefaults_types.go b/control-plane/api/v1alpha1/servicedefaults_types.go index c0bf553f65..325b80cefe 100644 --- a/control-plane/api/v1alpha1/servicedefaults_types.go +++ b/control-plane/api/v1alpha1/servicedefaults_types.go @@ -286,7 +286,7 @@ func (in *ServiceDefaults) Validate(consulMeta common.ConsulMeta) error { } if in.Spec.MaxInboundConnections < 0 { - allErrs = append(allErrs, field.Invalid(path.Child("maxinboundconnections"), in.Spec.MaxInboundConnections, "MaxInboundConnections must > 0")) + allErrs = append(allErrs, field.Invalid(path.Child("maxinboundconnections"), in.Spec.MaxInboundConnections, "MaxInboundConnections must be > 0")) } allErrs = append(allErrs, in.Spec.UpstreamConfig.validate(path.Child("upstreamConfig"), consulMeta.PartitionsEnabled)...)