From 218903339c28a7f3c47c598e93c71f030de7c79e Mon Sep 17 00:00:00 2001 From: Michael Zalimeni Date: Wed, 16 Aug 2023 15:33:54 -0400 Subject: [PATCH] Add `PrioritizeByLocality` to `ProxyDefaults` CRD In addition to service resolver, add this field to the CRD for proxy defaults for parity with Consul config options. --- .changelog/2357.txt | 3 -- .changelog/2784.txt | 3 ++ .../consul/templates/crd-proxydefaults.yaml | 9 +++++ .../api/v1alpha1/proxydefaults_types.go | 27 +++++++------ .../api/v1alpha1/proxydefaults_types_test.go | 25 ++++++++++++ .../api/v1alpha1/serviceresolver_types.go | 38 +------------------ .../v1alpha1/serviceresolver_types_test.go | 10 ++--- control-plane/api/v1alpha1/shared_types.go | 29 ++++++++++++++ .../api/v1alpha1/zz_generated.deepcopy.go | 37 ++++++++++-------- .../consul.hashicorp.com_proxydefaults.yaml | 9 +++++ 10 files changed, 118 insertions(+), 72 deletions(-) delete mode 100644 .changelog/2357.txt create mode 100644 .changelog/2784.txt diff --git a/.changelog/2357.txt b/.changelog/2357.txt deleted file mode 100644 index 7cc35f595a..0000000000 --- a/.changelog/2357.txt +++ /dev/null @@ -1,3 +0,0 @@ -```release-note:feature -Add the `PrioritizeByLocality` field to the `ServiceResolver` CRD. -``` diff --git a/.changelog/2784.txt b/.changelog/2784.txt new file mode 100644 index 0000000000..5b11ca3d43 --- /dev/null +++ b/.changelog/2784.txt @@ -0,0 +1,3 @@ +```release-note:feature +Add the `PrioritizeByLocality` field to the `ServiceResolver` and `ProxyDefaults` CRDs. +``` diff --git a/charts/consul/templates/crd-proxydefaults.yaml b/charts/consul/templates/crd-proxydefaults.yaml index 362672c1c1..1e931dd888 100644 --- a/charts/consul/templates/crd-proxydefaults.yaml +++ b/charts/consul/templates/crd-proxydefaults.yaml @@ -160,6 +160,15 @@ spec: type: string type: array type: object + prioritizeByLocality: + description: PrioritizeByLocality contains the configuration for + locality aware routing. + properties: + mode: + description: Mode specifies the behavior of PrioritizeByLocality + routing. Valid values are "", "none", and "failover". + type: string + type: object meshGateway: description: MeshGateway controls the default mesh gateway configuration for this service. diff --git a/control-plane/api/v1alpha1/proxydefaults_types.go b/control-plane/api/v1alpha1/proxydefaults_types.go index 1100cd107a..7b1529b941 100644 --- a/control-plane/api/v1alpha1/proxydefaults_types.go +++ b/control-plane/api/v1alpha1/proxydefaults_types.go @@ -95,6 +95,9 @@ type ProxyDefaultsSpec struct { EnvoyExtensions EnvoyExtensions `json:"envoyExtensions,omitempty"` // FailoverPolicy specifies the exact mechanism used for failover. FailoverPolicy *FailoverPolicy `json:"failoverPolicy,omitempty"` + // PrioritizeByLocality controls whether the locality of services within the + // local partition will be used to prioritize connectivity. + PrioritizeByLocality *PrioritizeByLocality `json:"prioritizeByLocality,omitempty"` } func (in *ProxyDefaults) GetObjectMeta() metav1.ObjectMeta { @@ -179,17 +182,18 @@ func (in *ProxyDefaults) SetLastSyncedTime(time *metav1.Time) { func (in *ProxyDefaults) ToConsul(datacenter string) capi.ConfigEntry { consulConfig := in.convertConfig() return &capi.ProxyConfigEntry{ - Kind: in.ConsulKind(), - Name: in.ConsulName(), - MeshGateway: in.Spec.MeshGateway.toConsul(), - Expose: in.Spec.Expose.toConsul(), - Config: consulConfig, - TransparentProxy: in.Spec.TransparentProxy.toConsul(), - MutualTLSMode: in.Spec.MutualTLSMode.toConsul(), - AccessLogs: in.Spec.AccessLogs.toConsul(), - EnvoyExtensions: in.Spec.EnvoyExtensions.toConsul(), - FailoverPolicy: in.Spec.FailoverPolicy.toConsul(), - Meta: meta(datacenter), + Kind: in.ConsulKind(), + Name: in.ConsulName(), + MeshGateway: in.Spec.MeshGateway.toConsul(), + Expose: in.Spec.Expose.toConsul(), + Config: consulConfig, + TransparentProxy: in.Spec.TransparentProxy.toConsul(), + MutualTLSMode: in.Spec.MutualTLSMode.toConsul(), + AccessLogs: in.Spec.AccessLogs.toConsul(), + EnvoyExtensions: in.Spec.EnvoyExtensions.toConsul(), + FailoverPolicy: in.Spec.FailoverPolicy.toConsul(), + PrioritizeByLocality: in.Spec.PrioritizeByLocality.toConsul(), + Meta: meta(datacenter), } } @@ -228,6 +232,7 @@ func (in *ProxyDefaults) Validate(_ common.ConsulMeta) error { allErrs = append(allErrs, in.Spec.Expose.validate(path.Child("expose"))...) allErrs = append(allErrs, in.Spec.EnvoyExtensions.validate(path.Child("envoyExtensions"))...) allErrs = append(allErrs, in.Spec.FailoverPolicy.validate(path.Child("failoverPolicy"))...) + allErrs = append(allErrs, in.Spec.PrioritizeByLocality.validate(path.Child("prioritizeByLocality"))...) if len(allErrs) > 0 { return apierrors.NewInvalid( diff --git a/control-plane/api/v1alpha1/proxydefaults_types_test.go b/control-plane/api/v1alpha1/proxydefaults_types_test.go index 07f894f322..6a965fce3a 100644 --- a/control-plane/api/v1alpha1/proxydefaults_types_test.go +++ b/control-plane/api/v1alpha1/proxydefaults_types_test.go @@ -98,6 +98,9 @@ func TestProxyDefaults_MatchesConsul(t *testing.T) { Mode: "sequential", Regions: []string{"us-west-1"}, }, + PrioritizeByLocality: &PrioritizeByLocality{ + Mode: "failover", + }, }, }, Theirs: &capi.ProxyConfigEntry{ @@ -161,6 +164,9 @@ func TestProxyDefaults_MatchesConsul(t *testing.T) { Mode: "sequential", Regions: []string{"us-west-1"}, }, + PrioritizeByLocality: &capi.ServiceResolverPrioritizeByLocality{ + Mode: "failover", + }, }, Matches: true, }, @@ -318,6 +324,9 @@ func TestProxyDefaults_ToConsul(t *testing.T) { Mode: "sequential", Regions: []string{"us-west-1"}, }, + PrioritizeByLocality: &PrioritizeByLocality{ + Mode: "none", + }, }, }, Exp: &capi.ProxyConfigEntry{ @@ -382,6 +391,9 @@ func TestProxyDefaults_ToConsul(t *testing.T) { Mode: "sequential", Regions: []string{"us-west-1"}, }, + PrioritizeByLocality: &capi.ServiceResolverPrioritizeByLocality{ + Mode: "none", + }, Meta: map[string]string{ common.SourceKey: common.SourceValue, common.DatacenterKey: "datacenter", @@ -652,6 +664,19 @@ func TestProxyDefaults_Validate(t *testing.T) { }, expectedErrMsg: `proxydefaults.consul.hashicorp.com "global" is invalid: spec.failoverPolicy.mode: Invalid value: "wrong-mode": must be one of "", "sequential", "order-by-locality"`, }, + "prioritize by locality invalid": { + input: &ProxyDefaults{ + ObjectMeta: metav1.ObjectMeta{ + Name: "global", + }, + Spec: ProxyDefaultsSpec{ + PrioritizeByLocality: &PrioritizeByLocality{ + Mode: "wrong-mode", + }, + }, + }, + expectedErrMsg: `proxydefaults.consul.hashicorp.com "global" is invalid: spec.prioritizeByLocality.mode: Invalid value: "wrong-mode": must be one of "", "none", "failover"`, + }, "multi-error": { input: &ProxyDefaults{ ObjectMeta: metav1.ObjectMeta{ diff --git a/control-plane/api/v1alpha1/serviceresolver_types.go b/control-plane/api/v1alpha1/serviceresolver_types.go index 714cd94c2a..3a8e907222 100644 --- a/control-plane/api/v1alpha1/serviceresolver_types.go +++ b/control-plane/api/v1alpha1/serviceresolver_types.go @@ -84,14 +84,7 @@ type ServiceResolverSpec struct { LoadBalancer *LoadBalancer `json:"loadBalancer,omitempty"` // PrioritizeByLocality controls whether the locality of services within the // local partition will be used to prioritize connectivity. - PrioritizeByLocality *ServiceResolverPrioritizeByLocality `json:"prioritizeByLocality,omitempty"` -} - -type ServiceResolverPrioritizeByLocality struct { - // Mode specifies the type of prioritization that will be performed - // when selecting nodes in the local partition. - // Valid values are: "" (default "none"), "none", and "failover". - Mode string `json:"mode,omitempty"` + PrioritizeByLocality *PrioritizeByLocality `json:"prioritizeByLocality,omitempty"` } type ServiceResolverRedirect struct { @@ -536,16 +529,6 @@ func (in *ServiceResolverFailover) toConsul() *capi.ServiceResolverFailover { } } -func (in *ServiceResolverPrioritizeByLocality) toConsul() *capi.ServiceResolverPrioritizeByLocality { - if in == nil { - return nil - } - - return &capi.ServiceResolverPrioritizeByLocality{ - Mode: in.Mode, - } -} - func (in ServiceResolverFailoverTarget) toConsul() capi.ServiceResolverFailoverTarget { return capi.ServiceResolverFailoverTarget{ Service: in.Service, @@ -655,25 +638,6 @@ func (in *ServiceResolverFailover) isEmpty() bool { return in.Service == "" && in.ServiceSubset == "" && in.Namespace == "" && len(in.Datacenters) == 0 && len(in.Targets) == 0 && in.Policy == nil && in.SamenessGroup == "" } -func (in *ServiceResolverPrioritizeByLocality) validate(path *field.Path) field.ErrorList { - var errs field.ErrorList - - if in == nil { - return nil - } - - switch in.Mode { - case "": - case "none": - case "failover": - default: - asJSON, _ := json.Marshal(in) - errs = append(errs, field.Invalid(path, string(asJSON), - "mode must be one of '', 'none', or 'failover'")) - } - return errs -} - func (in *ServiceResolverFailover) validate(path *field.Path, consulMeta common.ConsulMeta) field.ErrorList { var errs field.ErrorList if in.isEmpty() { diff --git a/control-plane/api/v1alpha1/serviceresolver_types_test.go b/control-plane/api/v1alpha1/serviceresolver_types_test.go index 11751aaa2a..3fbc2b4fce 100644 --- a/control-plane/api/v1alpha1/serviceresolver_types_test.go +++ b/control-plane/api/v1alpha1/serviceresolver_types_test.go @@ -66,7 +66,7 @@ func TestServiceResolver_MatchesConsul(t *testing.T) { Datacenter: "redirect_datacenter", Peer: "redirect_peer", }, - PrioritizeByLocality: &ServiceResolverPrioritizeByLocality{ + PrioritizeByLocality: &PrioritizeByLocality{ Mode: "failover", }, Failover: map[string]ServiceResolverFailover{ @@ -285,7 +285,7 @@ func TestServiceResolver_ToConsul(t *testing.T) { Datacenter: "redirect_datacenter", Partition: "redirect_partition", }, - PrioritizeByLocality: &ServiceResolverPrioritizeByLocality{ + PrioritizeByLocality: &PrioritizeByLocality{ Mode: "none", }, Failover: map[string]ServiceResolverFailover{ @@ -898,20 +898,20 @@ func TestServiceResolver_Validate(t *testing.T) { "spec.failover[failB].namespace: Invalid value: \"namespace-b\": Consul Enterprise namespaces must be enabled to set failover.namespace", }, }, - "prioritize by locality none": { + "prioritize by locality invalid": { input: &ServiceResolver{ ObjectMeta: metav1.ObjectMeta{ Name: "foo", }, Spec: ServiceResolverSpec{ - PrioritizeByLocality: &ServiceResolverPrioritizeByLocality{ + PrioritizeByLocality: &PrioritizeByLocality{ Mode: "bad", }, }, }, namespacesEnabled: false, expectedErrMsgs: []string{ - "mode must be one of '', 'none', or 'failover'", + "serviceresolver.consul.hashicorp.com \"foo\" is invalid: spec.prioritizeByLocality.mode: Invalid value: \"bad\": must be one of \"\", \"none\", \"failover\"", }, }, } diff --git a/control-plane/api/v1alpha1/shared_types.go b/control-plane/api/v1alpha1/shared_types.go index aa19c339da..0b1ec74d93 100644 --- a/control-plane/api/v1alpha1/shared_types.go +++ b/control-plane/api/v1alpha1/shared_types.go @@ -310,6 +310,35 @@ func (in *FailoverPolicy) validate(path *field.Path) field.ErrorList { return errs } +type PrioritizeByLocality struct { + // Mode specifies the type of prioritization that will be performed + // when selecting nodes in the local partition. + // Valid values are: "" (default "none"), "none", and "failover". + Mode string `json:"mode,omitempty"` +} + +func (in *PrioritizeByLocality) toConsul() *capi.ServiceResolverPrioritizeByLocality { + if in == nil { + return nil + } + + return &capi.ServiceResolverPrioritizeByLocality{ + Mode: in.Mode, + } +} + +func (in *PrioritizeByLocality) validate(path *field.Path) field.ErrorList { + var errs field.ErrorList + if in == nil { + return nil + } + modes := []string{"", "none", "failover"} + if !sliceContains(modes, in.Mode) { + errs = append(errs, field.Invalid(path.Child("mode"), in.Mode, notInSliceMessage(modes))) + } + return errs +} + func notInSliceMessage(slice []string) string { return fmt.Sprintf(`must be one of "%s"`, strings.Join(slice, `", "`)) } diff --git a/control-plane/api/v1alpha1/zz_generated.deepcopy.go b/control-plane/api/v1alpha1/zz_generated.deepcopy.go index 05000031ad..5de7c61c4e 100644 --- a/control-plane/api/v1alpha1/zz_generated.deepcopy.go +++ b/control-plane/api/v1alpha1/zz_generated.deepcopy.go @@ -1947,6 +1947,21 @@ func (in *PeeringMeshConfig) DeepCopy() *PeeringMeshConfig { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PrioritizeByLocality) DeepCopyInto(out *PrioritizeByLocality) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PrioritizeByLocality. +func (in *PrioritizeByLocality) DeepCopy() *PrioritizeByLocality { + if in == nil { + return nil + } + out := new(PrioritizeByLocality) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ProxyDefaults) DeepCopyInto(out *ProxyDefaults) { *out = *in @@ -2043,6 +2058,11 @@ func (in *ProxyDefaultsSpec) DeepCopyInto(out *ProxyDefaultsSpec) { *out = new(FailoverPolicy) (*in).DeepCopyInto(*out) } + if in.PrioritizeByLocality != nil { + in, out := &in.PrioritizeByLocality, &out.PrioritizeByLocality + *out = new(PrioritizeByLocality) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ProxyDefaultsSpec. @@ -2618,21 +2638,6 @@ func (in *ServiceResolverList) DeepCopyObject() runtime.Object { return nil } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *ServiceResolverPrioritizeByLocality) DeepCopyInto(out *ServiceResolverPrioritizeByLocality) { - *out = *in -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ServiceResolverPrioritizeByLocality. -func (in *ServiceResolverPrioritizeByLocality) DeepCopy() *ServiceResolverPrioritizeByLocality { - if in == nil { - return nil - } - out := new(ServiceResolverPrioritizeByLocality) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ServiceResolverRedirect) DeepCopyInto(out *ServiceResolverRedirect) { *out = *in @@ -2679,7 +2684,7 @@ func (in *ServiceResolverSpec) DeepCopyInto(out *ServiceResolverSpec) { } if in.PrioritizeByLocality != nil { in, out := &in.PrioritizeByLocality, &out.PrioritizeByLocality - *out = new(ServiceResolverPrioritizeByLocality) + *out = new(PrioritizeByLocality) **out = **in } } diff --git a/control-plane/config/crd/bases/consul.hashicorp.com_proxydefaults.yaml b/control-plane/config/crd/bases/consul.hashicorp.com_proxydefaults.yaml index 7084980bf0..25e61a51ee 100644 --- a/control-plane/config/crd/bases/consul.hashicorp.com_proxydefaults.yaml +++ b/control-plane/config/crd/bases/consul.hashicorp.com_proxydefaults.yaml @@ -156,6 +156,15 @@ spec: type: string type: array type: object + prioritizeByLocality: + description: PrioritizeByLocality contains the configuration for + locality aware routing. + properties: + mode: + description: Mode specifies the behavior of PrioritizeByLocality + routing. Valid values are "", "none", and "failover". + type: string + type: object meshGateway: description: MeshGateway controls the default mesh gateway configuration for this service.