Merge pull request #35 from elezar/add-golangci-lint

Add golangci lint

.common-ci.yml

@@ -19,7 +19,6 @@ default:
 
 variables:
   GIT_SUBMODULE_STRATEGY: recursive
-  BUILDIMAGE: "${CI_REGISTRY_IMAGE}/build:${CI_COMMIT_SHORT_SHA}"
   BUILD_MULTI_ARCH_IMAGES: "true"
 
 stages:

.github/workflows/golang.yaml

@@ -26,9 +26,6 @@ on:
 
 jobs:
   check:
-    # TODO: Disable golangci-lint until the repo has been onboarded.
-    # See https://github.com/NVIDIA/mig-parted/issues/34
-    if: false
     runs-on: ubuntu-latest
     steps:
     - uses: actions/checkout@v4

.golangci.yml

@@ -0,0 +1,30 @@
+run:
+  deadline: 10m
+
+linters:
+  enable:
+    - contextcheck
+    - errcheck
+    - gocritic
+    - gofmt
+    - goimports
+    - gosec
+    - gosimple
+    - govet
+    - ineffassign
+    - misspell
+    - staticcheck
+    - unconvert
+  disable: []
+
+linters-settings:
+  goimports:
+    local-prefixes: github.com/NVIDIA/mig-parted
+
+issues:
+  exclude-rules:
+  # We use math/rand instead of crypto/rand for unique names in config tests.
+  - path: pkg/mig/config/config_test.go
+    linters:
+    - gosec
+    text: "G404"

Makefile

