Apply manual fixes to appease linters

Drop enabling govets shadow because it's too annoying

Author: mmlb

.golangci.yml

@@ -15,12 +15,9 @@ linters-settings:
       - whyNoLint
       - wrapperFunc
   gocyclo:
-    min-complexity: 15
+    min-complexity: 31
   gofumpt:
     extra-rules: true
-  govet:
-    enable:
-      - shadow
   lll:
     line-length: 140
   misspell:

bmc/firmware.go

@@ -7,7 +7,6 @@ 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"
@@ -493,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
@@ -504,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 {
@@ -534,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)

bmc/inventory_test.go

@@ -6,7 +6,6 @@ import (
 	"time"
 
 	"github.com/bmc-toolbox/bmclib/v2/errors"
-	bmclibErrs "github.com/bmc-toolbox/bmclib/v2/errors"
 	"github.com/bmc-toolbox/common"
 	"github.com/stretchr/testify/assert"
 )
@@ -73,7 +72,7 @@ func TestInventoryFromInterfaces(t *testing.T) {
 		badImplementation  bool
 	}{
 		{"success with metadata", &common.Device{Common: common.Common{Vendor: "foo"}}, nil, 5 * time.Second, "foo", 1, false},
-		{"failure with bad implementation", nil, bmclibErrs.ErrProviderImplementation, 5 * time.Second, "foo", 1, true},
+		{"failure with bad implementation", nil, errors.ErrProviderImplementation, 5 * time.Second, "foo", 1, true},
 	}
 
 	for _, tc := range testCases {

client.go

@@ -148,12 +148,20 @@ func (c *Client) defaultTimeout(ctx context.Context) time.Duration {
 	return time.Until(deadline) / time.Duration(l)
 }
 
+func cloneHTTPClient(old *http.Client) *http.Client {
+	client := *old
+	transport, ok := client.Transport.(*http.Transport)
+	if !ok {
+		panic(fmt.Errorf("expected an http client of type http.Transport, got %T instead", client.Transport))
+	}
+	client.Transport = transport.Clone()
+	return &client
+}
+
 func (c *Client) registerRPCProvider() error {
 	driverRPC := rpc.New(c.providerConfig.rpc.ConsumerURL, c.Auth.Host, c.providerConfig.rpc.Opts.HMAC.Secrets)
 	c.providerConfig.rpc.Logger = c.Logger
-	httpClient := *c.httpClient
-	httpClient.Transport = c.httpClient.Transport.(*http.Transport).Clone()
-	c.providerConfig.rpc.HTTPClient = &httpClient
+	c.providerConfig.rpc.HTTPClient = cloneHTTPClient(c.httpClient)
 	if err := mergo.Merge(driverRPC, c.providerConfig.rpc, mergo.WithOverride, mergo.WithTransformers(&rpc.Provider{})); err != nil {
 		return fmt.Errorf("failed to merge user specified rpc config with the config defaults, rpc provider not available: %w", err)
 	}
@@ -183,18 +191,15 @@ func (c *Client) registerIPMIProvider() error {
 
 // register ASRR vendorapi provider
 func (c *Client) registerASRRProvider() {
-	asrHttpClient := *c.httpClient
-	asrHttpClient.Transport = c.httpClient.Transport.(*http.Transport).Clone()
-	driverAsrockrack := asrockrack.NewWithOptions(c.Auth.Host+":"+c.providerConfig.asrock.Port, c.Auth.User, c.Auth.Pass, c.Logger, asrockrack.WithHTTPClient(&asrHttpClient))
+	asrHttpClient := cloneHTTPClient(c.httpClient)
+	driverAsrockrack := asrockrack.NewWithOptions(c.Auth.Host+":"+c.providerConfig.asrock.Port, c.Auth.User, c.Auth.Pass, c.Logger, asrockrack.WithHTTPClient(asrHttpClient))
 	c.Registry.Register(asrockrack.ProviderName, asrockrack.ProviderProtocol, asrockrack.Features, nil, driverAsrockrack)
 }
 
 // register gofish provider
 func (c *Client) registerGofishProvider() {
-	gfHttpClient := *c.httpClient
-	gfHttpClient.Transport = c.httpClient.Transport.(*http.Transport).Clone()
 	gofishOpts := []redfish.Option{
-		redfish.WithHttpClient(&gfHttpClient),
+		redfish.WithHttpClient(cloneHTTPClient(c.httpClient)),
 		redfish.WithVersionsNotCompatible(c.providerConfig.gofish.VersionsNotCompatible),
 		redfish.WithUseBasicAuth(c.providerConfig.gofish.UseBasicAuth),
 		redfish.WithPort(c.providerConfig.gofish.Port),
@@ -233,29 +238,25 @@ func (c *Client) registerDellProvider() {
 
 // register supermicro vendorapi provider
 func (c *Client) registerSupermicroProvider() {
-	smcHttpClient := *c.httpClient
-	smcHttpClient.Transport = c.httpClient.Transport.(*http.Transport).Clone()
 	driverSupermicro := supermicro.NewClient(
 		c.Auth.Host,
 		c.Auth.User,
 		c.Auth.Pass,
 		c.Logger,
-		supermicro.WithHttpClient(&smcHttpClient),
+		supermicro.WithHttpClient(cloneHTTPClient(c.httpClient)),
 		supermicro.WithPort(c.providerConfig.supermicro.Port),
 	)
 
 	c.Registry.Register(supermicro.ProviderName, supermicro.ProviderProtocol, supermicro.Features, nil, driverSupermicro)
 }
 
 func (c *Client) registerOpenBMCProvider() {
-	httpClient := *c.httpClient
-	httpClient.Transport = c.httpClient.Transport.(*http.Transport).Clone()
 	driver := openbmc.New(
 		c.Auth.Host,
 		c.Auth.User,
 		c.Auth.Pass,
 		c.Logger,
-		openbmc.WithHttpClient(&httpClient),
+		openbmc.WithHttpClient(cloneHTTPClient(c.httpClient)),
 		openbmc.WithPort(c.providerConfig.openbmc.Port),
 	)
 
@@ -371,7 +372,7 @@ func (c *Client) Open(ctx context.Context) error {
 }
 
 // Close pass through to library function
-func (c *Client) Close(ctx context.Context) (err error) {
+func (c *Client) Close(ctx context.Context) (err error) { // nolint:contextcheck
 	ctx, span := c.traceprovider.Tracer(pkgName).Start(ctx, "Close")
 	defer span.End()
 

client_test.go

@@ -138,6 +138,7 @@ func TestWithConnectionTimeout(t *testing.T) {
 }
 
 func TestDefaultTimeout(t *testing.T) {
+	// nolint:containedctx
 	tests := map[string]struct {
 		ctx  context.Context
 		want func(n int) time.Duration
@@ -209,9 +210,9 @@ func (t *testProvider) BootDeviceSet(ctx context.Context, bootDevice string, set
 }
 
 func registryNames(r []*registrar.Driver) []string {
-	var names []string
-	for _, d := range r {
-		names = append(names, d.Name)
+	names := make([]string, len(r))
+	for i := range r {
+		names[i] = r[i].Name
 	}
 	return names
 }

internal/httpclient/httpclient_test.go

@@ -2,6 +2,7 @@ package httpclient
 
 import (
 	"crypto/x509"
+	"errors"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
@@ -52,17 +53,17 @@ func TestBuildWithOptions(t *testing.T) {
 				opts = append(opts, SecureTLSOption(tc.withCertPool(server.Certificate())))
 			}
 			client := Build(opts...)
-			req, _ := http.NewRequest(http.MethodGet, server.URL, nil)
+			req, _ := http.NewRequest(http.MethodGet, server.URL, http.NoBody) // nolint:noctx
 
-			_, err := client.Do(req)
+			resp, err := client.Do(req)
 			if tc.wantErr {
 				if err == nil {
 					t.Fatal("Missing expected error")
 				}
 
 				// Different versions of Go return different error messages so we just
 				// check that its a *url.Error{}
-				if _, ok := err.(*url.Error); !ok {
+				if errors.Is(err, &url.Error{}) {
 					t.Fatalf("Missing expected error: got %T: '%s'", err, err)
 				}
 				return
@@ -71,6 +72,8 @@ func TestBuildWithOptions(t *testing.T) {
 			if err != nil {
 				t.Fatalf("Got unexpected error %s", err)
 			}
+
+			resp.Body.Close()
 		})
 	}
 }

internal/redfishwrapper/bios_test.go

@@ -31,13 +31,13 @@ func biosConfigFromFixture(t *testing.T) map[string]string {
 	}
 
 	var bios map[string]any
-	err = json.Unmarshal([]byte(b), &bios)
+	err = json.Unmarshal(b, &bios)
 	if err != nil {
 		t.Fatalf("%s, failed to unmarshal fixture: %s", err.Error(), fixturePath)
 	}
 
 	expectedBiosConfig := make(map[string]string)
-	for k, v := range bios["Attributes"].(map[string]any) {
+	for k, v := range bios["Attributes"].(map[string]any) { // nolint:forcetypeassert
 		expectedBiosConfig[k] = fmt.Sprintf("%v", v)
 	}
 

internal/redfishwrapper/client.go

@@ -250,13 +250,13 @@ func redfishVersionMeetsOrExceeds(version string, major, minor, patch int) bool
 		return false
 	}
 
-	var rfVer []int64
-	for _, part := range parts {
-		ver, err := strconv.ParseInt(part, 10, 32)
+	rfVer := make([]int64, 3)
+	for i := range parts {
+		ver, err := strconv.ParseInt(parts[i], 10, 32)
 		if err != nil {
 			return false
 		}
-		rfVer = append(rfVer, ver)
+		rfVer[i] = ver
 	}
 
 	if rfVer[0] < int64(major) {

internal/redfishwrapper/firmware.go

@@ -317,39 +317,39 @@ func (p pipeReaderFakeSeeker) Seek(offset int64, whence int) (int64, error) {
 //
 // It creates a temporary form without reading in the update file payload and returns
 // sizeOf(form) + sizeOf(update file)
-func multipartPayloadSize(payload *multipartPayload) (int64, *bytes.Buffer, error) {
+func multipartPayloadSize(payload *multipartPayload) (int64, error) {
 	body := &bytes.Buffer{}
 	form := multipart.NewWriter(body)
 
 	// Add UpdateParameters field part
 	part, err := updateParametersFormField("UpdateParameters", form)
 	if err != nil {
-		return 0, body, err
+		return 0, err
 	}
 
 	if _, err = io.Copy(part, bytes.NewReader(payload.updateParameters)); err != nil {
-		return 0, body, err
+		return 0, err
 	}
 
 	// Add updateFile form
 	_, err = form.CreateFormFile("UpdateFile", filepath.Base(payload.updateFile.Name()))
 	if err != nil {
-		return 0, body, err
+		return 0, err
 	}
 
 	// determine update file size
 	finfo, err := payload.updateFile.Stat()
 	if err != nil {
-		return 0, body, err
+		return 0, err
 	}
 
 	// add terminating boundary to multipart form
 	err = form.Close()
 	if err != nil {
-		return 0, body, err
+		return 0, err
 	}
 
-	return int64(body.Len()) + finfo.Size(), body, nil
+	return int64(body.Len()) + finfo.Size(), nil
 }
 
 // runRequestWithMultipartPayload is a copy of https://github.com/stmcginnis/gofish/blob/main/client.go#L349
@@ -385,7 +385,7 @@ func (c *Client) runRequestWithMultipartPayload(url string, payload *multipartPa
 	//
 	// Without the content-length header the http client will set the Transfer-Encoding to 'chunked'
 	// and that does not work for some BMCs (iDracs).
-	contentLength, _, err := multipartPayloadSize(payload)
+	contentLength, err := multipartPayloadSize(payload)
 	if err != nil {
 		return nil, errors.Wrap(err, "error determining multipart payload size")
 	}

internal/redfishwrapper/firmware_test.go

@@ -115,13 +115,14 @@ func TestRunRequestWithMultipartPayload(t *testing.T) {
 				t.Fatal(err)
 			}
 
-			_, err = client.runRequestWithMultipartPayload(tc.updateURI, tc.payload)
+			resp, err := client.runRequestWithMultipartPayload(tc.updateURI, tc.payload)
 			if tc.err != nil {
 				assert.ErrorContains(t, err, tc.err.Error())
 				return
 			}
 
 			assert.Nil(t, err)
+			resp.Body.Close()
 			client.Close(context.Background())
 		})
 	}
@@ -392,7 +393,7 @@ func TestMultipartPayloadSize(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.testName, func(t *testing.T) {
-			gotSize, _, err := multipartPayloadSize(tc.payload)
+			gotSize, err := multipartPayloadSize(tc.payload)
 			if tc.errorMsg != "" {
 				assert.Contains(t, err.Error(), tc.errorMsg)
 			}