add golangci-lint and fix linting issues

tariq1890 · Aug 20, 2023 · 39e133f · 39e133f
1 parent 902fbf7
commit 39e133f
Show file tree

Hide file tree

Showing 22 changed files with 240 additions and 201 deletions.
diff --git a/.github/workflows/sanity.yml b/.github/workflows/sanity.yml
@@ -3,6 +3,7 @@ name: Sanity
 
 env:
   GO_VERSION: '1.17.x'
+  GOLANGCI_LINT_VERSION: 'v1.54.1'
 
 jobs:
   build:
@@ -22,7 +23,7 @@ jobs:
       with:
         go-version: ${{ env.GO_VERSION }}
     - name: Install golint
-      run: go install golang.org/x/lint/golint@latest
+      run: curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin ${GOLANGCI_LINT_VERSION}
     - name: Lint
       run: make lint
     - name: Fmt

diff --git a/.golangci.yml b/.golangci.yml
@@ -0,0 +1,24 @@
+run:
+  deadline: 10m
+linters:
+  disable-all: true
+  enable:
+    - gocritic
+    - gofmt
+    - goimports
+    - gosec
+    - gosimple
+    - govet
+    - ineffassign
+    - misspell
+    - staticcheck
+    - unconvert
+linters-settings:
+  gocritic:
+    disabled-checks:
+      - ifElseChain
+  goimports:
+    local-prefixes: github.com/container-orchestrated-devices/container-device-interface
+  gosec:
+    excludes:
+      - G306
diff --git a/Makefile b/Makefile
@@ -13,9 +13,8 @@ GO_CMD   := go
 GO_BUILD := $(GO_CMD) build
 GO_TEST  := $(GO_CMD) test -race -v -cover
 
-GO_LINT  := golint -set_exit_status
-GO_FMT   := gofmt
-GO_VET   := $(GO_CMD) vet
+GOLANGCI_LINT := golangci-lint run ./...
+GO_FMT        := gofmt
 
 CDI_PKG  := $(shell grep ^module go.mod | sed 's/^module *//g')
 
@@ -49,7 +48,7 @@ fmt format:
 	$(Q)$(GO_FMT) -s -d -w -e .
 
 lint:
-	$(Q)$(GO_LINT) -set_exit_status ./...
+	$(Q)$(GOLANGCI_LINT) ./...
 vet:
 	$(Q)$(GO_VET) ./...
 

diff --git a/internal/multierror/multierror_test.go b/internal/multierror/multierror_test.go
@@ -17,8 +17,8 @@
 package multierror
 
 import (
-	"testing"
 	"errors"
+	"testing"
 
 	"github.com/stretchr/testify/assert"
 )

diff --git a/internal/validation/validate.go b/internal/validation/validate.go
@@ -29,8 +29,7 @@ func ValidateSpecAnnotations(name string, any interface{}) error {
 		return nil
 	}
 
