api icon indicating copy to clipboard operation
api copied to clipboard

add support for header rewrite as hash key

Open ramaraochavali opened this issue 3 months ago • 13 comments

We have a use case to pick the rewritten header using regex when header based consistent hash is used. Envoy supports this. It is very difficult to craft an Envoy filter for this (need to replace the entire virtual host)

ramaraochavali avatar May 08 '24 12:05 ramaraochavali

@ramaraochavali: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
release-notes_api 0413bfcd3c30628bb6eb4e25342c0b740850d871 link false /test release-notes

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

istio-testing avatar May 08 '24 12:05 istio-testing

@whitneygriffith

costinm avatar May 08 '24 16:05 costinm

https://www.envoyproxy.io/docs/envoy/v1.30.1/api-v3/config/route/v3/route_components.proto.html#envoy-v3-api-msg-config-route-v3-routeaction-hashpolicy-header - Its been there for a while in Envoy

ramaraochavali avatar May 08 '24 16:05 ramaraochavali

TLDR to add a new field to a v1 API:

  • Add // +cue-gen:DestinationRule:releaseChannel:extended tag to the proto definition similar to what was done here to all API versions.
  • Update the stable validation policy to exclude the new field in the following manifests: base chart and istio-discovery chart

CC: @keithmattix

whitneygriffith avatar May 08 '24 16:05 whitneygriffith

Does it not work in envoy to do a normal header rewrite and use that as a key?

howardjohn avatar May 08 '24 16:05 howardjohn

Does it not work in envoy to do a normal header rewrite and use that as a key?

What do you mean by normal header rewrite?

ramaraochavali avatar May 09 '24 03:05 ramaraochavali

I mean just a standard http header rewrite in virtual service

On Wed, May 8, 2024, 8:46 PM Rama Chavali @.***> wrote:

Does it not work in envoy to do a normal header rewrite and use that as a key?

What do you mean by normal header rewrite?

— Reply to this email directly, view it on GitHub https://github.com/istio/api/pull/3180#issuecomment-2101879022, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEYGXJ3GCWOGVDRMVJMBJ3ZBLWP7AVCNFSM6AAAAABHM2NGEWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBRHA3TSMBSGI . You are receiving this because your review was requested.Message ID: @.***>

howardjohn avatar May 09 '24 03:05 howardjohn

No. We need just rewrite the header using regex capture groups (specifically path header) and use it as hash key but not rewrite the value of the header. Also there is no generic regex rewrite for headers in VS?

ramaraochavali avatar May 09 '24 05:05 ramaraochavali

I see. I worry this usage is extraordinarily niche. Consistent hash AND rewrite AND regex capture... this really feels like something best outside of core of Istio

howardjohn avatar May 09 '24 14:05 howardjohn

I also tried to solve this with Envoy filter but the filter means we need to replace the entire vhost/route which is not ideal.

ramaraochavali avatar May 09 '24 15:05 ramaraochavali

Maybe you can insert some custom filter that does some manipulation before the hashing runs

howardjohn avatar May 09 '24 15:05 howardjohn

My suggestion is to see if we can improve EnvoyFilter to make it easy to support this case.

Did anyone check the EnvoyGateway equivalent of EnvoyPatchPolicy - basically json patch ? Would something like that work for this case ( so we don't reinvent the wheel ) ?

costinm avatar May 09 '24 16:05 costinm

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

istio-testing avatar May 20 '24 20:05 istio-testing