more linter fixups

.golangci.yaml

@@ -27,6 +27,7 @@ linters:
     - nolintlint
     - musttag # causes issues with imported libs
     - depguard
+    - exportloopref
 
     # We should strive to enable these:
     - wrapcheck
@@ -56,9 +57,14 @@ linters-settings:
       - ok
       - c
       - tt
+      - tx
+      - rx
 
   gocritic:
     disabled-checks:
       - appendAssign
       # TODO(kradalby): Remove this
       - ifElseChain
+
+  nlreturn:
+    block-size: 4

flake.nix

@@ -32,7 +32,7 @@
 
           # When updating go.mod or go.sum, a new sha will need to be calculated,
           # update this if you have a mismatch after doing a change to thos files.
-          vendorHash = "sha256-CMkYTRjmhvTTrB7JbLj0cj9VEyzpG0iUWXkaOagwYTk=";
+          vendorHash = "sha256-Qoqu2k4vvnbRFLmT/v8lI+HCEWqJsHFs8uZRfNmwQpo=";
 
           subPackages = ["cmd/headscale"];
 

hscontrol/db/db_test.go

@@ -16,6 +16,7 @@ import (
 	"github.com/juanfont/headscale/hscontrol/types"
 	"github.com/juanfont/headscale/hscontrol/util"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"gorm.io/gorm"
 	"zgo.at/zcache/v2"
 )
