Merge pull request #1394 from Nordix/add/golangci-lint-workflow-kashif

🌱 Add golangci-lint github workflow

.github/workflows/golangci-lint.yml

@@ -0,0 +1,35 @@
+name: golangci-lint
+
+on:
+  pull_request:
+    types: [opened, edited, synchronize, reopened]
+
+# Remove all permissions from GITHUB_TOKEN except metadata.
+permissions: {}
+
+jobs:
+  golangci:
+    name: lint
+    runs-on: ubuntu-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        working-directory:
+          - ""
+          - test
+          - apis
+          - pkg/hardwareutils
+    steps:
+      - uses: actions/checkout@8ade135a41bc03ea155e62e844d188df1ea18608 # v4.1.0
+      - name: Calculate go version
+        id: vars
+        run: echo "go_version=$(make go-version)" >> $GITHUB_OUTPUT
+      - name: Set up Go
+        uses: actions/setup-go@93397bea11091df50f3d7e59dc26a7711a8bcfbe # v4.1.0
+        with:
+          go-version: ${{ steps.vars.outputs.go_version }}
+      - name: golangci-lint-${{matrix.working-directory}}
+        uses: golangci/golangci-lint-action@3a919529898de77ec3da873e3063ca4b10e7f5cc # v3.7.0
+        with:
+          version: v1.54.2
+          working-directory: ${{matrix.working-directory}}

.golangci.yaml

@@ -0,0 +1,160 @@
+run:
+  deadline: 5m
+  skip-dirs:
+    - mock*
+  skip-files:
+    - "zz_generated.*\\.go$"
+    - ".*conversion.*\\.go$"
+linters:
+  disable-all: true
+  enable:
+    - asciicheck
+    - bodyclose
+    - dogsled
+    # - errcheck
+    # - errorlint
+    - exportloopref
+    # - goconst
+    # - gocritic
+    # - godot
+    # - gofmt
+    # - goimports
+    # - gosimple
+    # - govet
+    # - importas
+    # - gosec
+    # - ineffassign
+    # - misspell
+    # - nakedret
+    # - nilerr
+    # - noctx
+    # - nolintlint
+    # - prealloc
+    # - predeclared
+    # - revive
+    # - rowserrcheck
+    # - staticcheck
+    # - stylecheck
+    # - thelper
+    # - typecheck
+    # - unconvert
+    # - unused
+    # - whitespace
+  # Run with --fast=false for more extensive checks
+  fast: true
+
+linters-settings:
+  gosec:
+    go: "1.20"
+    severity: medium
+    confidence: medium
+    concurrency: 8
+  importas:
+    no-unaliased: true
+    alias:
+      # Kubernetes
+      - pkg: k8s.io/api/core/v1
+        alias: corev1
+      - pkg: k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1
+        alias: apiextensionsv1
+      - pkg: k8s.io/apimachinery/pkg/apis/meta/v1
+        alias: metav1
+      - pkg: k8s.io/apimachinery/pkg/api/errors
+        alias: k8serrors
+      - pkg: k8s.io/apimachinery/pkg/util/errors
+        alias: kerrors
+      - pkg: k8s.io/component-base/logs/api/v1
+        alias: logsv1
+      # Controller Runtime
+      - pkg: sigs.k8s.io/controller-runtime
+        alias: ctrl
+      # BMO
+      - pkg: github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1
+        alias: metal3api
+  nolintlint:
+    allow-unused: false
+    allow-leading-space: false
+    require-specific: true
+  staticcheck:
+    go: "1.20"
+  stylecheck:
+    go: "1.20"
+  gocritic:
+    enabled-tags:
+      - experimental
+    disabled-checks:
+      - appendAssign
+      - dupImport # https://github.com/go-critic/go-critic/issues/845
+      - evalOrder
+      - ifElseChain
+      - octalLiteral
+      - regexpSimplify
+      - sloppyReassign
+      - truncateCmp
+      - typeDefFirst
+      - unnamedResult
+      - unnecessaryDefer
+      - whyNoLint
+      - wrapperFunc
+  unused:
+    go: "1.20"
+issues:
+  exclude-rules:
+    - path: test/e2e
+      linters:
+        - gosec
+    - path: _test\.go
+      linters:
+        - unused
+    # Specific exclude rules for deprecated fields that are still part of the codebase.
+    # These should be removed as the referenced deprecated item is removed from the project.
+    - linters:
+        - staticcheck
+      text: "SA1019:"
+    # Disable linters for conversion
+    - linters:
+        - staticcheck
+      text: "SA1019:"
+      path: .*(api|types)\/.*\/conversion.*\.go$
+    # Dot imports for gomega or ginkgo are allowed
+    # within test files.
+    - path: _test\.go
+      text: should not use dot imports
+    - path: (test|e2e)/.*.go
+      text: should not use dot imports
+    - linters:
+        - revive
+      text: "exported: exported method .*\\.(Reconcile|SetupWithManager|SetupWebhookWithManager) should have comment or be unexported"
+    # Exclude some packages or code to require comments, for example test code, or fake clients.
+    - linters:
+        - revive
+      text: exported (method|function|type|const) (.+) should have comment or be unexported
+      source: (func|type).*Fake.*
+    - linters:
+        - revive
+      text: exported (method|function|type|const) (.+) should have comment or be unexported
+      path: fake_\.go
+    - linters:
+        - revive
+      text: exported (method|function|type|const) (.+) should have comment or be unexported
+      path: .*(api|types)\/.*\/conversion.*\.go$
+    - linters:
+        - revive
+      text: "var-naming: don't use underscores in Go names;"
+      path: .*(api|types)\/.*\/conversion.*\.go$
+    - linters:
+        - revive
+      text: "receiver-naming: receiver name"
+      path: .*(api|types)\/.*\/conversion.*\.go$
+    - linters:
+        - stylecheck
+      text: "ST1003: should not use underscores in Go names;"
+      path: .*(api|types)\/.*\/conversion.*\.go$
+    - linters:
+        - stylecheck
+      text: "ST1016: methods on the same type should have the same receiver name"
+      path: .*(api|types)\/.*\/conversion.*\.go$
+  include:
+    - EXC0002 # include "missing comments" issues from golangci-lint
+  max-issues-per-linter: 0
+  max-same-issues: 0

