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

Add ability to accept direct non-proxy connections #56

Closed
wants to merge 2 commits into from

Conversation

BenPhegan
Copy link

Hi,

Have been building out a little service stubbing tool here based on your library (which is great by the way!): https://github.com/BenPhegan/fauxpi

One thing that would be useful is to be able to accept direct as well as proxied connections to enable service stubbing directly for apps that do not support proxy use.

A recent change 44e6720 has prevented direct connections, which is a good fix for the problem it addresses.

This pull request is to make this the default behaviour still, but enable the proxy to be set to respond to direct requests via setting an AcceptDirect bool on the proxy object.

Very new to Golang, so not sure if there is a better way to do this or whether the tests (copied from the inverse) are correct. Happy to discuss further!

Regards,

Ben Phegan

@elazarl
Copy link
Owner

elazarl commented Jul 15, 2014

Why do you want goproxy to receive direct HTTP requests?

Why won't you just process them with regular http instead? the proxy don't have to be the handler for everything.

@BenPhegan
Copy link
Author

You are right, it doesn't have to process everything.

Currently thought, there are really two ways that people can easily consume an over the wire service stub. For applications that support a HTTP proxy, already fauxpi will stub the calls based on files provided on disk using the functionality within goproxy. However, for applications that do not support a HTTP proxy, the best approach is to point them at a specific url/port to provide stubbing capability. As everything is already functional within the proxy to accept direct connections and use the exact same logic to provide stubbing, it seemed like a simple change to reallow direct connections but default to false.

The other approach would be to spin up another port and start serving the same functionality albeit via a different mechanism (no proxy, just straight HTTP). I was hoping that I would be able to server both from the same port/instance for simplicity, but I can investigate that approach if this patch doesn't fit with the direction of goproxy.

Thanks for taking a look regardless.

@elazarl
Copy link
Owner

elazarl commented Jul 16, 2014

Don't give up so quickly ;-)

I understand your need. I'm thinking of a way to implement that in a
cleaner manner.

What I'm thinking about is something like:

