webserver: proxy requests to indexserver via socket #487
Conversation
With this change webserver proxies request to /indexserver/ to the unix domain socket at `<index dir>/indexserver.socket`. Since this is a Sourcegraph feature, we put the proxy behind a flag which is toggled off per default. The motivation is to expose indexserver endpoints on webserver addressses, which in case of Sourcegraph, are easily accessible at runtime.
| log.Fatalf("failed to listen on socket: %s", err) | ||
| } | ||
| debug.Printf("serving HTTP on %s", socket) | ||
| log.Fatal(http.Serve(l, http.StripPrefix("/indexserver", mux))) |
There was a problem hiding this comment.
Are you sure that the prefix that you're stripping shouldn't be /indexserver/?
https://pkg.go.dev/net/http#StripPrefix
...The prefix must match exactly...
There was a problem hiding this comment.
I'm not really sure why you are stripping the prefix as well? Is this done because this is the path that the zoekt-webserver will proxy? If
There was a problem hiding this comment.
Yes, we have to strip it somewhere, otherwise the paths won't match. I agree, webserver seems the more natural place to do it, so that indexserver doesn't need to know about how the proxy operates.
Regarding stripping "/indexserver/" vs "/indexserver":
I did some manual testing, the answer seems to depend on where we call StripPrefix
- indexserver: only "/indexserver" works
- webserver: both work
| log.Fatalf("failed to listen on socket: %s", err) | ||
| } | ||
| debug.Printf("serving HTTP on %s", socket) | ||
| log.Fatal(http.Serve(l, http.StripPrefix("/indexserver", mux))) |
There was a problem hiding this comment.
I'm not really sure why you are stripping the prefix as well? Is this done because this is the path that the zoekt-webserver will proxy? If
|
@ggilmore @keegancsmith I ran some experiments on kubernetes. The good news is that the socket communication between webserver and indexserver works. The bad news is that the default permissions 755 of the socket are not enough for webserver to write to it, probably because indexserver and webserver run with different users. In the last commit of this PR I change the permissions of the socket to 777, see also the comment in the code. WDYT? Could this be a security issue? |
keegancsmith
left a comment
keegancsmith
left a comment
There was a problem hiding this comment.
To me this is the same as just using TCP if we could have a well defined port. IE its the same security model between the two containers:
- we want them to communicate with each other
- only those containers have the FS mounted.
This enables the proxy for Sourcegraph's webserver See #487
This enables the proxy for Sourcegraph's webserver See #487
Relates to sourcegraph/zoekt#487 We set the new flag for local dev and server.
Relates to sourcegraph/zoekt#487 We set the new flag for local dev and server.
3 participants
With this change, webserver proxies request to
<webserver>/indexserver/to the unix domain socket at<index dir>/indexserver.socket. Since this is a Sourcegraph feature, we put the proxy handler in webserver behind a flag which is toggled off per default.The motivation is to expose indexserver endpoints on webserver addresses, which in case of Sourcegraph, are easily accessible at runtime.
Test Plan
--indexserver_proxycurl http://localhost:6070/indexserver/