cmd/tailscale/cli: add set serve validations

This PR adds validations for the new new funnel/serve
commands under the following rules:
1. There is always a single config for one port (bg or fg).
2. Foreground configs under the same port cannot co-exists (for now).
3. Background configs can change as long as the serve type is the same.

Updates #8489

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

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"
 	}

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