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

modifier: adding modifier tag concept #69

Closed
wants to merge 1 commit into from

Conversation

jonmorehouse
Copy link

When digging into fabio, I found the need to be able to route to upstreams based upon a "prefix" in a URI.

For instance, let's say I have a service I want to call test at localhost:8000 and I want it accessible at localhost:9999/test/ via fabio. I would like a request to localhost:9999/test to point to localhost:8000/, not localhost:8000/test.

I added the concept of a "modifier" tag, which allows you to trigger whether or not we strip the prefix when routing to a specific target. I'd love to hear any thoughts, its my first steps in this code base and I'm sure there are plenty of things to do better!

This commit introduces the concept of a modifier tag which, as a first
pass supports stripping URL prefixes for a target.
@CLAassistant
Copy link

CLAassistant commented Mar 29, 2016

CLA assistant check
All committers have signed the CLA.

@@ -52,6 +53,16 @@ func (p *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) {
h = newHTTPProxy(t.URL, p.tr)
}

// strip the prefix, if the target was tagged properly
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was digging through this, and it seems like the Target object could have a Modify method which accepts an http.Request object and modifies the request to its liking. In the case that we had more modifier tags, this could be quite useful

@magiconair
Copy link
Contributor

Hi @jonmorehouse, this has been requested multiple times in #2 and #44 which is still open. Personally, I think stripping the URL prefix is the wrong approach since it moves essential configuration from your application into the load balancer and it makes your application behave differently with and without load balancer. IMO, in a lot of cases adding an additional handler isn't that much work but it makes the routing explicit.

However, I get that some people would like to this support for this and there are some edge cases where adding handlers is not easy. I've outlined a different syntax for the urlprefix tag in #42 which would allow this option more organically. Feel free to work on that and build the strip prefix option on top of that.

@aaronhurt
Copy link
Member

This is absolutely needed in our environment. We're currently using a very complex consul-template design that is based on tags including "proxy-standard" and "proxy-nostrip". Our standard tag does a regrep on the request and strips the route prefix before passing it on the the backend whereas the nostrip does the opposite.

@magiconair
Copy link
Contributor

@jonmorehouse I'm going to close this PR as the strip prefix feature has been implemented for #44 and shipped in v1.3.7. In any case I want to thank you for your submission to show interest in the project and this feature.

@magiconair magiconair closed this Feb 26, 2017
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants