{ipn/serve,cmd/tailscale/cli}: move some shared funcs to ipn

In preparation for changes to allow configuration of serve/funnel
from the web client, this commit moves some functionality that will
be shared between the CLI and web client to the ipn package's
serve.go file, where some other util funcs are already defined.

Updates #10261

Signed-off-by: Sonia Appasamy <sonia@tailscale.com>

cmd/tailscale/cli/serve_legacy.go

@@ -642,6 +642,9 @@ func (e *serveEnv) handleTCPServeRemove(ctx context.Context, src uint16) error {
 // Examples:
 //   - tailscale status
 //   - tailscale status --json
+//
+// TODO(tyler,marwan,sonia): `status` should also report foreground configs,
+// currently only reports background config.
 func (e *serveEnv) runServeStatus(ctx context.Context, args []string) error {
 	sc, err := e.lc.GetServeConfig(ctx)
 	if err != nil {

cmd/tailscale/cli/serve_v2.go

@@ -18,7 +18,6 @@ import (
 	"os/signal"
 	"path"
 	"path/filepath"
-	"slices"
 	"sort"
 	"strconv"
 	"strings"
@@ -334,7 +333,7 @@ func (e *serveEnv) runServeCombined(subcmd serveMode) execFunc {
 const backgroundExistsMsg = "background configuration already exists, use `tailscale %s --%s=%d off` to remove the existing configuration"
 
 func (e *serveEnv) validateConfig(sc *ipn.ServeConfig, port uint16, wantServe serveType) error {
-	sc, isFg := findConfig(sc, port)
+	sc, isFg := sc.FindConfig(port)
 	if sc == nil {
 		return nil
 	}
@@ -366,24 +365,6 @@ func serveFromPortHandler(tcp *ipn.TCPPortHandler) serveType {
 	}
 }
 
-// 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 {
@@ -535,7 +516,7 @@ func (e *serveEnv) applyWebServe(sc *ipn.ServeConfig, dnsName string, srvPort ui
 		}
 		h.Path = target
 	default:
-		t, err := expandProxyTargetDev(target, []string{"http", "https", "https+insecure"}, "http")
+		t, err := ipn.ExpandProxyTargetValue(target, []string{"http", "https", "https+insecure"}, "http")
 		if err != nil {
 			return err
 		}
@@ -585,7 +566,7 @@ func (e *serveEnv) applyTCPServe(sc *ipn.ServeConfig, dnsName string, srcType se
 		return fmt.Errorf("invalid TCP target %q", target)
 	}
 
-	targetURL, err := expandProxyTargetDev(target, []string{"tcp"}, "tcp")
+	targetURL, err := ipn.ExpandProxyTargetValue(target, []string{"tcp"}, "tcp")
 	if err != nil {
 		return fmt.Errorf("unable to expand target: %v", err)
 	}
@@ -865,60 +846,6 @@ func (e *serveEnv) removeTCPServe(sc *ipn.ServeConfig, src uint16) error {
 	return nil
 }
 
-// expandProxyTargetDev expands the supported target values to be proxied
-// allowing for input values to be a port number, a partial URL, or a full URL
-// including a path.
-//
-// examples:
-//   - 3000
-//   - localhost:3000
-//   - tcp://localhost:3000
-//   - http://localhost:3000
-//   - https://localhost:3000
-//   - https-insecure://localhost:3000
-//   - https-insecure://localhost:3000/foo
-func expandProxyTargetDev(target string, supportedSchemes []string, defaultScheme string) (string, error) {
-	const host = "127.0.0.1"
-
-	// support target being a port number
-	if port, err := strconv.ParseUint(target, 10, 16); err == nil {
-		return fmt.Sprintf("%s://%s:%d", defaultScheme, host, port), nil
-	}
-
-	// prepend scheme if not present
-	if !strings.Contains(target, "://") {
-		target = defaultScheme + "://" + target
-	}
-
-	// make sure we can parse the target
-	u, err := url.ParseRequestURI(target)
-	if err != nil {
-		return "", fmt.Errorf("invalid URL %w", err)
-	}
-
-	// ensure a supported scheme
-	if !slices.Contains(supportedSchemes, u.Scheme) {
-		return "", fmt.Errorf("must be a URL starting with one of the supported schemes: %v", supportedSchemes)
-	}
-
-	// validate the host.
-	switch u.Hostname() {
-	case "localhost", "127.0.0.1":
-	default:
-		return "", errors.New("only localhost or 127.0.0.1 proxies are currently supported")
-	}
-
-	// validate the port
-	port, err := strconv.ParseUint(u.Port(), 10, 16)
-	if err != nil || port == 0 {
-		return "", fmt.Errorf("invalid port %q", u.Port())
-	}
-
-	u.Host = fmt.Sprintf("%s:%d", host, port)
-
-	return u.String(), nil
-}
-
 // cleanURLPath ensures the path is clean and has a leading "/".
 func cleanURLPath(urlPath string) (string, error) {
 	if urlPath == "" {

cmd/tailscale/cli/serve_v2_test.go

@@ -1041,63 +1041,6 @@ func TestSrcTypeFromFlags(t *testing.T) {
 	}
 }
 
-func TestExpandProxyTargetDev(t *testing.T) {
-	tests := []struct {
-		name             string
-		input            string
-		defaultScheme    string
-		supportedSchemes []string
-		expected         string
-		wantErr          bool
-	}{
-		{name: "port-only", input: "8080", expected: "http://127.0.0.1:8080"},
-		{name: "hostname+port", input: "localhost:8080", expected: "http://127.0.0.1:8080"},
-		{name: "convert-localhost", input: "http://localhost:8080", expected: "http://127.0.0.1:8080"},
-		{name: "no-change", input: "http://127.0.0.1:8080", expected: "http://127.0.0.1:8080"},
-		{name: "include-path", input: "http://127.0.0.1:8080/foo", expected: "http://127.0.0.1:8080/foo"},
-		{name: "https-scheme", input: "https://localhost:8080", expected: "https://127.0.0.1:8080"},
-		{name: "https+insecure-scheme", input: "https+insecure://localhost:8080", expected: "https+insecure://127.0.0.1:8080"},
-		{name: "change-default-scheme", input: "localhost:8080", defaultScheme: "https", expected: "https://127.0.0.1:8080"},
-		{name: "change-supported-schemes", input: "localhost:8080", defaultScheme: "tcp", supportedSchemes: []string{"tcp"}, expected: "tcp://127.0.0.1:8080"},
-
-		// errors
-		{name: "invalid-port", input: "localhost:9999999", wantErr: true},
-		{name: "unsupported-scheme", input: "ftp://localhost:8080", expected: "", wantErr: true},
-		{name: "not-localhost", input: "https://tailscale.com:8080", expected: "", wantErr: true},
-		{name: "empty-input", input: "", expected: "", wantErr: true},
-	}
-
-	for _, tt := range tests {
-		defaultScheme := "http"
-		supportedSchemes := []string{"http", "https", "https+insecure"}
-
-		if tt.supportedSchemes != nil {
-			supportedSchemes = tt.supportedSchemes
-		}
-		if tt.defaultScheme != "" {
-			defaultScheme = tt.defaultScheme
-		}
-
-		t.Run(tt.name, func(t *testing.T) {
-			actual, err := expandProxyTargetDev(tt.input, supportedSchemes, defaultScheme)
-
-			if tt.wantErr == true && err == nil {
-				t.Errorf("Expected an error but got none")
-				return
-			}
-
-			if tt.wantErr == false && err != nil {
-				t.Errorf("Got an error, but didn't expect one: %v", err)
-				return
-			}
-
-			if actual != tt.expected {
-				t.Errorf("Got: %q; expected: %q", actual, tt.expected)
-			}
-		})
-	}
-}
-
 func TestCleanURLPath(t *testing.T) {
 	tests := []struct {
 		input    string

ipn/serve.go

@@ -9,6 +9,7 @@ import (
 	"net"
 	"net/netip"
 	"net/url"
+	"slices"
 	"strconv"
 	"strings"
 
@@ -234,6 +235,24 @@ func (sc *ServeConfig) IsServingHTTP(port uint16) bool {
 	return sc.TCP[port].HTTP
 }
 
+// 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 (sc *ServeConfig) FindConfig(port uint16) (*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
+}
+
 // IsFunnelOn reports whether if ServeConfig is currently allowing funnel
 // traffic for any host:port.
 //
@@ -257,19 +276,28 @@ func (sc *ServeConfig) IsFunnelOn() bool {
 // CheckFunnelAccess checks whether Funnel access is allowed for the given node
 // and port.
 // It checks:
-//  1. HTTPS is enabled on the Tailnet
+//  1. HTTPS is enabled on the tailnet
 //  2. the node has the "funnel" nodeAttr
 //  3. the port is allowed for Funnel
 //
 // The node arg should be the ipnstate.Status.Self node.
 func CheckFunnelAccess(port uint16, node *ipnstate.PeerStatus) error {
+	if err := NodeCanFunnel(node); err != nil {
+		return err
+	}
+	return CheckFunnelPort(port, node)
+}
+
+// NodeCanFunnel returns an error if the given node is not configured to allow
+// for Tailscale Funnel usage.
+func NodeCanFunnel(node *ipnstate.PeerStatus) error {
 	if !node.HasCap(tailcfg.CapabilityHTTPS) {
 		return errors.New("Funnel not available; HTTPS must be enabled. See https://tailscale.com/s/https.")
 	}
 	if !node.HasCap(tailcfg.NodeAttrFunnel) {
 		return errors.New("Funnel not available; \"funnel\" node attribute not set. See https://tailscale.com/s/no-funnel.")
 	}
-	return CheckFunnelPort(port, node)
+	return nil
 }
 
 // CheckFunnelPort checks whether the given port is allowed for Funnel.
@@ -355,6 +383,60 @@ func CheckFunnelPort(wantedPort uint16, node *ipnstate.PeerStatus) error {
 	return deny(portsStr)
 }
 
+// ExpandProxyTargetValue expands the supported target values to be proxied
+// allowing for input values to be a port number, a partial URL, or a full URL
+// including a path.
+//
+// examples:
+//   - 3000
+//   - localhost:3000
+//   - tcp://localhost:3000
+//   - http://localhost:3000
+//   - https://localhost:3000
+//   - https-insecure://localhost:3000
+//   - https-insecure://localhost:3000/foo
+func ExpandProxyTargetValue(target string, supportedSchemes []string, defaultScheme string) (string, error) {
+	const host = "127.0.0.1"
+
+	// support target being a port number
+	if port, err := strconv.ParseUint(target, 10, 16); err == nil {
+		return fmt.Sprintf("%s://%s:%d", defaultScheme, host, port), nil
+	}
+
+	// prepend scheme if not present
+	if !strings.Contains(target, "://") {
+		target = defaultScheme + "://" + target
+	}
+
+	// make sure we can parse the target
+	u, err := url.ParseRequestURI(target)
+	if err != nil {
+		return "", fmt.Errorf("invalid URL %w", err)
+	}
+
+	// ensure a supported scheme
+	if !slices.Contains(supportedSchemes, u.Scheme) {
+		return "", fmt.Errorf("must be a URL starting with one of the supported schemes: %v", supportedSchemes)
+	}
+
+	// validate the host.
+	switch u.Hostname() {
+	case "localhost", "127.0.0.1":
+	default:
+		return "", errors.New("only localhost or 127.0.0.1 proxies are currently supported")
+	}
+
+	// validate the port
+	port, err := strconv.ParseUint(u.Port(), 10, 16)
+	if err != nil || port == 0 {
+		return "", fmt.Errorf("invalid port %q", u.Port())
+	}
+
+	u.Host = fmt.Sprintf("%s:%d", host, port)
+
+	return u.String(), nil
+}
+
 // RangeOverTCPs ranges over both background and foreground TCPs.
 // If the returned bool from the given f is false, then this function stops
 // iterating immediately and does not check other foreground configs.

ipn/serve_test.go

@@ -126,3 +126,60 @@ func TestHasPathHandler(t *testing.T) {
 		})
 	}
 }
+
+func TestExpandProxyTargetDev(t *testing.T) {
+	tests := []struct {
+		name             string
+		input            string
+		defaultScheme    string
+		supportedSchemes []string
+		expected         string
+		wantErr          bool
+	}{
+		{name: "port-only", input: "8080", expected: "http://127.0.0.1:8080"},
+		{name: "hostname+port", input: "localhost:8080", expected: "http://127.0.0.1:8080"},
+		{name: "convert-localhost", input: "http://localhost:8080", expected: "http://127.0.0.1:8080"},
+		{name: "no-change", input: "http://127.0.0.1:8080", expected: "http://127.0.0.1:8080"},
+		{name: "include-path", input: "http://127.0.0.1:8080/foo", expected: "http://127.0.0.1:8080/foo"},
+		{name: "https-scheme", input: "https://localhost:8080", expected: "https://127.0.0.1:8080"},
+		{name: "https+insecure-scheme", input: "https+insecure://localhost:8080", expected: "https+insecure://127.0.0.1:8080"},
+		{name: "change-default-scheme", input: "localhost:8080", defaultScheme: "https", expected: "https://127.0.0.1:8080"},
+		{name: "change-supported-schemes", input: "localhost:8080", defaultScheme: "tcp", supportedSchemes: []string{"tcp"}, expected: "tcp://127.0.0.1:8080"},
+
+		// errors
+		{name: "invalid-port", input: "localhost:9999999", wantErr: true},
+		{name: "unsupported-scheme", input: "ftp://localhost:8080", expected: "", wantErr: true},
+		{name: "not-localhost", input: "https://tailscale.com:8080", expected: "", wantErr: true},
+		{name: "empty-input", input: "", expected: "", wantErr: true},
+	}
+
+	for _, tt := range tests {
+		defaultScheme := "http"
+		supportedSchemes := []string{"http", "https", "https+insecure"}
+
+		if tt.supportedSchemes != nil {
+			supportedSchemes = tt.supportedSchemes
+		}
+		if tt.defaultScheme != "" {
+			defaultScheme = tt.defaultScheme
+		}
+
+		t.Run(tt.name, func(t *testing.T) {
+			actual, err := ExpandProxyTargetValue(tt.input, supportedSchemes, defaultScheme)
+
+			if tt.wantErr == true && err == nil {
+				t.Errorf("Expected an error but got none")
+				return
+			}
+
+			if tt.wantErr == false && err != nil {
+				t.Errorf("Got an error, but didn't expect one: %v", err)
+				return
+			}
+
+			if actual != tt.expected {
+				t.Errorf("Got: %q; expected: %q", actual, tt.expected)
+			}
+		})
+	}
+}