cmd/get-hardware-details/main.go

@@ -28,7 +28,7 @@ func main() {
 	ironicTrustedCAFile := os.Getenv("IRONIC_CACERT_FILE")
 	ironicInsecureStr := os.Getenv("IRONIC_INSECURE")
 	ironicInsecure := false
-	if strings.ToLower(ironicInsecureStr) == "true" {
+	if strings.EqualFold(ironicInsecureStr, "true") {
 		ironicInsecure = true
 	}
 

cmd/make-bm-worker/main.go

@@ -105,5 +105,4 @@ func main() {
 	}
 
 	fmt.Fprint(os.Stdout, result)
-
 }

cmd/make-bm-worker/templates/templates_test.go

@@ -6,6 +6,7 @@ import (
 )
 
 func compareStrings(t *testing.T, s1, s2 string) bool {
+	t.Helper()
 	if s1 == s2 {
 		return true
 	}

cmd/make-virt-host/main.go

@@ -29,26 +29,26 @@ const (
    </interface>
 */
 
-// MAC is a hardware address for a NIC
+// MAC is a hardware address for a NIC.
 type MAC struct {
 	XMLName xml.Name `xml:"mac"`
 	Address string   `xml:"address,attr"`
 }
 
-// Source is the network to which the interface is attached
+// Source is the network to which the interface is attached.
 type Source struct {
 	XMLName xml.Name `xml:"source"`
 	Bridge  string   `xml:"bridge,attr"`
 }
 
-// Interface is one NIC
+// Interface is one NIC.
 type Interface struct {
 	XMLName xml.Name `xml:"interface"`
 	MAC     MAC      `xml:"mac"`
 	Source  Source   `xml:"source"`
 }
 
-// Domain is the main tag for the XML document
+// Domain is the main tag for the XML document.
 type Domain struct {
 	Interfaces []Interface `xml:"devices>interface"`
 }
@@ -113,7 +113,7 @@ vbmc list -f json -c 'Domain name' -c Port
 */
 
 // VBMC holds the parameters for describing a virtual machine
-// controller
+// controller.
 type VBMC struct {
 	Port int    `json:"Port"`
 	Name string `json:"Domain name"`

controllers/metal3.io/action_result.go

@@ -8,12 +8,12 @@ import (
 
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
-	metal3 "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1"
+	metal3api "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1"
 	"github.com/metal3-io/baremetal-operator/pkg/provisioner"
 )
 
 // This is an upper limit for the ErrorCount, so that the max backoff
-// timeout will not exceed (roughly) 8 hours
+// timeout will not exceed (roughly) 8 hours.
 const maxBackOffCount = 9
 
 func init() {
@@ -57,7 +57,7 @@ func (r actionUpdate) Dirty() bool {
 }
 
 // actionDelayed it's the same of an actionUpdate, but the requeue time
-// is calculated using a fixed backoff with jitter
+// is calculated using a fixed backoff with jitter.
 type actionDelayed struct {
 	actionUpdate
 }
@@ -120,7 +120,7 @@ func (r actionError) NeedsRegistration() bool {
 // and that the resource should be marked as in error.
 type actionFailed struct {
 	dirty      bool
-	ErrorType  metal3.ErrorType
+	ErrorType  metal3api.ErrorType
 	errorCount int
 }
 
@@ -133,9 +133,8 @@ type actionFailed struct {
 // 6  [32m, 1h4m]
 // 7  [1h4m, 2h8m]
 // 8  [2h8m, 4h16m]
-// 9  [4h16m, 8h32m]
+// 9  [4h16m, 8h32m].
 func calculateBackoff(errorCount int) time.Duration {
-
 	if errorCount > maxBackOffCount {
 		errorCount = maxBackOffCount
 	}

controllers/metal3.io/action_result_test.go

@@ -9,19 +9,16 @@ import (
 )
 
 func TestBackoffIncrements(t *testing.T) {
-
 	var backOff time.Duration
 	for i := 1; i <= maxBackOffCount; i++ {
 		prev := backOff
 		backOff = calculateBackoff(i)
 
 		assert.GreaterOrEqual(t, backOff.Milliseconds(), prev.Milliseconds())
 	}
-
 }
 
 func TestMaxBackoffDuration(t *testing.T) {
-
 	maxBackOffDuration := (time.Minute * time.Duration(math.Exp2(float64(maxBackOffCount)))).Milliseconds()
 
 	assert.LessOrEqual(t, calculateBackoff(maxBackOffCount-1).Milliseconds(), maxBackOffDuration)