add new linter (#4322)

Signed-off-by: oilbeater <[email protected]>
kubeovn · Jul 22, 2024 · 1ba9e7c · 1ba9e7c
1 parent d009a27
commit 1ba9e7c
Show file tree

Hide file tree

Showing 109 changed files with 1,111 additions and 1,039 deletions.
diff --git a/.github/workflows/build-x86-image.yaml b/.github/workflows/build-x86-image.yaml
@@ -23,7 +23,7 @@ concurrency:
 env:
   GO_VERSION: ''
   KIND_VERSION: v0.23.0
-  GOSEC_VERSION: '2.20.0'
+  GOLANGCI_LINT_VERSION: 'v1.59.1'
   HELM_VERSION: v3.15.3
   SUBMARINER_VERSION: '0.18.0'
 
@@ -167,14 +167,9 @@ jobs:
           go install -mod=mod github.com/onsi/ginkgo/v2/ginkgo
           make ut
 
-      - name: Install gosec
+      - name: Install golangci-lint
         run: |
-          tmp=$(mktemp -d)
-          archive="gosec_${{ env.GOSEC_VERSION }}_$(go env GOHOSTOS)_$(go env GOHOSTARCH).tar.gz"
-          wget -q -O "$tmp/$archive" https://github.com/securego/gosec/releases/download/v${{ env.GOSEC_VERSION }}/$archive
-          tar --no-same-owner -C "$tmp" -xzf "$tmp/$archive"
-          install "$tmp/gosec" /usr/local/bin
-          rm -rf $tmp
+          curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin $GOLANGCI_LINT_VERSION
 
       - name: Download base images
         if: needs.build-kube-ovn-base.outputs.build-base == 1

diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml
diff --git a/.golangci.yml b/.golangci.yml
@@ -15,6 +15,13 @@ linters:
     - unconvert
     - unused
     - errcheck
+    - gosec
+    - govet
+    - perfsprint
+    - usestdlibvars
+    - loggercheck
+    - whitespace
+    - errorlint
 
 issues:
   max-same-issues: 0
@@ -31,10 +38,20 @@ issues:
     - linters:
         - revive
       text: "VpcDnsList should be VpcDNSList" # api param not change
+    - linters:
+        - gosec
+      path: _test\.go
+    - linters:
+        - gosec
+      path: test/
 
 linters-settings:
   goimports:
     local-prefixes: github.com/kubeovn/kube-ovn
   gofumpt:
     extra-rules: true
+  perfsprint:
+    strconcat: false
+  errorlint:
+    asserts: false
 
diff --git a/Makefile b/Makefile
@@ -972,16 +972,7 @@ uninstall:
 
 .PHONY: lint
 lint:
-	@gofmt -d .
-	@if [ $$(gofmt -l . | wc -l) -ne 0 ]; then \
-		echo "Code differs from gofmt's style" 1>&2 && exit 1; \
-	fi
-	@GOOS=linux go vet ./...
-	@GOOS=linux gosec -exclude-dir=test -exclude-dir=pkg/client ./...
-
-.PHONY: gofumpt
-gofumpt:
-	gofumpt -w -extra .
+	golangci-lint run -v
 
 .PHONY: lint-windows
 lint-windows:

diff --git a/cmd/daemon/cniserver.go b/cmd/daemon/cniserver.go
@@ -1,6 +1,7 @@
 package daemon
 
 import (
+	"errors"
 	"fmt"
 	"net"
 	"net/http"
@@ -175,7 +176,7 @@ func initChassisAnno(cfg *daemon.Configuration) error {
 	chassesName := strings.TrimSpace(string(chassisID))
 	if chassesName == "" {
 		// not ready yet
-		err = fmt.Errorf("chassis id is empty")
+		err = errors.New("chassis id is empty")
 		klog.Error(err)
 		return err
 	}

diff --git a/cmd/daemon/init.go b/cmd/daemon/init.go
@@ -14,7 +14,7 @@ func initForOS() error {
 	// disable checksum for genev_sys_6081 as default
 	cmd := exec.Command("sh", "-c", "ethtool -K genev_sys_6081 tx off")
 	if err := cmd.Run(); err != nil {
-		err := fmt.Errorf("failed to set checksum off for genev_sys_6081, %v", err)
+		err := fmt.Errorf("failed to set checksum off for genev_sys_6081, %w", err)
 		// should not affect cni pod running if failed, just record err log
 		klog.Error(err)
 	}

diff --git a/go.mod b/go.mod
@@ -44,6 +44,7 @@ require (
 	gopkg.in/k8snetworkplumbingwg/multus-cni.v4 v4.0.2
 	k8s.io/api v0.30.3
 	k8s.io/apimachinery v0.30.3
+	k8s.io/apiserver v0.30.3
 	k8s.io/client-go v12.0.0+incompatible
 	k8s.io/klog/v2 v2.130.1
 	k8s.io/kubectl v0.30.3
@@ -242,7 +243,6 @@ require (
 	gopkg.in/yaml.v2 v2.4.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 	k8s.io/apiextensions-apiserver v0.30.3 // indirect
-	k8s.io/apiserver v0.30.3 // indirect
 	k8s.io/cli-runtime v0.30.3 // indirect
 	k8s.io/cloud-provider v0.30.3 // indirect
 	k8s.io/cluster-bootstrap v0.30.3 // indirect

diff --git a/pkg/controller/admin_network_policy.go b/pkg/controller/admin_network_policy.go
@@ -1,6 +1,7 @@
 package controller
 
 import (
+	"errors"
 	"fmt"
 	"reflect"
 	"strings"
@@ -200,7 +201,7 @@ func (c *Controller) processNextUpdateAnpWorkItem() bool {
 		}
 		if err := c.handleUpdateAnp(key); err != nil {
 			c.updateAnpQueue.AddRateLimited(key)
-			return fmt.Errorf("error syncing admin network policy %s: %v, requeuing", key.key, err)
+			return fmt.Errorf("error syncing admin network policy %s: %w, requeuing", key.key, err)
 		}
 		c.updateAnpQueue.Forget(obj)
 		return nil
@@ -358,10 +359,10 @@ func (c *Controller) handleAddAnp(key string) (err error) {
 	}
 
 	if err := c.OVNNbClient.Transact("add-ingress-acls", ingressACLOps); err != nil {
-		return fmt.Errorf("failed to add ingress acls for anp %s: %v", key, err)
+		return fmt.Errorf("failed to add ingress acls for anp %s: %w", key, err)
 	}
 	if err := c.deleteUnusedAddrSetForAnp(curIngressAddrSet, desiredIngressAddrSet); err != nil {
-		return fmt.Errorf("failed to delete unused ingress address set for anp %s: %v", key, err)
+		return fmt.Errorf("failed to delete unused ingress address set for anp %s: %w", key, err)
 	}
 
 	egressACLOps, err := c.OVNNbClient.DeleteAclsOps(pgName, portGroupKey, "from-lport", nil)
@@ -423,10 +424,10 @@ func (c *Controller) handleAddAnp(key string) (err error) {
 	}
 
 	if err := c.OVNNbClient.Transact("add-egress-acls", egressACLOps); err != nil {
-		return fmt.Errorf("failed to add egress acls for anp %s: %v", key, err)
+		return fmt.Errorf("failed to add egress acls for anp %s: %w", key, err)
 	}
 	if err := c.deleteUnusedAddrSetForAnp(curEgressAddrSet, desiredEgressAddrSet); err != nil {
-		return fmt.Errorf("failed to delete unused egress address set for anp %s: %v", key, err)
+		return fmt.Errorf("failed to delete unused egress address set for anp %s: %w", key, err)
 	}
 
 	return nil
@@ -600,26 +601,26 @@ func (c *Controller) fetchSelectedPods(anpSubject *v1alpha1.AdminNetworkPolicySu
 	if anpSubject.Namespaces != nil {
 		nsSelector, err := metav1.LabelSelectorAsSelector(anpSubject.Namespaces)
 		if err != nil {
-			return nil, fmt.Errorf("error creating ns label selector, %v", err)
+			return nil, fmt.Errorf("error creating ns label selector, %w", err)
 		}
 
 		ports, _, _, err = c.fetchPods(nsSelector, labels.Everything())
 		if err != nil {
-			return nil, fmt.Errorf("failed to fetch pods, %v", err)
+			return nil, fmt.Errorf("failed to fetch pods, %w", err)
 		}
 	} else {
 		nsSelector, err := metav1.LabelSelectorAsSelector(&anpSubject.Pods.NamespaceSelector)
 		if err != nil {
-			return nil, fmt.Errorf("error creating ns label selector, %v", err)
+			return nil, fmt.Errorf("error creating ns label selector, %w", err)
 		}
 		podSelector, err := metav1.LabelSelectorAsSelector(&anpSubject.Pods.PodSelector)
 		if err != nil {
-			return nil, fmt.Errorf("error creating pod label selector, %v", err)
+			return nil, fmt.Errorf("error creating pod label selector, %w", err)
 		}
 
 		ports, _, _, err = c.fetchPods(nsSelector, podSelector)
 		if err != nil {
-			return nil, fmt.Errorf("failed to fetch pods, %v", err)
+			return nil, fmt.Errorf("failed to fetch pods, %w", err)
 		}
 	}
 	klog.Infof("get selected ports for subject, %v", ports)
@@ -641,7 +642,7 @@ func (c *Controller) fetchPods(nsSelector, podSelector labels.Selector) ([]strin
 	for _, namespace := range namespaces {
 		pods, err := c.podsLister.Pods(namespace.Name).List(podSelector)
 		if err != nil {
-			return nil, nil, nil, fmt.Errorf("failed to list pods, %v", err)
+			return nil, nil, nil, fmt.Errorf("failed to list pods, %w", err)
 		}
 
 		for _, pod := range pods {
@@ -652,7 +653,7 @@ func (c *Controller) fetchPods(nsSelector, podSelector labels.Selector) ([]strin
 
 			podNets, err := c.getPodKubeovnNets(pod)
 			if err != nil {
-				return nil, nil, nil, fmt.Errorf("failed to get pod networks, %v", err)
+				return nil, nil, nil, fmt.Errorf("failed to get pod networks, %w", err)
 			}
 
 			for _, podNet := range podNets {
@@ -688,26 +689,26 @@ func (c *Controller) fetchIngressSelectedAddresses(ingressPeer *v1alpha1.AdminNe
 	if ingressPeer.Namespaces != nil {
 		nsSelector, err := metav1.LabelSelectorAsSelector(ingressPeer.Namespaces)
 		if err != nil {
-			return nil, nil, fmt.Errorf("error creating ns label selector, %v", err)
+			return nil, nil, fmt.Errorf("error creating ns label selector, %w", err)
 		}
 
 		_, v4Addresses, v6Addresses, err = c.fetchPods(nsSelector, labels.Everything())
 		if err != nil {
-			return nil, nil, fmt.Errorf("failed to fetch ingress peer addresses, %v", err)
+			return nil, nil, fmt.Errorf("failed to fetch ingress peer addresses, %w", err)
 		}
 	} else if ingressPeer.Pods != nil {
 		nsSelector, err := metav1.LabelSelectorAsSelector(&ingressPeer.Pods.NamespaceSelector)
 		if err != nil {
-			return nil, nil, fmt.Errorf("error creating ns label selector, %v", err)
+			return nil, nil, fmt.Errorf("error creating ns label selector, %w", err)
 		}
 		podSelector, err := metav1.LabelSelectorAsSelector(&ingressPeer.Pods.PodSelector)
 		if err != nil {
-			return nil, nil, fmt.Errorf("error creating pod label selector, %v", err)
+			return nil, nil, fmt.Errorf("error creating pod label selector, %w", err)
 		}
 
 		_, v4Addresses, v6Addresses, err = c.fetchPods(nsSelector, podSelector)
 		if err != nil {
-			return nil, nil, fmt.Errorf("failed to fetch ingress peer addresses, %v", err)
+			return nil, nil, fmt.Errorf("failed to fetch ingress peer addresses, %w", err)
 		}
 	}
 
@@ -723,29 +724,29 @@ func (c *Controller) fetchEgressSelectedAddresses(egressPeer *v1alpha1.AdminNetw
 	case egressPeer.Namespaces != nil:
 		nsSelector, err := metav1.LabelSelectorAsSelector(egressPeer.Namespaces)
 		if err != nil {
-			return nil, nil, fmt.Errorf("error creating ns label selector, %v", err)
+			return nil, nil, fmt.Errorf("error creating ns label selector, %w", err)
 		}
 
 		_, v4Addresses, v6Addresses, err = c.fetchPods(nsSelector, labels.Everything())
 		if err != nil {
-			return nil, nil, fmt.Errorf("failed to fetch egress peer addresses, %v", err)
+			return nil, nil, fmt.Errorf("failed to fetch egress peer addresses, %w", err)
 		}
 	case egressPeer.Pods != nil:
 		nsSelector, err := metav1.LabelSelectorAsSelector(&egressPeer.Pods.NamespaceSelector)
 		if err != nil {
-			return nil, nil, fmt.Errorf("error creating ns label selector, %v", err)
+			return nil, nil, fmt.Errorf("error creating ns label selector, %w", err)
 		}
 		podSelector, err := metav1.LabelSelectorAsSelector(&egressPeer.Pods.PodSelector)
 		if err != nil {
-			return nil, nil, fmt.Errorf("error creating pod label selector, %v", err)
+			return nil, nil, fmt.Errorf("error creating pod label selector, %w", err)
 		}
 
 		_, v4Addresses, v6Addresses, err = c.fetchPods(nsSelector, podSelector)
 		if err != nil {
-			return nil, nil, fmt.Errorf("failed to fetch egress peer addresses, %v", err)
+			return nil, nil, fmt.Errorf("failed to fetch egress peer addresses, %w", err)
 		}
 	default:
-		return nil, nil, fmt.Errorf("either Namespaces or Pods must be specified in egressPeer")
+		return nil, nil, errors.New("either Namespaces or Pods must be specified in egressPeer")
 	}
 
 	return v4Addresses, v6Addresses, nil

diff --git a/pkg/controller/baseline_admin_network_policy.go b/pkg/controller/baseline_admin_network_policy.go
@@ -173,7 +173,7 @@ func (c *Controller) processNextUpdateBanpWorkItem() bool {
 		}
 		if err := c.handleUpdateBanp(key); err != nil {
 			c.updateBanpQueue.AddRateLimited(key)
-			return fmt.Errorf("error syncing banp %s: %v, requeuing", key.key, err)
+			return fmt.Errorf("error syncing banp %s: %w, requeuing", key.key, err)
 		}
 		c.updateBanpQueue.Forget(obj)
 		return nil
@@ -319,10 +319,10 @@ func (c *Controller) handleAddBanp(key string) (err error) {
 	}
 
 	if err := c.OVNNbClient.Transact("add-ingress-acls", ingressACLOps); err != nil {
-		return fmt.Errorf("failed to add ingress acls for banp %s: %v", key, err)
+		return fmt.Errorf("failed to add ingress acls for banp %s: %w", key, err)
 	}
 	if err := c.deleteUnusedAddrSetForAnp(curIngressAddrSet, desiredIngressAddrSet); err != nil {
-		return fmt.Errorf("failed to delete unused ingress address set for banp %s: %v", key, err)
+		return fmt.Errorf("failed to delete unused ingress address set for banp %s: %w", key, err)
 	}
 
 	egressACLOps, err := c.OVNNbClient.DeleteAclsOps(pgName, portGroupKey, "from-lport", nil)
@@ -384,10 +384,10 @@ func (c *Controller) handleAddBanp(key string) (err error) {
 	}
 
 	if err := c.OVNNbClient.Transact("add-egress-acls", egressACLOps); err != nil {
-		return fmt.Errorf("failed to add egress acls for banp %s: %v", key, err)
+		return fmt.Errorf("failed to add egress acls for banp %s: %w", key, err)
 	}
 	if err := c.deleteUnusedAddrSetForAnp(curEgressAddrSet, desiredEgressAddrSet); err != nil {
-		return fmt.Errorf("failed to delete unused egress address set for banp %s: %v", key, err)
+		return fmt.Errorf("failed to delete unused egress address set for banp %s: %w", key, err)
 	}
 
 	return nil

diff --git a/pkg/controller/config.go b/pkg/controller/config.go
@@ -1,6 +1,7 @@
 package controller
 
 import (
+	"errors"
 	"flag"
 	"fmt"
 	"os"
@@ -265,7 +266,7 @@ func ParseFlags() (*Configuration, error) {
 	}
 
 	if config.NetworkType == util.NetworkTypeVlan && config.DefaultHostInterface == "" {
-		return nil, fmt.Errorf("no host nic for vlan")
+		return nil, errors.New("no host nic for vlan")
 	}
 
 	if config.DefaultGateway == "" {
@@ -302,7 +303,7 @@ func ParseFlags() (*Configuration, error) {
 
 	if err := util.CheckSystemCIDR([]string{config.NodeSwitchCIDR, config.DefaultCIDR, config.ServiceClusterIPRange}); err != nil {
 		klog.Error(err)
-		return nil, fmt.Errorf("check system cidr failed, %v", err)
+		return nil, fmt.Errorf("check system cidr failed, %w", err)
 	}
 
 	for _, ip := range strings.Split(*argNodeLocalDNSIP, ",") {

diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go
@@ -1204,7 +1204,7 @@ func (c *Controller) allSubnetReady(subnets ...string) (bool, error) {
 		exist, err := c.OVNNbClient.LogicalSwitchExists(lsName)
 		if err != nil {
 			klog.Error(err)
-			return false, fmt.Errorf("check logical switch %s exist: %v", lsName, err)
+			return false, fmt.Errorf("check logical switch %s exist: %w", lsName, err)
 		}
 
 		if !exist {