-
-
Notifications
You must be signed in to change notification settings - Fork 836
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 option to bind to a Unix socket instead of a TCP port #703
Changes from all commits
596cc5e
c72da0b
be8389e
fdee28a
1f7f246
9a30189
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,7 +14,7 @@ Usage of webhook: | |
-hotreload | ||
watch hooks file for changes and reload them automatically | ||
-http-methods string | ||
globally restrict allowed HTTP methods; separate methods with comma | ||
set default allowed HTTP methods (ie. "POST"); separate methods with comma | ||
-ip string | ||
ip the webhook should serve hooks on (default "0.0.0.0") | ||
-key string | ||
|
@@ -23,6 +23,8 @@ Usage of webhook: | |
list available TLS cipher suites | ||
-logfile string | ||
send log output to a file; implicitly enables verbose logging | ||
-max-multipart-mem int | ||
maximum memory in bytes for parsing multipart form data before disk caching (default 1048576) | ||
-nopanic | ||
do not panic if hooks cannot be loaded when webhook is not running in verbose mode | ||
-pidfile string | ||
|
@@ -35,6 +37,8 @@ Usage of webhook: | |
set group ID after opening listening port; must be used with setuid | ||
-setuid int | ||
set user ID after opening listening port; must be used with setgid | ||
-socket string | ||
path to a Unix socket (e.g. /tmp/webhook.sock) or Windows named pipe (e.g. \\.\pipe\webhook) to use instead of listening on an ip and port; if specified, the ip and port options are ignored | ||
Comment on lines
+40
to
+41
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The actual |
||
-template | ||
parse hooks file as a Go template | ||
-tls-min-version string | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
//go:build !windows | ||
// +build !windows | ||
|
||
package main | ||
|
||
import ( | ||
"flag" | ||
"fmt" | ||
"net" | ||
) | ||
|
||
func platformFlags() { | ||
flag.StringVar(&socket, "socket", "", "path to a Unix socket (e.g. /tmp/webhook.sock) to use instead of listening on an ip and port; if specified, the ip and port options are ignored") | ||
} | ||
|
||
func trySocketListener() (net.Listener, error) { | ||
if socket != "" { | ||
addr = fmt.Sprintf("{unix:%s}", socket) | ||
return net.Listen("unix", socket) | ||
} | ||
return nil, nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
//go:build windows | ||
// +build windows | ||
|
||
package main | ||
|
||
import ( | ||
"flag" | ||
"fmt" | ||
"github.com/Microsoft/go-winio" | ||
"net" | ||
) | ||
|
||
func platformFlags() { | ||
flag.StringVar(&socket, "socket", "", "path to a Windows named pipe (e.g. \\\\.\\pipe\\webhook) to use instead of listening on an ip and port; if specified, the ip and port options are ignored") | ||
} | ||
|
||
func trySocketListener() (net.Listener, error) { | ||
if socket != "" { | ||
addr = fmt.Sprintf("{pipe:%s}", socket) | ||
return winio.ListenPipe(socket, nil) | ||
} | ||
return nil, nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
//go:build !windows | ||
// +build !windows | ||
|
||
package main | ||
|
@@ -6,6 +7,7 @@ import ( | |
"log" | ||
"os" | ||
"os/signal" | ||
"strings" | ||
"syscall" | ||
) | ||
|
||
|
@@ -43,6 +45,15 @@ func watchForSignals() { | |
log.Print(err) | ||
} | ||
} | ||
if socket != "" && !strings.HasPrefix(socket, "@") { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you give a socket path starting with |
||
// we've been listening on a named Unix socket, delete it | ||
// before we exit so subsequent runs can re-bind the same | ||
// socket path | ||
err := os.Remove(socket) | ||
Comment on lines
+49
to
+52
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will almost certainly error out in combination with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we should do it after dropping the privileges then? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did wonder about either delaying opening the socket until after setuid or just forbidding the use of both My gut feeling is that the cleanest option is to make it an error if both There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, that makes sense. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, ok, I was assuming if you agreed with my analysis then I’d implement that change before you merged… I’ll prepare the systemd PR and roll this fix into that. |
||
if err != nil { | ||
log.Printf("Failed to remove socket file %s: %v", socket, err) | ||
} | ||
} | ||
os.Exit(0) | ||
|
||
default: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
//go:build !windows | ||
// +build !windows | ||
|
||
package main | ||
|
||
import ( | ||
"context" | ||
"io/ioutil" | ||
"net" | ||
"net/http" | ||
"os" | ||
"path" | ||
) | ||
|
||
func prepareTestSocket(_ string) (socketPath string, transport *http.Transport, cleanup func(), err error) { | ||
tmp, err := ioutil.TempDir("", "webhook-socket-") | ||
if err != nil { | ||
return "", nil, nil, err | ||
} | ||
cleanup = func() { os.RemoveAll(tmp) } | ||
socketPath = path.Join(tmp, "webhook.sock") | ||
socketDialer := &net.Dialer{} | ||
transport = &http.Transport{ | ||
DialContext: func(ctx context.Context, _, _ string) (net.Conn, error) { | ||
return socketDialer.DialContext(ctx, "unix", socketPath) | ||
}, | ||
} | ||
|
||
return socketPath, transport, cleanup, nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
//go:build windows | ||
// +build windows | ||
|
||
package main | ||
|
||
import ( | ||
"context" | ||
"github.com/Microsoft/go-winio" | ||
"net" | ||
"net/http" | ||
) | ||
|
||
func prepareTestSocket(hookTmpl string) (socketPath string, transport *http.Transport, cleanup func(), err error) { | ||
socketPath = "\\\\.\\pipe\\webhook-" + hookTmpl | ||
transport = &http.Transport{ | ||
DialContext: func(ctx context.Context, _, _ string) (net.Conn, error) { | ||
return winio.DialPipeContext(ctx, socketPath) | ||
}, | ||
} | ||
|
||
return socketPath, transport, nil, nil | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't something I've changed, just something where the snapshot of the
-help
message in this file had drifted out of date with respect to the code.