Enable golangci-lint and go unit test github actions (#409)

* Enable golangci-lint github action Signed-off-by: Kimmo Lehto <[email protected]> Signed-off-by: Kimmo Lehto <[email protected]> * Automatic fixes via --fix Signed-off-by: Kimmo Lehto <[email protected]> * Manual lint fixes Signed-off-by: Kimmo Lehto <[email protected]> * Make unit tests pass Signed-off-by: Kimmo Lehto <[email protected]> * Manual fixes round 2 Signed-off-by: Kimmo Lehto <[email protected]> * Normalize logging syntax to "%s: .." instead of "%s .." Signed-off-by: Kimmo Lehto <[email protected]> * Restore the logo, exlude in linter Signed-off-by: Kimmo Lehto <[email protected]> * Remove "hostKind" argument from GetDockerInfo Signed-off-by: Kimmo Lehto <[email protected]> * Restore the RunHooks title test Signed-off-by: Kimmo Lehto <[email protected]> * Disable perfsprint Signed-off-by: Kimmo Lehto <[email protected]> * Fixed phase/manager logic for the phase success Signed-off-by: Dimitar Dimitrov <[email protected]> --------- Signed-off-by: Kimmo Lehto <[email protected]> Signed-off-by: Kimmo Lehto <[email protected]> Signed-off-by: Dimitar Dimitrov <[email protected]> Co-authored-by: Dimitar Dimitrov <[email protected]>
Mirantis · Feb 8, 2024 · c758b02 · c758b02
1 parent 0d82a56
commit c758b02
Show file tree

Hide file tree

Showing 98 changed files with 1,904 additions and 1,117 deletions.
diff --git a/.github/workflows/golangci-lint.yaml b/.github/workflows/golangci-lint.yaml
@@ -0,0 +1,34 @@
+name: Go lint
+on:
+  pull_request:
+    paths:
+      - '**.go'
+      - 'go.mod'
+      - 'go.sum'
+      - '.golangci.yml'
+
+jobs:
+  lint:
+    name: Lint
+    runs-on: ubuntu-latest
+    if: github.ref != 'refs/heads/main'
+    steps:
+      - name: Check out code into the Go module directory
+        uses: actions/checkout@v4
+
+      - name: Set up Go
+        uses: actions/setup-go@v5
+        with:
+          go-version-file: go.mod
+          check-latest: true
+
+      - name: Check go.mod/go.sum to be consistent
+        run: go mod tidy -v && git diff --exit-code
+
+      - name: golangci-lint
+        uses: golangci/golangci-lint-action@v3
+        with:
+          version: latest
+          skip-cache: true
+          only-new-issues: false
+          args: --verbose
diff --git a/.golangci.yml b/.golangci.yml
@@ -1,30 +1,88 @@
-# Visit https://golangci-lint.run/ for usage documentation
-# and information on other useful linters
-issues:
-  max-per-linter: 0
-  max-same-issues: 0
+run:
+  timeout: 8m
+
+  skip-dirs-use-default: false
+  skip-files:
+    - ".*\\.gen\\.go"
+    - examples/*
+    - test/*
+    - logo.go
+    - logo_windows.go
+  tests: false
+  allow-parallel-runners: true
 
 linters:
-  disable-all: true
   enable:
+    # enabled by default
+    - errcheck
+    - gosimple
+    - govet
+    - ineffassign
+    - staticcheck
+    - unused
+    # additional
+    - asciicheck
+    - bidichk
+    - bodyclose
+    - containedctx
+    - contextcheck
+    - dupword
     - durationcheck
-    #- errcheck
+    - errchkjson
+    - errname
+    - errorlint
+    - execinquery
     - exportloopref
-    #- forcetypeassert
+    - forcetypeassert
+    - gci
+    - gocritic
     - godot
+    - goerr113
     - gofmt
-    - gosimple
-    - ineffassign
-    - makezero
+    - gofumpt
+    - goimports
+    - goprintffuncname
+    - gosec
+    - importas
+    - ireturn
+    - maintidx
+    - mirror
     - misspell
-    - nilerr
+    - nakedret
+    - nilnil
+    - nolintlint
+    - nosprintfhostport
+    - prealloc
     - predeclared
-    - staticcheck
+    - reassign
+    - revive
+    - stylecheck
     - tenv
     - unconvert
     - unparam
-    - unused
-    - vet
+    - usestdlibvars
+    - varnamelen
+    - wastedassign
+    - whitespace
+    - wrapcheck
 
-run:
-  timeout: 10m
+linters-settings:
+  varnamelen:
+    max-distance: 10
+    ignore-decls:
+      - w http.ResponseWriter
+      - r *http.Request
+      - i int
+      - n int
+      - p []byte
+      - mu sync.Mutex
+      - wg sync.WaitGroup
+      - h Host
+      - h os.Host
+      - h *api.Host
+      - ok bool
+      - s string
+
+issues:
+  max-issues-per-linter: 0
+  max-same-issues: 0
diff --git a/cmd/apply.go b/cmd/apply.go
@@ -1,6 +1,7 @@
 package cmd
 
 import (
+	"errors"
 	"fmt"
 	"os"
 	"time"
@@ -9,13 +10,14 @@ import (
 	"github.com/Mirantis/mcc/pkg/config"
 	"github.com/Mirantis/mcc/pkg/util"
 	"github.com/Mirantis/mcc/version"
-
 	"github.com/mattn/go-isatty"
 	log "github.com/sirupsen/logrus"
 	"github.com/urfave/cli/v2"
 	event "gopkg.in/segmentio/analytics-go.v3"
 )
 
+var errInvalidArguments = errors.New("invalid arguments")
+
 // NewApplyCommand creates new apply command to be called from cli.
 func NewApplyCommand() *cli.Command {
 	return &cli.Command{
@@ -47,7 +49,7 @@ func NewApplyCommand() *cli.Command {
 		After:  actions(closeAnalytics, upgradeCheckResult),
 		Action: func(ctx *cli.Context) (err error) {
 			if ctx.Int("concurrency") < 1 {
-				return fmt.Errorf("invalid --concurrency %d (must be 1 or more)", ctx.Int("concurrency"))
+				return fmt.Errorf("%w: invalid --concurrency %d (must be 1 or more)", errInvalidArguments, ctx.Int("concurrency"))
 			}
 
 			var logFile *os.File
@@ -57,40 +59,39 @@ func NewApplyCommand() *cli.Command {
 
 			product, err := config.ProductFromFile(ctx.String("config"))
 			if err != nil {
-				return
+				return fmt.Errorf("failed to load product config: %w", err)
 			}
 
 			defer func() {
 				if err != nil && logFile != nil {
 					log.Infof("See %s for more logs ", logFile.Name())
 				}
-
 			}()
 
 			// Add logger to dump all log levels to file
 			logFile, err = addFileLogger(product.ClusterName(), "apply.log")
 			if err != nil {
-				return
+				return fmt.Errorf("failed to add file logger: %w", err)
 			}
 
 			if isatty.IsTerminal(os.Stdout.Fd()) {
 				os.Stdout.WriteString(util.Logo)
-				os.Stdout.WriteString(fmt.Sprintf("   Mirantis Launchpad (c) 2022 Mirantis, Inc.                          %s\n\n", version.Version))
+				fmt.Fprintf(os.Stdout, "   Mirantis Launchpad (c) 2022 Mirantis, Inc.                          %s\n\n", version.Version)
 			}
 
 			err = product.Apply(ctx.Bool("disable-cleanup"), ctx.Bool("force"), ctx.Int("concurrency"))
-
 			if err != nil {
 				analytics.TrackEvent("Cluster Apply Failed", nil)
-			} else {
-				duration := time.Since(start)
-				props := event.Properties{
-					"duration": duration.Seconds(),
-				}
-				analytics.TrackEvent("Cluster Apply Completed", props)
+				return fmt.Errorf("failed to apply cluster: %w", err)
+			}
+
+			duration := time.Since(start)
+			props := event.Properties{
+				"duration": duration.Seconds(),
 			}
+			analytics.TrackEvent("Cluster Apply Completed", props)
 
-			return
+			return nil
 		},
 	}
 }
diff --git a/cmd/client_config.go b/cmd/client_config.go
@@ -1,6 +1,7 @@
 package cmd
 
 import (
+	"fmt"
 	"os"
 	"strings"
 
@@ -36,17 +37,17 @@ func NewClientConfigCommand() *cli.Command {
 		Action: func(ctx *cli.Context) error {
 			product, err := config.ProductFromFile(ctx.String("config"))
 			if err != nil {
-				return err
+				return fmt.Errorf("failed to read product configuration: %w", err)
 			}
 
-			err = product.ClientConfig()
-			if err != nil {
+			if err := product.ClientConfig(); err != nil {
 				analytics.TrackEvent("Client configuration download Failed", nil)
-			} else {
-				analytics.TrackEvent("Client configuration download Completed", nil)
+				return fmt.Errorf("failed to download client configuration: %w", err)
 			}
 
-			return err
+			analytics.TrackEvent("Client configuration download Completed", nil)
+
+			return nil
 		},
 	}
 }

diff --git a/cmd/common.go b/cmd/common.go
@@ -12,10 +12,9 @@ import (
 	"github.com/Mirantis/mcc/pkg/product/mke/phase"
 	"github.com/Mirantis/mcc/pkg/util"
 	"github.com/Mirantis/mcc/version"
-	"github.com/mitchellh/go-homedir"
-
 	"github.com/k0sproject/rig"
 	"github.com/k0sproject/rig/exec"
+	"github.com/mitchellh/go-homedir"
 	log "github.com/sirupsen/logrus"
 	"github.com/urfave/cli/v2"
 )
@@ -163,26 +162,27 @@ func initExec(ctx *cli.Context) error {
 
 func checkLicense(ctx *cli.Context) error {
 	if !ctx.Bool("accept-license") {
-		return analytics.RequireRegisteredUser()
+		if err := analytics.RequireRegisteredUser(); err != nil {
+			return fmt.Errorf("error while checking license agreement: %w", err)
+		}
 	}
 	return nil
 }
 
 func addFileLogger(clusterName, filename string) (*os.File, error) {
 	home, err := homedir.Dir()
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to get user home directory: %w", err)
 	}
 
 	clusterDir := path.Join(home, constant.StateBaseDir, "cluster", clusterName)
 	if err := util.EnsureDir(clusterDir); err != nil {
 		return nil, fmt.Errorf("error while creating directory for logs: %w", err)
 	}
 	logFileName := path.Join(clusterDir, filename)
-	logFile, err := os.OpenFile(logFileName, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0600)
-
+	logFile, err := os.OpenFile(logFileName, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0o600)
 	if err != nil {
-		return nil, fmt.Errorf("Failed to create log file at %s: %s", logFileName, err.Error())
+		return nil, fmt.Errorf("failed to create log file at %s: %w", logFileName, err)
 	}
 
 	// Send all logs to named file, this ensures we always have debug logs also available when needed.

diff --git a/cmd/describe.go b/cmd/describe.go
@@ -1,16 +1,16 @@
 package cmd
 
 import (
+	"errors"
 	"fmt"
 	"strings"
 	"time"
 
 	"github.com/Mirantis/mcc/pkg/analytics"
 	"github.com/Mirantis/mcc/pkg/config"
+	log "github.com/sirupsen/logrus"
 	"github.com/urfave/cli/v2"
 	event "gopkg.in/segmentio/analytics-go.v3"
-
-	log "github.com/sirupsen/logrus"
 )
 
 var reports = []string{"hosts", "mke", "msr", "config"}
@@ -24,6 +24,8 @@ func reportIsKnown(n string) bool {
 	return false
 }
 
+var errInvalidReport = errors.New("invalid report")
+
 // NewDescribeCommand creates new describe command to be called from cli.
 func NewDescribeCommand() *cli.Command {
 	return &cli.Command{
@@ -40,10 +42,10 @@ func NewDescribeCommand() *cli.Command {
 		Action: func(ctx *cli.Context) error {
 			report := ctx.Args().First()
 			if report == "" {
-				return fmt.Errorf("missing report name argument")
+				return fmt.Errorf("%w: missing report name", errInvalidReport)
 			}
 			if !reportIsKnown(report) {
-				return fmt.Errorf("unknown report %s - must be one of %s", report, strings.Join(reports, ","))
+				return fmt.Errorf("%w: unknown report %s - must be one of %s", errInvalidReport, report, strings.Join(reports, ","))
 			}
 
 			if !(ctx.Bool("debug") || ctx.Bool("trace")) {
@@ -55,21 +57,20 @@ func NewDescribeCommand() *cli.Command {
 
 			product, err := config.ProductFromFile(ctx.String("config"))
 			if err != nil {
-				return err
+				return fmt.Errorf("failed to load product config: %w", err)
 			}
 
 			err = product.Describe(ctx.Args().First())
-
 			if err != nil {
 				analytics.TrackEvent("Cluster Describe Failed", nil)
-			} else {
-				duration := time.Since(start)
-				props := event.Properties{
-					"duration": duration.Seconds(),
-				}
-				analytics.TrackEvent("Cluster Describe Completed", props)
+				return fmt.Errorf("failed to describe cluster: %w", err)
+			}
+			duration := time.Since(start)
+			props := event.Properties{
+				"duration": duration.Seconds(),
 			}
-			return err
+			analytics.TrackEvent("Cluster Describe Completed", props)
+			return nil
 		},
 	}
 }