Add gocritic linter (#1324)

* Add gocritic linter

`gocritic` provides diagnostics that check for bugs, performance, and style issues

We disable the following checks:

- commentFormatting
- captLocal
- deprecatedComment

This PR contains many `//nolint:gocritic` to disable `appendAssign`.

client/cmd/status.go

@@ -234,7 +234,7 @@ func mapPeers(peers []*proto.PeerState) peersStateOutput {
 			continue
 		}
 		if isPeerConnected {
-			peersConnected = peersConnected + 1
+			peersConnected++
 
 			localICE = pbPeerState.GetLocalIceCandidateType()
 			remoteICE = pbPeerState.GetRemoteIceCandidateType()
@@ -407,7 +407,7 @@ func parsePeers(peers peersStateOutput) string {
 			peerState.LastStatusUpdate.Format("2006-01-02 15:04:05"),
 		)
 
-		peersString = peersString + peerString
+		peersString += peerString
 	}
 	return peersString
 }

client/firewall/iptables/manager_linux.go

@@ -463,14 +463,16 @@ func (m *Manager) actionToStr(action fw.Action) string {
 }
 
 func (m *Manager) transformIPsetName(ipsetName string, sPort, dPort string) string {
-	if ipsetName == "" {
+	switch {
+	case ipsetName == "":
 		return ""
-	} else if sPort != "" && dPort != "" {
+	case sPort != "" && dPort != "":
 		return ipsetName + "-sport-dport"
-	} else if sPort != "" {
+	case sPort != "":
 		return ipsetName + "-sport"
-	} else if dPort != "" {
+	case dPort != "":
 		return ipsetName + "-dport"
+	default:
+		return ipsetName
 	}
-	return ipsetName
 }

client/firewall/nftables/manager_linux.go

@@ -791,7 +791,7 @@ func (m *Manager) flushWithBackoff() (err error) {
 				return err
 			}
 			time.Sleep(backoffTime)
-			backoffTime = backoffTime * 2
+			backoffTime *= 2
 			continue
 		}
 		break

client/internal/acl/manager_test.go

@@ -189,31 +189,33 @@ func TestDefaultManagerSquashRules(t *testing.T) {
 	}
 
 	r := rules[0]
