Merge pull request #81 from OpenCHAMI/71-bug-redfishv1updateserviceactionssimpleupdate-gives-errors-for-openbmc

Change manual update request to use `gofish` instead

README.md

@@ -175,7 +175,7 @@ Note: If the `cache` flag is not set, `magellan` will use `/tmp/$USER/magellan.d
 
 ### Updating Firmware
 
-The `magellan` tool is capable of updating firmware with using the `update` subcommand via the Redfish API. This may sometimes necessary if some of the `collect` output is missing or is not including what is expected. The subcommand expects there to be a running HTTP/HTTPS server running that has an accessible URL path to the firmware download. Specify the URL with the `--firmware-path` flag and the firmware type with the `--component` flag with all the other usual arguments like in the example below:
+The `magellan` tool is capable of updating firmware with using the `update` subcommand via the Redfish API. This may sometimes necessary if some of the `collect` output is missing or is not including what is expected. The subcommand expects there to be a running HTTP/HTTPS server running that has an accessible URL path to the firmware download. Specify the URL with the `--firmware-path` flag and the firmware type with the `--component` flag (optional) with all the other usual arguments like in the example below:
 
 ```bash
 ./magellan update 172.16.0.108:443 \

cmd/update.go

@@ -17,6 +17,7 @@ var (
 	component        string
 	transferProtocol string
 	showStatus       bool
+	Insecure         bool
 )
 
 // The `update` command provides an interface to easily update firmware
