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 <bcreane@gmail.com>
2026-06-12 18:52:17 -07:00
parent 7d18a06292
commit c48f953840
3 changed files with 335 additions and 15 deletions
@@ -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:<service-name> 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 {
@@ -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())
+		}
+	})
+}
@@ -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) {