cmd/tailscale: combine serve and funnel for debug wip funnel stream model (#9169)

> **Note**
> Behind the `TAILSCALE_USE_WIP_CODE` flag

In preparing for incoming CLI changes, this PR merges the code path for the `serve` and `funnel` subcommands.

See the parent issue for more context.

The following commands will run in foreground mode when using the environment flag.
```
tailscale serve localhost:3000
tailscae funnel localhost:3000
```

Replaces #9134
Updates #8489

Signed-off-by: Tyler Smalley <tyler@tailscale.com>
Signed-off-by: Marwan Sulaiman <marwan@tailscale.com>
Co-authored-by: Marwan Sulaiman <marwan@tailscale.com>

cmd/tailscale/cli/cli.go

@@ -121,7 +121,7 @@ func Run(args []string) (err error) {
 			ncCmd,
 			sshCmd,
 			funnelCmd(),
-			serveCmd,
+			serveCmd(),
 			versionCmd,
 			webCmd,
 			fileCmd,

cmd/tailscale/cli/funnel.go

@@ -14,6 +14,7 @@
 	"strings"
 
 	"github.com/peterbourgon/ff/v3/ffcli"
+	"tailscale.com/envknob"
 	"tailscale.com/ipn"
 	"tailscale.com/ipn/ipnstate"
 	"tailscale.com/tailcfg"
@@ -25,8 +26,8 @@
 	// This flag is used to switch to an in-development
 	// implementation of the tailscale funnel command.
 	// See https://github.com/tailscale/tailscale/issues/7844
-	if os.Getenv("TAILSCALE_FUNNEL_DEV") == "on" {
-		return newFunnelDevCommand(se)
+	if envknob.UseWIPCode() {
+		return newServeDevCommand(se, "funnel")
 	}
 	return newFunnelCommand(se)
 }

cmd/tailscale/cli/funnel_dev.go

@@ -1,112 +0,0 @@
-// Copyright (c) Tailscale Inc & AUTHORS
-// SPDX-License-Identifier: BSD-3-Clause
-
-package cli
-
-import (
-	"context"
-	"flag"
-	"fmt"
-	"io"
-	"os"
-	"strconv"
-	"strings"
-
-	"github.com/peterbourgon/ff/v3/ffcli"
-	"tailscale.com/ipn"
-)
-
-// newFunnelDevCommand returns a new "funnel" subcommand using e as its environment.
-// The funnel subcommand is used to turn on/off the Funnel service.
-// Funnel is off by default.
-// Funnel allows you to publish a 'tailscale serve' server publicly,
-// open to the entire internet.
-// newFunnelCommand shares the same serveEnv as the "serve" subcommand.
-// See newServeCommand and serve.go for more details.
-func newFunnelDevCommand(e *serveEnv) *ffcli.Command {
-	return &ffcli.Command{
-		Name:      "funnel",
-		ShortHelp: "Turn on/off Funnel service",
-		ShortUsage: strings.Join([]string{
-			"funnel <port>",
-			"funnel status [--json]",
-		}, "\n  "),
-		LongHelp: strings.Join([]string{
-			"Funnel allows you to expose your local",
-			"server publicly to the entire internet.",
-			"Note that it only supports https servers at this point.",
-			"This command is in development and is unsupported",
-		}, "\n"),
-		Exec:      e.runFunnelDev,
-		UsageFunc: usageFunc,
-		Subcommands: []*ffcli.Command{
-			{
-				Name:      "status",
-				Exec:      e.runServeStatus,
-				ShortHelp: "show current serve/Funnel status",
-				FlagSet: e.newFlags("funnel-status", func(fs *flag.FlagSet) {
-					fs.BoolVar(&e.json, "json", false, "output JSON")
-				}),
-				UsageFunc: usageFunc,
-			},
-		},
-	}
-}
-
-// runFunnelDev is the entry point for the "tailscale funnel" subcommand and
-// manages turning on/off Funnel. Funnel is off by default.
-//
-// Note: funnel is only supported on single DNS name for now. (2023-08-18)
-func (e *serveEnv) runFunnelDev(ctx context.Context, args []string) error {
-	if len(args) != 1 {
-		return flag.ErrHelp
-	}
-	var source string
-	port64, err := strconv.ParseUint(args[0], 10, 16)
-	if err == nil {
-		source = fmt.Sprintf("http://127.0.0.1:%d", port64)
-	} else {
-		source, err = expandProxyTarget(args[0])
-	}
-	if err != nil {
-		return err
-	}
-
-	st, err := e.getLocalClientStatusWithoutPeers(ctx)
-	if err != nil {
-		return fmt.Errorf("getting client status: %w", err)
-	}
-
-	if err := e.verifyFunnelEnabled(ctx, st, 443); err != nil {
-		return err
-	}
-
-	dnsName := strings.TrimSuffix(st.Self.DNSName, ".")
-	hp := ipn.HostPort(dnsName + ":443") // TODO(marwan-at-work): support the 2 other ports
-
-	// In the streaming case, the process stays running in the
-	// foreground and prints out connections to the HostPort.
-	//
-	// The local backend handles updating the ServeConfig as
-	// necessary, then restores it to its original state once
-	// the process's context is closed or the client turns off
-	// Tailscale.
-	return e.streamServe(ctx, ipn.ServeStreamRequest{
-		HostPort:   hp,
-		Source:     source,
-		MountPoint: "/", // TODO(marwan-at-work): support multiple mount points
-	})
-}
-
-func (e *serveEnv) streamServe(ctx context.Context, req ipn.ServeStreamRequest) error {
-	stream, err := e.lc.StreamServe(ctx, req)
-	if err != nil {
-		return err
-	}
-	defer stream.Close()
-
-	fmt.Fprintf(os.Stderr, "Funnel started on \"https://%s\".\n", strings.TrimSuffix(string(req.HostPort), ":443"))
-	fmt.Fprintf(os.Stderr, "Press Ctrl-C to stop Funnel.\n\n")
-	_, err = io.Copy(os.Stdout, stream)
-	return err
-}

cmd/tailscale/cli/serve.go

@@ -25,6 +25,7 @@
 
 	"github.com/peterbourgon/ff/v3/ffcli"
 	"tailscale.com/client/tailscale"
+	"tailscale.com/envknob"
 	"tailscale.com/ipn"
 	"tailscale.com/ipn/ipnstate"
 	"tailscale.com/tailcfg"
@@ -32,7 +33,16 @@
 	"tailscale.com/version"
 )
 
-var serveCmd = newServeCommand(&serveEnv{lc: &localClient})
+var serveCmd = func() *ffcli.Command {
+	se := &serveEnv{lc: &localClient}
+	// This flag is used to switch to an in-development
+	// implementation of the tailscale funnel command.
+	// See https://github.com/tailscale/tailscale/issues/7844
+	if envknob.UseWIPCode() {
+		return newServeDevCommand(se, "serve")
+	}
+	return newServeCommand(se)
+}
 
 // newServeCommand returns a new "serve" subcommand using e as its environment.
 func newServeCommand(e *serveEnv) *ffcli.Command {

cmd/tailscale/cli/serve_dev.go

@@ -0,0 +1,147 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+package cli
+
+import (
+	"context"
+	"flag"
+	"fmt"
+	"io"
+	"log"
+	"os"
+	"os/signal"
+	"strconv"
+	"strings"
+
+	"github.com/peterbourgon/ff/v3/ffcli"
+	"tailscale.com/ipn"
+)
+
+type execFunc func(ctx context.Context, args []string) error
+
+type commandInfo struct {
+	ShortHelp string
+	LongHelp  string
+}
+
+var infoMap = map[string]commandInfo{
+	"serve": {
+		ShortHelp: "Serve content and local servers on your tailnet",
+		LongHelp: strings.Join([]string{
+			"Serve lets you  share a local server securely within your tailnet.",
+			"To share a local server on the internet, use \"tailscale funnel\"",
+		}, "\n"),
+	},
+	"funnel": {
+		ShortHelp: "Serve content and local servers on the internet",
+		LongHelp: strings.Join([]string{
+			"Funnel lets you share a local server on the internet using Tailscale.",
+			"To share only within your tailnet, use \"tailscale serve\"",
+		}, "\n"),
+	},
+}
+
+// newServeDevCommand returns a new "serve" subcommand using e as its environment.
+func newServeDevCommand(e *serveEnv, subcmd string) *ffcli.Command {
+	if subcmd != "serve" && subcmd != "funnel" {
+		log.Fatalf("newServeDevCommand called with unknown subcmd %q", subcmd)
+	}
+
+	info := infoMap[subcmd]
+
+	return &ffcli.Command{
+		Name:      subcmd,
+		ShortHelp: info.ShortHelp,
+		ShortUsage: strings.Join([]string{
+			fmt.Sprintf("%s <target>", subcmd),
+			fmt.Sprintf("%s status [--json]", subcmd),
+			fmt.Sprintf("%s reset", subcmd),
+		}, "\n  "),
+		LongHelp:  info.LongHelp,
+		Exec:      e.runServeDev(subcmd == "funnel"),
+		UsageFunc: usageFunc,
+		Subcommands: []*ffcli.Command{
+			// TODO(tyler+marwan-at-work) Implement set, unset, and logs subcommands
+			{
+				Name:      "status",
+				Exec:      e.runServeStatus,
+				ShortHelp: "view current proxy configuration",
+				FlagSet: e.newFlags("serve-status", func(fs *flag.FlagSet) {
+					fs.BoolVar(&e.json, "json", false, "output JSON")
+				}),
+				UsageFunc: usageFunc,
+			},
+			{
+				Name:      "reset",
+				ShortHelp: "reset current serve/funnel config",
+				Exec:      e.runServeReset,
+				FlagSet:   e.newFlags("serve-reset", nil),
+				UsageFunc: usageFunc,
+			},
+		},
+	}
+}
+
+// runServeDev is the entry point for the "tailscale {serve,funnel}" commands.
+func (e *serveEnv) runServeDev(funnel bool) execFunc {
+	return func(ctx context.Context, args []string) error {
+		ctx, cancel := signal.NotifyContext(ctx, os.Interrupt)
+		defer cancel()
+		if len(args) != 1 {
+			return flag.ErrHelp
+		}
+		var source string
+		port64, err := strconv.ParseUint(args[0], 10, 16)
+		if err == nil {
+			source = fmt.Sprintf("http://127.0.0.1:%d", port64)
+		} else {
+			source, err = expandProxyTarget(args[0])
+		}
+		if err != nil {
+			return err
+		}
+
+		st, err := e.getLocalClientStatusWithoutPeers(ctx)
+		if err != nil {
+			return fmt.Errorf("getting client status: %w", err)
+		}
+
+		if funnel {
+			if err := e.verifyFunnelEnabled(ctx, st, 443); err != nil {
+				return err
+			}
+		}
+
+		dnsName := strings.TrimSuffix(st.Self.DNSName, ".")
+		hp := ipn.HostPort(dnsName + ":443") // TODO(marwan-at-work): support the 2 other ports
+
+		// In the streaming case, the process stays running in the
+		// foreground and prints out connections to the HostPort.
+		//
+		// The local backend handles updating the ServeConfig as
+		// necessary, then restores it to its original state once
+		// the process's context is closed or the client turns off
+		// Tailscale.
+		// TODO(tyler+marwan-at-work) support flag to run in the background
+		return e.streamServe(ctx, ipn.ServeStreamRequest{
+			Funnel:     funnel,
+			HostPort:   hp,
+			Source:     source,
+			MountPoint: "/", // TODO(marwan-at-work): support multiple mount points
+		})
+	}
+}
+
+func (e *serveEnv) streamServe(ctx context.Context, req ipn.ServeStreamRequest) error {
+	stream, err := e.lc.StreamServe(ctx, req)
+	if err != nil {
+		return err
+	}
+	defer stream.Close()
+
+	fmt.Fprintf(os.Stderr, "Serve started on \"https://%s\".\n", strings.TrimSuffix(string(req.HostPort), ":443"))
+	fmt.Fprintf(os.Stderr, "Press Ctrl-C to stop.\n\n")
+	_, err = io.Copy(os.Stdout, stream)
+	return err
+}

ipn/ipnlocal/serve.go

@@ -356,10 +356,12 @@ func setHandler(sc *ipn.ServeConfig, req ipn.ServeStreamRequest) {
 	wsc.Handlers[req.MountPoint] = &ipn.HTTPHandler{
 		Proxy: req.Source,
 	}
-	if sc.AllowFunnel == nil {
-		sc.AllowFunnel = make(map[ipn.HostPort]bool)
+	if req.Funnel {
+		if sc.AllowFunnel == nil {
+			sc.AllowFunnel = make(map[ipn.HostPort]bool)
+		}
+		sc.AllowFunnel[req.HostPort] = true
 	}
-	sc.AllowFunnel[req.HostPort] = true
 }
 
 func deleteHandler(sc *ipn.ServeConfig, req ipn.ServeStreamRequest, port uint16) {

ipn/localapi/localapi.go

@@ -864,6 +864,10 @@ func (h *Handler) serveServeConfig(w http.ResponseWriter, r *http.Request) {
 // serveStreamServe handles foreground serve and funnel streams. This is
 // currently in development per https://github.com/tailscale/tailscale/issues/8489
 func (h *Handler) serveStreamServe(w http.ResponseWriter, r *http.Request) {
+	if !envknob.UseWIPCode() {
+		http.Error(w, "stream serve not yet available", http.StatusNotImplemented)
+		return
+	}
 	if !h.PermitWrite {
 		// Write permission required because we modify the ServeConfig.
 		http.Error(w, "serve stream denied", http.StatusForbidden)

ipn/serve.go

@@ -93,6 +93,10 @@ type ServeStreamRequest struct {
 	// MountPoint is the path prefix for
 	// the given HostPort.
 	MountPoint string `json:",omitempty"`
+
+	// Funnel indicates whether the request
+	// is a serve request or a funnel one.
+	Funnel bool `json:",omitempty"`
 }
 
 // FunnelRequestLog is the JSON type written out to io.Writers