House/spring cleaning

.github/workflows/codeql-analysis.yml

@@ -40,6 +40,11 @@ jobs:
     - name: Checkout repository
       uses: actions/checkout@v4
 
+    - name: Install Go
+      uses: actions/setup-go@v4
+      with:
+        go-version-file: go.mod
+
     # Initializes the CodeQL tools for scanning.
     - name: Initialize CodeQL
       uses: github/codeql-action/init@v2
@@ -48,11 +53,11 @@ jobs:
         # If you wish to specify custom queries, you can do so here or in a config file.
         # By default, queries listed here will override any specified in a config file.
         # Prefix the list here with "+" to use these queries and those in the config file.
-        
+
         # Details on CodeQL's query packs refer to : https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-code-scanning#using-queries-in-ql-packs
         # queries: security-extended,security-and-quality
 
-        
+
     # Autobuild attempts to build any compiled languages  (C/C++, C#, or Java).
     # If this step fails, then you should remove it and run the build manually (see below)
     - name: Autobuild
@@ -61,7 +66,7 @@ jobs:
     # ℹ️ Command-line programs to run using the OS shell.
     # 📚 See https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepsrun
 
-    #   If the Autobuild fails above, remove it and uncomment the following three lines. 
+    #   If the Autobuild fails above, remove it and uncomment the following three lines.
     #   modify them (or add more) to build your code if your project, please refer to the EXAMPLE below for guidance.
 
     # - run: |

.github/workflows/push-pr-lint.yaml

@@ -5,19 +5,19 @@ jobs:
   lint-test:
     runs-on: ubuntu-latest
     steps:
+    - name: Checkout code
+      uses: actions/checkout@v4
+
     - name: Install Go
       uses: actions/setup-go@v4
       with:
-        go-version: '^1.19'
-
-    - name: Checkout code
-      uses: actions/checkout@v4
+        go-version-file: go.mod
 
     - name: golangci-lint
       uses: golangci/golangci-lint-action@v3
       with:
         args: --config .golangci.yml
-        version: v1.55.2
+        version: v1.57
 
     - name: Test
       run: go test ./...

.golangci.yml

@@ -1,123 +1,93 @@
+#
+# This file lives in the github.com/metal-toolbox/golangci-lint-config repo.
+#
+# Do not edit this file outside of this repo otherwise we will be grumpy.
+# Seriously though, this is meant to help promote a "standard" config and coding style.
+# If you don't like something, lets have a discussion in GitHub issues!
+#
+
 linters-settings:
-  govet:
-    # TODO: enable and fix struct alignments
-    #enable:
-    #  - fieldalignment
-    check-shadowing: true
-    settings:
-      printf:
-        funcs:
-          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Infof
-          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Warnf
-          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Errorf
-          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Fatalf
-  golint:
-    min-confidence: 0
-  gocyclo:
-    min-complexity: 10
-  maligned:
-    suggest-new: true
   dupl:
-    threshold: 100
+    threshold: 125
   goconst:
     min-len: 2
     min-occurrences: 2
-  depguard:
-    list-type: blacklist
-    packages:
-      # logging is allowed only by logutils.Log, logrus
-      # is allowed to use only in logutils package
-      - github.com/sirupsen/logrus
-  misspell:
-    locale: US
-    auto-fix: true
-  lll:
-    line-length: 140
-  goimports:
-    local-prefixes: github.com/golangci/golangci-lint
   gocritic:
     enabled-tags:
+      - experimental
       - performance
       - style
-      - experimental
     disabled-checks:
+      - whyNoLint
       - wrapperFunc
+  gocyclo:
+    min-complexity: 15
   gofumpt:
     extra-rules: true
+  govet:
+    enable:
+      - shadow
+  lll:
+    line-length: 140
+  misspell:
+    locale: US
+  revive:
+    confidence: 0
 
 linters:
