Tooling updates + add Proxy support to httpclient.DefaultTransport

.github/workflows/ci.yaml

@@ -19,7 +19,7 @@ jobs:
       uses: golangci/golangci-lint-action@v3
       with:
         args: -v --config .golangci.yml --timeout=5m
-        version: latest
+        version: v1.64
     - name: make all-checks
       run: make all-checks
   test:

.golangci.yml

@@ -1,125 +1,95 @@
-  govet:
-    auto-fix: true
-    linters-settings:
-    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
+#
+# This file originates from github.com/metal-toolbox/golangci-lint-config repo.
+#
+
+linters-settings:
   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:
-    auto-fix: true
     enabled-tags:
+      - experimental
       - performance
       - style
-      - experimental
     disabled-checks:
+      - whyNoLint
       - wrapperFunc
+  gocyclo:
+    min-complexity: 31
   gofumpt:
     extra-rules: true
-    auto-fix: true
-  wsl:
-    auto-fix: true
-  stylecheck:
-    auto-fix: true
+  lll:
+    line-length: 140
+  misspell:
+    locale: US
+  revive:
+    confidence: 0
 
 linters:
-  enable:
-    - errcheck
-    - gosimple
-    - govet
-    - gofmt
-    - gocyclo
-    - ineffassign
-    - stylecheck
-    - deadcode
-    - staticcheck
-    - structcheck
-    - unused
-    - prealloc
-    - typecheck
-    - varcheck
-    # additional linters
-    - bodyclose
-    - gocritic
-    - whitespace
+  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:
+    - canonicalheader # some bmcs may care about header case (against http spec)
+    - copyloopvar # requires go >=1.22
+    - cyclop
+    - depguard
+    - err113 # todo(*maybe* enable in future PR)
+    - exhaustruct
+    - forbidigo
+    - funlen
+    - gochecknoglobals
+    - gochecknoinits
+    - gocognit
+    - goconst
+    - gocritic # todo(enable in future PR)
+    - godot
+    - godox
+    - gosec # todo(enable in future PR)
+    - inamedparam
+    - interfacebloat
+    - intrange # requires go >=1.22
+    - ireturn # should be enabled, ironlib needs some changes
+    - lll # not previously enabled, ironlib and mctl both fail this
+    - mnd
+    - nestif
+    - nilnil
+    - nlreturn
+    - nolintlint
+    - nonamedreturns # should be enabled, probably
+    - paralleltest
+    - perfsprint
+    - revive # todo(enable in future PR)
+    - tagliatelle
+    - tenv # deprecated (since v1.64.0) due to: Duplicate feature in another linter. Replaced by usetesting.
+    - testpackage
+    - thelper # should be enabled
+    - varnamelen
+    - wrapcheck
     - wsl
-    - goimports
-    - golint
-    - misspell
-    - goerr113
-    - noctx
-  enable-all: false
-  disable-all: true
-
-run:
-  skip-dirs:
-
 
 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"
+      text: ST(1003|1016)
+    - path: bmc/(firmware|sel|user).go
+      linters:
+        - dupl
+    - path: bmc/(firmware|power|reset)_test.go
+      linters:
+        - dupl
+    - path: internal/redfishwrapper/client_test.go
+      linters:
+        - dupl
+    - path: providers/supermicro/firmware_bios_test.go
+      linters:
+        - dupl
+    - path: providers/supermicro/x11_firmware_bmc_test.go
+      linters:
+        - dupl
+    - path: providers/supermicro/x11_firmware_(bios|bmc).go
+      linters:
+        - dupl

bmc/bios.go

@@ -55,7 +55,6 @@ Loop:
 				err = multierror.Append(err, errors.WithMessagef(vErr, "provider: %v", elem.name))
 				err = multierror.Append(err, vErr)
 				continue
-
 			}
 			metadata.SuccessfulProvider = elem.name
 			return biosConfig, metadata, nil
@@ -83,7 +82,6 @@ Loop:
 				err = multierror.Append(err, errors.WithMessagef(vErr, "provider: %v", elem.name))
 				err = multierror.Append(err, vErr)
 				continue
-
 			}
 			metadata.SuccessfulProvider = elem.name
 			return metadata, nil
@@ -111,7 +109,6 @@ Loop:
 				err = multierror.Append(err, errors.WithMessagef(vErr, "provider: %v", elem.name))
 				err = multierror.Append(err, vErr)
 				continue
