From 26a34921551a946993a77172ba9c1e81a25d77d7 Mon Sep 17 00:00:00 2001 From: Megrez Lu Date: Fri, 15 Mar 2024 18:39:33 +0800 Subject: [PATCH 1/7] add proposal for path and queryParams support Signed-off-by: Megrez Lu --- docs/proposals/20240315-pathandqueryparams.md | 152 ++++++++++++++++++ 1 file changed, 152 insertions(+) create mode 100644 docs/proposals/20240315-pathandqueryparams.md diff --git a/docs/proposals/20240315-pathandqueryparams.md b/docs/proposals/20240315-pathandqueryparams.md new file mode 100644 index 0000000000..038467dda1 --- /dev/null +++ b/docs/proposals/20240315-pathandqueryparams.md @@ -0,0 +1,152 @@ +--- +title: Add Path and QueryParams support to trafficRoutings +authors: + - "@lujiajing1126" +reviewers: + - "@YYY" +creation-date: 2024-03-15 +last-updated: 2024-03-15 +status: implementable +--- + +# Add Path and QueryParams support to trafficRoutings + +Support Path and QueryParams as HttpMatch conditions. + +## Table of Contents + +A table of contents is helpful for quickly jumping to sections of a proposal and for highlighting +any additional information provided beyond the standard proposal template. +[Tools for generating](https://github.com/ekalinin/github-markdown-toc) a table of contents from markdown are available. + +- [Add Path and QueryParams support to trafficRoutings](#title) + - [Table of Contents](#table-of-contents) + - [Motivation](#motivation) + - [Goals](#goals) + - [Non-Goals/Future Work](#non-goalsfuture-work) + - [Proposal](#proposal) + - [User Stories](#user-stories) + - [Story 1](#story-1) + - [Story 2](#story-2) + - [Requirements (Optional)](#requirements-optional) + - [Functional Requirements](#functional-requirements) + - [FR1](#fr1) + - [FR2](#fr2) + - [Non-Functional Requirements](#non-functional-requirements) + - [NFR1](#nfr1) + - [NFR2](#nfr2) + - [Implementation Details/Notes/Constraints](#implementation-detailsnotesconstraints) + - [Risks and Mitigations](#risks-and-mitigations) + - [Alternatives](#alternatives) + - [Upgrade Strategy](#upgrade-strategy) + - [Additional Details](#additional-details) + - [Test Plan [optional]](#test-plan-optional) + - [Implementation History](#implementation-history) + +## Motivation + +So far, OpenKruise Rollouts simply supports `Header` as the matchers for traffic routing. Path and QueryParams are also commonly used for traffic routing in A/B testing scenarios, + +- Path can be used for A/B testing at API/RPC level, +- QueryParams can be used in A/B testing for HTMLs and other cases where Header is not under control + +### Goals + +- Support Path and/or QueryParams for Gateway API, Istio Mesh and Ingress (Specfically, MSE and Gateway API) + +### Non-Goals/Future Work + +- `SourceLabels` and `SourceNamespace` are also commonly used, but only applicable to interservice communication. It can be left in the future. + +## Proposal + +The following fields are necessary in the `HttpRouteMatch` struct, + +| FieldName | Type | Description | +| --------- | ---- | ----------- | +| Path | *gatewayv1beta1.HTTPPathMatch | Path specifies a HTTP request path matcher. | +| QueryParams | []gatewayv1beta1.HTTPQueryParamMatch | QueryParams specifies HTTP query parameter matchers. Multiple match values are ANDed together, meaning, a request must match all the specified query parameters to select the route. | + +```go +type HttpRouteMatch struct { + // New + + // Path specifies a HTTP request path matcher. + // Supported list: + // - Istio: https://istio.io/latest/docs/reference/config/networking/virtual-service/#HTTPMatchRequest + // + // +optional + Path *gatewayv1beta1.HTTPPathMatch `json:"path,omitempty"` + + // QueryParams specifies HTTP query parameter matchers. Multiple match + // values are ANDed together, meaning, a request must match all the + // specified query parameters to select the route. + // Supported list: + // - Istio: https://istio.io/latest/docs/reference/config/networking/virtual-service/#HTTPMatchRequest + // - MSE Ingress: https://help.aliyun.com/zh/ack/ack-managed-and-ack-dedicated/user-guide/annotations-supported-by-mse-ingress-gateways-1 + // Header/Cookie > QueryParams + // - Gateway API + // + // +listType=map + // +listMapKey=name + // +optional + // +kubebuilder:validation:MaxItems=16 + QueryParams []gatewayv1beta1.HTTPQueryParamMatch `json:"queryParams,omitempty"` + + // Existing + + // Headers specifies HTTP request header matchers. Multiple match values are + // ANDed together, meaning, a request must match all the specified headers + // to select the route. + // + // +listType=map + // +listMapKey=name + // +optional + // +kubebuilder:validation:MaxItems=16 + Headers []gatewayv1beta1.HTTPHeaderMatch `json:"headers,omitempty"` +} +``` + +Matches define conditions used to match incoming HTTP requests to the canary service. Each match condition may contain serveral conditions as children which are independent of each other. + +For Gateway API, only a single match rule will be applied since Gateway API use ANDed rules if multiple ones are defined, i.e. the match will evaluate to be true only if all conditions are satisfied. Priority: Header > QueryParams, for backwards-compatibility. + +Only one of the `weight` and `matches` will come into effect. If both fields are configured, then matches takes precedence. + +### User Stories + +Currently, only one trafficRouting provider is supported. Then we may consider them case by case, + +- MSE Ingress: User can define queryParams, but it **SHOULD NOT** be used together with Headers due to priority and backward-compatibility. +- Istio: User can define path and/or queryParams and use without any issue. +- Gateway API: User can define path and/or queryParams. But one of them can take effect if and only if Headers are not defined. +- + +### Implementation Details/Notes/Constraints + +#### MSE Ingress + +Due to backward-compatibility, if both Headers and QueryParams are defined, Headers should be used. + +Then according to the documentation of [MSE Ingress](https://help.aliyun.com/zh/ack/ack-managed-and-ack-dedicated/user-guide/advanced-usage-of-mse-ingress?spm=a2c4g.11186623.0.0.58a26dc4q4GCmn#p-qar-ac2-lvw), if Header and Query Parameter are combined, both conditions MUST be fulfilled. + +So the priority in Rollouts is Header(Cookie) > QueryParams. + +#### Gateway API + +Since Gateway API uses logical AND for all matches, only a single match condition will be selected. + +For backwards-comptability, the priority in Rollouts is Header(Cookie) > Path > QueryParams. + +## Additional Details + +### Test Plan [optional] + +## Implementation History + +- [ ] MM/DD/YYYY: Proposed idea in an issue or [community meeting] +- [ ] MM/DD/YYYY: Compile a Google Doc following the CAEP template (link here) +- [ ] MM/DD/YYYY: First round of feedback from community +- [ ] MM/DD/YYYY: Present proposal at a [community meeting] +- [ ] MM/DD/YYYY: Open proposal PR + From 8f2314d5d0c419363f66c888c4d68fba33cd5afd Mon Sep 17 00:00:00 2001 From: Megrez Lu Date: Fri, 15 Mar 2024 18:43:05 +0800 Subject: [PATCH 2/7] polish toc Signed-off-by: Megrez Lu --- docs/proposals/20240315-pathandqueryparams.md | 20 +++---------------- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/docs/proposals/20240315-pathandqueryparams.md b/docs/proposals/20240315-pathandqueryparams.md index 038467dda1..7af525b7be 100644 --- a/docs/proposals/20240315-pathandqueryparams.md +++ b/docs/proposals/20240315-pathandqueryparams.md @@ -15,30 +15,16 @@ Support Path and QueryParams as HttpMatch conditions. ## Table of Contents -A table of contents is helpful for quickly jumping to sections of a proposal and for highlighting -any additional information provided beyond the standard proposal template. -[Tools for generating](https://github.com/ekalinin/github-markdown-toc) a table of contents from markdown are available. - -- [Add Path and QueryParams support to trafficRoutings](#title) +- [Add Path and QueryParams support to trafficRoutings](#add-path-and-queryparams-support-to-trafficroutings) - [Table of Contents](#table-of-contents) - [Motivation](#motivation) - [Goals](#goals) - [Non-Goals/Future Work](#non-goalsfuture-work) - [Proposal](#proposal) - [User Stories](#user-stories) - - [Story 1](#story-1) - - [Story 2](#story-2) - - [Requirements (Optional)](#requirements-optional) - - [Functional Requirements](#functional-requirements) - - [FR1](#fr1) - - [FR2](#fr2) - - [Non-Functional Requirements](#non-functional-requirements) - - [NFR1](#nfr1) - - [NFR2](#nfr2) - [Implementation Details/Notes/Constraints](#implementation-detailsnotesconstraints) - - [Risks and Mitigations](#risks-and-mitigations) - - [Alternatives](#alternatives) - - [Upgrade Strategy](#upgrade-strategy) + - [MSE Ingress](#mse-ingress) + - [Gateway API](#gateway-api) - [Additional Details](#additional-details) - [Test Plan [optional]](#test-plan-optional) - [Implementation History](#implementation-history) From 30ad52bd1dbc8dd0dcf35899884c896d4b79eb14 Mon Sep 17 00:00:00 2001 From: Megrez Lu Date: Fri, 15 Mar 2024 18:47:43 +0800 Subject: [PATCH 3/7] fix typo and polish Signed-off-by: Megrez Lu --- docs/proposals/20240315-pathandqueryparams.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/proposals/20240315-pathandqueryparams.md b/docs/proposals/20240315-pathandqueryparams.md index 7af525b7be..7b0fb0eac9 100644 --- a/docs/proposals/20240315-pathandqueryparams.md +++ b/docs/proposals/20240315-pathandqueryparams.md @@ -38,7 +38,7 @@ So far, OpenKruise Rollouts simply supports `Header` as the matchers for traffic ### Goals -- Support Path and/or QueryParams for Gateway API, Istio Mesh and Ingress (Specfically, MSE and Gateway API) +- Support Path and/or QueryParams for Gateway API, Istio Mesh and Ingress (Specifically, MSE and Gateway API) ### Non-Goals/Future Work @@ -81,7 +81,7 @@ type HttpRouteMatch struct { // Existing - // Headers specifies HTTP request header matchers. Multiple match values are + // Headers specifies HTTP request header matchers. Multiple match values are // ANDed together, meaning, a request must match all the specified headers // to select the route. // @@ -106,7 +106,6 @@ Currently, only one trafficRouting provider is supported. Then we may consider t - MSE Ingress: User can define queryParams, but it **SHOULD NOT** be used together with Headers due to priority and backward-compatibility. - Istio: User can define path and/or queryParams and use without any issue. - Gateway API: User can define path and/or queryParams. But one of them can take effect if and only if Headers are not defined. -- ### Implementation Details/Notes/Constraints From 02719934ba73ae119448fc30310a9eb49b17b509 Mon Sep 17 00:00:00 2001 From: Megrez Lu Date: Mon, 18 Mar 2024 11:46:31 +0800 Subject: [PATCH 4/7] fix typo Signed-off-by: Megrez Lu --- docs/proposals/20240315-pathandqueryparams.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/proposals/20240315-pathandqueryparams.md b/docs/proposals/20240315-pathandqueryparams.md index 7b0fb0eac9..6ca957a6af 100644 --- a/docs/proposals/20240315-pathandqueryparams.md +++ b/docs/proposals/20240315-pathandqueryparams.md @@ -121,7 +121,7 @@ So the priority in Rollouts is Header(Cookie) > QueryParams. Since Gateway API uses logical AND for all matches, only a single match condition will be selected. -For backwards-comptability, the priority in Rollouts is Header(Cookie) > Path > QueryParams. +For backwards-compatibility, the priority in Rollouts is Header(Cookie) > Path > QueryParams. ## Additional Details From b54109405c83842a1c05a75bdd34ef8f804597ab Mon Sep 17 00:00:00 2001 From: Megrez Lu Date: Mon, 18 Mar 2024 12:10:09 +0800 Subject: [PATCH 5/7] fix typo Signed-off-by: Megrez Lu --- docs/proposals/20240315-pathandqueryparams.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/proposals/20240315-pathandqueryparams.md b/docs/proposals/20240315-pathandqueryparams.md index 6ca957a6af..1ed8a36837 100644 --- a/docs/proposals/20240315-pathandqueryparams.md +++ b/docs/proposals/20240315-pathandqueryparams.md @@ -93,7 +93,7 @@ type HttpRouteMatch struct { } ``` -Matches define conditions used to match incoming HTTP requests to the canary service. Each match condition may contain serveral conditions as children which are independent of each other. +Matches define conditions used to match incoming HTTP requests to the canary service. Each match condition may contain several conditions as children which are independent of each other. For Gateway API, only a single match rule will be applied since Gateway API use ANDed rules if multiple ones are defined, i.e. the match will evaluate to be true only if all conditions are satisfied. Priority: Header > QueryParams, for backwards-compatibility. From 08e238ddee7af3e17748444c1e6095f62a00bc41 Mon Sep 17 00:00:00 2001 From: Megrez Lu Date: Wed, 15 May 2024 15:56:19 +0800 Subject: [PATCH 6/7] update semantics Signed-off-by: Megrez Lu --- docs/proposals/20240315-pathandqueryparams.md | 47 +++++++++---------- 1 file changed, 21 insertions(+), 26 deletions(-) diff --git a/docs/proposals/20240315-pathandqueryparams.md b/docs/proposals/20240315-pathandqueryparams.md index 1ed8a36837..33df8a79ed 100644 --- a/docs/proposals/20240315-pathandqueryparams.md +++ b/docs/proposals/20240315-pathandqueryparams.md @@ -5,7 +5,7 @@ authors: reviewers: - "@YYY" creation-date: 2024-03-15 -last-updated: 2024-03-15 +last-updated: 2024-05-15 status: implementable --- @@ -60,10 +60,22 @@ type HttpRouteMatch struct { // Path specifies a HTTP request path matcher. // Supported list: // - Istio: https://istio.io/latest/docs/reference/config/networking/virtual-service/#HTTPMatchRequest + // - GatewayAPI: If path is defined, the whole HttpRouteMatch will be used as a standalone matcher // // +optional Path *gatewayv1beta1.HTTPPathMatch `json:"path,omitempty"` + // Headers specifies HTTP request header matchers. Multiple match values are + // ANDed together, meaning, a request must match all the specified headers + // to select the route. + // + // +listType=map + // +listMapKey=name + // +optional + // +kubebuilder:validation:MaxItems=16 + Headers []gatewayv1beta1.HTTPHeaderMatch `json:"headers,omitempty"` + + // Existing // QueryParams specifies HTTP query parameter matchers. Multiple match // values are ANDed together, meaning, a request must match all the // specified query parameters to select the route. @@ -78,50 +90,33 @@ type HttpRouteMatch struct { // +optional // +kubebuilder:validation:MaxItems=16 QueryParams []gatewayv1beta1.HTTPQueryParamMatch `json:"queryParams,omitempty"` - - // Existing - - // Headers specifies HTTP request header matchers. Multiple match values are - // ANDed together, meaning, a request must match all the specified headers - // to select the route. - // - // +listType=map - // +listMapKey=name - // +optional - // +kubebuilder:validation:MaxItems=16 - Headers []gatewayv1beta1.HTTPHeaderMatch `json:"headers,omitempty"` } ``` -Matches define conditions used to match incoming HTTP requests to the canary service. Each match condition may contain several conditions as children which are independent of each other. +`Matches` defines conditions used to match incoming HTTP requests to the canary service. Each match condition may contain several criterias as children which are independent of each other. -For Gateway API, only a single match rule will be applied since Gateway API use ANDed rules if multiple ones are defined, i.e. the match will evaluate to be true only if all conditions are satisfied. Priority: Header > QueryParams, for backwards-compatibility. - -Only one of the `weight` and `matches` will come into effect. If both fields are configured, then matches takes precedence. +Only one of the `weight` and `matches` will come into effect. If both fields are configured, then `matches` takes precedence. ### User Stories Currently, only one trafficRouting provider is supported. Then we may consider them case by case, -- MSE Ingress: User can define queryParams, but it **SHOULD NOT** be used together with Headers due to priority and backward-compatibility. +- MSE Ingress: User can define queryParams together with headers. But due to priority of MSE ingress itself, Headers & QueryParams > Cookie > Weight. - Istio: User can define path and/or queryParams and use without any issue. -- Gateway API: User can define path and/or queryParams. But one of them can take effect if and only if Headers are not defined. +- Gateway API: User can define path, headers and/or queryParams. But path follows a special rule which will be described in details below. ### Implementation Details/Notes/Constraints #### MSE Ingress -Due to backward-compatibility, if both Headers and QueryParams are defined, Headers should be used. - -Then according to the documentation of [MSE Ingress](https://help.aliyun.com/zh/ack/ack-managed-and-ack-dedicated/user-guide/advanced-usage-of-mse-ingress?spm=a2c4g.11186623.0.0.58a26dc4q4GCmn#p-qar-ac2-lvw), if Header and Query Parameter are combined, both conditions MUST be fulfilled. - -So the priority in Rollouts is Header(Cookie) > QueryParams. +According to the documentation of [MSE Ingress](https://help.aliyun.com/zh/ack/ack-managed-and-ack-dedicated/user-guide/advanced-usage-of-mse-ingress?spm=a2c4g.11186623.0.0.58a26dc4q4GCmn#p-qar-ac2-lvw), if Header and Query Parameter are combined, both conditions MUST be fulfilled. #### Gateway API -Since Gateway API uses logical AND for all matches, only a single match condition will be selected. +Considering the Gateway API, all `HttpRouteMatch`es defined in the `matches` field will be applied to the existing rules from the Gateway API one by one if path is not used. +For example, M `HttpRouteMatch`es (without `Path`) with N `HTTPRouteRule`s will generate M x N (Cartesian product) `HTTPRouteRule`s to the canary service. -For backwards-compatibility, the priority in Rollouts is Header(Cookie) > Path > QueryParams. +If path is defined in the `HttpRouteMatch`, the whole `HttpRouteMatch` will be directly mapped as a `HTTPRouteRule` in the Gateway API to avoid potential path conflict. ## Additional Details From 13ea328364a87358a0025edab5fe3e19f1292cce Mon Sep 17 00:00:00 2001 From: Megrez Lu Date: Wed, 15 May 2024 16:27:55 +0800 Subject: [PATCH 7/7] fix typo Signed-off-by: Megrez Lu --- docs/proposals/20240315-pathandqueryparams.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/proposals/20240315-pathandqueryparams.md b/docs/proposals/20240315-pathandqueryparams.md index 33df8a79ed..72b11afb1d 100644 --- a/docs/proposals/20240315-pathandqueryparams.md +++ b/docs/proposals/20240315-pathandqueryparams.md @@ -93,7 +93,7 @@ type HttpRouteMatch struct { } ``` -`Matches` defines conditions used to match incoming HTTP requests to the canary service. Each match condition may contain several criterias as children which are independent of each other. +`Matches` defines conditions used to match incoming HTTP requests to the canary service. Each match condition may contain several criteria as children which are independent of each other. Only one of the `weight` and `matches` will come into effect. If both fields are configured, then `matches` takes precedence.