Enable linters

mt-sre · Oct 21, 2023 · 1a451ae · 1a451ae
1 parent 03809c6
commit 1a451ae
Show file tree

Hide file tree

Showing 17 changed files with 312 additions and 113 deletions.
diff --git a/.golangci.yaml b/.golangci.yaml
@@ -1,9 +1,166 @@
 linters:
+  disable-all: true
+  # One can't use disable and disable-all. disable-all is preferred to avoid lint failing when golangci gets updated and linters are added.
+  # at the same time but it is nice to explicitly state which linters we do not want.
+  #disable:
+  #- containedctx     # Context.Context within a struct a struct (as opposed to a func parameter) is nasty for many reasons, but actually sometimes useful.
+  #- contextcheck     # Not that useful/false positives.
+  #- cyclop           # Cyclo complexity already calculated by other linters.
+  #- depguard         # We do not restrict imports.
+  #- dogsled          # Could be nice in the future.
+  #- dupl             # Could be nice in the future.
+  #- exhaustive       # Not that useful/false positives.
+  #- exhaustruct      # Not that useful/false positives.
+  #- forcetypeassert  # We want a panic in situations when a type is asserted.
+  #- funlen           # We really like long functions.
+  #- gci              # Conflicts with other linters.
+  #- gochecknoglobals # We need globals.
+  #- gochecknoinits   # We have a lot of those.
+  #- gocognit         # Not that useful/false positives.
+  #- godox            # Not that useful/false positives.
+  #- gomnd            # The annoy is strong.
+  #- gomoddirectives  # Not that useful/false positives.
+  #- interfacebloat   # WE do bloaty interfaces.
+  #- inamedparam # Not that useful.
+  #- ireturn          # Not that useful/false positives.
+  #- musttag          # Dislikes our deps.
+  #- nakedret         # Naked return good return.
+  #- nlreturn         # Could be nice in the future.
+  #- nonamedreturns   # Named return good return.
+  #- rowserrcheck     # No SQL here.
+  #- sqlclosecheck    # No SQL here.
+  #- testpackage      # Could be nice in the future.
+  #- unparam          # Not that useful/false positives.
+  #- varnamelen       # Not that useful/false positives.
+  #- wrapcheck        # Not that useful/false positives.
+  #- wsl              # Not that useful/false positives.
+  #- zerologlint      # No zerolog here.
   enable:
+  - asasalint
+  - asciicheck
+  - bidichk
+  - bodyclose
+  - decorder
+  - dupword
+  - durationcheck
+  - errcheck
+  - errchkjson
+  - errname
+  - errorlint
+  - execinquery
+  - exportloopref
+  - forbidigo
+  - ginkgolinter
+  - gocheckcompilerdirectives
+  - gochecksumtype
+  - goconst
+  - gocritic
+  - gocyclo
+  - godot
+  - goerr113
+  - gofmt
+  - gofumpt
+  - goheader
+  - goimports
+  - gosmopolitan
+  - govet
+  - grouper
+  - importas
+  - ineffassign
+  - lll
+  - loggercheck
+  - maintidx
+  - makezero
+  - mirror
+  - misspell
+  - musttag
+  - nestif
+  - nilerr
   - nilnil
-  disable:
-  - scopelint # deprecated
-  - exhaustive # was creating false positives left and right
-  presets:
-  - bugs
+  - noctx
+  - nolintlint
+  - nosprintfhostport
+  - paralleltest
+  - perfsprint
+  - prealloc
+  - predeclared
+  - promlinter
+  - protogetter
+  - reassign
+  - revive
+  - sloglint
+  - staticcheck
+  - stylecheck
+  - tagalign
+  - tagliatelle
+  - tenv
+  - testableexamples
+  - testifylint
+  - thelper
+  - tparallel
+  - typecheck
+  - unconvert
   - unused
