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

Support a ResponseBodyModifier filter #1998

Open
arkodg opened this issue May 5, 2023 · 9 comments
Open

Support a ResponseBodyModifier filter #1998

arkodg opened this issue May 5, 2023 · 9 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature. kind/gep PRs related to Gateway Enhancement Proposal(GEP) needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.

Comments

@arkodg
Copy link
Contributor

arkodg commented May 5, 2023

What would you like to be added:
A filter within HTTPRoute to allow the user to specify the configuration to return a custom response body
based on the status code of the response

Why this is needed:
To return a more informative and visually pleasing response whenever errors such as 404 or 5XX are encountered in the data plane or backend

NGINX supports this https://www.jajaldoang.com/post/nginx-custom-error-response/
Emissary (based on Envoy) supports this https://www.getambassador.io/docs/emissary/latest/topics/running/custom-error-responses
HAProxy supports this https://www.haproxy.com/blog/serve-dynamic-custom-error-pages-with-haproxy/

@arkodg arkodg added the kind/feature Categorizes issue or PR as related to a new feature. label May 5, 2023
@howardjohn
Copy link
Contributor

Do we really need it based on the response? And not just a static response based on request?

We are pretty quickly going to divulge into needing to do the ~full HTTPRoute matching on the response IMO, which is not great.

Even serving a static response is tricky. We don't want folks putting 10MB pages into YAML for example

@arkodg
Copy link
Contributor Author

arkodg commented May 5, 2023

@howardjohn this is specific to return a custom response based on bad statuses (like 5XX)
linking some open issues from users in other implementations

@howardjohn
Copy link
Contributor

Both of those issues are actually about customizing response codes return from the proxy for various things. The issue sounds like its asking to look at the backend's response to determine the response though?

Big difference, IMO, between customizing filter-driven responses and backend responses.

@arkodg
Copy link
Contributor Author

arkodg commented May 5, 2023

sure we can help the author, filter based on local reply or response from backend

some more error pages on the web to highlight the use case

@howardjohn
Copy link
Contributor

howardjohn commented May 5, 2023 via email

@arkodg
Copy link
Contributor Author

arkodg commented May 5, 2023

the default backend approach can handle these cases

  • route/rule to backend not found

but cannot handle these cases

  • backend responded with 5XX probably because an internal method/API call timed out
  • backend responded with a 404 because the resource wasn't found

@RWDai
Copy link

RWDai commented May 9, 2023

the default backend approach can handle these cases

  • route/rule to backend not found

but cannot handle these cases

  • backend responded with 5XX probably because an internal method/API call timed out
  • backend responded with a 404 because the resource wasn't found

and more case:

  • Response body needs to be encrypted
  • Response body needs to be compress

@shaneutt shaneutt added the triage/needs-information Indicates an issue needs more information in order to work on it. label May 9, 2023
@shaneutt
Copy link
Member

shaneutt commented Jul 3, 2023

Sounds good, should be proposed as a GEP.

/triage accepted
/kind gep

However we're currently focused on getting our GA release completed (see milestone v1.0.0) and this would not seem to be something that would need to block the release, so we're not ready to prioritize this and we would ask that v1.0.0 milestone issues get priority so we can focus there, and come back to this after the release.

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. kind/gep PRs related to Gateway Enhancement Proposal(GEP) labels Jul 3, 2023
@shaneutt shaneutt removed the triage/needs-information Indicates an issue needs more information in order to work on it. label Jul 3, 2023
@k8s-triage-robot
Copy link

This issue has not been updated in over 1 year, and should be re-triaged.

You can:

  • Confirm that this issue is still relevant with /triage accepted (org members only)
  • Close this issue with /close

For more details on the triage process, see https://www.kubernetes.dev/docs/guide/issue-triage/

/remove-triage accepted

@k8s-ci-robot k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. and removed triage/accepted Indicates an issue or PR is ready to be actively worked on. labels Jul 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. kind/gep PRs related to Gateway Enhancement Proposal(GEP) needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.
Projects
None yet
Development

No branches or pull requests

6 participants