envoyproxy / envoy

Cloud-native high-performance edge/middle/service proxy
https://www.envoyproxy.io
Apache License 2.0
24.87k stars 4.78k forks source link

Matcher-based route config should support `runtime_fraction` #24239

Open euroelessar opened 1 year ago

euroelessar commented 1 year ago

Description: Currently XDS matcher in route configuration context can route based on headers only, but routing based on runtime_fraction is desired for gradual rollouts.

One way to implement is to expose streamId (or randomId) in HttpMatchingData alongside requestHeaders: https://github.com/envoyproxy/envoy/blob/4e40d6ad5f98cd3c86f1398bb8990b1d88bdfe92/envoy/http/filter.h#L1078-L1084

Then add RuntimeFractionMatchInput in addition to HttpRequestHeaderMatchInput, smth like:

message RuntimeMatchInput {
  // Sets input to `true` if runtime key is enabled and `false` otherwise.
  core.v3.RuntimeFractionalPercent runtime_fraction = 1;
}

And allow it as an input source for HTTP route configuration matcher (and maybe others as well, like RBAC, as I assume it also may want to have gradual rollout functionality).

Is it something which is acceptable to add to envoy?

Relevant Links: https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/http/http_routing.html#routing-via-generic-matching

euroelessar commented 1 year ago

Followup on discussion in #6602, cc: @mattklein123 @snowp @kyessenov

mattklein123 commented 1 year ago

This makes sense to me.

llu94 commented 1 year ago

I'd like to work on this unless @euroelessar is already working on this?

euroelessar commented 1 year ago

@llu94 hi, we haven't started working on it yet, so feel free to grab it do you have a timeline in mind when you will be able to make the change?

llu94 commented 1 year ago

@euroelessar I was thinking to get this done before February. Let me know if that time line is too slow for you guys

euroelessar commented 1 year ago

So, it looks like we have to split the functionality in two pieces as HttpDataInput itself has no access to Runtime::Loader. So, I suggest to expose randomness as a separate field of HttpMatchingData.randomValue() with corresponding data input factory (not sure if it should be an extension or part of the binary?) and expose runtime's featureEnabled logic via protocol-independent data matcher. As a bonus it allows codifying logic like "enable this part of the matching tree for a dynamically defined ratio of source ip addresses" and similar.

euroelessar commented 1 year ago

It looks like both TCP and HTTP layers have a notion of stream id, so we can add it to data input for both of them.

@snowp @mattklein123 Hi, any thoughts on where to put this new input factory? Piggyback into source/extensions/matching/network/common, add some new extension, put somewhere else?

kyessenov commented 1 year ago

Yeah, I'd be fine with that directory. TBH we need to re-organize the directory structure anyways, we have three somewhat overlapping places: