cmd/tailscale/cli: include services in non-JSON serve status

The "tailscale serve status" human-readable output previously iterated
only sc.TCP, sc.Web, and sc.AllowFunnel, omitting any entries from
sc.Services. The --json output marshals the full ServeConfig and so
already showed services, leading users to think nothing was configured
when only services existed.

Extract the rendering loop into printServeStatusTrees and have it
also iterate sc.Services in deterministic order, prefixing each
service URL with the service name followed by a space (e.g.
"svc:db https://db.example.ts.net") so service entries are visually
distinct from node-level serves. Tun-mode services render as
"svc:<name> tun (L3 forwarding)".

Add tests that exercise node-only, service-only, and mixed configs,
plus a parity check that every service name and node Web hostport
visible in the JSON also appears in the human-readable output.

Updates tailscale/corp#34163

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Change-Id: Ie48858a8d8afd7184979d0fe2ab21ebd6fd0d4a0

cmd/tailscale/cli/serve_legacy.go

@@ -628,7 +628,7 @@ func (e *serveEnv) runServeStatus(ctx context.Context, args []string) error {
 		return nil
 	}
 	printFunnelStatus(ctx)
-	if sc == nil || (len(sc.TCP) == 0 && len(sc.Web) == 0 && len(sc.AllowFunnel) == 0) {
+	if isServeConfigEmpty(sc) {
 		printf("No serve config\n")
 		return nil
 	}
@@ -636,18 +636,8 @@ func (e *serveEnv) runServeStatus(ctx context.Context, args []string) error {
 	if err != nil {
 		return err
 	}
-	if sc.IsTCPForwardingAny() {
-		if err := printTCPStatusTree(ctx, sc, st); err != nil {
-			return err
-		}
-		printf("\n")
-	}
-	for hp := range sc.Web {
-		err := e.printWebStatusTree(sc, hp)
-		if err != nil {
-			return err
-		}
-		printf("\n")
+	if err := printServeStatusTrees(sc, st); err != nil {
+		return err
 	}
 	printFunnelWarning(sc)
 	return nil
@@ -678,7 +668,7 @@ func printTCPStatusTree(ctx context.Context, sc *ipn.ServeConfig, st *ipnstate.S
 	return nil
 }
 
-func (e *serveEnv) printWebStatusTree(sc *ipn.ServeConfig, hp ipn.HostPort) error {
+func printWebStatusTree(sc *ipn.ServeConfig, hp ipn.HostPort) error {
 	// No-op if no serve config
 	if sc == nil {
 		return nil
@@ -709,17 +699,6 @@ func (e *serveEnv) printWebStatusTree(sc *ipn.ServeConfig, hp ipn.HostPort) erro
 		printf("%s://%s%s (%s)\n", scheme, hostname, portPart, fStatus)
 	}
 	printf("%s://%s%s (%s)\n", scheme, host, portPart, fStatus)
-	srvTypeAndDesc := func(h *ipn.HTTPHandler) (string, string) {
-		switch {
-		case h.Path != "":
-			return "path", h.Path
-		case h.Proxy != "":
-			return "proxy", h.Proxy
-		case h.Text != "":
-			return "text", "\"" + elipticallyTruncate(h.Text, 20) + "\""
-		}
-		return "", ""
-	}
 
 	mounts := slicesx.MapKeys(sc.Web[hp].Handlers)
 	sort.Slice(mounts, func(i, j int) bool {
@@ -729,7 +708,7 @@ func (e *serveEnv) printWebStatusTree(sc *ipn.ServeConfig, hp ipn.HostPort) erro
 
 	for _, m := range mounts {
 		h := sc.Web[hp].Handlers[m]
-		t, d := srvTypeAndDesc(h)
+		t, d := serveHandlerDesc(h)
 		printf("%s %s%s %-5s %s\n", "|--", m, strings.Repeat(" ", maxLen-len(m)), t, d)
 	}
 

cmd/tailscale/cli/serve_status.go

@@ -0,0 +1,161 @@
+// Copyright (c) Tailscale Inc & contributors
+// SPDX-License-Identifier: BSD-3-Clause
+
+//go:build !ts_omit_serve
+
+package cli
+
+import (
+	"context"
+	"fmt"
+	"maps"
+	"net"
+	"slices"
+	"sort"
+	"strconv"
+	"strings"
+
+	"tailscale.com/ipn"
+	"tailscale.com/ipn/ipnstate"
+	"tailscale.com/tailcfg"
+	"tailscale.com/util/slicesx"
+)
+
+// isServeConfigEmpty reports whether sc has no user-visible configuration
+// to render in the non-JSON status output.
+func isServeConfigEmpty(sc *ipn.ServeConfig) bool {
+	return sc == nil || (len(sc.TCP) == 0 && len(sc.Web) == 0 && len(sc.Services) == 0 && len(sc.AllowFunnel) == 0)
+}
+
+// printServeStatusTrees prints the tree-style human-readable status of sc,
+// including any node-level TCP and Web serve entries and any configured
+// services, to the package Stdout. It does not print the funnel-status
+// header, the no-config message, or the trailing funnel warning — callers
+// are expected to handle those.
+//
+// Ordering is deterministic: node TCP forwards (existing behavior), then
+// node Web entries by HostPort, then services by name.
+func printServeStatusTrees(sc *ipn.ServeConfig, st *ipnstate.Status) error {
+	if sc == nil {
+		return nil
+	}
+	if sc.IsTCPForwardingAny() {
+		if err := printTCPStatusTree(context.Background(), sc, st); err != nil {
+			return err
+		}
+		printf("\n")
+	}
+	for _, hp := range slices.Sorted(maps.Keys(sc.Web)) {
+		if err := printWebStatusTree(sc, hp); err != nil {
+			return err
+		}
+		printf("\n")
+	}
+	for _, name := range slices.Sorted(maps.Keys(sc.Services)) {
+		if err := printServiceStatusTree(sc, st, name); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+// printServiceStatusTree prints the tree-style status for a single
+// configured service. Each rendered URL/forward line is prefixed with the
+// service name and a space (e.g. "svc:db https://db.example.ts.net") so
+// service entries are visually distinct from node-level serves.
+func printServiceStatusTree(sc *ipn.ServeConfig, st *ipnstate.Status, name tailcfg.ServiceName) error {
+	svc, ok := sc.Services[name]
+	if !ok || svc == nil {
+		return nil
+	}
+
+	if svc.Tun {
+		printf("%s tun (L3 forwarding)\n\n", name)
+		return nil
+	}
+
+	suffix := ""
+	if st != nil && st.CurrentTailnet != nil {
+		suffix = st.CurrentTailnet.MagicDNSSuffix
+	}
+	host := name.WithoutPrefix()
+	if suffix != "" {
+		host = host + "." + suffix
+	}
+
+	// TCP forwards configured directly on the service.
+	tcpPorts := slices.Sorted(maps.Keys(svc.TCP))
+	for _, p := range tcpPorts {
+		h := svc.TCP[p]
+		if h == nil || h.TCPForward == "" {
+			continue
+		}
+		tlsStatus := "TLS over TCP"
+		if h.TerminateTLS != "" {
+			tlsStatus = "TLS terminated"
+		}
+		hp := ipn.HostPort(net.JoinHostPort(host, strconv.Itoa(int(p))))
+		printf("%s tcp://%s (%s)\n", name, hp, tlsStatus)
+		printf("|--> tcp://%s\n\n", h.TCPForward)
+	}
+
+	// Web entries (HTTP/HTTPS).
+	for _, hp := range slices.Sorted(maps.Keys(svc.Web)) {
+		if err := printServiceWebStatusTree(sc, svc, name, hp); err != nil {
+			return err
+		}
+		printf("\n")
+	}
+
+	return nil
+}
+
+// printServiceWebStatusTree renders one entry of svc.Web for the given
+// service. It mirrors the layout of printWebStatusTree but uses
+// service-specific scheme/handler lookups via sc.IsServingHTTP(_, name).
+func printServiceWebStatusTree(sc *ipn.ServeConfig, svc *ipn.ServiceConfig, name tailcfg.ServiceName, hp ipn.HostPort) error {
+	host, portStr, _ := net.SplitHostPort(string(hp))
+	port, err := parseServePort(portStr)
+	if err != nil {
+		return fmt.Errorf("invalid port %q: %w", portStr, err)
+	}
+	scheme := "https"
+	if sc.IsServingHTTP(port, name) {
+		scheme = "http"
+	}
+	portPart := ":" + portStr
+	if scheme == "http" && portStr == "80" || scheme == "https" && portStr == "443" {
+		portPart = ""
+	}
+	printf("%s %s://%s%s\n", name, scheme, host, portPart)
+
+	web := svc.Web[hp]
+	if web == nil || len(web.Handlers) == 0 {
+		return nil
+	}
+	mounts := slicesx.MapKeys(web.Handlers)
+	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 := web.Handlers[m]
+		t, d := serveHandlerDesc(h)
+		printf("|-- %s%s %-5s %s\n", m, strings.Repeat(" ", maxLen-len(m)), t, d)
+	}
+	return nil
+}
+
+// serveHandlerDesc returns the type label and description for an HTTPHandler,
+// matching the format used by the existing node Web tree printer.
+func serveHandlerDesc(h *ipn.HTTPHandler) (string, string) {
+	switch {
+	case h.Path != "":
+		return "path", h.Path
+	case h.Proxy != "":
+		return "proxy", h.Proxy
+	case h.Text != "":
+		return "text", "\"" + elipticallyTruncate(h.Text, 20) + "\""
+	}
+	return "", ""
+}

cmd/tailscale/cli/serve_status_test.go

@@ -0,0 +1,233 @@
+// Copyright (c) Tailscale Inc & contributors
+// SPDX-License-Identifier: BSD-3-Clause
+
+//go:build !ts_omit_serve
+
+package cli
+
+import (
+	"bytes"
+	"encoding/json"
+	"io"
+	"strings"
+	"testing"
+
+	"tailscale.com/ipn"
+	"tailscale.com/ipn/ipnstate"
+	"tailscale.com/tailcfg"
+	"tailscale.com/tstest"
+)
+
+// statusTestStatus is a minimal ipnstate.Status used by serve-status tests.
+var statusTestStatus = &ipnstate.Status{
+	BackendState: ipn.Running.String(),
+	Self: &ipnstate.PeerStatus{
+		DNSName: "foo.test.ts.net.",
+	},
+	CurrentTailnet: &ipnstate.TailnetStatus{MagicDNSSuffix: "test.ts.net"},
+}
+
+func TestPrintServeStatusTrees(t *testing.T) {
+	tests := []struct {
+		name    string
+		sc      *ipn.ServeConfig
+		wantSub []string // substrings that must appear in the output
+		notWant []string // substrings that must NOT appear
+	}{
+		{
+			name: "node web tailnet only",
+			sc: &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"},
+					}},
+				},
+			},
+			wantSub: []string{
+				"https://foo.test.ts.net",
+				"tailnet only",
+				"proxy",
+				"http://127.0.0.1:3000",
+			},
+			notWant: []string{"Service ", "Funnel on"},
+		},
+		{
+			name: "node tcp funnel on",
+			sc: &ipn.ServeConfig{
+				TCP: map[uint16]*ipn.TCPPortHandler{2222: {TCPForward: "127.0.0.1:22"}},
+				AllowFunnel: map[ipn.HostPort]bool{
+					"foo.test.ts.net:2222": true,
+				},
+			},
+			wantSub: []string{
+				"tcp://foo.test.ts.net:2222",
+				"TLS over TCP",
+				"Funnel on",
+				"|--> tcp://127.0.0.1:22",
+			},
+			notWant: []string{"Service ", "tailnet only"},
+		},
+		{
+			name: "service web only",
+			sc: &ipn.ServeConfig{
+				Services: map[tailcfg.ServiceName]*ipn.ServiceConfig{
+					"svc:db": {
+						TCP: map[uint16]*ipn.TCPPortHandler{443: {HTTPS: true}},
+						Web: map[ipn.HostPort]*ipn.WebServerConfig{
+							"db.test.ts.net:443": {Handlers: map[string]*ipn.HTTPHandler{
+								"/": {Proxy: "http://127.0.0.1:5432"},
+							}},
+						},
+					},
+				},
+			},
+			wantSub: []string{
+				"svc:db https://db.test.ts.net",
+				"proxy",
+				"http://127.0.0.1:5432",
+			},
+			notWant: []string{"Funnel on", "Service svc:"},
+		},
+		{
+			name: "service tcp forward",
+			sc: &ipn.ServeConfig{
+				Services: map[tailcfg.ServiceName]*ipn.ServiceConfig{
+					"svc:ssh": {
+						TCP: map[uint16]*ipn.TCPPortHandler{2222: {TCPForward: "127.0.0.1:22"}},
+					},
+				},
+			},
+			wantSub: []string{
+				"svc:ssh tcp://ssh.test.ts.net:2222",
+				"|--> tcp://127.0.0.1:22",
+			},
+			notWant: []string{"Service svc:"},
+		},
+		{
+			name: "service tun",
+			sc: &ipn.ServeConfig{
+				Services: map[tailcfg.ServiceName]*ipn.ServiceConfig{
+					"svc:vpn": {Tun: true},
+				},
+			},
+			wantSub: []string{
+				"svc:vpn tun (L3 forwarding)",
+			},
+			notWant: []string{"https://", "tcp://", "Funnel on", "Service svc:"},
+		},
+		{
+			name: "node and services mixed",
+			sc: &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"},
+					}},
+				},
+				AllowFunnel: map[ipn.HostPort]bool{
+					"foo.test.ts.net:443": true,
+				},
+				Services: map[tailcfg.ServiceName]*ipn.ServiceConfig{
+					"svc:db": {
+						TCP: map[uint16]*ipn.TCPPortHandler{5432: {TCPForward: "127.0.0.1:5432"}},
+					},
+				},
+			},
+			wantSub: []string{
+				"https://foo.test.ts.net",
+				"Funnel on",
+				"svc:db tcp://db.test.ts.net:5432",
+				"|--> tcp://127.0.0.1:5432",
+			},
+			notWant: []string{"Service svc:"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var buf bytes.Buffer
+			tstest.Replace(t, &Stdout, io.Writer(&buf))
+			tstest.Replace(t, &Stderr, io.Discard)
+
+			if err := printServeStatusTrees(tt.sc, statusTestStatus); err != nil {
+				t.Fatalf("printServeStatusTrees: %v", err)
+			}
+			out := buf.String()
+			for _, s := range tt.wantSub {
+				if !strings.Contains(out, s) {
+					t.Errorf("output missing %q\n--- output ---\n%s", s, out)
+				}
+			}
+			for _, s := range tt.notWant {
+				if strings.Contains(out, s) {
+					t.Errorf("output unexpectedly contains %q\n--- output ---\n%s", s, out)
+				}
+			}
+		})
+	}
+}
+
+// TestPrintServeStatusTreesParity asserts that every service name and
+// HostPort key visible in the JSON serialization of a ServeConfig also
+// appears in the human-readable output. This is the parity contract from
+// issue #34163.
+func TestPrintServeStatusTreesParity(t *testing.T) {
+	sc := &ipn.ServeConfig{
+		TCP: map[uint16]*ipn.TCPPortHandler{
+			443:  {HTTPS: true},
+			2222: {TCPForward: "127.0.0.1:22"},
+		},
+		Web: map[ipn.HostPort]*ipn.WebServerConfig{
+			"foo.test.ts.net:443": {Handlers: map[string]*ipn.HTTPHandler{
+				"/": {Proxy: "http://127.0.0.1:3000"},
+			}},
+		},
+		AllowFunnel: map[ipn.HostPort]bool{
+			"foo.test.ts.net:2222": true,
+		},
+		Services: map[tailcfg.ServiceName]*ipn.ServiceConfig{
+			"svc:db": {
+				TCP: map[uint16]*ipn.TCPPortHandler{5432: {TCPForward: "127.0.0.1:5432"}},
+			},
+			"svc:web": {
+				TCP: map[uint16]*ipn.TCPPortHandler{443: {HTTPS: true}},
+				Web: map[ipn.HostPort]*ipn.WebServerConfig{
+					"web.test.ts.net:443": {Handlers: map[string]*ipn.HTTPHandler{
+						"/api": {Proxy: "http://127.0.0.1:9000"},
+					}},
+				},
+			},
+			"svc:vpn": {Tun: true},
+		},
+	}
+
+	// JSON dump; just verify it's non-empty so we don't assert on
+	// schema-internal field names.
+	if _, err := json.MarshalIndent(sc, "", "  "); err != nil {
+		t.Fatalf("MarshalIndent: %v", err)
+	}
+
+	var buf bytes.Buffer
+	tstest.Replace(t, &Stdout, io.Writer(&buf))
+	tstest.Replace(t, &Stderr, io.Discard)
+
+	if err := printServeStatusTrees(sc, statusTestStatus); err != nil {
+		t.Fatalf("printServeStatusTrees: %v", err)
+	}
+	out := buf.String()
+
+	// Every service name in sc.Services must appear in the human output.
+	for name := range sc.Services {
+		if !strings.Contains(out, name.String()) {
+			t.Errorf("human output missing service %q\n%s", name, out)
+		}
+	}
+	// Every node-level Web HostPort must appear (host portion at least).
+	for hp := range sc.Web {
+		host := strings.SplitN(string(hp), ":", 2)[0]
+		if !strings.Contains(out, host) {
+			t.Errorf("human output missing node web host %q\n%s", host, out)
+		}
+	}
+}