-	switch v := any.(type) {
-	case map[string]interface{}:
+	if v, ok := any.(map[string]interface{}); ok {
 		annotations := make(map[string]string)
 		for k, v := range v {
 			if s, ok := v.(string); ok {

diff --git a/pkg/cdi/annotations_test.go b/pkg/cdi/annotations_test.go
@@ -339,10 +339,8 @@ func TestUpdateAnnotations(t *testing.T) {
 				annotations, err = UpdateAnnotations(annotations, i.plugin, i.devID, i.devices)
 				if !tc.invalid {
 					require.NoError(t, err, "CDI device injection annotation")
-				} else {
-					if err != nil {
-						break
-					}
+				} else if err != nil {
+					break
 				}
 			}
 			if tc.invalid {

diff --git a/pkg/cdi/cache.go b/pkg/cdi/cache.go
@@ -26,10 +26,11 @@ import (
 	"strings"
 	"sync"
 
-	"github.com/container-orchestrated-devices/container-device-interface/internal/multierror"
-	cdi "github.com/container-orchestrated-devices/container-device-interface/specs-go"
 	"github.com/fsnotify/fsnotify"
 	oci "github.com/opencontainers/runtime-spec/specs-go"
+
+	"github.com/container-orchestrated-devices/container-device-interface/internal/multierror"
+	cdi "github.com/container-orchestrated-devices/container-device-interface/specs-go"
 )
 
 // Option is an option to change some aspect of default CDI behavior.

diff --git a/pkg/cdi/cache_test.go b/pkg/cdi/cache_test.go
@@ -27,11 +27,12 @@ import (
 	"testing"
 	"time"
 
-	"github.com/container-orchestrated-devices/container-device-interface/pkg/cdi/validate"
-	cdi "github.com/container-orchestrated-devices/container-device-interface/specs-go"
 	oci "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/stretchr/testify/require"
 	"sigs.k8s.io/yaml"
+
+	"github.com/container-orchestrated-devices/container-device-interface/pkg/cdi/validate"
+	cdi "github.com/container-orchestrated-devices/container-device-interface/specs-go"
 )
 
 func TestNewCache(t *testing.T) {
@@ -177,6 +178,7 @@ devices:
 				}
 			} else {
 				dir, err = mkTestDir(t, nil)
+				require.NoError(t, err)
 			}
 
 			cache, err = NewCache(WithSpecDirs(
@@ -554,6 +556,7 @@ devices:
 							opts = append(opts, WithAutoRefresh(false))
 						}
 						cache, err = NewCache(opts...)
+						require.NoError(t, err)
 						require.NotNil(t, cache)
 					} else {
 						err = updateSpecDirs(t, dir, update.etc, update.run)
@@ -718,7 +721,7 @@ devices:
 					}
 
 					select {
-					case _ = <-stopCh:
+					case <-stopCh:
 						return
 					default:
 					}
@@ -748,7 +751,7 @@ devices:
 					}
 
 					select {
-					case _ = <-stopCh:
+					case <-stopCh:
 						return
 					default:
 					}
@@ -771,10 +774,10 @@ devices:
 				// from updater()'s create+write+rename loop)
 				for {
 					select {
-					case _ = <-stopCh:
+					case <-stopCh:
 						go osSync()
 						return
-					case _ = <-sync.C:
+					case <-sync.C:
 						go osSync()
 						sync.Reset(2 * time.Second)
 					}
@@ -803,7 +806,7 @@ devices:
 				select {
 				case err = <-errCh:
 					require.NotNil(t, err)
-				case _ = <-done:
+				case <-done:
 					close(stopCh)
 					wg.Wait()
 					return
@@ -1685,7 +1688,7 @@ devices:
 				"-0",
 			},
 			expected: [][]string{
-				[]string{
+				{
 					"vendor.com/device=dev1",
 				},
 				nil,
@@ -1746,25 +1749,25 @@ devices:
 				"-2",
 			},
 			expected: [][]string{
-				[]string{
+				{
 					"vendor.com/device=dev1",
 				},
-				[]string{
+				{
 					"vendor.com/device=dev1",
 					"vendor.com/device=dev2",
 				},
-				[]string{
+				{
 					"vendor.com/device=dev1",
 					"vendor.com/device=dev2",
 					"vendor.com/device=dev3",
 					"vendor.com/device=dev4",
 				},
-				[]string{
+				{
 					"vendor.com/device=dev2",
 					"vendor.com/device=dev3",
 					"vendor.com/device=dev4",
 				},
-				[]string{
+				{
 					"vendor.com/device=dev3",
 					"vendor.com/device=dev4",
 				},
@@ -1811,7 +1814,7 @@ devices:
 				)
 
 				if data[0] == '-' {
-					delIdx, err = strconv.Atoi(string(data[1:]))
+					delIdx, err = strconv.Atoi(data[1:])
 					require.NoError(t, err)
 
 					err = cache.RemoveSpec(specs[delIdx])

diff --git a/pkg/cdi/container-edits.go b/pkg/cdi/container-edits.go
@@ -24,9 +24,10 @@ import (
 	"sort"
 	"strings"
 
-	"github.com/container-orchestrated-devices/container-device-interface/specs-go"
 	oci "github.com/opencontainers/runtime-spec/specs-go"
 	ocigen "github.com/opencontainers/runtime-tools/generate"
+
+	"github.com/container-orchestrated-devices/container-device-interface/specs-go"
 )
 
 const (
@@ -90,14 +91,17 @@ func (e *ContainerEdits) Apply(spec *oci.Spec) error {
 			return err
 		}
 		dev := d.ToOCI()
-		if dev.UID == nil && spec.Process != nil {
-			if uid := spec.Process.User.UID; uid > 0 {
-				dev.UID = &uid
+
+		if spec.Process != nil {
+			if dev.UID == nil {
+				if uid := spec.Process.User.UID; uid > 0 {
+					dev.UID = &uid
+				}
 			}
-		}
-		if dev.GID == nil && spec.Process != nil {
-			if gid := spec.Process.User.GID; gid > 0 {
-				dev.GID = &gid
+			if dev.GID == nil {
+				if gid := spec.Process.User.GID; gid > 0 {
+					dev.GID = &gid
+				}
 			}
 		}
 

diff --git a/pkg/cdi/container-edits_test.go b/pkg/cdi/container-edits_test.go
@@ -17,19 +17,18 @@
 package cdi
 
 import (
-	"os"
 	"testing"
 
-	"github.com/container-orchestrated-devices/container-device-interface/specs-go"
-	cdi "github.com/container-orchestrated-devices/container-device-interface/specs-go"
 	oci "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/stretchr/testify/require"
+
+	"github.com/container-orchestrated-devices/container-device-interface/specs-go"
+	cdi "github.com/container-orchestrated-devices/container-device-interface/specs-go"
 )
 
 func TestValidateContainerEdits(t *testing.T) {
 	type testCase struct {
 		name    string
-		spec    *oci.Spec
 		edits   *cdi.ContainerEdits
 		invalid bool
 	}
@@ -625,8 +624,3 @@ func TestAppend(t *testing.T) {
 		})
 	}
 }
-
-func fileMode(mode int) *os.FileMode {
-	fm := os.FileMode(mode)
-	return &fm
-}
diff --git a/pkg/cdi/container-edits_unix.go b/pkg/cdi/container-edits_unix.go
@@ -42,11 +42,9 @@ func (d *DeviceNode) fillMissingInfo() error {
 
 	if d.Type == "" {
 		d.Type = string(hostDev.Type)
-	} else {
-		if d.Type != string(hostDev.Type) {
-			return fmt.Errorf("CDI device (%q, %q), host type mismatch (%s, %s)",
-				d.Path, d.HostPath, d.Type, string(hostDev.Type))
-		}
+	} else if d.Type != string(hostDev.Type) {
+		return fmt.Errorf("CDI device (%q, %q), host type mismatch (%s, %s)",
+			d.Path, d.HostPath, d.Type, string(hostDev.Type))
 	}
 	if d.Major == 0 && d.Type != "p" {
 		d.Major = hostDev.Major

diff --git a/pkg/cdi/device.go b/pkg/cdi/device.go
@@ -19,10 +19,11 @@ package cdi
 import (
 	"fmt"
 
+	oci "github.com/opencontainers/runtime-spec/specs-go"
+
 	"github.com/container-orchestrated-devices/container-device-interface/internal/validation"
 	"github.com/container-orchestrated-devices/container-device-interface/pkg/parser"
 	cdi "github.com/container-orchestrated-devices/container-device-interface/specs-go"
-	oci "github.com/opencontainers/runtime-spec/specs-go"
 )
 
 // Device represents a CDI device of a Spec.
@@ -67,7 +68,7 @@ func (d *Device) edits() *ContainerEdits {
 
 // Validate the device.
 func (d *Device) validate() error {
-	if err := ValidateDeviceName(d.Name); err != nil {
+	if err := parser.ValidateDeviceName(d.Name); err != nil {
 		return err
 	}
 	name := d.Name

diff --git a/pkg/cdi/device_test.go b/pkg/cdi/device_test.go
@@ -19,8 +19,9 @@ package cdi
 import (
 	"testing"
 
-	cdi "github.com/container-orchestrated-devices/container-device-interface/specs-go"
 	"github.com/stretchr/testify/require"
+
+	cdi "github.com/container-orchestrated-devices/container-device-interface/specs-go"
 )
 
 func TestDeviceValidate(t *testing.T) {