Extend linter rules (netbirdio#1300)

- dupword checks for duplicate words in the source code
- durationcheck checks for two durations multiplied together
- forbidigo forbids identifiers
- mirror reports wrong mirror patterns of bytes/strings usage
- misspell finds commonly misspelled English words in comments
- predeclared finds code that shadows one of Go's predeclared identifiers
- thelper detects Go test helpers without t.Helper() call and checks the consistency of test helpers

.golangci.yaml

@@ -32,9 +32,16 @@ linters:
     - unused # checks for unused constants, variables, functions and types
     ## disable by default but the have interesting results so lets add them
     - bodyclose # checks whether HTTP response body is closed successfully
+    - dupword # dupword checks for duplicate words in the source code
+    - durationcheck # durationcheck checks for two durations multiplied together
+    - forbidigo # forbidigo forbids identifiers
+    - mirror # mirror reports wrong mirror patterns of bytes/strings usage
+    - misspell # misspess finds commonly misspelled English words in comments
     - nilerr # finds the code that returns nil even if it checks that the error is not nil
     - nilnil # checks that there is no simultaneous return of nil error and an invalid value
+    - predeclared # predeclared finds code that shadows one of Go's predeclared identifiers
     - sqlclosecheck # checks that sql.Rows and sql.Stmt are closed
+    - thelper # thelper detects Go test helpers without t.Helper() call and checks the consistency of test helpers.
     - wastedassign # wastedassign finds wasted assignment statements
 issues:
   # Maximum count of issues with the same text.
@@ -43,12 +50,20 @@ issues:
   max-same-issues: 5
 
   exclude-rules:
+    # allow fmt
+    - path: management/cmd/root.go
+      linters: forbidigo
+    - path: signal/cmd/root.go
+      linters: forbidigo
     - path: sharedsock/filter.go
       linters:
       - unused
     - path: client/firewall/iptables/rule.go
       linters:
       - unused
+    - path: test.go
+      linters:
+      - mirror
     - path: mock.go
       linters:
-      - nilnil
+      - nilnil

client/android/login.go

@@ -205,8 +205,8 @@ func (a *Auth) foregroundGetTokenInfo(urlOpener URLOpener) (*auth.TokenInfo, err
 
 	go urlOpener.Open(flowInfo.VerificationURIComplete)
 
-	waitTimeout := time.Duration(flowInfo.ExpiresIn)
-	waitCTX, cancel := context.WithTimeout(a.ctx, waitTimeout*time.Second)
+	waitTimeout := time.Duration(flowInfo.ExpiresIn) * time.Second
+	waitCTX, cancel := context.WithTimeout(a.ctx, waitTimeout)
 	defer cancel()
 	tokenInfo, err := oAuthFlow.WaitToken(waitCTX, flowInfo)
 	if err != nil {

client/cmd/login.go

@@ -177,8 +177,8 @@ func foregroundGetTokenInfo(ctx context.Context, cmd *cobra.Command, config *int
 
 	openURL(cmd, flowInfo.VerificationURIComplete, flowInfo.UserCode)
 
-	waitTimeout := time.Duration(flowInfo.ExpiresIn)
-	waitCTX, c := context.WithTimeout(context.TODO(), waitTimeout*time.Second)
+	waitTimeout := time.Duration(flowInfo.ExpiresIn) * time.Second
+	waitCTX, c := context.WithTimeout(context.TODO(), waitTimeout)
 	defer c()
 
 	tokenInfo, err := oAuthFlow.WaitToken(waitCTX, flowInfo)

client/cmd/root.go

@@ -92,7 +92,7 @@ func init() {
 	rootCmd.PersistentFlags().StringVar(&adminURL, "admin-url", "", fmt.Sprintf("Admin Panel URL [http|https]://[host]:[port] (default \"%s\")", internal.DefaultAdminURL))
 	rootCmd.PersistentFlags().StringVarP(&configPath, "config", "c", defaultConfigPath, "Netbird config file location")
 	rootCmd.PersistentFlags().StringVarP(&logLevel, "log-level", "l", "info", "sets Netbird log level")
-	rootCmd.PersistentFlags().StringVar(&logFile, "log-file", defaultLogFile, "sets Netbird log path. If console is specified the the log will be output to stdout")
+	rootCmd.PersistentFlags().StringVar(&logFile, "log-file", defaultLogFile, "sets Netbird log path. If console is specified the log will be output to stdout")
 	rootCmd.PersistentFlags().StringVarP(&setupKey, "setup-key", "k", "", "Setup key obtained from the Management Service Dashboard (used to register peer)")
 	rootCmd.PersistentFlags().StringVar(&preSharedKey, "preshared-key", "", "Sets Wireguard PreSharedKey property. If set, then only peers that have the same key can communicate.")
 	rootCmd.PersistentFlags().StringVarP(&hostName, "hostname", "n", "", "Sets a custom hostname for the device")

client/cmd/testutil.go

@@ -22,6 +22,7 @@ import (
 )
 
 func startTestingServices(t *testing.T) string {
+	t.Helper()
 	config := &mgmt.Config{}
 	_, err := util.ReadJson("../testdata/management.json", config)
 	if err != nil {
@@ -44,6 +45,7 @@ func startTestingServices(t *testing.T) string {
 }
 
 func startSignal(t *testing.T) (*grpc.Server, net.Listener) {
+	t.Helper()
 	lis, err := net.Listen("tcp", ":0")
 	if err != nil {
 		t.Fatal(err)
@@ -60,6 +62,7 @@ func startSignal(t *testing.T) (*grpc.Server, net.Listener) {
 }
 
 func startManagement(t *testing.T, config *mgmt.Config) (*grpc.Server, net.Listener) {
+	t.Helper()
 	lis, err := net.Listen("tcp", ":0")
 	if err != nil {
 		t.Fatal(err)
@@ -98,6 +101,7 @@ func startManagement(t *testing.T, config *mgmt.Config) (*grpc.Server, net.Liste
 func startClientDaemon(
 	t *testing.T, ctx context.Context, managementURL, configPath string,
 ) (*grpc.Server, net.Listener) {
+	t.Helper()
 	lis, err := net.Listen("tcp", "127.0.0.1:0")
 	if err != nil {
 		t.Fatal(err)

client/firewall/iptables/manager_linux_test.go

@@ -206,6 +206,7 @@ func TestIptablesManagerIPSet(t *testing.T) {
 }
 
 func checkRuleSpecs(t *testing.T, ipv4Client *iptables.IPTables, chainName string, mustExists bool, rulespec ...string) {
+    t.Helper()
 	exists, err := ipv4Client.Exists("filter", chainName, rulespec...)
 	require.NoError(t, err, "failed to check rule")
 	require.Falsef(t, !exists && mustExists, "rule '%v' does not exist", rulespec)

client/internal/dns/server_test.go

@@ -771,6 +771,7 @@ func TestDNSPermanent_matchOnly(t *testing.T) {
 }
 
 func createWgInterfaceWithBind(t *testing.T) (*iface.WGIface, error) {
+	t.Helper()
 	ov := os.Getenv("NB_WG_KERNEL_DISABLED")
 	defer os.Setenv("NB_WG_KERNEL_DISABLED", ov)
 

client/internal/engine.go

@@ -640,7 +640,7 @@ func (e *Engine) updateNetworkMap(networkMap *mgmProto.NetworkMap) error {
 				if config.GetSshConfig() != nil && config.GetSshConfig().GetSshPubKey() != nil {
 					err := e.sshServer.AddAuthorizedKey(config.WgPubKey, string(config.GetSshConfig().GetSshPubKey()))
 					if err != nil {
-						log.Warnf("failed adding authroized key to SSH DefaultServer %v", err)
+						log.Warnf("failed adding authorized key to SSH DefaultServer %v", err)
 					}
 				}
 			}

client/internal/peer/status.go

@@ -174,13 +174,13 @@ func (d *Status) UpdatePeerState(receivedState State) error {
 	return nil
 }
 
-func shouldSkipNotify(new, curr State) bool {
+func shouldSkipNotify(received, curr State) bool {
 	switch {
-	case new.ConnStatus == StatusConnecting:
+	case received.ConnStatus == StatusConnecting:
 		return true
-	case new.ConnStatus == StatusDisconnected && curr.ConnStatus == StatusConnecting:
+	case received.ConnStatus == StatusDisconnected && curr.ConnStatus == StatusConnecting:
 		return true
-	case new.ConnStatus == StatusDisconnected && curr.ConnStatus == StatusDisconnected:
+	case received.ConnStatus == StatusDisconnected && curr.ConnStatus == StatusDisconnected:
 		return curr.IP != ""
 	default:
 		return false

client/internal/routemanager/systemops_nonandroid_test.go

@@ -123,7 +123,7 @@ func TestGetExistingRIBRouteGateway(t *testing.T) {
 
 func TestAddExistAndRemoveRouteNonAndroid(t *testing.T) {
 	defaultGateway, err := getExistingRIBRouteGateway(netip.MustParsePrefix("0.0.0.0/0"))
-	fmt.Println("defaultGateway: ", defaultGateway)
+	t.Log("defaultGateway: ", defaultGateway)
 	if err != nil {
 		t.Fatal("shouldn't return error when fetching the gateway: ", err)
 	}
@@ -207,7 +207,7 @@ func TestAddExistAndRemoveRouteNonAndroid(t *testing.T) {
 			// route should either not have been added or should have been removed
 			// In case of already existing route, it should not have been added (but still exist)
 			ok, err := existsInRouteTable(testCase.prefix)
-			fmt.Println("Buffer string: ", buf.String())
+			t.Log("Buffer string: ", buf.String())
 			require.NoError(t, err, "should not return err")
 			if !strings.Contains(buf.String(), "because it already exists") {
 				require.False(t, ok, "route should not exist")

client/system/info_darwin.go

@@ -3,7 +3,6 @@ package system
 import (
 	"bytes"
 	"context"
-	"fmt"
 	"os"
 	"os/exec"
 	"runtime"
@@ -21,7 +20,7 @@ func GetInfo(ctx context.Context) *Info {
 	utsname := unix.Utsname{}
 	err := unix.Uname(&utsname)
 	if err != nil {
-		fmt.Println("getInfo:", err)
+		log.Warnf("getInfo: %s", err)
 	}
 	sysName := string(bytes.Split(utsname.Sysname[:], []byte{0})[0])
 	machine := string(bytes.Split(utsname.Machine[:], []byte{0})[0])

client/system/info_linux.go

@@ -6,13 +6,14 @@ package system
 import (
 	"bytes"
 	"context"
-	"fmt"
 	"os"
 	"os/exec"
 	"runtime"
 	"strings"
 	"time"
 
+	log "github.com/sirupsen/logrus"
+
 	"github.com/netbirdio/netbird/version"
 )
 
@@ -67,7 +68,7 @@ func _getInfo() string {
 	cmd.Stderr = &stderr
 	err := cmd.Run()
 	if err != nil {
-		fmt.Println("getInfo:", err)
+		log.Warnf("getInfo: %s", err)
 	}
 	return out.String()
 }
@@ -81,7 +82,7 @@ func _getReleaseInfo() string {
 	cmd.Stderr = &stderr
 	err := cmd.Run()
 	if err != nil {
-		fmt.Println("getReleaseInfo:", err)
+		log.Warnf("geucwReleaseInfo: %s", err)
 	}
 	return out.String()
 }

client/ui/client_ui.go

@@ -69,7 +69,7 @@ func main() {
 		a.Run()
 	} else {
 		if err := checkPIDFile(); err != nil {
-			fmt.Println(err)
+			log.Errorf("check PID file: %v", err)
 			return
 		}
 		systray.Run(client.onTrayReady, client.onTrayExit)

iface/module_linux_test.go

@@ -132,6 +132,7 @@ func resetGlobals() {
 }
 
 func createFiles(t *testing.T) (string, []module) {
+    t.Helper()
 	writeFile := func(path, text string) {
 		if err := os.WriteFile(path, []byte(text), 0644); err != nil {
 			t.Fatal(err)
@@ -167,6 +168,7 @@ func createFiles(t *testing.T) (string, []module) {
 }
 
 func getRandomLoadedModule(t *testing.T) (string, error) {
+    t.Helper()
 	f, err := os.Open("/proc/modules")
 	if err != nil {
 		return "", err

management/client/client_test.go

@@ -31,6 +31,7 @@ import (
 const ValidKey = "A2C8E62B-38F5-4553-B31E-DD66C696CEBB"
 
 func startManagement(t *testing.T) (*grpc.Server, net.Listener) {
+	t.Helper()
 	level, _ := log.ParseLevel("debug")
 	log.SetLevel(level)
 
@@ -81,6 +82,7 @@ func startManagement(t *testing.T) (*grpc.Server, net.Listener) {
 }
 
 func startMockManagement(t *testing.T) (*grpc.Server, net.Listener, *mock_server.ManagementServiceServerMock, wgtypes.Key) {
+	t.Helper()
 	lis, err := net.Listen("tcp", ":0")
 	if err != nil {
 		t.Fatal(err)
@@ -168,7 +170,7 @@ func TestClient_LoginUnregistered_ShouldThrow_401(t *testing.T) {
 		t.Error("expecting err on unregistered login, got nil")
 	}
 	if s, ok := status.FromError(err); !ok || s.Code() != codes.PermissionDenied {
-		t.Errorf("expecting err code %d denied on on unregistered login got %d", codes.PermissionDenied, s.Code())
+		t.Errorf("expecting err code %d denied on unregistered login got %d", codes.PermissionDenied, s.Code())
 	}
 }
 

management/cmd/root.go

@@ -67,7 +67,7 @@ func init() {
 	rootCmd.MarkFlagRequired("config") //nolint
 
 	rootCmd.PersistentFlags().StringVar(&logLevel, "log-level", "info", "")
-	rootCmd.PersistentFlags().StringVar(&logFile, "log-file", defaultLogFile, "sets Netbird log path. If console is specified the the log will be output to stdout")
+	rootCmd.PersistentFlags().StringVar(&logFile, "log-file", defaultLogFile, "sets Netbird log path. If console is specified the log will be output to stdout")
 	rootCmd.AddCommand(mgmtCmd)
 
 	migrationCmd.PersistentFlags().StringVar(&mgmtDataDir, "datadir", defaultMgmtDataDir, "server data directory location")

management/server/account.go

@@ -1574,7 +1574,7 @@ func (am *DefaultAccountManager) GetAllConnectedPeers() (map[string]struct{}, er
 
 func isDomainValid(domain string) bool {
 	re := regexp.MustCompile(`^([a-z0-9]+(-[a-z0-9]+)*\.)+[a-z]{2,}$`)
-	return re.Match([]byte(domain))
+	return re.MatchString(domain)
 }
 
 // GetDNSDomain returns the configured dnsDomain

management/server/account_test.go

@@ -25,6 +25,7 @@ import (
 )
 
 func verifyCanAddPeerToAccount(t *testing.T, manager AccountManager, account *Account, userID string) {
+	t.Helper()
 	peer := &Peer{
 		Key:  "BhRPtynAAYRDy08+q4HTMsos8fs4plTP4NOSh7C1ry8=",
 		Name: "test-host@netbird.io",
@@ -52,6 +53,7 @@ func verifyCanAddPeerToAccount(t *testing.T, manager AccountManager, account *Ac
 }
 
 func verifyNewAccountHasDefaultFields(t *testing.T, account *Account, createdBy string, domain string, expectedUsers []string) {
+	t.Helper()
 	if len(account.Peers) != 0 {
 		t.Errorf("expected account to have len(Peers) = %v, got %v", 0, len(account.Peers))
 	}
@@ -1131,6 +1133,7 @@ func TestAccountManager_DeletePeer(t *testing.T) {
 }
 
 func getEvent(t *testing.T, accountID string, manager AccountManager, eventType activity.Activity) *activity.Event {
+	t.Helper()
 	for {
 		select {
 		case <-time.After(time.Second):
@@ -2038,6 +2041,7 @@ func TestAccount_UserGroupsRemoveFromPeers(t *testing.T) {
 }
 
 func createManager(t *testing.T) (*DefaultAccountManager, error) {
+	t.Helper()
 	store, err := createStore(t)
 	if err != nil {
 		return nil, err
@@ -2047,6 +2051,7 @@ func createManager(t *testing.T) (*DefaultAccountManager, error) {
 }
 
 func createStore(t *testing.T) (Store, error) {
+	t.Helper()
 	dataDir := t.TempDir()
 	store, err := NewStoreFromJson(dataDir, nil)
 	if err != nil {

management/server/dns_test.go

@@ -186,6 +186,7 @@ func TestGetNetworkMap_DNSConfigSync(t *testing.T) {
 }
 
 func createDNSManager(t *testing.T) (*DefaultAccountManager, error) {
+	t.Helper()
 	store, err := createDNSStore(t)
 	if err != nil {
 		return nil, err
@@ -195,6 +196,7 @@ func createDNSManager(t *testing.T) (*DefaultAccountManager, error) {
 }
 
 func createDNSStore(t *testing.T) (Store, error) {
+	t.Helper()
 	dataDir := t.TempDir()
 	store, err := NewStoreFromJson(dataDir, nil)
 	if err != nil {
@@ -205,6 +207,7 @@ func createDNSStore(t *testing.T) (Store, error) {
 }
 
 func initTestDNSAccount(t *testing.T, am *DefaultAccountManager) (*Account, error) {
+	t.Helper()
 	peer1 := &Peer{
 		Key:  dnsPeer1Key,
 		Name: "test-host1@netbird.io",

management/server/event_test.go

@@ -11,6 +11,7 @@ import (
 
 func generateAndStoreEvents(t *testing.T, manager *DefaultAccountManager, typ activity.Activity, initiatorID, targetID,
 	accountID string, count int) {
+	t.Helper()
 	for i := 0; i < count; i++ {
 		_, err := manager.eventStore.Save(&activity.Event{
 			Timestamp:   time.Now().UTC(),

management/server/file_store_test.go

@@ -580,6 +580,7 @@ func TestFileStore_SavePeerStatus(t *testing.T) {
 }
 
 func newStore(t *testing.T) *FileStore {
+	t.Helper()
 	store, err := NewFileStore(t.TempDir(), nil)
 	if err != nil {
 		t.Errorf("failed creating a new store")

management/server/http/groups_handler_test.go

@@ -230,7 +230,7 @@ func TestWriteGroup(t *testing.T) {
 			expectedBody:   false,
 		},
 		{
-			name:        "Write Group PUT not not change Issue",
+			name:        "Write Group PUT not change Issue",
 			requestType: http.MethodPut,
 			requestPath: "/api/groups/id-jwt-group",
 			requestBody: bytes.NewBuffer(

management/server/http/peers_handler_test.go

@@ -3,7 +3,6 @@ package http
 import (
 	"bytes"
 	"encoding/json"
-	"fmt"
 	"io"
 	"net"
 	"net/http"
@@ -232,7 +231,7 @@ func TestGetPeers(t *testing.T) {
 				}
 			}
 
-			fmt.Println(got)
+			t.Log(got)
 
 			assert.Equal(t, got.Name, tc.expectedPeer.Name)
 			assert.Equal(t, got.Version, tc.expectedPeer.Meta.WtVersion)

management/server/http/setupkeys_handler_test.go

@@ -220,6 +220,7 @@ func TestSetupKeysHandlers(t *testing.T) {
 }
 
 func assertKeys(t *testing.T, got *api.SetupKey, expected *api.SetupKey) {
+	t.Helper()
 	// this comparison is done manually because when converting to JSON dates formatted differently
 	// assert.Equal(t, got.UpdatedAt, tc.expectedSetupKey.UpdatedAt) //doesn't work
 	assert.WithinDurationf(t, got.UpdatedAt, expected.UpdatedAt, 0, "")