Remove database from Mapper

This commit changes the internals of the mapper to
track all the changes to peers over its lifetime.

This means that it no longer depends on the database
and this should hopefully help with locks and timing issues.
When the mapper is created, it needs the current list of peers,
the world view, when the polling session was started. Then as
update changes are called, it tracks the changes and generates
responses based on its internal list.

As a side, the types.Machines and types.MachinesP, as well as
types.Machine being passed as a full struct and pointer has been
changed to always be pointers, everywhere.

Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com>
This commit is contained in:
Kristoffer Dalby 2023-08-09 22:20:05 +02:00 committed by Kristoffer Dalby
parent 3b0749a320
commit 387aa03adb
15 changed files with 251 additions and 236 deletions

View file

@ -39,8 +39,6 @@ const (
var debugDumpMapResponsePath = envknob.String("HEADSCALE_DEBUG_DUMP_MAPRESPONSE_PATH")
type Mapper struct {
db *db.HSDatabase
privateKey2019 *key.MachinePrivate
isNoise bool
@ -55,11 +53,16 @@ type Mapper struct {
uid string
created time.Time
seq uint64
// Map isnt concurrency safe, so we need to ensure
// only one func is accessing it over time.
mu sync.Mutex
peers map[uint64]*types.Machine
}
func NewMapper(
machine *types.Machine,
db *db.HSDatabase,
peers types.Machines,
privateKey *key.MachinePrivate,
isNoise bool,
derpMap *tailcfg.DERPMap,
@ -77,8 +80,6 @@ func NewMapper(
uid, _ := util.GenerateRandomStringDNSSafe(mapperIDLength)
return &Mapper{
db: db,
privateKey2019: privateKey,
isNoise: isNoise,
@ -91,6 +92,9 @@ func NewMapper(
uid: uid,
created: time.Now(),
seq: 0,
// TODO: populate
peers: peers.IDMap(),
}
}
@ -121,7 +125,7 @@ func fullMapResponse(
logtail bool,
randomClientPort bool,
) (*tailcfg.MapResponse, error) {
tailnode, err := tailNode(*machine, pol, dnsCfg, baseDomain)
tailnode, err := tailNode(machine, pol, dnsCfg, baseDomain)
if err != nil {
return nil, err
}
@ -160,9 +164,7 @@ func fullMapResponse(
}
// Filter out peers that have expired.
peers = lo.Filter(peers, func(item types.Machine, index int) bool {
return !item.IsExpired()
})
peers = filterExpiredAndNotReady(peers)
// If there are filter rules present, see if there are any machines that cannot
// access eachother at all and remove them from the peers.
@ -175,7 +177,7 @@ func fullMapResponse(
dnsConfig := generateDNSConfig(
dnsCfg,
baseDomain,
*machine,
machine,
peers,
)
@ -232,7 +234,7 @@ func generateUserProfiles(
func generateDNSConfig(
base *tailcfg.DNSConfig,
baseDomain string,
machine types.Machine,
machine *types.Machine,
peers types.Machines,
) *tailcfg.DNSConfig {
dnsConfig := base.Clone()
@ -275,7 +277,7 @@ func generateDNSConfig(
//
// This will produce a resolver like:
// `https://dns.nextdns.io/<nextdns-id>?device_name=node-name&device_model=linux&device_ip=100.64.0.1`
func addNextDNSMetadata(resolvers []*dnstype.Resolver, machine types.Machine) {
func addNextDNSMetadata(resolvers []*dnstype.Resolver, machine *types.Machine) {
for _, resolver := range resolvers {
if strings.HasPrefix(resolver.Addr, nextDNSDoHPrefix) {
attrs := url.Values{
@ -298,20 +300,13 @@ func (m *Mapper) FullMapResponse(
machine *types.Machine,
pol *policy.ACLPolicy,
) ([]byte, error) {
peers, err := m.db.ListPeers(machine)
if err != nil {
log.Error().
Caller().
Err(err).
Msg("Cannot fetch peers")
return nil, err
}
m.mu.Lock()
defer m.mu.Unlock()
mapResponse, err := fullMapResponse(
pol,
machine,
peers,
machineMapToList(m.peers),
m.baseDomain,
m.dnsCfg,
m.derpMap,
@ -382,42 +377,33 @@ func (m *Mapper) DERPMapResponse(
func (m *Mapper) PeerChangedResponse(
mapRequest tailcfg.MapRequest,
machine *types.Machine,
machineIDs []uint64,
changed types.Machines,
pol *policy.ACLPolicy,
) ([]byte, error) {
m.mu.Lock()
defer m.mu.Unlock()
var err error
changed := make(types.Machines, len(machineIDs))
lastSeen := make(map[tailcfg.NodeID]bool)
peersList, err := m.db.ListPeers(machine)
if err != nil {
return nil, err
}
peers := peersList.IDMap()
for idx, machineID := range machineIDs {
changed[idx] = peers[machineID]
// Update our internal map.
for _, machine := range changed {
m.peers[machine.ID] = machine
// We have just seen the node, let the peers update their list.
lastSeen[tailcfg.NodeID(machineID)] = true
lastSeen[tailcfg.NodeID(machine.ID)] = true
}
rules, sshPolicy, err := policy.GenerateFilterAndSSHRules(
pol,
machine,
peersList,
machineMapToList(m.peers),
)
if err != nil {
return nil, err
}
changed = lo.Filter(changed, func(item types.Machine, index int) bool {
// Filter out nodes that are expired OR
// nodes that has no endpoints, this typically means they have
// registered, but are not configured.
return !item.IsExpired() || len(item.Endpoints) > 0
})
changed = filterExpiredAndNotReady(changed)
// If there are filter rules present, see if there are any machines that cannot
// access eachother at all and remove them from the changed.
@ -449,6 +435,14 @@ func (m *Mapper) PeerRemovedResponse(
machine *types.Machine,
removed []tailcfg.NodeID,
) ([]byte, error) {
m.mu.Lock()
defer m.mu.Unlock()
// remove from our internal map
for _, id := range removed {
delete(m.peers, uint64(id))
}
resp := m.baseMapResponse(machine)
resp.PeersRemoved = removed
@ -604,3 +598,22 @@ func (m *Mapper) baseMapResponse(_ *types.Machine) tailcfg.MapResponse {
return resp
}
func machineMapToList(machines map[uint64]*types.Machine) types.Machines {
ret := make(types.Machines, 0)
for _, machine := range machines {
ret = append(ret, machine)
}
return ret
}
func filterExpiredAndNotReady(peers types.Machines) types.Machines {
return lo.Filter(peers, func(item *types.Machine, index int) bool {
// Filter out nodes that are expired OR
// nodes that has no endpoints, this typically means they have
// registered, but are not configured.
return !item.IsExpired() || len(item.Endpoints) > 0
})
}

View file

@ -18,8 +18,8 @@ import (
)
func (s *Suite) TestGetMapResponseUserProfiles(c *check.C) {
mach := func(hostname, username string, userid uint) types.Machine {
return types.Machine{
mach := func(hostname, username string, userid uint) *types.Machine {
return &types.Machine{
Hostname: hostname,
UserID: userid,
User: types.User{
@ -34,7 +34,7 @@ func (s *Suite) TestGetMapResponseUserProfiles(c *check.C) {
machine2InShared1 := mach("test_get_shared_nodes_4", "user1", 1)
userProfiles := generateUserProfiles(
&machineInShared1,
machineInShared1,
types.Machines{
machineInShared2, machineInShared3, machine2InShared1,
},
@ -91,8 +91,8 @@ func TestDNSConfigMapResponse(t *testing.T) {
for _, tt := range tests {
t.Run(fmt.Sprintf("with-magicdns-%v", tt.magicDNS), func(t *testing.T) {
mach := func(hostname, username string, userid uint) types.Machine {
return types.Machine{
mach := func(hostname, username string, userid uint) *types.Machine {
return &types.Machine{
Hostname: hostname,
UserID: userid,
User: types.User{
@ -243,7 +243,7 @@ func Test_fullMapResponse(t *testing.T) {
},
}
peer1 := types.Machine{
peer1 := &types.Machine{
ID: 1,
MachineKey: "mkey:f08305b4ee4250b95a70f3b7504d048d75d899993c624a26d422c67af0422507",
NodeKey: "nodekey:9b2ffa7e08cc421a3d2cca9012280f6a236fd0de0b4ce005b30a98ad930306fe",
@ -295,7 +295,7 @@ func Test_fullMapResponse(t *testing.T) {
},
}
peer2 := types.Machine{
peer2 := &types.Machine{
ID: 2,
MachineKey: "mkey:f08305b4ee4250b95a70f3b7504d048d75d899993c624a26d422c67af0422507",
NodeKey: "nodekey:9b2ffa7e08cc421a3d2cca9012280f6a236fd0de0b4ce005b30a98ad930306fe",
@ -341,7 +341,7 @@ func Test_fullMapResponse(t *testing.T) {
name: "no-pol-no-peers-map-response",
pol: &policy.ACLPolicy{},
machine: mini,
peers: []types.Machine{},
peers: types.Machines{},
baseDomain: "",
dnsConfig: &tailcfg.DNSConfig{},
derpMap: &tailcfg.DERPMap{},
@ -369,7 +369,7 @@ func Test_fullMapResponse(t *testing.T) {
name: "no-pol-with-peer-map-response",
pol: &policy.ACLPolicy{},
machine: mini,
peers: []types.Machine{
peers: types.Machines{
peer1,
},
baseDomain: "",
@ -410,7 +410,7 @@ func Test_fullMapResponse(t *testing.T) {
},
},
machine: mini,
peers: []types.Machine{
peers: types.Machines{
peer1,
peer2,
},

View file

@ -41,7 +41,7 @@ func tailNodes(
// tailNode converts a Machine into a Tailscale Node. includeRoutes is false for shared nodes
// as per the expected behaviour in the official SaaS.
func tailNode(
machine types.Machine,
machine *types.Machine,
pol *policy.ACLPolicy,
dnsConfig *tailcfg.DNSConfig,
baseDomain string,

View file

@ -45,7 +45,7 @@ func TestTailNode(t *testing.T) {
tests := []struct {
name string
machine types.Machine
machine *types.Machine
pol *policy.ACLPolicy
dnsConfig *tailcfg.DNSConfig
baseDomain string
@ -54,7 +54,7 @@ func TestTailNode(t *testing.T) {
}{
{
name: "empty-machine",
machine: types.Machine{},
machine: &types.Machine{},
pol: &policy.ACLPolicy{},
dnsConfig: &tailcfg.DNSConfig{},
baseDomain: "",
@ -63,7 +63,7 @@ func TestTailNode(t *testing.T) {
},
{
name: "minimal-machine",
machine: types.Machine{
machine: &types.Machine{
ID: 0,
MachineKey: "mkey:f08305b4ee4250b95a70f3b7504d048d75d899993c624a26d422c67af0422507",
NodeKey: "nodekey:9b2ffa7e08cc421a3d2cca9012280f6a236fd0de0b4ce005b30a98ad930306fe",