From fc9754b5f0827ca1398dbfd41b6581cf44057fd8 Mon Sep 17 00:00:00 2001 From: Harry Harpham Date: Tue, 13 Jan 2026 14:26:20 -0700 Subject: [PATCH] ipn/ipnlocal: allow retrieval of serve config ETags from local API This change adds API to ipn.LocalBackend to retrieve the ETag when querying for the current serve config. This allows consumers of ipn.LocalBackend.SetServeConfig to utilize the concurrency control offered by ETags. Previous to this change, utilizing serve config ETags required copying the local backend's internal ETag calcuation. The local API server was previously copying the local backend's ETag calculation as described above. With this change, the local API server now uses the new ETag retrieval function instead. Serve config ETags are therefore now opaque to clients, in line with best practices. Fixes tailscale/corp#35857 Signed-off-by: Harry Harpham --- ipn/ipnlocal/serve.go | 35 +++++++++++++++++++++++++--------- ipn/ipnlocal/serve_test.go | 39 +++++++++++++++++--------------------- ipn/localapi/serve.go | 10 +++++----- 3 files changed, 48 insertions(+), 36 deletions(-) diff --git a/ipn/ipnlocal/serve.go b/ipn/ipnlocal/serve.go index 4d6055bbd..3824d6f36 100644 --- a/ipn/ipnlocal/serve.go +++ b/ipn/ipnlocal/serve.go @@ -293,6 +293,15 @@ func (b *LocalBackend) updateServeTCPPortNetMapAddrListenersLocked(ports []uint1 } } +func generateServeConfigETag(sc ipn.ServeConfigView) (string, error) { + j, err := json.Marshal(sc) + if err != nil { + return "", fmt.Errorf("encoding config: %w", err) + } + sum := sha256.Sum256(j) + return hex.EncodeToString(sum[:]), nil +} + // SetServeConfig establishes or replaces the current serve config. // ETag is an optional parameter to enforce Optimistic Concurrency Control. // If it is an empty string, then the config will be overwritten. @@ -327,17 +336,11 @@ func (b *LocalBackend) setServeConfigLocked(config *ipn.ServeConfig, etag string // not changed from the last config. prevConfig := b.serveConfig if etag != "" { - // Note that we marshal b.serveConfig - // and not use b.lastServeConfJSON as that might - // be a Go nil value, which produces a different - // checksum from a JSON "null" value. - prevBytes, err := json.Marshal(prevConfig) + prevETag, err := generateServeConfigETag(prevConfig) if err != nil { - return fmt.Errorf("error encoding previous config: %w", err) + return fmt.Errorf("generating ETag for previous config: %w", err) } - sum := sha256.Sum256(prevBytes) - previousEtag := hex.EncodeToString(sum[:]) - if etag != previousEtag { + if etag != prevETag { return ErrETagMismatch } } @@ -392,6 +395,20 @@ func (b *LocalBackend) ServeConfig() ipn.ServeConfigView { return b.serveConfig } +// ServeConfigETag provides a view of the current serve mappings and an ETag, +// which can later be provided to [LocalBackend.SetServeConfig] to implement +// Optimistic Concurrency Control. +// +// If serving is not configured, the returned view is not Valid. +func (b *LocalBackend) ServeConfigETag() (scv ipn.ServeConfigView, etag string, err error) { + sc := b.ServeConfig() + etag, err = generateServeConfigETag(sc) + if err != nil { + return ipn.ServeConfigView{}, "", fmt.Errorf("generating ETag: %w", err) + } + return sc, etag, nil +} + // DeleteForegroundSession deletes a ServeConfig's foreground session // in the LocalBackend if it exists. It also ensures check, delete, and // set operations happen within the same mutex lock to avoid any races. diff --git a/ipn/ipnlocal/serve_test.go b/ipn/ipnlocal/serve_test.go index 6ee2181a0..0892545cc 100644 --- a/ipn/ipnlocal/serve_test.go +++ b/ipn/ipnlocal/serve_test.go @@ -9,9 +9,7 @@ import ( "bytes" "cmp" "context" - "crypto/sha256" "crypto/tls" - "encoding/hex" "encoding/json" "errors" "fmt" @@ -222,16 +220,6 @@ func TestGetServeHandler(t *testing.T) { } } -func getEtag(t *testing.T, b any) string { - t.Helper() - bts, err := json.Marshal(b) - if err != nil { - t.Fatal(err) - } - sum := sha256.Sum256(bts) - return hex.EncodeToString(sum[:]) -} - // TestServeConfigForeground tests the inter-dependency // between a ServeConfig and a WatchIPNBus: // 1. Creating a WatchIPNBus returns a sessionID, that @@ -544,8 +532,14 @@ func TestServeConfigServices(t *testing.T) { func TestServeConfigETag(t *testing.T) { b := newTestBackend(t) - // a nil config with initial etag should succeed - err := b.SetServeConfig(nil, getEtag(t, nil)) + // the etag should be valid even when there is no config + _, emptyStateETag, err := b.ServeConfigETag() + if err != nil { + t.Fatal(err) + } + + // a nil config with the empty-state etag should succeed + err = b.SetServeConfig(nil, emptyStateETag) if err != nil { t.Fatal(err) } @@ -556,7 +550,7 @@ func TestServeConfigETag(t *testing.T) { t.Fatal("expected an error but got nil") } - // a new config with no etag should succeed + // a new config with the empty-state etag should succeed conf := &ipn.ServeConfig{ Web: map[ipn.HostPort]*ipn.WebServerConfig{ "example.ts.net:443": {Handlers: map[string]*ipn.HTTPHandler{ @@ -564,15 +558,14 @@ func TestServeConfigETag(t *testing.T) { }}, }, } - err = b.SetServeConfig(conf, getEtag(t, nil)) + err = b.SetServeConfig(conf, emptyStateETag) if err != nil { t.Fatal(err) } - confView := b.ServeConfig() - etag := getEtag(t, confView) - if etag == "" { - t.Fatal("expected to get an etag but got an empty string") + confView, etag, err := b.ServeConfigETag() + if err != nil { + t.Fatal(err) } conf = confView.AsStruct() mak.Set(&conf.AllowFunnel, "example.ts.net:443", true) @@ -596,8 +589,10 @@ func TestServeConfigETag(t *testing.T) { } // replacing an existing config with the new etag should succeed - newCfg := b.ServeConfig() - etag = getEtag(t, newCfg) + _, etag, err = b.ServeConfigETag() + if err != nil { + t.Fatal(err) + } err = b.SetServeConfig(nil, etag) if err != nil { t.Fatal(err) diff --git a/ipn/localapi/serve.go b/ipn/localapi/serve.go index 56c8b486c..efbbde06f 100644 --- a/ipn/localapi/serve.go +++ b/ipn/localapi/serve.go @@ -6,8 +6,6 @@ package localapi import ( - "crypto/sha256" - "encoding/hex" "encoding/json" "errors" "fmt" @@ -31,14 +29,16 @@ func (h *Handler) serveServeConfig(w http.ResponseWriter, r *http.Request) { http.Error(w, "serve config denied", http.StatusForbidden) return } - config := h.b.ServeConfig() + config, etag, err := h.b.ServeConfigETag() + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } bts, err := json.Marshal(config) if err != nil { http.Error(w, "error encoding config: "+err.Error(), http.StatusInternalServerError) return } - sum := sha256.Sum256(bts) - etag := hex.EncodeToString(sum[:]) w.Header().Set("Etag", etag) w.Header().Set("Content-Type", "application/json") w.Write(bts)