Proxy to unix: socket

[eiszfuchs]

These are the changes so far as discussed in #406.

Open tasks:

• Cover cases with tests
• Clean up test case
• Verify tests not breaking on Windows

[mholt]

Below, you're checking for a "unix:" prefix - I suppose that a url parse somewhere converts it so the prefix is "unix://" ?

[eiszfuchs]

Yes, it does. That line could use a comment. I discovered that by accident.

[mholt]

Cool, looks like a great start. :)

[mholt]

How's this coming, @eiszfuchs ? Once there are some tests we can get this into the next release.

[eiszfuchs]

Oh! Hey! Sorry, I needed some days of rest over the holidays.

I might be able to write tests this or next week, but regarding Windows support, I'd be completely lost.
Aa far as I understood these sockets, Windows doesn't support them at all, but caddy isn't to blame for that. The feature just wouldn't be available.

[mholt]

That sounds good to me! I wouldn't expect Windows to support them.

[eiszfuchs]

Hm. I've had a look at the existing tests as of now, and since my changes mostly affected reverseproxy, I was wondering if there is a reason this file isn't covered by tests yet.
Also, I'm a bit lost where to start. I looked for tests that would check configurations against caddy's behaviour as I'm used to them, but I couldn't find any. My approach would have been to write a test that checks whether http proxy directives work and if unix directives work, too.

[mholt]

With these changes so far, code coverage is:

So all that really needs to be tested is that creating a proxy with a unix socket works. I just realized we don't have a test case for a proxy on a regular socket (other than testing the WebSocket proxying in proxy_test.go), so this will knock out two birds with one stone. 😄

Basically, create a new httptest.Server using httptest.NewUnstartedServer - then you can set its Listener to a unix socket listener, then start the test server. That's the backend you're proxying to. Give it a handler that asserts that A) it was called, and B) that any expected values were correct. Then call NewSingleHostReverseProxy with the target URL being to the unix socket. Have it proxy a http.NewRequest and then assert to make sure the backend was called. That's it! Here's a simpler example along these lines: https://github.com/mholt/caddy/blob/master/caddy/letsencrypt/handler_test.go#L30

[eiszfuchs]

Hello again.
I'm almost falling asleep and got this far:

func TestUnixSocketProxy(t *testing.T) {
    var proxySuccess bool
    ts := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        proxySuccess = true
    }))

    ln, err := net.Listen("unix", "/Users/eiszfuchs/socket_test")
    if err != nil {
        t.Fatalf("Unable to listen: %v", err)
    }
    ts.Listener = ln

    ts.Start()
    defer ts.Close()

    // TODO: Create client/connection and check response

    if !proxySuccess {
        t.Fatal("Expected request to be proxied, but it wasn't")
    }
}

I think I still don't quite get what I am actually checking.
What really bothers me is that this test needs to create a file and requires permissions for that. Since I'm not running tests as root, I have to specify a folder in which I can create new files - which would be my home folder. This test so far fails, but it will fail on other machines, anyway, because my username is in there.

[mholt]

@eiszfuchs Good work!

You don't need to use an absolute path for the unix socket, right? Will a relative path work? If so, just put it in the cwd or something and delete it when done.

All that's left otherwise is to use CreateSingleHostReverseProxy to run the test.

[eiszfuchs]

Ahoy!
So, I figured I could just resolve a relative path into an absolute path and use that for the test. Duh.
My test looks like this now:

func TestUnixSocketProxy(t *testing.T) {
    trialMsg := "Is it working?"

    var proxySuccess bool
    ts := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        proxySuccess = true

        fmt.Fprint(w, trialMsg)
    }))

    socketPath, err := filepath.Abs("./test_socket")
    if err != nil {
        t.Fatalf("Unable to get absolute path: %v", err)
    }

    ln, err := net.Listen("unix", socketPath)
    if err != nil {
        t.Fatalf("Unable to listen: %v", err)
    }
    ts.Listener = ln

    ts.Start()
    defer ts.Close()

    url := strings.Replace(ts.URL, "http://", "unix:", 1)
    p := newWebSocketTestProxy(url)

    echoProxy := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        p.ServeHTTP(w, r)
    }))
    defer echoProxy.Close()

    res, err := http.Get(echoProxy.URL)
    if err != nil {
        t.Fatalf("Unable to GET: %v", err)
    }

    greeting, err := ioutil.ReadAll(res.Body)
    res.Body.Close()
    if err != nil {
        t.Fatalf("Unable to GET: %v", err)
    }

    if !proxySuccess {
        t.Fatal("Expected request to be proxied, but it wasn't")
    }

    actualMsg := fmt.Sprintf("%s", greeting)

    if actualMsg != trialMsg {
        t.Errorf("Expected '%s' but got '%s' instead", trialMsg, actualMsg)
    }
}

And the best part is: It covers the right code now!
Do you see anything fishy in here? I expect it to be. This looks like a lot of nonsense to me.
As I said, I'm not quite familiar with Go yet, but figuring out what I was doing there was kinda fun!

[mholt]

That looks good! Last things to do:

• Add that test to your PR
• Also add a comment as described here
• Sign the CLA

And I'll merge it in.

[CLAassistant]

All committers have accepted the CLA.

[eiszfuchs]

Yuk. I'm making a mess. Will rebase my branch.

[eiszfuchs]

Oh man, that was a lot of work. I start to figure out how to rebase, though.

[mholt]

Awesome, almost done! Just gotta get those tests passing in Windows. Since users won't be using Unix sockets on Windows we can just run the test if the OS isn't Windows. At the top of the test:

if runtime.GOOS == "windows" {
    return
}

[eiszfuchs]

Okay, added it! Hit return too early, though, so I pushed it before I was able to squash.
Is that a problem?

[mholt]

Yeah, if you could squash it one more time that'd be perfect! Great work.

[eiszfuchs]

Alright! That should be it! 🍻

[mholt]

Woo! Nicely done. Cheers.

[eiszfuchs]

Crazy stuff! Thanks for your time!

[mholt]

Thank you. Hopefully this will not be the last contribution we see from you 😄