move logic for validating node names (#2127)

* move logic for validating node names

this commits moves the generation of "given names" of nodes
into the registration function, and adds validation of renames
to RenameNode using the same logic.

Fixes #2121

Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com>

* fix double arg

Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com>

---------

Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com>

.github/workflows/test-integration.yaml

@@ -52,6 +52,7 @@ jobs:
           - TestExpireNode
           - TestNodeOnlineStatus
           - TestPingAllByIPManyUpDown
+          - Test2118DeletingOnlineNodePanics
           - TestEnablingRoutes
           - TestHASubnetRouterFailover
           - TestEnableDisableAutoApprovedRoute

hscontrol/auth.go

@@ -66,7 +66,7 @@ func (h *Headscale) handleRegister(
 	regReq tailcfg.RegisterRequest,
 	machineKey key.MachinePublic,
 ) {
-	logInfo, logTrace, logErr := logAuthFunc(regReq, machineKey)
+	logInfo, logTrace, _ := logAuthFunc(regReq, machineKey)
 	now := time.Now().UTC()
 	logTrace("handleRegister called, looking up machine in DB")
 	node, err := h.db.GetNodeByAnyKey(machineKey, regReq.NodeKey, regReq.OldNodeKey)
@@ -105,16 +105,6 @@ func (h *Headscale) handleRegister(
 
 		logInfo("Node not found in database, creating new")
 
-		givenName, err := h.db.GenerateGivenName(
-			machineKey,
-			regReq.Hostinfo.Hostname,
-		)
-		if err != nil {
-			logErr(err, "Failed to generate given name for node")
-
-			return
-		}
-
 		// The node did not have a key to authenticate, which means
 		// that we rely on a method that calls back some how (OpenID or CLI)
 		// We create the node and then keep it around until a callback
@@ -122,7 +112,6 @@ func (h *Headscale) handleRegister(
 		newNode := types.Node{
 			MachineKey: machineKey,
 			Hostname:   regReq.Hostinfo.Hostname,
-			GivenName:  givenName,
 			NodeKey:    regReq.NodeKey,
 			LastSeen:   &now,
 			Expiry:     &time.Time{},
@@ -354,21 +343,8 @@ func (h *Headscale) handleAuthKey(
 	} else {
 		now := time.Now().UTC()
 
-		givenName, err := h.db.GenerateGivenName(machineKey, registerRequest.Hostinfo.Hostname)
-		if err != nil {
-			log.Error().
-				Caller().
-				Str("func", "RegistrationHandler").
-				Str("hostinfo.name", registerRequest.Hostinfo.Hostname).
-				Err(err).
-				Msg("Failed to generate given name for node")
-
-			return
-		}
-
 		nodeToRegister := types.Node{
 			Hostname:       registerRequest.Hostinfo.Hostname,
-			GivenName:      givenName,
 			UserID:         pak.User.ID,
 			User:           pak.User,
 			MachineKey:     machineKey,

hscontrol/db/node.go

@@ -90,20 +90,6 @@ func (hsdb *HSDatabase) ListEphemeralNodes() (types.Nodes, error) {
 	})
 }
 
-func listNodesByGivenName(tx *gorm.DB, givenName string) (types.Nodes, error) {
-	nodes := types.Nodes{}
-	if err := tx.
-		Preload("AuthKey").
-		Preload("AuthKey.User").
-		Preload("User").
-		Preload("Routes").
-		Where("given_name = ?", givenName).Find(&nodes).Error; err != nil {
-		return nil, err
-	}
-
-	return nodes, nil
-}
-
 func (hsdb *HSDatabase) getNode(user string, name string) (*types.Node, error) {
 	return Read(hsdb.DB, func(rx *gorm.DB) (*types.Node, error) {
 		return getNode(rx, user, name)
@@ -242,9 +228,9 @@ func SetTags(
 }
 
 // RenameNode takes a Node struct and a new GivenName for the nodes
-// and renames it.
+// and renames it. If the name is not unique, it will return an error.
 func RenameNode(tx *gorm.DB,
-	nodeID uint64, newName string,
+	nodeID types.NodeID, newName string,
 ) error {
 	err := util.CheckForFQDNRules(
 		newName,
@@ -253,6 +239,15 @@ func RenameNode(tx *gorm.DB,
 		return fmt.Errorf("renaming node: %w", err)
 	}
 
+	uniq, err := isUnqiueName(tx, newName)
+	if err != nil {
+		return fmt.Errorf("checking if name is unique: %w", err)
+	}
+
+	if !uniq {
+		return fmt.Errorf("name is not unique: %s", newName)
+	}
+
 	if err := tx.Model(&types.Node{}).Where("id = ?", nodeID).Update("given_name", newName).Error; err != nil {
 		return fmt.Errorf("failed to rename node in the database: %w", err)
 	}
@@ -415,6 +410,15 @@ func RegisterNode(tx *gorm.DB, node types.Node, ipv4 *netip.Addr, ipv6 *netip.Ad
 	node.IPv4 = ipv4
 	node.IPv6 = ipv6
 
+	if node.GivenName == "" {
+		givenName, err := ensureUniqueGivenName(tx, node.Hostname)
+		if err != nil {
+			return nil, fmt.Errorf("failed to ensure unique given name: %w", err)
+		}
+
+		node.GivenName = givenName
+	}
+
 	if err := tx.Save(&node).Error; err != nil {
 		return nil, fmt.Errorf("failed register(save) node in the database: %w", err)
 	}
@@ -642,40 +646,32 @@ func generateGivenName(suppliedName string, randomSuffix bool) (string, error) {
 	return normalizedHostname, nil
 }
 
-func (hsdb *HSDatabase) GenerateGivenName(
-	mkey key.MachinePublic,
-	suppliedName string,
-) (string, error) {
-	return Read(hsdb.DB, func(rx *gorm.DB) (string, error) {
-		return GenerateGivenName(rx, mkey, suppliedName)
-	})
+func isUnqiueName(tx *gorm.DB, name string) (bool, error) {
+	nodes := types.Nodes{}
+	if err := tx.
+		Where("given_name = ?", name).Find(&nodes).Error; err != nil {
+		return false, err
+	}
+
+	return len(nodes) == 0, nil
 }
 
-func GenerateGivenName(
+func ensureUniqueGivenName(
 	tx *gorm.DB,
-	mkey key.MachinePublic,
-	suppliedName string,
+	name string,
 ) (string, error) {
-	givenName, err := generateGivenName(suppliedName, false)
+	givenName, err := generateGivenName(name, false)
 	if err != nil {
 		return "", err
 	}
 
-	// Tailscale rules (may differ) https://tailscale.com/kb/1098/machine-names/
-	nodes, err := listNodesByGivenName(tx, givenName)
+	unique, err := isUnqiueName(tx, givenName)
 	if err != nil {
 		return "", err
 	}
 
-	var nodeFound *types.Node
-	for idx, node := range nodes {
-		if node.GivenName == givenName {
-			nodeFound = nodes[idx]
-		}
-	}
-
-	if nodeFound != nil && nodeFound.MachineKey.String() != mkey.String() {
-		postfixedName, err := generateGivenName(suppliedName, true)
+	if !unique {
+		postfixedName, err := generateGivenName(name, true)
 		if err != nil {
 			return "", err
 		}

hscontrol/db/node_test.go

@@ -19,6 +19,7 @@ import (
 	"github.com/puzpuzpuz/xsync/v3"
 	"github.com/stretchr/testify/assert"
 	"gopkg.in/check.v1"
+	"gorm.io/gorm"
 	"tailscale.com/tailcfg"
 	"tailscale.com/types/key"
 	"tailscale.com/types/ptr"
@@ -313,51 +314,6 @@ func (s *Suite) TestExpireNode(c *check.C) {
 	c.Assert(nodeFromDB.IsExpired(), check.Equals, true)
 }
 
-func (s *Suite) TestGenerateGivenName(c *check.C) {
-	user1, err := db.CreateUser("user-1")
-	c.Assert(err, check.IsNil)
-
-	pak, err := db.CreatePreAuthKey(user1.Name, false, false, nil, nil)
-	c.Assert(err, check.IsNil)
-
-	_, err = db.getNode("user-1", "testnode")
-	c.Assert(err, check.NotNil)
-
-	nodeKey := key.NewNode()
-	machineKey := key.NewMachine()
-
-	machineKey2 := key.NewMachine()
-
-	node := &types.Node{
-		ID:             0,
-		MachineKey:     machineKey.Public(),
-		NodeKey:        nodeKey.Public(),
-		Hostname:       "hostname-1",
-		GivenName:      "hostname-1",
-		UserID:         user1.ID,
-		RegisterMethod: util.RegisterMethodAuthKey,
-		AuthKeyID:      ptr.To(pak.ID),
-	}
-
-	trx := db.DB.Save(node)
-	c.Assert(trx.Error, check.IsNil)
-
-	givenName, err := db.GenerateGivenName(machineKey2.Public(), "hostname-2")
-	comment := check.Commentf("Same user, unique nodes, unique hostnames, no conflict")
-	c.Assert(err, check.IsNil, comment)
-	c.Assert(givenName, check.Equals, "hostname-2", comment)
-
-	givenName, err = db.GenerateGivenName(machineKey.Public(), "hostname-1")
-	comment = check.Commentf("Same user, same node, same hostname, no conflict")
-	c.Assert(err, check.IsNil, comment)
-	c.Assert(givenName, check.Equals, "hostname-1", comment)
-
-	givenName, err = db.GenerateGivenName(machineKey2.Public(), "hostname-1")
-	comment = check.Commentf("Same user, unique nodes, same hostname, conflict")
-	c.Assert(err, check.IsNil, comment)
-	c.Assert(givenName, check.Matches, fmt.Sprintf("^hostname-1-[a-z0-9]{%d}$", NodeGivenNameHashLength), comment)
-}
-
 func (s *Suite) TestSetTags(c *check.C) {
 	user, err := db.CreateUser("test")
 	c.Assert(err, check.IsNil)
@@ -778,3 +734,100 @@ func TestListEphemeralNodes(t *testing.T) {
 	assert.Equal(t, nodeEph.UserID, ephemeralNodes[0].UserID)
 	assert.Equal(t, nodeEph.Hostname, ephemeralNodes[0].Hostname)
 }
+
+func TestRenameNode(t *testing.T) {
+	db, err := newTestDB()
+	if err != nil {
+		t.Fatalf("creating db: %s", err)
+	}
+
+	user, err := db.CreateUser("test")
+	assert.NoError(t, err)
+
+	user2, err := db.CreateUser("test2")
+	assert.NoError(t, err)
+
+	node := types.Node{
+		ID:             0,
+		MachineKey:     key.NewMachine().Public(),
+		NodeKey:        key.NewNode().Public(),
+		Hostname:       "test",
+		UserID:         user.ID,
+		RegisterMethod: util.RegisterMethodAuthKey,
+	}
+
+	node2 := types.Node{
+		ID:             0,
+		MachineKey:     key.NewMachine().Public(),
+		NodeKey:        key.NewNode().Public(),
+		Hostname:       "test",
+		UserID:         user2.ID,
+		RegisterMethod: util.RegisterMethodAuthKey,
+	}
+
+	err = db.DB.Save(&node).Error
+	assert.NoError(t, err)
+
+	err = db.DB.Save(&node2).Error
+	assert.NoError(t, err)
+
+	err = db.DB.Transaction(func(tx *gorm.DB) error {
+		_, err := RegisterNode(tx, node, nil, nil)
+		if err != nil {
+			return err
+		}
+		_, err = RegisterNode(tx, node2, nil, nil)
+		return err
+	})
+	assert.NoError(t, err)
+
+	nodes, err := db.ListNodes()
+	assert.NoError(t, err)
+
+	assert.Len(t, nodes, 2)
+
+	t.Logf("node1 %s %s", nodes[0].Hostname, nodes[0].GivenName)
+	t.Logf("node2 %s %s", nodes[1].Hostname, nodes[1].GivenName)
+
+	assert.Equal(t, nodes[0].Hostname, nodes[0].GivenName)
+	assert.NotEqual(t, nodes[1].Hostname, nodes[1].GivenName)
+	assert.Equal(t, nodes[0].Hostname, nodes[1].Hostname)
+	assert.NotEqual(t, nodes[0].Hostname, nodes[1].GivenName)
+	assert.Contains(t, nodes[1].GivenName, nodes[0].Hostname)
+	assert.Equal(t, nodes[0].GivenName, nodes[1].Hostname)
+	assert.Len(t, nodes[0].Hostname, 4)
+	assert.Len(t, nodes[1].Hostname, 4)
+	assert.Len(t, nodes[0].GivenName, 4)
+	assert.Len(t, nodes[1].GivenName, 13)
+
+	// Nodes can be renamed to a unique name
+	err = db.Write(func(tx *gorm.DB) error {
+		return RenameNode(tx, nodes[0].ID, "newname")
+	})
+	assert.NoError(t, err)
+
+	nodes, err = db.ListNodes()
+	assert.NoError(t, err)
+	assert.Len(t, nodes, 2)
+	assert.Equal(t, nodes[0].Hostname, "test")
+	assert.Equal(t, nodes[0].GivenName, "newname")
+
+	// Nodes can reuse name that is no longer used
+	err = db.Write(func(tx *gorm.DB) error {
+		return RenameNode(tx, nodes[1].ID, "test")
+	})
+	assert.NoError(t, err)
+
+	nodes, err = db.ListNodes()
+	assert.NoError(t, err)
+	assert.Len(t, nodes, 2)
+	assert.Equal(t, nodes[0].Hostname, "test")
+	assert.Equal(t, nodes[0].GivenName, "newname")
+	assert.Equal(t, nodes[1].GivenName, "test")
+
+	// Nodes cannot be renamed to used names
+	err = db.Write(func(tx *gorm.DB) error {
+		return RenameNode(tx, nodes[0].ID, "test")
+	})
+	assert.ErrorContains(t, err, "name is not unique")
+}

hscontrol/grpcv1.go

@@ -373,7 +373,7 @@ func (api headscaleV1APIServer) RenameNode(
 	node, err := db.Write(api.h.db.DB, func(tx *gorm.DB) (*types.Node, error) {
 		err := db.RenameNode(
 			tx,
-			request.GetNodeId(),
+			types.NodeID(request.GetNodeId()),
 			request.GetNewName(),
 		)
 		if err != nil {
@@ -802,18 +802,12 @@ func (api headscaleV1APIServer) DebugCreateNode(
 		return nil, err
 	}
 
-	givenName, err := api.h.db.GenerateGivenName(mkey, request.GetName())
-	if err != nil {
-		return nil, err
-	}
-
 	nodeKey := key.NewNode()
 
 	newNode := types.Node{
 		MachineKey: mkey,
 		NodeKey:    nodeKey.Public(),
 		Hostname:   request.GetName(),
-		GivenName:  givenName,
 		User:       *user,
 
 		Expiry:   &time.Time{},

hscontrol/poll.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"math/rand/v2"
 	"net/http"
+	"slices"
 	"sort"
 	"strings"
 	"time"
@@ -273,6 +274,12 @@ func (m *mapSession) serveLongPoll() {
 				return
 			}
 
+			// If the node has been removed from headscale, close the stream
+			if slices.Contains(update.Removed, m.node.ID) {
+				m.tracef("node removed, closing stream")
+				return
+			}
+
 			m.tracef("received stream update: %s %s", update.Type.String(), update.Message)
 			mapResponseUpdateReceived.WithLabelValues(update.Type.String()).Inc()
 

hscontrol/types/config.go

@@ -732,6 +732,9 @@ func prefixV6() (*netip.Prefix, error) {
 // LoadCLIConfig returns the needed configuration for the CLI client
 // of Headscale to connect to a Headscale server.
 func LoadCLIConfig() (*Config, error) {
+	logConfig := logConfig()
+	zerolog.SetGlobalLevel(logConfig.Level)
+
 	return &Config{
 		DisableUpdateCheck: viper.GetBool("disable_check_updates"),
 		UnixSocket:         viper.GetString("unix_socket"),
@@ -741,6 +744,7 @@ func LoadCLIConfig() (*Config, error) {
 			Timeout:  viper.GetDuration("cli.timeout"),
 			Insecure: viper.GetBool("cli.insecure"),
 		},
+		Log: logConfig,
 	}, nil
 }
 

integration/control.go

@@ -6,8 +6,8 @@ import (
 )
 
 type ControlServer interface {
-	Shutdown() error
-	SaveLog(string) error
+	Shutdown() (string, string, error)
+	SaveLog(string) (string, string, error)
 	SaveProfile(string) error
 	Execute(command []string) (string, error)
 	WriteFile(path string, content []byte) error

integration/dockertestutil/logs.go

@@ -17,10 +17,10 @@ func SaveLog(
 	pool *dockertest.Pool,
 	resource *dockertest.Resource,
 	basePath string,
-) error {
+) (string, string, error) {
 	err := os.MkdirAll(basePath, os.ModePerm)
 	if err != nil {
-		return err
+		return "", "", err
 	}
 
 	var stdout bytes.Buffer
@@ -41,28 +41,30 @@ func SaveLog(
 		},
 	)
 	if err != nil {
-		return err
+		return "", "", err
 	}
 
 	log.Printf("Saving logs for %s to %s\n", resource.Container.Name, basePath)
 
+	stdoutPath := path.Join(basePath, resource.Container.Name+".stdout.log")
 	err = os.WriteFile(
-		path.Join(basePath, resource.Container.Name+".stdout.log"),
+		stdoutPath,
 		stdout.Bytes(),
 		filePerm,
 	)
 	if err != nil {
-		return err
+		return "", "", err
 	}
 
+	stderrPath := path.Join(basePath, resource.Container.Name+".stderr.log")
 	err = os.WriteFile(
-		path.Join(basePath, resource.Container.Name+".stderr.log"),
+		stderrPath,
 		stderr.Bytes(),
 		filePerm,
 	)
 	if err != nil {
-		return err
+		return "", "", err
 	}
 
-	return nil
+	return stdoutPath, stderrPath, nil
 }

integration/general_test.go

@@ -954,3 +954,102 @@ func TestPingAllByIPManyUpDown(t *testing.T) {
 		t.Logf("%d successful pings out of %d", success, len(allClients)*len(allIps))
 	}
 }
+
+func Test2118DeletingOnlineNodePanics(t *testing.T) {
+	IntegrationSkip(t)
+	t.Parallel()
+
+	scenario, err := NewScenario(dockertestMaxWait())
+	assertNoErr(t, err)
+	defer scenario.ShutdownAssertNoPanics(t)
+
+	// TODO(kradalby): it does not look like the user thing works, only second
+	// get created? maybe only when many?
+	spec := map[string]int{
+		"user1": 1,
+		"user2": 1,
+	}
+
+	err = scenario.CreateHeadscaleEnv(spec,
+		[]tsic.Option{},
+		hsic.WithTestName("deletenocrash"),
+		hsic.WithEmbeddedDERPServerOnly(),
+		hsic.WithTLS(),
+		hsic.WithHostnameAsServerURL(),
+	)
+	assertNoErrHeadscaleEnv(t, err)
+
+	allClients, err := scenario.ListTailscaleClients()
+	assertNoErrListClients(t, err)
+
+	allIps, err := scenario.ListTailscaleClientsIPs()
+	assertNoErrListClientIPs(t, err)
+
+	err = scenario.WaitForTailscaleSync()
+	assertNoErrSync(t, err)
+
+	allAddrs := lo.Map(allIps, func(x netip.Addr, index int) string {
+		return x.String()
+	})
+
+	success := pingAllHelper(t, allClients, allAddrs)
+	t.Logf("%d successful pings out of %d", success, len(allClients)*len(allIps))
+
+	headscale, err := scenario.Headscale()
+	assertNoErr(t, err)
+
+	// Test list all nodes after added otherUser
+	var nodeList []v1.Node
+	err = executeAndUnmarshal(
+		headscale,
+		[]string{
+			"headscale",
+			"nodes",
+			"list",
+			"--output",
+			"json",
+		},
+		&nodeList,
+	)
+	assert.Nil(t, err)
+	assert.Len(t, nodeList, 2)
+	assert.True(t, nodeList[0].Online)
+	assert.True(t, nodeList[1].Online)
+
+	// Delete the first node, which is online
+	_, err = headscale.Execute(
+		[]string{
+			"headscale",
+			"nodes",
+			"delete",
+			"--identifier",
+			// Delete the last added machine
+			fmt.Sprintf("%d", nodeList[0].Id),
+			"--output",
+			"json",
+			"--force",
+		},
+	)
+	assert.Nil(t, err)
+
+	time.Sleep(2 * time.Second)
+
+	// Ensure that the node has been deleted, this did not occur due to a panic.
+	var nodeListAfter []v1.Node
+	err = executeAndUnmarshal(
+		headscale,
+		[]string{
+			"headscale",
+			"nodes",
+			"list",
+			"--output",
+			"json",
+		},
+		&nodeListAfter,
+	)
+	assert.Nil(t, err)
+	assert.Len(t, nodeListAfter, 1)
+	assert.True(t, nodeListAfter[0].Online)
+	assert.Equal(t, nodeList[1].Id, nodeListAfter[0].Id)
+
+}

integration/hsic/hsic.go

@@ -398,8 +398,8 @@ func (t *HeadscaleInContainer) hasTLS() bool {
 }
 
 // Shutdown stops and cleans up the Headscale container.
-func (t *HeadscaleInContainer) Shutdown() error {
-	err := t.SaveLog("/tmp/control")
+func (t *HeadscaleInContainer) Shutdown() (string, string, error) {
+	stdoutPath, stderrPath, err := t.SaveLog("/tmp/control")
 	if err != nil {
 		log.Printf(
 			"Failed to save log from control: %s",
@@ -458,12 +458,12 @@ func (t *HeadscaleInContainer) Shutdown() error {
 		t.pool.Purge(t.pgContainer)
 	}
 
-	return t.pool.Purge(t.container)
+	return stdoutPath, stderrPath, t.pool.Purge(t.container)
 }
 
 // SaveLog saves the current stdout log of the container to a path
 // on the host system.
-func (t *HeadscaleInContainer) SaveLog(path string) error {
+func (t *HeadscaleInContainer) SaveLog(path string) (string, string, error) {
 	return dockertestutil.SaveLog(t.pool, t.container, path)
 }
 

integration/scenario.go

@@ -8,6 +8,7 @@ import (
 	"os"
 	"sort"
 	"sync"
+	"testing"
 	"time"
 
 	v1 "github.com/juanfont/headscale/gen/go/headscale/v1"
@@ -18,6 +19,7 @@ import (
 	"github.com/ory/dockertest/v3"
 	"github.com/puzpuzpuz/xsync/v3"
 	"github.com/samber/lo"
+	"github.com/stretchr/testify/assert"
 	"golang.org/x/sync/errgroup"
 	"tailscale.com/envknob"
 )
@@ -187,13 +189,9 @@ func NewScenario(maxWait time.Duration) (*Scenario, error) {
 	}, nil
 }
 
-// Shutdown shuts down and cleans up all the containers (ControlServer, TailscaleClient)
-// and networks associated with it.
-// In addition, it will save the logs of the ControlServer to `/tmp/control` in the
-// environment running the tests.
-func (s *Scenario) Shutdown() {
+func (s *Scenario) ShutdownAssertNoPanics(t *testing.T) {
 	s.controlServers.Range(func(_ string, control ControlServer) bool {
-		err := control.Shutdown()
+		stdoutPath, stderrPath, err := control.Shutdown()
 		if err != nil {
 			log.Printf(
 				"Failed to shut down control: %s",
@@ -201,6 +199,16 @@ func (s *Scenario) Shutdown() {
 			)
 		}
 
+		if t != nil {
+			stdout, err := os.ReadFile(stdoutPath)
+			assert.NoError(t, err)
+			assert.NotContains(t, string(stdout), "panic")
+
+			stderr, err := os.ReadFile(stderrPath)
+			assert.NoError(t, err)
+			assert.NotContains(t, string(stderr), "panic")
+		}
+
 		return true
 	})
 
@@ -224,6 +232,14 @@ func (s *Scenario) Shutdown() {
 	// }
 }
 
+// Shutdown shuts down and cleans up all the containers (ControlServer, TailscaleClient)
+// and networks associated with it.
+// In addition, it will save the logs of the ControlServer to `/tmp/control` in the
+// environment running the tests.
+func (s *Scenario) Shutdown() {
+	s.ShutdownAssertNoPanics(nil)
+}
+
 // Users returns the name of all users associated with the Scenario.
 func (s *Scenario) Users() []string {
 	users := make([]string, 0)

integration/tsic/tsic.go

@@ -998,7 +998,9 @@ func (t *TailscaleInContainer) WriteFile(path string, data []byte) error {
 // SaveLog saves the current stdout log of the container to a path
 // on the host system.
 func (t *TailscaleInContainer) SaveLog(path string) error {
-	return dockertestutil.SaveLog(t.pool, t.container, path)
+	// TODO(kradalby): Assert if tailscale logs contains panics.
+	_, _, err := dockertestutil.SaveLog(t.pool, t.container, path)
+	return err
 }
 
 // ReadFile reads a file from the Tailscale container.