-
Notifications
You must be signed in to change notification settings - Fork 472
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support for specifying the filter execution order in HTTPRoute #1598
Comments
Hi @Amila-Rukshan thanks for the request. Can you help us to better understand the context and use case? What implementation(s) are you particularly interested in having implement this and is this more of a user-facing request? OR what implementation(s) do you work on? Could you describe a concrete use-case (e.g. explain what |
@shaneutt thanks for replying! |
Understood, thank you. So is this more of a user-facing request, or what implementation(s) are you hoping to add this to? |
@shaneutt what about having another optional property called For example:
|
If we need order I would think that should be defined by the order in the list |
The Kubernetes project currently lacks enough contributors to adequately respond to all issues. This bot triages un-triaged issues according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues. This bot triages un-triaged issues according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle rotten |
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs. This bot triages issues according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /close not-planned |
@k8s-triage-robot: Closing this issue, marking it as "Not Planned". In response to this:
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 agree with this. In any case it will be good to address this in the spec. Can we reopen this issue? Or is it okay if I submit a PR to fix the spec to specify an order? |
Thanks for brining this one back up @sanjaypujare! It looks like I missed this the first time, but I don't think there's anything about the existing filters list that makes it unordered. We simply haven't provided any guidance in the spec in terms of ordering. This was initially with the thought that many underlying dataplanes have a fixed order of operations that could not support this kind of custom ordering for at least some features. With that said, it probably makes sense to add some guidance that implementations should respect the order filters are defined in wherever possible. Each implementation should also likely document any implementation-specific limitations on filter ordering they have. @sanjaypujare if you're up for making an update to the spec, I think it would be helpful to at least provide some recommendations here. |
/assign @sanjaypujare |
So this is what I am thinking: I would like to change the text at https://github.com/kubernetes-sigs/gateway-api/blob/main/apis/v1/httproute_types.go#L199 (and a similar one for GRPCRoute if applicable) to say something like: "The filters are applied in the order they are listed. In case an implementation has a fixed order of operations that cannot support the listed order, the implementation will reject the whole Route with an error." Note that the spec already specifies list order for first marching rule in case of multiple matches: "matching precedence MUST be granted to the FIRST matching rule (in list order) with a match" so there is a precedent for list order of route rules in a route. We can extend the semantics to filters as well. Let me know what you think. If okay, I'll open a PR. |
There are no comments so I'll take it as there are no objections and I'll open a PR to specify list order for filters. |
I personally think rejecting the Route is a bit much in this case. We could theoretically have a standard way to communicate that filters will not be executed in the order specified, but there are some cases where order doesn't really matter and the warning message may cause unnecessary confusion. |
Good point about rejecting. How about indicating through |
Maybe this is unrelated/irrelevant, but I think we have to be really careful here, especially with #713 being so widely used by the community. Let's take for example the Route above:
Not let's add some policy which applies auth to that HTTPRoute
If the user wanted to ensure that |
Probably one of the ways is to use
The same link above (Kong's plugin ordering) also describes a couple of use-cases where users or implementors benefit from exposing the order. |
I don't disagree with this, but I definitely think it should fall under the umbrella of implementation specific if we decide to go forward with it at all. |
Okay, so I suggest the following addition/change to the spec (at https://github.com/kubernetes-sigs/gateway-api/blob/main/apis/v1/httproute_types.go#L199). The wording will be something like this: "This spec does not mandate any specific ordering for the execution of filters. However an implementation may decide that the filters will be executed in the listed order. In case the implementation requires a certain order of operations that cannot support the listed order, the implementation will reject the whole Route with the error For now I am not addressing the issue of a policy's order vis-a-vis the filters' order but I will do it later. @robscott @shaneutt @EItanya are you okay? If yes, then I'll open a PR |
SGTM, thanks @sanjaypujare! One tiny nit - there may be room to just reject an individual route rule instead of the whole route. |
Yes, but an individual route rule doesn't have a Status like the Status an HTTPRoute has unless I am missing something. |
Yep, you're right. You could add this as a reason to the following condition though: gateway-api/apis/v1/shared_types.go Lines 402 to 435 in 5658635
|
What would you like to be added:
There are scenarios where the filters should be applied in a predefined order. But the filters does not support a way to order individual filter since it's an unordered list.
For example following HTTPRoute has multiple filters,
How can someone specify the order like
my-policy-1
,my-policy-2
, and thenurlRewrite
filters are applied in order?Docs says this is not supported yet: https://gateway-api.sigs.k8s.io/references/spec/#gateway.networking.k8s.io/v1beta1.HTTPRouteFilter
We would like to have the support for this.
Any thought on this?
Thanks!
The text was updated successfully, but these errors were encountered: