Provide a Zuul filter for rewriting Location header in responses about spring-cloud-netflix HOT 14 CLOSED

Rough POC with separate filters for content and headers:
https://gist.github.com/celkins/a8ff36c1e7677bca70f7

from spring-cloud-netflix.

@celkins looks nice. I had a working version #33 that fixed the prefix of the customers and stores api's to /api and then in the ui, the proxy prefix is /api and it is not stripped off. This is more dynamic. Any concerns about speed using the regex replaces on every request? It uses the common proxy headers to replace the host and port.

from spring-cloud-netflix.

The code could definitely use a few runs through the ringer before being ready for production. As you see, one obvious improvement would be to compile the regex during construction rather than (implicitly) recompiling it for every request. Another concern is the lack of response buffering and complete ignorance of content encoding. Header comparison is probably ripe for improvement as well.

I guess at this point I'm looking for opinions on whether this is a good approach and something that might have a place here. I've implemented similar things at least twice before—one based on Jetty's proxy servlet and another using Spring Integration—but neither has seemed quite right, and I'm hoping a Zuul-based solution will work better.

Demo project here.

from spring-cloud-netflix.

TODO: Add a note in docs about not stripping the prefix is all the services share a prefix with the proxy (/api & /api/* for example). 8a5e45c

from spring-cloud-netflix.

Documentation is now updated for the mapping options 79db272. Also updated spring-cloud-samples/customers-stores@5f5f9ef to use this configuration.

from spring-cloud-netflix.

I would suggest that either change the class to not be final, so it can be sub-classed, or provide an option to set the order. As it stands now, there is no way for someone to take this class and use it and still control the order if they need to, without forking the code.

from spring-cloud-netflix.

@zampettim which class?

from spring-cloud-netflix.

I was looking at the Gist of ContentUrlRewritingFilter, which is marked as final. If there is something more recent I should look at instead, just let me know. The key point I'm trying to make is that as we start to see more pre-built filters, it needs to be kept in mind that the order of filters is really more of a runtime config decision, and less a compile-time decision. Having the ability to override or influence the order will eventually become important.

from spring-cloud-netflix.

Sorry I don't follow you. The code in the gist pasted above by @celkins is not part of this library. Or did I misunderstand?

from spring-cloud-netflix.

@dsyer You are correct, my comments are about the code in the Gist posted in @celkins. The subject of this issue is about a ZuulFilter that does content re-writing, and @celkins posted a Gist with a possible solution. My comment was directed to the possibility of taking his work and including it in the library. It appears that is not happening, or is not part of this ticket. That is fine. I didn't mean to confuse anyone.

In general though, I would like to see any ZuulFilters that are included in the spring-cloud library be built in such a way that there is a way to override the ordering information, either through sub-classing or with a method to update the order. Should I open another issue to track this point instead?

from spring-cloud-netflix.

@zampettim please open another issue for your zuul filter extensibility. I'm going to close this, as I don't think we want to maintain a general rewrite filter.

from spring-cloud-netflix.

This is re-opened so we can talk about a narrower scope: rewriting the "location" header only (no monkeying with response bodies).

from spring-cloud-netflix.

That sounds like something we could handle.

from spring-cloud-netflix.

Closing this since #1863 addressed this issue

from spring-cloud-netflix.