[BUG] /redfish/v1/UpdateService/Actions/SimpleUpdate gives errors for OpenBMC

[desnudaenlaplaya]

description
A couple of things:

1. as commented here example, for instance, the correct redfish command seems /redfish/v1/UpdateService/Actions/UpdateService.SimpleUpdate not /redfish/v1/UpdateService/Actions/SimpleUpdate. Maybe the current version was valid before but not anymore, at least in OpenBMC ( https://github.com/openbmc/bmcweb/blob/8b0783859c1d0f50b1e1a6fe7ca9b12b5bfeaa18/redfish-core/schema/dmtf/json-schema/UpdateService.v1_15_0.json#L32 )
2. the OpenBMC does not needs the parameter component, in fact it gives an error. So better do it optional.

To Reproduce

1. Launch a server running the latest OpenBMC version.
2. bmcweb option insecure-tftp-update needed to be enabled
3. launch this

 ./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 --component BMC

Fix
This works for the use case above:

$ git diff README.md internal/update.go
diff --git a/README.md b/README.md
index 6cb34be..424fe7f 100644
--- a/README.md
+++ b/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 \
diff --git a/internal/update.go b/internal/update.go
index 9191818..0e82806 100644
--- a/internal/update.go
+++ b/internal/update.go
@@ -20,6 +20,15 @@ type UpdateParams struct {
 // 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)
@@ -29,16 +38,29 @@ func UpdateFirmwareRemote(q *UpdateParams) error {
        uri.User = url.UserPassword(q.Username, q.Password)
 
        // set up other vars
-       updateUrl := fmt.Sprintf("%s/redfish/v1/UpdateService/Actions/SimpleUpdate", uri.String())
+       updateUrl := fmt.Sprintf("%s/redfish/v1/UpdateService/Actions/UpdateService.SimpleUpdate", uri.String())
        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,
+
+       var b = map[string]any{}
+       // check if Component isn't empty
+       if q.Component == "" {
+               fmt.Printf("WARNING: parameter --component seems empty [%v] (BMC or BIOS)\n", string(q.Component))
+               b = map[string]any{
+                       // no need to specify the Component
+                       "TransferProtocol": q.TransferProtocol,
+                       "ImageURI":         q.FirmwarePath,
+               }
+       } else {
+               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)
(END)

Additional context

bmcweb option insecure-tftp-update is not used officially by no one, I guess because is not safe

openbmc/openbmc@c1e8468#diff-9e1405074ee4c45cde46c892809bfd079a972f86a42d5e3639c63cb19fabc646

[alexlovelltroy]

Thanks for such a clear bug report. We'll try to get a branch with a fix ready for you to test in the next 24 hours.

[alexlovelltroy]

The linked branch has updated code that shifts from creating the update request directly to using gofish instead. I don't have anything set up to test against OpenBMC. If you have suggestions for a test harness, I'd be happy to work on adding it as an integration test.

Please give the branch a shot and let me know if it works for you. I'll work toward a PR once we've had a few successful tests.

https://github.com/OpenCHAMI/magellan/tree/71-bug-redfishv1updateserviceactionssimpleupdate-gives-errors-for-openbmc

[davidallendj]

I know this issue is about the update service URL, but would it be possible to replace the direct request with gofish in GetUpdateStatus as well?
https://github.com/OpenCHAMI/magellan/blob/71-bug-redfishv1updateserviceactionssimpleupdate-gives-errors-for-openbmc/internal/update.go#L75

Might also be worth adding an insecure flag that defaults to false instead.
https://github.com/OpenCHAMI/magellan/blob/71-bug-redfishv1updateserviceactionssimpleupdate-gives-errors-for-openbmc/internal/update.go#L41