proxy.OnRequest(...).Serve(func (w http.ResponseWriter, req *http.Request) {
...
}

I think it should work better than what you've had in mind, and would
fulfill your need.

Am I correct?

On Wed, Jul 16, 2014 at 12:31 AM, Ben Phegan notifications@github.com
wrote:

You are right, it doesn't have to process everything.

Currently thought, there are really two ways that people can easily
consume an over the wire service stub. For applications that support a HTTP
proxy, already fauxpi will stub the calls based on files provided on disk
using the functionality within goproxy. However, for applications that do
not support a HTTP proxy, the best approach is to point them at a specific
url/port to provide stubbing capability. As everything is already
functional within the proxy to accept direct connections and use the exact
same logic to provide stubbing, it seemed like a simple change to reallow
direct connections but default to false.

The other approach would be to spin up another port and start serving the
same functionality albeit via a different mechanism (no proxy, just
straight HTTP). I was hoping that I would be able to server both from the
same port/instance for simplicity, but I can investigate that approach if
this patch doesn't fit with the direction of goproxy.

Thanks for taking a look regardless.


Reply to this email directly or view it on GitHub
#56 (comment).

@BenPhegan
Copy link
Author

It would certainly be more explicit. Working through what this would mean for consumer code, if I wanted to implement both direct and proxied resolution, I would need to register a proxy.OnRequest(...).DoFunc(...) for the proxy capability, as well as registering a proxy.OnRequest(...).Serve(...) for the direct connections. However the OnRequest func would then need to differentiate based on the URL.IsAbs() to ensure I only trigger one or the other.

Have I understood correctly? I think this would still work, particularly when you do not have a requirement to do both possibly on a per-requests basis.

@elazarl
Copy link
Owner

elazarl commented Jul 16, 2014

Can you please write a small script examplifying how you would use the API?

I want to add this capability in a well designed manner. It's easy to add
public API, but hard to remove...

Let's see an example use case, and try to see how would it look with a
certain solution.

Thanks,

On Wed, Jul 16, 2014 at 1:42 PM, Ben Phegan notifications@github.com
wrote:

It would certainly be more explicit. Working through what this would mean
for consumer code, if I wanted to implement both direct and proxied
resolution, I would need to register a proxy.OnRequest(...).DoFunc(...) for
the proxy capability, as well as registering a
proxy.OnRequest(...).Serve(...) for the direct connections. However the
OnRequest func would then need to differentiate based on the URL.IsAbs() to
ensure I only trigger one or the other.

Have I understood correctly? I think this would still work, particularly
when you do not have a requirement to do both possibly on a per-requests
basis.


Reply to this email directly or view it on GitHub
#56 (comment).

@BenPhegan
Copy link
Author

Right, so hopefully this will illustrate how I would need to use it based on the suggestion: http://play.golang.org/p/1ynC6MmRmT

package main

import (
    "fmt"
    "github.com/elazarl/goproxy"
    "net/http"
)

func main() {
    fmt.Println("Hello, playground")
    proxy := goproxy.NewProxyHttpServer()

    proxy.OnRequest(proxyRequest()).DoFunc(respond())
    proxy.OnRequest(directRequest()).Serve(respond())

    http.ListenAndServe(":"+"8080", proxy)
}

type ResponseGenerator func(r *http.Request, ctx *goproxy.ProxyCtx) (*http.Request, *http.Response)

func proxyRequest() goproxy.ReqConditionFunc {
    return func(req *http.Request, ctx *goproxy.ProxyCtx) bool {
        return !req.URL.IsAbs()
    }
}

func directRequest() goproxy.ReqConditionFunc {
    return func(req *http.Request, ctx *goproxy.ProxyCtx) bool {
        return req.URL.IsAbs()
    }
}

func respond() ResponseGenerator {
    return func(r *http.Request, ctx *goproxy.ProxyCtx) (*http.Request, *http.Response) {
        return r, goproxy.NewResponse(r, "application/json", 201, "")
    }
}

Of course this doesn't compile, but I think the intent would be that the .Serve(...) would not do the URL.IsAbs() check?

Although this would work, it just means that the consumer has to be very explicit about which requests they will "Server" and which they will "Proxy".

@BenPhegan
Copy link
Author

Hey, just wondering if you had any feedback on this one?

@elazarl
Copy link
Owner

elazarl commented Aug 8, 2014

Sorry, I forgot about it.

I'll look at it today.
On Aug 8, 2014 4:39 AM, "Ben Phegan" notifications@github.com wrote:

Hey, just wondering if you had any feedback on this one?


Reply to this email directly or view it on GitHub
#56 (comment).

@elazarl
Copy link
Owner

elazarl commented Aug 10, 2014

OK, I think I got it. How about setting a default http.Handler for non-proxy requests?

proxy := goproxy.NewHTTPProxy()
proxy.HTTPHandler = http.Dir(".")
http.ListenAndServe(proxy)

Now direct requests to the proxy would serve files from the local directory

$ curl http://localhost:8080/foo.txt
foo content

While proxy requests would be served as usual:

$ curl -x localhost:8080 http://www.qhm.mod.uk/robots.txt
User-agent: *
Disallow: /portsmouth/cms/
Disallow: /portsmouth/admin/
Disallow: /clyde/cms/
Disallow: /clyde/admin/
Disallow: /plymouth/cms/
Disallow: /plymouth/admin/
Disallow: /cms/
Disallow: /admin/
Allow: /

What do you think?

@elazarl
Copy link
Owner

elazarl commented Aug 22, 2014

@BenPhegan since I got no reply to my suggestion, and I think it's OK, I'll implement it next week.

Like the wedding scene in the movies, if you want to stop it, speak now ;-)

@BenPhegan
Copy link
Author

Hey, sorry. Only just getting back to this.

Just to clarify, this would be a single handler that could be written by the implementor to do whatever is required? It may work but it probably means it would be difficult to reuse much of the code written for the proxy code paths.

