From 652739fd17c04d17a80104116f6641599777e3f2 Mon Sep 17 00:00:00 2001 From: Mike Beaumont Date: Wed, 25 Sep 2024 17:44:42 +0200 Subject: [PATCH 1/4] fix(MeshLoadBalancingStrategy): only allow MeshGateway and to.targetRef.kind: Mesh if using loadBalancer Signed-off-by: Mike Beaumont --- pkg/core/resources/apis/mesh/validators.go | 7 ++++- .../api/v1alpha1/validator.go | 29 ++++++++++++++++--- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/pkg/core/resources/apis/mesh/validators.go b/pkg/core/resources/apis/mesh/validators.go index 42481d889237..7e5729a87cdf 100644 --- a/pkg/core/resources/apis/mesh/validators.go +++ b/pkg/core/resources/apis/mesh/validators.go @@ -50,6 +50,7 @@ type ValidateSelectorsOpts struct { type ValidateTargetRefOpts struct { SupportedKinds []common_api.TargetRefKind + SupportedKindsError string GatewayListenerTagsAllowed bool // AllowedInvalidNames field allows to provide names that deviate from // standard naming conventions in specific scenarios. I.e. normally, @@ -357,7 +358,11 @@ func ValidateTargetRef( return err } if !slices.Contains(opts.SupportedKinds, ref.Kind) { - err.AddViolation("kind", "value is not supported") + errMsg := "value is not supported" + if optsErr := opts.SupportedKindsError; optsErr != "" { + errMsg = optsErr + } + err.AddViolation("kind", errMsg) return err } diff --git a/pkg/plugins/policies/meshloadbalancingstrategy/api/v1alpha1/validator.go b/pkg/plugins/policies/meshloadbalancingstrategy/api/v1alpha1/validator.go index a567b37aaa47..7ce58718a099 100644 --- a/pkg/plugins/policies/meshloadbalancingstrategy/api/v1alpha1/validator.go +++ b/pkg/plugins/policies/meshloadbalancingstrategy/api/v1alpha1/validator.go @@ -43,13 +43,34 @@ func validateTo(topTargetRef common_api.TargetRef, to []To) validators.Validatio var verr validators.ValidationError for idx, toItem := range to { path := validators.RootedAt("to").Index(idx) - verr.AddErrorAt(path.Field("targetRef"), mesh.ValidateTargetRef(toItem.TargetRef, &mesh.ValidateTargetRefOpts{ - SupportedKinds: []common_api.TargetRefKind{ + var supportedKinds []common_api.TargetRefKind + var supportedKindsError string + switch topTargetRef.Kind { + case common_api.MeshGateway: + if toItem.Default.LoadBalancer != nil { + supportedKindsError = fmt.Sprintf("value is not supported, only %s is allowed if loadBalancer is set", common_api.Mesh) + supportedKinds = []common_api.TargetRefKind{ + common_api.Mesh, + } + } else { + supportedKinds = []common_api.TargetRefKind{ + common_api.Mesh, + common_api.MeshService, + common_api.MeshMultiZoneService, + } + } + default: + supportedKinds = []common_api.TargetRefKind{ common_api.Mesh, common_api.MeshService, common_api.MeshMultiZoneService, - }, - })) + } + } + errs := mesh.ValidateTargetRef(toItem.TargetRef, &mesh.ValidateTargetRefOpts{ + SupportedKinds: supportedKinds, + SupportedKindsError: supportedKindsError, + }) + verr.AddErrorAt(path.Field("targetRef"), errs) if toItem.TargetRef.Kind == common_api.MeshExternalService && topTargetRef.Kind != common_api.Mesh { verr.AddViolationAt(path.Field("targetRef.kind"), "kind MeshExternalService is only allowed with targetRef.kind: Mesh as it is configured on the Zone Egress and shared by all clients in the mesh") } From 2a0d70a24c345ea6a07c26686625493ef33e9da6 Mon Sep 17 00:00:00 2001 From: Mike Beaumont Date: Wed, 25 Sep 2024 17:45:03 +0200 Subject: [PATCH 2/4] test(MeshLoadBalancingStrategy): add newly invalid case, adjust valid case Signed-off-by: Mike Beaumont --- .../api/v1alpha1/validator_test.go | 27 ++++++++++++++++--- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/pkg/plugins/policies/meshloadbalancingstrategy/api/v1alpha1/validator_test.go b/pkg/plugins/policies/meshloadbalancingstrategy/api/v1alpha1/validator_test.go index 31d877c4c0fb..1f48f5a3afb6 100644 --- a/pkg/plugins/policies/meshloadbalancingstrategy/api/v1alpha1/validator_test.go +++ b/pkg/plugins/policies/meshloadbalancingstrategy/api/v1alpha1/validator_test.go @@ -425,6 +425,29 @@ to: - to: type: AnyExcept +`), + ErrorCases( + "invalid MeshGateway and to MeshService", + []validators.Violation{{ + Field: "spec.to[0].targetRef.kind", + Message: "value is not supported, only Mesh is allowed if loadBalancer is set", + }}, + ` +type: MeshLoadBalancingStrategy +mesh: mesh-1 +name: route-1 +targetRef: + kind: MeshGateway + name: edge-gateway +to: + - targetRef: + kind: MeshService + name: svc-1 + default: + loadBalancer: + type: LeastRequest + leastRequest: + activeRequestBias: "1.3" `), ) @@ -535,10 +558,6 @@ to: default: localityAwareness: disabled: true - loadBalancer: - type: LeastRequest - leastRequest: - activeRequestBias: "1.3" `), XEntry( "to MeshExternalService", From 5027c671d1858048048d6af6244eefb84a1274ee Mon Sep 17 00:00:00 2001 From: Mike Beaumont Date: Thu, 26 Sep 2024 16:32:42 +0200 Subject: [PATCH 3/4] test(e2e): fix MLBS in MGW test Signed-off-by: Mike Beaumont --- test/e2e_env/kubernetes/gateway/gateway.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/e2e_env/kubernetes/gateway/gateway.go b/test/e2e_env/kubernetes/gateway/gateway.go index 9851e972efc2..a48f42add872 100644 --- a/test/e2e_env/kubernetes/gateway/gateway.go +++ b/test/e2e_env/kubernetes/gateway/gateway.go @@ -548,8 +548,7 @@ spec: name: simple-gateway to: - targetRef: - kind: MeshService - name: test-server-mlbs_%[2]s_svc_80 + kind: Mesh default: loadBalancer: type: RingHash From 6b58526869c65bc890ccf8f1b9b316706f742653 Mon Sep 17 00:00:00 2001 From: Mike Beaumont Date: Thu, 26 Sep 2024 17:14:16 +0200 Subject: [PATCH 4/4] docs(UPGRADE.md): add note Signed-off-by: Mike Beaumont --- UPGRADE.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/UPGRADE.md b/UPGRADE.md index d6bc37175eb9..8a46c4672c5f 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -14,6 +14,12 @@ Policies targeting `spec.targetRef.kind: MeshGateway` can now only target `kind: `to[].targetRef`. Previously MeshService, MeshExternalService, MeshMultiZoneService were allowed but the resulting configuration was ambiguous and nondeterministic. +### MeshLoadBalancingStrategy + +Policies targeting `spec.targetRef.kind: MeshGateway` and setting the `spec.loadBalancer` field can now only target `kind: Mesh` in +`to[].targetRef`. Previously MeshService, MeshExternalService, MeshMultiZoneService were allowed but the resulting configuration +was ambiguous and nondeterministic. + ### MeshExternalService #### Removal of unix sockets support