-  enable:
-    - errcheck
-    - gosimple
-    - govet
-    - gofmt
-    - gocyclo
-    - ineffassign
-    - stylecheck
-    - misspell
-    - staticcheck
-    - unused
-    - prealloc
-    - typecheck
-      # XXX: add me back! - revive
-    # additional linters
-    - bodyclose
-    - gocritic
-    - goerr113
-    - goimports
+  enable-all: true
+  disable-all: false
+  # Linters we don't like
+  # Comments help explain why its disabled or point at ones we should not disable but will take a little work
+  # If its not commented its likely because its just too annoying or we don't find useful
+  disable:
+    - copyloopvar # requires go >=1.22
+    - cyclop
+    - deadcode # deprecated
+    - depguard
+    - errname # maybe should be enabled
+    - exhaustivestruct # deprecated
+    - exhaustruct
+    - forbidigo
+    - funlen
+    - gochecknoglobals
+    - gochecknoinits
+    - gocognit
+    - goconst
+    - godot
+    - godox
+    - golint # deprecated
     - gomnd
-    - misspell
-    - noctx
-    - stylecheck
-    - whitespace
-  enable-all: false
-  disable-all: true
-
-run:
-  build-tags:
-    - gingonic
-  skip-dirs:
-    - scripts
-    - docker
-    - samples
-  #modules-download-mode: vendor
+    - ifshort # deprecated
+    - inamedparam
+    - interfacebloat
+    - interfacer # deprecated
+    - intrange # requires go >=1.22
+    - ireturn # should be enabled, ironlib needs some changes
+    - lll # not previously enabled, ironlib and mctl both fail this
+    - maligned # deprecated
+    - nestif
+    - nilnil
+    - nlreturn
+    - nolintlint
+    - nonamedreturns # should be enabled, probably
+    - nosnakecase # deprecated
+    - paralleltest
+    - perfsprint
+    - scopelint # deprecated
+    - structcheck # deprecated
+    - tagliatelle
+    - tenv # should be enabled
+    - testpackage
+    - testifylint # should be enabled
+    - thelper # should be enabled
+    - varcheck # deprecated
+    - varnamelen
+    - wrapcheck
+    - wsl
 
 issues:
   exclude-rules:
-    - linters:
-        - gosec
-      text: "weak cryptographic primitive"
-
     - linters:
         - stylecheck
       text: "ST1016"
