From c48f953840373a51215329bc9f8bcb64255aaf4f Mon Sep 17 00:00:00 2001 From: Brendan Creane Date: Fri, 12 Jun 2026 18:52:17 -0700 Subject: [PATCH] cmd/tailscale/cli, ipn/conffile: accept legacy serve config in set-config (#20056) tailscale serve set-config now also accepts the legacy raw ipn.ServeConfig format (as emitted by `tailscale serve status --json` and consumed via TS_SERVE_CONFIG, which has no "version" field), so the common serve-status-edit-set workflow stops failing. Only the services-oriented content is applied; any node-level fields are skipped with a warning to stderr pointing users at get-config to migrate. Fixes tailscale/corp#39793 Signed-off-by: Brendan Creane --- cmd/tailscale/cli/serve_v2.go | 72 ++++++++-- cmd/tailscale/cli/serve_v2_test.go | 213 +++++++++++++++++++++++++++++ ipn/conffile/serveconf.go | 65 ++++++++- 3 files changed, 335 insertions(+), 15 deletions(-) diff --git a/cmd/tailscale/cli/serve_v2.go b/cmd/tailscale/cli/serve_v2.go index 13f5c09b8..5b1676ed2 100644 --- a/cmd/tailscale/cli/serve_v2.go +++ b/cmd/tailscale/cli/serve_v2.go @@ -800,24 +800,57 @@ func (e *serveEnv) runServeGetConfig(ctx context.Context, args []string) (err er return err } +// serveConfigDocsURL documents the Services configuration file format that set-config prefers +const serveConfigDocsURL = "https://tailscale.com/kb/1589/tailscale-services-configuration-file" + +const serveLegacyFormatWarning = "Warning: %q is in the legacy raw serve config format " + + "(as emitted by `tailscale serve status --json`), which is deprecated for set-config. " + + "Applying its services only. To migrate, run `tailscale serve get-config` to save your " + + "configuration in the supported format; see %s\n" + +const serveLegacyDroppedWarning = "Warning: ignoring node-level fields not supported by set-config: %s\n" + +// legacyNodeLevelFields returns the names of the populated top-level fields in +// sc, other than Services, that set-config does not apply (it is services-only). +func legacyNodeLevelFields(sc *ipn.ServeConfig) []string { + rest := sc.Clone() + rest.Services = nil + b, err := json.Marshal(rest) + if err != nil { + return nil + } + var m map[string]json.RawMessage + if err := json.Unmarshal(b, &m); err != nil { + return nil + } + fields := make([]string, 0, len(m)) + for k := range m { + fields = append(fields, k) + } + sort.Strings(fields) + return fields +} + func (e *serveEnv) runServeSetConfig(ctx context.Context, args []string) (err error) { if len(args) != 1 { return errors.New("must specify filename") } + filename := args[0] forSingleService := e.service.Validate() == nil - - var scf *conffile.ServicesConfigFile if e.allServices && forSingleService { return errors.New("cannot specify both --all and --service") - } else if e.allServices { - scf, err = conffile.LoadServicesConfig(args[0], "") - } else if forSingleService { - scf, err = conffile.LoadServicesConfig(args[0], e.service.String()) - } else { + } + if !e.allServices && !forSingleService { return errors.New("must specify either --service=svc: or --all") } + + forService := "" + if forSingleService { + forService = e.service.String() + } + scf, err := conffile.LoadServicesConfig(filename, forService) if err != nil { - return fmt.Errorf("could not read config from file %q: %w", args[0], err) + return fmt.Errorf("could not read config from file %q: %w", filename, err) } st, err := e.getLocalClientStatusWithoutPeers(ctx) @@ -842,6 +875,29 @@ func (e *serveEnv) runServeSetConfig(ctx context.Context, args []string) (err er } advertisedServices := set.Set[string]{} + if scf.Version == conffile.LegacyVersion { + // Legacy raw ipn.ServeConfig (e.g. "tailscale serve status --json" + // output). Deprecated for set-config; apply only its services-oriented + // content, with a migration warning to stderr (never stdout, which + // callers may pipe). + legacy := scf.Legacy + fmt.Fprintf(e.stderr(), serveLegacyFormatWarning, filename, serveConfigDocsURL) + if dropped := legacyNodeLevelFields(legacy); len(dropped) > 0 { + fmt.Fprintf(e.stderr(), serveLegacyDroppedWarning, strings.Join(dropped, ", ")) + } + for name, svcCfg := range legacy.Services { + if forSingleService && name != e.service { + continue + } + mak.Set(&sc.Services, name, svcCfg.Clone()) + advertisedServices.Add(name.String()) + } + if forSingleService && sc.Services[e.service] == nil { + return fmt.Errorf("service %q not found in %q", e.service, filename) + } + } + + // scf.Services is nil for the legacy format, making this loop a no-op then. for name, details := range scf.Services { for ppr, ep := range details.Endpoints { if ep.Protocol == conffile.ProtoTUN { diff --git a/cmd/tailscale/cli/serve_v2_test.go b/cmd/tailscale/cli/serve_v2_test.go index 1d2a8ef86..994f443b6 100644 --- a/cmd/tailscale/cli/serve_v2_test.go +++ b/cmd/tailscale/cli/serve_v2_test.go @@ -2361,3 +2361,216 @@ func ptrToReadOnlySlice[T any](s []T) *views.Slice[T] { vs := views.SliceOf(s) return &vs } + +func writeTmpServeConfig(t *testing.T, body string) string { + t.Helper() + p := filepath.Join(t.TempDir(), "serve.json") + if err := os.WriteFile(p, []byte(body), 0600); err != nil { + t.Fatal(err) + } + return p +} + +// TestRunServeSetConfig covers set-config accepting both the declarative +// Services configuration file (with a "version" field) and the legacy raw +// ipn.ServeConfig format (no "version"), the latter applying services only and +// warning on stderr. +func TestRunServeSetConfig(t *testing.T) { + const fooSvc = tailcfg.ServiceName("svc:foo") + + t.Run("legacy_all_services_only", func(t *testing.T) { + lc := &fakeLocalServeClient{config: &ipn.ServeConfig{}} + var stdout, stderr bytes.Buffer + e := &serveEnv{lc: lc, allServices: true, testStdout: &stdout, testStderr: &stderr} + path := writeTmpServeConfig(t, `{"Services":{"svc:foo":{"TCP":{"443":{"HTTPS":true}}}}}`) + + if err := e.runServeSetConfig(context.Background(), []string{path}); err != nil { + t.Fatal(err) + } + if lc.setCount != 1 { + t.Fatalf("setCount = %d, want 1", lc.setCount) + } + svc := lc.config.Services[fooSvc] + if svc == nil || svc.TCP[443] == nil || !svc.TCP[443].HTTPS { + t.Errorf("svc:foo TCP/443 HTTPS not applied; got %+v", lc.config.Services) + } + if !slices.Contains(lc.prefs.AdvertiseServices, fooSvc.String()) { + t.Errorf("svc:foo not advertised; AdvertiseServices=%v", lc.prefs.AdvertiseServices) + } + if !strings.Contains(stderr.String(), "legacy raw serve config format") || + !strings.Contains(stderr.String(), serveConfigDocsURL) { + t.Errorf("missing legacy migration warning; stderr:\n%s", stderr.String()) + } + if strings.Contains(stderr.String(), "ignoring node-level fields") { + t.Errorf("unexpected dropped-fields warning; stderr:\n%s", stderr.String()) + } + if stdout.Len() != 0 { + t.Errorf("stdout must stay clean, got:\n%s", stdout.String()) + } + }) + + t.Run("legacy_drops_node_level_fields", func(t *testing.T) { + lc := &fakeLocalServeClient{config: &ipn.ServeConfig{}} + var stdout, stderr bytes.Buffer + e := &serveEnv{lc: lc, allServices: true, testStdout: &stdout, testStderr: &stderr} + path := writeTmpServeConfig(t, `{ + "TCP":{"443":{"HTTPS":true}}, + "Web":{"foo.test.ts.net:443":{"Handlers":{"/":{"Proxy":"http://127.0.0.1:3000"}}}}, + "AllowFunnel":{"foo.test.ts.net:443":true}, + "Services":{"svc:foo":{"Tun":true}} + }`) + + if err := e.runServeSetConfig(context.Background(), []string{path}); err != nil { + t.Fatal(err) + } + if svc := lc.config.Services[fooSvc]; svc == nil || !svc.Tun { + t.Errorf("svc:foo Tun not applied; got %+v", lc.config.Services) + } + // Fields are derived from the JSON dynamically and sorted. + if !strings.Contains(stderr.String(), "ignoring node-level fields not supported by set-config: AllowFunnel, TCP, Web") { + t.Errorf("missing/incorrect dropped-fields warning; stderr:\n%s", stderr.String()) + } + // Node-level content must not have leaked into the applied config. + if len(lc.config.TCP) != 0 || len(lc.config.Web) != 0 { + t.Errorf("node-level TCP/Web should not be applied; got TCP=%v Web=%v", lc.config.TCP, lc.config.Web) + } + if stdout.Len() != 0 { + t.Errorf("stdout must stay clean, got:\n%s", stdout.String()) + } + }) + + t.Run("versionless_new_format_errors", func(t *testing.T) { + // A Services config file (lowercase "services"/"endpoints") that omits + // the required "version" field must error, not be misread as a legacy + // raw ServeConfig and silently wipe the existing config. + existing := &ipn.ServeConfig{Services: map[tailcfg.ServiceName]*ipn.ServiceConfig{ + fooSvc: {Tun: true}, + }} + lc := &fakeLocalServeClient{config: existing} + e := &serveEnv{lc: lc, allServices: true, testStdout: &bytes.Buffer{}, testStderr: &bytes.Buffer{}} + path := writeTmpServeConfig(t, `{"services":{"svc:foo":{"endpoints":{"tcp:443":"https://localhost:8000"}}}}`) + + err := e.runServeSetConfig(context.Background(), []string{path}) + if err == nil || !strings.Contains(err.Error(), "version") { + t.Fatalf("err = %v, want an error mentioning the missing version field", err) + } + if lc.setCount != 0 { + t.Errorf("setCount = %d, want 0 (existing config must not be wiped)", lc.setCount) + } + if lc.config.Services[fooSvc] == nil { + t.Errorf("existing svc:foo was wiped; got %+v", lc.config.Services) + } + }) + + t.Run("version_0_0_0_rejected", func(t *testing.T) { + // A file can never forge the internal LegacyVersion ("0.0.0") sentinel + // that LoadServicesConfig uses to wrap a legacy raw config: it is + // rejected as an unsupported version, and must not wipe existing config. + existing := &ipn.ServeConfig{Services: map[tailcfg.ServiceName]*ipn.ServiceConfig{ + fooSvc: {Tun: true}, + }} + lc := &fakeLocalServeClient{config: existing} + e := &serveEnv{lc: lc, allServices: true, testStdout: &bytes.Buffer{}, testStderr: &bytes.Buffer{}} + path := writeTmpServeConfig(t, `{"version":"0.0.0","services":{}}`) + + err := e.runServeSetConfig(context.Background(), []string{path}) + if err == nil || !strings.Contains(err.Error(), `unsupported config file version "0.0.0"`) { + t.Fatalf("err = %v, want an 'unsupported config file version \"0.0.0\"' error", err) + } + if lc.setCount != 0 { + t.Errorf("setCount = %d, want 0 (existing config must not be wiped)", lc.setCount) + } + if lc.config.Services[fooSvc] == nil { + t.Errorf("existing svc:foo was wiped; got %+v", lc.config.Services) + } + }) + + t.Run("legacy_service_selects_one", func(t *testing.T) { + lc := &fakeLocalServeClient{config: &ipn.ServeConfig{}} + var stdout, stderr bytes.Buffer + e := &serveEnv{lc: lc, service: fooSvc, testStdout: &stdout, testStderr: &stderr} + path := writeTmpServeConfig(t, `{"Services":{"svc:foo":{"Tun":true},"svc:bar":{"Tun":true}}}`) + + if err := e.runServeSetConfig(context.Background(), []string{path}); err != nil { + t.Fatal(err) + } + if lc.config.Services[fooSvc] == nil { + t.Errorf("svc:foo not applied; got %+v", lc.config.Services) + } + if lc.config.Services[tailcfg.ServiceName("svc:bar")] != nil { + t.Errorf("svc:bar should not be applied with --service=svc:foo; got %+v", lc.config.Services) + } + }) + + t.Run("legacy_service_missing_errors", func(t *testing.T) { + lc := &fakeLocalServeClient{config: &ipn.ServeConfig{}} + e := &serveEnv{lc: lc, service: tailcfg.ServiceName("svc:missing"), testStdout: &bytes.Buffer{}, testStderr: &bytes.Buffer{}} + path := writeTmpServeConfig(t, `{"Services":{"svc:foo":{"Tun":true}}}`) + + err := e.runServeSetConfig(context.Background(), []string{path}) + if err == nil || !strings.Contains(err.Error(), "not found") { + t.Fatalf("err = %v, want a 'not found' error", err) + } + if lc.setCount != 0 { + t.Errorf("setCount = %d, want 0", lc.setCount) + } + }) + + t.Run("both_flags_error", func(t *testing.T) { + lc := &fakeLocalServeClient{config: &ipn.ServeConfig{}} + e := &serveEnv{lc: lc, allServices: true, service: fooSvc, testStdout: &bytes.Buffer{}, testStderr: &bytes.Buffer{}} + err := e.runServeSetConfig(context.Background(), []string{"unused.json"}) + if err == nil || !strings.Contains(err.Error(), "cannot specify both") { + t.Fatalf("err = %v, want 'cannot specify both'", err) + } + }) + + t.Run("neither_flag_error", func(t *testing.T) { + lc := &fakeLocalServeClient{config: &ipn.ServeConfig{}} + e := &serveEnv{lc: lc, testStdout: &bytes.Buffer{}, testStderr: &bytes.Buffer{}} + err := e.runServeSetConfig(context.Background(), []string{"unused.json"}) + if err == nil || !strings.Contains(err.Error(), "must specify either") { + t.Fatalf("err = %v, want 'must specify either'", err) + } + }) + + t.Run("new_format_all_no_warning", func(t *testing.T) { + lc := &fakeLocalServeClient{config: &ipn.ServeConfig{}} + var stdout, stderr bytes.Buffer + e := &serveEnv{lc: lc, allServices: true, testStdout: &stdout, testStderr: &stderr} + path := writeTmpServeConfig(t, `{"version":"0.0.1","services":{"svc:foo":{"endpoints":{"tcp:443":"https://localhost:8000"}}}}`) + + if err := e.runServeSetConfig(context.Background(), []string{path}); err != nil { + t.Fatal(err) + } + if lc.setCount != 1 { + t.Fatalf("setCount = %d, want 1", lc.setCount) + } + if lc.config.Services[fooSvc] == nil { + t.Errorf("svc:foo not applied; got %+v", lc.config.Services) + } + if stderr.Len() != 0 { + t.Errorf("new format must not warn; stderr:\n%s", stderr.String()) + } + if stdout.Len() != 0 { + t.Errorf("stdout must stay clean, got:\n%s", stdout.String()) + } + }) + + t.Run("new_format_single_service_no_warning", func(t *testing.T) { + lc := &fakeLocalServeClient{config: &ipn.ServeConfig{}} + var stdout, stderr bytes.Buffer + e := &serveEnv{lc: lc, service: fooSvc, testStdout: &stdout, testStderr: &stderr} + path := writeTmpServeConfig(t, `{"version":"0.0.1","endpoints":{"tcp:443":"https://localhost:8000"}}`) + + if err := e.runServeSetConfig(context.Background(), []string{path}); err != nil { + t.Fatal(err) + } + if lc.config.Services[fooSvc] == nil { + t.Errorf("svc:foo not applied; got %+v", lc.config.Services) + } + if stderr.Len() != 0 { + t.Errorf("new format must not warn; stderr:\n%s", stderr.String()) + } + }) +} diff --git a/ipn/conffile/serveconf.go b/ipn/conffile/serveconf.go index 0d3360292..b58badc53 100644 --- a/ipn/conffile/serveconf.go +++ b/ipn/conffile/serveconf.go @@ -15,17 +15,34 @@ import ( jsonv2 "github.com/go-json-experiment/json" "github.com/go-json-experiment/json/jsontext" + "tailscale.com/ipn" "tailscale.com/tailcfg" "tailscale.com/types/opt" "tailscale.com/util/mak" ) +// LegacyVersion is the sentinel [ServicesConfigFile.Version] used to mark a +// config that was loaded from the legacy raw [ipn.ServeConfig] format (a +// version-less file, such as "tailscale serve status --json" output). When +// Version is LegacyVersion, [ServicesConfigFile.Legacy] is set and Services is +// nil. It is never written to disk; the on-disk format always uses "0.0.1". +const LegacyVersion = "0.0.0" + // ServicesConfigFile is the config file format for services configuration. type ServicesConfigFile struct { - // Version is always "0.0.1" and always present. + // Version is "0.0.1" for the declarative services configuration file + // format, or [LegacyVersion] ("0.0.0") when this value was produced by + // [LoadServicesConfig] from a legacy raw ipn.ServeConfig file (in which + // case Legacy is set instead of Services). Version string `json:"version"` Services map[tailcfg.ServiceName]*ServiceDetailsFile `json:"services,omitzero"` + + // Legacy holds a raw ipn.ServeConfig parsed from a version-less file (e.g. + // "tailscale serve status --json" output). It is non-nil only when Version + // is [LegacyVersion]. It is an in-memory loading artifact and is never + // serialized. + Legacy *ipn.ServeConfig `json:"-"` } // ServiceDetailsFile is the config syntax for an individual Tailscale Service. @@ -145,6 +162,15 @@ func (t *Target) MarshalText() ([]byte, error) { return []byte(out), nil } +// LoadServicesConfig loads a serve config file as a [ServicesConfigFile]. +// +// If the file has a top-level "version" field it is parsed as that versioned +// declarative format. Otherwise it is treated as a legacy raw [ipn.ServeConfig] +// (such as "tailscale serve status --json" emits): the returned +// ServicesConfigFile has Version [LegacyVersion] and its Legacy field set to the +// parsed raw config, with Services left nil. +// +// forService is used only for the versioned Services configuration file format. func LoadServicesConfig(filename string, forService string) (*ServicesConfigFile, error) { data, err := os.ReadFile(filename) if err != nil { @@ -165,13 +191,38 @@ func LoadServicesConfig(filename string, forService string) (*ServicesConfigFile if err = jsonv2.Unmarshal(json, &ver); err != nil { return nil, fmt.Errorf("could not parse config file version: %w", err) } - switch ver.Version { - case "": - return nil, errors.New("config file must have \"version\" field") - case "0.0.1": - return loadConfigV0(json, forService) + if ver.Version == "" { + // No "version" field. This is either the legacy raw ipn.ServeConfig + // (e.g. "tailscale serve status --json" output, which set-config still + // accepts) or a Services configuration file whose required "version" + // field was omitted. Distinguish them by the Services config format's + // lowercase "services"/"endpoints" keys, which never appear in a raw + // ServeConfig: it uses capitalized "Services" and has no "endpoints" + // key, and jsonv2 matches case-sensitively. Without this check a + // version-less Services config file would parse as an empty + // ServeConfig and silently wipe the existing config. + var probe struct { + Services jsontext.Value `json:"services"` + Endpoints jsontext.Value `json:"endpoints"` + } + if err := jsonv2.Unmarshal(json, &probe); err == nil && + (len(probe.Services) > 0 || len(probe.Endpoints) > 0) { + return nil, errors.New(`config file looks like a Services configuration file but is missing the required "version" field`) + } + // Legacy raw ipn.ServeConfig: parse leniently (like set-raw and + // TS_SERVE_CONFIG) so "serve status --json" round-trips keep working. + // It is returned wrapped in a ServicesConfigFile with the LegacyVersion + // sentinel so the public function signature stays stable. + legacy := new(ipn.ServeConfig) + if err := jsonv2.Unmarshal(json, legacy); err != nil { + return nil, fmt.Errorf("could not parse serve config: %w", err) + } + return &ServicesConfigFile{Version: LegacyVersion, Legacy: legacy}, nil } - return nil, fmt.Errorf("unsupported config file version %q", ver.Version) + if ver.Version != "0.0.1" { + return nil, fmt.Errorf("unsupported config file version %q", ver.Version) + } + return loadConfigV0(json, forService) } func loadConfigV0(json []byte, forService string) (*ServicesConfigFile, error) {