Do not access node ID when node is not found

.github/pull_request_template.md

@@ -12,7 +12,7 @@ If you find mistakes in the documentation, please submit a fix to the documentat
 
 <!-- Please tick if the following things apply. You… -->
 
-- [ ] have read the [CONTRIBUTING.md](./CONTRIBUTING.md) file
+- [ ] read the [CONTRIBUTING guidelines](README.md#contributing)
 - [ ] raised a GitHub issue or discussed it on the projects chat beforehand
 - [ ] added unit tests
 - [ ] added integration tests

.github/workflows/test-integration.yaml

@@ -26,7 +26,6 @@ jobs:
           - TestPreAuthKeyCommand
           - TestPreAuthKeyCommandWithoutExpiry
           - TestPreAuthKeyCommandReusableEphemeral
-          - TestPreAuthKeyCorrectUserLoggedInCommand
           - TestApiKeyCommand
           - TestNodeTagCommand
           - TestNodeAdvertiseTagNoACLCommand

CHANGELOG.md

@@ -56,7 +56,6 @@ after improving the test harness as part of adopting [#1460](https://github.com/
 - Add support for deleting api keys [#1702](https://github.com/juanfont/headscale/pull/1702)
 - Add command to backfill IP addresses for nodes missing IPs from configured prefixes. [#1869](https://github.com/juanfont/headscale/pull/1869)
 - Log available update as warning [#1877](https://github.com/juanfont/headscale/pull/1877)
-- Add `autogroup:internet` to Policy [#1917](https://github.com/juanfont/headscale/pull/1917)
 
 ## 0.22.3 (2023-05-12)
 

CONTRIBUTING.md

@@ -1,34 +0,0 @@
-# Contributing
-
-Headscale is "Open Source, acknowledged contribution", this means that any contribution will have to be discussed with the maintainers before being added to the project.
-This model has been chosen to reduce the risk of burnout by limiting the maintenance overhead of reviewing and validating third-party code.
-
-## Why do we have this model?
-
-Headscale has a small maintainer team that tries to balance working on the project, fixing bugs and reviewing contributions.
-
-When we work on issues ourselves, we develop first hand knowledge of the code and it makes it possible for us to maintain and own the code as the project develops.
-
-Code contributions are seen as a positive thing. People enjoy and engage with our project, but it also comes with some challenges; we have to understand the code, we have to understand the feature, we might have to become familiar with external libraries or services and we think about security implications. All those steps are required during the reviewing process. After the code has been merged, the feature has to be maintained. Any changes reliant on external services must be updated and expanded accordingly. 
-
-The review and day-1 maintenance adds a significant burden on the maintainers. Often we hope that the contributor will help out, but we found that most of the time, they disappear after their new feature was added.
-
-This means that when someone contributes, we are mostly happy about it, but we do have to run it through a series of checks to establish if we actually can maintain this feature.
-
-## What do we require?
-
-A general description is provided here and an explicit list is provided in our pull request template.
-
-All new features have to start out with a design document, which should be discussed on the issue tracker (not discord). It should include a use case for the feature, how it can be implemented, who will implement it and a plan for maintaining it.
-
-All features have to be end-to-end tested (integration tests) and have good unit test coverage to ensure that they work as expected. This will also ensure that the feature continues to work as expected over time. If a change cannot be tested, a strong case for why this is not possible needs to be presented.
-
-The contributor should help to maintain the feature over time. In case the feature is not maintained probably, the maintainers reserve themselves the right to remove features they redeem as unmaintainable. This should help to improve the quality of the software and keep it in a maintainable state.
-
-## Bug fixes
-
-Headscale is open to code contributions for bug fixes without discussion.
-
-## Documentation
-
-If you find mistakes in the documentation, please submit a fix to the documentation.

Dockerfile.debug

@@ -2,24 +2,31 @@
 # and are in no way endorsed by Headscale's maintainers as an
 # official nor supported release or distribution.
 
-FROM docker.io/golang:1.22-bookworm
+FROM docker.io/golang:1.22-bookworm AS build
 ARG VERSION=dev
 ENV GOPATH /go
 WORKDIR /go/src/headscale
 
+COPY go.mod go.sum /go/src/headscale/
+RUN go mod download
+
+COPY . .
+
+RUN CGO_ENABLED=0 GOOS=linux go install -ldflags="-s -w -X github.com/juanfont/headscale/cmd/headscale/cli.Version=$VERSION" -a ./cmd/headscale
+RUN test -e /go/bin/headscale
+
+# Debug image
+FROM docker.io/golang:1.22-bookworm
+
+COPY --from=build /go/bin/headscale /bin/headscale
+ENV TZ UTC
+
 RUN apt-get update \
   && apt-get install --no-install-recommends --yes less jq \
   && rm -rf /var/lib/apt/lists/* \
   && apt-get clean
 RUN mkdir -p /var/run/headscale
 
-COPY go.mod go.sum /go/src/headscale/
-RUN go mod download
-
-COPY . .
-
-RUN CGO_ENABLED=0 GOOS=linux go install -ldflags="-s -w -X github.com/juanfont/headscale/cmd/headscale/cli.Version=$VERSION" -a ./cmd/headscale && test -e /go/bin/headscale
-
 # Need to reset the entrypoint or everything will run as a busybox script
 ENTRYPOINT []
 EXPOSE 8080/tcp

README.md

@@ -95,7 +95,15 @@ The maintainers work together on setting the direction for the project. The unde
 
 ## Contributing
 
-Please read the [CONTRIBUTING.md](./CONTRIBUTING.md) file.
+Headscale is "Open Source, acknowledged contribution", this means that any
+contribution will have to be discussed with the Maintainers before being submitted.
+
+This model has been chosen to reduce the risk of burnout by limiting the
+maintenance overhead of reviewing and validating third-party code.
+
+Headscale is open to code contributions for bug fixes without discussion.
+
+If you find mistakes in the documentation, please submit a fix to the documentation.
 
 ### Requirements
 

cmd/headscale/cli/root.go

@@ -51,7 +51,7 @@ func initConfig() {
 
 	cfg, err := types.GetHeadscaleConfig()
 	if err != nil {
-		log.Fatal().Err(err).Msg("Failed to read headscale configuration")
+		log.Fatal().Caller().Err(err).Msg("Failed to get headscale configuration")
 	}
 
 	machineOutput := HasMachineOutputFlag()

hscontrol/app.go

@@ -137,7 +137,7 @@ func NewHeadscale(cfg *types.Config) (*Headscale, error) {
 		noisePrivateKey:    noisePrivateKey,
 		registrationCache:  registrationCache,
 		pollNetMapStreamWG: sync.WaitGroup{},
-		nodeNotifier:       notifier.NewNotifier(cfg),
+		nodeNotifier:       notifier.NewNotifier(),
 		mapSessions:        make(map[types.NodeID]*mapSession),
 	}
 
@@ -800,23 +800,10 @@ func (h *Headscale) Serve() error {
 				}
 
 			default:
-				trace := log.Trace().Msgf
 				log.Info().
 					Str("signal", sig.String()).
 					Msg("Received signal to stop, shutting down gracefully")
 
-				trace("closing map sessions")
-				wg := sync.WaitGroup{}
-				for _, mapSess := range h.mapSessions {
-					wg.Add(1)
-					go func() {
-						mapSess.close()
-						wg.Done()
-					}()
-				}
-				wg.Wait()
-
-				trace("waiting for netmap stream to close")
 				h.pollNetMapStreamWG.Wait()
 
 				// Gracefully shut down servers
@@ -824,44 +811,32 @@ func (h *Headscale) Serve() error {
 					context.Background(),
 					types.HTTPShutdownTimeout,
 				)
-				trace("shutting down debug http server")
 				if err := debugHTTPServer.Shutdown(ctx); err != nil {
 					log.Error().Err(err).Msg("Failed to shutdown prometheus http")
 				}
-				trace("shutting down main http server")
 				if err := httpServer.Shutdown(ctx); err != nil {
 					log.Error().Err(err).Msg("Failed to shutdown http")
 				}
-
-				trace("shutting down grpc server (socket)")
 				grpcSocket.GracefulStop()
 
 				if grpcServer != nil {
-					trace("shutting down grpc server (external)")
 					grpcServer.GracefulStop()
 					grpcListener.Close()
 				}
 
 				if tailsqlContext != nil {
-					trace("shutting down tailsql")
 					tailsqlContext.Done()
 				}
 
-				trace("closing node notifier")
-				h.nodeNotifier.Close()
-
 				// Close network listeners
-				trace("closing network listeners")
 				debugHTTPListener.Close()
 				httpListener.Close()
 				grpcGatewayConn.Close()
 
 				// Stop listening (and unlink the socket if unix type):
-				trace("closing socket listener")
 				socketListener.Close()
 
 				// Close db connections
-				trace("closing database connection")
 				err = h.db.Close()
 				if err != nil {
 					log.Error().Err(err).Msg("Failed to close db")

hscontrol/auth.go

@@ -315,16 +315,13 @@ func (h *Headscale) handleAuthKey(
 
 		node.NodeKey = nodeKey
 		node.AuthKeyID = uint(pak.ID)
-		node.Expiry = &registerRequest.Expiry
-		node.User = pak.User
-		node.UserID = pak.UserID
-		err := h.db.DB.Save(node).Error
+		err := h.db.NodeSetExpiry(node.ID, registerRequest.Expiry)
 		if err != nil {
 			log.Error().
 				Caller().
 				Str("node", node.Hostname).
 				Err(err).
-				Msg("failed to save node after logging in with auth key")
+				Msg("Failed to refresh node")
 
 			return
 		}
@@ -347,7 +344,7 @@ func (h *Headscale) handleAuthKey(
 		}
 
 		ctx := types.NotifyCtx(context.Background(), "handle-authkey", "na")
-		h.nodeNotifier.NotifyAll(ctx, types.StateUpdate{Type: types.StatePeerChanged, ChangeNodes: []types.NodeID{node.ID}})
+		h.nodeNotifier.NotifyWithIgnore(ctx, types.StateUpdateExpire(node.ID, registerRequest.Expiry), node.ID)
 	} else {
 		now := time.Now().UTC()
 

hscontrol/notifier/metrics.go

@@ -18,12 +18,7 @@ var (
 		Namespace: prometheusNamespace,
 		Name:      "notifier_update_sent_total",
 		Help:      "total count of update sent on nodes channel",
-	}, []string{"status", "type", "trigger"})
-	notifierUpdateReceived = promauto.NewCounterVec(prometheus.CounterOpts{
-		Namespace: prometheusNamespace,
-		Name:      "notifier_update_received_total",
-		Help:      "total count of updates received by notifier",
-	}, []string{"type", "trigger"})
+	}, []string{"status", "type"})
 	notifierNodeUpdateChans = promauto.NewGauge(prometheus.GaugeOpts{
 		Namespace: prometheusNamespace,
 		Name:      "notifier_open_channels_total",

hscontrol/notifier/notifier.go

@@ -3,7 +3,7 @@ package notifier
 import (
 	"context"
 	"fmt"
-	"sort"
+	"slices"
 	"strings"
 	"sync"
 	"time"
@@ -11,32 +11,19 @@ import (
 	"github.com/juanfont/headscale/hscontrol/types"
 	"github.com/puzpuzpuz/xsync/v3"
 	"github.com/rs/zerolog/log"
-	"tailscale.com/tailcfg"
-	"tailscale.com/util/set"
 )
 
 type Notifier struct {
 	l         sync.RWMutex
 	nodes     map[types.NodeID]chan<- types.StateUpdate
 	connected *xsync.MapOf[types.NodeID, bool]
-	b         *batcher
 }
 
-func NewNotifier(cfg *types.Config) *Notifier {
-	n := &Notifier{
+func NewNotifier() *Notifier {
+	return &Notifier{
 		nodes:     make(map[types.NodeID]chan<- types.StateUpdate),
 		connected: xsync.NewMapOf[types.NodeID, bool](),
 	}
-	b := newBatcher(cfg.Tuning.BatchChangeDelay, n)
-	n.b = b
-	// TODO(kradalby): clean this up
-	go b.doWork()
-	return n
-}
-
-// Close stops the batcher inside the notifier.
-func (n *Notifier) Close() {
-	n.b.close()
 }
 
 func (n *Notifier) AddNode(nodeID types.NodeID, c chan<- types.StateUpdate) {
@@ -121,8 +108,13 @@ func (n *Notifier) NotifyWithIgnore(
 	update types.StateUpdate,
 	ignoreNodeIDs ...types.NodeID,
 ) {
-	notifierUpdateReceived.WithLabelValues(update.Type.String(), types.NotifyOriginKey.Value(ctx)).Inc()
-	n.b.addOrPassthrough(update)
+	for nodeID := range n.nodes {
+		if slices.Contains(ignoreNodeIDs, nodeID) {
+			continue
+		}
+
+		n.NotifyByNodeID(ctx, update, nodeID)
+	}
 }
 
 func (n *Notifier) NotifyByNodeID(
@@ -147,10 +139,10 @@ func (n *Notifier) NotifyByNodeID(
 			log.Error().
 				Err(ctx.Err()).
 				Uint64("node.id", nodeID.Uint64()).
-				Any("origin", types.NotifyOriginKey.Value(ctx)).
-				Any("origin-hostname", types.NotifyHostnameKey.Value(ctx)).
+				Any("origin", ctx.Value("origin")).
+				Any("origin-hostname", ctx.Value("hostname")).
 				Msgf("update not sent, context cancelled")
-			notifierUpdateSent.WithLabelValues("cancelled", update.Type.String(), types.NotifyOriginKey.Value(ctx)).Inc()
+			notifierUpdateSent.WithLabelValues("cancelled", update.Type.String()).Inc()
 
 			return
 		case c <- update:
@@ -159,23 +151,11 @@ func (n *Notifier) NotifyByNodeID(
 				Any("origin", ctx.Value("origin")).
 				Any("origin-hostname", ctx.Value("hostname")).
 				Msgf("update successfully sent on chan")
-			notifierUpdateSent.WithLabelValues("ok", update.Type.String(), types.NotifyOriginKey.Value(ctx)).Inc()
+			notifierUpdateSent.WithLabelValues("ok", update.Type.String()).Inc()
 		}
 	}
 }
 
-func (n *Notifier) sendAll(update types.StateUpdate) {
-	start := time.Now()
-	n.l.RLock()
-	defer n.l.RUnlock()
-	notifierWaitForLock.WithLabelValues("send-all").Observe(time.Since(start).Seconds())
-
-	for _, c := range n.nodes {
-		c <- update
-		notifierUpdateSent.WithLabelValues("ok", update.Type.String(), "send-all").Inc()
-	}
-}
-
 func (n *Notifier) String() string {
 	n.l.RLock()
 	defer n.l.RUnlock()
@@ -197,166 +177,3 @@ func (n *Notifier) String() string {
 
 	return b.String()
 }
-
-type batcher struct {
-	tick *time.Ticker
-
-	mu sync.Mutex
-
-	cancelCh chan struct{}
-
-	changedNodeIDs set.Slice[types.NodeID]
-	nodesChanged   bool
-	patches        map[types.NodeID]tailcfg.PeerChange
-	patchesChanged bool
-
-	n *Notifier
-}
-
-func newBatcher(batchTime time.Duration, n *Notifier) *batcher {
-	return &batcher{
-		tick:     time.NewTicker(batchTime),
-		cancelCh: make(chan struct{}),
-		patches:  make(map[types.NodeID]tailcfg.PeerChange),
-		n:        n,
-	}
-
-}
-
-func (b *batcher) close() {
-	b.cancelCh <- struct{}{}
-}
-
-// addOrPassthrough adds the update to the batcher, if it is not a
-// type that is currently batched, it will be sent immediately.
-func (b *batcher) addOrPassthrough(update types.StateUpdate) {
-	b.mu.Lock()
-	defer b.mu.Unlock()
-
-	switch update.Type {
-	case types.StatePeerChanged:
-		b.changedNodeIDs.Add(update.ChangeNodes...)
-		b.nodesChanged = true
-
-	case types.StatePeerChangedPatch:
-		for _, newPatch := range update.ChangePatches {
-			if curr, ok := b.patches[types.NodeID(newPatch.NodeID)]; ok {
-				overwritePatch(&curr, newPatch)
-				b.patches[types.NodeID(newPatch.NodeID)] = curr
-			} else {
-				b.patches[types.NodeID(newPatch.NodeID)] = *newPatch
-			}
-		}
-		b.patchesChanged = true
-
-	default:
-		b.n.sendAll(update)
-	}
-}
-
-// flush sends all the accumulated patches to all
-// nodes in the notifier.
-func (b *batcher) flush() {
-	b.mu.Lock()
-	defer b.mu.Unlock()
-
-	if b.nodesChanged || b.patchesChanged {
-		var patches []*tailcfg.PeerChange
-		// If a node is getting a full update from a change
-		// node update, then the patch can be dropped.
-		for nodeID, patch := range b.patches {
-			if b.changedNodeIDs.Contains(nodeID) {
-				delete(b.patches, nodeID)
-			} else {
-				patches = append(patches, &patch)
-			}
-		}
-
-		changedNodes := b.changedNodeIDs.Slice().AsSlice()
-		sort.Slice(changedNodes, func(i, j int) bool {
-			return changedNodes[i] < changedNodes[j]
-		})
-
-		if b.changedNodeIDs.Slice().Len() > 0 {
-			update := types.StateUpdate{
-				Type:        types.StatePeerChanged,
-				ChangeNodes: changedNodes,
-			}
-
-			b.n.sendAll(update)
-		}
-
-		if len(patches) > 0 {
-			patchUpdate := types.StateUpdate{
-				Type:          types.StatePeerChangedPatch,
-				ChangePatches: patches,
-			}
-
-			b.n.sendAll(patchUpdate)
-		}
-
-		b.changedNodeIDs = set.Slice[types.NodeID]{}
-		b.nodesChanged = false
-		b.patches = make(map[types.NodeID]tailcfg.PeerChange, len(b.patches))
-		b.patchesChanged = false
-	}
-}
-
-func (b *batcher) doWork() {
-	for {
-		select {
-		case <-b.cancelCh:
-			return
-		case <-b.tick.C:
-			b.flush()
-		}
-	}
-}
-
-// overwritePatch takes the current patch and a newer patch
-// and override any field that has changed
-func overwritePatch(currPatch, newPatch *tailcfg.PeerChange) {
-	if newPatch.DERPRegion != 0 {
-		currPatch.DERPRegion = newPatch.DERPRegion
-	}
-
-	if newPatch.Cap != 0 {
-		currPatch.Cap = newPatch.Cap
-	}
-
-	if newPatch.CapMap != nil {
-		currPatch.CapMap = newPatch.CapMap
-	}
-
-	if newPatch.Endpoints != nil {
-		currPatch.Endpoints = newPatch.Endpoints
-	}
-
-	if newPatch.Key != nil {
-		currPatch.Key = newPatch.Key
-	}
-
-	if newPatch.KeySignature != nil {
-		currPatch.KeySignature = newPatch.KeySignature
-	}
-
-	if newPatch.DiscoKey != nil {
-		currPatch.DiscoKey = newPatch.DiscoKey
-	}
-
-	if newPatch.Online != nil {
-		currPatch.Online = newPatch.Online
-	}
-
-	if newPatch.LastSeen != nil {
-		currPatch.LastSeen = newPatch.LastSeen
-	}
-
-	if newPatch.KeyExpiry != nil {
-		currPatch.KeyExpiry = newPatch.KeyExpiry
-	}
-
-	if newPatch.Capabilities != nil {
-		currPatch.Capabilities = newPatch.Capabilities
-	}
-}

hscontrol/notifier/notifier_test.go

@@ -1,249 +0,0 @@
-package notifier
-
-import (
-	"context"
-	"net/netip"
-	"testing"
-	"time"
-
-	"github.com/google/go-cmp/cmp"
-	"github.com/juanfont/headscale/hscontrol/types"
-	"github.com/juanfont/headscale/hscontrol/util"
-	"tailscale.com/tailcfg"
-)
-
-func TestBatcher(t *testing.T) {
-	tests := []struct {
-		name    string
-		updates []types.StateUpdate
-		want    []types.StateUpdate
-	}{
-		{
-			name: "full-passthrough",
-			updates: []types.StateUpdate{
-				{
-					Type: types.StateFullUpdate,
-				},
-			},
-			want: []types.StateUpdate{
-				{
-					Type: types.StateFullUpdate,
-				},
-			},
-		},
-		{
-			name: "derp-passthrough",
-			updates: []types.StateUpdate{
-				{
-					Type: types.StateDERPUpdated,
-				},
-			},
-			want: []types.StateUpdate{
-				{
-					Type: types.StateDERPUpdated,
-				},
-			},
-		},
-		{
-			name: "single-node-update",
-			updates: []types.StateUpdate{
-				{
-					Type: types.StatePeerChanged,
-					ChangeNodes: []types.NodeID{
-						2,
-					},
-				},
-			},
-			want: []types.StateUpdate{
-				{
-					Type: types.StatePeerChanged,
-					ChangeNodes: []types.NodeID{
-						2,
-					},
-				},
-			},
-		},
-		{
-			name: "merge-node-update",
-			updates: []types.StateUpdate{
-				{
-					Type: types.StatePeerChanged,
-					ChangeNodes: []types.NodeID{
-						2, 4,
-					},
-				},
-				{
-					Type: types.StatePeerChanged,
-					ChangeNodes: []types.NodeID{
-						2, 3,
-					},
-				},
-			},
-			want: []types.StateUpdate{
-				{
-					Type: types.StatePeerChanged,
-					ChangeNodes: []types.NodeID{
-						2, 3, 4,
-					},
-				},
-			},
-		},
-		{
-			name: "single-patch-update",
-			updates: []types.StateUpdate{
-				{
-					Type: types.StatePeerChangedPatch,
-					ChangePatches: []*tailcfg.PeerChange{
-						{
-							NodeID:     2,
-							DERPRegion: 5,
-						},
-					},
-				},
-			},
-			want: []types.StateUpdate{
-				{
-					Type: types.StatePeerChangedPatch,
-					ChangePatches: []*tailcfg.PeerChange{
-						{
-							NodeID:     2,
-							DERPRegion: 5,
-						},
-					},
-				},
-			},
-		},
-		{
-			name: "merge-patch-to-same-node-update",
-			updates: []types.StateUpdate{
-				{
-					Type: types.StatePeerChangedPatch,
-					ChangePatches: []*tailcfg.PeerChange{
-						{
-							NodeID:     2,
-							DERPRegion: 5,
-						},
-					},
-				},
-				{
-					Type: types.StatePeerChangedPatch,
-					ChangePatches: []*tailcfg.PeerChange{
-						{
-							NodeID:     2,
-							DERPRegion: 6,
-						},
-					},
-				},
-			},
-			want: []types.StateUpdate{
-				{
-					Type: types.StatePeerChangedPatch,
-					ChangePatches: []*tailcfg.PeerChange{
-						{
-							NodeID:     2,
-							DERPRegion: 6,
-						},
-					},
-				},
-			},
-		},
-		{
-			name: "merge-patch-to-multiple-node-update",
-			updates: []types.StateUpdate{
-				{
-					Type: types.StatePeerChangedPatch,
-					ChangePatches: []*tailcfg.PeerChange{
-						{
-							NodeID: 3,
-							Endpoints: []netip.AddrPort{
-								netip.MustParseAddrPort("1.1.1.1:9090"),
-							},
-						},
-					},
-				},
-				{
-					Type: types.StatePeerChangedPatch,
-					ChangePatches: []*tailcfg.PeerChange{
-						{
-							NodeID: 3,
-							Endpoints: []netip.AddrPort{
-								netip.MustParseAddrPort("1.1.1.1:9090"),
-								netip.MustParseAddrPort("2.2.2.2:8080"),
-							},
-						},
-					},
-				},
-				{
-					Type: types.StatePeerChangedPatch,
-					ChangePatches: []*tailcfg.PeerChange{
-						{
-							NodeID:     4,
-							DERPRegion: 6,
-						},
-					},
-				},
-				{
-					Type: types.StatePeerChangedPatch,
-					ChangePatches: []*tailcfg.PeerChange{
-						{
-							NodeID: 4,
-							Cap:    tailcfg.CapabilityVersion(54),
-						},
-					},
-				},
-			},
-			want: []types.StateUpdate{
-				{
-					Type: types.StatePeerChangedPatch,
-					ChangePatches: []*tailcfg.PeerChange{
-						{
-							NodeID: 3,
-							Endpoints: []netip.AddrPort{
-								netip.MustParseAddrPort("1.1.1.1:9090"),
-								netip.MustParseAddrPort("2.2.2.2:8080"),
-							},
-						},
-						{
-							NodeID:     4,
-							DERPRegion: 6,
-							Cap:        tailcfg.CapabilityVersion(54),
-						},
-					},
-				},
-			},
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			n := NewNotifier(&types.Config{
-				Tuning: types.Tuning{
-					// We will call flush manually for the tests,
-					// so do not run the worker.
-					BatchChangeDelay: time.Hour,
-				},
-			})
-
-			ch := make(chan types.StateUpdate, 30)
-			defer close(ch)
-			n.AddNode(1, ch)
-			defer n.RemoveNode(1)
-
-			for _, u := range tt.updates {
-				n.NotifyAll(context.Background(), u)
-			}
-
-			n.b.flush()
-
-			var got []types.StateUpdate
-			for len(ch) > 0 {
-				out := <-ch
-				got = append(got, out)
-			}
-
-			if diff := cmp.Diff(tt.want, got, util.Comparers...); diff != "" {
-				t.Errorf("batcher() unexpected result (-want +got):\n%s", diff)
-			}
-		})
-	}
-}

hscontrol/policy/acls.go

@@ -36,38 +36,6 @@ const (
 	expectedTokenItems = 2
 )
 
-var theInternetSet *netipx.IPSet
-
-// theInternet returns the IPSet for the Internet.
-// https://www.youtube.com/watch?v=iDbyYGrswtg
-func theInternet() *netipx.IPSet {
-	if theInternetSet != nil {
-		return theInternetSet
-	}
-
-	var internetBuilder netipx.IPSetBuilder
-	internetBuilder.AddPrefix(netip.MustParsePrefix("2000::/3"))
-	internetBuilder.AddPrefix(netip.MustParsePrefix("0.0.0.0/0"))
-
-	// Delete Private network addresses
-	// https://datatracker.ietf.org/doc/html/rfc1918
-	internetBuilder.RemovePrefix(netip.MustParsePrefix("fc00::/7"))
-	internetBuilder.RemovePrefix(netip.MustParsePrefix("10.0.0.0/8"))
-	internetBuilder.RemovePrefix(netip.MustParsePrefix("172.16.0.0/12"))
-	internetBuilder.RemovePrefix(netip.MustParsePrefix("192.168.0.0/16"))
-
-	// Delete Tailscale networks
-	internetBuilder.RemovePrefix(netip.MustParsePrefix("fd7a:115c:a1e0::/48"))
-	internetBuilder.RemovePrefix(netip.MustParsePrefix("100.64.0.0/10"))
-
-	// Delete "cant find DHCP networks"
-	internetBuilder.RemovePrefix(netip.MustParsePrefix("fe80::/10")) // link-loca
-	internetBuilder.RemovePrefix(netip.MustParsePrefix("169.254.0.0/16"))
-
-	theInternetSet, _ := internetBuilder.IPSet()
-	return theInternetSet
-}
-
 // For some reason golang.org/x/net/internal/iana is an internal package.
 const (
 	protocolICMP     = 1   // Internet Control Message
@@ -253,28 +221,28 @@ func ReduceFilterRules(node *types.Node, rules []tailcfg.FilterRule) []tailcfg.F
 		// record if the rule is actually relevant for the given node.
 		dests := []tailcfg.NetPortRange{}
 
-	DEST_LOOP:
 		for _, dest := range rule.DstPorts {
 			expanded, err := util.ParseIPSet(dest.IP, nil)
 			// Fail closed, if we cant parse it, then we should not allow
 			// access.
 			if err != nil {
-				continue DEST_LOOP
+				continue
 			}
 
 			if node.InIPSet(expanded) {
 				dests = append(dests, dest)
-				continue DEST_LOOP
 			}
 
 			// If the node exposes routes, ensure they are note removed
 			// when the filters are reduced.
 			if node.Hostinfo != nil {
+				// TODO(kradalby): Evaluate if we should only keep
+				// the routes if the route is enabled. This will
+				// require database access in this part of the code.
 				if len(node.Hostinfo.RoutableIPs) > 0 {
 					for _, routableIP := range node.Hostinfo.RoutableIPs {
-						if expanded.OverlapsPrefix(routableIP) {
+						if expanded.ContainsPrefix(routableIP) {
 							dests = append(dests, dest)
-							continue DEST_LOOP
 						}
 					}
 				}
@@ -549,7 +517,6 @@ func (pol *ACLPolicy) expandSource(
 // - a host
 // - an ip
 // - a cidr
-// - an autogroup
 // and transform these in IPAddresses.
 func (pol *ACLPolicy) ExpandAlias(
 	nodes types.Nodes,
@@ -575,10 +542,6 @@ func (pol *ACLPolicy) ExpandAlias(
 		return pol.expandIPsFromTag(alias, nodes)
 	}
 
-	if isAutoGroup(alias) {
-		return expandAutoGroup(alias)
-	}
-
 	// if alias is a user
 	if ips, err := pol.expandIPsFromUser(alias, nodes); ips != nil {
 		return ips, err
@@ -899,16 +862,6 @@ func (pol *ACLPolicy) expandIPsFromIPPrefix(
 	return build.IPSet()
 }
 
-func expandAutoGroup(alias string) (*netipx.IPSet, error) {
-	switch {
-	case strings.HasPrefix(alias, "autogroup:internet"):
-		return theInternet(), nil
-
-	default:
-		return nil, fmt.Errorf("unknown autogroup %q", alias)
-	}
-}
-
 func isWildcard(str string) bool {
 	return str == "*"
 }
@@ -921,10 +874,6 @@ func isTag(str string) bool {
 	return strings.HasPrefix(str, "tag:")
 }
 
-func isAutoGroup(str string) bool {
-	return strings.HasPrefix(str, "autogroup:")
-}
-
 // TagsOfNode will return the tags of the current node.
 // Invalid tags are tags added by a user on a node, and that user doesn't have authority to add this tag.
 // Valid tags are tags added by a user that is allowed in the ACL policy to add this tag.

hscontrol/policy/acls_test.go

@@ -1765,108 +1765,6 @@ func TestACLPolicy_generateFilterRules(t *testing.T) {
 	}
 }
 
-// tsExitNodeDest is the list of destination IP ranges that are allowed when
-// you dump the filter list from a Tailscale node connected to Tailscale SaaS.
-var tsExitNodeDest = []tailcfg.NetPortRange{
-	{
-		IP:    "0.0.0.0-9.255.255.255",
-		Ports: tailcfg.PortRangeAny,
-	},
-	{
-		IP:    "11.0.0.0-100.63.255.255",
-		Ports: tailcfg.PortRangeAny,
-	},
-	{
-		IP:    "100.128.0.0-169.253.255.255",
-		Ports: tailcfg.PortRangeAny,
-	},
-	{
-		IP:    "169.255.0.0-172.15.255.255",
-		Ports: tailcfg.PortRangeAny,
-	},
-	{
-		IP:    "172.32.0.0-192.167.255.255",
-		Ports: tailcfg.PortRangeAny,
-	},
-	{
-		IP:    "192.169.0.0-255.255.255.255",
-		Ports: tailcfg.PortRangeAny,
-	},
-	{
-		IP:    "2000::-3fff:ffff:ffff:ffff:ffff:ffff:ffff:ffff",
-		Ports: tailcfg.PortRangeAny,
-	},
-}
-
-// hsExitNodeDest is the list of destination IP ranges that are allowed when
-// we use headscale "autogroup:internet"
-var hsExitNodeDest = []tailcfg.NetPortRange{
-	{IP: "0.0.0.0/5", Ports: tailcfg.PortRangeAny},
-	{IP: "8.0.0.0/7", Ports: tailcfg.PortRangeAny},
-	{IP: "11.0.0.0/8", Ports: tailcfg.PortRangeAny},
-	{IP: "12.0.0.0/6", Ports: tailcfg.PortRangeAny},
-	{IP: "16.0.0.0/4", Ports: tailcfg.PortRangeAny},
-	{IP: "32.0.0.0/3", Ports: tailcfg.PortRangeAny},
-	{IP: "64.0.0.0/3", Ports: tailcfg.PortRangeAny},
-	{IP: "96.0.0.0/6", Ports: tailcfg.PortRangeAny},
-	{IP: "100.0.0.0/10", Ports: tailcfg.PortRangeAny},
-	{IP: "100.128.0.0/9", Ports: tailcfg.PortRangeAny},
-	{IP: "101.0.0.0/8", Ports: tailcfg.PortRangeAny},
-	{IP: "102.0.0.0/7", Ports: tailcfg.PortRangeAny},
-	{IP: "104.0.0.0/5", Ports: tailcfg.PortRangeAny},
-	{IP: "112.0.0.0/4", Ports: tailcfg.PortRangeAny},
-	{IP: "128.0.0.0/3", Ports: tailcfg.PortRangeAny},
-	{IP: "160.0.0.0/5", Ports: tailcfg.PortRangeAny},
-	{IP: "168.0.0.0/8", Ports: tailcfg.PortRangeAny},
-	{IP: "169.0.0.0/9", Ports: tailcfg.PortRangeAny},
-	{IP: "169.128.0.0/10", Ports: tailcfg.PortRangeAny},
-	{IP: "169.192.0.0/11", Ports: tailcfg.PortRangeAny},
-	{IP: "169.224.0.0/12", Ports: tailcfg.PortRangeAny},
-	{IP: "169.240.0.0/13", Ports: tailcfg.PortRangeAny},
-	{IP: "169.248.0.0/14", Ports: tailcfg.PortRangeAny},
-	{IP: "169.252.0.0/15", Ports: tailcfg.PortRangeAny},
-	{IP: "169.255.0.0/16", Ports: tailcfg.PortRangeAny},
-	{IP: "170.0.0.0/7", Ports: tailcfg.PortRangeAny},
-	{IP: "172.0.0.0/12", Ports: tailcfg.PortRangeAny},
-	{IP: "172.32.0.0/11", Ports: tailcfg.PortRangeAny},
-	{IP: "172.64.0.0/10", Ports: tailcfg.PortRangeAny},
-	{IP: "172.128.0.0/9", Ports: tailcfg.PortRangeAny},
-	{IP: "173.0.0.0/8", Ports: tailcfg.PortRangeAny},
-	{IP: "174.0.0.0/7", Ports: tailcfg.PortRangeAny},
-	{IP: "176.0.0.0/4", Ports: tailcfg.PortRangeAny},
-	{IP: "192.0.0.0/9", Ports: tailcfg.PortRangeAny},
-	{IP: "192.128.0.0/11", Ports: tailcfg.PortRangeAny},
-	{IP: "192.160.0.0/13", Ports: tailcfg.PortRangeAny},
-	{IP: "192.169.0.0/16", Ports: tailcfg.PortRangeAny},
-	{IP: "192.170.0.0/15", Ports: tailcfg.PortRangeAny},
-	{IP: "192.172.0.0/14", Ports: tailcfg.PortRangeAny},
-	{IP: "192.176.0.0/12", Ports: tailcfg.PortRangeAny},
-	{IP: "192.192.0.0/10", Ports: tailcfg.PortRangeAny},
-	{IP: "193.0.0.0/8", Ports: tailcfg.PortRangeAny},
-	{IP: "194.0.0.0/7", Ports: tailcfg.PortRangeAny},
-	{IP: "196.0.0.0/6", Ports: tailcfg.PortRangeAny},
-	{IP: "200.0.0.0/5", Ports: tailcfg.PortRangeAny},
-	{IP: "208.0.0.0/4", Ports: tailcfg.PortRangeAny},
-	{IP: "224.0.0.0/3", Ports: tailcfg.PortRangeAny},
-	{IP: "2000::/3", Ports: tailcfg.PortRangeAny},
-}
-
-func TestTheInternet(t *testing.T) {
-	internetSet := theInternet()
-
-	internetPrefs := internetSet.Prefixes()
-
-	for i, _ := range internetPrefs {
-		if internetPrefs[i].String() != hsExitNodeDest[i].IP {
-			t.Errorf("prefix from internet set %q != hsExit list %q", internetPrefs[i].String(), hsExitNodeDest[i].IP)
-		}
-	}
-
-	if len(internetPrefs) != len(hsExitNodeDest) {
-		t.Fatalf("expected same length of prefixes, internet: %d, hsExit: %d", len(internetPrefs), len(hsExitNodeDest))
-	}
-}
-
 func TestReduceFilterRules(t *testing.T) {
 	tests := []struct {
 		name  string
@@ -1971,473 +1869,15 @@ func TestReduceFilterRules(t *testing.T) {
 				},
 			},
 		},
-		{
-			name: "1786-reducing-breaks-exit-nodes-the-client",
-			pol: ACLPolicy{
-				Hosts: Hosts{
-					// Exit node
-					"internal": netip.MustParsePrefix("100.64.0.100/32"),
-				},
-				Groups: Groups{
-					"group:team": {"user3", "user2", "user1"},
-				},
-				ACLs: []ACL{
-					{
-						Action:  "accept",
-						Sources: []string{"group:team"},
-						Destinations: []string{
-							"internal:*",
-						},
-					},
-					{
-						Action:  "accept",
-						Sources: []string{"group:team"},
-						Destinations: []string{
-							"autogroup:internet:*",
-						},
-					},
-				},
-			},
-			node: &types.Node{
-				IPv4: iap("100.64.0.1"),
-				IPv6: iap("fd7a:115c:a1e0::1"),
-				User: types.User{Name: "user1"},
-			},
-			peers: types.Nodes{
-				&types.Node{
-					IPv4: iap("100.64.0.2"),
-					IPv6: iap("fd7a:115c:a1e0::2"),
-					User: types.User{Name: "user2"},
-				},
-				// "internal" exit node
-				&types.Node{
-					IPv4: iap("100.64.0.100"),
-					IPv6: iap("fd7a:115c:a1e0::100"),
-					User: types.User{Name: "user100"},
-					Hostinfo: &tailcfg.Hostinfo{
-						RoutableIPs: []netip.Prefix{types.ExitRouteV4, types.ExitRouteV6},
-					},
-				},
-			},
-			want: []tailcfg.FilterRule{},
-		},
-		{
-			name: "1786-reducing-breaks-exit-nodes-the-exit",
-			pol: ACLPolicy{
-				Hosts: Hosts{
-					// Exit node
-					"internal": netip.MustParsePrefix("100.64.0.100/32"),
-				},
-				Groups: Groups{
-					"group:team": {"user3", "user2", "user1"},
-				},
-				ACLs: []ACL{
-					{
-						Action:  "accept",
-						Sources: []string{"group:team"},
-						Destinations: []string{
-							"internal:*",
-						},
-					},
-					{
-						Action:  "accept",
-						Sources: []string{"group:team"},
-						Destinations: []string{
-							"autogroup:internet:*",
-						},
-					},
-				},
-			},
-			node: &types.Node{
-				IPv4: iap("100.64.0.100"),
-				IPv6: iap("fd7a:115c:a1e0::100"),
-				User: types.User{Name: "user100"},
-				Hostinfo: &tailcfg.Hostinfo{
-					RoutableIPs: []netip.Prefix{types.ExitRouteV4, types.ExitRouteV6},
-				},
-			},
-			peers: types.Nodes{
-				&types.Node{
-					IPv4: iap("100.64.0.2"),
-					IPv6: iap("fd7a:115c:a1e0::2"),
-					User: types.User{Name: "user2"},
-				},
-				&types.Node{
-					IPv4: iap("100.64.0.1"),
-					IPv6: iap("fd7a:115c:a1e0::1"),
-					User: types.User{Name: "user1"},
-				},
-			},
-			want: []tailcfg.FilterRule{
-				{
-					SrcIPs: []string{"100.64.0.1/32", "100.64.0.2/32", "fd7a:115c:a1e0::1/128", "fd7a:115c:a1e0::2/128"},
-					DstPorts: []tailcfg.NetPortRange{
-						{
-							IP:    "100.64.0.100/32",
-							Ports: tailcfg.PortRangeAny,
-						},
-						{
-							IP:    "fd7a:115c:a1e0::100/128",
-							Ports: tailcfg.PortRangeAny,
-						},
-					},
-				},
-				{
-					SrcIPs:   []string{"100.64.0.1/32", "100.64.0.2/32", "fd7a:115c:a1e0::1/128", "fd7a:115c:a1e0::2/128"},
-					DstPorts: hsExitNodeDest,
-				},
-			},
-		},
-		{
-			name: "1786-reducing-breaks-exit-nodes-the-example-from-issue",
-			pol: ACLPolicy{
-				Hosts: Hosts{
-					// Exit node
-					"internal": netip.MustParsePrefix("100.64.0.100/32"),
-				},
-				Groups: Groups{
-					"group:team": {"user3", "user2", "user1"},
-				},
-				ACLs: []ACL{
-					{
-						Action:  "accept",
-						Sources: []string{"group:team"},
-						Destinations: []string{
-							"internal:*",
-						},
-					},
-					{
-						Action:  "accept",
-						Sources: []string{"group:team"},
-						Destinations: []string{
-							"0.0.0.0/5:*",
-							"8.0.0.0/7:*",
-							"11.0.0.0/8:*",
-							"12.0.0.0/6:*",
-							"16.0.0.0/4:*",
-							"32.0.0.0/3:*",
-							"64.0.0.0/2:*",
-							"128.0.0.0/3:*",
-							"160.0.0.0/5:*",
-							"168.0.0.0/6:*",
-							"172.0.0.0/12:*",
-							"172.32.0.0/11:*",
-							"172.64.0.0/10:*",
-							"172.128.0.0/9:*",
-							"173.0.0.0/8:*",
-							"174.0.0.0/7:*",
-							"176.0.0.0/4:*",
-							"192.0.0.0/9:*",
-							"192.128.0.0/11:*",
-							"192.160.0.0/13:*",
-							"192.169.0.0/16:*",
-							"192.170.0.0/15:*",
-							"192.172.0.0/14:*",
-							"192.176.0.0/12:*",
-							"192.192.0.0/10:*",
-							"193.0.0.0/8:*",
-							"194.0.0.0/7:*",
-							"196.0.0.0/6:*",
-							"200.0.0.0/5:*",
-							"208.0.0.0/4:*",
-						},
-					},
-				},
-			},
-			node: &types.Node{
-				IPv4: iap("100.64.0.100"),
-				IPv6: iap("fd7a:115c:a1e0::100"),
-				User: types.User{Name: "user100"},
-				Hostinfo: &tailcfg.Hostinfo{
-					RoutableIPs: []netip.Prefix{types.ExitRouteV4, types.ExitRouteV6},
-				},
-			},
-			peers: types.Nodes{
-				&types.Node{
-					IPv4: iap("100.64.0.2"),
-					IPv6: iap("fd7a:115c:a1e0::2"),
-					User: types.User{Name: "user2"},
-				},
-				&types.Node{
-					IPv4: iap("100.64.0.1"),
-					IPv6: iap("fd7a:115c:a1e0::1"),
-					User: types.User{Name: "user1"},
-				},
-			},
-			want: []tailcfg.FilterRule{
-				{
-					SrcIPs: []string{"100.64.0.1/32", "100.64.0.2/32", "fd7a:115c:a1e0::1/128", "fd7a:115c:a1e0::2/128"},
-					DstPorts: []tailcfg.NetPortRange{
-						{
-							IP:    "100.64.0.100/32",
-							Ports: tailcfg.PortRangeAny,
-						},
-						{
-							IP:    "fd7a:115c:a1e0::100/128",
-							Ports: tailcfg.PortRangeAny,
-						},
-					},
-				},
-				{
-					SrcIPs: []string{"100.64.0.1/32", "100.64.0.2/32", "fd7a:115c:a1e0::1/128", "fd7a:115c:a1e0::2/128"},
-					DstPorts: []tailcfg.NetPortRange{
-						{IP: "0.0.0.0/5", Ports: tailcfg.PortRangeAny},
-						{IP: "8.0.0.0/7", Ports: tailcfg.PortRangeAny},
-						{IP: "11.0.0.0/8", Ports: tailcfg.PortRangeAny},
-						{IP: "12.0.0.0/6", Ports: tailcfg.PortRangeAny},
-						{IP: "16.0.0.0/4", Ports: tailcfg.PortRangeAny},
-						{IP: "32.0.0.0/3", Ports: tailcfg.PortRangeAny},
-						{IP: "64.0.0.0/2", Ports: tailcfg.PortRangeAny},
-						{IP: "fd7a:115c:a1e0::1/128", Ports: tailcfg.PortRangeAny},
-						{IP: "fd7a:115c:a1e0::2/128", Ports: tailcfg.PortRangeAny},
-						{IP: "fd7a:115c:a1e0::100/128", Ports: tailcfg.PortRangeAny},
-						{IP: "128.0.0.0/3", Ports: tailcfg.PortRangeAny},
-						{IP: "160.0.0.0/5", Ports: tailcfg.PortRangeAny},
-						{IP: "168.0.0.0/6", Ports: tailcfg.PortRangeAny},
-						{IP: "172.0.0.0/12", Ports: tailcfg.PortRangeAny},
-						{IP: "172.32.0.0/11", Ports: tailcfg.PortRangeAny},
-						{IP: "172.64.0.0/10", Ports: tailcfg.PortRangeAny},
-						{IP: "172.128.0.0/9", Ports: tailcfg.PortRangeAny},
-						{IP: "173.0.0.0/8", Ports: tailcfg.PortRangeAny},
-						{IP: "174.0.0.0/7", Ports: tailcfg.PortRangeAny},
-						{IP: "176.0.0.0/4", Ports: tailcfg.PortRangeAny},
-						{IP: "192.0.0.0/9", Ports: tailcfg.PortRangeAny},
-						{IP: "192.128.0.0/11", Ports: tailcfg.PortRangeAny},
-						{IP: "192.160.0.0/13", Ports: tailcfg.PortRangeAny},
-						{IP: "192.169.0.0/16", Ports: tailcfg.PortRangeAny},
-						{IP: "192.170.0.0/15", Ports: tailcfg.PortRangeAny},
-						{IP: "192.172.0.0/14", Ports: tailcfg.PortRangeAny},
-						{IP: "192.176.0.0/12", Ports: tailcfg.PortRangeAny},
-						{IP: "192.192.0.0/10", Ports: tailcfg.PortRangeAny},
-						{IP: "193.0.0.0/8", Ports: tailcfg.PortRangeAny},
-						{IP: "194.0.0.0/7", Ports: tailcfg.PortRangeAny},
-						{IP: "196.0.0.0/6", Ports: tailcfg.PortRangeAny},
-						{IP: "200.0.0.0/5", Ports: tailcfg.PortRangeAny},
-						{IP: "208.0.0.0/4", Ports: tailcfg.PortRangeAny},
-					},
-				},
-			},
-		},
-		{
-			name: "1786-reducing-breaks-exit-nodes-app-connector-like",
-			pol: ACLPolicy{
-				Hosts: Hosts{
-					// Exit node
-					"internal": netip.MustParsePrefix("100.64.0.100/32"),
-				},
-				Groups: Groups{
-					"group:team": {"user3", "user2", "user1"},
-				},
-				ACLs: []ACL{
-					{
-						Action:  "accept",
-						Sources: []string{"group:team"},
-						Destinations: []string{
-							"internal:*",
-						},
-					},
-					{
-						Action:  "accept",
-						Sources: []string{"group:team"},
-						Destinations: []string{
-							"8.0.0.0/8:*",
-							"16.0.0.0/8:*",
-						},
-					},
-				},
-			},
-			node: &types.Node{
-				IPv4: iap("100.64.0.100"),
-				IPv6: iap("fd7a:115c:a1e0::100"),
-				User: types.User{Name: "user100"},
-				Hostinfo: &tailcfg.Hostinfo{
-					RoutableIPs: []netip.Prefix{netip.MustParsePrefix("8.0.0.0/16"), netip.MustParsePrefix("16.0.0.0/16")},
-				},
-			},
-			peers: types.Nodes{
-				&types.Node{
-					IPv4: iap("100.64.0.2"),
-					IPv6: iap("fd7a:115c:a1e0::2"),
-					User: types.User{Name: "user2"},
-				},
-				&types.Node{
-					IPv4: iap("100.64.0.1"),
-					IPv6: iap("fd7a:115c:a1e0::1"),
-					User: types.User{Name: "user1"},
-				},
-			},
-			want: []tailcfg.FilterRule{
-				{
-					SrcIPs: []string{"100.64.0.1/32", "100.64.0.2/32", "fd7a:115c:a1e0::1/128", "fd7a:115c:a1e0::2/128"},
-					DstPorts: []tailcfg.NetPortRange{
-						{
-							IP:    "100.64.0.100/32",
-							Ports: tailcfg.PortRangeAny,
-						},
-						{
-							IP:    "fd7a:115c:a1e0::100/128",
-							Ports: tailcfg.PortRangeAny,
-						},
-					},
-				},
-				{
-					SrcIPs: []string{"100.64.0.1/32", "100.64.0.2/32", "fd7a:115c:a1e0::1/128", "fd7a:115c:a1e0::2/128"},
-					DstPorts: []tailcfg.NetPortRange{
-						{
-							IP:    "8.0.0.0/8",
-							Ports: tailcfg.PortRangeAny,
-						},
-						{
-							IP:    "16.0.0.0/8",
-							Ports: tailcfg.PortRangeAny,
-						},
-					},
-				},
-			},
-		},
-		{
-			name: "1786-reducing-breaks-exit-nodes-app-connector-like2",
-			pol: ACLPolicy{
-				Hosts: Hosts{
-					// Exit node
-					"internal": netip.MustParsePrefix("100.64.0.100/32"),
-				},
-				Groups: Groups{
-					"group:team": {"user3", "user2", "user1"},
-				},
-				ACLs: []ACL{
-					{
-						Action:  "accept",
-						Sources: []string{"group:team"},
-						Destinations: []string{
-							"internal:*",
-						},
-					},
-					{
-						Action:  "accept",
-						Sources: []string{"group:team"},
-						Destinations: []string{
-							"8.0.0.0/16:*",
-							"16.0.0.0/16:*",
-						},
-					},
-				},
-			},
-			node: &types.Node{
-				IPv4: iap("100.64.0.100"),
-				IPv6: iap("fd7a:115c:a1e0::100"),
-				User: types.User{Name: "user100"},
-				Hostinfo: &tailcfg.Hostinfo{
-					RoutableIPs: []netip.Prefix{netip.MustParsePrefix("8.0.0.0/8"), netip.MustParsePrefix("16.0.0.0/8")},
-				},
-			},
-			peers: types.Nodes{
-				&types.Node{
-					IPv4: iap("100.64.0.2"),
-					IPv6: iap("fd7a:115c:a1e0::2"),
-					User: types.User{Name: "user2"},
-				},
-				&types.Node{
-					IPv4: iap("100.64.0.1"),
-					IPv6: iap("fd7a:115c:a1e0::1"),
-					User: types.User{Name: "user1"},
-				},
-			},
-			want: []tailcfg.FilterRule{
-				{
-					SrcIPs: []string{"100.64.0.1/32", "100.64.0.2/32", "fd7a:115c:a1e0::1/128", "fd7a:115c:a1e0::2/128"},
-					DstPorts: []tailcfg.NetPortRange{
-						{
-							IP:    "100.64.0.100/32",
-							Ports: tailcfg.PortRangeAny,
-						},
-						{
-							IP:    "fd7a:115c:a1e0::100/128",
-							Ports: tailcfg.PortRangeAny,
-						},
-					},
-				},
-				{
-					SrcIPs: []string{"100.64.0.1/32", "100.64.0.2/32", "fd7a:115c:a1e0::1/128", "fd7a:115c:a1e0::2/128"},
-					DstPorts: []tailcfg.NetPortRange{
-						{
-							IP:    "8.0.0.0/16",
-							Ports: tailcfg.PortRangeAny,
-						},
-						{
-							IP:    "16.0.0.0/16",
-							Ports: tailcfg.PortRangeAny,
-						},
-					},
-				},
-			},
-		},
-		{
-			name: "1817-reduce-breaks-32-mask",
-			pol: ACLPolicy{
-				Hosts: Hosts{
-					"vlan1": netip.MustParsePrefix("172.16.0.0/24"),
-					"dns1":  netip.MustParsePrefix("172.16.0.21/32"),
-				},
-				Groups: Groups{
-					"group:access": {"user1"},
-				},
-				ACLs: []ACL{
-					{
-						Action:  "accept",
-						Sources: []string{"group:access"},
-						Destinations: []string{
-							"tag:access-servers:*",
-							"dns1:*",
-						},
-					},
-				},
-			},
-			node: &types.Node{
-				IPv4: iap("100.64.0.100"),
-				IPv6: iap("fd7a:115c:a1e0::100"),
-				User: types.User{Name: "user100"},
-				Hostinfo: &tailcfg.Hostinfo{
-					RoutableIPs: []netip.Prefix{netip.MustParsePrefix("172.16.0.0/24")},
-				},
-				ForcedTags: types.StringList{"tag:access-servers"},
-			},
-			peers: types.Nodes{
-				&types.Node{
-					IPv4: iap("100.64.0.1"),
-					IPv6: iap("fd7a:115c:a1e0::1"),
-					User: types.User{Name: "user1"},
-				},
-			},
-			want: []tailcfg.FilterRule{
-				{
-					SrcIPs: []string{"100.64.0.1/32", "fd7a:115c:a1e0::1/128"},
-					DstPorts: []tailcfg.NetPortRange{
-						{
-							IP:    "100.64.0.100/32",
-							Ports: tailcfg.PortRangeAny,
-						},
-						{
-							IP:    "fd7a:115c:a1e0::100/128",
-							Ports: tailcfg.PortRangeAny,
-						},
-						{
-							IP:    "172.16.0.21/32",
-							Ports: tailcfg.PortRangeAny,
-						},
-					},
-				},
-			},
-		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got, _ := tt.pol.CompileFilterRules(
+			rules, _ := tt.pol.CompileFilterRules(
 				append(tt.peers, tt.node),
 			)
 
-			got = ReduceFilterRules(tt.node, got)
+			got := ReduceFilterRules(tt.node, rules)
 
 			if diff := cmp.Diff(tt.want, got); diff != "" {
 				log.Trace().Interface("got", got).Msg("result")

hscontrol/poll.go

@@ -66,16 +66,10 @@ func (h *Headscale) newMapSession(
 ) *mapSession {
 	warnf, infof, tracef, errf := logPollFunc(req, node)
 
-	var updateChan chan types.StateUpdate
-	if req.Stream {
-		// Use a buffered channel in case a node is not fully ready
-		// to receive a message to make sure we dont block the entire
-		// notifier.
-		updateChan = make(chan types.StateUpdate, h.cfg.Tuning.NodeMapSessionBufferedChanSize)
-		updateChan <- types.StateUpdate{
-			Type: types.StateFullUpdate,
-		}
-	}
+	// Use a buffered channel in case a node is not fully ready
+	// to receive a message to make sure we dont block the entire
+	// notifier.
+	updateChan := make(chan types.StateUpdate, h.cfg.Tuning.NodeMapSessionBufferedChanSize)
 
 	return &mapSession{
 		h:      h,
@@ -224,26 +218,33 @@ func (m *mapSession) serve() {
 	ctx, cancel := context.WithCancel(context.WithValue(m.ctx, nodeNameContextKey, m.node.Hostname))
 	defer cancel()
 
+	// TODO(kradalby): Make this available through a tuning envvar
+	wait := time.Second
+
+	// Add a circuit breaker, if the loop is not interrupted
+	// inbetween listening for the channels, some updates
+	// might get stale and stucked in the "changed" map
+	// defined below.
+	blockBreaker := time.NewTicker(wait)
+
+	// true means changed, false means removed
+	var changed map[types.NodeID]bool
+	var patches []*tailcfg.PeerChange
+	var derp bool
+
+	// Set full to true to immediatly send a full mapresponse
+	full := true
+	prev := time.Now()
+	lastMessage := ""
+
 	// Loop through updates and continuously send them to the
 	// client.
 	for {
-		// consume channels with update, keep alives or "batch" blocking signals
-		select {
-		case <-m.cancelCh:
-			m.tracef("poll cancelled received")
-			return
-		case <-ctx.Done():
-			m.tracef("poll context done")
-			return
-
-		// Consume all updates sent to node
-		case update := <-m.ch:
-			m.tracef("received stream update: %s %s", update.Type.String(), update.Message)
-			mapResponseUpdateReceived.WithLabelValues(update.Type.String()).Inc()
-
+		// If a full update has been requested or there are patches, then send it immediately
+		// otherwise wait for the "batching" of changes or patches
+		if full || patches != nil || (changed != nil && time.Since(prev) > wait) {
 			var data []byte
 			var err error
-			var lastMessage string
 
 			// Ensure the node object is updated, for example, there
 			// might have been a hostinfo update in a sidechannel
@@ -255,43 +256,62 @@ func (m *mapSession) serve() {
 				return
 			}
 
+			// If there are patches _and_ fully changed nodes, filter the
+			// patches and remove all patches that are present for the full
+			// changes updates. This allows us to send them as part of the
+			// PeerChange update, but only for nodes that are not fully changed.
+			// The fully changed nodes will be updated from the database and
+			// have all the updates needed.
+			// This means that the patches left are for nodes that has no
+			// updates that requires a full update.
+			// Patches are not suppose to be mixed in, but can be.
+			//
+			// From tailcfg docs:
+			// These are applied after Peers* above, but in practice the
+			// control server should only send these on their own, without
+			//
+			// Currently, there is no effort to merge patch updates, they
+			// are all sent, and the client will apply them in order.
+			// TODO(kradalby): Merge Patches for the same IDs to send less
+			// data and give the client less work.
+			if patches != nil && changed != nil {
+				var filteredPatches []*tailcfg.PeerChange
+
+				for _, patch := range patches {
+					if _, ok := changed[types.NodeID(patch.NodeID)]; !ok {
+						filteredPatches = append(filteredPatches, patch)
+					}
+				}
+
+				patches = filteredPatches
+			}
+
 			updateType := "full"
-			switch update.Type {
-			case types.StateFullUpdate:
+			// When deciding what update to send, the following is considered,
+			// Full is a superset of all updates, when a full update is requested,
+			// send only that and move on, all other updates will be present in
+			// a full map response.
+			//
+			// If a map of changed nodes exists, prefer sending that as it will
+			// contain all the updates for the node, including patches, as it
+			// is fetched freshly from the database when building the response.
+			//
+			// If there is full changes registered, but we have patches for individual
+			// nodes, send them.
+			//
+			// Finally, if a DERP map is the only request, send that alone.
+			if full {
 				m.tracef("Sending Full MapResponse")
 				data, err = m.mapper.FullMapResponse(m.req, m.node, m.h.ACLPolicy, fmt.Sprintf("from mapSession: %p, stream: %t", m, m.isStreaming()))
-			case types.StatePeerChanged:
-				changed := make(map[types.NodeID]bool, len(update.ChangeNodes))
-
-				for _, nodeID := range update.ChangeNodes {
-					changed[nodeID] = true
-				}
-
-				lastMessage = update.Message
+			} else if changed != nil {
 				m.tracef(fmt.Sprintf("Sending Changed MapResponse: %v", lastMessage))
-				data, err = m.mapper.PeerChangedResponse(m.req, m.node, changed, update.ChangePatches, m.h.ACLPolicy, lastMessage)
+				data, err = m.mapper.PeerChangedResponse(m.req, m.node, changed, patches, m.h.ACLPolicy, lastMessage)
 				updateType = "change"
-
-			case types.StatePeerChangedPatch:
+			} else if patches != nil {
 				m.tracef(fmt.Sprintf("Sending Changed Patch MapResponse: %v", lastMessage))
-				data, err = m.mapper.PeerChangedPatchResponse(m.req, m.node, update.ChangePatches, m.h.ACLPolicy)
+				data, err = m.mapper.PeerChangedPatchResponse(m.req, m.node, patches, m.h.ACLPolicy)
 				updateType = "patch"
-			case types.StatePeerRemoved:
-				changed := make(map[types.NodeID]bool, len(update.Removed))
-
-				for _, nodeID := range update.Removed {
-					changed[nodeID] = false
-				}
-				m.tracef(fmt.Sprintf("Sending Changed MapResponse: %v", lastMessage))
-				data, err = m.mapper.PeerChangedResponse(m.req, m.node, changed, update.ChangePatches, m.h.ACLPolicy, lastMessage)
-				updateType = "remove"
-			case types.StateSelfUpdate:
-				lastMessage = update.Message
-				m.tracef(fmt.Sprintf("Sending Changed MapResponse: %v", lastMessage))
-				// create the map so an empty (self) update is sent
-				data, err = m.mapper.PeerChangedResponse(m.req, m.node, make(map[types.NodeID]bool), update.ChangePatches, m.h.ACLPolicy, lastMessage)
-				updateType = "remove"
-			case types.StateDERPUpdated:
+			} else if derp {
 				m.tracef("Sending DERPUpdate MapResponse")
 				data, err = m.mapper.DERPMapResponse(m.req, m.node, m.h.DERPMap)
 				updateType = "derp"
@@ -328,6 +348,68 @@ func (m *mapSession) serve() {
 				m.tracef("update sent")
 			}
 
+			// reset
+			changed = nil
+			patches = nil
+			lastMessage = ""
+			full = false
+			derp = false
+			prev = time.Now()
+		}
+
+		// consume channels with update, keep alives or "batch" blocking signals
+		select {
+		case <-m.cancelCh:
+			m.tracef("poll cancelled received")
+			return
+		case <-ctx.Done():
+			m.tracef("poll context done")
+			return
+
+			// Avoid infinite block that would potentially leave
+		// some updates in the changed map.
+		case <-blockBreaker.C:
+			continue
+
+		// Consume all updates sent to node
+		case update := <-m.ch:
+			m.tracef("received stream update: %s %s", update.Type.String(), update.Message)
+			mapResponseUpdateReceived.WithLabelValues(update.Type.String()).Inc()
+
+			switch update.Type {
+			case types.StateFullUpdate:
+				full = true
+			case types.StatePeerChanged:
+				if changed == nil {
+					changed = make(map[types.NodeID]bool)
+				}
+
+				for _, nodeID := range update.ChangeNodes {
+					changed[nodeID] = true
+				}
+
+				lastMessage = update.Message
+			case types.StatePeerChangedPatch:
+				patches = append(patches, update.ChangePatches...)
+			case types.StatePeerRemoved:
+				if changed == nil {
+					changed = make(map[types.NodeID]bool)
+				}
+
+				for _, nodeID := range update.Removed {
+					changed[nodeID] = false
+				}
+			case types.StateSelfUpdate:
+				// create the map so an empty (self) update is sent
+				if changed == nil {
+					changed = make(map[types.NodeID]bool)
+				}
+
+				lastMessage = update.Message
+			case types.StateDERPUpdated:
+				derp = true
+			}
+
 		case <-m.keepAliveTicker.C:
 			data, err := m.mapper.KeepAliveResponse(m.req, m.node)
 			if err != nil {

hscontrol/types/common.go

@@ -10,7 +10,6 @@ import (
 	"time"
 
 	"tailscale.com/tailcfg"
-	"tailscale.com/util/ctxkey"
 )
 
 const (
@@ -184,14 +183,10 @@ func StateUpdateExpire(nodeID NodeID, expiry time.Time) StateUpdate {
 	}
 }
 
-var (
-	NotifyOriginKey   = ctxkey.New("notify.origin", "")
-	NotifyHostnameKey = ctxkey.New("notify.hostname", "")
-)
-
 func NotifyCtx(ctx context.Context, origin, hostname string) context.Context {
-	ctx2, _ := context.WithTimeout(ctx, 3*time.Second)
-	ctx2 = NotifyOriginKey.WithValue(ctx2, origin)
-	ctx2 = NotifyHostnameKey.WithValue(ctx2, hostname)
+	ctx2, _ := context.WithTimeout(
+		context.WithValue(context.WithValue(ctx, "hostname", hostname), "origin", origin),
+		3*time.Second,
+	)
 	return ctx2
 }

hscontrol/types/config.go

@@ -650,10 +650,6 @@ func GetHeadscaleConfig() (*Config, error) {
 		return nil, err
 	}
 
-	if prefix4 == nil && prefix6 == nil {
-		return nil, fmt.Errorf("no IPv4 or IPv6 prefix configured, minimum one prefix is required")
-	}
-
 	allocStr := viper.GetString("prefixes.allocation")
 	var alloc IPAllocationStrategy
 	switch allocStr {
@@ -662,7 +658,7 @@ func GetHeadscaleConfig() (*Config, error) {
 	case string(IPAllocationStrategyRandom):
 		alloc = IPAllocationStrategyRandom
 	default:
-		return nil, fmt.Errorf("config error, prefixes.allocation is set to %s, which is not a valid strategy, allowed options: %s, %s", allocStr, IPAllocationStrategySequential, IPAllocationStrategyRandom)
+		log.Fatal().Msgf("config error, prefixes.allocation is set to %s, which is not a valid strategy, allowed options: %s, %s", allocStr, IPAllocationStrategySequential, IPAllocationStrategyRandom)
 	}
 
 	dnsConfig, baseDomain := GetDNSConfig()

hscontrol/types/node.go

@@ -306,15 +306,11 @@ func (node *Node) AfterFind(tx *gorm.DB) error {
 	}
 	node.NodeKey = nodeKey
 
-	// DiscoKey might be empty if a node has not sent it to headscale.
-	// This means that this might fail if the disco key is empty.
-	if node.DiscoKeyDatabaseField != "" {
-		var discoKey key.DiscoPublic
-		if err := discoKey.UnmarshalText([]byte(node.DiscoKeyDatabaseField)); err != nil {
-			return fmt.Errorf("unmarshalling disco key from db: %w", err)
-		}
-		node.DiscoKey = discoKey
+	var discoKey key.DiscoPublic
+	if err := discoKey.UnmarshalText([]byte(node.DiscoKeyDatabaseField)); err != nil {
+		return fmt.Errorf("unmarshalling disco key from db: %w", err)
 	}
+	node.DiscoKey = discoKey
 
 	endpoints := make([]netip.AddrPort, len(node.EndpointsDatabaseField))
 	for idx, ep := range node.EndpointsDatabaseField {

integration/cli_test.go

@@ -388,101 +388,6 @@ func TestPreAuthKeyCommandReusableEphemeral(t *testing.T) {
 	assert.Len(t, listedPreAuthKeys, 3)
 }
 
-func TestPreAuthKeyCorrectUserLoggedInCommand(t *testing.T) {
-	IntegrationSkip(t)
-	t.Parallel()
-
-	user1 := "user1"
-	user2 := "user2"
-
-	scenario, err := NewScenario(dockertestMaxWait())
-	assertNoErr(t, err)
-	defer scenario.Shutdown()
-
-	spec := map[string]int{
-		user1: 1,
-		user2: 0,
-	}
-
-	err = scenario.CreateHeadscaleEnv(spec, []tsic.Option{}, hsic.WithTestName("clipak"))
-	assertNoErr(t, err)
-
-	headscale, err := scenario.Headscale()
-	assertNoErr(t, err)
-
-	var user2Key v1.PreAuthKey
-
-	err = executeAndUnmarshal(
-		headscale,
-		[]string{
-			"headscale",
-			"preauthkeys",
-			"--user",
-			user2,
-			"create",
-			"--reusable",
-			"--expiration",
-			"24h",
-			"--output",
-			"json",
-			"--tags",
-			"tag:test1,tag:test2",
-		},
-		&user2Key,
-	)
-	assertNoErr(t, err)
-
-	allClients, err := scenario.ListTailscaleClients()
-	assertNoErrListClients(t, err)
-
-	assert.Len(t, allClients, 1)
-
-	client := allClients[0]
-
-	// Log out from user1
-	err = client.Logout()
-	assertNoErr(t, err)
-
-	err = scenario.WaitForTailscaleLogout()
-	assertNoErr(t, err)
-
-	status, err := client.Status()
-	assertNoErr(t, err)
-	if status.BackendState == "Starting" || status.BackendState == "Running" {
-		t.Fatalf("expected node to be logged out, backend state: %s", status.BackendState)
-	}
-
-	err = client.Login(headscale.GetEndpoint(), user2Key.GetKey())
-	assertNoErr(t, err)
-
-	status, err = client.Status()
-	assertNoErr(t, err)
-	if status.BackendState != "Running" {
-		t.Fatalf("expected node to be logged in, backend state: %s", status.BackendState)
-	}
-
-	if status.Self.UserID.String() != "userid:2" {
-		t.Fatalf("expected node to be logged in as userid:2, got: %s", status.Self.UserID.String())
-	}
-
-	var listNodes []v1.Node
-	err = executeAndUnmarshal(
-		headscale,
-		[]string{
-			"headscale",
-			"nodes",
-			"list",
-			"--output",
-			"json",
-		},
-		&listNodes,
-	)
-	assert.Nil(t, err)
-	assert.Len(t, listNodes, 1)
-
-	assert.Equal(t, "user2", listNodes[0].User.Name)
-}
-
 func TestApiKeyCommand(t *testing.T) {
 	IntegrationSkip(t)
 	t.Parallel()

integration/hsic/hsic.go

@@ -11,7 +11,6 @@ import (
 	"encoding/pem"
 	"errors"
 	"fmt"
-	"io"
 	"log"
 	"math/big"
 	"net"
@@ -397,14 +396,6 @@ func (t *HeadscaleInContainer) Shutdown() error {
 		)
 	}
 
-	err = t.SaveMetrics("/tmp/control/metrics.txt")
-	if err != nil {
-		log.Printf(
-			"Failed to metrics from control: %s",
-			err,
-		)
-	}
-
 	// Send a interrupt signal to the "headscale" process inside the container
 	// allowing it to shut down gracefully and flush the profile to disk.
 	// The container will live for a bit longer due to the sleep at the end.
@@ -457,25 +448,6 @@ func (t *HeadscaleInContainer) SaveLog(path string) error {
 	return dockertestutil.SaveLog(t.pool, t.container, path)
 }
 
-func (t *HeadscaleInContainer) SaveMetrics(savePath string) error {
-	resp, err := http.Get(fmt.Sprintf("http://%s:9090/metrics", t.hostname))
-	if err != nil {
-		return fmt.Errorf("getting metrics: %w", err)
-	}
-	defer resp.Body.Close()
-	out, err := os.Create(savePath)
-	if err != nil {
-		return fmt.Errorf("creating file for metrics: %w", err)
-	}
-	defer out.Close()
-	_, err = io.Copy(out, resp.Body)
-	if err != nil {
-		return fmt.Errorf("copy response to file: %w", err)
-	}
-
-	return nil
-}
-
 func (t *HeadscaleInContainer) SaveProfile(savePath string) error {
 	tarFile, err := t.FetchPath("/tmp/profile")
 	if err != nil {

integration/route_test.go

@@ -252,7 +252,7 @@ func TestHASubnetRouterFailover(t *testing.T) {
 
 	scenario, err := NewScenario(dockertestMaxWait())
 	assertNoErrf(t, "failed to create scenario: %s", err)
-	defer scenario.Shutdown()
+	// defer scenario.Shutdown()
 
 	spec := map[string]int{
 		user: 3,