From d1a323fa9dc36e34f0af5703ff162a71c197f6e8 Mon Sep 17 00:00:00 2001 From: Yury Gargay Date: Mon, 27 Nov 2023 16:40:02 +0100 Subject: [PATCH] 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`. --- .golangci.yaml | 7 ++++++ client/cmd/status.go | 4 ++-- client/firewall/iptables/manager_linux.go | 12 ++++++---- client/firewall/nftables/manager_linux.go | 2 +- client/internal/acl/manager_test.go | 18 +++++++------- client/internal/auth/device_flow.go | 5 ++-- client/internal/auth/oauth.go | 8 +++---- client/internal/config.go | 4 ++-- client/internal/dns/network_manager_linux.go | 2 +- client/internal/dns/server.go | 2 +- client/internal/ebpf/ebpf/manager_linux.go | 2 +- client/internal/engine.go | 24 ++++++++----------- client/internal/engine_test.go | 2 +- .../internal/routemanager/iptables_linux.go | 8 +++---- .../internal/routemanager/nftables_linux.go | 8 +++---- .../routemanager/nftables_linux_test.go | 20 ++++++++-------- client/ssh/client.go | 8 +++---- client/system/info_linux.go | 4 ++-- iface/wg_configurer_nonandroid.go | 2 +- management/client/client_test.go | 14 +++++------ management/server/account.go | 7 +++--- management/server/http/policies_handler.go | 2 +- management/server/http/setupkeys_handler.go | 9 +++---- management/server/idp/zitadel.go | 8 +++---- management/server/metrics/selfhosted.go | 10 ++++---- management/server/network.go | 2 +- management/server/policy.go | 2 +- management/server/setupkey.go | 2 +- sharedsock/sock_linux.go | 2 +- signal/client/grpc.go | 9 +++---- util/retry.go | 2 +- 31 files changed, 110 insertions(+), 101 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index d0926fffc..d847e63c3 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -12,6 +12,12 @@ linters-settings: # Default: false check-type-assertions: false + gocritic: + disabled-checks: + - commentFormatting + - captLocal + - deprecatedComment + govet: # Enable all analyzers. # Default: false @@ -42,6 +48,7 @@ linters: - dupword # dupword checks for duplicate words in the source code - durationcheck # durationcheck checks for two durations multiplied together - forbidigo # forbidigo forbids identifiers + - gocritic # provides diagnostics that check for bugs, performance and style issues - 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 diff --git a/client/cmd/status.go b/client/cmd/status.go index 9dfd042f8..74d2061ff 100644 --- a/client/cmd/status.go +++ b/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 } diff --git a/client/firewall/iptables/manager_linux.go b/client/firewall/iptables/manager_linux.go index 4ce904df6..b9243f4ca 100644 --- a/client/firewall/iptables/manager_linux.go +++ b/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 } diff --git a/client/firewall/nftables/manager_linux.go b/client/firewall/nftables/manager_linux.go index 6c46048b4..93379bad8 100644 --- a/client/firewall/nftables/manager_linux.go +++ b/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 diff --git a/client/internal/acl/manager_test.go b/client/internal/acl/manager_test.go index 25de2a57f..d55a1cad6 100644 --- a/client/internal/acl/manager_test.go +++ b/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 } diff --git a/client/internal/auth/device_flow.go b/client/internal/auth/device_flow.go index c28e42772..3c51fe4f5 100644 --- a/client/internal/auth/device_flow.go +++ b/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 } diff --git a/client/internal/auth/oauth.go b/client/internal/auth/oauth.go index 82adf91b9..23bde2be2 100644 --- a/client/internal/auth/oauth.go +++ b/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) } } diff --git a/client/internal/config.go b/client/internal/config.go index cd665016b..646848a2f 100644 --- a/client/internal/config.go +++ b/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) } diff --git a/client/internal/dns/network_manager_linux.go b/client/internal/dns/network_manager_linux.go index 8bc3ef297..d5c2f60b2 100644 --- a/client/internal/dns/network_manager_linux.go +++ b/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 { diff --git a/client/internal/dns/server.go b/client/internal/dns/server.go index 6655a6e4e..9bb9a76a9 100644 --- a/client/internal/dns/server.go +++ b/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) diff --git a/client/internal/ebpf/ebpf/manager_linux.go b/client/internal/ebpf/ebpf/manager_linux.go index 9dfdc0ad1..7520a6387 100644 --- a/client/internal/ebpf/ebpf/manager_linux.go +++ b/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 { diff --git a/client/internal/engine.go b/client/internal/engine.go index 35fd822f2..4d461b746 100644 --- a/client/internal/engine.go +++ b/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 } diff --git a/client/internal/engine_test.go b/client/internal/engine_test.go index a855cf051..08cd29da8 100644 --- a/client/internal/engine_test.go +++ b/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) diff --git a/client/internal/routemanager/iptables_linux.go b/client/internal/routemanager/iptables_linux.go index 9f6019305..e9fbb7d3c 100644 --- a/client/internal/routemanager/iptables_linux.go +++ b/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 diff --git a/client/internal/routemanager/nftables_linux.go b/client/internal/routemanager/nftables_linux.go index e62b1a404..3ecfa9630 100644 --- a/client/internal/routemanager/nftables_linux.go +++ b/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) diff --git a/client/internal/routemanager/nftables_linux_test.go b/client/internal/routemanager/nftables_linux_test.go index dec800156..d60d53e50 100644 --- a/client/internal/routemanager/nftables_linux_test.go +++ b/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{ diff --git a/client/ssh/client.go b/client/ssh/client.go index 29ebb2481..2dc70e8fc 100644 --- a/client/ssh/client.go +++ b/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 } } diff --git a/client/system/info_linux.go b/client/system/info_linux.go index a4ab9f931..21a4d482a 100644 --- a/client/system/info_linux.go +++ b/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] diff --git a/iface/wg_configurer_nonandroid.go b/iface/wg_configurer_nonandroid.go index 6749c0966..3d9aff7a9 100644 --- a/iface/wg_configurer_nonandroid.go +++ b/iface/wg_configurer_nonandroid.go @@ -141,7 +141,7 @@ func (c *wGConfigurer) removeAllowedIP(peerKey string, allowedIP string) error { for i, existingAllowedIP := range existingPeer.AllowedIPs { if existingAllowedIP.String() == ipNet.String() { - newAllowedIPs = append(existingPeer.AllowedIPs[:i], existingPeer.AllowedIPs[i+1:]...) + newAllowedIPs = append(existingPeer.AllowedIPs[:i], existingPeer.AllowedIPs[i+1:]...) //nolint:gocritic break } } diff --git a/management/client/client_test.go b/management/client/client_test.go index c5e5b8140..9ebb58420 100644 --- a/management/client/client_test.go +++ b/management/client/client_test.go @@ -285,7 +285,7 @@ func Test_SystemMetaDataFromClient(t *testing.T) { testKey, err := wgtypes.GenerateKey() if err != nil { - log.Fatal(err) + t.Fatal(err) } serverAddr := lis.Addr().String() @@ -293,12 +293,12 @@ func Test_SystemMetaDataFromClient(t *testing.T) { testClient, err := NewClient(ctx, serverAddr, testKey, false) if err != nil { - log.Fatalf("error while creating testClient: %v", err) + t.Fatalf("error while creating testClient: %v", err) } key, err := testClient.GetServerPublicKey() if err != nil { - log.Fatalf("error while getting server public key from testclient, %v", err) + t.Fatalf("error while getting server public key from testclient, %v", err) } var actualMeta *mgmtProto.PeerSystemMeta @@ -364,7 +364,7 @@ func Test_GetDeviceAuthorizationFlow(t *testing.T) { testKey, err := wgtypes.GenerateKey() if err != nil { - log.Fatal(err) + t.Fatal(err) } serverAddr := lis.Addr().String() @@ -372,7 +372,7 @@ func Test_GetDeviceAuthorizationFlow(t *testing.T) { client, err := NewClient(ctx, serverAddr, testKey, false) if err != nil { - log.Fatalf("error while creating testClient: %v", err) + t.Fatalf("error while creating testClient: %v", err) } expectedFlowInfo := &mgmtProto.DeviceAuthorizationFlow{ @@ -408,7 +408,7 @@ func Test_GetPKCEAuthorizationFlow(t *testing.T) { testKey, err := wgtypes.GenerateKey() if err != nil { - log.Fatal(err) + t.Fatal(err) } serverAddr := lis.Addr().String() @@ -416,7 +416,7 @@ func Test_GetPKCEAuthorizationFlow(t *testing.T) { client, err := NewClient(ctx, serverAddr, testKey, false) if err != nil { - log.Fatalf("error while creating testClient: %v", err) + t.Fatalf("error while creating testClient: %v", err) } expectedFlowInfo := &mgmtProto.PKCEAuthorizationFlow{ diff --git a/management/server/account.go b/management/server/account.go index fb5198190..09071d103 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -950,14 +950,15 @@ func (am *DefaultAccountManager) newAccount(userID, domain string) (*Account, er _, err := am.Store.GetAccount(accountId) statusErr, _ := status.FromError(err) - if err == nil { + switch { + case err == nil: log.Warnf("an account with ID already exists, retrying...") continue - } else if statusErr.Type() == status.NotFound { + case statusErr.Type() == status.NotFound: newAccount := newAccountWithId(accountId, userID, domain) am.StoreEvent(userID, newAccount.Id, accountId, activity.AccountCreated, nil) return newAccount, nil - } else { + default: return nil, err } } diff --git a/management/server/http/policies_handler.go b/management/server/http/policies_handler.go index c8f58f8a4..f8c876a41 100644 --- a/management/server/http/policies_handler.go +++ b/management/server/http/policies_handler.go @@ -300,7 +300,7 @@ func toPolicyResponse(account *server.Account, policy *server.Policy) *api.Polic Action: api.PolicyRuleAction(r.Action), } if len(r.Ports) != 0 { - portsCopy := r.Ports[:] + portsCopy := r.Ports rule.Ports = &portsCopy } for _, gid := range r.Sources { diff --git a/management/server/http/setupkeys_handler.go b/management/server/http/setupkeys_handler.go index cddae672c..4adf3fdd0 100644 --- a/management/server/http/setupkeys_handler.go +++ b/management/server/http/setupkeys_handler.go @@ -192,13 +192,14 @@ func writeSuccess(w http.ResponseWriter, key *server.SetupKey) { func toResponseBody(key *server.SetupKey) *api.SetupKey { var state string - if key.IsExpired() { + switch { + case key.IsExpired(): state = "expired" - } else if key.IsRevoked() { + case key.IsRevoked(): state = "revoked" - } else if key.IsOverUsed() { + case key.IsOverUsed(): state = "overused" - } else { + default: state = "valid" } diff --git a/management/server/idp/zitadel.go b/management/server/idp/zitadel.go index 5325e51be..926f078b2 100644 --- a/management/server/idp/zitadel.go +++ b/management/server/idp/zitadel.go @@ -463,11 +463,9 @@ func (zp zitadelProfile) userData() *UserData { if zp.Human != nil { email = zp.Human.Email.Email name = zp.Human.Profile.DisplayName - } else { - if len(zp.LoginNames) > 0 { - email = zp.LoginNames[0] - name = zp.LoginNames[0] - } + } else if len(zp.LoginNames) > 0 { + email = zp.LoginNames[0] + name = zp.LoginNames[0] } return &UserData{ diff --git a/management/server/metrics/selfhosted.go b/management/server/metrics/selfhosted.go index e5ef6893e..90d69b47b 100644 --- a/management/server/metrics/selfhosted.go +++ b/management/server/metrics/selfhosted.go @@ -200,14 +200,14 @@ func (w *Worker) generateProperties() properties { expirationEnabled++ } - groups = groups + len(account.Groups) - routes = routes + len(account.Routes) + groups += len(account.Groups) + routes += len(account.Routes) for _, route := range account.Routes { if len(route.PeerGroups) > 0 { routesWithRGGroups++ } } - nameservers = nameservers + len(account.NameServerGroups) + nameservers += len(account.NameServerGroups) for _, policy := range account.Policies { for _, rule := range policy.Rules { @@ -231,10 +231,10 @@ func (w *Worker) generateProperties() properties { } for _, key := range account.SetupKeys { - setupKeysUsage = setupKeysUsage + key.UsedTimes + setupKeysUsage += key.UsedTimes if key.Ephemeral { ephemeralPeersSKs++ - ephemeralPeersSKUsage = ephemeralPeersSKUsage + key.UsedTimes + ephemeralPeersSKUsage += key.UsedTimes } } diff --git a/management/server/network.go b/management/server/network.go index c5b165cae..daa67f2dc 100644 --- a/management/server/network.go +++ b/management/server/network.go @@ -66,7 +66,7 @@ func NewNetwork() *Network { func (n *Network) IncSerial() { n.mu.Lock() defer n.mu.Unlock() - n.Serial = n.Serial + 1 + n.Serial++ } // CurrentSerial returns the Network.Serial of the network (latest state id) diff --git a/management/server/policy.go b/management/server/policy.go index 24188f93c..fc222cb40 100644 --- a/management/server/policy.go +++ b/management/server/policy.go @@ -406,7 +406,7 @@ func (am *DefaultAccountManager) ListPolicies(accountID, userID string) ([]*Poli return nil, status.Errorf(status.PermissionDenied, "Only Administrators can view policies") } - return account.Policies[:], nil + return account.Policies, nil } func (am *DefaultAccountManager) deletePolicy(account *Account, policyID string) (*Policy, error) { diff --git a/management/server/setupkey.go b/management/server/setupkey.go index 3bd14b61e..d347fb181 100644 --- a/management/server/setupkey.go +++ b/management/server/setupkey.go @@ -137,7 +137,7 @@ func (key *SetupKey) HiddenCopy(length int) *SetupKey { // IncrementUsage makes a copy of a key, increments the UsedTimes by 1 and sets LastUsed to now func (key *SetupKey) IncrementUsage() *SetupKey { c := key.Copy() - c.UsedTimes = c.UsedTimes + 1 + c.UsedTimes++ c.LastUsed = time.Now().UTC() return c } diff --git a/sharedsock/sock_linux.go b/sharedsock/sock_linux.go index b823bb508..656fdc8ca 100644 --- a/sharedsock/sock_linux.go +++ b/sharedsock/sock_linux.go @@ -248,7 +248,7 @@ func (s *SharedSocket) ReadFrom(b []byte) (n int, addr net.Addr, err error) { decodedLayers := make([]gopacket.LayerType, 0, 3) - err = parser.DecodeLayers(pkt.buf[:], &decodedLayers) + err = parser.DecodeLayers(pkt.buf, &decodedLayers) if err != nil { return 0, nil, err } diff --git a/signal/client/grpc.go b/signal/client/grpc.go index 9f70234e9..7aa9f9ce9 100644 --- a/signal/client/grpc.go +++ b/signal/client/grpc.go @@ -354,16 +354,17 @@ func (c *GrpcClient) receive(stream proto.SignalExchange_ConnectStreamClient, for { msg, err := stream.Recv() - if s, ok := status.FromError(err); ok && s.Code() == codes.Canceled { + switch s, ok := status.FromError(err); { + case ok && s.Code() == codes.Canceled: log.Debugf("stream canceled (usually indicates shutdown)") return err - } else if s.Code() == codes.Unavailable { + case s.Code() == codes.Unavailable: log.Debugf("Signal Service is unavailable") return err - } else if err == io.EOF { + case err == io.EOF: log.Debugf("Signal Service stream closed by server") return err - } else if err != nil { + case err != nil: return err } log.Tracef("received a new message from Peer [fingerprint: %s]", msg.Key) diff --git a/util/retry.go b/util/retry.go index 3bffcf288..2d5fbf6cf 100644 --- a/util/retry.go +++ b/util/retry.go @@ -15,7 +15,7 @@ func Retry(attempts int, sleep time.Duration, toExec func() error, onError func( if attempts--; attempts > 0 { jitter := time.Duration(rand.Int63n(int64(sleep))) - sleep = sleep + jitter/2 + sleep += jitter / 2 onError(err) time.Sleep(sleep)