@@ -27,8 +28,8 @@ var updateCmd = &cobra.Command{
 	Short: "Update BMC node firmware",
 	Long: "Perform an firmware update using Redfish by providing a remote firmware URL and component.\n\n" +
 		"Examples:\n" +
-		"  magellan update 172.16.0.108:443 --username bmc_username --password bmc_password --firmware-url http://172.16.0.200:8005/firmware/bios/image.RBU --component BIOS\n" +
+		"  magellan update 172.16.0.108:443 --insecure --username bmc_username --password bmc_password --firmware-url http://172.16.0.200:8005/firmware/bios/image.RBU --component BIOS\n" +
-		"  magellan update 172.16.0.108:443 --status --username bmc_username --password bmc_password",
+		"  magellan update 172.16.0.108:443 --insecure --status --username bmc_username --password bmc_password",
 	Run: func(cmd *cobra.Command, args []string) {
 		// check that we have at least one host
 		if len(args) <= 0 {
@@ -44,6 +45,7 @@ var updateCmd = &cobra.Command{
 					FirmwareVersion:  firmwareVersion,
 					Component:        component,
 					TransferProtocol: transferProtocol,
+					Insecure:         Insecure,
 					CollectParams: magellan.CollectParams{
 						URI:      arg,
 						Username: username,
@@ -63,6 +65,7 @@ var updateCmd = &cobra.Command{
 				FirmwareVersion:  firmwareVersion,
 				Component:        component,
 				TransferProtocol: strings.ToUpper(transferProtocol),
+				Insecure:         Insecure,
 				CollectParams: magellan.CollectParams{
 					URI:      arg,
 					Username: username,
@@ -85,6 +88,7 @@ func init() {
 	updateCmd.Flags().StringVar(&firmwareVersion, "firmware-version", "", "Set the version of firmware to be installed")
 	updateCmd.Flags().StringVar(&component, "component", "", "Set the component to upgrade (BMC|BIOS)")
 	updateCmd.Flags().BoolVar(&showStatus, "status", false, "Get the status of the update")
+	updateCmd.Flags().BoolVar(&Insecure, "insecure", false, "Allow insecure connections to the server")
 
 	checkBindFlagError(viper.BindPFlag("update.username", updateCmd.Flags().Lookup("username")))
 	checkBindFlagError(viper.BindPFlag("update.password", updateCmd.Flags().Lookup("password")))
@@ -93,6 +97,7 @@ func init() {
 	checkBindFlagError(viper.BindPFlag("update.firmware-version", updateCmd.Flags().Lookup("firmware-version")))
 	checkBindFlagError(viper.BindPFlag("update.component", updateCmd.Flags().Lookup("component")))
 	checkBindFlagError(viper.BindPFlag("update.status", updateCmd.Flags().Lookup("status")))
+	checkBindFlagError(viper.BindPFlag("update.insecure", updateCmd.Flags().Lookup("insecure")))
 
 	rootCmd.AddCommand(updateCmd)
 }

go.mod

@@ -52,6 +52,8 @@ require (
 	github.com/subosito/gotenv v1.6.0 // indirect
 	go.uber.org/atomic v1.9.0 // indirect
 	go.uber.org/multierr v1.9.0 // indirect
+	golang.org/x/crypto v0.31.0 // indirect
+	golang.org/x/sys v0.28.0 // indirect
 	golang.org/x/sys v0.29.0 // indirect
 	golang.org/x/text v0.21.0 // indirect
 	gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect

go.sum

@@ -123,6 +123,10 @@ golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACk
 golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
 golang.org/x/crypto v0.19.0/go.mod h1:Iy9bg/ha4yyC70EfRS8jz+B6ybOBKMaSxLj6P6oBDfU=
 golang.org/x/crypto v0.21.0/go.mod h1:0BP7YvVV9gBbVKyeTG0Gyn+gZm94bibOW5BjDEYAOMs=
+golang.org/x/crypto v0.31.0 h1:ihbySMvVjLAeSH1IbfcRTkD/iNscyz8rGzjF/E5hV6U=
+golang.org/x/crypto v0.31.0/go.mod h1:kDsLvtWBEx7MV9tJOj9bnXsPbxwJQ6csT/x4KIN4Ssk=
+golang.org/x/crypto v0.22.0 h1:g1v0xeRhjcugydODzvb3mEM9SQ0HGp9s/nh3COQ/C30=
+golang.org/x/crypto v0.22.0/go.mod h1:vr6Su+7cTlO45qkww3VDJlzDn0ctJvRgYbC2NvXHt+M=
 golang.org/x/crypto v0.32.0 h1:euUpcYgM8WcP71gNpTqQCn6rC2t6ULUPiOzfWaXVVfc=
 golang.org/x/crypto v0.32.0/go.mod h1:ZnnJkOaASj8g0AjIduWNlq2NRxL0PlBrbKVyZ6V/Ugc=
 golang.org/x/exp v0.0.0-20240409090435-93d18d7e34b8 h1:ESSUROHIBHg7USnszlcdmjBEwdMj9VUvU+OPk4yl2mc=
@@ -151,6 +155,12 @@ golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
 golang.org/x/sys v0.18.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
+golang.org/x/sys v0.28.0 h1:Fksou7UEQUWlKvIdsqzJmUmCX3cZuD2+P3XyyzwMhlA=
+golang.org/x/sys v0.28.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
+golang.org/x/sys v0.19.0 h1:q5f1RH2jigJ1MoAWp2KTp3gm5zAGFUTarQZ5U386+4o=
+golang.org/x/sys v0.19.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
+golang.org/x/sys v0.29.0 h1:TPYlXGxvx1MGTn2GiZDhnjPA9wZzZeGKHHmKhHYvgaU=
+golang.org/x/sys v0.29.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
 golang.org/x/sys v0.29.0 h1:TPYlXGxvx1MGTn2GiZDhnjPA9wZzZeGKHHmKhHYvgaU=
 golang.org/x/sys v0.29.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
 golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
@@ -166,6 +176,8 @@ golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8=
 golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8=
 golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU=
 golang.org/x/text v0.21.0 h1:zyQAAkrwaneQ066sspRyJaG9VNi/YJ1NfzcGB3hZ/qo=
+golang.org/x/text v0.21.0 h1:zyQAAkrwaneQ066sspRyJaG9VNi/YJ1NfzcGB3hZ/qo=
+golang.org/x/text v0.21.0 h1:zyQAAkrwaneQ066sspRyJaG9VNi/YJ1NfzcGB3hZ/qo=
 golang.org/x/text v0.21.0/go.mod h1:4IBbMaMmOPCJ8SecivzSH54+73PCFmPWxNTLm+vZkEQ=
 golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
 golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=

pkg/crawler/identify.go

@@ -0,0 +1,69 @@
+package crawler
+
+import (
+	"fmt"
+
+	"github.com/stmcginnis/gofish"
+	"github.com/stmcginnis/gofish/redfish"
+)
+
+// BMCInfo represents relevant information about a BMC
+type BMCInfo struct {
+	Manufacturer    string `json:"manufacturer"`
+	Model           string `json:"model"`
+	SerialNumber    string `json:"serial_number"`
+	FirmwareVersion string `json:"firmware_version"`
+	ManagerType     string `json:"manager_type"`
+	UUID            string `json:"uuid"`
+}
+
+// IsBMC checks if a given Manager is a BMC based on its type and associations
+func IsBMC(manager *redfish.Manager) bool {
+	if manager == nil {
+		return false
+	}
+
+	// Valid BMC types in Redfish
+	bmcTypes := map[string]bool{
+		"BMC":                  true,
+		"ManagementController": true, // Some BMCs use this type
+	}
+
+	// Check if ManagerType matches a BMC type
+	if !bmcTypes[string(manager.ManagerType)] {
+		return false
+	}
+
+	return false // Otherwise, it's likely a chassis manager or other device
+}
+
+// GetBMCInfo retrieves details of all available BMCs
+func GetBMCInfo(client *gofish.APIClient) ([]BMCInfo, error) {
+	var bmcList []BMCInfo
+
+	// Retrieve all managers (BMCs and other managers)
+	managers, err := client.Service.Managers()
+	if err != nil {
+		return nil, fmt.Errorf("failed to retrieve managers: %v", err)
+	}
+
+	// Iterate through each manager and collect BMC details
+	for _, manager := range managers {
+		if !IsBMC(manager) {
+			continue // Skip if it's not a BMC
+		}
+
+		bmc := BMCInfo{
+			Manufacturer:    manager.Manufacturer,
+			Model:           manager.Model,
+			SerialNumber:    manager.SerialNumber,
+			FirmwareVersion: manager.FirmwareVersion,
+			ManagerType:     string(manager.ManagerType), // Convert ManagerType to string
+			UUID:            manager.UUID,
+		}
+
+		bmcList = append(bmcList, bmc)
+	}
+
+	return bmcList, nil
+}

pkg/crawler/main.go

@@ -149,6 +149,24 @@ func CrawlBMCForSystems(config CrawlerConfig) ([]InventoryDetail, error) {
 }
 
 // CrawlBMCForSystems pulls BMC manager information.
+// CrawlBMCForManagers connects to a BMC (Baseboard Management Controller) using the provided configuration,
+// retrieves the ServiceRoot, and then fetches the list of managers from the ServiceRoot.
+//
+// Parameters:
+//   - config: A CrawlerConfig struct containing the URI, username, password, and other connection details.
+//
+// Returns:
+//   - []Manager: A slice of Manager structs representing the managers retrieved from the BMC.
+//   - error: An error object if any error occurs during the connection or retrieval process.
+//
+// The function performs the following steps:
+//  1. Initializes a gofish client with the provided configuration.
+//  2. Attempts to connect to the BMC using the gofish client.
+//  3. Handles specific connection errors such as 404 (ServiceRoot not found) and 401 (authentication failed).
+//  4. Logs out from the client after the operations are completed.
+//  5. Retrieves the ServiceRoot from the connected BMC.
+//  6. Fetches the list of managers from the ServiceRoot.
+//  7. Returns the list of managers and any error encountered during the process.
 func CrawlBMCForManagers(config CrawlerConfig) ([]Manager, error) {
 
 	// get username and password from secret store
@@ -193,6 +211,27 @@ func CrawlBMCForManagers(config CrawlerConfig) ([]Manager, error) {
 	return walkManagers(rf_managers, config.URI)
 }
 
+// walkSystems processes a list of Redfish computer systems and their associated chassis,
+// and returns a list of inventory details for each system.
+//
+// Parameters:
+//   - rf_systems: A slice of pointers to redfish.ComputerSystem objects representing the computer systems to be processed.
+//   - rf_chassis: A pointer to a redfish.Chassis object representing the chassis associated with the computer systems.
+//   - baseURI: A string representing the base URI for constructing resource URIs.
+//
+// Returns:
+//   - A slice of InventoryDetail objects containing detailed information about each computer system.
+//   - An error if any issues occur while processing the computer systems or their associated resources.
+//
+// The function performs the following steps:
+//  1. Iterates over each computer system in rf_systems.
+//  2. Constructs an InventoryDetail object for each computer system, populating fields such as URI, UUID, Name, Manufacturer, SystemType, Model, Serial, BiosVersion, PowerState, ProcessorCount, ProcessorType, and MemoryTotal.
+//  3. If rf_chassis is not nil, populates additional chassis-related fields in the InventoryDetail object.
+//  4. Retrieves and processes Ethernet interfaces for each computer system, adding them to the EthernetInterfaces field of the InventoryDetail object.
+//  5. Retrieves and processes Network interfaces and their associated network adapters for each computer system, adding them to the NetworkInterfaces field of the InventoryDetail object.
+//  6. Processes trusted modules for each computer system, adding them to the TrustedModules field of the InventoryDetail object.
+//  7. Appends the populated InventoryDetail object to the systems slice.
+//  8. Returns the systems slice and any error encountered during processing.
 func walkSystems(rf_systems []*redfish.ComputerSystem, rf_chassis *redfish.Chassis, baseURI string) ([]InventoryDetail, error) {
 	systems := []InventoryDetail{}
 	for _, rf_computersystem := range rf_systems {
@@ -281,6 +320,23 @@ func walkSystems(rf_systems []*redfish.ComputerSystem, rf_chassis *redfish.Chass
 	return systems, nil
 }
 
+// walkManagers processes a list of Redfish managers and extracts relevant information
+// to create a slice of Manager objects.
+//
+// Parameters:
+//
+//	rf_managers - A slice of pointers to redfish.Manager objects representing the Redfish managers to be processed.
+//	baseURI - A string representing the base URI to be used for constructing URIs for the managers and their Ethernet interfaces.
+//
+// Returns:
+//
+//	A slice of Manager objects containing the extracted information from the provided Redfish managers.
+//	An error if any issues occur while retrieving Ethernet interfaces from the managers.
+//
+// The function iterates over each Redfish manager, retrieves its Ethernet interfaces,
+// and constructs a Manager object with the relevant details, including Ethernet interface information.
+// If an error occurs while retrieving Ethernet interfaces, the function logs the error and returns the managers
+// collected so far along with the error.
 func walkManagers(rf_managers []*redfish.Manager, baseURI string) ([]Manager, error) {
 	var managers []Manager
 	for _, rf_manager := range rf_managers {

pkg/update.go

@@ -1,12 +1,14 @@
 package magellan
 
 import (
-	"encoding/json"
 	"fmt"
-	"net/http"
 	"net/url"
 
-	"github.com/OpenCHAMI/magellan/pkg/client"
+	"github.com/stmcginnis/gofish"
+	"github.com/stmcginnis/gofish/redfish"
+
+	"github.com/stmcginnis/gofish"
+	"github.com/stmcginnis/gofish/redfish"
 )
 
 type UpdateParams struct {
@@ -15,43 +17,54 @@ type UpdateParams struct {
 	FirmwareVersion  string
 	Component        string
 	TransferProtocol string
+	Insecure         bool
 }
 
 // UpdateFirmwareRemote() uses 'gofish' to update the firmware of a BMC node.
 // The function expects the firmware URL, firmware version, and component flags to be
 // set from the CLI to perform a firmware update.
+// Example:
+// ./magellan update https://192.168.23.40 --username root --password 0penBmc
+// --firmware-url http://192.168.23.19:1337/obmc-phosphor-image.static.mtd.tar
+// --scheme TFTP
+//
+// being:
+// q.URI https://192.168.23.40
+// q.TransferProtocol TFTP
+// q.FirmwarePath http://192.168.23.19:1337/obmc-phosphor-image.static.mtd.tar
 func UpdateFirmwareRemote(q *UpdateParams) error {
 	// parse URI to set up full address
 	uri, err := url.ParseRequestURI(q.URI)
 	if err != nil {
 		return fmt.Errorf("failed to parse URI: %w", err)
 	}
-	uri.User = url.UserPassword(q.Username, q.Password)
+  
+	// Connect to the Redfish service using gofish
+	client, err := gofish.Connect(gofish.ClientConfig{Endpoint: uri.String(), Username: q.Username, Password: q.Password, Insecure: q.Insecure})
+	if err != nil {
+		return fmt.Errorf("failed to connect to Redfish service: %w", err)
+	}
+	defer client.Logout()
 
-	// set up other vars
+	// Retrieve the UpdateService from the Redfish client
-	updateUrl := fmt.Sprintf("%s/redfish/v1/UpdateService/Actions/SimpleUpdate", uri.String())
+	updateService, err := client.Service.UpdateService()
-	headers := map[string]string{
-		"Content-Type":  "application/json",
-		"cache-control": "no-cache",
-	}
-	b := map[string]any{
-		"UpdateComponent":  q.Component, // BMC, BIOS
-		"TransferProtocol": q.TransferProtocol,
-		"ImageURI":         q.FirmwarePath,
-	}
-	data, err := json.Marshal(b)
 	if err != nil {
-		return fmt.Errorf("failed to marshal data: %v", err)
+		return fmt.Errorf("failed to get update service: %w", err)
 	}
-	res, body, err := client.MakeRequest(nil, updateUrl, "POST", data, headers)
+
+	// Build the update request payload
+	req := redfish.SimpleUpdateParameters{
+		ImageURI:         q.FirmwarePath,
+		TransferProtocol: redfish.TransferProtocolType(q.TransferProtocol),
+	}
+
+	// Execute the SimpleUpdate action
+	err = updateService.SimpleUpdate(&req)
 	if err != nil {
-		return fmt.Errorf("something went wrong: %v", err)
+		return fmt.Errorf("firmware update failed: %w", err)
-	} else if res == nil {
-		return fmt.Errorf("no response returned (url: %s)", updateUrl)
-	}
-	if len(body) > 0 {
-		fmt.Printf("%d: %v\n", res.StatusCode, string(body))
 	}
+	fmt.Println("Firmware update initiated successfully.")
+
 	return nil
 }
 
@@ -61,18 +74,23 @@ func GetUpdateStatus(q *UpdateParams) error {
 	if err != nil {
 		return fmt.Errorf("failed to parse URI: %w", err)
 	}
-	uri.User = url.UserPassword(q.Username, q.Password)
+
-	updateUrl := fmt.Sprintf("%s/redfish/v1/UpdateService", uri.String())
+	// Connect to the Redfish service using gofish
-	res, body, err := client.MakeRequest(nil, updateUrl, "GET", nil, nil)
+	client, err := gofish.Connect(gofish.ClientConfig{Endpoint: uri.String(), Username: q.Username, Password: q.Password, Insecure: q.Insecure})
 	if err != nil {
-		return fmt.Errorf("something went wrong: %v", err)
+		return fmt.Errorf("failed to connect to Redfish service: %w", err)
-	} else if res == nil {
-		return fmt.Errorf("no response returned (url: %s)", updateUrl)
-	} else if res.StatusCode != http.StatusOK {
-		return fmt.Errorf("returned status code %d", res.StatusCode)
 	}
-	if len(body) > 0 {
+	defer client.Logout()
-		fmt.Printf("%v\n", string(body))
+
+	// Retrieve the UpdateService from the Redfish client
+	updateService, err := client.Service.UpdateService()
+	if err != nil {
+		return fmt.Errorf("failed to get update service: %w", err)
 	}
+
+	// Get the update status
+	status := updateService.Status
+	fmt.Printf("Update Status: %v\n", status)
+
 	return nil
 }