cmd/tailscale: combine funnel and serve under dev flag

This PR combines the funnel and serve code under the same path.
However, it is using the new code which means features being
added to the funnel command will automatically be added to serve but
also things that are missing are missing from both.

Updates #8489

Signed-off-by: Marwan Sulaiman <marwan@tailscale.com>

cmd/tailscale/cli/cli.go

@@ -121,7 +121,7 @@ change in the future.
 			ncCmd,
 			sshCmd,
 			funnelCmd(),
-			serveCmd,
+			serveCmd(),
 			versionCmd,
 			webCmd,
 			fileCmd,

cmd/tailscale/cli/funnel_dev.go

@@ -4,16 +4,10 @@
 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.
@@ -37,7 +31,7 @@ func newFunnelDevCommand(e *serveEnv) *ffcli.Command {
 			"Note that it only supports https servers at this point.",
 			"This command is in development and is unsupported",
 		}, "\n"),
-		Exec:      e.runFunnelDev,
+		Exec:      e.runServeDev(true),
 		UsageFunc: usageFunc,
 		Subcommands: []*ffcli.Command{
 			{
@@ -52,61 +46,3 @@ func newFunnelDevCommand(e *serveEnv) *ffcli.Command {
 		},
 	}
 }
-
-// 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

@@ -32,7 +32,16 @@ import (
 	"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 os.Getenv("TAILSCALE_FUNNEL_DEV") == "on" {
+		return newServeDevCommand(se)
+	}
+	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,114 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+package cli
+
+import (
+	"context"
+	"flag"
+	"fmt"
+	"io"
+	"os"
+	"os/signal"
+	"strconv"
+	"strings"
+
+	"github.com/peterbourgon/ff/v3/ffcli"
+	"tailscale.com/ipn"
+)
+
+type execFunc func(ctx context.Context, args []string) error
+
+// newServeDevCommand returns a new "serve" subcommand using e as its environment.
+func newServeDevCommand(e *serveEnv) *ffcli.Command {
+	return &ffcli.Command{
+		Name:      "serve",
+		ShortHelp: "Serve content and local servers on your tailnet",
+		ShortUsage: strings.Join([]string{
+			"serve <port>",
+			"serve status [--json]",
+		}, "\n  "),
+		LongHelp: strings.TrimSpace(`
+The 'tailscale serve' set of commands allows you to serve
+content and local servers from your Tailscale node to
+your tailnet.
+`),
+		Exec:      e.runServeDev(false),
+		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,
+			},
+		},
+	}
+}
+
+// runServeDev is the entry point for the "tailscale serve|funnel" subcommand.
+//
+// Note: funnel is only supported on single DNS name for now. (2023-08-18)
+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.
+		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/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