types: lock tailcfg DNS config access for extra-records updates

The watcher write raced per-client map builds cloning ExtraRecords.
This commit is contained in:
Kristoffer Dalby
2026-06-05 14:23:23 +00:00
committed by Kristoffer Dalby
parent ec94573258
commit 5a70a72988
4 changed files with 95 additions and 5 deletions
+2 -2
View File
@@ -377,7 +377,7 @@ func (h *Headscale) scheduledTasks(ctx context.Context) {
continue
}
h.cfg.TailcfgDNSConfig.ExtraRecords = records
h.cfg.SetExtraRecords(records)
h.Change(change.ExtraRecords())
@@ -667,7 +667,7 @@ func (h *Headscale) Serve() error {
return fmt.Errorf("setting up extrarecord manager: %w", err)
}
h.cfg.TailcfgDNSConfig.ExtraRecords = h.extraRecordMan.Records()
h.cfg.SetExtraRecords(h.extraRecordMan.Records())
go h.extraRecordMan.Run()
defer h.extraRecordMan.Close()
@@ -0,0 +1,60 @@
package mapper
import (
"sync"
"testing"
"github.com/juanfont/headscale/hscontrol/types"
"tailscale.com/tailcfg"
)
// TestExtraRecordsConcurrentUpdateNoRace exercises the extra-records watcher
// write path (Config.SetExtraRecords) concurrently with per-client DNS config
// builds (generateDNSConfig -> Config.CloneTailcfgDNSConfig). Both must go
// through the shared lock so the run is race-free under -race.
func TestExtraRecordsConcurrentUpdateNoRace(t *testing.T) {
uid := uint(1)
cfg := &types.Config{
TailcfgDNSConfig: &tailcfg.DNSConfig{
ExtraRecords: []tailcfg.DNSRecord{
{Name: "initial.example.com", Type: "A", Value: "100.64.0.1"},
},
},
}
node := (&types.Node{
Hostname: "race-node",
UserID: &uid,
User: &types.User{Name: "racer"},
}).View()
const iterations = 2000
var wg sync.WaitGroup
// Writer: the extra-records update path.
wg.Go(func() {
for i := range iterations {
recs := []tailcfg.DNSRecord{{Name: "a.example.com", Type: "A", Value: "100.64.0.2"}}
if i%2 == 0 {
recs = append(recs, tailcfg.DNSRecord{Name: "b.example.com", Type: "A", Value: "100.64.0.3"})
}
cfg.SetExtraRecords(recs)
}
})
// Readers: the per-client map build path.
const readers = 8
for range readers {
wg.Go(func() {
for range iterations {
if d := generateDNSConfig(cfg, node, nil); d != nil {
_ = len(d.ExtraRecords)
}
}
})
}
wg.Wait()
}
+2 -3
View File
@@ -135,12 +135,11 @@ func generateDNSConfig(
node types.NodeView,
capMap tailcfg.NodeCapMap,
) *tailcfg.DNSConfig {
if cfg.TailcfgDNSConfig == nil {
dnsConfig := cfg.CloneTailcfgDNSConfig()
if dnsConfig == nil {
return nil
}
dnsConfig := cfg.TailcfgDNSConfig.Clone()
profile := nextDNSProfileFromCapMap(capMap)
if profile != "" {
applyNextDNSProfile(dnsConfig.Resolvers, profile)
+31
View File
@@ -8,6 +8,7 @@ import (
"net/url"
"os"
"strings"
"sync"
"time"
"github.com/coreos/go-oidc/v3/oidc"
@@ -1482,3 +1483,33 @@ func (d *deprecator) Log() {
log.Warn().Msg("\n" + d.String())
}
}
// tailcfgDNSMu guards concurrent access to the mutable ExtraRecords of
// [Config.TailcfgDNSConfig] between the extra-records file watcher (writer)
// and the per-client map builds that clone it (readers). It is a package-level
// lock so [Config] stays freely copyable during construction.
var tailcfgDNSMu sync.RWMutex
// CloneTailcfgDNSConfig returns a deep copy of [Config.TailcfgDNSConfig], or
// nil if none is set. Safe for concurrent use with [Config.SetExtraRecords].
func (c *Config) CloneTailcfgDNSConfig() *tailcfg.DNSConfig {
tailcfgDNSMu.RLock()
defer tailcfgDNSMu.RUnlock()
if c.TailcfgDNSConfig == nil {
return nil
}
return c.TailcfgDNSConfig.Clone()
}
// SetExtraRecords replaces the ExtraRecords of [Config.TailcfgDNSConfig]. Safe
// for concurrent use with [Config.CloneTailcfgDNSConfig].
func (c *Config) SetExtraRecords(records []tailcfg.DNSRecord) {
tailcfgDNSMu.Lock()
defer tailcfgDNSMu.Unlock()
if c.TailcfgDNSConfig != nil {
c.TailcfgDNSConfig.ExtraRecords = records
}
}