@@ -44,7 +45,7 @@ func TestMigrations(t *testing.T) {
 				routes, err := Read(h.DB, func(rx *gorm.DB) (types.Routes, error) {
 					return GetRoutes(rx)
 				})
-				assert.NoError(t, err)
+				require.NoError(t, err)
 
 				assert.Len(t, routes, 10)
 				want := types.Routes{
@@ -70,7 +71,7 @@ func TestMigrations(t *testing.T) {
 				routes, err := Read(h.DB, func(rx *gorm.DB) (types.Routes, error) {
 					return GetRoutes(rx)
 				})
-				assert.NoError(t, err)
+				require.NoError(t, err)
 
 				assert.Len(t, routes, 4)
 				want := types.Routes{
@@ -132,7 +133,7 @@ func TestMigrations(t *testing.T) {
 
 					return append(kratest, testkra...), nil
 				})
-				assert.NoError(t, err)
+				require.NoError(t, err)
 
 				assert.Len(t, keys, 5)
 				want := []types.PreAuthKey{
@@ -177,7 +178,7 @@ func TestMigrations(t *testing.T) {
 				nodes, err := Read(h.DB, func(rx *gorm.DB) (types.Nodes, error) {
 					return ListNodes(rx)
 				})
-				assert.NoError(t, err)
+				require.NoError(t, err)
 
 				for _, node := range nodes {
 					assert.Falsef(t, node.MachineKey.IsZero(), "expected non zero machinekey")

hscontrol/db/ip_test.go

@@ -12,6 +12,7 @@ import (
 	"github.com/juanfont/headscale/hscontrol/types"
 	"github.com/juanfont/headscale/hscontrol/util"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"tailscale.com/net/tsaddr"
 	"tailscale.com/types/ptr"
 )
@@ -457,7 +458,12 @@ func TestBackfillIPAddresses(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			db := tt.dbFunc()
 
-			alloc, err := NewIPAllocator(db, tt.prefix4, tt.prefix6, types.IPAllocationStrategySequential)
+			alloc, err := NewIPAllocator(
+				db,
+				tt.prefix4,
+				tt.prefix6,
+				types.IPAllocationStrategySequential,
+			)
 			if err != nil {
 				t.Fatalf("failed to set up ip alloc: %s", err)
 			}
@@ -482,24 +488,29 @@ func TestBackfillIPAddresses(t *testing.T) {
 }
 
 func TestIPAllocatorNextNoReservedIPs(t *testing.T) {
-	alloc, err := NewIPAllocator(db, ptr.To(tsaddr.CGNATRange()), ptr.To(tsaddr.TailscaleULARange()), types.IPAllocationStrategySequential)
+	alloc, err := NewIPAllocator(
+		db,
+		ptr.To(tsaddr.CGNATRange()),
+		ptr.To(tsaddr.TailscaleULARange()),
+		types.IPAllocationStrategySequential,
+	)
 	if err != nil {
 		t.Fatalf("failed to set up ip alloc: %s", err)
 	}
 
 	// Validate that we do not give out 100.100.100.100
 	nextQuad100, err := alloc.next(na("100.100.100.99"), ptr.To(tsaddr.CGNATRange()))
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.Equal(t, na("100.100.100.101"), *nextQuad100)
 
 	// Validate that we do not give out fd7a:115c:a1e0::53
 	nextQuad100v6, err := alloc.next(na("fd7a:115c:a1e0::52"), ptr.To(tsaddr.TailscaleULARange()))
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.Equal(t, na("fd7a:115c:a1e0::54"), *nextQuad100v6)
 
 	// Validate that we do not give out fd7a:115c:a1e0::53
 	nextChrome, err := alloc.next(na("100.115.91.255"), ptr.To(tsaddr.CGNATRange()))
 	t.Logf("chrome: %s", nextChrome.String())
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.Equal(t, na("100.115.94.0"), *nextChrome)
 }

hscontrol/db/node_test.go

@@ -17,6 +17,7 @@ import (
 	"github.com/juanfont/headscale/hscontrol/util"
 	"github.com/puzpuzpuz/xsync/v3"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"gopkg.in/check.v1"
 	"gorm.io/gorm"
 	"tailscale.com/net/tsaddr"
@@ -558,17 +559,17 @@ func TestAutoApproveRoutes(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			adb, err := newTestDB()
-			assert.NoError(t, err)
+			require.NoError(t, err)
 			pol, err := policy.LoadACLPolicyFromBytes([]byte(tt.acl))
 
-			assert.NoError(t, err)
+			require.NoError(t, err)
 			assert.NotNil(t, pol)
 
 			user, err := adb.CreateUser("test")
-			assert.NoError(t, err)
+			require.NoError(t, err)
 
 			pak, err := adb.CreatePreAuthKey(user.Name, false, false, nil, nil)
-			assert.NoError(t, err)
+			require.NoError(t, err)
 
 			nodeKey := key.NewNode()
 			machineKey := key.NewMachine()
@@ -590,21 +591,21 @@ func TestAutoApproveRoutes(t *testing.T) {
 			}
 
 			trx := adb.DB.Save(&node)
-			assert.NoError(t, trx.Error)
+			require.NoError(t, trx.Error)
 
 			sendUpdate, err := adb.SaveNodeRoutes(&node)
-			assert.NoError(t, err)
+			require.NoError(t, err)
 			assert.False(t, sendUpdate)
 
 			node0ByID, err := adb.GetNodeByID(0)
-			assert.NoError(t, err)
+			require.NoError(t, err)
 
 			// TODO(kradalby): Check state update
 			err = adb.EnableAutoApprovedRoutes(pol, node0ByID)
-			assert.NoError(t, err)
+			require.NoError(t, err)
 
 			enabledRoutes, err := adb.GetEnabledRoutes(node0ByID)
-			assert.NoError(t, err)
+			require.NoError(t, err)
 			assert.Len(t, enabledRoutes, len(tt.want))
 
 			tsaddr.SortPrefixes(enabledRoutes)
@@ -697,13 +698,13 @@ func TestListEphemeralNodes(t *testing.T) {
 	}
 
 	user, err := db.CreateUser("test")
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	pak, err := db.CreatePreAuthKey(user.Name, false, false, nil, nil)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	pakEph, err := db.CreatePreAuthKey(user.Name, false, true, nil, nil)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	node := types.Node{
 		ID:             0,
@@ -726,16 +727,16 @@ func TestListEphemeralNodes(t *testing.T) {
 	}
 
 	err = db.DB.Save(&node).Error
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	err = db.DB.Save(&nodeEph).Error
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	nodes, err := db.ListNodes()
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	ephemeralNodes, err := db.ListEphemeralNodes()
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	assert.Len(t, nodes, 2)
 	assert.Len(t, ephemeralNodes, 1)
@@ -753,10 +754,10 @@ func TestRenameNode(t *testing.T) {
 	}
 
 	user, err := db.CreateUser("test")
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	user2, err := db.CreateUser("test2")
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	node := types.Node{
 		ID:             0,
@@ -777,10 +778,10 @@ func TestRenameNode(t *testing.T) {
 	}
 
 	err = db.DB.Save(&node).Error
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	err = db.DB.Save(&node2).Error
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	err = db.DB.Transaction(func(tx *gorm.DB) error {
 		_, err := RegisterNode(tx, node, nil, nil)
@@ -790,10 +791,10 @@ func TestRenameNode(t *testing.T) {
 		_, err = RegisterNode(tx, node2, nil, nil)
 		return err
 	})
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	nodes, err := db.ListNodes()
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	assert.Len(t, nodes, 2)
 
@@ -815,26 +816,26 @@ func TestRenameNode(t *testing.T) {
 	err = db.Write(func(tx *gorm.DB) error {
 		return RenameNode(tx, nodes[0].ID, "newname")
 	})
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	nodes, err = db.ListNodes()
-	assert.NoError(t, err)
+	require.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, "test", nodes[0].Hostname)
+	assert.Equal(t, "newname", nodes[0].GivenName)
 
 	// 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)
+	require.NoError(t, err)
 
 	nodes, err = db.ListNodes()
-	assert.NoError(t, err)
+	require.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")
+	assert.Equal(t, "test", nodes[0].Hostname)
+	assert.Equal(t, "newname", nodes[0].GivenName)
+	assert.Equal(t, "test", nodes[1].GivenName)
 
 	// Nodes cannot be renamed to used names
 	err = db.Write(func(tx *gorm.DB) error {

hscontrol/oidc.go

@@ -488,7 +488,7 @@ func (a *AuthProviderOIDC) registerNode(
 }
 
 // TODO(kradalby):
-// Rewrite in elem-go
+// Rewrite in elem-go.
 func renderOIDCCallbackTemplate(
 	user *types.User,
 ) (*bytes.Buffer, error) {

hscontrol/policy/acls.go

@@ -599,7 +599,7 @@ func (pol *ACLPolicy) ExpandAlias(
 // TODO(kradalby): It is quite hard to understand what this function is doing,
 // it seems like it trying to ensure that we dont include nodes that are tagged
 // when we look up the nodes owned by a user.
-// This should be refactored to be more clear as part of the Tags work in #1369
+// This should be refactored to be more clear as part of the Tags work in #1369.
 func excludeCorrectlyTaggedNodes(
 	aclPolicy *ACLPolicy,
 	nodes types.Nodes,