From 886d623d90d569c08d694ea1262517a4160da723 Mon Sep 17 00:00:00 2001 From: Manuel Mendez Date: Mon, 10 Mar 2025 21:07:13 -0400 Subject: [PATCH 1/8] examples: Ignore binaries --- examples/bios/.gitignore | 2 ++ examples/create-users/.gitignore | 2 ++ examples/floppy-image/.gitignore | 2 ++ examples/install-firmware/.gitignore | 2 ++ examples/inventory/.gitignore | 2 ++ examples/reset_bmc/.gitignore | 2 ++ examples/rpc/.gitignore | 2 ++ examples/screenshot/.gitignore | 2 ++ examples/sel/.gitignore | 2 ++ examples/status/.gitignore | 2 ++ examples/virtualmedia/.gitignore | 2 ++ 11 files changed, 22 insertions(+) create mode 100644 examples/bios/.gitignore create mode 100644 examples/create-users/.gitignore create mode 100644 examples/floppy-image/.gitignore create mode 100644 examples/install-firmware/.gitignore create mode 100644 examples/inventory/.gitignore create mode 100644 examples/reset_bmc/.gitignore create mode 100644 examples/rpc/.gitignore create mode 100644 examples/screenshot/.gitignore create mode 100644 examples/sel/.gitignore create mode 100644 examples/status/.gitignore create mode 100644 examples/virtualmedia/.gitignore diff --git a/examples/bios/.gitignore b/examples/bios/.gitignore new file mode 100644 index 00000000..fff980bf --- /dev/null +++ b/examples/bios/.gitignore @@ -0,0 +1,2 @@ +bios +bios.exe diff --git a/examples/create-users/.gitignore b/examples/create-users/.gitignore new file mode 100644 index 00000000..f8721c72 --- /dev/null +++ b/examples/create-users/.gitignore @@ -0,0 +1,2 @@ +create-users +create-users.exe diff --git a/examples/floppy-image/.gitignore b/examples/floppy-image/.gitignore new file mode 100644 index 00000000..214bcc89 --- /dev/null +++ b/examples/floppy-image/.gitignore @@ -0,0 +1,2 @@ +floppy-image +floppy-image.exe diff --git a/examples/install-firmware/.gitignore b/examples/install-firmware/.gitignore new file mode 100644 index 00000000..111e1454 --- /dev/null +++ b/examples/install-firmware/.gitignore @@ -0,0 +1,2 @@ +install-firmware +install-firmware.exe diff --git a/examples/inventory/.gitignore b/examples/inventory/.gitignore new file mode 100644 index 00000000..ce6a1f3f --- /dev/null +++ b/examples/inventory/.gitignore @@ -0,0 +1,2 @@ +inventory +inventory.exe diff --git a/examples/reset_bmc/.gitignore b/examples/reset_bmc/.gitignore new file mode 100644 index 00000000..00df598b --- /dev/null +++ b/examples/reset_bmc/.gitignore @@ -0,0 +1,2 @@ +reset_bmc +reset_bmc.exe diff --git a/examples/rpc/.gitignore b/examples/rpc/.gitignore new file mode 100644 index 00000000..b029264c --- /dev/null +++ b/examples/rpc/.gitignore @@ -0,0 +1,2 @@ +rpc +rpc.exe diff --git a/examples/screenshot/.gitignore b/examples/screenshot/.gitignore new file mode 100644 index 00000000..0ae472d4 --- /dev/null +++ b/examples/screenshot/.gitignore @@ -0,0 +1,2 @@ +screenshot +screenshot.exe diff --git a/examples/sel/.gitignore b/examples/sel/.gitignore new file mode 100644 index 00000000..9262e0b2 --- /dev/null +++ b/examples/sel/.gitignore @@ -0,0 +1,2 @@ +sel +sel.exe diff --git a/examples/status/.gitignore b/examples/status/.gitignore new file mode 100644 index 00000000..b6b1a40e --- /dev/null +++ b/examples/status/.gitignore @@ -0,0 +1,2 @@ +status +status.exe diff --git a/examples/virtualmedia/.gitignore b/examples/virtualmedia/.gitignore new file mode 100644 index 00000000..30f741db --- /dev/null +++ b/examples/virtualmedia/.gitignore @@ -0,0 +1,2 @@ +virtualmedia +virtualmedia.exe From 6d8b23c097f3f9c55fc0102386017759c6a716f8 Mon Sep 17 00:00:00 2001 From: Manuel Mendez Date: Wed, 12 Mar 2025 11:25:05 -0400 Subject: [PATCH 2/8] Reformat with gofumpt --- bmc/boot_device_test.go | 2 +- bmc/firmware_test.go | 1 + bmc/floppy.go | 1 - client.go | 5 +- constants/constants.go | 2 +- examples/create-users/main.go | 1 - examples/reset_bmc/reset_bmc.go | 1 - examples/screenshot/main.go | 2 +- examples/virtualmedia/main.go | 1 - internal/ipmi/ipmi.go | 2 +- internal/redfishwrapper/bios.go | 2 - internal/redfishwrapper/client_test.go | 5 +- internal/redfishwrapper/firmware.go | 13 ++-- internal/redfishwrapper/firmware_test.go | 6 +- internal/redfishwrapper/inventory.go | 1 - internal/redfishwrapper/inventory_collect.go | 2 - .../redfishwrapper/inventory_collect_test.go | 4 +- internal/redfishwrapper/task.go | 4 +- internal/redfishwrapper/task_test.go | 2 +- internal/sum/sum_test.go | 3 - providers/asrockrack/asrockrack.go | 30 +++++----- providers/asrockrack/firmware.go | 1 - providers/asrockrack/helpers.go | 16 +++-- providers/asrockrack/helpers_test.go | 1 + providers/asrockrack/inventory.go | 1 - providers/asrockrack/mock_test.go | 8 ++- providers/asrockrack/user.go | 6 +- providers/asrockrack/user_test.go | 60 +++++++++---------- providers/dell/idrac_test.go | 2 +- providers/intelamt/intelamt.go | 14 ++--- providers/ipmitool/ipmitool.go | 26 ++++---- providers/ipmitool/ipmitool_test.go | 2 +- providers/openbmc/openbmc.go | 2 +- providers/redfish/main_test.go | 6 +- providers/redfish/redfish.go | 34 +++++------ providers/rpc/signature.go | 1 - providers/supermicro/floppy.go | 6 +- providers/supermicro/supermicro.go | 42 ++++++------- providers/supermicro/supermicro_test.go | 4 -- providers/supermicro/x11_firmware_bios.go | 3 - providers/supermicro/x11_firmware_bmc.go | 2 - providers/supermicro/x11_firmware_bmc_test.go | 2 +- providers/supermicro/x12.go | 6 +- 43 files changed, 139 insertions(+), 196 deletions(-) diff --git a/bmc/boot_device_test.go b/bmc/boot_device_test.go index 17e10549..0007c76d 100644 --- a/bmc/boot_device_test.go +++ b/bmc/boot_device_test.go @@ -3,10 +3,10 @@ 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" diff --git a/bmc/firmware_test.go b/bmc/firmware_test.go index dd6f29b0..7f3c0e8a 100644 --- a/bmc/firmware_test.go +++ b/bmc/firmware_test.go @@ -68,6 +68,7 @@ func TestFirmwareInstall(t *testing.T) { }) } } + func TestFirmwareInstallFromInterfaces(t *testing.T) { testCases := []struct { testName string diff --git a/bmc/floppy.go b/bmc/floppy.go index 41331409..ddd55eb3 100644 --- a/bmc/floppy.go +++ b/bmc/floppy.go @@ -77,7 +77,6 @@ func MountFloppyImageFromInterfaces(ctx context.Context, image io.Reader, p []in "no FloppyImageMounter implementations found", ), ) - } return mountFloppyImage(ctx, image, providers) diff --git a/client.go b/client.go index 4bd09517..f584fed3 100644 --- a/client.go +++ b/client.go @@ -208,7 +208,6 @@ func (c *Client) registerGofishProvider() { // register Intel AMT provider func (c *Client) registerIntelAMTProvider() { - iamtOpts := []intelamt.Option{ intelamt.WithLogger(c.Logger), intelamt.WithHostScheme(c.providerConfig.intelamt.HostScheme), @@ -221,7 +220,7 @@ func (c *Client) registerIntelAMTProvider() { // register Dell gofish provider func (c *Client) registerDellProvider() { dellGofishHttpClient := *c.httpClient - //dellGofishHttpClient.Transport = c.httpClient.Transport.(*http.Transport).Clone() + // dellGofishHttpClient.Transport = c.httpClient.Transport.(*http.Transport).Clone() dellGofishOpts := []dell.Option{ dell.WithHttpClient(&dellGofishHttpClient), dell.WithVersionsNotCompatible(c.providerConfig.dell.VersionsNotCompatible), @@ -373,7 +372,6 @@ func (c *Client) Open(ctx context.Context) error { // Close pass through to library function func (c *Client) Close(ctx context.Context) (err error) { - ctx, span := c.traceprovider.Tracer(pkgName).Start(ctx, "Close") defer span.End() @@ -613,7 +611,6 @@ func (c *Client) FirmwareInstallStatus(ctx context.Context, installVersion, comp metadata.RegisterSpanAttributes(c.Auth.Host, span) return status, err - } // PostCodeGetter pass through library function to return the BIOS/UEFI POST code diff --git a/constants/constants.go b/constants/constants.go index 5b3f44fa..d6f8a18d 100644 --- a/constants/constants.go +++ b/constants/constants.go @@ -33,7 +33,7 @@ const ( // Redfish firmware apply at constants // FirmwareApplyImmediate sets the firmware to be installed immediately after upload Immediate OperationApplyTime = "Immediate" - //FirmwareApplyOnReset sets the firmware to be install on device power cycle/reset + // FirmwareApplyOnReset sets the firmware to be install on device power cycle/reset OnReset OperationApplyTime = "OnReset" // FirmwareOnStartUpdateRequest sets the firmware install to begin after the start request has been sent. OnStartUpdateRequest OperationApplyTime = "OnStartUpdateRequest" diff --git a/examples/create-users/main.go b/examples/create-users/main.go index abffdaf7..8dc201d7 100644 --- a/examples/create-users/main.go +++ b/examples/create-users/main.go @@ -95,5 +95,4 @@ func main() { } l.WithField("count", i).Info("created users") - } diff --git a/examples/reset_bmc/reset_bmc.go b/examples/reset_bmc/reset_bmc.go index d87d7dde..7febfe11 100644 --- a/examples/reset_bmc/reset_bmc.go +++ b/examples/reset_bmc/reset_bmc.go @@ -44,5 +44,4 @@ func main() { if err != nil { l.Error(err) } - } diff --git a/examples/screenshot/main.go b/examples/screenshot/main.go index dff9bfd1..941b7706 100644 --- a/examples/screenshot/main.go +++ b/examples/screenshot/main.go @@ -73,7 +73,7 @@ func main() { } filename := fmt.Sprintf("screenshot." + fileType) - fh, err := os.OpenFile(filename, os.O_WRONLY|os.O_CREATE, 0600) + fh, err := os.OpenFile(filename, os.O_WRONLY|os.O_CREATE, 0o600) if err != nil { l.WithError(err).Error() diff --git a/examples/virtualmedia/main.go b/examples/virtualmedia/main.go index 6dd0ec14..6bc54907 100644 --- a/examples/virtualmedia/main.go +++ b/examples/virtualmedia/main.go @@ -13,7 +13,6 @@ import ( ) func main() { - ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second) defer cancel() diff --git a/internal/ipmi/ipmi.go b/internal/ipmi/ipmi.go index 35196178..98c74424 100644 --- a/internal/ipmi/ipmi.go +++ b/internal/ipmi/ipmi.go @@ -73,7 +73,7 @@ func New(username string, password string, host string, opts ...Option) (ipmi *I func (i *Ipmi) run(ctx context.Context, command []string) (output string, err error) { var out []byte - var ipmiCiphers = []string{"3", "17"} + ipmiCiphers := []string{"3", "17"} ipmiArgs := []string{"-I", "lanplus", "-U", i.Username, "-E", "-N", "5"} if strings.Contains(i.Host, ":") { diff --git a/internal/redfishwrapper/bios.go b/internal/redfishwrapper/bios.go index b6c5f8d5..8aaa5c90 100644 --- a/internal/redfishwrapper/bios.go +++ b/internal/redfishwrapper/bios.go @@ -61,7 +61,6 @@ func (c *Client) SetBiosConfiguration(ctx context.Context, biosConfig map[string // TODO(jwb) We should handle passing different apply times here err = bios.UpdateBiosAttributesApplyAt(settingsAttributes, common.OnResetApplyTime) - if err != nil { return err } @@ -87,7 +86,6 @@ func (c *Client) ResetBiosConfiguration(ctx context.Context) (err error) { } err = bios.ResetBios() - if err != nil { return err } diff --git a/internal/redfishwrapper/client_test.go b/internal/redfishwrapper/client_test.go index 40190753..7a001af0 100644 --- a/internal/redfishwrapper/client_test.go +++ b/internal/redfishwrapper/client_test.go @@ -138,7 +138,7 @@ func TestManagerOdataID(t *testing.T) { ctx := context.Background() - //os.Setenv("DEBUG_BMCLIB", "true") + // os.Setenv("DEBUG_BMCLIB", "true") client := NewClient(parsedURL.Hostname(), parsedURL.Port(), "", "") err = client.Open(ctx) @@ -291,7 +291,7 @@ func TestGetBootProgress(t *testing.T) { "/redfish/v1/Systems/1": endpointFunc(t, "smc_1.14.0_systems_1.json"), }, expect: []*redfish.BootProgress{ - &redfish.BootProgress{ + { LastState: redfish.SystemHardwareInitializationCompleteBootProgressTypes, }, }, @@ -339,5 +339,4 @@ func TestGetBootProgress(t *testing.T) { assert.ElementsMatch(t, tc.expect, got) }) } - } diff --git a/internal/redfishwrapper/firmware.go b/internal/redfishwrapper/firmware.go index aed6e3fe..3b0d7150 100644 --- a/internal/redfishwrapper/firmware.go +++ b/internal/redfishwrapper/firmware.go @@ -29,11 +29,9 @@ const ( multipartHttpUpload installMethod = "multipartUpload" ) -var ( - // the URI for starting a firmware update via StartUpdate is defined in the Redfish Resource and - // Schema Guide (2024.1) - startUpdateURI = "/redfish/v1/UpdateService/Actions/UpdateService.StartUpdate" -) +// the URI for starting a firmware update via StartUpdate is defined in the Redfish Resource and +// Schema Guide (2024.1) +var startUpdateURI = "/redfish/v1/UpdateService/Actions/UpdateService.StartUpdate" var ( errMultiPartPayload = errors.New("error preparing multipart payload") @@ -108,7 +106,7 @@ func (c *Client) FirmwareUpload(ctx context.Context, updateFile *os.File, params // The response contains a location header pointing to the task URI // Location: /redfish/v1/TaskService/Tasks/JID_467696020275 - var location = resp.Header.Get("Location") + location := resp.Header.Get("Location") if strings.Contains(location, "/TaskService/Tasks/") { return taskIDFromLocationHeader(location) } @@ -152,7 +150,7 @@ func (c *Client) StartUpdateForUploadedFirmware(ctx context.Context) (taskID str return "", errors.Wrap(errStartUpdate, "unexpected status code returned: "+resp.Status) } - var location = resp.Header.Get("Location") + location := resp.Header.Get("Location") if strings.Contains(location, "/TaskService/Tasks/") { return taskIDFromLocationHeader(location) } @@ -263,7 +261,6 @@ func (c *Client) unstructuredHttpUpload(url string, update io.Reader) (*http.Res payloadReadSeeker := bytes.NewReader(b) return c.RunRawRequestWithHeaders(http.MethodPost, url, payloadReadSeeker, "application/octet-stream", nil) - } // firmwareUpdateMethodURI returns the updateMethod and URI diff --git a/internal/redfishwrapper/firmware_test.go b/internal/redfishwrapper/firmware_test.go index 5d8c7033..1599fea1 100644 --- a/internal/redfishwrapper/firmware_test.go +++ b/internal/redfishwrapper/firmware_test.go @@ -25,7 +25,7 @@ func TestRunRequestWithMultipartPayload(t *testing.T) { // init things tmpdir := t.TempDir() binPath := filepath.Join(tmpdir, "test.bin") - err := os.WriteFile(binPath, []byte(`HELLOWORLD`), 0600) + err := os.WriteFile(binPath, []byte(`HELLOWORLD`), 0o600) if err != nil { t.Fatal(err) } @@ -343,7 +343,6 @@ func TestUpdateParametersFormField(t *testing.T) { // Validate the created multipart form content err = writer.Close() assert.NoError(t, err) - }) } } @@ -358,14 +357,13 @@ func TestMultipartPayloadSize(t *testing.T) { "foobar", struct{}{}, }) - if err != nil { t.Fatal(err) } tmpdir := t.TempDir() binPath := filepath.Join(tmpdir, "test.bin") - err = os.WriteFile(binPath, []byte(`HELLOWORLD`), 0600) + err = os.WriteFile(binPath, []byte(`HELLOWORLD`), 0o600) if err != nil { t.Fatal(err) } diff --git a/internal/redfishwrapper/inventory.go b/internal/redfishwrapper/inventory.go index 7d4076aa..648b6efb 100644 --- a/internal/redfishwrapper/inventory.go +++ b/internal/redfishwrapper/inventory.go @@ -190,7 +190,6 @@ func (c *Client) chassisAttributes(ctx context.Context, device *common.Device, f } return nil - } func (c *Client) systemAttributes(device *common.Device, failOnError bool, softwareInventory []*redfish.SoftwareInventory) (err error) { diff --git a/internal/redfishwrapper/inventory_collect.go b/internal/redfishwrapper/inventory_collect.go index e441638d..343de616 100644 --- a/internal/redfishwrapper/inventory_collect.go +++ b/internal/redfishwrapper/inventory_collect.go @@ -194,7 +194,6 @@ func (c *Client) collectNetworkPortInfo( firmware string, softwareInventory []*redfish.SoftwareInventory, ) { - if adapter != nil { nicPort.Vendor = adapter.Manufacturer nicPort.Model = adapter.Model @@ -484,7 +483,6 @@ func (c *Client) collectDIMMs(sys *redfish.ComputerSystem, device *common.Device // collecCPLDs populates the device with CPLD component attributes func (c *Client) collectCPLDs(device *common.Device, softwareInventory []*redfish.SoftwareInventory) (err error) { - cpld := &common.CPLD{ Common: common.Common{ Vendor: common.FormatVendorName(device.Vendor), diff --git a/internal/redfishwrapper/inventory_collect_test.go b/internal/redfishwrapper/inventory_collect_test.go index 35f95d5b..098d4241 100644 --- a/internal/redfishwrapper/inventory_collect_test.go +++ b/internal/redfishwrapper/inventory_collect_test.go @@ -110,7 +110,6 @@ func TestInventoryCollectNetworkPortInfo(t *testing.T) { } }) } - } func TestInventoryCollectEthernetInfo(t *testing.T) { @@ -166,7 +165,8 @@ func TestInventoryCollectEthernetInfo(t *testing.T) { name: "full", nicPort: testNicPort, ethernetInterfaces: testMatchingEthList, - wantedNicPort: wNicPortFull}, + wantedNicPort: wNicPortFull, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/internal/redfishwrapper/task.go b/internal/redfishwrapper/task.go index d9f8a64d..08a3667a 100644 --- a/internal/redfishwrapper/task.go +++ b/internal/redfishwrapper/task.go @@ -12,9 +12,7 @@ import ( redfish "github.com/stmcginnis/gofish/redfish" ) -var ( - errUnexpectedTaskState = errors.New("unexpected task state") -) +var errUnexpectedTaskState = errors.New("unexpected task state") func (c *Client) Task(ctx context.Context, taskID string) (*redfish.Task, error) { tasks, err := c.Tasks(ctx) diff --git a/internal/redfishwrapper/task_test.go b/internal/redfishwrapper/task_test.go index d34077a7..9646cd9c 100644 --- a/internal/redfishwrapper/task_test.go +++ b/internal/redfishwrapper/task_test.go @@ -277,7 +277,7 @@ func TestTask(t *testing.T) { ctx := context.Background() - //os.Setenv("DEBUG_BMCLIB", "true") + // os.Setenv("DEBUG_BMCLIB", "true") client := NewClient(parsedURL.Hostname(), parsedURL.Port(), "", "", WithBasicAuthEnabled(true)) err = client.Open(ctx) diff --git a/internal/sum/sum_test.go b/internal/sum/sum_test.go index c7bee90e..465f31c0 100644 --- a/internal/sum/sum_test.go +++ b/internal/sum/sum_test.go @@ -32,7 +32,6 @@ func TestExec_Run(t *testing.T) { // Call the run function _, err := exec.run(ctx, "GetBIOSInfo") - // Check the output and error if err != nil { t.Errorf("Expected no error, got: %v", err) @@ -59,7 +58,6 @@ func TestExec_SetBiosConfiguration(t *testing.T) { // Call the SetBiosConfiguration function err := exec.SetBiosConfiguration(ctx, biosConfig) - // Check for any errors if err != nil { t.Errorf("Expected no error, got: %v", err) @@ -76,7 +74,6 @@ func TestExec_GetBiosConfiguration(t *testing.T) { // Call the SetBiosConfiguration function biosConfig, err := exec.GetBiosConfiguration(ctx) - // Check for any errors if err != nil { t.Errorf("Expected no error, got: %v", err) diff --git a/providers/asrockrack/asrockrack.go b/providers/asrockrack/asrockrack.go index 721c10dc..fcbbb0e6 100644 --- a/providers/asrockrack/asrockrack.go +++ b/providers/asrockrack/asrockrack.go @@ -27,22 +27,20 @@ const ( E3C246D4I_NL = "E3C246D4I-NL" ) -var ( - // Features implemented by asrockrack https - Features = registrar.Features{ - providers.FeaturePostCodeRead, - providers.FeatureBmcReset, - providers.FeatureUserCreate, - providers.FeatureUserUpdate, - providers.FeatureFirmwareUpload, - providers.FeatureFirmwareInstallUploaded, - providers.FeatureFirmwareTaskStatus, - providers.FeatureFirmwareInstallSteps, - providers.FeatureInventoryRead, - providers.FeaturePowerSet, - providers.FeaturePowerState, - } -) +// Features implemented by asrockrack https +var Features = registrar.Features{ + providers.FeaturePostCodeRead, + providers.FeatureBmcReset, + providers.FeatureUserCreate, + providers.FeatureUserUpdate, + providers.FeatureFirmwareUpload, + providers.FeatureFirmwareInstallUploaded, + providers.FeatureFirmwareTaskStatus, + providers.FeatureFirmwareInstallSteps, + providers.FeatureInventoryRead, + providers.FeaturePowerSet, + providers.FeaturePowerState, +} // ASRockRack holds the status and properties of a connection to a asrockrack bmc type ASRockRack struct { diff --git a/providers/asrockrack/firmware.go b/providers/asrockrack/firmware.go index 24bd2973..6ac01102 100644 --- a/providers/asrockrack/firmware.go +++ b/providers/asrockrack/firmware.go @@ -51,7 +51,6 @@ func (a *ASRockRack) FirmwareUpload(ctx context.Context, component string, file } return "", errors.Wrap(bmclibErrs.ErrFirmwareUpload, "component unsupported: "+component) - } func (a *ASRockRack) firmwareUploadBMC(ctx context.Context, file *os.File) error { diff --git a/providers/asrockrack/helpers.go b/providers/asrockrack/helpers.go index c562e746..480796b4 100644 --- a/providers/asrockrack/helpers.go +++ b/providers/asrockrack/helpers.go @@ -127,15 +127,13 @@ type biosUpdateAction struct { Action int `json:"action"` } -var ( - knownPOSTCodes = map[int]string{ - 160: constants.POSTStateOS, - 2: constants.POSTStateBootINIT, // no differentiation between BIOS init and PXE boot - 144: constants.POSTStateUEFI, - 154: constants.POSTStateUEFI, - 178: constants.POSTStateUEFI, - } -) +var knownPOSTCodes = map[int]string{ + 160: constants.POSTStateOS, + 2: constants.POSTStateBootINIT, // no differentiation between BIOS init and PXE boot + 144: constants.POSTStateUEFI, + 154: constants.POSTStateUEFI, + 178: constants.POSTStateUEFI, +} func (a *ASRockRack) listUsers(ctx context.Context) ([]*UserAccount, error) { resp, statusCode, err := a.queryHTTPS(ctx, "api/settings/users", "GET", nil, nil, 0) diff --git a/providers/asrockrack/helpers_test.go b/providers/asrockrack/helpers_test.go index 88d41452..bbdf5a88 100644 --- a/providers/asrockrack/helpers_test.go +++ b/providers/asrockrack/helpers_test.go @@ -47,6 +47,7 @@ func TestInventoryInfo(t *testing.T) { assert.Equal(t, "CPU", inventory[0].DeviceType) assert.Equal(t, "Storage device", inventory[5].DeviceType) } + func Test_fruInfo(t *testing.T) { err := aClient.httpsLogin(context.TODO()) if err != nil { diff --git a/providers/asrockrack/inventory.go b/providers/asrockrack/inventory.go index d571a241..785a3a4b 100644 --- a/providers/asrockrack/inventory.go +++ b/providers/asrockrack/inventory.go @@ -206,7 +206,6 @@ func (a *ASRockRack) componentAttributesE3C246(ctx context.Context, fwInfo *firm }, ) } - } return nil diff --git a/providers/asrockrack/mock_test.go b/providers/asrockrack/mock_test.go index b471f853..85264608 100644 --- a/providers/asrockrack/mock_test.go +++ b/providers/asrockrack/mock_test.go @@ -35,9 +35,11 @@ var ( ) // setup test BMC -var server *httptest.Server -var bmcURL *url.URL -var fwUpgradeState *testFwUpgradeState +var ( + server *httptest.Server + bmcURL *url.URL + fwUpgradeState *testFwUpgradeState +) type testFwUpgradeState struct { FlashModeSet bool diff --git a/providers/asrockrack/user.go b/providers/asrockrack/user.go index 91105503..42a7f1a7 100644 --- a/providers/asrockrack/user.go +++ b/providers/asrockrack/user.go @@ -11,10 +11,8 @@ import ( "github.com/bmc-toolbox/bmclib/v2/internal" ) -var ( - // TODO: standardize these across Redfish, IPMI, Vendor GUI - validRoles = []string{"Administrator", "Operator", "User"} -) +// TODO: standardize these across Redfish, IPMI, Vendor GUI +var validRoles = []string{"Administrator", "Operator", "User"} // UserAccount is a ASRR BMC user account struct type UserAccount struct { diff --git a/providers/asrockrack/user_test.go b/providers/asrockrack/user_test.go index 38d2a9b8..311e9362 100644 --- a/providers/asrockrack/user_test.go +++ b/providers/asrockrack/user_test.go @@ -23,28 +23,25 @@ type testCase struct { tName string } -var ( - // common set of test cases - testCases = []testCase{ - - { - "foo", - "baz", - "", - false, - bmclibErrs.ErrInvalidUserRole, - "role not defined", - }, - { - "foo", - "", - "Administrator", - false, - bmclibErrs.ErrUserParamsRequired, - "param not defined", - }, - } -) +// common set of test cases +var testCases = []testCase{ + { + "foo", + "baz", + "", + false, + bmclibErrs.ErrInvalidUserRole, + "role not defined", + }, + { + "foo", + "", + "Administrator", + false, + bmclibErrs.ErrUserParamsRequired, + "param not defined", + }, +} func Test_UserRead(t *testing.T) { expected := []map[string]string{ @@ -87,17 +84,17 @@ func Test_UserRead(t *testing.T) { } func Test_UserCreate(t *testing.T) { - tests := testCases tests = append(tests, - []testCase{{ - "root", - "calvin", - "Administrator", - true, - nil, - "user account is created", - }, + []testCase{ + { + "root", + "calvin", + "Administrator", + true, + nil, + "user account is created", + }, { "admin", "foo", @@ -196,7 +193,6 @@ func Test_createUser(t *testing.T) { } assert.Equal(t, "application/json", contentType) - } func Test_userAccounts(t *testing.T) { diff --git a/providers/dell/idrac_test.go b/providers/dell/idrac_test.go index 69a91a68..bf05e238 100644 --- a/providers/dell/idrac_test.go +++ b/providers/dell/idrac_test.go @@ -103,7 +103,7 @@ func Test_Screenshot(t *testing.T) { t.Fatal(err) } - //os.Setenv("DEBUG_BMCLIB", "true") + // os.Setenv("DEBUG_BMCLIB", "true") client := New(parsedURL.Hostname(), "", "", logr.Discard(), WithPort(parsedURL.Port()), WithUseBasicAuth(true)) err = client.Open(context.TODO()) diff --git a/providers/intelamt/intelamt.go b/providers/intelamt/intelamt.go index 001ebcc6..95376840 100644 --- a/providers/intelamt/intelamt.go +++ b/providers/intelamt/intelamt.go @@ -18,14 +18,12 @@ const ( ProviderProtocol = "AMT" ) -var ( - // Features implemented by the AMT provider - Features = registrar.Features{ - providers.FeaturePowerSet, - providers.FeaturePowerState, - providers.FeatureBootDeviceSet, - } -) +// Features implemented by the AMT provider +var Features = registrar.Features{ + providers.FeaturePowerSet, + providers.FeaturePowerState, + providers.FeatureBootDeviceSet, +} // iamtClient interface allows us to mock the client for testing type iamtClient interface { diff --git a/providers/ipmitool/ipmitool.go b/providers/ipmitool/ipmitool.go index 69f37ccc..eabf4144 100644 --- a/providers/ipmitool/ipmitool.go +++ b/providers/ipmitool/ipmitool.go @@ -19,20 +19,18 @@ const ( ProviderProtocol = "ipmi" ) -var ( - // Features implemented by ipmitool - Features = registrar.Features{ - providers.FeaturePowerSet, - providers.FeaturePowerState, - providers.FeatureUserRead, - providers.FeatureBmcReset, - providers.FeatureBootDeviceSet, - providers.FeatureClearSystemEventLog, - providers.FeatureGetSystemEventLog, - providers.FeatureGetSystemEventLogRaw, - providers.FeatureDeactivateSOL, - } -) +// Features implemented by ipmitool +var Features = registrar.Features{ + providers.FeaturePowerSet, + providers.FeaturePowerState, + providers.FeatureUserRead, + providers.FeatureBmcReset, + providers.FeatureBootDeviceSet, + providers.FeatureClearSystemEventLog, + providers.FeatureGetSystemEventLog, + providers.FeatureGetSystemEventLogRaw, + providers.FeatureDeactivateSOL, +} // Conn for Ipmitool connection details type Conn struct { diff --git a/providers/ipmitool/ipmitool_test.go b/providers/ipmitool/ipmitool_test.go index de395bc1..46d685cf 100644 --- a/providers/ipmitool/ipmitool_test.go +++ b/providers/ipmitool/ipmitool_test.go @@ -23,7 +23,7 @@ func TestMain(m *testing.M) { os.Setenv("PATH", path) fmt.Println(os.Getenv("PATH")) f := filepath.Join(tempDir, "ipmitool") - err = os.WriteFile(f, []byte{}, 0755) + err = os.WriteFile(f, []byte{}, 0o755) if err != nil { os.RemoveAll(tempDir) os.Exit(3) diff --git a/providers/openbmc/openbmc.go b/providers/openbmc/openbmc.go index 8e9cde11..45efdbdb 100644 --- a/providers/openbmc/openbmc.go +++ b/providers/openbmc/openbmc.go @@ -126,7 +126,7 @@ func (c *Conn) Open(ctx context.Context) (err error) { } func (c *Conn) deviceSupported(ctx context.Context) error { - var host = c.host + host := c.host if !strings.HasPrefix(host, "https://") && !strings.HasPrefix(host, "http://") { host = "https://" + host } diff --git a/providers/redfish/main_test.go b/providers/redfish/main_test.go index 2a71b818..4a72da4e 100644 --- a/providers/redfish/main_test.go +++ b/providers/redfish/main_test.go @@ -34,9 +34,9 @@ func jsonResponse(endpoint string) []byte { "/redfish/v1/Managers/iDRAC.Embedded.1/LogServices/Sel/Entries/1": fixturesDir + "/v1/dell/selentries/1.json", "/redfish/v1/Managers/iDRAC.Embedded.1/LogServices/Sel/Entries/2": fixturesDir + "/v1/dell/selentries/2.json", - "/redfish/v1/": fixturesDir + "/v1/serviceroot.json", - "/redfish/v1/UpdateService": fixturesDir + "/v1/updateservice.json", - "/redfish/v1/Systems": fixturesDir + "/v1/systems.json", + "/redfish/v1/": fixturesDir + "/v1/serviceroot.json", + "/redfish/v1/UpdateService": fixturesDir + "/v1/updateservice.json", + "/redfish/v1/Systems": fixturesDir + "/v1/systems.json", } fh, err := os.Open(jsonResponsesMap[endpoint]) diff --git a/providers/redfish/redfish.go b/providers/redfish/redfish.go index 07caa920..e738580b 100644 --- a/providers/redfish/redfish.go +++ b/providers/redfish/redfish.go @@ -23,24 +23,22 @@ const ( ProviderProtocol = "redfish" ) -var ( - // Features implemented by gofish - Features = registrar.Features{ - providers.FeaturePowerSet, - providers.FeaturePowerState, - providers.FeatureUserCreate, - providers.FeatureUserUpdate, - providers.FeatureUserDelete, - providers.FeatureBootDeviceSet, - providers.FeatureVirtualMedia, - providers.FeatureInventoryRead, - providers.FeatureBmcReset, - providers.FeatureClearSystemEventLog, - providers.FeatureGetBiosConfiguration, - providers.FeatureSetBiosConfiguration, - providers.FeatureResetBiosConfiguration, - } -) +// Features implemented by gofish +var Features = registrar.Features{ + providers.FeaturePowerSet, + providers.FeaturePowerState, + providers.FeatureUserCreate, + providers.FeatureUserUpdate, + providers.FeatureUserDelete, + providers.FeatureBootDeviceSet, + providers.FeatureVirtualMedia, + providers.FeatureInventoryRead, + providers.FeatureBmcReset, + providers.FeatureClearSystemEventLog, + providers.FeatureGetBiosConfiguration, + providers.FeatureSetBiosConfiguration, + providers.FeatureResetBiosConfiguration, +} // Conn details for redfish client type Conn struct { diff --git a/providers/rpc/signature.go b/providers/rpc/signature.go index 724187ee..693f91d9 100644 --- a/providers/rpc/signature.go +++ b/providers/rpc/signature.go @@ -19,7 +19,6 @@ func createSignaturePayload(body []byte, h http.Header) []byte { // add headers to signature payload, no space between values. for _, val := range h { body = append(body, []byte(strings.Join(val, ""))...) - } return body diff --git a/providers/supermicro/floppy.go b/providers/supermicro/floppy.go index 835b70f4..5bacdb29 100644 --- a/providers/supermicro/floppy.go +++ b/providers/supermicro/floppy.go @@ -15,9 +15,7 @@ import ( "github.com/pkg/errors" ) -var ( - errFloppyImageMounted = errors.New("floppy image is currently mounted") -) +var errFloppyImageMounted = errors.New("floppy image is currently mounted") func (c *Client) floppyImageMounted(ctx context.Context) (bool, error) { if err := c.serviceClient.redfishSession(ctx); err != nil { @@ -116,7 +114,6 @@ func (c *Client) MountFloppyImage(ctx context.Context, image io.Reader) error { map[string]string{"Content-Type": payloadWriter.FormDataContentType()}, 0, ) - if err != nil { return errors.Wrap(ErrMultipartForm, err.Error()) } @@ -146,7 +143,6 @@ func (c *Client) UnmountFloppyImage(ctx context.Context) error { nil, 0, ) - if err != nil { return err } diff --git a/providers/supermicro/supermicro.go b/providers/supermicro/supermicro.go index c4a2d1e4..2d93cb54 100644 --- a/providers/supermicro/supermicro.go +++ b/providers/supermicro/supermicro.go @@ -39,27 +39,25 @@ const ( ProviderProtocol = "vendorapi" ) -var ( - // Features implemented - Features = registrar.Features{ - providers.FeatureScreenshot, - providers.FeatureMountFloppyImage, - providers.FeatureUnmountFloppyImage, - providers.FeatureFirmwareUpload, - providers.FeatureFirmwareInstallUploaded, - providers.FeatureFirmwareTaskStatus, - providers.FeatureFirmwareInstallSteps, - providers.FeatureInventoryRead, - providers.FeaturePowerSet, - providers.FeaturePowerState, - providers.FeatureBmcReset, - providers.FeatureGetBiosConfiguration, - providers.FeatureSetBiosConfiguration, - providers.FeatureSetBiosConfigurationFromFile, - providers.FeatureResetBiosConfiguration, - providers.FeatureBootProgress, - } -) +// Features implemented +var Features = registrar.Features{ + providers.FeatureScreenshot, + providers.FeatureMountFloppyImage, + providers.FeatureUnmountFloppyImage, + providers.FeatureFirmwareUpload, + providers.FeatureFirmwareInstallUploaded, + providers.FeatureFirmwareTaskStatus, + providers.FeatureFirmwareInstallSteps, + providers.FeatureInventoryRead, + providers.FeaturePowerSet, + providers.FeaturePowerState, + providers.FeatureBmcReset, + providers.FeatureGetBiosConfiguration, + providers.FeatureSetBiosConfiguration, + providers.FeatureSetBiosConfigurationFromFile, + providers.FeatureResetBiosConfiguration, + providers.FeatureBootProgress, +} // supports // @@ -584,7 +582,6 @@ func (c *serviceClient) query(ctx context.Context, endpoint, method string, payl if cookie.Name == "SID" && cookie.Value != "" { req.AddCookie(cookie) } - } var reqDump []byte @@ -634,7 +631,6 @@ func hostIP(hostURL string) (string, error) { if strings.Contains(hostURLParsed.Host, ":") { return strings.Split(hostURLParsed.Host, ":")[0], nil - } return hostURLParsed.Host, nil diff --git a/providers/supermicro/supermicro_test.go b/providers/supermicro/supermicro_test.go index 462df6ea..8d3064dc 100644 --- a/providers/supermicro/supermicro_test.go +++ b/providers/supermicro/supermicro_test.go @@ -67,7 +67,6 @@ func TestParseToken(t *testing.T) { t.Run(tc.name, func(t *testing.T) { gotToken := parseToken(tc.body) assert.Equal(t, tc.expectToken, gotToken) - }) } } @@ -237,7 +236,6 @@ func TestOpen(t *testing.T) { assert.Nil(t, err) }) } - } func TestClose(t *testing.T) { @@ -293,7 +291,6 @@ func TestClose(t *testing.T) { assert.Nil(t, client.serviceClient.redfish) }) } - } func TestInitScreenPreview(t *testing.T) { @@ -354,7 +351,6 @@ func TestInitScreenPreview(t *testing.T) { } assert.Nil(t, err) - }) } } diff --git a/providers/supermicro/x11_firmware_bios.go b/providers/supermicro/x11_firmware_bios.go index 0b989e2d..14cb7bf2 100644 --- a/providers/supermicro/x11_firmware_bios.go +++ b/providers/supermicro/x11_firmware_bios.go @@ -119,7 +119,6 @@ func (c *x11) checkComponentUpdateMisc(ctx context.Context, stage string) error } func (c *x11) setBIOSFirmwareInstallMode(ctx context.Context) error { - payload := []byte(`op=BIOS_UPLOAD.XML&r=(0,0)&_=`) headers := map[string]string{ @@ -159,7 +158,6 @@ func (c *x11) setBIOSFirmwareInstallMode(ctx context.Context) error { default: return unexpectedResponseErr(payload, body, status) } - } func (c *x11) setBiosUpdateStart(ctx context.Context) error { @@ -261,7 +259,6 @@ func (c *x11) uploadBIOSFirmware(ctx context.Context, fwReader io.Reader) error map[string]string{"Content-Type": payloadWriter.FormDataContentType()}, 0, ) - if err != nil { return errors.Wrap(ErrMultipartForm, err.Error()) } diff --git a/providers/supermicro/x11_firmware_bmc.go b/providers/supermicro/x11_firmware_bmc.go index 246aa5e8..0ffa6218 100644 --- a/providers/supermicro/x11_firmware_bmc.go +++ b/providers/supermicro/x11_firmware_bmc.go @@ -88,7 +88,6 @@ func (c *x11) setBMCFirmwareInstallMode(ctx context.Context) error { default: return errors.Wrap(ErrFirmwareInstallMode, "set firmware install mode returned unexpected response body") } - } // -----------------------------212212001131894333502018521064 @@ -172,7 +171,6 @@ func (c *x11) uploadBMCFirmware(ctx context.Context, fwReader io.Reader) error { map[string]string{"Content-Type": payloadWriter.FormDataContentType()}, 0, ) - if err != nil { return errors.Wrap(ErrMultipartForm, err.Error()) } diff --git a/providers/supermicro/x11_firmware_bmc_test.go b/providers/supermicro/x11_firmware_bmc_test.go index 81cd8f08..69ec8911 100644 --- a/providers/supermicro/x11_firmware_bmc_test.go +++ b/providers/supermicro/x11_firmware_bmc_test.go @@ -173,7 +173,7 @@ func TestX11UploadBMCFirmware(t *testing.T) { if tc.fwFilename != "" { tmpdir := t.TempDir() binPath := filepath.Join(tmpdir, tc.fwFilename) - err := os.WriteFile(binPath, []byte(tc.fwFileContents), 0600) + err := os.WriteFile(binPath, []byte(tc.fwFileContents), 0o600) if err != nil { t.Fatal(err) } diff --git a/providers/supermicro/x12.go b/providers/supermicro/x12.go index a4a7c682..e1c024a4 100644 --- a/providers/supermicro/x12.go +++ b/providers/supermicro/x12.go @@ -53,9 +53,7 @@ func (c *x12) queryDeviceModel(ctx context.Context) (string, error) { return c.model, nil } -var ( - errUploadTaskIDEmpty = errors.New("firmware upload request returned empty firmware upload verify TaskID") -) +var errUploadTaskIDEmpty = errors.New("firmware upload request returned empty firmware upload verify TaskID") func (c *x12) supportsInstall(component string) error { errComponentNotSupported := fmt.Errorf("component %s on device %s not supported", component, c.model) @@ -277,7 +275,7 @@ func (c *x12) redfishOdataID(ctx context.Context, component string) (string, err case common.SlugBIOS: // hardcoded since SMCs without the DCMS license will throw license errors return "/redfish/v1/Systems/1/Bios", nil - //return c.redfish.SystemsBIOSOdataID(ctx) + // return c.redfish.SystemsBIOSOdataID(ctx) } return "", errUnsupported From bace01488e5d454a71007207f09f02940daf8dae Mon Sep 17 00:00:00 2001 From: Manuel Mendez Date: Wed, 12 Mar 2025 11:26:09 -0400 Subject: [PATCH 3/8] Get rid of unnecessary groups in imports Automated using the following command: ``` git ls-files '*.go' | xargs -I% sh -c ' sed "/^import (/,/^)/ { /^\s*$/ d }" % >%.tmp && goimports -w %.tmp && (if cmp -s % %.tmp; then rm %.tmp; else mv %.tmp %; fi)' ``` --- bmc/firmware.go | 1 - bmc/inventory.go | 5 ++--- examples/bios/main.go | 1 - internal/redfishwrapper/firmware.go | 5 ++--- internal/redfishwrapper/inventory.go | 3 +-- internal/redfishwrapper/system.go | 1 - internal/sum/sum.go | 1 - providers/asrockrack/firmware.go | 3 +-- providers/asrockrack/user.go | 3 +-- providers/asrockrack/user_test.go | 3 +-- providers/dell/idrac.go | 3 +-- providers/openbmc/firmware.go | 3 +-- providers/redfish/redfish.go | 5 ++--- providers/supermicro/supermicro.go | 8 +++----- providers/supermicro/x11_firmware_bmc.go | 3 +-- 15 files changed, 16 insertions(+), 32 deletions(-) diff --git a/bmc/firmware.go b/bmc/firmware.go index f6235c5a..9b7ee89a 100644 --- a/bmc/firmware.go +++ b/bmc/firmware.go @@ -9,7 +9,6 @@ import ( "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" ) diff --git a/bmc/inventory.go b/bmc/inventory.go index 7a9dd07b..cd9a0f43 100644 --- a/bmc/inventory.go +++ b/bmc/inventory.go @@ -4,11 +4,10 @@ import ( "context" "fmt" - "github.com/hashicorp/go-multierror" - "github.com/pkg/errors" - bmclibErrs "github.com/bmc-toolbox/bmclib/v2/errors" "github.com/bmc-toolbox/common" + "github.com/hashicorp/go-multierror" + "github.com/pkg/errors" ) // InventoryGetter defines methods to retrieve device hardware and firmware inventory diff --git a/examples/bios/main.go b/examples/bios/main.go index f9bc5c1d..121eb36f 100644 --- a/examples/bios/main.go +++ b/examples/bios/main.go @@ -13,7 +13,6 @@ import ( bmclib "github.com/bmc-toolbox/bmclib/v2" "github.com/bmc-toolbox/bmclib/v2/providers" logrusr "github.com/bombsimon/logrusr/v2" - "github.com/sirupsen/logrus" ) diff --git a/internal/redfishwrapper/firmware.go b/internal/redfishwrapper/firmware.go index 3b0d7150..82ee6a36 100644 --- a/internal/redfishwrapper/firmware.go +++ b/internal/redfishwrapper/firmware.go @@ -15,11 +15,10 @@ import ( "strings" "time" - "github.com/pkg/errors" - "github.com/stmcginnis/gofish/redfish" - "github.com/bmc-toolbox/bmclib/v2/constants" bmclibErrs "github.com/bmc-toolbox/bmclib/v2/errors" + "github.com/pkg/errors" + "github.com/stmcginnis/gofish/redfish" ) type installMethod string diff --git a/internal/redfishwrapper/inventory.go b/internal/redfishwrapper/inventory.go index 648b6efb..e387cc3d 100644 --- a/internal/redfishwrapper/inventory.go +++ b/internal/redfishwrapper/inventory.go @@ -5,9 +5,8 @@ import ( "strings" bmclibErrs "github.com/bmc-toolbox/bmclib/v2/errors" - "github.com/pkg/errors" - "github.com/bmc-toolbox/common" + "github.com/pkg/errors" redfish "github.com/stmcginnis/gofish/redfish" ) diff --git a/internal/redfishwrapper/system.go b/internal/redfishwrapper/system.go index 5fa10863..119bf6d6 100644 --- a/internal/redfishwrapper/system.go +++ b/internal/redfishwrapper/system.go @@ -4,7 +4,6 @@ import ( "context" bmclibErrs "github.com/bmc-toolbox/bmclib/v2/errors" - "github.com/pkg/errors" redfish "github.com/stmcginnis/gofish/redfish" ) diff --git a/internal/sum/sum.go b/internal/sum/sum.go index b643a959..99d82e5c 100644 --- a/internal/sum/sum.go +++ b/internal/sum/sum.go @@ -10,7 +10,6 @@ import ( "strings" ex "github.com/bmc-toolbox/bmclib/v2/internal/executor" - "github.com/bmc-toolbox/common" "github.com/bmc-toolbox/common/config" "github.com/go-logr/logr" diff --git a/providers/asrockrack/firmware.go b/providers/asrockrack/firmware.go index 6ac01102..5146a87b 100644 --- a/providers/asrockrack/firmware.go +++ b/providers/asrockrack/firmware.go @@ -7,12 +7,11 @@ import ( "strings" "time" - "github.com/pkg/errors" - "github.com/bmc-toolbox/bmclib/v2/constants" bmclibErrs "github.com/bmc-toolbox/bmclib/v2/errors" "github.com/bmc-toolbox/bmclib/v2/internal" "github.com/bmc-toolbox/common" + "github.com/pkg/errors" ) const ( diff --git a/providers/asrockrack/user.go b/providers/asrockrack/user.go index 42a7f1a7..b82e320f 100644 --- a/providers/asrockrack/user.go +++ b/providers/asrockrack/user.go @@ -5,10 +5,9 @@ import ( "fmt" "strings" - "github.com/pkg/errors" - bmclibErrs "github.com/bmc-toolbox/bmclib/v2/errors" "github.com/bmc-toolbox/bmclib/v2/internal" + "github.com/pkg/errors" ) // TODO: standardize these across Redfish, IPMI, Vendor GUI diff --git a/providers/asrockrack/user_test.go b/providers/asrockrack/user_test.go index 311e9362..39432e3c 100644 --- a/providers/asrockrack/user_test.go +++ b/providers/asrockrack/user_test.go @@ -7,9 +7,8 @@ import ( "os" "testing" - "github.com/stretchr/testify/assert" - bmclibErrs "github.com/bmc-toolbox/bmclib/v2/errors" + "github.com/stretchr/testify/assert" ) // NOTE: user accounts are defined in mock_test.go as JSON payload in the userPayload var diff --git a/providers/dell/idrac.go b/providers/dell/idrac.go index 7ea6e724..58995ee8 100644 --- a/providers/dell/idrac.go +++ b/providers/dell/idrac.go @@ -10,6 +10,7 @@ import ( "net/http" "strings" + bmclibErrs "github.com/bmc-toolbox/bmclib/v2/errors" "github.com/bmc-toolbox/bmclib/v2/internal/httpclient" "github.com/bmc-toolbox/bmclib/v2/internal/redfishwrapper" "github.com/bmc-toolbox/bmclib/v2/providers" @@ -17,8 +18,6 @@ import ( "github.com/go-logr/logr" "github.com/jacobweinstock/registrar" "github.com/pkg/errors" - - bmclibErrs "github.com/bmc-toolbox/bmclib/v2/errors" ) const ( diff --git a/providers/openbmc/firmware.go b/providers/openbmc/firmware.go index d5fd492a..5284fd98 100644 --- a/providers/openbmc/firmware.go +++ b/providers/openbmc/firmware.go @@ -8,10 +8,9 @@ import ( "time" "github.com/bmc-toolbox/bmclib/v2/constants" - "github.com/bmc-toolbox/common" - bmcliberrs "github.com/bmc-toolbox/bmclib/v2/errors" rfw "github.com/bmc-toolbox/bmclib/v2/internal/redfishwrapper" + "github.com/bmc-toolbox/common" "github.com/pkg/errors" "github.com/stmcginnis/gofish/redfish" ) diff --git a/providers/redfish/redfish.go b/providers/redfish/redfish.go index e738580b..c30e8014 100644 --- a/providers/redfish/redfish.go +++ b/providers/redfish/redfish.go @@ -5,15 +5,14 @@ import ( "crypto/x509" "net/http" + "github.com/bmc-toolbox/bmclib/v2/bmc" + bmclibErrs "github.com/bmc-toolbox/bmclib/v2/errors" "github.com/bmc-toolbox/bmclib/v2/internal/httpclient" "github.com/bmc-toolbox/bmclib/v2/internal/redfishwrapper" "github.com/bmc-toolbox/bmclib/v2/providers" "github.com/bmc-toolbox/common" "github.com/go-logr/logr" "github.com/jacobweinstock/registrar" - - "github.com/bmc-toolbox/bmclib/v2/bmc" - bmclibErrs "github.com/bmc-toolbox/bmclib/v2/errors" ) const ( diff --git a/providers/supermicro/supermicro.go b/providers/supermicro/supermicro.go index 2d93cb54..9834751c 100644 --- a/providers/supermicro/supermicro.go +++ b/providers/supermicro/supermicro.go @@ -17,19 +17,17 @@ import ( "time" "github.com/bmc-toolbox/bmclib/v2/constants" + bmclibconsts "github.com/bmc-toolbox/bmclib/v2/constants" + bmclibErrs "github.com/bmc-toolbox/bmclib/v2/errors" "github.com/bmc-toolbox/bmclib/v2/internal/httpclient" "github.com/bmc-toolbox/bmclib/v2/internal/redfishwrapper" "github.com/bmc-toolbox/bmclib/v2/internal/sum" "github.com/bmc-toolbox/bmclib/v2/providers" "github.com/bmc-toolbox/common" - "github.com/stmcginnis/gofish/redfish" - "github.com/go-logr/logr" "github.com/jacobweinstock/registrar" "github.com/pkg/errors" - - bmclibconsts "github.com/bmc-toolbox/bmclib/v2/constants" - bmclibErrs "github.com/bmc-toolbox/bmclib/v2/errors" + "github.com/stmcginnis/gofish/redfish" ) const ( diff --git a/providers/supermicro/x11_firmware_bmc.go b/providers/supermicro/x11_firmware_bmc.go index 0ffa6218..859dcea2 100644 --- a/providers/supermicro/x11_firmware_bmc.go +++ b/providers/supermicro/x11_firmware_bmc.go @@ -14,10 +14,9 @@ import ( "strings" "time" - "github.com/pkg/errors" - "github.com/bmc-toolbox/bmclib/v2/constants" bmclibErrs "github.com/bmc-toolbox/bmclib/v2/errors" + "github.com/pkg/errors" ) var ( From 9b560cbdfc33fa80f99b4f47c7ff92c7744d7d6a Mon Sep 17 00:00:00 2001 From: Manuel Mendez Date: Wed, 12 Mar 2025 11:36:04 -0400 Subject: [PATCH 4/8] Pin golangci-lint version in github action Going to go from disable-all,enable-some to enable-all,disable-some so we are going to want to avoid golangci-lint changing out from under us. --- .github/workflows/ci.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 637c879d..228f9a72 100644 --- a/.github/workflows/ci.yaml +++ b/.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: From 2347ae2d8f71ce6c1022de0ea1518f2b4edf132f Mon Sep 17 00:00:00 2001 From: Manuel Mendez Date: Wed, 12 Mar 2025 11:54:43 -0400 Subject: [PATCH 5/8] Update .golangci.yml to version based on file from metal-toolbox org --- .golangci.yml | 187 ++++++++++++--------------- errors/errors.go | 2 +- internal/redfishwrapper/task.go | 4 +- internal/redfishwrapper/task_test.go | 4 +- 4 files changed, 86 insertions(+), 111 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index d785b2c9..7f69cd9f 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,125 +1,100 @@ - 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: 15 gofumpt: extra-rules: true - auto-fix: true - wsl: - auto-fix: true - stylecheck: - auto-fix: true + govet: + enable: + - shadow + 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) + - errname # maybe should be enabled + - 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 # should be enabled + - testifylint # should be enabled + - 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 diff --git a/errors/errors.go b/errors/errors.go index 37a7d5d1..9f0eaeaa 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -112,7 +112,7 @@ var ( ErrBMCColdResetRequired = errors.New("BMC cold reset required") // ErrHostPowercycleRequired is returned when a host powercycle is required. - ErrHostPowercycleRequired = errors.New("Host power cycle required") + ErrHostPowercycleRequired = errors.New("Host power cycle required") // nolint:stylecheck // ErrSessionExpired is returned when the BMC session is not valid // the receiver can then choose to request a new session. diff --git a/internal/redfishwrapper/task.go b/internal/redfishwrapper/task.go index 08a3667a..859139d8 100644 --- a/internal/redfishwrapper/task.go +++ b/internal/redfishwrapper/task.go @@ -74,13 +74,13 @@ func (c *Client) ConvertTaskState(state string) constants.TaskState { switch strings.ToLower(state) { case "starting", "downloading", "downloaded", "scheduling": return constants.Initializing - case "running", "stopping", "cancelling": + case "running", "stopping", "cancelling": // nolint:misspell return constants.Running case "pending", "new": return constants.Queued case "scheduled": return constants.PowerCycleHost - case "interrupted", "killed", "exception", "cancelled", "suspended", "failed": + case "interrupted", "killed", "exception", "cancelled", "suspended", "failed": // nolint:misspell return constants.Failed case "completed": return constants.Complete diff --git a/internal/redfishwrapper/task_test.go b/internal/redfishwrapper/task_test.go index 9646cd9c..0ae1159f 100644 --- a/internal/redfishwrapper/task_test.go +++ b/internal/redfishwrapper/task_test.go @@ -24,14 +24,14 @@ func TestConvertTaskState(t *testing.T) { {"scheduling state", "scheduling", constants.Initializing}, {"running state", "running", constants.Running}, {"stopping state", "stopping", constants.Running}, - {"cancelling state", "cancelling", constants.Running}, + {"cancelling state", "cancelling", constants.Running}, // nolint:misspell {"pending state", "pending", constants.Queued}, {"new state", "new", constants.Queued}, {"scheduled state", "scheduled", constants.PowerCycleHost}, {"interrupted state", "interrupted", constants.Failed}, {"killed state", "killed", constants.Failed}, {"exception state", "exception", constants.Failed}, - {"cancelled state", "cancelled", constants.Failed}, + {"cancelled state", "cancelled", constants.Failed}, // nolint:misspell {"suspended state", "suspended", constants.Failed}, {"failed state", "failed", constants.Failed}, {"completed state", "completed", constants.Complete}, From e7ee28575db997d35bb278e86f5bff0dae18ef13 Mon Sep 17 00:00:00 2001 From: Manuel Mendez Date: Wed, 12 Mar 2025 13:15:11 -0400 Subject: [PATCH 6/8] Apply automatic fixes from golangci-lint linters --- bmc/bios.go | 4 -- bmc/connection.go | 6 +-- bmc/firmware.go | 9 +--- bmc/inventory.go | 1 - bmc/postcode.go | 1 - bmc/screenshot.go | 1 - bmc/sel.go | 3 -- bmc/virtual_media.go | 6 +-- bmc/virtual_media_test.go | 2 +- client.go | 6 +-- constants/constants.go | 2 +- errors/errors.go | 2 +- internal/ipmi/ipmi.go | 50 +++++++++---------- internal/redfishwrapper/firmware_test.go | 2 +- internal/redfishwrapper/inventory.go | 3 -- internal/redfishwrapper/inventory_collect.go | 6 --- internal/redfishwrapper/power.go | 1 - internal/redfishwrapper/virtual_media.go | 8 +-- internal/sshclient/sshclient.go | 2 +- providers/asrockrack/asrockrack.go | 4 +- providers/asrockrack/firmware.go | 2 +- providers/asrockrack/mock_test.go | 30 +++++------ providers/asrockrack/user_test.go | 2 +- providers/dell/firmware.go | 3 +- providers/dell/idrac.go | 4 +- providers/intelamt/intelamt.go | 2 +- providers/redfish/redfish.go | 2 +- providers/rpc/http_test.go | 2 +- providers/rpc/rpc.go | 2 +- providers/supermicro/firmware_bios_test.go | 2 +- providers/supermicro/supermicro_test.go | 8 +-- providers/supermicro/x11_firmware_bios.go | 1 - providers/supermicro/x11_firmware_bmc_test.go | 2 +- 33 files changed, 78 insertions(+), 103 deletions(-) diff --git a/bmc/bios.go b/bmc/bios.go index 2acba9e3..2daf7d0a 100644 --- a/bmc/bios.go +++ b/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 diff --git a/bmc/connection.go b/bmc/connection.go index 8529af4c..897bb18d 100644 --- a/bmc/connection.go +++ b/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() diff --git a/bmc/firmware.go b/bmc/firmware.go index 9b7ee89a..a3df6a56 100644 --- a/bmc/firmware.go +++ b/bmc/firmware.go @@ -25,7 +25,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 @@ -54,7 +54,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 @@ -96,7 +95,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 { @@ -138,7 +137,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 @@ -298,7 +296,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 @@ -401,7 +398,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 @@ -472,7 +468,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 diff --git a/bmc/inventory.go b/bmc/inventory.go index cd9a0f43..de6e72ed 100644 --- a/bmc/inventory.go +++ b/bmc/inventory.go @@ -40,7 +40,6 @@ func inventory(ctx context.Context, generic []inventoryGetterProvider) (device * err = multierror.Append(err, errors.WithMessagef(vErr, "provider: %v", elem.name)) err = multierror.Append(err, vErr) continue - } metadataLocal.SuccessfulProvider = elem.name return device, metadataLocal, nil diff --git a/bmc/postcode.go b/bmc/postcode.go index 015e8cb4..9382275d 100644 --- a/bmc/postcode.go +++ b/bmc/postcode.go @@ -43,7 +43,6 @@ func postCode(ctx context.Context, generic []postCodeGetterProvider) (status str err = multierror.Append(err, errors.WithMessagef(vErr, "provider: %v", elem.name)) err = multierror.Append(err, vErr) continue - } metadataLocal.SuccessfulProvider = elem.name return status, code, metadataLocal, nil diff --git a/bmc/screenshot.go b/bmc/screenshot.go index 62812ccf..68314b63 100644 --- a/bmc/screenshot.go +++ b/bmc/screenshot.go @@ -38,7 +38,6 @@ func screenshot(ctx context.Context, generic []screenshotGetterProvider) (image if vErr != nil { err = multierror.Append(err, errors.WithMessagef(vErr, "provider: %v", elem.name)) continue - } metadataLocal.SuccessfulProvider = elem.name return image, fileType, metadataLocal, nil diff --git a/bmc/sel.go b/bmc/sel.go index de9c78ff..e2e5f94e 100644 --- a/bmc/sel.go +++ b/bmc/sel.go @@ -47,7 +47,6 @@ func clearSystemEventLog(ctx context.Context, timeout time.Duration, s []systemE metadataLocal.SuccessfulProvider = elem.name return metadataLocal, nil } - } return metadataLocal, multierror.Append(err, errors.New("failed to reset System Event Log")) @@ -101,7 +100,6 @@ func getSystemEventLog(ctx context.Context, timeout time.Duration, s []systemEve metadataLocal.SuccessfulProvider = elem.name return sel, metadataLocal, nil } - } return nil, metadataLocal, multierror.Append(err, errors.New("failed to get System Event Log")) @@ -155,7 +153,6 @@ func getSystemEventLogRaw(ctx context.Context, timeout time.Duration, s []system metadataLocal.SuccessfulProvider = elem.name return eventlog, metadataLocal, nil } - } return eventlog, metadataLocal, multierror.Append(err, errors.New("failed to get System Event Log")) diff --git a/bmc/virtual_media.go b/bmc/virtual_media.go index 6641a0ed..97977844 100644 --- a/bmc/virtual_media.go +++ b/bmc/virtual_media.go @@ -10,7 +10,7 @@ import ( // VirtualMediaSetter controls the virtual media attached to a machine type VirtualMediaSetter interface { - SetVirtualMedia(ctx context.Context, kind string, mediaURL string) (ok bool, err error) + SetVirtualMedia(ctx context.Context, kind, mediaURL string) (ok bool, err error) } // VirtualMediaProviders is an internal struct to correlate an implementation/provider and its name @@ -20,7 +20,7 @@ type virtualMediaProviders struct { } // setVirtualMedia sets the virtual media. -func setVirtualMedia(ctx context.Context, kind string, mediaURL string, b []virtualMediaProviders) (ok bool, metadata Metadata, err error) { +func setVirtualMedia(ctx context.Context, kind, mediaURL string, b []virtualMediaProviders) (ok bool, metadata Metadata, err error) { var metadataLocal Metadata for _, elem := range b { @@ -51,7 +51,7 @@ func setVirtualMedia(ctx context.Context, kind string, mediaURL string, b []virt } // SetVirtualMediaFromInterfaces identifies implementations of the virtualMediaSetter interface and passes the found implementations to the setVirtualMedia() wrapper -func SetVirtualMediaFromInterfaces(ctx context.Context, kind string, mediaURL string, generic []interface{}) (ok bool, metadata Metadata, err error) { +func SetVirtualMediaFromInterfaces(ctx context.Context, kind, mediaURL string, generic []interface{}) (ok bool, metadata Metadata, err error) { bdSetters := make([]virtualMediaProviders, 0) for _, elem := range generic { if elem == nil { diff --git a/bmc/virtual_media_test.go b/bmc/virtual_media_test.go index e39fecbb..d72f8689 100644 --- a/bmc/virtual_media_test.go +++ b/bmc/virtual_media_test.go @@ -15,7 +15,7 @@ type virtualMediaTester struct { MakeErrorOut bool } -func (r *virtualMediaTester) SetVirtualMedia(ctx context.Context, kind string, mediaURL string) (ok bool, err error) { +func (r *virtualMediaTester) SetVirtualMedia(ctx context.Context, kind, mediaURL string) (ok bool, err error) { if r.MakeErrorOut { return ok, errors.New("setting virtual media failed") } diff --git a/client.go b/client.go index f584fed3..52214bd6 100644 --- a/client.go +++ b/client.go @@ -501,7 +501,7 @@ func (c *Client) SetBootDevice(ctx context.Context, bootDevice string, setPersis // server. Specifically, the method ejects any currently attached virtual media, and then if // mediaURL isn't empty, attaches a virtual media device of type kind whose contents are // streamed from the indicated URL. -func (c *Client) SetVirtualMedia(ctx context.Context, kind string, mediaURL string) (ok bool, err error) { +func (c *Client) SetVirtualMedia(ctx context.Context, kind, mediaURL string) (ok bool, err error) { ctx, span := c.traceprovider.Tracer(pkgName).Start(ctx, "SetVirtualMedia") defer span.End() @@ -588,7 +588,7 @@ func (c *Client) ResetBiosConfiguration(ctx context.Context) (err error) { } // FirmwareInstall pass through library function to upload firmware and install firmware -func (c *Client) FirmwareInstall(ctx context.Context, component string, operationApplyTime string, forceInstall bool, reader io.Reader) (taskID string, err error) { +func (c *Client) FirmwareInstall(ctx context.Context, component, operationApplyTime string, forceInstall bool, reader io.Reader) (taskID string, err error) { ctx, span := c.traceprovider.Tracer(pkgName).Start(ctx, "FirmwareInstall") defer span.End() @@ -599,7 +599,7 @@ func (c *Client) FirmwareInstall(ctx context.Context, component string, operatio return taskID, err } -// Note: this interface is to be deprecated in favour of a more generic FirmwareTaskStatus. +// Note: this interface is to be deprecated in favor of a more generic FirmwareTaskStatus. // // FirmwareInstallStatus pass through library function to check firmware install status func (c *Client) FirmwareInstallStatus(ctx context.Context, installVersion, component, taskID string) (status string, err error) { diff --git a/constants/constants.go b/constants/constants.go index d6f8a18d..87db4dcd 100644 --- a/constants/constants.go +++ b/constants/constants.go @@ -11,7 +11,7 @@ type ( ) const ( - // EnvEnableDebug is the const for the environment variable to cause bmclib to dump debugging debugging information. + // EnvEnableDebug is the const for the environment variable to cause bmclib to dump debugging information. // the valid parameter for this environment variable is 'true' EnvEnableDebug = "DEBUG_BMCLIB" diff --git a/errors/errors.go b/errors/errors.go index 9f0eaeaa..34dcaa5b 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -15,7 +15,7 @@ var ( // ErrNotAuthenticated is returned when the session is not active. ErrNotAuthenticated = errors.New("not authenticated") - // ErrNon200Response is returned when bmclib recieves an unexpected non-200 status code for a query + // ErrNon200Response is returned when bmclib receives an unexpected non-200 status code for a query ErrNon200Response = errors.New("non-200 response returned for the endpoint") // ErrNotImplemented is returned for not implemented methods called diff --git a/internal/ipmi/ipmi.go b/internal/ipmi/ipmi.go index 98c74424..f7b386ae 100644 --- a/internal/ipmi/ipmi.go +++ b/internal/ipmi/ipmi.go @@ -45,7 +45,7 @@ func WithLogger(log logr.Logger) Option { } // New returns a new ipmi instance -func New(username string, password string, host string, opts ...Option) (ipmi *Ipmi, err error) { +func New(username, password, host string, opts ...Option) (ipmi *Ipmi, err error) { ipmi = &Ipmi{ Username: username, Password: password, @@ -133,13 +133,13 @@ func formatOptions(opts []string) []cmdOpt { func (i *Ipmi) PowerCycle(ctx context.Context) (status bool, err error) { output, err := i.run(ctx, []string{"chassis", "power", "cycle"}) if err != nil { - return false, fmt.Errorf("%v: %v", err, output) + return false, fmt.Errorf("%w: %v", err, output) } if strings.HasPrefix(output, "Chassis Power Control: Cycle") { return true, err } - return false, fmt.Errorf("%v: %v", err, output) + return false, fmt.Errorf("%w: %v", err, output) } // ForceRestart does the chassis power cycle even if the chassis is turned off. @@ -149,7 +149,7 @@ func (i *Ipmi) PowerCycle(ctx context.Context) (status bool, err error) { func (i *Ipmi) ForceRestart(ctx context.Context) (status bool, err error) { output, err := i.run(ctx, []string{"chassis", "power", "status"}) if err != nil { - return false, fmt.Errorf("%v: %v", err, output) + return false, fmt.Errorf("%w: %v", err, output) } command := "on" @@ -158,29 +158,29 @@ func (i *Ipmi) ForceRestart(ctx context.Context) (status bool, err error) { command = "cycle" reply = "Cycle" } else if !strings.HasPrefix(output, "Chassis Power is off") { - return false, fmt.Errorf("%v: %v", err, output) + return false, fmt.Errorf("%w: %v", err, output) } output, err = i.run(ctx, []string{"chassis", "power", command}) if err != nil { - return false, fmt.Errorf("%v: %v", err, output) + return false, fmt.Errorf("%w: %v", err, output) } if strings.HasPrefix(output, "Chassis Power Control: "+reply) { return true, err } - return false, fmt.Errorf("%v: %v", err, output) + return false, fmt.Errorf("%w: %v", err, output) } // PowerReset reboots the machine via bmc func (i *Ipmi) PowerReset(ctx context.Context) (status bool, err error) { output, err := i.run(ctx, []string{"chassis", "power", "reset"}) if err != nil { - return false, fmt.Errorf("%v: %v", err, output) + return false, fmt.Errorf("%w: %v", err, output) } if !strings.HasPrefix(output, "Chassis Power Control: Reset") { - return false, fmt.Errorf("%v: %v", err, output) + return false, fmt.Errorf("%w: %v", err, output) } return true, err } @@ -189,26 +189,26 @@ func (i *Ipmi) PowerReset(ctx context.Context) (status bool, err error) { func (i *Ipmi) PowerCycleBmc(ctx context.Context) (status bool, err error) { output, err := i.run(ctx, []string{"mc", "reset", "cold"}) if err != nil { - return false, fmt.Errorf("%v: %v", err, output) + return false, fmt.Errorf("%w: %v", err, output) } if strings.HasPrefix(output, "Sent cold reset command to MC") { return true, err } - return false, fmt.Errorf("%v: %v", err, output) + return false, fmt.Errorf("%w: %v", err, output) } // PowerResetBmc reboots the bmc we are connected to func (i *Ipmi) PowerResetBmc(ctx context.Context, resetType string) (ok bool, err error) { output, err := i.run(ctx, []string{"mc", "reset", strings.ToLower(resetType)}) if err != nil { - return false, fmt.Errorf("%v: %v", err, output) + return false, fmt.Errorf("%w: %v", err, output) } if strings.HasPrefix(output, fmt.Sprintf("Sent %v reset command to MC", strings.ToLower(resetType))) { return true, err } - return false, fmt.Errorf("%v: %v", err, output) + return false, fmt.Errorf("%w: %v", err, output) } // PowerOn power on the machine via bmc @@ -223,7 +223,7 @@ func (i *Ipmi) PowerOn(ctx context.Context) (status bool, err error) { output, err := i.run(ctx, []string{"chassis", "power", "on"}) if err != nil { - return false, fmt.Errorf("%v: %v", err, output) + return false, fmt.Errorf("%w: %v", err, output) } if strings.HasPrefix(output, "Chassis Power Control: Up/On") { @@ -236,13 +236,13 @@ func (i *Ipmi) PowerOn(ctx context.Context) (status bool, err error) { func (i *Ipmi) PowerOnForce(ctx context.Context) (status bool, err error) { output, err := i.run(ctx, []string{"chassis", "power", "on"}) if err != nil { - return false, fmt.Errorf("%v: %v", err, output) + return false, fmt.Errorf("%w: %v", err, output) } if strings.HasPrefix(output, "Chassis Power Control: Up/On") { return true, err } - return false, fmt.Errorf("%v: %v", err, output) + return false, fmt.Errorf("%w: %v", err, output) } // PowerOff power off the machine via bmc @@ -251,7 +251,7 @@ func (i *Ipmi) PowerOff(ctx context.Context) (status bool, err error) { if strings.Contains(output, "Chassis Power Control: Down/Off") { return true, err } - return false, fmt.Errorf("%v: %v", err, output) + return false, fmt.Errorf("%w: %v", err, output) } // PowerSoft power off the machine via bmc @@ -263,7 +263,7 @@ func (i *Ipmi) PowerSoft(ctx context.Context) (status bool, err error) { output, err := i.run(ctx, []string{"chassis", "power", "soft"}) if !strings.Contains(output, "Chassis Power Control: Soft") { - return false, fmt.Errorf("%v: %v", err, output) + return false, fmt.Errorf("%w: %v", err, output) } return true, err } @@ -272,13 +272,13 @@ func (i *Ipmi) PowerSoft(ctx context.Context) (status bool, err error) { func (i *Ipmi) PxeOnceEfi(ctx context.Context) (status bool, err error) { output, err := i.run(ctx, []string{"chassis", "bootdev", "pxe", "options=efiboot"}) if err != nil { - return false, fmt.Errorf("%v: %v", err, output) + return false, fmt.Errorf("%w: %v", err, output) } if strings.Contains(output, "Set Boot Device to pxe") { return true, err } - return false, fmt.Errorf("%v: %v", err, output) + return false, fmt.Errorf("%w: %v", err, output) } // BootDeviceSet sets the next boot device with options @@ -302,26 +302,26 @@ func (i *Ipmi) BootDeviceSet(ctx context.Context, bootDevice string, setPersiste output, err := i.run(ctx, ipmiCmd) if err != nil { - return false, fmt.Errorf("%v: %v", err, output) + return false, fmt.Errorf("%w: %v", err, output) } if strings.Contains(output, fmt.Sprintf("Set Boot Device to %v", strings.ToLower(bootDevice))) { return true, err } - return false, fmt.Errorf("%v: %v", err, output) + return false, fmt.Errorf("%w: %v", err, output) } // PxeOnceMbr makes the machine to boot via pxe once using MBR func (i *Ipmi) PxeOnceMbr(ctx context.Context) (status bool, err error) { output, err := i.run(ctx, []string{"chassis", "bootdev", "pxe"}) if err != nil { - return false, fmt.Errorf("%v: %v", err, output) + return false, fmt.Errorf("%w: %v", err, output) } if strings.Contains(output, "Set Boot Device to pxe") { return true, err } - return false, fmt.Errorf("%v: %v", err, output) + return false, fmt.Errorf("%w: %v", err, output) } // PxeOnce makes the machine to boot via pxe once using MBR @@ -333,7 +333,7 @@ func (i *Ipmi) PxeOnce(ctx context.Context) (status bool, err error) { func (i *Ipmi) IsOn(ctx context.Context) (status bool, err error) { output, err := i.run(ctx, []string{"chassis", "power", "status"}) if err != nil { - return false, fmt.Errorf("%v: %v", err, output) + return false, fmt.Errorf("%w: %v", err, output) } if strings.Contains(output, "Chassis Power is on") { diff --git a/internal/redfishwrapper/firmware_test.go b/internal/redfishwrapper/firmware_test.go index 1599fea1..94aa5009 100644 --- a/internal/redfishwrapper/firmware_test.go +++ b/internal/redfishwrapper/firmware_test.go @@ -39,7 +39,7 @@ func TestRunRequestWithMultipartPayload(t *testing.T) { defer os.Remove(binPath) multipartEndpoint := func(w http.ResponseWriter, r *http.Request) { - if r.Method != "POST" { + if r.Method != http.MethodPost { w.WriteHeader(http.StatusNotFound) } diff --git a/internal/redfishwrapper/inventory.go b/internal/redfishwrapper/inventory.go index e387cc3d..9e7d2804 100644 --- a/internal/redfishwrapper/inventory.go +++ b/internal/redfishwrapper/inventory.go @@ -176,7 +176,6 @@ func (c *Client) chassisAttributes(ctx context.Context, device *common.Device, f if err != nil && failOnError { return err } - } err = c.collectCPLDs(device, softwareInventory) @@ -264,7 +263,6 @@ func (c *Client) firmwareAttributes(slug, id string, firmwareObj *common.Firmwar // include previously installed firmware attributes if strings.HasPrefix(inv.ID, "Previous") { if strings.Contains(inv.ID, id) || strings.EqualFold(slug, inv.Name) { - if firmwareObj == nil { firmwareObj = &common.Firmware{} } @@ -283,7 +281,6 @@ func (c *Client) firmwareAttributes(slug, id string, firmwareObj *common.Firmwar // update firmwareObj with installed firmware attributes if strings.HasPrefix(inv.ID, "Installed") { if strings.Contains(inv.ID, id) || strings.EqualFold(slug, inv.Name) { - if firmwareObj == nil { firmwareObj = &common.Firmware{} } diff --git a/internal/redfishwrapper/inventory_collect.go b/internal/redfishwrapper/inventory_collect.go index 343de616..83d1cfcc 100644 --- a/internal/redfishwrapper/inventory_collect.go +++ b/internal/redfishwrapper/inventory_collect.go @@ -77,7 +77,6 @@ func (c *Client) collectPSUs(ch *redfish.Chassis, device *common.Device, softwar c.firmwareAttributes(common.SlugPSU, psu.ID, p.Firmware, softwareInventory) device.PSUs = append(device.PSUs, p) - } return nil } @@ -160,7 +159,6 @@ func (c *Client) collectNICs(sys *redfish.ComputerSystem, device *common.Device, portFirmwareVersion := getFirmwareVersionFromController(adapter.Controllers, len(ports)) for _, networkPort := range ports { - // populate network ports general data nicPort := &common.NICPort{} c.collectNetworkPortInfo(nicPort, adapter, networkPort, portFirmwareVersion, softwareInventory) @@ -200,7 +198,6 @@ func (c *Client) collectNetworkPortInfo( } if networkPort != nil { - nicPort.Description = networkPort.Description nicPort.PCIVendorID = networkPort.VendorID nicPort.Status = &common.Status{ @@ -358,9 +355,7 @@ func (c *Client) collectDrives(sys *redfish.ComputerSystem, device *common.Devic c.firmwareAttributes("Disk", drive.ID, d.Firmware, softwareInventory) device.Drives = append(device.Drives, d) - } - } return nil @@ -375,7 +370,6 @@ func (c *Client) collectStorageControllers(sys *redfish.ComputerSystem, device * for _, member := range storage { for _, controller := range member.StorageControllers { - cs := &common.StorageController{ Common: common.Common{ Description: controller.Name, diff --git a/internal/redfishwrapper/power.go b/internal/redfishwrapper/power.go index f08b6e03..5790ed53 100644 --- a/internal/redfishwrapper/power.go +++ b/internal/redfishwrapper/power.go @@ -119,7 +119,6 @@ func (c *Client) SystemReset(ctx context.Context) (ok bool, err error) { system.DisableEtagMatch(c.disableEtagMatch) err = system.Reset(rf.PowerCycleResetType) if err != nil { - _, _ = c.SystemPowerOff(ctx) for wait := 1; wait < 10; wait++ { diff --git a/internal/redfishwrapper/virtual_media.go b/internal/redfishwrapper/virtual_media.go index c358e932..fdedeee7 100644 --- a/internal/redfishwrapper/virtual_media.go +++ b/internal/redfishwrapper/virtual_media.go @@ -10,7 +10,7 @@ import ( ) // Set the virtual media attached to the system, or just eject everything if mediaURL is empty. -func (c *Client) SetVirtualMedia(ctx context.Context, kind string, mediaURL string) (bool, error) { +func (c *Client) SetVirtualMedia(ctx context.Context, kind, mediaURL string) (bool, error) { managers, err := c.Managers(ctx) if err != nil { return false, err @@ -54,15 +54,15 @@ func (c *Client) SetVirtualMedia(ctx context.Context, kind string, mediaURL stri // Only ejecting the media was requested. if vm.Inserted && vm.SupportsMediaEject { if err := vm.EjectMedia(); err != nil { - return false, fmt.Errorf("error ejecting media: %v", err) + return false, fmt.Errorf("error ejecting media: %w", err) } } return true, nil } - // Ejecting the media before inserting a new new media makes the success rate of inserting the new media higher. + // Ejecting the media before inserting a new media makes the success rate of inserting the new media higher. if vm.Inserted && vm.SupportsMediaEject { if err := vm.EjectMedia(); err != nil { - return false, fmt.Errorf("error ejecting media before inserting media: %v", err) + return false, fmt.Errorf("error ejecting media before inserting media: %w", err) } } diff --git a/internal/sshclient/sshclient.go b/internal/sshclient/sshclient.go index 0533e88a..c1464223 100644 --- a/internal/sshclient/sshclient.go +++ b/internal/sshclient/sshclient.go @@ -23,7 +23,7 @@ type SSHClient struct { } // New creates a new SSH client -func New(addr string, username string, password string) (*SSHClient, error) { +func New(addr, username, password string) (*SSHClient, error) { cfg := &ssh.ClientConfig{ User: username, Auth: []ssh.AuthMethod{ diff --git a/providers/asrockrack/asrockrack.go b/providers/asrockrack/asrockrack.go index fcbbb0e6..23a55358 100644 --- a/providers/asrockrack/asrockrack.go +++ b/providers/asrockrack/asrockrack.go @@ -80,12 +80,12 @@ func WithHTTPClient(c *http.Client) ASRockOption { } // New returns a new ASRockRack instance ready to be used -func New(ip string, username string, password string, log logr.Logger) *ASRockRack { +func New(ip, username, password string, log logr.Logger) *ASRockRack { return NewWithOptions(ip, username, password, log) } // NewWithOptions returns a new ASRockRack instance with options ready to be used -func NewWithOptions(ip string, username string, password string, log logr.Logger, opts ...ASRockOption) *ASRockRack { +func NewWithOptions(ip, username, password string, log logr.Logger, opts ...ASRockOption) *ASRockRack { r := &ASRockRack{ ip: ip, username: username, diff --git a/providers/asrockrack/firmware.go b/providers/asrockrack/firmware.go index 5146a87b..516e1abd 100644 --- a/providers/asrockrack/firmware.go +++ b/providers/asrockrack/firmware.go @@ -184,7 +184,7 @@ func (a *ASRockRack) FirmwareTaskStatus(ctx context.Context, kind constants.Firm } // firmwareUpdateBIOSStatus returns the BIOS firmware install status -func (a *ASRockRack) firmwareUpdateStatus(ctx context.Context, component string, installVersion string) (state constants.TaskState, status string, err error) { +func (a *ASRockRack) firmwareUpdateStatus(ctx context.Context, component, installVersion string) (state constants.TaskState, status string, err error) { var endpoint string component = strings.ToUpper(component) switch component { diff --git a/providers/asrockrack/mock_test.go b/providers/asrockrack/mock_test.go index 85264608..1dcc7755 100644 --- a/providers/asrockrack/mock_test.go +++ b/providers/asrockrack/mock_test.go @@ -99,27 +99,27 @@ func mockASRockBMC() *httptest.Server { func index(w http.ResponseWriter, r *http.Request) { switch r.Method { - case "GET": + case http.MethodGet: _, _ = w.Write([]byte(`ASRockRack`)) } } func userAccountList(w http.ResponseWriter, r *http.Request) { switch r.Method { - case "GET": + case http.MethodGet: if os.Getenv("TEST_FAIL_QUERY") != "" { w.WriteHeader(http.StatusInternalServerError) } else { _, _ = w.Write(usersPayload) } - case "PUT": + case http.MethodPut: httpRequestTestVar = r } } func biosFirmwareUpgrade(w http.ResponseWriter, r *http.Request) { switch r.Method { - case "POST": + case http.MethodPost: switch r.RequestURI { case "/api/asrr/maintenance/BIOS/firmware": @@ -139,7 +139,7 @@ func biosFirmwareUpgrade(w http.ResponseWriter, r *http.Request) { func bmcFirmwareUpgrade(w http.ResponseWriter, r *http.Request) { switch r.Method { - case "GET": + case http.MethodGet: switch r.RequestURI { // 3. bmc verifies uploaded firmware image case "/api/maintenance/firmware/verification": @@ -168,7 +168,7 @@ func bmcFirmwareUpgrade(w http.ResponseWriter, r *http.Request) { resp = bytes.Replace(resp, []byte("__PERCENT__"), []byte(strconv.Itoa(fwUpgradeState.UpgradePercent)), 1) _, _ = w.Write(resp) } - case "PUT": + case http.MethodPut: switch r.RequestURI { // 1. set device to flash mode @@ -208,7 +208,7 @@ func bmcFirmwareUpgrade(w http.ResponseWriter, r *http.Request) { _, _ = b.ReadFrom(r.Body) _, _ = w.Write(b.Bytes()) } - case "POST": + case http.MethodPost: switch r.RequestURI { case "/api/maintenance/reset": w.WriteHeader(http.StatusOK) @@ -242,28 +242,28 @@ func bmcFirmwareUpgrade(w http.ResponseWriter, r *http.Request) { func fwinfo(w http.ResponseWriter, r *http.Request) { switch r.Method { - case "GET": + case http.MethodGet: _, _ = w.Write(fwinfoResponse) } } func fruinfo(w http.ResponseWriter, r *http.Request) { switch r.Method { - case "GET": + case http.MethodGet: _, _ = w.Write(fruinfoResponse) } } func inventoryinfo(w http.ResponseWriter, r *http.Request) { switch r.Method { - case "GET": + case http.MethodGet: _, _ = w.Write(inventoryinfoResponse) } } func sensorsinfo(w http.ResponseWriter, r *http.Request) { switch r.Method { - case "GET": + case http.MethodGet: fh, err := os.Open("./fixtures/E3C246D4I-NL/sensors.json") if err != nil { log.Fatal(err) @@ -279,21 +279,21 @@ func sensorsinfo(w http.ResponseWriter, r *http.Request) { func biosPOSTCodeinfo(w http.ResponseWriter, r *http.Request) { switch r.Method { - case "GET": + case http.MethodGet: _, _ = w.Write(biosPOSTCodeResponse) } } func chassisStatusInfo(w http.ResponseWriter, r *http.Request) { switch r.Method { - case "GET": + case http.MethodGet: _, _ = w.Write(chassisStatusResponse) } } func session(w http.ResponseWriter, r *http.Request) { switch r.Method { - case "POST": + case http.MethodPost: // login to BMC b, _ := io.ReadAll(r.Body) if string(b) == string(loginPayload) { @@ -308,7 +308,7 @@ func session(w http.ResponseWriter, r *http.Request) { } else { w.WriteHeader(http.StatusBadRequest) } - case "DELETE": + case http.MethodDelete: if r.Header.Get("X-Csrftoken") != "l5L29IP7" { w.WriteHeader(http.StatusBadRequest) } diff --git a/providers/asrockrack/user_test.go b/providers/asrockrack/user_test.go index 39432e3c..866d897b 100644 --- a/providers/asrockrack/user_test.go +++ b/providers/asrockrack/user_test.go @@ -75,7 +75,7 @@ func Test_UserRead(t *testing.T) { } // test account retrieval failure error - os.Setenv("TEST_FAIL_QUERY", "womp womp") + os.Setenv("TEST_FAIL_QUERY", "womp") defer os.Unsetenv("TEST_FAIL_QUERY") _, err = aClient.UserRead(context.TODO()) diff --git a/providers/dell/firmware.go b/providers/dell/firmware.go index 47ca86de..2bbd7298 100644 --- a/providers/dell/firmware.go +++ b/providers/dell/firmware.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "io" + "net/http" "os" "strings" "time" @@ -165,7 +166,7 @@ func (c *Conn) job(jobID string) (*Dell, error) { return nil, errors.Wrap(errLookup, err.Error()) } - if resp.StatusCode != 200 { + if resp.StatusCode != http.StatusOK { return nil, errors.Wrap(errLookup, "unexpected status code: "+resp.Status) } diff --git a/providers/dell/idrac.go b/providers/dell/idrac.go index 58995ee8..1e41b83c 100644 --- a/providers/dell/idrac.go +++ b/providers/dell/idrac.go @@ -135,7 +135,7 @@ func (c *Conn) Open(ctx context.Context) (err error) { // is available across various BMC vendors, we verify the device we're connected to is dell. if err := c.deviceSupported(ctx); err != nil { if er := c.redfishwrapper.Close(ctx); er != nil { - return fmt.Errorf("%v: %w", err, er) + return fmt.Errorf("%w: %w", err, er) } return err @@ -276,7 +276,7 @@ func (c *Conn) Screenshot(ctx context.Context) (image []byte, fileType string, e return nil, "", errors.Wrap(bmclibErrs.ErrScreenshot, err.Error()) } - if resp.StatusCode != 200 { + if resp.StatusCode != http.StatusOK { return nil, "", errors.Wrap(bmclibErrs.ErrScreenshot, resp.Status) } diff --git a/providers/intelamt/intelamt.go b/providers/intelamt/intelamt.go index 95376840..721e244d 100644 --- a/providers/intelamt/intelamt.go +++ b/providers/intelamt/intelamt.go @@ -71,7 +71,7 @@ type Config struct { } // New creates a new AMT connection -func New(host string, user string, pass string, opts ...Option) *Conn { +func New(host, user, pass string, opts ...Option) *Conn { defaultClient := &Config{ HostScheme: "http", Port: 16992, diff --git a/providers/redfish/redfish.go b/providers/redfish/redfish.go index c30e8014..51e7c9a7 100644 --- a/providers/redfish/redfish.go +++ b/providers/redfish/redfish.go @@ -213,7 +213,7 @@ func (c *Conn) BootDeviceOverrideGet(ctx context.Context) (bmc.BootDeviceOverrid } // SetVirtualMedia sets the virtual media -func (c *Conn) SetVirtualMedia(ctx context.Context, kind string, mediaURL string) (ok bool, err error) { +func (c *Conn) SetVirtualMedia(ctx context.Context, kind, mediaURL string) (ok bool, err error) { return c.redfishwrapper.SetVirtualMedia(ctx, kind, mediaURL) } diff --git a/providers/rpc/http_test.go b/providers/rpc/http_test.go index 14dcf801..33e7cd61 100644 --- a/providers/rpc/http_test.go +++ b/providers/rpc/http_test.go @@ -64,7 +64,7 @@ func TestResponseKVS(t *testing.T) { expected []interface{} }{ "one": { - resp: &http.Response{StatusCode: 200, Header: http.Header{"Content-Type": []string{"application/json"}}, Body: io.NopCloser(strings.NewReader(`{"id":1,"host":"127.0.0.1"}`))}, + resp: &http.Response{StatusCode: http.StatusOK, Header: http.Header{"Content-Type": []string{"application/json"}}, Body: io.NopCloser(strings.NewReader(`{"id":1,"host":"127.0.0.1"}`))}, expected: []interface{}{"response", responseDetails{ StatusCode: 200, Headers: http.Header{"Content-Type": {"application/json"}}, diff --git a/providers/rpc/rpc.go b/providers/rpc/rpc.go index 0432c2fd..852c6ac5 100644 --- a/providers/rpc/rpc.go +++ b/providers/rpc/rpc.go @@ -132,7 +132,7 @@ type Experimental struct { } // New returns a new Config containing all the defaults for the rpc provider. -func New(consumerURL string, host string, secrets Secrets) *Provider { +func New(consumerURL, host string, secrets Secrets) *Provider { // defaults c := &Provider{ Host: host, diff --git a/providers/supermicro/firmware_bios_test.go b/providers/supermicro/firmware_bios_test.go index 3ab4a128..1c780021 100644 --- a/providers/supermicro/firmware_bios_test.go +++ b/providers/supermicro/firmware_bios_test.go @@ -153,7 +153,7 @@ func Test_setBIOSFirmwareInstallMode(t *testing.T) { "400", "/cgi/ipmi.cgi", func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(400) + w.WriteHeader(http.StatusBadRequest) }, }, } diff --git a/providers/supermicro/supermicro_test.go b/providers/supermicro/supermicro_test.go index 8d3064dc..663615bb 100644 --- a/providers/supermicro/supermicro_test.go +++ b/providers/supermicro/supermicro_test.go @@ -194,7 +194,7 @@ func TestOpen(t *testing.T) { assert.Equal(t, r.Method, http.MethodPost) assert.Equal(t, r.Header.Get("Content-Type"), "application/x-www-form-urlencoded") response := []byte(`barf`) - w.WriteHeader(401) + w.WriteHeader(http.StatusUnauthorized) _, _ = w.Write(response) }, }, @@ -261,7 +261,7 @@ func TestClose(t *testing.T) { t.Fatal(err) } - w.WriteHeader(200) + w.WriteHeader(http.StatusOK) }, }, } @@ -325,7 +325,7 @@ func TestInitScreenPreview(t *testing.T) { "400", "/cgi/op.cgi", func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(400) + w.WriteHeader(http.StatusBadRequest) }, }, } @@ -382,7 +382,7 @@ func TestFetchScreenPreview(t *testing.T) { "400", "/cgi/url_redirect.cgi", func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(400) + w.WriteHeader(http.StatusBadRequest) }, }, } diff --git a/providers/supermicro/x11_firmware_bios.go b/providers/supermicro/x11_firmware_bios.go index 14cb7bf2..417e3e16 100644 --- a/providers/supermicro/x11_firmware_bios.go +++ b/providers/supermicro/x11_firmware_bios.go @@ -85,7 +85,6 @@ func (c *x11) checkComponentUpdateMisc(ctx context.Context, stage string) error // When SYSOFF=1 the system requires a power cycle default: return errors.New("unknown stage: " + stage) - } headers := map[string]string{ diff --git a/providers/supermicro/x11_firmware_bmc_test.go b/providers/supermicro/x11_firmware_bmc_test.go index 69ec8911..44eb7ce0 100644 --- a/providers/supermicro/x11_firmware_bmc_test.go +++ b/providers/supermicro/x11_firmware_bmc_test.go @@ -73,7 +73,7 @@ func TestX11SetBMCFirmwareInstallMode(t *testing.T) { "400", "/cgi/ipmi.cgi", func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(400) + w.WriteHeader(http.StatusBadRequest) }, }, } From eb44621983c3b84c0f2d1b7a06e480d9742862d4 Mon Sep 17 00:00:00 2001 From: Manuel Mendez Date: Mon, 10 Mar 2025 17:46:12 -0400 Subject: [PATCH 7/8] Apply manual fixes to appease linters Drop enabling govets shadow because it's too annoying --- .golangci.yml | 5 +--- bmc/firmware.go | 7 +++-- bmc/inventory_test.go | 3 +-- client.go | 33 ++++++++++++------------ client_test.go | 7 ++--- internal/httpclient/httpclient_test.go | 9 ++++--- internal/redfishwrapper/bios_test.go | 4 +-- internal/redfishwrapper/client.go | 8 +++--- internal/redfishwrapper/firmware.go | 16 ++++++------ internal/redfishwrapper/firmware_test.go | 5 ++-- internal/redfishwrapper/task.go | 4 ++- providers/asrockrack/firmware.go | 4 +-- providers/asrockrack/helpers.go | 2 +- providers/asrockrack/user_test.go | 4 +-- providers/dell/idrac.go | 4 ++- providers/rpc/http_test.go | 5 +++- providers/rpc/signature.go | 16 ++++++------ providers/supermicro/supermicro.go | 18 ++++--------- providers/supermicro/supermicro_test.go | 2 +- 19 files changed, 77 insertions(+), 79 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 7f69cd9f..92c38b0e 100644 --- a/.golangci.yml +++ b/.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: diff --git a/bmc/firmware.go b/bmc/firmware.go index a3df6a56..a30ed8d5 100644 --- a/bmc/firmware.go +++ b/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) diff --git a/bmc/inventory_test.go b/bmc/inventory_test.go index b1b7401d..c394c00e 100644 --- a/bmc/inventory_test.go +++ b/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 { diff --git a/client.go b/client.go index 52214bd6..31bdc002 100644 --- a/client.go +++ b/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,14 +238,12 @@ 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), ) @@ -248,14 +251,12 @@ func (c *Client) registerSupermicroProvider() { } 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() diff --git a/client_test.go b/client_test.go index 4b25deb1..16219967 100644 --- a/client_test.go +++ b/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 } diff --git a/internal/httpclient/httpclient_test.go b/internal/httpclient/httpclient_test.go index a70b797f..fdf10642 100644 --- a/internal/httpclient/httpclient_test.go +++ b/internal/httpclient/httpclient_test.go @@ -2,6 +2,7 @@ package httpclient import ( "crypto/x509" + "errors" "fmt" "net/http" "net/http/httptest" @@ -52,9 +53,9 @@ 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") @@ -62,7 +63,7 @@ func TestBuildWithOptions(t *testing.T) { // 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() }) } } diff --git a/internal/redfishwrapper/bios_test.go b/internal/redfishwrapper/bios_test.go index 643ebda2..ece0ce30 100644 --- a/internal/redfishwrapper/bios_test.go +++ b/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) } diff --git a/internal/redfishwrapper/client.go b/internal/redfishwrapper/client.go index 245f2acd..ec8a6dd7 100644 --- a/internal/redfishwrapper/client.go +++ b/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) { diff --git a/internal/redfishwrapper/firmware.go b/internal/redfishwrapper/firmware.go index 82ee6a36..2f3a45a4 100644 --- a/internal/redfishwrapper/firmware.go +++ b/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") } diff --git a/internal/redfishwrapper/firmware_test.go b/internal/redfishwrapper/firmware_test.go index 94aa5009..b563b50c 100644 --- a/internal/redfishwrapper/firmware_test.go +++ b/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) } diff --git a/internal/redfishwrapper/task.go b/internal/redfishwrapper/task.go index 859139d8..fe761cdf 100644 --- a/internal/redfishwrapper/task.go +++ b/internal/redfishwrapper/task.go @@ -58,7 +58,7 @@ func (c *Client) taskMessagesAsString(messages []common.Message) string { return "" } - var found []string + var found []string // nolint:prealloc for _, m := range messages { if m.Message == "" { continue @@ -95,6 +95,8 @@ func (c *Client) TaskStateActive(state constants.TaskState) (bool, error) { return true, nil case constants.Complete, constants.Failed: return false, nil + case constants.PowerCycleHost, constants.Unknown: + fallthrough default: return false, errors.Wrap(errUnexpectedTaskState, string(state)) } diff --git a/providers/asrockrack/firmware.go b/providers/asrockrack/firmware.go index 516e1abd..7922744d 100644 --- a/providers/asrockrack/firmware.go +++ b/providers/asrockrack/firmware.go @@ -215,7 +215,7 @@ func (a *ASRockRack) firmwareUpdateStatus(ctx context.Context, component, instal case 2: return constants.Complete, status, nil default: - a.log.V(3).WithValues("state", progress.State).Info("warn", "bmc returned unknown flash progress state") + a.log.V(3).WithValues("state", progress.State).Info("warn: bmc returned unknown flash progress state") } } @@ -262,7 +262,7 @@ func (a *ASRockRack) versionInstalled(ctx context.Context, component, version st fwInfo, err := a.firmwareInfo(ctx) if err != nil { err = errors.Wrap(err, "error querying for firmware info: ") - a.log.V(3).Info("warn", err.Error()) + a.log.V(3).Info("warn", "err", err.Error()) return versionStrError, err } diff --git a/providers/asrockrack/helpers.go b/providers/asrockrack/helpers.go index 480796b4..f9418c72 100644 --- a/providers/asrockrack/helpers.go +++ b/providers/asrockrack/helpers.go @@ -413,7 +413,7 @@ func (a *ASRockRack) fruInfo(ctx context.Context) ([]*fru, error) { return nil, fmt.Errorf("non 200 response: %d", statusCode) } - data := []map[string]*fru{} + data := []map[string]*fru{} // nolint:musttag err = json.Unmarshal(resp, &data) if err != nil { return nil, err diff --git a/providers/asrockrack/user_test.go b/providers/asrockrack/user_test.go index 866d897b..f0db7cb0 100644 --- a/providers/asrockrack/user_test.go +++ b/providers/asrockrack/user_test.go @@ -4,7 +4,6 @@ import ( "context" "errors" "net/http" - "os" "testing" bmclibErrs "github.com/bmc-toolbox/bmclib/v2/errors" @@ -75,8 +74,7 @@ func Test_UserRead(t *testing.T) { } // test account retrieval failure error - os.Setenv("TEST_FAIL_QUERY", "womp") - defer os.Unsetenv("TEST_FAIL_QUERY") + t.Setenv("TEST_FAIL_QUERY", "womp womp") // nolint:dupword _, err = aClient.UserRead(context.TODO()) assert.Equal(t, errors.Is(err, bmclibErrs.ErrRetrievingUserAccounts), true) diff --git a/providers/dell/idrac.go b/providers/dell/idrac.go index 1e41b83c..0b4f4241 100644 --- a/providers/dell/idrac.go +++ b/providers/dell/idrac.go @@ -242,7 +242,7 @@ func (c *Conn) SendNMI(ctx context.Context) error { } // deviceManufacturer returns the device manufacturer and model attributes -func (c *Conn) deviceManufacturer(ctx context.Context) (vendor string, err error) { +func (c *Conn) deviceManufacturer(context.Context) (vendor string, err error) { systems, err := c.redfishwrapper.Systems() if err != nil { return "", errors.Wrap(errManufacturerUnknown, err.Error()) @@ -271,6 +271,8 @@ func (c *Conn) Screenshot(ctx context.Context) (image []byte, fileType string, e return nil, "", errors.Wrap(bmclibErrs.ErrScreenshot, err.Error()) } + defer resp.Body.Close() + body, err := io.ReadAll(resp.Body) if err != nil { return nil, "", errors.Wrap(bmclibErrs.ErrScreenshot, err.Error()) diff --git a/providers/rpc/http_test.go b/providers/rpc/http_test.go index 33e7cd61..2611487f 100644 --- a/providers/rpc/http_test.go +++ b/providers/rpc/http_test.go @@ -16,7 +16,10 @@ import ( func testRequest(method, reqURL string, body RequestPayload, headers http.Header) *http.Request { buf := new(bytes.Buffer) - _ = json.NewEncoder(buf).Encode(body) + err := json.NewEncoder(buf).Encode(body) + if err != nil { + panic(err) + } req, _ := http.NewRequestWithContext(context.Background(), method, reqURL, buf) req.Header = headers return req diff --git a/providers/rpc/signature.go b/providers/rpc/signature.go index 693f91d9..0467071a 100644 --- a/providers/rpc/signature.go +++ b/providers/rpc/signature.go @@ -48,9 +48,9 @@ func sign(data []byte, h Hashes, prefixSigDisabled bool) (Signatures, error) { // ToShort returns the short version of an algorithm. func (a Algorithm) ToShort() Algorithm { switch a { - case SHA256: + case SHA256, SHA256Short: return SHA256Short - case SHA512: + case SHA512, SHA512Short: return SHA512Short default: return a @@ -59,18 +59,18 @@ func (a Algorithm) ToShort() Algorithm { // NewSHA256 returns a map of SHA256 HMACs from the given secrets. func NewSHA256(secret ...string) Hashes { - var hsh []hash.Hash - for _, s := range secret { - hsh = append(hsh, hmac.New(sha256.New, []byte(s))) + hsh := make([]hash.Hash, len(secret)) + for i := range secret { + hsh[i] = hmac.New(sha256.New, []byte(secret[i])) } return Hashes{SHA256: hsh} } // NewSHA512 returns a map of SHA512 HMACs from the given secrets. func NewSHA512(secret ...string) Hashes { - var hsh []hash.Hash - for _, s := range secret { - hsh = append(hsh, hmac.New(sha512.New, []byte(s))) + hsh := make([]hash.Hash, len(secret)) + for i := range secret { + hsh[i] = hmac.New(sha512.New, []byte(secret[i])) } return Hashes{SHA512: hsh} } diff --git a/providers/supermicro/supermicro.go b/providers/supermicro/supermicro.go index 9834751c..9e659161 100644 --- a/providers/supermicro/supermicro.go +++ b/providers/supermicro/supermicro.go @@ -17,7 +17,6 @@ import ( "time" "github.com/bmc-toolbox/bmclib/v2/constants" - bmclibconsts "github.com/bmc-toolbox/bmclib/v2/constants" bmclibErrs "github.com/bmc-toolbox/bmclib/v2/errors" "github.com/bmc-toolbox/bmclib/v2/internal/httpclient" "github.com/bmc-toolbox/bmclib/v2/internal/redfishwrapper" @@ -411,7 +410,7 @@ func (c *Client) Screenshot(ctx context.Context) (image []byte, fileType string, // retrieve screen preview image, errFetch := c.fetchScreenPreview(ctx) if errFetch != nil { - return nil, "", err + return nil, "", err // nolint:nilerr } return image, fileType, nil @@ -525,7 +524,7 @@ func (c *serviceClient) supportsFirmwareInstall(model string) error { return errors.Wrap(ErrModelUnsupported, "firmware install not supported for: "+model) } -func (c *serviceClient) query(ctx context.Context, endpoint, method string, payload io.Reader, headers map[string]string, contentLength int64) ([]byte, int, error) { +func (c *serviceClient) query(ctx context.Context, endpoint, method string, payload io.Reader, headers map[string]string, _ int64) ([]byte, int, error) { var body []byte var err error var req *http.Request @@ -563,13 +562,6 @@ func (c *serviceClient) query(ctx context.Context, endpoint, method string, payl req.Header.Add(k, v) } - // Content-Length headers are ignored, unless defined in this manner - // https://go.googlesource.com/go/+/go1.20/src/net/http/request.go#165 - // https://go.googlesource.com/go/+/go1.20/src/net/http/request.go#91 - if contentLength > 0 { - req.ContentLength = contentLength - } - endpointURL, err := url.Parse(hostEndpoint) if err != nil { return nil, 0, err @@ -584,7 +576,7 @@ func (c *serviceClient) query(ctx context.Context, endpoint, method string, payl var reqDump []byte - if os.Getenv(bmclibconsts.EnvEnableDebug) == "true" { + if os.Getenv(constants.EnvEnableDebug) == "true" { reqDump, _ = httputil.DumpRequestOut(req, true) } @@ -595,7 +587,7 @@ func (c *serviceClient) query(ctx context.Context, endpoint, method string, payl // cookies are visible after the request has been made, so we dump the request and cookies here // https://github.com/golang/go/issues/22745 - if os.Getenv(bmclibconsts.EnvEnableDebug) == "true" { + if os.Getenv(constants.EnvEnableDebug) == "true" { fmt.Println(string(reqDump)) for _, v := range req.Cookies() { @@ -605,7 +597,7 @@ func (c *serviceClient) query(ctx context.Context, endpoint, method string, payl } // debug dump response - if os.Getenv(bmclibconsts.EnvEnableDebug) == "true" { + if os.Getenv(constants.EnvEnableDebug) == "true" { respDump, _ := httputil.DumpResponse(resp, true) fmt.Println(string(respDump)) diff --git a/providers/supermicro/supermicro_test.go b/providers/supermicro/supermicro_test.go index 663615bb..4487ca47 100644 --- a/providers/supermicro/supermicro_test.go +++ b/providers/supermicro/supermicro_test.go @@ -117,7 +117,7 @@ func TestOpen(t *testing.T) { "foo", "bar", handlerFuncMap{ - "/": func(w http.ResponseWriter, r *http.Request) { + "/": func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusOK) }, "/redfish/v1/": endpointFunc(t, "serviceroot.json"), From 0d3b939114cb94afae9f2c3eb0428a888257ed93 Mon Sep 17 00:00:00 2001 From: Manuel Mendez Date: Tue, 11 Mar 2025 14:33:47 -0400 Subject: [PATCH 8/8] Undisable more golangci-lint linters --- .golangci.yml | 4 +- bmc/boot_device_test.go | 5 ++- bmc/firmware_test.go | 16 ++++---- bmc/inventory_test.go | 2 +- bmc/nmi_test.go | 5 ++- bmc/postcode_test.go | 2 +- bmc/sel_test.go | 37 ++++++++++--------- errors/errors.go | 6 +-- internal/redfishwrapper/bios_test.go | 3 +- internal/redfishwrapper/firmware_test.go | 21 ++++++----- internal/redfishwrapper/main_test.go | 2 + internal/redfishwrapper/task_test.go | 13 ++++--- internal/sum/sum_test.go | 1 + providers/asrockrack/inventory_test.go | 4 +- providers/asrockrack/user_test.go | 12 +++--- providers/dell/firmware_test.go | 5 ++- providers/dell/idrac_test.go | 6 +-- providers/redfish/sel_test.go | 2 +- providers/supermicro/firmware_bios_test.go | 9 +++-- providers/supermicro/supermicro_test.go | 30 ++++++++------- providers/supermicro/x11_firmware_bmc_test.go | 29 ++++++++------- 21 files changed, 113 insertions(+), 101 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 92c38b0e..3e7dba78 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -37,7 +37,6 @@ linters: - cyclop - depguard - err113 # todo(*maybe* enable in future PR) - - errname # maybe should be enabled - exhaustruct - forbidigo - funlen @@ -64,8 +63,7 @@ linters: - perfsprint - revive # todo(enable in future PR) - tagliatelle - - tenv # should be enabled - - testifylint # should be enabled + - tenv # deprecated (since v1.64.0) due to: Duplicate feature in another linter. Replaced by usetesting. - testpackage - thelper # should be enabled - varnamelen diff --git a/bmc/boot_device_test.go b/bmc/boot_device_test.go index 0007c76d..5c759734 100644 --- a/bmc/boot_device_test.go +++ b/bmc/boot_device_test.go @@ -10,6 +10,7 @@ import ( "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) diff --git a/bmc/firmware_test.go b/bmc/firmware_test.go index 7f3c0e8a..8200f877 100644 --- a/bmc/firmware_test.go +++ b/bmc/firmware_test.go @@ -64,7 +64,7 @@ func TestFirmwareInstall(t *testing.T) { } assert.Equal(t, tc.returnTaskID, taskID) assert.Equal(t, tc.providerName, metadata.SuccessfulProvider) - assert.Equal(t, tc.providersAttempted, len(metadata.ProvidersAttempted)) + assert.Len(t, metadata.ProvidersAttempted, tc.providersAttempted) }) } } @@ -160,7 +160,7 @@ func TestFirmwareInstallStatus(t *testing.T) { } assert.Equal(t, tc.returnStatus, taskID) assert.Equal(t, tc.providerName, metadata.SuccessfulProvider) - assert.Equal(t, tc.providersAttempted, len(metadata.ProvidersAttempted)) + assert.Len(t, metadata.ProvidersAttempted, tc.providersAttempted) }) } } @@ -254,7 +254,7 @@ func TestFirmwareInstallUploadAndInitiate(t *testing.T) { } assert.Equal(t, tc.returnTaskID, taskID) assert.Equal(t, tc.providerName, metadata.SuccessfulProvider) - assert.Equal(t, tc.providersAttempted, len(metadata.ProvidersAttempted)) + assert.Len(t, metadata.ProvidersAttempted, tc.providersAttempted) }) } } @@ -348,7 +348,7 @@ func TestFirmwareInstallUploaded(t *testing.T) { } assert.Equal(t, tc.returnTaskID, taskID) assert.Equal(t, tc.providerName, metadata.SuccessfulProvider) - assert.Equal(t, tc.providersAttempted, len(metadata.ProvidersAttempted)) + assert.Len(t, metadata.ProvidersAttempted, tc.providersAttempted) }) } } @@ -442,7 +442,7 @@ func TestFirmwareUpload(t *testing.T) { } assert.Equal(t, tc.returnTaskID, taskID) assert.Equal(t, tc.providerName, metadata.SuccessfulProvider) - assert.Equal(t, tc.providersAttempted, len(metadata.ProvidersAttempted)) + assert.Len(t, metadata.ProvidersAttempted, tc.providersAttempted) }) } } @@ -547,7 +547,7 @@ func TestFirmwareInstallSteps(t *testing.T) { } assert.Equal(t, tc.returnSteps, steps) assert.Equal(t, tc.providerName, metadata.SuccessfulProvider) - assert.Equal(t, tc.providersAttempted, len(metadata.ProvidersAttempted)) + assert.Len(t, metadata.ProvidersAttempted, tc.providersAttempted) }) } } @@ -605,7 +605,7 @@ func TestFirmwareTaskStatus(t *testing.T) { assert.Equal(t, tc.returnState, state) assert.Equal(t, tc.returnStatus, status) assert.Equal(t, tc.providerName, metadata.SuccessfulProvider) - assert.Equal(t, tc.providersAttempted, len(metadata.ProvidersAttempted)) + assert.Len(t, metadata.ProvidersAttempted, tc.providersAttempted) }) } } @@ -653,7 +653,7 @@ func TestFirmwareTaskStatusFromInterfaces(t *testing.T) { assert.Equal(t, tc.returnState, state) assert.Equal(t, tc.returnStatus, status) assert.Equal(t, tc.providerName, metadata.SuccessfulProvider) - assert.Equal(t, tc.providersAttempted, len(metadata.ProvidersAttempted)) + assert.Len(t, metadata.ProvidersAttempted, tc.providersAttempted) }) } } diff --git a/bmc/inventory_test.go b/bmc/inventory_test.go index c394c00e..1489f697 100644 --- a/bmc/inventory_test.go +++ b/bmc/inventory_test.go @@ -56,7 +56,7 @@ func TestInventory(t *testing.T) { } assert.Equal(t, tc.returnDevice, device) assert.Equal(t, tc.providerName, metadata.SuccessfulProvider) - assert.Equal(t, tc.providersAttempted, len(metadata.ProvidersAttempted)) + assert.Len(t, metadata.ProvidersAttempted, tc.providersAttempted) }) } } diff --git a/bmc/nmi_test.go b/bmc/nmi_test.go index c9a8e418..a9de6527 100644 --- a/bmc/nmi_test.go +++ b/bmc/nmi_test.go @@ -7,6 +7,7 @@ import ( "github.com/pkg/errors" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) type mockNMISender struct { @@ -113,9 +114,9 @@ func TestSendNMIFromInterface(t *testing.T) { metadata, err := SendNMIFromInterface(context.Background(), timeout, tt.mockSenders) if tt.errMsg == "" { - assert.NoError(t, err) + require.NoError(t, err) } else { - assert.ErrorContains(t, err, tt.errMsg) + require.ErrorContains(t, err, tt.errMsg) } assert.Equal(t, tt.expectedMetadata, metadata) diff --git a/bmc/postcode_test.go b/bmc/postcode_test.go index 75a67060..7582b4bf 100644 --- a/bmc/postcode_test.go +++ b/bmc/postcode_test.go @@ -59,7 +59,7 @@ func TestPostCode(t *testing.T) { assert.Equal(t, tc.returnStatus, status) assert.Equal(t, tc.returnCode, code) assert.Equal(t, tc.providerName, metadata.SuccessfulProvider) - assert.Equal(t, tc.providersAttempted, len(metadata.ProvidersAttempted)) + assert.Len(t, metadata.ProvidersAttempted, tc.providersAttempted) }) } } diff --git a/bmc/sel_test.go b/bmc/sel_test.go index 1273e59a..4224945b 100644 --- a/bmc/sel_test.go +++ b/bmc/sel_test.go @@ -7,6 +7,7 @@ import ( "github.com/pkg/errors" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) type mockSystemEventLogService struct { @@ -37,13 +38,13 @@ func TestClearSystemEventLog(t *testing.T) { // Test with a mock SystemEventLogService that returns nil mockService := &mockSystemEventLogService{name: "mock1", err: nil} metadata, err := clearSystemEventLog(ctx, timeout, []systemEventLogProviders{{name: mockService.name, systemEventLogProvider: mockService}}) - assert.Nil(t, err) - assert.Equal(t, mockService.name, metadata.SuccessfulProvider) + require.NoError(t, err) + require.Equal(t, mockService.name, metadata.SuccessfulProvider) // Test with a mock SystemEventLogService that returns an error mockService = &mockSystemEventLogService{name: "mock2", err: errors.New("mock error")} metadata, err = clearSystemEventLog(ctx, timeout, []systemEventLogProviders{{name: mockService.name, systemEventLogProvider: mockService}}) - assert.NotNil(t, err) + require.Error(t, err) assert.NotEqual(t, mockService.name, metadata.SuccessfulProvider) } @@ -53,18 +54,18 @@ func TestClearSystemEventLogFromInterfaces(t *testing.T) { // Test with an empty slice metadata, err := ClearSystemEventLogFromInterfaces(ctx, timeout, []interface{}{}) - assert.NotNil(t, err) - assert.Empty(t, metadata.SuccessfulProvider) + require.Error(t, err) + require.Empty(t, metadata.SuccessfulProvider) // Test with a slice containing a non-SystemEventLog object metadata, err = ClearSystemEventLogFromInterfaces(ctx, timeout, []interface{}{"not a SystemEventLog Service"}) - assert.NotNil(t, err) - assert.Empty(t, metadata.SuccessfulProvider) + require.Error(t, err) + require.Empty(t, metadata.SuccessfulProvider) // Test with a slice containing a mock SystemEventLogService that returns nil mockService := &mockSystemEventLogService{name: "mock1"} metadata, err = ClearSystemEventLogFromInterfaces(ctx, timeout, []interface{}{mockService}) - assert.Nil(t, err) + require.NoError(t, err) assert.Equal(t, mockService.name, metadata.SuccessfulProvider) } @@ -75,12 +76,12 @@ func TestGetSystemEventLog(t *testing.T) { // Test with a mock SystemEventLogService that returns nil mockService := &mockSystemEventLogService{name: "mock1", err: nil} _, _, err := getSystemEventLog(ctx, timeout, []systemEventLogProviders{{name: mockService.name, systemEventLogProvider: mockService}}) - assert.Nil(t, err) + require.NoError(t, err) // Test with a mock SystemEventLogService that returns an error mockService = &mockSystemEventLogService{name: "mock2", err: errors.New("mock error")} _, _, err = getSystemEventLog(ctx, timeout, []systemEventLogProviders{{name: mockService.name, systemEventLogProvider: mockService}}) - assert.NotNil(t, err) + assert.Error(t, err) } func TestGetSystemEventLogFromInterfaces(t *testing.T) { @@ -89,16 +90,16 @@ func TestGetSystemEventLogFromInterfaces(t *testing.T) { // Test with an empty slice _, _, err := GetSystemEventLogFromInterfaces(ctx, timeout, []interface{}{}) - assert.NotNil(t, err) + require.Error(t, err) // Test with a slice containing a non-SystemEventLog object _, _, err = GetSystemEventLogFromInterfaces(ctx, timeout, []interface{}{"not a SystemEventLog Service"}) - assert.NotNil(t, err) + require.Error(t, err) // Test with a slice containing a mock SystemEventLogService that returns nil mockService := &mockSystemEventLogService{name: "mock1"} _, _, err = GetSystemEventLogFromInterfaces(ctx, timeout, []interface{}{mockService}) - assert.Nil(t, err) + assert.NoError(t, err) } func TestGetSystemEventLogRaw(t *testing.T) { @@ -108,12 +109,12 @@ func TestGetSystemEventLogRaw(t *testing.T) { // Test with a mock SystemEventLogService that returns nil mockService := &mockSystemEventLogService{name: "mock1", err: nil} _, _, err := getSystemEventLogRaw(ctx, timeout, []systemEventLogProviders{{name: mockService.name, systemEventLogProvider: mockService}}) - assert.Nil(t, err) + require.NoError(t, err) // Test with a mock SystemEventLogService that returns an error mockService = &mockSystemEventLogService{name: "mock2", err: errors.New("mock error")} _, _, err = getSystemEventLogRaw(ctx, timeout, []systemEventLogProviders{{name: mockService.name, systemEventLogProvider: mockService}}) - assert.NotNil(t, err) + assert.Error(t, err) } func TestGetSystemEventLogRawFromInterfaces(t *testing.T) { @@ -122,14 +123,14 @@ func TestGetSystemEventLogRawFromInterfaces(t *testing.T) { // Test with an empty slice _, _, err := GetSystemEventLogRawFromInterfaces(ctx, timeout, []interface{}{}) - assert.NotNil(t, err) + require.Error(t, err) // Test with a slice containing a non-SystemEventLog object _, _, err = GetSystemEventLogRawFromInterfaces(ctx, timeout, []interface{}{"not a SystemEventLog Service"}) - assert.NotNil(t, err) + require.Error(t, err) // Test with a slice containing a mock SystemEventLogService that returns nil mockService := &mockSystemEventLogService{name: "mock1"} _, _, err = GetSystemEventLogRawFromInterfaces(ctx, timeout, []interface{}{mockService}) - assert.Nil(t, err) + assert.NoError(t, err) } diff --git a/errors/errors.go b/errors/errors.go index 34dcaa5b..c3e2270e 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -128,14 +128,14 @@ var ( ErrBMCUpdating = errors.New("a BMC firmware update is in progress") ) -type ErrUnsupportedHardware struct { +type UnsupportedHardwareError struct { msg string } -func (e *ErrUnsupportedHardware) Error() string { +func (e *UnsupportedHardwareError) Error() string { return fmt.Sprintf("Hardware not supported: %s", e.msg) } func NewErrUnsupportedHardware(s string) error { - return &ErrUnsupportedHardware{s} + return &UnsupportedHardwareError{s} } diff --git a/internal/redfishwrapper/bios_test.go b/internal/redfishwrapper/bios_test.go index ece0ce30..cf3330d0 100644 --- a/internal/redfishwrapper/bios_test.go +++ b/internal/redfishwrapper/bios_test.go @@ -12,6 +12,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func biosConfigFromFixture(t *testing.T) map[string]string { @@ -87,7 +88,7 @@ func TestGetBiosConfiguration(t *testing.T) { } biosConfig, err := client.GetBiosConfiguration(ctx) - assert.Nil(t, err) + require.NoError(t, err) assert.Equal(t, tc.expectedBiosConfig, biosConfig) }) } diff --git a/internal/redfishwrapper/firmware_test.go b/internal/redfishwrapper/firmware_test.go index b563b50c..a175efd5 100644 --- a/internal/redfishwrapper/firmware_test.go +++ b/internal/redfishwrapper/firmware_test.go @@ -16,6 +16,7 @@ import ( bmclibErrs "github.com/bmc-toolbox/bmclib/v2/errors" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.uber.org/goleak" ) @@ -121,7 +122,7 @@ func TestRunRequestWithMultipartPayload(t *testing.T) { return } - assert.Nil(t, err) + require.NoError(t, err) resp.Body.Close() client.Close(context.Background()) }) @@ -204,8 +205,8 @@ func TestFirmwareInstallMethodURI(t *testing.T) { return } - assert.Nil(t, err) - assert.Equal(t, tc.expectInstallMethod, gotMethod) + require.NoError(t, err) + require.Equal(t, tc.expectInstallMethod, gotMethod) assert.Equal(t, tc.expectUpdateURI, gotURI) client.Close(context.Background()) @@ -248,7 +249,7 @@ func TestTaskIDFromResponseBody(t *testing.T) { return } - assert.Nil(t, err) + require.NoError(t, err) assert.Equal(t, tc.expectedID, taskID) }) } @@ -301,7 +302,7 @@ func TestTaskIDFromLocationHeader(t *testing.T) { return } - assert.Nil(t, err) + require.NoError(t, err) assert.Equal(t, tc.expectedID, taskID) }) } @@ -336,10 +337,10 @@ func TestUpdateParametersFormField(t *testing.T) { return } - assert.NoError(t, err) - assert.Contains(t, buf.String(), `Content-Disposition: form-data; name="UpdateParameters`) - assert.Contains(t, buf.String(), `Content-Type: application/json`) - assert.NotNil(t, output) + require.NoError(t, err) + require.Contains(t, buf.String(), `Content-Disposition: form-data; name="UpdateParameters`) + require.Contains(t, buf.String(), `Content-Type: application/json`) + require.NotNil(t, output) // Validate the created multipart form content err = writer.Close() @@ -398,7 +399,7 @@ func TestMultipartPayloadSize(t *testing.T) { assert.Contains(t, err.Error(), tc.errorMsg) } - assert.Nil(t, err) + require.NoError(t, err) assert.Equal(t, tc.expectedSize, gotSize) }) } diff --git a/internal/redfishwrapper/main_test.go b/internal/redfishwrapper/main_test.go index b1be7d67..6b62c44b 100644 --- a/internal/redfishwrapper/main_test.go +++ b/internal/redfishwrapper/main_test.go @@ -28,7 +28,9 @@ func mustReadFile(t *testing.T, filename string) []byte { } var endpointFunc = func(t *testing.T, file string) http.HandlerFunc { + t.Helper() return func(w http.ResponseWriter, r *http.Request) { + t.Helper() if file == "404" { w.WriteHeader(http.StatusNotFound) } diff --git a/internal/redfishwrapper/task_test.go b/internal/redfishwrapper/task_test.go index 0ae1159f..c431cf04 100644 --- a/internal/redfishwrapper/task_test.go +++ b/internal/redfishwrapper/task_test.go @@ -10,6 +10,7 @@ import ( "github.com/bmc-toolbox/bmclib/v2/constants" bmclibErrs "github.com/bmc-toolbox/bmclib/v2/errors" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestConvertTaskState(t *testing.T) { @@ -209,9 +210,9 @@ func TestTaskStatus(t *testing.T) { return } - assert.Nil(t, err) - assert.Equal(t, tc.expectedState, state) - assert.Equal(t, tc.expectedStatus, status) + require.NoError(t, err) + require.Equal(t, tc.expectedState, state) + require.Equal(t, tc.expectedStatus, status) client.Close(context.Background()) }) @@ -291,9 +292,9 @@ func TestTask(t *testing.T) { return } - assert.Nil(t, err) - assert.NotNil(t, got) - assert.Equal(t, tc.expectTaskStatus, string(got.TaskStatus)) + require.NoError(t, err) + require.NotNil(t, got) + require.Equal(t, tc.expectTaskStatus, string(got.TaskStatus)) assert.Equal(t, tc.expectTaskState, string(got.TaskState)) client.Close(context.Background()) diff --git a/internal/sum/sum_test.go b/internal/sum/sum_test.go index 465f31c0..5a19171b 100644 --- a/internal/sum/sum_test.go +++ b/internal/sum/sum_test.go @@ -9,6 +9,7 @@ import ( ) func newFakeSum(t *testing.T, fixtureName string) *Sum { + t.Helper() e := &Sum{ Executor: ex.NewFakeExecutor("sum"), } diff --git a/providers/asrockrack/inventory_test.go b/providers/asrockrack/inventory_test.go index 8d7e1c3a..ddcaa5da 100644 --- a/providers/asrockrack/inventory_test.go +++ b/providers/asrockrack/inventory_test.go @@ -22,7 +22,7 @@ func TestGetInventory(t *testing.T) { assert.Equal(t, "0.01.00", device.BMC.Firmware.Installed) assert.Equal(t, "000000ca", device.CPUs[0].Firmware.Installed) assert.Equal(t, "Intel(R) Xeon(R) E-2278G CPU @ 3.40GHz", device.CPUs[0].Model) - assert.Equal(t, 2, len(device.Memory)) - assert.Equal(t, 2, len(device.Drives)) + assert.Len(t, device.Memory, 2) + assert.Len(t, device.Drives, 2) assert.Equal(t, "OK", device.Status.Health) } diff --git a/providers/asrockrack/user_test.go b/providers/asrockrack/user_test.go index f0db7cb0..61cb8d3a 100644 --- a/providers/asrockrack/user_test.go +++ b/providers/asrockrack/user_test.go @@ -2,12 +2,12 @@ package asrockrack import ( "context" - "errors" "net/http" "testing" bmclibErrs "github.com/bmc-toolbox/bmclib/v2/errors" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // NOTE: user accounts are defined in mock_test.go as JSON payload in the userPayload var @@ -69,7 +69,7 @@ func Test_UserRead(t *testing.T) { for _, tt := range testCases { ok, err := aClient.UserCreate(context.TODO(), tt.user, tt.pass, tt.role) - assert.Equal(t, errors.Is(err, tt.err), true, tt.tName) + require.ErrorIs(t, err, tt.err, tt.tName) assert.Equal(t, tt.ok, ok, tt.tName) } @@ -77,7 +77,7 @@ func Test_UserRead(t *testing.T) { t.Setenv("TEST_FAIL_QUERY", "womp womp") // nolint:dupword _, err = aClient.UserRead(context.TODO()) - assert.Equal(t, errors.Is(err, bmclibErrs.ErrRetrievingUserAccounts), true) + assert.ErrorIs(t, err, bmclibErrs.ErrRetrievingUserAccounts) } func Test_UserCreate(t *testing.T) { @@ -110,7 +110,7 @@ func Test_UserCreate(t *testing.T) { for _, tt := range tests { ok, err := aClient.UserCreate(context.TODO(), tt.user, tt.pass, tt.role) - assert.Equal(t, errors.Is(err, tt.err), true, tt.tName) + require.ErrorIs(t, err, tt.err, tt.tName) assert.Equal(t, tt.ok, ok, tt.tName) } } @@ -145,7 +145,7 @@ func Test_UserUpdate(t *testing.T) { for _, tt := range tests { ok, err := aClient.UserUpdate(context.TODO(), tt.user, tt.pass, tt.role) - assert.Equal(t, errors.Is(err, tt.err), true, tt.tName) + require.ErrorIs(t, err, tt.err, tt.tName) assert.Equal(t, tt.ok, ok, tt.tName) } } @@ -223,6 +223,6 @@ func Test_userAccounts(t *testing.T) { t.Error(err) } - assert.Equal(t, 10, len(accounts)) + assert.Len(t, accounts, 10) assert.Equal(t, account0, accounts[0]) } diff --git a/providers/dell/firmware_test.go b/providers/dell/firmware_test.go index 77f099e2..1850d26e 100644 --- a/providers/dell/firmware_test.go +++ b/providers/dell/firmware_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestConvFirmwareTaskOem(t *testing.T) { @@ -83,10 +84,10 @@ func TestConvFirmwareTaskOem(t *testing.T) { t.Run(tc.name, func(t *testing.T) { job, err := convFirmwareTaskOem(tc.oemdata) if tc.expectedErr == "" { - assert.NoError(t, err) + require.NoError(t, err) assert.Equal(t, tc.expectedJob, job) } else { - assert.Error(t, err) + require.Error(t, err) assert.Contains(t, err.Error(), tc.expectedErr) } }) diff --git a/providers/dell/idrac_test.go b/providers/dell/idrac_test.go index bf05e238..40ed1d58 100644 --- a/providers/dell/idrac_test.go +++ b/providers/dell/idrac_test.go @@ -68,15 +68,15 @@ func Test_Screenshot(t *testing.T) { "/redfish/v1/Systems/System.Embedded.1": endpointFunc("/systems_embedded.1.json"), // screenshot endpoint redfishV1Prefix + screenshotEndpoint: func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, r.Method, http.MethodPost) + assert.Equal(t, http.MethodPost, r.Method) - assert.Equal(t, r.Header.Get("Content-Type"), "application/json") + assert.Equal(t, "application/json", r.Header.Get("Content-Type")) b, err := io.ReadAll(r.Body) if err != nil { t.Fatal(err) } - assert.Equal(t, []byte(`{"FileType":"ServerScreenShot"}`), b) + assert.JSONEq(t, `{"FileType":"ServerScreenShot"}`, string(b)) encoded := base64.RawStdEncoding.EncodeToString(img) respFmtStr := `{"@Message.ExtendedInfo":[{"Message":"Successfully Completed Request","MessageArgs":[],"MessageArgs@odata.count":0,"MessageId":"Base.1.8.Success","RelatedProperties":[],"RelatedProperties@odata.count":0,"Resolution":"None","Severity":"OK"},{"Message":"The Export Server Screen Shot operation successfully exported the server screen shot file.","MessageArgs":[],"MessageArgs@odata.count":0,"MessageId":"IDRAC.2.5.LC080","RelatedProperties":[],"RelatedProperties@odata.count":0,"Resolution":"Download the encoded Base64 format server screen shot file, decode the Base64 file and then save it as a *.png file.","Severity":"Informational"}],"ServerScreenshotFile":"%s"}` diff --git a/providers/redfish/sel_test.go b/providers/redfish/sel_test.go index 1f41ceb7..05da4a9a 100644 --- a/providers/redfish/sel_test.go +++ b/providers/redfish/sel_test.go @@ -15,7 +15,7 @@ func Test_GetSystemEventLog(t *testing.T) { } assert.NotNil(t, entries) - assert.Equal(t, 2, len(entries)) + assert.Len(t, entries, 2) } // Write tests for GetSystemEventLogRaw diff --git a/providers/supermicro/firmware_bios_test.go b/providers/supermicro/firmware_bios_test.go index 1c780021..5fff8047 100644 --- a/providers/supermicro/firmware_bios_test.go +++ b/providers/supermicro/firmware_bios_test.go @@ -11,6 +11,7 @@ import ( "github.com/bmc-toolbox/bmclib/v2/internal/httpclient" "github.com/go-logr/logr" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func Test_setComponentUpdateMisc(t *testing.T) { @@ -81,7 +82,7 @@ func Test_setComponentUpdateMisc(t *testing.T) { } serviceClient := newBmcServiceClient(parsedURL.Hostname(), parsedURL.Port(), "foo", "bar", httpclient.Build()) - assert.Nil(t, err) + require.NoError(t, err) serviceClient.csrfToken = "foobar" client := &x11{serviceClient: serviceClient, log: logr.Discard()} @@ -93,7 +94,7 @@ func Test_setComponentUpdateMisc(t *testing.T) { return } - assert.Nil(t, err) + assert.NoError(t, err) } }) } @@ -172,7 +173,7 @@ func Test_setBIOSFirmwareInstallMode(t *testing.T) { } serviceClient := newBmcServiceClient(parsedURL.Hostname(), parsedURL.Port(), "foo", "bar", httpclient.Build()) - assert.Nil(t, err) + require.NoError(t, err) serviceClient.csrfToken = "foobar" client := &x11{serviceClient: serviceClient, log: logr.Discard()} @@ -184,7 +185,7 @@ func Test_setBIOSFirmwareInstallMode(t *testing.T) { return } - assert.Nil(t, err) + assert.NoError(t, err) } }) } diff --git a/providers/supermicro/supermicro_test.go b/providers/supermicro/supermicro_test.go index 4487ca47..bd704180 100644 --- a/providers/supermicro/supermicro_test.go +++ b/providers/supermicro/supermicro_test.go @@ -13,6 +13,7 @@ import ( "github.com/bmc-toolbox/bmclib/v2/internal/redfishwrapper" "github.com/go-logr/logr" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) const ( @@ -91,6 +92,7 @@ func mustReadFile(t *testing.T, filename string) []byte { } var endpointFunc = func(t *testing.T, file string) http.HandlerFunc { + t.Helper() return func(w http.ResponseWriter, r *http.Request) { // expect either GET or Delete methods if r.Method != http.MethodGet && r.Method != http.MethodPost && r.Method != http.MethodDelete { @@ -123,7 +125,7 @@ func TestOpen(t *testing.T) { "/redfish/v1/": endpointFunc(t, "serviceroot.json"), // first request to login "/cgi/login.cgi": func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, r.Method, http.MethodPost) + assert.Equal(t, http.MethodPost, r.Method) assert.Equal(t, "application/x-www-form-urlencoded", r.Header.Get("Content-Type")) b, err := io.ReadAll(r.Body) @@ -154,7 +156,7 @@ func TestOpen(t *testing.T) { // second request for the csrf token "/cgi/url_redirect.cgi": func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, r.Method, http.MethodGet) + assert.Equal(t, http.MethodGet, r.Method) assert.Equal(t, "url_name=topmenu", r.URL.RawQuery) _, err := io.ReadAll(r.Body) if err != nil { @@ -166,7 +168,7 @@ func TestOpen(t *testing.T) { }, // request for model "/cgi/ipmi.cgi": func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, r.Method, http.MethodPost) + assert.Equal(t, http.MethodPost, r.Method) assert.Equal(t, "application/x-www-form-urlencoded; charset=UTF-8", r.Header.Get("Content-Type")) b, err := io.ReadAll(r.Body) @@ -191,8 +193,8 @@ func TestOpen(t *testing.T) { "bar", handlerFuncMap{ "/cgi/login.cgi": func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, r.Method, http.MethodPost) - assert.Equal(t, r.Header.Get("Content-Type"), "application/x-www-form-urlencoded") + assert.Equal(t, http.MethodPost, r.Method) + assert.Equal(t, "application/x-www-form-urlencoded", r.Header.Get("Content-Type")) response := []byte(`barf`) w.WriteHeader(http.StatusUnauthorized) _, _ = w.Write(response) @@ -233,7 +235,7 @@ func TestOpen(t *testing.T) { return } - assert.Nil(t, err) + assert.NoError(t, err) }) } } @@ -254,7 +256,7 @@ func TestClose(t *testing.T) { "bar", "/cgi/logout.cgi", func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, r.Method, http.MethodGet) + assert.Equal(t, http.MethodGet, r.Method) _, err := io.ReadAll(r.Body) if err != nil { @@ -287,7 +289,7 @@ func TestClose(t *testing.T) { return } - assert.Nil(t, err) + require.NoError(t, err) assert.Nil(t, client.serviceClient.redfish) }) } @@ -305,8 +307,8 @@ func TestInitScreenPreview(t *testing.T) { "", "/cgi/op.cgi", func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, r.Method, http.MethodPost) - assert.Equal(t, r.Header.Get("Content-Type"), "application/x-www-form-urlencoded") + assert.Equal(t, http.MethodPost, r.Method) + assert.Equal(t, "application/x-www-form-urlencoded", r.Header.Get("Content-Type")) b, err := io.ReadAll(r.Body) if err != nil { @@ -350,7 +352,7 @@ func TestInitScreenPreview(t *testing.T) { return } - assert.Nil(t, err) + assert.NoError(t, err) }) } } @@ -369,8 +371,8 @@ func TestFetchScreenPreview(t *testing.T) { "", "/cgi/url_redirect.cgi", func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, r.Method, http.MethodGet) - assert.Equal(t, r.Header.Get("Content-Type"), "application/x-www-form-urlencoded") + assert.Equal(t, http.MethodGet, r.Method) + assert.Equal(t, "application/x-www-form-urlencoded", r.Header.Get("Content-Type")) assert.Equal(t, "url_name=Snapshot&url_type=img", r.URL.RawQuery) _, _ = w.Write([]byte(`fake image is fake`)) @@ -408,7 +410,7 @@ func TestFetchScreenPreview(t *testing.T) { return } - assert.Nil(t, err) + require.NoError(t, err) assert.Equal(t, tc.expectImage, image) }) } diff --git a/providers/supermicro/x11_firmware_bmc_test.go b/providers/supermicro/x11_firmware_bmc_test.go index 44eb7ce0..7cf858fb 100644 --- a/providers/supermicro/x11_firmware_bmc_test.go +++ b/providers/supermicro/x11_firmware_bmc_test.go @@ -17,6 +17,7 @@ import ( "github.com/bmc-toolbox/bmclib/v2/internal/httpclient" "github.com/go-logr/logr" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestX11SetBMCFirmwareInstallMode(t *testing.T) { @@ -92,7 +93,7 @@ func TestX11SetBMCFirmwareInstallMode(t *testing.T) { } serviceClient := newBmcServiceClient(parsedURL.Hostname(), parsedURL.Port(), "foo", "bar", httpclient.Build()) - assert.Nil(t, err) + require.NoError(t, err) client := &x11{serviceClient: serviceClient, log: logr.Discard()} @@ -103,7 +104,7 @@ func TestX11SetBMCFirmwareInstallMode(t *testing.T) { return } - assert.Nil(t, err) + assert.NoError(t, err) } }) } @@ -133,7 +134,7 @@ func TestX11UploadBMCFirmware(t *testing.T) { // validate content type mediaType, params, err := mime.ParseMediaType(r.Header.Get("Content-Type")) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, "multipart/form-data", mediaType) @@ -142,13 +143,13 @@ func TestX11UploadBMCFirmware(t *testing.T) { // validate firmware image part part, err := reader.NextPart() - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, `form-data; name="fw_image"; filename="blob.bin"`, part.Header.Get("Content-Disposition")) // validate csrf-token part part, err = reader.NextPart() - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, `form-data; name="CSRF_TOKEN"`, part.Header.Get("Content-Disposition")) }, @@ -187,7 +188,7 @@ func TestX11UploadBMCFirmware(t *testing.T) { } serviceClient := newBmcServiceClient(parsedURL.Hostname(), parsedURL.Port(), "foo", "bar", httpclient.Build()) - assert.Nil(t, err) + require.NoError(t, err) serviceClient.csrfToken = "foobar" client := &x11{serviceClient: serviceClient, log: logr.Discard()} @@ -198,7 +199,7 @@ func TestX11UploadBMCFirmware(t *testing.T) { return } - assert.Nil(t, err) + assert.NoError(t, err) } }) } @@ -269,7 +270,7 @@ func TestX11VerifyBMCFirmwareVersion(t *testing.T) { } serviceClient := newBmcServiceClient(parsedURL.Hostname(), parsedURL.Port(), "foo", "bar", httpclient.Build()) - assert.Nil(t, err) + require.NoError(t, err) serviceClient.csrfToken = "foobar" client := &x11{serviceClient: serviceClient, log: logr.Discard()} @@ -280,7 +281,7 @@ func TestX11VerifyBMCFirmwareVersion(t *testing.T) { return } - assert.Nil(t, err) + assert.NoError(t, err) } }) } @@ -351,7 +352,7 @@ func TestX11InitiateBMCFirmwareInstall(t *testing.T) { } serviceClient := newBmcServiceClient(parsedURL.Hostname(), parsedURL.Port(), "foo", "bar", httpclient.Build()) - assert.Nil(t, err) + require.NoError(t, err) serviceClient.csrfToken = "foobar" client := &x11{serviceClient: serviceClient, log: logr.Discard()} @@ -362,7 +363,7 @@ func TestX11InitiateBMCFirmwareInstall(t *testing.T) { return } - assert.Nil(t, err) + assert.NoError(t, err) } }) } @@ -515,7 +516,7 @@ func TestX11StatusBMCFirmwareInstall(t *testing.T) { } serviceClient := newBmcServiceClient(parsedURL.Hostname(), parsedURL.Port(), "foo", "bar", httpclient.Build()) - assert.Nil(t, err) + require.NoError(t, err) serviceClient.csrfToken = "foobar" client := &x11{serviceClient: serviceClient, log: logr.Discard()} @@ -529,8 +530,8 @@ func TestX11StatusBMCFirmwareInstall(t *testing.T) { } } - assert.Nil(t, err) - assert.Equal(t, tc.expectState, gotState) + require.NoError(t, err) + require.Equal(t, tc.expectState, gotState) assert.Equal(t, tc.expectStatus, gotStatus) }) }