diff --git a/cmd/tailscale/cli/serve_dev.go b/cmd/tailscale/cli/serve_dev.go index 967eb2680..79eb33bd5 100644 --- a/cmd/tailscale/cli/serve_dev.go +++ b/cmd/tailscale/cli/serve_dev.go @@ -266,6 +266,9 @@ func (e *serveEnv) runServeCombined(subcmd serveMode) execFunc { if turnOff { err = e.unsetServe(sc, dnsName, srvType, srvPort, mount) } else { + if err := validateConfig(sc, srvPort, srvType); err != nil { + return err + } err = e.setServe(sc, st, dnsName, srvType, srvPort, mount, args[0], funnel) msg = e.messageForPort(sc, st, dnsName, srvPort) } @@ -301,6 +304,54 @@ func (e *serveEnv) runServeCombined(subcmd serveMode) execFunc { } } +func validateConfig(sc *ipn.ServeConfig, port uint16, wantServe serveType) error { + sc, isFg := findConfig(sc, port) + if sc == nil { + return nil + } + if isFg { + return errors.New("foreground already exists under this port") + } + existingServe := serveFromPortHandler(sc.TCP[port]) + if wantServe != existingServe { + return fmt.Errorf("want %q but port is already serving %q", wantServe, existingServe) + } + return nil +} + +func serveFromPortHandler(tcp *ipn.TCPPortHandler) serveType { + switch { + case tcp.HTTP: + return serveTypeHTTP + case tcp.HTTPS: + return serveTypeHTTPS + case tcp.TerminateTLS != "": + return serveTypeTLSTerminatedTCP + case tcp.TCPForward != "": + return serveTypeTCP + default: + return -1 + } +} + +// findConfig finds a config that contains the given port, which can be +// the top level background config or an inner foreground one. The second +// result is true if it's foreground +func findConfig(sc *ipn.ServeConfig, port uint16) (*ipn.ServeConfig, bool) { + if sc == nil { + return nil, false + } + if _, ok := sc.TCP[port]; ok { + return sc, false + } + for _, sc := range sc.Foreground { + if _, ok := sc.TCP[port]; ok { + return sc, true + } + } + return nil, false +} + func (e *serveEnv) setServe(sc *ipn.ServeConfig, st *ipnstate.Status, dnsName string, srvType serveType, srvPort uint16, mount string, target string, allowFunnel bool) error { // update serve config based on the type switch srvType { @@ -745,13 +796,13 @@ func cleanURLPath(urlPath string) (string, error) { func (s serveType) String() string { switch s { case serveTypeHTTP: - return "httpListener" + return "http" case serveTypeHTTPS: - return "httpsListener" + return "https" case serveTypeTCP: - return "tcpListener" + return "tcp" case serveTypeTLSTerminatedTCP: - return "tlsTerminatedTCPListener" + return "tls-terminated-tcp" default: return "unknownServeType" } diff --git a/cmd/tailscale/cli/serve_dev_test.go b/cmd/tailscale/cli/serve_dev_test.go index 45b797562..61bb7aa48 100644 --- a/cmd/tailscale/cli/serve_dev_test.go +++ b/cmd/tailscale/cli/serve_dev_test.go @@ -697,7 +697,7 @@ type step struct { }) add(step{ // try to start a web handler on the same port command: cmd("serve --https=443 --bg localhost:3000"), - wantErr: exactErr(errHelp, "errHelp"), + wantErr: anyErr(), }) add(step{reset: true}) add(step{ // start a web handler on port 443 @@ -781,6 +781,106 @@ type step struct { } } +func TestValidateConfig(t *testing.T) { + tests := [...]struct { + name string + desc string + cfg *ipn.ServeConfig + servePort uint16 + serveType serveType + wantErr bool + }{ + { + name: "nil_config", + desc: "when config is nil, all requests valid", + cfg: nil, + servePort: 3000, + serveType: serveTypeHTTPS, + }, + { + name: "new_bg_tcp", + desc: "no error when config exists but we're adding a new bg tcp port", + cfg: &ipn.ServeConfig{ + TCP: map[uint16]*ipn.TCPPortHandler{ + 443: {HTTPS: true}, + }, + }, + servePort: 10000, + serveType: serveTypeHTTPS, + }, + { + name: "override_bg_tcp", + desc: "no error when overwriting previous port under the same serve type", + cfg: &ipn.ServeConfig{ + TCP: map[uint16]*ipn.TCPPortHandler{ + 443: {TCPForward: "http://localhost:4545"}, + }, + }, + servePort: 443, + serveType: serveTypeTCP, + }, + { + name: "override_bg_tcp", + desc: "error when overwriting previous port under a different serve type ", + cfg: &ipn.ServeConfig{ + TCP: map[uint16]*ipn.TCPPortHandler{ + 443: {HTTPS: true}, + }, + }, + servePort: 443, + serveType: serveTypeHTTP, + wantErr: true, + }, + { + name: "new_fg_port", + desc: "no error when serving a new foreground port", + cfg: &ipn.ServeConfig{ + TCP: map[uint16]*ipn.TCPPortHandler{ + 443: {HTTPS: true}, + }, + Foreground: map[string]*ipn.ServeConfig{ + "abc123": { + TCP: map[uint16]*ipn.TCPPortHandler{ + 3000: {HTTPS: true}, + }, + }, + }, + }, + servePort: 4040, + serveType: serveTypeTCP, + }, + { + name: "same_fg_port", + desc: "error when overwriting a previous fg port", + cfg: &ipn.ServeConfig{ + Foreground: map[string]*ipn.ServeConfig{ + "abc123": { + TCP: map[uint16]*ipn.TCPPortHandler{ + 3000: {HTTPS: true}, + }, + }, + }, + }, + servePort: 3000, + serveType: serveTypeTCP, + wantErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := validateConfig(tc.cfg, tc.servePort, tc.serveType) + if err == nil && tc.wantErr { + t.Fatal("expected an error but got nil") + } + if err != nil && !tc.wantErr { + t.Fatalf("expected no error but got: %v", err) + } + }) + } + +} + func TestSrcTypeFromFlags(t *testing.T) { tests := []struct { name string