Merge pull request #402 from bmc-toolbox/bios-cfg-regression-fix-0

providers/supermicro/supermicro.go: Initialize sum 'client' during serviceclient init
bmc-toolbox · Oct 29, 2024 · f669e7b · f669e7b
2 parents 490323d + 0a6db1f
commit f669e7b
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 11 deletions.
diff --git a/providers/supermicro/firmware_bios_test.go b/providers/supermicro/firmware_bios_test.go
@@ -80,7 +80,9 @@ func Test_setComponentUpdateMisc(t *testing.T) {
 				t.Fatal(err)
 			}
 
-			serviceClient := newBmcServiceClient(parsedURL.Hostname(), parsedURL.Port(), "foo", "bar", httpclient.Build())
+			serviceClient, err := newBmcServiceClient(parsedURL.Hostname(), parsedURL.Port(), "foo", "bar", httpclient.Build())
+			assert.Nil(t, err)
+
 			serviceClient.csrfToken = "foobar"
 			client := &x11{serviceClient: serviceClient, log: logr.Discard()}
 
@@ -169,7 +171,9 @@ func Test_setBIOSFirmwareInstallMode(t *testing.T) {
 				t.Fatal(err)
 			}
 
-			serviceClient := newBmcServiceClient(parsedURL.Hostname(), parsedURL.Port(), "foo", "bar", httpclient.Build())
+			serviceClient, err := newBmcServiceClient(parsedURL.Hostname(), parsedURL.Port(), "foo", "bar", httpclient.Build())
+			assert.Nil(t, err)
+
 			serviceClient.csrfToken = "foobar"
 			client := &x11{serviceClient: serviceClient, log: logr.Discard()}
 

diff --git a/providers/supermicro/supermicro.go b/providers/supermicro/supermicro.go
@@ -136,14 +136,21 @@ func NewClient(host, user, pass string, log logr.Logger, opts ...Option) *Client
 		opt(defaultConfig)
 	}
 
-	serviceClient := newBmcServiceClient(
+	serviceClient, err := newBmcServiceClient(
 		host,
 		defaultConfig.Port,
 		user,
 		pass,
 		httpclient.Build(defaultConfig.httpClientSetupFuncs...),
 	)
 
+	// We probably want to treat this as a fatal error and/or pass the error back to the caller
+	// I did not want to chase that thread atm, so we intentionally return nil here if
+	// newBmcServiceClient returns an error.
+	if err != nil {
+		return nil
+	}
+
 	return &Client{
 		serviceClient: serviceClient,
 		log:           log,
@@ -443,12 +450,20 @@ type serviceClient struct {
 	sum       *sum.Sum
 }
 
-func newBmcServiceClient(host, port, user, pass string, client *http.Client) *serviceClient {
+func newBmcServiceClient(host, port, user, pass string, client *http.Client) (*serviceClient, error) {
+	sc := &serviceClient{host: host, port: port, user: user, pass: pass, client: client}
+
 	if !strings.HasPrefix(host, "https://") && !strings.HasPrefix(host, "http://") {
-		host = "https://" + host
+		sc.host = "https://" + host
+	}
+
+	s, err := sum.New(host, user, pass)
+	if err != nil {
+		return nil, err
 	}
+	sc.sum = s
 
-	return &serviceClient{host: host, port: port, user: user, pass: pass, client: client}
+	return sc, nil
 }
 
 func (c *serviceClient) setCsrfToken(t string) {

diff --git a/providers/supermicro/x11_firmware_bmc_test.go b/providers/supermicro/x11_firmware_bmc_test.go
@@ -91,7 +91,9 @@ func TestX11SetBMCFirmwareInstallMode(t *testing.T) {
 				t.Fatal(err)
 			}
 
-			serviceClient := newBmcServiceClient(parsedURL.Hostname(), parsedURL.Port(), "foo", "bar", httpclient.Build())
+			serviceClient, err := newBmcServiceClient(parsedURL.Hostname(), parsedURL.Port(), "foo", "bar", httpclient.Build())
+			assert.Nil(t, err)
+
 			client := &x11{serviceClient: serviceClient, log: logr.Discard()}
 
 			if err := client.setBMCFirmwareInstallMode(context.Background()); err != nil {
@@ -184,7 +186,8 @@ func TestX11UploadBMCFirmware(t *testing.T) {
 				defer os.Remove(binPath)
 			}
 
-			serviceClient := newBmcServiceClient(parsedURL.Hostname(), parsedURL.Port(), "foo", "bar", httpclient.Build())
+			serviceClient, err := newBmcServiceClient(parsedURL.Hostname(), parsedURL.Port(), "foo", "bar", httpclient.Build())
+			assert.Nil(t, err)
 			serviceClient.csrfToken = "foobar"
 			client := &x11{serviceClient: serviceClient, log: logr.Discard()}
 
@@ -265,7 +268,8 @@ func TestX11VerifyBMCFirmwareVersion(t *testing.T) {
 				t.Fatal(err)
 			}
 
-			serviceClient := newBmcServiceClient(parsedURL.Hostname(), parsedURL.Port(), "foo", "bar", httpclient.Build())
+			serviceClient, err := newBmcServiceClient(parsedURL.Hostname(), parsedURL.Port(), "foo", "bar", httpclient.Build())
+			assert.Nil(t, err)
 			serviceClient.csrfToken = "foobar"
 			client := &x11{serviceClient: serviceClient, log: logr.Discard()}
 
@@ -346,7 +350,8 @@ func TestX11InitiateBMCFirmwareInstall(t *testing.T) {
 				t.Fatal(err)
 			}
 
-			serviceClient := newBmcServiceClient(parsedURL.Hostname(), parsedURL.Port(), "foo", "bar", httpclient.Build())
+			serviceClient, err := newBmcServiceClient(parsedURL.Hostname(), parsedURL.Port(), "foo", "bar", httpclient.Build())
+			assert.Nil(t, err)
 			serviceClient.csrfToken = "foobar"
 			client := &x11{serviceClient: serviceClient, log: logr.Discard()}
 
@@ -509,7 +514,9 @@ func TestX11StatusBMCFirmwareInstall(t *testing.T) {
 				t.Fatal(err)
 			}
 
-			serviceClient := newBmcServiceClient(parsedURL.Hostname(), parsedURL.Port(), "foo", "bar", httpclient.Build())
+			serviceClient, err := newBmcServiceClient(parsedURL.Hostname(), parsedURL.Port(), "foo", "bar", httpclient.Build())
+			assert.Nil(t, err)
+
 			serviceClient.csrfToken = "foobar"
 			client := &x11{serviceClient: serviceClient, log: logr.Discard()}