-	if r.PeerIP != "0.0.0.0" {
+	switch {
+	case r.PeerIP != "0.0.0.0":
 		t.Errorf("IP should be 0.0.0.0, got: %v", r.PeerIP)
 		return
-	} else if r.Direction != mgmProto.FirewallRule_IN {
+	case r.Direction != mgmProto.FirewallRule_IN:
 		t.Errorf("direction should be IN, got: %v", r.Direction)
 		return
-	} else if r.Protocol != mgmProto.FirewallRule_ALL {
+	case r.Protocol != mgmProto.FirewallRule_ALL:
 		t.Errorf("protocol should be ALL, got: %v", r.Protocol)
 		return
-	} else if r.Action != mgmProto.FirewallRule_ACCEPT {
+	case r.Action != mgmProto.FirewallRule_ACCEPT:
 		t.Errorf("action should be ACCEPT, got: %v", r.Action)
 		return
 	}
 
 	r = rules[1]
-	if r.PeerIP != "0.0.0.0" {
+	switch {
+	case r.PeerIP != "0.0.0.0":
 		t.Errorf("IP should be 0.0.0.0, got: %v", r.PeerIP)
 		return
-	} else if r.Direction != mgmProto.FirewallRule_OUT {
+	case r.Direction != mgmProto.FirewallRule_OUT:
 		t.Errorf("direction should be OUT, got: %v", r.Direction)
 		return
-	} else if r.Protocol != mgmProto.FirewallRule_ALL {
+	case r.Protocol != mgmProto.FirewallRule_ALL:
 		t.Errorf("protocol should be ALL, got: %v", r.Protocol)
 		return
-	} else if r.Action != mgmProto.FirewallRule_ACCEPT {
+	case r.Action != mgmProto.FirewallRule_ACCEPT:
 		t.Errorf("action should be ACCEPT, got: %v", r.Action)
 		return
 	}

client/internal/auth/device_flow.go

@@ -4,12 +4,13 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"github.com/netbirdio/netbird/client/internal"
 	"io"
 	"net/http"
 	"net/url"
 	"strings"
 	"time"
+
+	"github.com/netbirdio/netbird/client/internal"
 )
 
 // HostedGrantType grant type for device flow on Hosted
@@ -174,7 +175,7 @@ func (d *DeviceAuthorizationFlow) WaitToken(ctx context.Context, info AuthFlowIn
 				if tokenResponse.Error == "authorization_pending" {
 					continue
 				} else if tokenResponse.Error == "slow_down" {
-					interval = interval + (3 * time.Second)
+					interval += (3 * time.Second)
 					ticker.Reset(interval)
 					continue
 				}

client/internal/auth/oauth.go

@@ -92,15 +92,15 @@ func authenticateWithPKCEFlow(ctx context.Context, config *internal.Config) (OAu
 func authenticateWithDeviceCodeFlow(ctx context.Context, config *internal.Config) (OAuthFlow, error) {
 	deviceFlowInfo, err := internal.GetDeviceAuthorizationFlowInfo(ctx, config.PrivateKey, config.ManagementURL)
 	if err != nil {
-		s, ok := gstatus.FromError(err)
-		if ok && s.Code() == codes.NotFound {
+		switch s, ok := gstatus.FromError(err); {
+		case ok && s.Code() == codes.NotFound:
 			return nil, fmt.Errorf("no SSO provider returned from management. " +
 				"Please proceed with setting up this device using setup keys " +
 				"https://docs.netbird.io/how-to/register-machines-using-setup-keys")
-		} else if ok && s.Code() == codes.Unimplemented {
+		case ok && s.Code() == codes.Unimplemented:
 			return nil, fmt.Errorf("the management server, %s, does not support SSO providers, "+
 				"please update your server or use Setup Keys to login", config.ManagementURL)
-		} else {
+		default:
 			return nil, fmt.Errorf("getting device authorization flow info failed with error: %v", err)
 		}
 	}

client/internal/config.go

@@ -273,9 +273,9 @@ func parseURL(serviceName, serviceURL string) (*url.URL, error) {
 	if parsedMgmtURL.Port() == "" {
 		switch parsedMgmtURL.Scheme {
 		case "https":
-			parsedMgmtURL.Host = parsedMgmtURL.Host + ":443"
+			parsedMgmtURL.Host += ":443"
 		case "http":
-			parsedMgmtURL.Host = parsedMgmtURL.Host + ":80"
+			parsedMgmtURL.Host += ":80"
 		default:
 			log.Infof("unable to determine a default port for schema %s in URL %s", parsedMgmtURL.Scheme, serviceURL)
 		}

client/internal/dns/network_manager_linux.go

@@ -122,7 +122,7 @@ func (n *networkManagerDbusConfigurator) applyDNSConfig(config hostDNSConfig) er
 		searchDomains = append(searchDomains, dns.Fqdn(dConf.domain))
 	}
 
-	newDomainList := append(searchDomains, matchDomains...)
+	newDomainList := append(searchDomains, matchDomains...) //nolint:gocritic
 
 	priority := networkManagerDbusSearchDomainOnlyPriority
 	switch {

client/internal/dns/server.go

@@ -252,7 +252,7 @@ func (s *DefaultServer) applyConfiguration(update nbdns.Config) error {
 	if err != nil {
 		return fmt.Errorf("not applying dns update, error: %v", err)
 	}
-	muxUpdates := append(localMuxUpdates, upstreamMuxUpdates...)
+	muxUpdates := append(localMuxUpdates, upstreamMuxUpdates...) //nolint:gocritic
 
 	s.updateMux(muxUpdates)
 	s.updateLocalResolver(localRecords)

client/internal/ebpf/ebpf/manager_linux.go

@@ -50,7 +50,7 @@ func GetEbpfManagerInstance() manager.Manager {
 }
 
 func (tf *GeneralManager) setFeatureFlag(feature uint16) {
-	tf.featureFlags = tf.featureFlags | feature
+	tf.featureFlags |= feature
 }
 
 func (tf *GeneralManager) loadXdp() error {

client/internal/engine.go

@@ -204,14 +204,12 @@ func (e *Engine) Start() error {
 			e.dnsServer = dns.NewDefaultServerPermanentUpstream(e.ctx, e.wgInterface, e.mobileDep.HostDNSAddresses, *dnsConfig, e.mobileDep.NetworkChangeListener)
 			go e.mobileDep.DnsReadyListener.OnReady()
 		}
-	} else {
+	} else if e.dnsServer == nil {
 		// todo fix custom address
-		if e.dnsServer == nil {
-			e.dnsServer, err = dns.NewDefaultServer(e.ctx, e.wgInterface, e.config.CustomDNSAddress)
-			if err != nil {
-				e.close()
-				return err
-			}
+		e.dnsServer, err = dns.NewDefaultServer(e.ctx, e.wgInterface, e.config.CustomDNSAddress)
+		if err != nil {
+			e.close()
+			return err
 		}
 	}
 
@@ -490,15 +488,13 @@ func (e *Engine) updateSSH(sshConf *mgmProto.SSHConfig) error {
 		} else {
 			log.Debugf("SSH server is already running")
 		}
-	} else {
+	} else if !isNil(e.sshServer) {
 		// Disable SSH server request, so stop it if it was running
-		if !isNil(e.sshServer) {
-			err := e.sshServer.Stop()
-			if err != nil {
-				log.Warnf("failed to stop SSH server %v", err)
-			}
-			e.sshServer = nil
+		err := e.sshServer.Stop()
+		if err != nil {
+			log.Warnf("failed to stop SSH server %v", err)
 		}
+		e.sshServer = nil
 	}
 	return nil
 }

client/internal/engine_test.go

@@ -869,7 +869,7 @@ loop:
 		case <-ticker.C:
 			totalConnected := 0
 			for _, engine := range engines {
-				totalConnected = totalConnected + getConnectedPeers(engine)
+				totalConnected += getConnectedPeers(engine)
 			}
 			if totalConnected == expectedConnected {
 				log.Infof("total connected=%d", totalConnected)

client/internal/routemanager/iptables_linux.go

@@ -173,7 +173,7 @@ func (i *iptablesManager) addJumpRules() error {
 		return err
 	}
 	if i.ipv4Client != nil {
-		rule := append(iptablesDefaultForwardingRule, ipv4Forwarding)
+		rule := append(iptablesDefaultForwardingRule, ipv4Forwarding) //nolint:gocritic
 
 		err = i.ipv4Client.Insert(iptablesFilterTable, iptablesForwardChain, 1, rule...)
 		if err != nil {
@@ -181,7 +181,7 @@ func (i *iptablesManager) addJumpRules() error {
 		}
 		i.rules[ipv4][ipv4Forwarding] = rule
 
-		rule = append(iptablesDefaultNatRule, ipv4Nat)
+		rule = append(iptablesDefaultNatRule, ipv4Nat) //nolint:gocritic
 		err = i.ipv4Client.Insert(iptablesNatTable, iptablesPostRoutingChain, 1, rule...)
 		if err != nil {
 			return err
@@ -190,14 +190,14 @@ func (i *iptablesManager) addJumpRules() error {
 	}
 
 	if i.ipv6Client != nil {
-		rule := append(iptablesDefaultForwardingRule, ipv6Forwarding)
+		rule := append(iptablesDefaultForwardingRule, ipv6Forwarding) //nolint:gocritic
 		err = i.ipv6Client.Insert(iptablesFilterTable, iptablesForwardChain, 1, rule...)
 		if err != nil {
 			return err
 		}
 		i.rules[ipv6][ipv6Forwarding] = rule
 
-		rule = append(iptablesDefaultNatRule, ipv6Nat)
+		rule = append(iptablesDefaultNatRule, ipv6Nat) //nolint:gocritic
 		err = i.ipv6Client.Insert(iptablesNatTable, iptablesPostRoutingChain, 1, rule...)
 		if err != nil {
 			return err

client/internal/routemanager/nftables_linux.go

@@ -300,7 +300,7 @@ func (n *nftablesManager) acceptForwardRule(sourceNetwork string) error {
 	dst := generateCIDRMatcherExpressions("destination", "0.0.0.0/0")
 
 	var exprs []expr.Any
-	exprs = append(src, append(dst, &expr.Verdict{
+	exprs = append(src, append(dst, &expr.Verdict{ //nolint:gocritic
 		Kind: expr.VerdictAccept,
 	})...)
 
@@ -322,7 +322,7 @@ func (n *nftablesManager) acceptForwardRule(sourceNetwork string) error {
 	src = generateCIDRMatcherExpressions("source", "0.0.0.0/0")
 	dst = generateCIDRMatcherExpressions("destination", sourceNetwork)
 
-	exprs = append(src, append(dst, &expr.Verdict{
+	exprs = append(src, append(dst, &expr.Verdict{ //nolint:gocritic
 		Kind: expr.VerdictAccept,
 	})...)
 
@@ -421,9 +421,9 @@ func (n *nftablesManager) insertRoutingRule(format, chain string, pair routerPai
 
 	var expression []expr.Any
 	if isNat {
-		expression = append(sourceExp, append(destExp, &expr.Counter{}, &expr.Masq{})...)
+		expression = append(sourceExp, append(destExp, &expr.Counter{}, &expr.Masq{})...) //nolint:gocritic
 	} else {
-		expression = append(sourceExp, append(destExp, exprCounterAccept...)...)
+		expression = append(sourceExp, append(destExp, exprCounterAccept...)...) //nolint:gocritic
 	}
 
 	ruleKey := genKey(format, pair.ID)

client/internal/routemanager/nftables_linux_test.go

@@ -44,7 +44,7 @@ func TestNftablesManager_RestoreOrCreateContainers(t *testing.T) {
 	sourceExp := generateCIDRMatcherExpressions("source", pair.source)
 	destExp := generateCIDRMatcherExpressions("destination", pair.destination)
 
-	forward4Exp := append(sourceExp, append(destExp, exprCounterAccept...)...)
+	forward4Exp := append(sourceExp, append(destExp, exprCounterAccept...)...) //nolint:gocritic
 	forward4RuleKey := genKey(forwardingFormat, pair.ID)
 	inserted4Forwarding := nftablesTestingClient.InsertRule(&nftables.Rule{
 		Table:    manager.tableIPv4,
@@ -53,7 +53,7 @@ func TestNftablesManager_RestoreOrCreateContainers(t *testing.T) {
 		UserData: []byte(forward4RuleKey),
 	})
 
-	nat4Exp := append(sourceExp, append(destExp, &expr.Counter{}, &expr.Masq{})...)
+	nat4Exp := append(sourceExp, append(destExp, &expr.Counter{}, &expr.Masq{})...) //nolint:gocritic
 	nat4RuleKey := genKey(natFormat, pair.ID)
 
 	inserted4Nat := nftablesTestingClient.InsertRule(&nftables.Rule{
@@ -76,7 +76,7 @@ func TestNftablesManager_RestoreOrCreateContainers(t *testing.T) {
 	sourceExp = generateCIDRMatcherExpressions("source", pair.source)
 	destExp = generateCIDRMatcherExpressions("destination", pair.destination)
 
-	forward6Exp := append(sourceExp, append(destExp, exprCounterAccept...)...)
+	forward6Exp := append(sourceExp, append(destExp, exprCounterAccept...)...) //nolint:gocritic
 	forward6RuleKey := genKey(forwardingFormat, pair.ID)
 	inserted6Forwarding := nftablesTestingClient.InsertRule(&nftables.Rule{
 		Table:    manager.tableIPv6,
@@ -85,7 +85,7 @@ func TestNftablesManager_RestoreOrCreateContainers(t *testing.T) {
 		UserData: []byte(forward6RuleKey),
 	})
 
-	nat6Exp := append(sourceExp, append(destExp, &expr.Counter{}, &expr.Masq{})...)
+	nat6Exp := append(sourceExp, append(destExp, &expr.Counter{}, &expr.Masq{})...) //nolint:gocritic
 	nat6RuleKey := genKey(natFormat, pair.ID)
 
 	inserted6Nat := nftablesTestingClient.InsertRule(&nftables.Rule{
@@ -149,7 +149,7 @@ func TestNftablesManager_InsertRoutingRules(t *testing.T) {
 
 			sourceExp := generateCIDRMatcherExpressions("source", testCase.inputPair.source)
 			destExp := generateCIDRMatcherExpressions("destination", testCase.inputPair.destination)
-			testingExpression := append(sourceExp, destExp...)
+			testingExpression := append(sourceExp, destExp...) //nolint:gocritic
 			fwdRuleKey := genKey(forwardingFormat, testCase.inputPair.ID)
 
 			found := 0
@@ -188,7 +188,7 @@ func TestNftablesManager_InsertRoutingRules(t *testing.T) {
 
 			sourceExp = generateCIDRMatcherExpressions("source", getInPair(testCase.inputPair).source)
 			destExp = generateCIDRMatcherExpressions("destination", getInPair(testCase.inputPair).destination)
-			testingExpression = append(sourceExp, destExp...)
+			testingExpression = append(sourceExp, destExp...) //nolint:gocritic
 			inFwdRuleKey := genKey(inForwardingFormat, testCase.inputPair.ID)
 
 			found = 0
@@ -252,7 +252,7 @@ func TestNftablesManager_RemoveRoutingRules(t *testing.T) {
 			sourceExp := generateCIDRMatcherExpressions("source", testCase.inputPair.source)
 			destExp := generateCIDRMatcherExpressions("destination", testCase.inputPair.destination)
 
-			forwardExp := append(sourceExp, append(destExp, exprCounterAccept...)...)
+			forwardExp := append(sourceExp, append(destExp, exprCounterAccept...)...) //nolint:gocritic
 			forwardRuleKey := genKey(forwardingFormat, testCase.inputPair.ID)
 			insertedForwarding := nftablesTestingClient.InsertRule(&nftables.Rule{
 				Table:    table,
@@ -261,7 +261,7 @@ func TestNftablesManager_RemoveRoutingRules(t *testing.T) {
 				UserData: []byte(forwardRuleKey),
 			})
 
-			natExp := append(sourceExp, append(destExp, &expr.Counter{}, &expr.Masq{})...)
+			natExp := append(sourceExp, append(destExp, &expr.Counter{}, &expr.Masq{})...) //nolint:gocritic
 			natRuleKey := genKey(natFormat, testCase.inputPair.ID)
 
 			insertedNat := nftablesTestingClient.InsertRule(&nftables.Rule{
@@ -274,7 +274,7 @@ func TestNftablesManager_RemoveRoutingRules(t *testing.T) {
 			sourceExp = generateCIDRMatcherExpressions("source", getInPair(testCase.inputPair).source)
 			destExp = generateCIDRMatcherExpressions("destination", getInPair(testCase.inputPair).destination)
 
-			forwardExp = append(sourceExp, append(destExp, exprCounterAccept...)...)
+			forwardExp = append(sourceExp, append(destExp, exprCounterAccept...)...) //nolint:gocritic
 			inForwardRuleKey := genKey(inForwardingFormat, testCase.inputPair.ID)
 			insertedInForwarding := nftablesTestingClient.InsertRule(&nftables.Rule{
 				Table:    table,
@@ -283,7 +283,7 @@ func TestNftablesManager_RemoveRoutingRules(t *testing.T) {
 				UserData: []byte(inForwardRuleKey),
 			})
 
-			natExp = append(sourceExp, append(destExp, &expr.Counter{}, &expr.Masq{})...)
+			natExp = append(sourceExp, append(destExp, &expr.Counter{}, &expr.Masq{})...) //nolint:gocritic
 			inNatRuleKey := genKey(inNatFormat, testCase.inputPair.ID)
 
 			insertedInNat := nftablesTestingClient.InsertRule(&nftables.Rule{

client/ssh/client.go

@@ -2,11 +2,12 @@ package ssh
 
 import (
 	"fmt"
-	"golang.org/x/crypto/ssh"
-	"golang.org/x/term"
 	"net"
 	"os"
 	"time"
+
+	"golang.org/x/crypto/ssh"
+	"golang.org/x/term"
 )
 
 // Client wraps crypto/ssh Client to simplify usage
@@ -73,8 +74,7 @@ func (c *Client) OpenTerminal() error {
 
 	if err := session.Wait(); err != nil {
 		if e, ok := err.(*ssh.ExitError); ok {
-			switch e.ExitStatus() {
-			case 130:
+			if e.ExitStatus() == 130 {
 				return nil
 			}
 		}

client/system/info_linux.go

@@ -44,8 +44,8 @@ func GetInfo(ctx context.Context) *Info {
 		}
 	}
 
-	osStr := strings.Replace(info, "\n", "", -1)
-	osStr = strings.Replace(osStr, "\r\n", "", -1)
+	osStr := strings.ReplaceAll(info, "\n", "")
+	osStr = strings.ReplaceAll(osStr, "\r\n", "")
 	osInfo := strings.Split(osStr, " ")
 	if osName == "" {
 		osName = osInfo[3]