api
api copied to clipboard
istio
Add regex rewrite
In our gateway case, we definitely need this awesome feature to meet so many path routes. Maybe EnvoyFilter can support regex rewrite for data plane. But we are tired to name these routes that require regular rewriting. Only if we pick meaningful names for these routes, EnvoyFilter can match these route names and apply regex rewrite policy. Adding regex rewrite api for rewrite in HTTPRoute is the easy way to fix this case. I think someone needs this feature like me.
Based on this pr https://github.com/istio/api/pull/1566, introduce RegexMatchAndSubstitute which may be used in the future for similar cases. @howardjohn @hzxuzhonghu
Fixes https://github.com/istio/istio/issues/22290 and https://github.com/istio/istio/issues/24172.
🤔 🐛 You appear to be fixing a bug in Go code, yet your PR doesn't include updates to any test files. Did you forget to add a test?
Courtesy of your friendly test nag.
😊 Welcome @SpecialYang! This is either your first contribution to the Istio api repo, or it's been awhile since you've been here.
You can learn more about the Istio working groups, code of conduct, and contributing guidelines by referring to Contributing to Istio.
Thanks for contributing!
Courtesy of your friendly welcome wagon.
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.
We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.
Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).
ℹ️ Googlers: Go here for more info.
![google-cla[bot] avatar](https://avatars.githubusercontent.com/in/42202?v=4 "Published by a pub.dev verified publisher"){kind=small}
Hi @SpecialYang. Thanks for your PR.
I'm waiting for a istio member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.
Once the patch is verified, the new status will be reflected by the ok-to-test label.
I understand the commands that are listed here.
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/test-infra repository.
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.
We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.
Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).
ℹ️ Googlers: Go here for more info.
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.
We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.
Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).
ℹ️ Googlers: Go here for more info.
I am personally fine with this, but wanted to echo @costinm concern: https://github.com/istio/api/pull/1566#issuecomment-767631005
At least we should bounce the idea with the K8S Services WG - and the other projects planning to support the K8S APIs. I assume since this didn't make it to 1.9 we can afford few weeks to discuss with the relevant WGs.
lgtm
At least we should bounce the idea with the K8S Services WG
Is this the service-apis?
I am personally fine with this, but wanted to echo @costinm concern: #1566 (comment)
At least we should bounce the idea with the K8S Services WG - and the other projects planning to support the K8S APIs. I assume since this didn't make it to 1.9 we can afford few weeks to discuss with the relevant WGs.
Same question. Why is it related to service APIs?
The regex rewrite policy for uri is basic feature in most gateway products. Our gateway envoy get runtime configs from istiod via xds protocol. We must apply many EnvoyFilter configs and pick up meaningful names for target routes. We want an easy way just like configuring the regex rewrite policy directly in HTTPRoute, which is clear.
As I know, current istiod use RE2 for all regex match cases. I think we can also use it in regex rewrite until istiod support different regex styles.
I'm sure many users will look forward to this feature. Thanks.
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.
We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.
Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).
ℹ️ Googlers: Go here for more info.
I am personally fine with this, but wanted to echo @costinm concern: #1566 (comment)
At least we should bounce the idea with the K8S Services WG - and the other projects planning to support the K8S APIs. I assume since this didn't make it to 1.9 we can afford few weeks to discuss with the relevant WGs.
Same question. Why is it related to service APIs?
The regex rewrite policy for uri is basic feature in most gateway products. Our gateway envoy get runtime configs from istiod via xds protocol. We must apply many
EnvoyFilterconfigs and pick up meaningful names for target routes. We want an easy way just like configuring the regex rewrite policy directly in HTTPRoute, which is clear.As I know, current istiod use RE2 for all regex match cases. I think we can also use it in regex rewrite until istiod support different regex styles.
I'm sure many users will look forward to this feature. Thanks.
@howardjohn @costinm PTAL. Thanks.
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.
We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.
Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).
ℹ️ Googlers: Go here for more info.
@SpecialYang: The following test failed, say /retest to rerun all failed tests:
| Test name | Commit | Details | Rerun command |
|---|---|---|---|
| release-notes_api | 156fd06d0dae3dab1ed67f980c3a557f302ec850 | link | /test release-notes_api |
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/test-infra repository. I understand the commands that are listed here.
:frowning_face: Sorry, but only Googlers may change the label cla: yes.
Is this done? i desire this function!
cc @howardjohn @hzxuzhonghu @costinm Is there any latest feedback here?
@SpecialYang: 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/test-infra repository.
@howardjohn will this ever see the light? I opened https://github.com/istio/istio/issues/22290 one year and a half ago :(
@SpecialYang @howardjohn @hzxuzhonghu Anyone working on this PR?
I am working on this. But the community seems don't accept this api change yet. We add this feature in our internal version of istio and run in production. Hope community can rethink about this very common feature to fix this long-alive issue https://github.com/istio/istio/issues/22290.
I would recommend giving feedback on https://github.com/kubernetes-sigs/gateway-api/pull/731, as Istio will implement this API.
I would recommend giving feedback on kubernetes-sigs/gateway-api#731, as Istio will implement this API.
What's the relationship between gateway-api and istio virtual service? Do you mean that virtualservice would not implement complicated features, users who need these features should use gateway-api instead?
What's the relationship between gateway-api and istio virtual service? Do you mean that virtualservice would not implement complicated features, users who need these features should use gateway-api instead?
The gateway-api can be seen as the next evolution of the Istio APIs (in the future), with a common API shared by many proxy implementations. By getting this feature into that API it would make it an obvious choice to also add it to VirtualService
@SpecialYang @howardjohn I'm very sorry for mentioning, but what is the current status?
The process of discussion/implementing this feature seems not transparent at all. :( Is this PR not merged due to conflicts or there's some internal maintainers decision that this wouldn't be merged due to some ideological considerations? Or maybe you are just waiting for approval of https://github.com/kubernetes-sigs/gateway-api/pull/731 proposal?
I can't believe this feature isn't in istio... why hasn't this 1 yo PR not been merged into the project yet?
@howardjohn Is the necessary gateway api fields been added? If not should we just go ahead and add this support. This seems pretty reasonable request and some of our service owners are also asking for this feature.
@SpecialYang , @istio-policy-bot , @googlebot . What happened in the end with this pull, are they going to implement it or not?
🤔 🐛 You appear to be fixing a bug in Go code, yet your PR doesn't include updates to any test files. Did you forget to add a test?
Courtesy of your friendly test nag.
Can you review this PR, please ?
@dcberg @duderino @linsun @louiscryan @nrjpoddar @smawson @istio/technical-oversight-committee
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/test-infra repository.