From 19faaff95c6f32a2fae26f003b467fb623962d09 Mon Sep 17 00:00:00 2001 From: KevinLiang10 <37811973+KevinLiang10@users.noreply.github.com> Date: Tue, 22 Jul 2025 16:23:51 -0400 Subject: [PATCH] cmd/tailscale/cli: revert key for web config for services to FQDN (#16627) This commit reverts the key of Web field in ipn.ServiceConfig to use FQDN instead of service name for the host part of HostPort. This change is because k8s operator already build base on the assumption of the part being FQDN. We don't want to break the code with dependency. Fixes tailscale/corp#30695 Signed-off-by: KevinLiang10 <37811973+KevinLiang10@users.noreply.github.com> --- cmd/tailscale/cli/serve_legacy.go | 2 +- cmd/tailscale/cli/serve_legacy_test.go | 1 + cmd/tailscale/cli/serve_v2.go | 40 ++++++++++++++------------ cmd/tailscale/cli/serve_v2_test.go | 37 ++++++++++++------------ cmd/tsidp/tsidp.go | 2 +- ipn/ipnlocal/serve.go | 4 ++- ipn/serve.go | 10 +++---- 7 files changed, 51 insertions(+), 45 deletions(-) diff --git a/cmd/tailscale/cli/serve_legacy.go b/cmd/tailscale/cli/serve_legacy.go index 7c79f7f7b..1a05d0543 100644 --- a/cmd/tailscale/cli/serve_legacy.go +++ b/cmd/tailscale/cli/serve_legacy.go @@ -363,7 +363,7 @@ func (e *serveEnv) handleWebServe(ctx context.Context, srvPort uint16, useTLS bo return errHelp } - sc.SetWebHandler(h, dnsName, srvPort, mount, useTLS) + sc.SetWebHandler(h, dnsName, srvPort, mount, useTLS, noService.String()) if !reflect.DeepEqual(cursc, sc) { if err := e.lc.SetServeConfig(ctx, sc); err != nil { diff --git a/cmd/tailscale/cli/serve_legacy_test.go b/cmd/tailscale/cli/serve_legacy_test.go index 6b053fbd7..1ea76e72c 100644 --- a/cmd/tailscale/cli/serve_legacy_test.go +++ b/cmd/tailscale/cli/serve_legacy_test.go @@ -876,6 +876,7 @@ var fakeStatus = &ipnstate.Status{ tailcfg.CapabilityFunnelPorts + "?ports=443,8443": nil, }, }, + CurrentTailnet: &ipnstate.TailnetStatus{MagicDNSSuffix: "test.ts.net"}, } func (lc *fakeLocalServeClient) StatusWithoutPeers(ctx context.Context) (*ipnstate.Status, error) { diff --git a/cmd/tailscale/cli/serve_v2.go b/cmd/tailscale/cli/serve_v2.go index 8832a232d..056bfabb0 100644 --- a/cmd/tailscale/cli/serve_v2.go +++ b/cmd/tailscale/cli/serve_v2.go @@ -331,6 +331,7 @@ func (e *serveEnv) runServeCombined(subcmd serveMode) execFunc { return fmt.Errorf("getting client status: %w", err) } dnsName := strings.TrimSuffix(st.Self.DNSName, ".") + magicDNSSuffix := st.CurrentTailnet.MagicDNSSuffix // set parent serve config to always be persisted // at the top level, but a nested config might be @@ -394,7 +395,7 @@ func (e *serveEnv) runServeCombined(subcmd serveMode) execFunc { var msg string if turnOff { // only unset serve when trying to unset with type and port flags. - err = e.unsetServe(sc, st, dnsName, srvType, srvPort, mount) + err = e.unsetServe(sc, dnsName, srvType, srvPort, mount, magicDNSSuffix) } else { if err := e.validateConfig(parentSC, srvPort, srvType, svcName); err != nil { return err @@ -406,7 +407,7 @@ func (e *serveEnv) runServeCombined(subcmd serveMode) execFunc { if len(args) > 0 { target = args[0] } - err = e.setServe(sc, dnsName, srvType, srvPort, mount, target, funnel) + err = e.setServe(sc, dnsName, srvType, srvPort, mount, target, funnel, magicDNSSuffix) msg = e.messageForPort(sc, st, dnsName, srvType, srvPort) } if err != nil { @@ -585,12 +586,12 @@ func serveFromPortHandler(tcp *ipn.TCPPortHandler) serveType { } } -func (e *serveEnv) setServe(sc *ipn.ServeConfig, dnsName string, srvType serveType, srvPort uint16, mount string, target string, allowFunnel bool) error { +func (e *serveEnv) setServe(sc *ipn.ServeConfig, dnsName string, srvType serveType, srvPort uint16, mount string, target string, allowFunnel bool, mds string) error { // update serve config based on the type switch srvType { case serveTypeHTTPS, serveTypeHTTP: useTLS := srvType == serveTypeHTTPS - err := e.applyWebServe(sc, dnsName, srvPort, useTLS, mount, target) + err := e.applyWebServe(sc, dnsName, srvPort, useTLS, mount, target, mds) if err != nil { return fmt.Errorf("failed apply web serve: %w", err) } @@ -643,11 +644,10 @@ func (e *serveEnv) messageForPort(sc *ipn.ServeConfig, st *ipnstate.Status, dnsN var webConfig *ipn.WebServerConfig var tcpHandler *ipn.TCPPortHandler ips := st.TailscaleIPs + magicDNSSuffix := st.CurrentTailnet.MagicDNSSuffix host := dnsName - displayedHost := dnsName if forService { - displayedHost = strings.Join([]string{svcName.WithoutPrefix(), st.CurrentTailnet.MagicDNSSuffix}, ".") - host = svcName.WithoutPrefix() + host = strings.Join([]string{svcName.WithoutPrefix(), magicDNSSuffix}, ".") } hp := ipn.HostPort(net.JoinHostPort(host, strconv.Itoa(int(srvPort)))) @@ -687,7 +687,7 @@ func (e *serveEnv) messageForPort(sc *ipn.ServeConfig, st *ipnstate.Status, dnsN output.WriteString("\n\n") svc := sc.Services[svcName] if srvType == serveTypeTUN && svc.Tun { - output.WriteString(fmt.Sprintf(msgRunningTunService, displayedHost)) + output.WriteString(fmt.Sprintf(msgRunningTunService, host)) output.WriteString("\n") output.WriteString(fmt.Sprintf(msgDisableServiceTun, dnsName)) output.WriteString("\n") @@ -716,7 +716,7 @@ func (e *serveEnv) messageForPort(sc *ipn.ServeConfig, st *ipnstate.Status, dnsN }) for _, m := range mounts { t, d := srvTypeAndDesc(webConfig.Handlers[m]) - output.WriteString(fmt.Sprintf("%s://%s%s%s\n", scheme, displayedHost, portPart, m)) + output.WriteString(fmt.Sprintf("%s://%s%s%s\n", scheme, host, portPart, m)) output.WriteString(fmt.Sprintf("%s %-5s %s\n\n", "|--", t, d)) } } else if tcpHandler != nil { @@ -726,7 +726,7 @@ func (e *serveEnv) messageForPort(sc *ipn.ServeConfig, st *ipnstate.Status, dnsN tlsStatus = "TLS terminated" } - output.WriteString(fmt.Sprintf("|-- tcp://%s:%d (%s)\n", displayedHost, srvPort, tlsStatus)) + output.WriteString(fmt.Sprintf("|-- tcp://%s:%d (%s)\n", host, srvPort, tlsStatus)) for _, a := range ips { ipp := net.JoinHostPort(a.String(), strconv.Itoa(int(srvPort))) output.WriteString(fmt.Sprintf("|-- tcp://%s\n", ipp)) @@ -755,7 +755,7 @@ func (e *serveEnv) messageForPort(sc *ipn.ServeConfig, st *ipnstate.Status, dnsN return output.String() } -func (e *serveEnv) applyWebServe(sc *ipn.ServeConfig, dnsName string, srvPort uint16, useTLS bool, mount, target string) error { +func (e *serveEnv) applyWebServe(sc *ipn.ServeConfig, dnsName string, srvPort uint16, useTLS bool, mount, target string, mds string) error { h := new(ipn.HTTPHandler) switch { case strings.HasPrefix(target, "text:"): @@ -797,7 +797,7 @@ func (e *serveEnv) applyWebServe(sc *ipn.ServeConfig, dnsName string, srvPort ui return errors.New("cannot serve web; already serving TCP") } - sc.SetWebHandler(h, dnsName, srvPort, mount, useTLS) + sc.SetWebHandler(h, dnsName, srvPort, mount, useTLS, mds) return nil } @@ -850,11 +850,12 @@ func (e *serveEnv) applyFunnel(sc *ipn.ServeConfig, dnsName string, srvPort uint } // unsetServe removes the serve config for the given serve port. -// dnsName is a FQDN or a serviceName (with `svc:` prefix). -func (e *serveEnv) unsetServe(sc *ipn.ServeConfig, st *ipnstate.Status, dnsName string, srvType serveType, srvPort uint16, mount string) error { +// dnsName is a FQDN or a serviceName (with `svc:` prefix). mds +// is the Magic DNS suffix, which is used to recreate serve's host. +func (e *serveEnv) unsetServe(sc *ipn.ServeConfig, dnsName string, srvType serveType, srvPort uint16, mount string, mds string) error { switch srvType { case serveTypeHTTPS, serveTypeHTTP: - err := e.removeWebServe(sc, st, dnsName, srvPort, mount) + err := e.removeWebServe(sc, dnsName, srvPort, mount, mds) if err != nil { return fmt.Errorf("failed to remove web serve: %w", err) } @@ -1010,8 +1011,9 @@ func isLegacyInvocation(subcmd serveMode, args []string) (string, bool) { // removeWebServe removes a web handler from the serve config // and removes funnel if no remaining mounts exist for the serve port. // The srvPort argument is the serving port and the mount argument is -// the mount point or registered path to remove. -func (e *serveEnv) removeWebServe(sc *ipn.ServeConfig, st *ipnstate.Status, dnsName string, srvPort uint16, mount string) error { +// the mount point or registered path to remove. mds is the Magic DNS suffix, +// which is used to recreate serve's host. +func (e *serveEnv) removeWebServe(sc *ipn.ServeConfig, dnsName string, srvPort uint16, mount string, mds string) error { if sc == nil { return nil } @@ -1026,7 +1028,7 @@ func (e *serveEnv) removeWebServe(sc *ipn.ServeConfig, st *ipnstate.Status, dnsN if svc == nil { return errors.New("service does not exist") } - hostName = svcName.WithoutPrefix() + hostName = strings.Join([]string{svcName.WithoutPrefix(), mds}, ".") webServeMap = svc.Web } @@ -1063,7 +1065,7 @@ func (e *serveEnv) removeWebServe(sc *ipn.ServeConfig, st *ipnstate.Status, dnsN } if forService { - sc.RemoveServiceWebHandler(st, svcName, srvPort, mounts) + sc.RemoveServiceWebHandler(svcName, hostName, srvPort, mounts) } else { sc.RemoveWebHandler(dnsName, srvPort, mounts, true) } diff --git a/cmd/tailscale/cli/serve_v2_test.go b/cmd/tailscale/cli/serve_v2_test.go index 2ba0b3f84..95bf5b101 100644 --- a/cmd/tailscale/cli/serve_v2_test.go +++ b/cmd/tailscale/cli/serve_v2_test.go @@ -1299,7 +1299,7 @@ func TestMessageForPort(t *testing.T) { "foo.test.ts.net:443": true, }, }, - status: &ipnstate.Status{}, + status: &ipnstate.Status{CurrentTailnet: &ipnstate.TailnetStatus{MagicDNSSuffix: "test.ts.net"}}, dnsName: "foo.test.ts.net", srvType: serveTypeHTTPS, srvPort: 443, @@ -1328,7 +1328,7 @@ func TestMessageForPort(t *testing.T) { }, }, }, - status: &ipnstate.Status{}, + status: &ipnstate.Status{CurrentTailnet: &ipnstate.TailnetStatus{MagicDNSSuffix: "test.ts.net"}}, dnsName: "foo.test.ts.net", srvType: serveTypeHTTP, srvPort: 80, @@ -1352,7 +1352,7 @@ func TestMessageForPort(t *testing.T) { 80: {HTTP: true}, }, Web: map[ipn.HostPort]*ipn.WebServerConfig{ - "foo:80": { + "foo.test.ts.net:80": { Handlers: map[string]*ipn.HTTPHandler{ "/": {Proxy: "http://localhost:3000"}, }, @@ -1396,7 +1396,7 @@ func TestMessageForPort(t *testing.T) { 80: {HTTP: true}, }, Web: map[ipn.HostPort]*ipn.WebServerConfig{ - "bar:80": { + "bar.test.ts.net:80": { Handlers: map[string]*ipn.HTTPHandler{ "/": {Proxy: "http://localhost:3000"}, }, @@ -1440,7 +1440,7 @@ func TestMessageForPort(t *testing.T) { 2200: {HTTPS: true}, }, Web: map[ipn.HostPort]*ipn.WebServerConfig{ - "foo:2200": { + "foo.test.ts.net:2200": { Handlers: map[string]*ipn.HTTPHandler{ "/": {Proxy: "http://localhost:3000"}, }, @@ -1670,6 +1670,7 @@ func TestIsLegacyInvocation(t *testing.T) { func TestSetServe(t *testing.T) { e := &serveEnv{} + magicDNSSuffix := "test.ts.net" tests := []struct { name string desc string @@ -1816,7 +1817,7 @@ func TestSetServe(t *testing.T) { "svc:bar": { TCP: map[uint16]*ipn.TCPPortHandler{80: {HTTP: true}}, Web: map[ipn.HostPort]*ipn.WebServerConfig{ - "bar:80": { + "bar.test.ts.net:80": { Handlers: map[string]*ipn.HTTPHandler{ "/": {Proxy: "http://localhost:3000"}, }, @@ -1834,7 +1835,7 @@ func TestSetServe(t *testing.T) { "svc:bar": { TCP: map[uint16]*ipn.TCPPortHandler{80: {HTTP: true}}, Web: map[ipn.HostPort]*ipn.WebServerConfig{ - "bar:80": { + "bar.test.ts.net:80": { Handlers: map[string]*ipn.HTTPHandler{ "/": {Proxy: "http://localhost:3000"}, }, @@ -1853,7 +1854,7 @@ func TestSetServe(t *testing.T) { "svc:bar": { TCP: map[uint16]*ipn.TCPPortHandler{80: {HTTP: true}}, Web: map[ipn.HostPort]*ipn.WebServerConfig{ - "bar:80": { + "bar.test.ts.net:80": { Handlers: map[string]*ipn.HTTPHandler{ "/": {Proxy: "http://localhost:3001"}, }, @@ -1871,7 +1872,7 @@ func TestSetServe(t *testing.T) { "svc:bar": { TCP: map[uint16]*ipn.TCPPortHandler{80: {HTTP: true}}, Web: map[ipn.HostPort]*ipn.WebServerConfig{ - "bar:80": { + "bar.test.ts.net:80": { Handlers: map[string]*ipn.HTTPHandler{ "/": {Proxy: "http://localhost:3000"}, }, @@ -1893,12 +1894,12 @@ func TestSetServe(t *testing.T) { 88: {HTTP: true}, }, Web: map[ipn.HostPort]*ipn.WebServerConfig{ - "bar:80": { + "bar.test.ts.net:80": { Handlers: map[string]*ipn.HTTPHandler{ "/": {Proxy: "http://localhost:3000"}, }, }, - "bar:88": { + "bar.test.ts.net:88": { Handlers: map[string]*ipn.HTTPHandler{ "/": {Proxy: "http://localhost:3001"}, }, @@ -1916,7 +1917,7 @@ func TestSetServe(t *testing.T) { "svc:bar": { TCP: map[uint16]*ipn.TCPPortHandler{80: {HTTP: true}}, Web: map[ipn.HostPort]*ipn.WebServerConfig{ - "bar:80": { + "bar.test.ts.net:80": { Handlers: map[string]*ipn.HTTPHandler{ "/": {Proxy: "http://localhost:3000"}, }, @@ -1937,7 +1938,7 @@ func TestSetServe(t *testing.T) { 80: {HTTP: true}, }, Web: map[ipn.HostPort]*ipn.WebServerConfig{ - "bar:80": { + "bar.test.ts.net:80": { Handlers: map[string]*ipn.HTTPHandler{ "/": {Proxy: "http://localhost:3000"}, "/added": {Proxy: "http://localhost:3001"}, @@ -1965,7 +1966,7 @@ func TestSetServe(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := e.setServe(tt.cfg, tt.dnsName, tt.srvType, tt.srvPort, tt.mountPath, tt.target, tt.allowFunnel) + err := e.setServe(tt.cfg, tt.dnsName, tt.srvType, tt.srvPort, tt.mountPath, tt.target, tt.allowFunnel, magicDNSSuffix) if err != nil && !tt.expectErr { t.Fatalf("got error: %v; did not expect error.", err) } @@ -2030,7 +2031,7 @@ func TestUnsetServe(t *testing.T) { 80: {HTTP: true}, }, Web: map[ipn.HostPort]*ipn.WebServerConfig{ - "bar:80": { + "bar.test.ts.net:80": { Handlers: map[string]*ipn.HTTPHandler{ "/": {Proxy: "http://localhost:3000"}, }, @@ -2124,7 +2125,7 @@ func TestUnsetServe(t *testing.T) { 80: {HTTP: true}, }, Web: map[ipn.HostPort]*ipn.WebServerConfig{ - "bar:80": { + "bar.test.ts.net:80": { Handlers: map[string]*ipn.HTTPHandler{ "/": {Proxy: "http://localhost:3000"}, }, @@ -2199,7 +2200,7 @@ func TestUnsetServe(t *testing.T) { 80: {HTTP: true}, }, Web: map[ipn.HostPort]*ipn.WebServerConfig{ - "bar:80": { + "bar.test.ts.net:80": { Handlers: map[string]*ipn.HTTPHandler{ "/": {Proxy: "http://localhost:3000"}, }, @@ -2224,7 +2225,7 @@ func TestUnsetServe(t *testing.T) { if tt.setServeEnv { e = tt.serveEnv } - err := e.unsetServe(tt.cfg, tt.st, tt.dnsName, tt.srvType, tt.srvPort, tt.mount) + err := e.unsetServe(tt.cfg, tt.dnsName, tt.srvType, tt.srvPort, tt.mount, tt.st.CurrentTailnet.MagicDNSSuffix) if err != nil && !tt.expectErr { t.Fatalf("got error: %v; did not expect error.", err) } diff --git a/cmd/tsidp/tsidp.go b/cmd/tsidp/tsidp.go index 6a0c2d89e..8df68cd74 100644 --- a/cmd/tsidp/tsidp.go +++ b/cmd/tsidp/tsidp.go @@ -270,7 +270,7 @@ func serveOnLocalTailscaled(ctx context.Context, lc *local.Client, st *ipnstate. foregroundSc.SetFunnel(serverURL, dstPort, shouldFunnel) foregroundSc.SetWebHandler(&ipn.HTTPHandler{ Proxy: fmt.Sprintf("https://%s", net.JoinHostPort(serverURL, strconv.Itoa(int(dstPort)))), - }, serverURL, uint16(*flagPort), "/", true) + }, serverURL, uint16(*flagPort), "/", true, st.CurrentTailnet.MagicDNSSuffix) err = lc.SetServeConfig(ctx, sc) if err != nil { return nil, watcherChan, fmt.Errorf("could not set serve config: %v", err) diff --git a/ipn/ipnlocal/serve.go b/ipn/ipnlocal/serve.go index 28262251c..36738b881 100644 --- a/ipn/ipnlocal/serve.go +++ b/ipn/ipnlocal/serve.go @@ -1014,7 +1014,9 @@ func (b *LocalBackend) webServerConfig(hostname string, forVIPService tailcfg.Se return c, false } if forVIPService != "" { - key := ipn.HostPort(net.JoinHostPort(forVIPService.WithoutPrefix(), fmt.Sprintf("%d", port))) + magicDNSSuffix := b.currentNode().NetMap().MagicDNSSuffix() + fqdn := strings.Join([]string{forVIPService.WithoutPrefix(), magicDNSSuffix}, ".") + key := ipn.HostPort(net.JoinHostPort(fqdn, fmt.Sprintf("%d", port))) return b.serveConfig.FindServiceWeb(forVIPService, key) } key := ipn.HostPort(net.JoinHostPort(hostname, fmt.Sprintf("%d", port))) diff --git a/ipn/serve.go b/ipn/serve.go index fae0ad5d6..a0f1334d7 100644 --- a/ipn/serve.go +++ b/ipn/serve.go @@ -343,8 +343,9 @@ func (sc *ServeConfig) FindConfig(port uint16) (*ServeConfig, bool) { // SetWebHandler sets the given HTTPHandler at the specified host, port, // and mount in the serve config. sc.TCP is also updated to reflect web // serving usage of the given port. The st argument is needed when setting -// a web handler for a service, otherwise it can be nil. -func (sc *ServeConfig) SetWebHandler(handler *HTTPHandler, host string, port uint16, mount string, useTLS bool) { +// a web handler for a service, otherwise it can be nil. mds is the Magic DNS +// suffix, which is used to recreate serve's host. +func (sc *ServeConfig) SetWebHandler(handler *HTTPHandler, host string, port uint16, mount string, useTLS bool, mds string) { if sc == nil { sc = new(ServeConfig) } @@ -353,7 +354,7 @@ func (sc *ServeConfig) SetWebHandler(handler *HTTPHandler, host string, port uin webServerMap := &sc.Web hostName := host if svcName := tailcfg.AsServiceName(host); svcName != "" { - hostName = svcName.WithoutPrefix() + hostName = strings.Join([]string{svcName.WithoutPrefix(), mds}, ".") svc, ok := sc.Services[svcName] if !ok { svc = new(ServiceConfig) @@ -464,8 +465,7 @@ func (sc *ServeConfig) RemoveWebHandler(host string, port uint16, mounts []strin // RemoveServiceWebHandler deletes the web handlers at all of the given mount points // for the provided host and port in the serve config for the given service. -func (sc *ServeConfig) RemoveServiceWebHandler(st *ipnstate.Status, svcName tailcfg.ServiceName, port uint16, mounts []string) { - hostName := svcName.WithoutPrefix() +func (sc *ServeConfig) RemoveServiceWebHandler(svcName tailcfg.ServiceName, hostName string, port uint16, mounts []string) { hp := HostPort(net.JoinHostPort(hostName, strconv.Itoa(int(port)))) svc, ok := sc.Services[svcName]