+  - usestdlibvars
+  - wastedassign
+  - whitespace
+issues:
+  max-same-issues: 0
+  max-issues-per-linter: 0
+  include:
+  - EXC0001 # Check errors returned by io.Closer.
+  - EXC0002 # Require docs on exported pkg.
+  - EXC0003 # Just don't name a test pkg `test`.
+  - EXC0004 # Unsafe pkg is scary.
+  - EXC0005 # No ineffectual breaks.
+  - EXC0006 # Unsafe pkg is scary.
+  #- EXC0007 # Launching a subprocess using variables is not that odd.
+  #- EXC0008 # Duplicated check.
+  #- EXC0009 # World readable files are actually not a CVE
+  #- EXC0010 # Reading a file with a non literal path is not a CVE either.
+  - EXC0011 # Docs.
+  - EXC0012 # Docs.
+  - EXC0013 # Docs.
+  - EXC0014 # Docs.
+  - EXC0015 # Docs.
+run:
+  skip-files: [ "zz_generated.deepcopy.go$" ]
+linters-settings:
+  goimports:
+    local-prefixes: github.com/mt-sre/devkube
+  importas:
+    no-unaliased: false
+    no-extra-aliases: true
+    alias:
+    - pkg: k8s\.io\/api\/([^\/]+)\/([^\/]+)
+      alias: $1$2
+    - pkg: k8s\.io\/apimachinery\/pkg\/([^\/]+)
+      alias: $1
+    - pkg: k8s\.io\/apimachinery\/pkg\/apis\/([^\/]+)\/([^\/]+)
+      alias: $1$2
+    - pkg: package-operator\.run\/apis
+      alias: apis
+    - pkg: package-operator\.run\/apis\/([^\/]+)\/([^\/]+)
+      alias: $1$2
+    - pkg: k8s\.io\/apiextensions-apiserver\/pkg\/apis\/([^\/]+)\/([^\/]+)
+      alias: $1$2
+    - pkg: github\.com\/openshift\/api\/([^\/]+)\/([^\/]+)
+      alias: $1$2
+    - pkg: package-operator.run/internal/controllers/hostedclusters/([^\/]+)\/([^\/]+)
+      alias: $1$2
+    - pkg: k8s\.io\/apimachinery\/pkg\/api\/([^\/]+)
+      alias: apimachinery$1
+    - pkg: sigs\.k8s\.io\/controller-runtime
+      alias: ctrl
+    - pkg: package-operator\.run\/internal\/cmd
+      alias: internalcmd
+    - pkg: package-operator\.run\/internal\/probing
+      alias: internalprobing
+    - pkg: github\.com\/google\/go-containerregistry\/pkg\/v1
+      alias: containerregistrypkgv1
+    - pkg: k8s\.io\/utils\/clock\/testing
+      alias: clocktesting
+    - pkg: k8s\.io\/apiserver\/pkg\/cel
+      alias: apiservercel
+    - pkg: k8s\.io\/apiserver\/pkg\/apis\/cel
+      alias: apiserverapiscel
diff --git a/dev/build.go b/dev/build.go
@@ -41,7 +41,7 @@ type ImagePushInfo struct {
 	DigestFile string
 }
 