Ideally I would love to have the same code paths available in any OnRequest(...).DoFunc(...) both direct and via proxy. Even registering the same code twice would be ok, like OnRequest(...).DoFunc(SomeFunc()) and OnDirect(...).DoFunc(SomeFunc()).

I am not sure that you should blend my requirements (loading responses from file) and goproxy. There is a currently a cleanliness of design intent here, and I wouldn't want to mess with that. The differentiator is definitely direct or proxy. Again, not sure the best way of flagging this, but suggestions would be:

  1. Flag on the struct as per pull request (but I understand why you may not want this)
  2. Explicit method calls to differentiate between direct and proxy
  3. Do not include the functionality at all. This would mean implementing on another port a different HTTP server and dealing with the logic myself. Again, happy to take this path if you don't think this fits easily within goproxy.

So I guess I just stood up at the wedding and embarrassed myself! :)

@elazarl
Copy link
Owner

elazarl commented Aug 24, 2014

Thanks for getting back to me.

The thing is, net/http has plenty of muxers, what would the benefit of
using the same OnRequest logic in both places? Can't you use, say,
gorilla/mux for the non-proxy requests, and OnRequest for the proxy
requests?

On Sun, Aug 24, 2014 at 1:58 PM, Ben Phegan notifications@github.com
wrote:

Hey, sorry. Only just getting back to this.

Just to clarify, this would be a single handler that could be written by
the implementor to do whatever is required? It may work but it probably
means it would be difficult to reuse much of the code written for the proxy
code paths.

Ideally I would love to have the same code paths available in any
OnRequest(...).DoFunc(...) both direct and via proxy. Even registering the
same code twice would be ok, like OnRequest(...).DoFunc(SomeFunc()) and
OnDirect(...).DoFunc(SomeFunc()).

I am not sure that you should blend my requirements (loading responses
from file) and goproxy. There is a currently a cleanliness of design intent
here, and I wouldn't want to mess with that. The differentiator is
definitely direct or proxy. Again, not sure the best way of flagging this,
but suggestions would be:

  1. Flag on the struct as per pull request (but I understand why you
    may not want this)
  2. Explicit method calls to differentiate between direct and proxy
  3. Do not include the functionality at all. This would mean
    implementing on another port a different HTTP server and dealing with the
    logic myself. Again, happy to take this path if you don't think this fits
    easily within goproxy.

So I guess I just stood up at the wedding and embarrassed myself! :)


Reply to this email directly or view it on GitHub
#56 (comment).

@BenPhegan
Copy link
Author

Yes, I could definitely use gorilla/mux (or another similar mux). Although for example gorilla/mux uses something similar to this for runtime determination of match:

    r.MatcherFunc(func(r *http.Request, rm *RouteMatch) bool {
        return r.ProtoMajor == 0
    })

Which is a different method implementation for matching than goproxy. I could easily write the code so that I could consume through both matching approaches, but it is additional overhead.

If you are suggesting that I put the mux in front of goproxy, and only proxy when it is not a direct connection...that might work as well. I could also put the other mux on another port. I was just hoping for a single codebase approach to the problem.

Just to be clear, what I am trying to achieve:

  1. Proxy/service stubber sits on a single port
  2. Proxy aware application can have its environment set to use this proxy, and the responses to its REST requests can be stubbed from on disk responses.
  3. Proxy unaware application can have its target endpoint set to be the proxy, and still have its requests stubbed from possibly the same or similar on disk responses.

Much of the approach (or thought behind it) is covered here: http://ben.phegan.name/index.php/2014/07/28/service-stubbing-and-virtualisation/

Hope that makes sense, but I am not sure it helps answer the question. I am thinking about how best to articulate what I am trying to do in the context of goproxy so that I don't waste any more of your time (whilst still getting the best outcome!). Will have a think and get back to you.

@elazarl elazarl closed this in 54808b6 Sep 8, 2014
elazarl added a commit that referenced this pull request Sep 8, 2014
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.

None yet

2 participants