Skip to content
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

Zuul Filters should not have an output type #777

Closed
carl-mastrangelo opened this issue Apr 14, 2020 · 5 comments
Closed

Zuul Filters should not have an output type #777

carl-mastrangelo opened this issue Apr 14, 2020 · 5 comments
Labels
Milestone

Comments

@carl-mastrangelo
Copy link
Contributor

There are 3 types of filters in zuul today:

  1. Inbound
  2. Endpoint
  3. Outbound

Inbound filters take in a request, and output a request. Endpoint filters take in a request, and output a response. Outbound filters take in a response, and output a response. This means that Endpoint filters are pretty different than the other two types. In addition, endpoint filters must have a filter order of 0, meaning the don't participate in chaining.

Inbound filters (the majority) frequently mutate the request passed in, but always return the same object back. Because there are so many, it is expensive to make the request immutable, use a builder pattern, or a "with" pattern. The same occurs for outbound filters.

Therefore, I suggest filters be split into inbound, outbound, and endpoint. Endpoint filters will no longer share a type hierarchy. Inbound and outbound filters will take a single request or response, and simply mutate it. Lastly, since SessionContext is a first class concept and frequently needs to be pulled out of the request, I believe it should be passed as a second parameter to each of these new types.

@carl-mastrangelo carl-mastrangelo added the 3.x Zuul 3 label Apr 14, 2020
@carl-mastrangelo carl-mastrangelo added this to the Zuul 3 milestone Apr 14, 2020
@steffentemplin
Copy link

Hey,

as a community user of Zuul 2, I would argue that being able to fully exchange the processed request/response objects within a filter provides great flexibility. Especially from a consumer perspective, where you cannot alter Zuul Core but solely depend on its provided extension points.

I would agree that the default use-case is at max. altering a certain property of the request/response object though. So what about rather providing a new set of abstract superclasses for the 90% use-cases, like in your proposal. But in addition keep the possibility to for example inherit from something further up in the class hierarchy for inbound and outbound filters where one could still return a completely different object?

@carl-mastrangelo
Copy link
Contributor Author

Hi @steffentemplin

That seems like it would work. In your filters, when you return an object from your filters, is it always the same type? I think we could implement the parent class approach you mentioned, though I would still like to simplify the API. Would the following API work for you?

<T> T apply(T t);

BTW, I didn't realize that there were many users of Zuul 2. You should take a look at #771 for other filter changes we are considering.

@steffentemplin
Copy link

That would definitely work, thanks for considering it and pointing me to #771!

Copy link

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the Stale label Sep 15, 2024
Copy link

This issue was closed because it has been stalled for 7 days with no activity.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Sep 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants