From 35d7b3aa2736719cb3aaedbf0b8e4bd6eb797e28 Mon Sep 17 00:00:00 2001 From: Tyler Smalley Date: Mon, 23 Oct 2023 13:24:21 -0700 Subject: [PATCH] cmd/tailscale/cli: updates help and background messaging (#9929) * Fixes issue with template string not being provided in help text * Updates background information to provide full URL, including path, to make it clear the source and destination * Restores some tests * Removes AllowFunnel in ServeConfig if no proxy exists for that port. updates #8489 Signed-off-by: Tyler Smalley --- cmd/tailscale/cli/serve_v2.go | 47 +++++++----- cmd/tailscale/cli/serve_v2_test.go | 116 +++++++++++++---------------- 2 files changed, 80 insertions(+), 83 deletions(-) diff --git a/cmd/tailscale/cli/serve_v2.go b/cmd/tailscale/cli/serve_v2.go index 61800409e..a2b8cae56 100644 --- a/cmd/tailscale/cli/serve_v2.go +++ b/cmd/tailscale/cli/serve_v2.go @@ -46,16 +46,13 @@ a partial URL (e.g., localhost:3000), or a full URL including a path (e.g., http EXAMPLES - Expose an HTTP server running at 127.0.0.1:3000 in the foreground: - $ tailscale %s 3000 + $ tailscale %[1]s 3000 - Expose an HTTP server running at 127.0.0.1:3000 in the background: - $ tailscale %s --bg 3000 - - - Expose an HTTPS server with a valid certificate at https://localhost:8443 - $ tailscale %s https://localhost:8443 + $ tailscale %[1]s --bg 3000 - Expose an HTTPS server with invalid or self-signed certificates at https://localhost:8443 - $ tailscale %s https+insecure://localhost:8443 + $ tailscale %[1]s https+insecure://localhost:8443 For more examples and use cases visit our docs site https://tailscale.com/kb/1247/funnel-serve-use-cases `) @@ -119,7 +116,7 @@ func newServeV2Command(e *serveEnv, subcmd serveMode) *ffcli.Command { fmt.Sprintf("%s status [--json]", info.Name), fmt.Sprintf("%s reset", info.Name), }, "\n "), - LongHelp: info.LongHelp + fmt.Sprintf(strings.TrimSpace(serveHelpCommon), info.Name, info.Name), + LongHelp: info.LongHelp + fmt.Sprintf(strings.TrimSpace(serveHelpCommon), info.Name), Exec: e.runServeCombined(subcmd), FlagSet: e.newFlags("serve-set", func(fs *flag.FlagSet) { @@ -397,6 +394,10 @@ func (e *serveEnv) setServe(sc *ipn.ServeConfig, st *ipnstate.Status, dnsName st return fmt.Errorf("failed apply web serve: %w", err) } case serveTypeTCP, serveTypeTLSTerminatedTCP: + if e.setPath != "" { + return fmt.Errorf("cannot mount a path for TCP serve") + } + err := e.applyTCPServe(sc, dnsName, srvType, srvPort, target) if err != nil { return fmt.Errorf("failed to apply TCP serve: %w", err) @@ -431,7 +432,7 @@ func (e *serveEnv) messageForPort(sc *ipn.ServeConfig, st *ipnstate.Status, dnsN } else { output.WriteString(msgServeAvailable) } - output.WriteString("\n") + output.WriteString("\n\n") scheme := "https" if sc.IsServingHTTP(srvPort) { @@ -444,13 +445,6 @@ func (e *serveEnv) messageForPort(sc *ipn.ServeConfig, st *ipnstate.Status, dnsN portPart = "" } - output.WriteString(fmt.Sprintf("%s://%s%s\n\n", scheme, dnsName, portPart)) - - if !e.bg { - output.WriteString(msgToExit) - return output.String() - } - srvTypeAndDesc := func(h *ipn.HTTPHandler) (string, string) { switch { case h.Path != "": @@ -472,12 +466,12 @@ func (e *serveEnv) messageForPort(sc *ipn.ServeConfig, st *ipnstate.Status, dnsN sort.Slice(mounts, func(i, j int) bool { return len(mounts[i]) < len(mounts[j]) }) - maxLen := len(mounts[len(mounts)-1]) for _, m := range mounts { h := sc.Web[hp].Handlers[m] t, d := srvTypeAndDesc(h) - output.WriteString(fmt.Sprintf("%s %s%s %-5s %s\n", "|--", m, strings.Repeat(" ", maxLen-len(m)), t, d)) + output.WriteString(fmt.Sprintf("%s://%s%s%s\n", scheme, dnsName, portPart, m)) + output.WriteString(fmt.Sprintf("%s %-5s %s\n\n", "|--", t, d)) } } else if sc.TCP[srvPort] != nil { h := sc.TCP[srvPort] @@ -487,6 +481,7 @@ func (e *serveEnv) messageForPort(sc *ipn.ServeConfig, st *ipnstate.Status, dnsN tlsStatus = "TLS terminated" } + output.WriteString(fmt.Sprintf("%s://%s%s\n", scheme, dnsName, portPart)) output.WriteString(fmt.Sprintf("|-- tcp://%s (%s)\n", hp, tlsStatus)) for _, a := range st.TailscaleIPs { ipp := net.JoinHostPort(a.String(), strconv.Itoa(int(srvPort))) @@ -495,11 +490,15 @@ func (e *serveEnv) messageForPort(sc *ipn.ServeConfig, st *ipnstate.Status, dnsN output.WriteString(fmt.Sprintf("|--> tcp://%s\n", h.TCPForward)) } + if !e.bg { + output.WriteString(msgToExit) + return output.String() + } + subCmd := infoMap[e.subcmd].Name - subCmdSentance := strings.ToUpper(string(subCmd[0])) + subCmd[1:] + subCmdUpper := strings.ToUpper(string(subCmd[0])) + subCmd[1:] - output.WriteString("\n") - output.WriteString(fmt.Sprintf(msgRunningInBackground, subCmdSentance)) + output.WriteString(fmt.Sprintf(msgRunningInBackground, subCmdUpper)) output.WriteString("\n") output.WriteString(fmt.Sprintf(msgDisableProxy, subCmd, srvType.String(), srvPort)) @@ -623,6 +622,9 @@ func (e *serveEnv) applyFunnel(sc *ipn.ServeConfig, dnsName string, srvPort uint // TODO: add error handling for if toggling for existing sc if allowFunnel { mak.Set(&sc.AllowFunnel, hp, true) + } else if _, exists := sc.AllowFunnel[hp]; exists { + fmt.Printf("Removing Funnel for %s\n", hp) + delete(sc.AllowFunnel, hp) } } @@ -821,6 +823,7 @@ func (e *serveEnv) removeWebServe(sc *ipn.ServeConfig, dnsName string, srvPort u } if len(sc.Web[hp].Handlers) == 0 { delete(sc.Web, hp) + delete(sc.AllowFunnel, hp) delete(sc.TCP, srvPort) } @@ -838,6 +841,10 @@ func (e *serveEnv) removeWebServe(sc *ipn.ServeConfig, dnsName string, srvPort u delete(sc.AllowFunnel, hp) } + if len(sc.AllowFunnel) == 0 { + sc.AllowFunnel = nil + } + return nil } diff --git a/cmd/tailscale/cli/serve_v2_test.go b/cmd/tailscale/cli/serve_v2_test.go index 3c474bcbb..4fabf7504 100644 --- a/cmd/tailscale/cli/serve_v2_test.go +++ b/cmd/tailscale/cli/serve_v2_test.go @@ -586,65 +586,55 @@ func TestServeDevConfigMutations(t *testing.T) { }, }, }) - // TODO(tylersmalley) resolve these failures - // add(step{ // turn funnel off for primary port 443 - // command: cmd("serve --https=443 --set-path=/bar localhost:3001"), - // want: &ipn.ServeConfig{ - // AllowFunnel: map[ipn.HostPort]bool{"foo.test.ts.net:8443": true}, - // TCP: map[uint16]*ipn.TCPPortHandler{443: {HTTPS: true}, 8443: {HTTPS: true}}, - // Web: map[ipn.HostPort]*ipn.WebServerConfig{ - // "foo.test.ts.net:443": {Handlers: map[string]*ipn.HTTPHandler{ - // "/": {Proxy: "http://127.0.0.1:3000"}, - // }}, - // "foo.test.ts.net:8443": {Handlers: map[string]*ipn.HTTPHandler{ - // "/bar": {Proxy: "http://127.0.0.1:3001"}, - // }}, - // }, - // }, - // }) - // add(step{ // remove secondary port - // command: cmd("https:8443 /bar off"), - // want: &ipn.ServeConfig{ - // AllowFunnel: map[ipn.HostPort]bool{"foo.test.ts.net:8443": true}, - // TCP: map[uint16]*ipn.TCPPortHandler{443: {HTTPS: true}}, - // Web: map[ipn.HostPort]*ipn.WebServerConfig{ - // "foo.test.ts.net:443": {Handlers: map[string]*ipn.HTTPHandler{ - // "/": {Proxy: "http://127.0.0.1:3000"}, - // }}, - // }, - // }, - // }) - // add(step{ // start a tcp forwarder on 8443 - // command: cmd("tcp:8443 tcp://localhost:5432"), - // want: &ipn.ServeConfig{ - // AllowFunnel: map[ipn.HostPort]bool{"foo.test.ts.net:8443": true}, - // TCP: map[uint16]*ipn.TCPPortHandler{443: {HTTPS: true}, 8443: {TCPForward: "127.0.0.1:5432"}}, - // Web: map[ipn.HostPort]*ipn.WebServerConfig{ - // "foo.test.ts.net:443": {Handlers: map[string]*ipn.HTTPHandler{ - // "/": {Proxy: "http://127.0.0.1:3000"}, - // }}, - // }, - // }, - // }) - // add(step{ // remove primary port http handler - // command: cmd("https:443 / off"), - // want: &ipn.ServeConfig{ - // AllowFunnel: map[ipn.HostPort]bool{"foo.test.ts.net:8443": true}, - // TCP: map[uint16]*ipn.TCPPortHandler{8443: {TCPForward: "127.0.0.1:5432"}}, - // }, - // }) - // add(step{ // remove tcp forwarder - // command: cmd("tls-terminated-tcp:8443 off"), - // want: &ipn.ServeConfig{ - // AllowFunnel: map[ipn.HostPort]bool{"foo.test.ts.net:8443": true}, - // }, - // }) - // add(step{ // turn off funnel - // command: cmd("funnel 8443 off"), - // want: &ipn.ServeConfig{}, - // }) + add(step{ // turn funnel off for primary port 443 + command: cmd("serve --bg localhost:3000"), + want: &ipn.ServeConfig{ + AllowFunnel: map[ipn.HostPort]bool{"foo.test.ts.net:8443": true}, + TCP: map[uint16]*ipn.TCPPortHandler{443: {HTTPS: true}, 8443: {HTTPS: true}}, + Web: map[ipn.HostPort]*ipn.WebServerConfig{ + "foo.test.ts.net:443": {Handlers: map[string]*ipn.HTTPHandler{ + "/": {Proxy: "http://127.0.0.1:3000"}, + }}, + "foo.test.ts.net:8443": {Handlers: map[string]*ipn.HTTPHandler{ + "/bar": {Proxy: "http://127.0.0.1:3001"}, + }}, + }, + }, + }) + add(step{ // remove secondary port + command: cmd("serve --https=8443 --set-path=/bar off"), + want: &ipn.ServeConfig{ + TCP: map[uint16]*ipn.TCPPortHandler{443: {HTTPS: true}}, + Web: map[ipn.HostPort]*ipn.WebServerConfig{ + "foo.test.ts.net:443": {Handlers: map[string]*ipn.HTTPHandler{ + "/": {Proxy: "http://127.0.0.1:3000"}, + }}, + }, + }, + }) + add(step{ // start a tcp forwarder on 8443 + command: cmd("serve --bg --tcp=8443 tcp://localhost:5432"), + want: &ipn.ServeConfig{ + TCP: map[uint16]*ipn.TCPPortHandler{443: {HTTPS: true}, 8443: {TCPForward: "127.0.0.1:5432"}}, + Web: map[ipn.HostPort]*ipn.WebServerConfig{ + "foo.test.ts.net:443": {Handlers: map[string]*ipn.HTTPHandler{ + "/": {Proxy: "http://127.0.0.1:3000"}, + }}, + }, + }, + }) + add(step{ // remove primary port http handler + command: cmd("serve off"), + want: &ipn.ServeConfig{ + TCP: map[uint16]*ipn.TCPPortHandler{8443: {TCPForward: "127.0.0.1:5432"}}, + }, + }) + add(step{ // remove tcp forwarder + command: cmd("serve --tls-terminated-tcp=8443 off"), + want: &ipn.ServeConfig{}, + }) - // // tricky steps + // tricky steps add(step{reset: true}) add(step{ // a directory with a trailing slash mount point command: cmd("serve --https=443 --set-path=/dir " + filepath.Join(td, "subdir")), @@ -1123,9 +1113,9 @@ func TestMessageForPort(t *testing.T) { srvPort: 443, expected: strings.Join([]string{ msgFunnelAvailable, - "https://foo.test.ts.net", "", - "|-- / proxy http://127.0.0.1:3000", + "https://foo.test.ts.net/", + "|-- proxy http://127.0.0.1:3000", "", fmt.Sprintf(msgRunningInBackground, "Funnel"), fmt.Sprintf(msgDisableProxy, "funnel", "https", 443), @@ -1152,9 +1142,9 @@ func TestMessageForPort(t *testing.T) { srvPort: 80, expected: strings.Join([]string{ msgServeAvailable, - "https://foo.test.ts.net:80", "", - "|-- / proxy http://127.0.0.1:3000", + "https://foo.test.ts.net:80/", + "|-- proxy http://127.0.0.1:3000", "", fmt.Sprintf(msgRunningInBackground, "Serve"), fmt.Sprintf(msgDisableProxy, "serve", "http", 80), @@ -1173,7 +1163,7 @@ func TestMessageForPort(t *testing.T) { } if actual != tt.expected { - t.Errorf("Got: %q; expected: %q", actual, tt.expected) + t.Errorf("\nGot: %q\nExpected: %q", actual, tt.expected) } }) }