-  exclude:
-    # Default excludes from `golangci-lint run --help` with EXC0002 removed
-    # EXC0001 errcheck: Almost all programs ignore errors on these functions and in most cases it's ok
-    - Error return value of .((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*print(f|ln)?|os\.(Un)?Setenv). is not checked
-    # EXC0002 golint: Annoying issue about not having a comment. The rare codebase has such comments
-    # - (comment on exported (method|function|type|const)|should have( a package)? comment|comment should be of the form)
-    # EXC0003 golint: False positive when tests are defined in package 'test'
-    - func name will be used as test\.Test.* by other packages, and that stutters; consider calling this
-    # EXC0004 govet: Common false positives
-    - (possible misuse of unsafe.Pointer|should have signature)
-    # EXC0005 staticcheck: Developers tend to write in C-style with an explicit 'break' in a 'switch', so it's ok to ignore
-    - ineffective break statement. Did you mean to break out of the outer loop
-    # EXC0006 gosec: Too many false-positives on 'unsafe' usage
-    - Use of unsafe calls should be audited
-    # EXC0007 gosec: Too many false-positives for parametrized shell calls
-    - Subprocess launch(ed with variable|ing should be audited)
-    # EXC0008 gosec: Duplicated errcheck checks
-    - (G104|G307)
-    # EXC0009 gosec: Too many issues in popular repos
-    - (Expect directory permissions to be 0750 or less|Expect file permissions to be 0600 or less)
-    # EXC0010 gosec: False positive is triggered by 'src, err := ioutil.ReadFile(filename)'
-    - Potential file inclusion via variable
-exclude-use-default: false
-
-# golangci.com configuration
-# https://github.com/golangci/golangci/wiki/Configuration
-#service:
-#  golangci-lint-version: 1.15.x # use the fixed version to not introduce new linters unexpectedly
-#  prepare:
-#    - echo "here I can run custom commands, but no preparation needed for this repo"

Dockerfile

@@ -1,5 +1,5 @@
 # build ironlib wrapper binaries
-FROM golang:1.20-alpine AS stage0
+FROM golang:1.22-alpine AS stage0
 
 WORKDIR /workspace
 

Makefile

@@ -1,11 +1,8 @@
-LINTER_EXPECTED_VERSION   := "1.55.2"
-
 .DEFAULT_GOAL := help
 
 ## lint
 lint:
-	(golangci-lint --version | grep -q "${LINTER_EXPECTED_VERSION}" && golangci-lint run --config .golangci.yml) \
-		|| echo "expected linter version: ${LINTER_EXPECTED_VERSION}"
+	go run github.com/golangci/golangci-lint/cmd/golangci-lint@v1.57 run --config .golangci.yml
 
 ## Go test
 test: lint

actions/inventory.go

@@ -8,14 +8,13 @@ import (
 	"strings"
 
 	"github.com/bmc-toolbox/common"
+	"github.com/metal-toolbox/ironlib/firmware"
+	"github.com/metal-toolbox/ironlib/model"
+	"github.com/metal-toolbox/ironlib/utils"
 	"github.com/pkg/errors"
 	"github.com/r3labs/diff/v2"
 	"github.com/sirupsen/logrus"
 	"golang.org/x/exp/slices"
-
-	"github.com/metal-toolbox/ironlib/firmware"
-	"github.com/metal-toolbox/ironlib/model"
-	"github.com/metal-toolbox/ironlib/utils"
 )
 
 var (
@@ -877,6 +876,7 @@ func (a *InventoryCollectorAction) vetChanges(changes diff.Changelog) diff.Chang
 
 	for _, change := range changes {
 		// Skip changes that delete items
+		change := change
 		if a.acceptChange(&change) {
 			accepted = append(accepted, change)
 		}
@@ -904,28 +904,54 @@ func (a *InventoryCollectorAction) acceptChange(change *diff.Change) bool {
 func (a *InventoryCollectorAction) vetUpdate(change *diff.Change) bool {
 	// allow vendor, model field changes only if the older value was not defined
 	if slices.Contains(change.Path, "Vendor") || slices.Contains(change.Path, "Model") {
-		if strings.TrimSpace(change.To.(string)) != "" && strings.TrimSpace(change.From.(string)) == "" {
+		from, ok := change.From.(string)
+		if !ok {
+			return false
+		}
+		to, ok := change.To.(string)
+		if !ok {
+			return false
+		}
+		if strings.TrimSpace(to) != "" && strings.TrimSpace(from) == "" {
 			return true
 		}
 	}
 
 	// accept description if its longer than the older value
 	if slices.Contains(change.Path, "Description") {
-		if len(strings.TrimSpace(change.To.(string))) > len(strings.TrimSpace(change.From.(string))) {
+		from, ok := change.From.(string)
+		if !ok {
+			return false
+		}
+		to, ok := change.To.(string)
+		if !ok {
+			return false
+		}
+
+		if len(strings.TrimSpace(to)) > len(strings.TrimSpace(from)) {
 			return true
 		}
 	}
 
 	// keep product name if not empty
 	if slices.Contains(change.Path, "ProductName") {
-		if strings.TrimSpace(change.From.(string)) != "" {
+		from, ok := change.From.(string)
+		if !ok {
+			return false
+		}
+
+		if strings.TrimSpace(from) != "" {
 			return false
 		}
 	}
 
 	// keep existing serial value if not empty
 	if slices.Contains(change.Path, "Serial") {
-		if strings.TrimSpace(change.From.(string)) != "" {
+		from, ok := change.From.(string)
+		if !ok {
+			return false
+		}
+		if strings.TrimSpace(from) != "" {
 			return false
 		}
 	}

actions/storage_controller.go

@@ -12,9 +12,7 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
-var (
-	ErrVirtualDiskManagerUtilNotIdentified = errors.New("virtual disk management utility not identifed")
-)
+var ErrVirtualDiskManagerUtilNotIdentified = errors.New("virtual disk management utility not identifed")
 
 type StorageControllerAction struct {
 	Logger *logrus.Logger

examples/firmware-install/firmware-install.go

@@ -38,5 +38,4 @@ func main() {
 	if err != nil {
 		logger.Fatal(err)
 	}
-
 }