@@ -27,7 +27,7 @@ EXAMPLE_TARGETS := $(patsubst %,example-%,$(EXAMPLES))
 CMDS := $(patsubst ./cmd/%/,%,$(sort $(dir $(wildcard ./cmd/*/))))
 CMD_TARGETS := $(patsubst %,cmd-%, $(CMDS))
 
-CHECK_TARGETS := assert-fmt vet lint ineffassign misspell
+CHECK_TARGETS := lint
 MAKE_TARGETS := binaries build check fmt lint-internal test examples cmds coverage generate $(CHECK_TARGETS)
 
 TARGETS := $(MAKE_TARGETS) $(EXAMPLE_TARGETS) $(CMD_TARGETS)
@@ -66,43 +66,24 @@ fmt:
 	go list -f '{{.Dir}}' $(MODULE)/... \
 		| xargs gofmt -s -l -w
 
-assert-fmt:
-	go list -f '{{.Dir}}' $(MODULE)/... \
-		| xargs gofmt -s -l > fmt.out
-	@if [ -s fmt.out ]; then \
-		echo "\nERROR: The following files are not formatted:\n"; \
-		cat fmt.out; \
-		rm fmt.out; \
-		exit 1; \
-	else \
-		rm fmt.out; \
-	fi
-
-ineffassign:
-	ineffassign $(MODULE)/...
+goimports:
+	go list -f {{.Dir}} $(MODULE)/... \
+		| xargs goimports -local $(MODULE) -w
 
 lint:
-# We use `go list -f '{{.Dir}}' $(MODULE)/...` to skip the `vendor` folder.
-	go list -f '{{.Dir}}' $(MODULE)/... | grep -v /internal/ | xargs golint -set_exit_status
-
-lint-internal:
-# We use `go list -f '{{.Dir}}' $(MODULE)/...` to skip the `vendor` folder.
-	go list -f '{{.Dir}}' $(MODULE)/internal/... | xargs golint -set_exit_status
-
-misspell:
-	misspell $(MODULE)/...
-
-vet:
-	go vet $(MODULE)/...
+	golangci-lint run ./...
 
 COVERAGE_FILE := coverage.out
 test: build cmds
-	go test -v -coverprofile=$(COVERAGE_FILE) $(MODULE)/...
+	go test -coverprofile=$(COVERAGE_FILE) $(MODULE)/cmd/... $(MODULE)/internal/... $(MODULE)/api/...
 
 coverage: test
 	cat $(COVERAGE_FILE) | grep -v "_mock.go" > $(COVERAGE_FILE).no-mocks
 	go tool cover -func=$(COVERAGE_FILE).no-mocks
 
+generate:
+	go generate $(MODULE)/...
+
 $(DOCKER_TARGETS): docker-%:
 	@echo "Running 'make $(*)' in container image $(BUILDIMAGE)"
 	$(DOCKER) run \

api/hooks/v1/hooks.go

@@ -66,7 +66,7 @@ func (h HooksMap) Run(name string, envs EnvsMap, output bool) error {
 // It injects the environment variables associated with the provided EnvMap,
 // and optionally prints the output for each hook to stdout and stderr.
 func (h *HookSpec) Run(envs EnvsMap, output bool) error {
-	cmd := exec.Command(h.Command, h.Args...)
+	cmd := exec.Command(h.Command, h.Args...) //nolint:gosec
 	cmd.Env = h.Envs.Combine(envs).Format()
 	cmd.Dir = h.Workdir
 	if output {
@@ -77,7 +77,7 @@ func (h *HookSpec) Run(envs EnvsMap, output bool) error {
 }
 
 // Combine merges to EnvMaps together
-// Overlapping enviroment variables from e2 take precendence over those in e.
+// Overlapping environment variables from e2 take precedence over those in e.
 func (e EnvsMap) Combine(e2 EnvsMap) EnvsMap {
 	combined := make(EnvsMap)
 	for k, v := range e {

api/hooks/v1/hooks_test.go

@@ -49,7 +49,7 @@ func captureOutput(f func() error) (string, error) {
 	go func() {
 		var buf bytes.Buffer
 		wg.Done()
-		io.Copy(&buf, reader)
+		_, _ = io.Copy(&buf, reader)
 		out <- buf.String()
 	}()
 

api/spec/v1/helpers.go

@@ -48,17 +48,15 @@ func (ms *MigConfigSpec) MatchesDeviceFilter(deviceID types.DeviceID) bool {
 
 // MatchesAllDevices checks a 'MigConfigSpec' to see if it matches on 'all' devices.
 func (ms *MigConfigSpec) MatchesAllDevices() bool {
-	switch devices := ms.Devices.(type) {
-	case string:
+	if devices, ok := ms.Devices.(string); ok {
 		return devices == "all"
 	}
 	return false
 }
 
 // MatchesDevices checks a 'MigConfigSpec' to see if it matches on a device at the specified 'index'.
 func (ms *MigConfigSpec) MatchesDevices(index int) bool {
-	switch devices := ms.Devices.(type) {
-	case []int:
+	if devices, ok := ms.Devices.([]int); ok {
 		for _, d := range devices {
 			if index == d {
 				return true

api/spec/v1/spec.go

@@ -57,8 +57,7 @@ func (s *Spec) UnmarshalJSON(b []byte) error {
 
 	result := Spec{}
 	for k, v := range spec {
-		switch k {
-		case "version":
+		if k == "version" {
 			var version string
 			err := json.Unmarshal(v, &version)
 			if err != nil {

api/spec/v1/spec_test.go

@@ -19,16 +19,17 @@ package v1
 import (
 	"testing"
 
-	"github.com/NVIDIA/mig-parted/pkg/types"
 	"github.com/stretchr/testify/require"
 	"sigs.k8s.io/yaml"
+
+	"github.com/NVIDIA/mig-parted/pkg/types"
 )
 
 func TestMarshallUnmarshall(t *testing.T) {
 	spec := Spec{
 		Version: "v1",
 		MigConfigs: map[string]MigConfigSpecSlice{
-			"valid-format-non-existant-devices": []MigConfigSpec{
+			"valid-format-non-existent-devices": []MigConfigSpec{
 				{
 					Devices:    "all",
 					MigEnabled: true,

cmd/nvidia-mig-manager/main.go

@@ -23,10 +23,11 @@ import (
 	"strings"
 	"sync"
 
-	"github.com/NVIDIA/mig-parted/internal/info"
 	log "github.com/sirupsen/logrus"
 	cli "github.com/urfave/cli/v2"
 
+	"github.com/NVIDIA/mig-parted/internal/info"
+
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/client-go/kubernetes"
@@ -275,7 +276,7 @@ func start(c *cli.Context) error {
 			log.Errorf("Error: %s", err)
 			continue
 		}
-		log.Infof("Successfuly updated to MIG config: %s", value)
+		log.Infof("Successfully updated to MIG config: %s", value)
 	}
 }
 

cmd/nvidia-mig-parted/apply/apply.go

@@ -21,11 +21,12 @@ import (
 	"os"
 	"reflect"
 
+	"github.com/sirupsen/logrus"
+	cli "github.com/urfave/cli/v2"
+
 	hooks "github.com/NVIDIA/mig-parted/api/hooks/v1"
 	"github.com/NVIDIA/mig-parted/cmd/nvidia-mig-parted/assert"
 	"github.com/NVIDIA/mig-parted/internal/nvml"
-	"github.com/sirupsen/logrus"
-	cli "github.com/urfave/cli/v2"
 
 	"sigs.k8s.io/yaml"
 )
@@ -183,7 +184,7 @@ func (c *Context) ApplyMigConfig() error {
 func applyWrapper(c *cli.Context, f *Flags) error {
 	err := CheckFlags(f)
 	if err != nil {
-		cli.ShowSubcommandHelp(c)
+		_ = cli.ShowSubcommandHelp(c)
 		return err
 	}
 

cmd/nvidia-mig-parted/assert/assert.go

@@ -22,12 +22,13 @@ import (
 	"os"
 	"strings"
 
+	"github.com/sirupsen/logrus"
+	cli "github.com/urfave/cli/v2"
+
 	v1 "github.com/NVIDIA/mig-parted/api/spec/v1"
 	"github.com/NVIDIA/mig-parted/cmd/nvidia-mig-parted/util"
 	"github.com/NVIDIA/mig-parted/internal/nvml"
 	"github.com/NVIDIA/mig-parted/pkg/types"
-	"github.com/sirupsen/logrus"
-	cli "github.com/urfave/cli/v2"
 
 	"sigs.k8s.io/yaml"
 )
@@ -103,7 +104,7 @@ func BuildCommand() *cli.Command {
 func assertWrapper(c *cli.Context, f *Flags) error {
 	err := CheckFlags(f)
 	if err != nil {
-		cli.ShowSubcommandHelp(c)
+		_ = cli.ShowSubcommandHelp(c)
 		return err
 	}
 
@@ -233,7 +234,8 @@ func WalkSelectedMigConfigForEachGPU(migConfig v1.MigConfigSpecSlice, f func(*v1
 
 			log.Debugf("  GPU %v: %v", i, deviceID)
 
-			err = f(&mc, i, deviceID)
+			migConfigSpec := mc
+			err = f(&migConfigSpec, i, deviceID)
 			if err != nil {
 				return err
 			}

cmd/nvidia-mig-parted/checkpoint/checkpoint.go

@@ -22,12 +22,13 @@ import (
 	"os"
 	"strings"
 
+	"github.com/sirupsen/logrus"
+	cli "github.com/urfave/cli/v2"
+
 	checkpoint "github.com/NVIDIA/mig-parted/api/checkpoint/v1"
 	"github.com/NVIDIA/mig-parted/cmd/nvidia-mig-parted/util"
 	"github.com/NVIDIA/mig-parted/internal/nvml"
 	"github.com/NVIDIA/mig-parted/pkg/mig/state"
-	"github.com/sirupsen/logrus"
-	cli "github.com/urfave/cli/v2"
 )
 
 var log = logrus.New()
@@ -81,7 +82,7 @@ func CheckFlags(f *Flags) error {
 func checkpointWrapper(c *cli.Context, f *Flags) error {
 	err := CheckFlags(f)
 	if err != nil {
-		cli.ShowSubcommandHelp(c)
+		_ = cli.ShowSubcommandHelp(c)
 		return err
 	}
 
@@ -107,8 +108,12 @@ func checkpointWrapper(c *cli.Context, f *Flags) error {
 		return fmt.Errorf("error marshalling MIG state to json: %v", err)
 	}
 
-	err = os.WriteFile(f.CheckpointFile, []byte(j), 0666)
+	checkpointFile, err := os.Create(f.CheckpointFile)
 	if err != nil {
+		return fmt.Errorf("error creating checkpoint file: %w", err)
+	}
+	defer checkpointFile.Close()
+	if _, err := checkpointFile.Write(j); err != nil {
 		return fmt.Errorf("error writing checkpoint file: %v", err)
 	}
 

cmd/nvidia-mig-parted/export/export.go

@@ -22,11 +22,12 @@ import (
 	"io"
 	"os"
 
-	"github.com/NVIDIA/mig-parted/api/spec/v1"
-	"github.com/NVIDIA/mig-parted/internal/nvml"
 	"github.com/sirupsen/logrus"
 	cli "github.com/urfave/cli/v2"
 
+	v1 "github.com/NVIDIA/mig-parted/api/spec/v1"
+	"github.com/NVIDIA/mig-parted/internal/nvml"
+
 	yaml "gopkg.in/yaml.v2"
 )
 
@@ -91,7 +92,7 @@ func BuildCommand() *cli.Command {
 func exportWrapper(c *cli.Context, f *Flags) error {
 	err := CheckFlags(f)
 	if err != nil {
-		cli.ShowSubcommandHelp(c)
+		_ = cli.ShowSubcommandHelp(c)
 		return err
 	}
 
@@ -131,13 +132,17 @@ func WriteOutput(w io.Writer, spec *v1.Spec, f *Flags) error {
 		if err != nil {
 			return fmt.Errorf("error unmarshaling MIG config to YAML: %v", err)
 		}
-		w.Write(output)
+		if _, err := w.Write(output); err != nil {
+			return fmt.Errorf("error writing YAML output: %w", err)
+		}
 	case JSONFormat:
 		output, err := json.MarshalIndent(spec, "", "  ")
 		if err != nil {
 			return fmt.Errorf("error unmarshaling MIG config to JSON: %v", err)
 		}
-		w.Write(output)
+		if _, err := w.Write(output); err != nil {
+			return fmt.Errorf("error writing JSON output: %w", err)
+		}
 	}
 	return nil
 }

cmd/nvidia-mig-parted/export/export_test.go

@@ -19,8 +19,9 @@ package export
 import (
 	"testing"
 
-	"github.com/NVIDIA/mig-parted/api/spec/v1"
 	"github.com/stretchr/testify/require"
+
+	v1 "github.com/NVIDIA/mig-parted/api/spec/v1"
 )
 
 func TestMergeConfigSpecs(t *testing.T) {

cmd/nvidia-mig-parted/main.go

@@ -19,15 +19,16 @@ package main
 import (
 	"os"
 
+	log "github.com/sirupsen/logrus"
+	cli "github.com/urfave/cli/v2"
+
 	"github.com/NVIDIA/mig-parted/cmd/nvidia-mig-parted/apply"
 	"github.com/NVIDIA/mig-parted/cmd/nvidia-mig-parted/assert"
 	"github.com/NVIDIA/mig-parted/cmd/nvidia-mig-parted/checkpoint"
 	"github.com/NVIDIA/mig-parted/cmd/nvidia-mig-parted/export"
 	"github.com/NVIDIA/mig-parted/cmd/nvidia-mig-parted/restore"
 	"github.com/NVIDIA/mig-parted/cmd/nvidia-mig-parted/util"
 	"github.com/NVIDIA/mig-parted/internal/info"
-	log "github.com/sirupsen/logrus"
-	cli "github.com/urfave/cli/v2"
 )
 
 // Flags holds variables that represent the set of top level flags that can be passed to the mig-parted CLI.