-
 			}
 			metadata.SuccessfulProvider = elem.name
 			return metadata, nil
@@ -139,7 +136,6 @@ Loop:
 				err = multierror.Append(err, errors.WithMessagef(vErr, "provider: %v", elem.name))
 				err = multierror.Append(err, vErr)
 				continue
-
 			}
 			metadata.SuccessfulProvider = elem.name
 			return metadata, nil

bmc/boot_device_test.go

@@ -3,13 +3,14 @@ package bmc
 import (
 	"context"
 	"errors"
+	"fmt"
 	"testing"
 	"time"
 
-	"fmt"
 	"github.com/google/go-cmp/cmp"
 	"github.com/hashicorp/go-multierror"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 type bootDeviceTester struct {
@@ -236,9 +237,9 @@ func TestBootDeviceOverrideGet(t *testing.T) {
 			override, metadata, err := GetBootDeviceOverrideFromInterface(ctx, 0, testCase.getters)
 
 			if testCase.expectedErrorMsg != "" {
-				assert.ErrorContains(t, err, testCase.expectedErrorMsg)
+				require.ErrorContains(t, err, testCase.expectedErrorMsg)
 			} else {
-				assert.Nil(t, err)
+				require.NoError(t, err)
 			}
 			assert.Equal(t, testCase.expectedOverride, override)
 			assert.Equal(t, testCase.expectedMetadata, &metadata)

bmc/connection.go

@@ -40,13 +40,13 @@ func OpenConnectionFromInterfaces(ctx context.Context, timeout time.Duration, pr
 	}
 
 	// Create a context with the specified timeout. This is done for backward compatibility but
-	// we should consider removing the timeout parameter alltogether given the context will
+	// we should consider removing the timeout parameter altogether given the context will
 	// container the timeout.
 	ctx, cancel := context.WithTimeout(ctx, timeout)
 	defer cancel()
 
 	// result facilitates communication of data between the concurrent opener goroutines and
-	// the the parent goroutine.
+	// the parent goroutine.
 	type result struct {
 		ProviderName string
 		Opener       Opener
@@ -135,7 +135,7 @@ func closeConnection(ctx context.Context, c []connectionProviders) (metadata Met
 	return metadata, multierror.Append(err, errors.New("failed to close connection"))
 }
 
-// CloseConnectionFromInterfaces identifies implementations of the Closer() interface and and passes the found implementations to the closeConnection() wrapper
+// CloseConnectionFromInterfaces identifies implementations of the Closer() interface and passes the found implementations to the closeConnection() wrapper
 func CloseConnectionFromInterfaces(ctx context.Context, generic []interface{}) (metadata Metadata, err error) {
 	metadata = newMetadata()
 

bmc/firmware.go

@@ -7,9 +7,7 @@ import (
 	"os"
 
 	"github.com/bmc-toolbox/bmclib/v2/constants"
-	bconsts "github.com/bmc-toolbox/bmclib/v2/constants"
 	bmclibErrs "github.com/bmc-toolbox/bmclib/v2/errors"
-
 	"github.com/hashicorp/go-multierror"
 	"github.com/pkg/errors"
 )
@@ -26,7 +24,7 @@ type FirmwareInstaller interface {
 	//
 	// return values:
 	// taskID - A taskID is returned if the update process on the BMC returns an identifier for the update process.
-	FirmwareInstall(ctx context.Context, component string, operationApplyTime string, forceInstall bool, reader io.Reader) (taskID string, err error)
+	FirmwareInstall(ctx context.Context, component, operationApplyTime string, forceInstall bool, reader io.Reader) (taskID string, err error)
 }
 
 // firmwareInstallerProvider is an internal struct to correlate an implementation/provider and its name
@@ -55,7 +53,6 @@ func firmwareInstall(ctx context.Context, component, operationApplyTime string,
 				err = multierror.Append(err, errors.WithMessagef(vErr, "provider: %v", elem.name))
 				metadata.FailedProviderDetail[elem.name] = err.Error()
 				continue
-
 			}
 			metadata.SuccessfulProvider = elem.name
 			return taskID, metadata, nil
@@ -97,7 +94,7 @@ func FirmwareInstallFromInterfaces(ctx context.Context, component, operationAppl
 	return firmwareInstall(ctx, component, operationApplyTime, forceInstall, reader, implementations)
 }
 
-// Note: this interface is to be deprecated in favour of a more generic FirmwareTaskVerifier.
+// Note: this interface is to be deprecated in favor of a more generic FirmwareTaskVerifier.
 //
 // FirmwareInstallVerifier defines an interface to check firmware install status
 type FirmwareInstallVerifier interface {
@@ -139,7 +136,6 @@ func firmwareInstallStatus(ctx context.Context, installVersion, component, taskI
 				err = multierror.Append(err, errors.WithMessagef(vErr, "provider: %v", elem.name))
 				metadata.FailedProviderDetail[elem.name] = err.Error()
 				continue
-
 			}
 			metadata.SuccessfulProvider = elem.name
 			return status, metadata, nil
@@ -299,7 +295,6 @@ func firmwareInstallUploaded(ctx context.Context, component, uploadTaskID string
 				err = multierror.Append(err, errors.WithMessagef(vErr, "provider: %v", elem.name))
 				metadata.FailedProviderDetail[elem.name] = err.Error()
 				continue
-
 			}
 			metadata.SuccessfulProvider = elem.name
 			return installTaskID, metadata, nil
@@ -402,7 +397,6 @@ func firmwareInstallSteps(ctx context.Context, component string, generic []firmw
 				err = multierror.Append(err, errors.WithMessagef(vErr, "provider: %v", elem.name))
 				metadata.FailedProviderDetail[elem.name] = err.Error()
 				continue
-
 			}
 			metadata.SuccessfulProvider = elem.name
 			return steps, metadata, nil
@@ -473,7 +467,6 @@ func firmwareUpload(ctx context.Context, component string, file *os.File, generi
 				err = multierror.Append(err, errors.WithMessagef(vErr, "provider: %v", elem.name))
 				metadata.FailedProviderDetail[elem.name] = err.Error()
 				continue
-
 			}
 			metadata.SuccessfulProvider = elem.name
 			return taskID, metadata, nil
@@ -499,7 +492,7 @@ type FirmwareTaskVerifier interface {
 	// return values:
 	// state - returns one of the FirmwareTask statuses (see devices/constants.go).
 	// status - returns firmware task progress or other arbitrary task information.
-	FirmwareTaskStatus(ctx context.Context, kind bconsts.FirmwareInstallStep, component, taskID, installVersion string) (state constants.TaskState, status string, err error)
+	FirmwareTaskStatus(ctx context.Context, kind constants.FirmwareInstallStep, component, taskID, installVersion string) (state constants.TaskState, status string, err error)
 }
 
 // firmwareTaskVerifierProvider is an internal struct to correlate an implementation/provider and its name
@@ -510,7 +503,7 @@ type firmwareTaskVerifierProvider struct {
 
 // firmwareTaskStatus returns the status of the firmware upload process.
 
-func firmwareTaskStatus(ctx context.Context, kind bconsts.FirmwareInstallStep, component, taskID, installVersion string, generic []firmwareTaskVerifierProvider) (state constants.TaskState, status string, metadata Metadata, err error) {
+func firmwareTaskStatus(ctx context.Context, kind constants.FirmwareInstallStep, component, taskID, installVersion string, generic []firmwareTaskVerifierProvider) (state constants.TaskState, status string, metadata Metadata, err error) {
 	metadata = newMetadata()
 
 	for _, elem := range generic {
@@ -540,7 +533,7 @@ func firmwareTaskStatus(ctx context.Context, kind bconsts.FirmwareInstallStep, c
 }
 
 // FirmwareTaskStatusFromInterfaces identifies implementations of the FirmwareTaskVerifier interface and passes the found implementations to the firmwareTaskStatus() wrapper.
-func FirmwareTaskStatusFromInterfaces(ctx context.Context, kind bconsts.FirmwareInstallStep, component, taskID, installVersion string, generic []interface{}) (state constants.TaskState, status string, metadata Metadata, err error) {
+func FirmwareTaskStatusFromInterfaces(ctx context.Context, kind constants.FirmwareInstallStep, component, taskID, installVersion string, generic []interface{}) (state constants.TaskState, status string, metadata Metadata, err error) {
 	metadata = newMetadata()
 
 	implementations := make([]firmwareTaskVerifierProvider, 0)