-// execCommand is replaced with helper function when testing
+// execCommand is replaced with helper function when testing.
 var execCommand = exec.Command
 
 func execError(command []string, err error) error {
@@ -56,8 +56,7 @@ func newExecCmd(args []string, cacheDir string) *exec.Cmd {
 	return cmd
 }
 
-// BuildImage is a generic image build function,
-// requires the binaries to be built beforehand
+// BuildImage is a generic image build function, requires the binaries to be built beforehand.
 func BuildImage(buildInfo *ImageBuildInfo, deps []interface{}) error {
 	if len(deps) > 0 {
 		mg.SerialDeps(deps...)
@@ -85,7 +84,7 @@ func BuildImage(buildInfo *ImageBuildInfo, deps []interface{}) error {
 }
 
 // BuildPackage builds a package image using the package operator CLI,
-// requires `kubectl-package` command to be available on the system
+// requires `kubectl-package` command to be available on the system.
 func BuildPackage(buildInfo *PackageBuildInfo, deps []interface{}) error {
 	if len(deps) > 0 {
 		mg.SerialDeps(deps...)

diff --git a/dev/build_test.go b/dev/build_test.go
@@ -6,7 +6,7 @@ import (
 	"testing"
 
 	"github.com/magefile/mage/mg"
-	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 type buildImgTestCase struct {
@@ -97,7 +97,11 @@ var (
 		},
 		commands: [][]string{
 			{string(ContainerRuntimePodman), "push", "--digestfile=test_DigestFile", "test_ImageTag"},
-			{string(ContainerRuntimePodman), "login", "-u=" + os.Getenv("QUAY_USER"), "-p=" + os.Getenv("QUAY_TOKEN"), "quay.io"},
+			{
+				string(ContainerRuntimePodman), "login",
+				"-u=" + os.Getenv("QUAY_USER"),
+				"-p=" + os.Getenv("QUAY_TOKEN"), "quay.io",
+			},
 		},
 	}
 
@@ -115,10 +119,10 @@ var (
 		"podman":  &podmanPushCase,
 	}
 
-	// currentTestCase is used in TestXXXX_HelperProcess to identify which test ran it
+	// currentTestCase is used in TestXXXX_HelperProcess to identify which test ran it.
 	currentTestCase string
 
-	// helperProcess is used by mockExecCommand to determine which helper process to run
+	// helperProcess is used by mockExecCommand to determine which helper process to run.
 	helperProcess string
 )
 
@@ -161,6 +165,8 @@ func mockExecCommand(command string, args ...string) *exec.Cmd {
 }
 
 func TestBuildImage_HelperProcess(t *testing.T) {
+	t.Parallel()
+
 	if os.Getenv("GO_WANT_HELPER_PROCESS") != "1" {
 		return
 	}
@@ -173,19 +179,26 @@ func TestBuildImage_HelperProcess(t *testing.T) {
 }
 
 func TestBuildImage(t *testing.T) {
+	t.Parallel()
+
 	execCommand = mockExecCommand
 	defer func() { execCommand = exec.Command }()
 	helperProcess = buildImgHelper
 
-	for _, tc := range buildImgTestCases {
+	for i := range buildImgTestCases {
+		tc := buildImgTestCases[i]
 		currentTestCase = tc.name
 		t.Run(tc.name, func(t *testing.T) {
-			assert.NoError(t, BuildImage(&tc.buildInfo, []interface{}{}))
+			t.Parallel()
+
+			require.NoError(t, BuildImage(&tc.buildInfo, []interface{}{}))
 		})
 	}
 }
 
 func TestBuildPackage_HelperProcess(t *testing.T) {
+	t.Parallel()
+
 	if os.Getenv("GO_WANT_HELPER_PROCESS") != "1" {
 		return
 	}
@@ -198,19 +211,26 @@ func TestBuildPackage_HelperProcess(t *testing.T) {
 }
 
 func TestBuildPackage(t *testing.T) {
+	t.Parallel()
+
 	execCommand = mockExecCommand
 	defer func() { execCommand = exec.Command }()
 	helperProcess = buildPkgHelper
 
-	for _, tc := range buildPkgTestCases {
+	for i := range buildPkgTestCases {
+		tc := buildPkgTestCases[i]
 		currentTestCase = tc.name
 		t.Run(tc.name, func(t *testing.T) {
-			assert.NoError(t, BuildPackage(&tc.buildInfo, []interface{}{}))
+			t.Parallel()
+
+			require.NoError(t, BuildPackage(&tc.buildInfo, []interface{}{}))
 		})
 	}
 }
 
 func TestPushImage_HelperProcess(t *testing.T) {
+	t.Parallel()
+
 	if os.Getenv("GO_WANT_HELPER_PROCESS") != "1" {
 		return
 	}
@@ -223,14 +243,19 @@ func TestPushImage_HelperProcess(t *testing.T) {
 }
 
 func TestPushImage(t *testing.T) {
+	t.Parallel()
+
 	execCommand = mockExecCommand
 	defer func() { execCommand = exec.Command }()
 	helperProcess = pushHelper
 
-	for _, tc := range pushTestCases {
+	for i := range pushTestCases {
+		tc := pushTestCases[i]
 		currentTestCase = tc.name
 		t.Run(tc.name, func(t *testing.T) {
-			assert.NoError(t, PushImage(&tc.pushInfo, mg.F(func() {})))
+			t.Parallel()
+
+			require.NoError(t, PushImage(&tc.pushInfo, mg.F(func() {})))
 		})
 	}
 }