stmcginnis / gofish

Gofish is a Golang client library for DMTF Redfish and SNIA Swordfish interaction.
BSD 3-Clause "New" or "Revised" License
211 stars 112 forks source link

Setting "write_protected" to unsupported value does not throw an error. #276

Closed prajwalpatil25 closed 4 months ago

prajwalpatil25 commented 11 months ago

Virtual Media attribute "write_protected" does not throw an error when set to unsupported value.

API example: POST redfish/v1/Managers/iDRAC.Embedded.1/VirtualMedia/RemovableDisk/Actions/VirtualMedia.InsertMedia Request body:

{
    "Image":"http://<file-share-url>/redhat/RHEL8/8.8/BaseOS/x86_64/os/images/efiboot.img",
    "WriteProtected": false,
    "TransferMethod": "Stream",
}

Response:

{
    "error": {
        "@Message.ExtendedInfo": [
            {
                "Message": "Unable to Process the request because the value entered for the parameter WriteProtected is not supported by the implementation.",
                "MessageArgs": [
                    "WriteProtected"
                ],
                "MessageArgs@odata.count": 1,
                "MessageId": "IDRAC.2.6.SYS458",
                "RelatedProperties": [
                    "#/WriteProtected"
                ],
                "RelatedProperties@odata.count": 1,
                "Resolution": "Correct the value of the parameter identified in the message and retry the operation.For information about supported values, see the Redfish User's Guide available on the support site.",
                "Severity": "Warning"
            }
        ],
        "code": "Base.1.8.GeneralError",
        "message": "A general error has occurred. See ExtendedInfo for more information"
    }
}

In above request, passing WriteProtected as "false" throws error "Unable to Process the request because the value entered for the parameter WriteProtected is not supported by the implementation." but passing same config to virtualMedia.InsertMediaConfig does not throw any error and after successful execution the value returned by redfish.GetVirtualMedia for WriteProtected is "true".

stmcginnis commented 11 months ago

Hi @prajwalpatil25 - this would be something you would need to raise with your vendor. It is up to the Redfish implementation to return an appropriate errors for things not supported by that implementation.

prajwalpatil25 commented 11 months ago

Hi @stmcginnis , As shown in the issue summary API returns an error when "WriteProtected": false is been provided in the request body. However, when I create a same payload and call virtualMedia.InsertMediaConfig() with that payload, it doesn't throw any error. There are two possibilities, either the payload is been modified before call to Post() or the error is not captured correctly.

Here's the sample code I tried:

package main

import (
    "fmt"

    "github.com/stmcginnis/gofish"
    "github.com/stmcginnis/gofish/redfish"
)

func main() {
    config := gofish.ClientConfig{
        Endpoint:  "https://<ip>",
        Username:  "user",
        Password:  "password",
        Insecure:  true,
        BasicAuth: true,
    }

    c, err := gofish.Connect(config)
    if err != nil {
        fmt.Println("Error in connection")
        panic(err)
    }

    service := c.Service

    // virtualMedia, err = redfish.GetVirtualMedia(service.GetClient(), "/redfish/v1/Systems/System.Embedded.1/VirtualMedia/1")
    virtualMedia, err := redfish.GetVirtualMedia(service.GetClient(), "/redfish/v1/Managers/iDRAC.Embedded.1/VirtualMedia/CD")
    if err != nil {
        fmt.Printf("Error getting virtual media: %s\n", err)
        return
    }

    fmt.Printf("Value of inserted before attach: %t\n", virtualMedia.Inserted)

    virtualMediaConfig := redfish.VirtualMediaConfig{
        Image:                "http://linuxlib.us.dell.com/pub/redhat/RHEL8/8.8/BaseOS/x86_64/iso/RHEL-8.8.0-20230411.3-x86_64-boot.iso",
        Inserted:             true,
        TransferMethod:       "Stream",
        TransferProtocolType: "HTTP",
        WriteProtected:       false,
    }

    fmt.Printf("Value of WriteProtected from config: %t\n", virtualMediaConfig.WriteProtected)
    fmt.Println("Inserting...")

    err = virtualMedia.InsertMediaConfig(virtualMediaConfig)
    if err != nil {
        fmt.Printf("Couldn't mount Virtual Media: %s\n", err)
        return
    }

    // virtualMedia, err = redfish.GetVirtualMedia(service.GetClient(), "/redfish/v1/Systems/System.Embedded.1/VirtualMedia/1")
    virtualMedia, err = redfish.GetVirtualMedia(service.GetClient(), "/redfish/v1/Managers/iDRAC.Embedded.1/VirtualMedia/CD")
    if err != nil {
        fmt.Printf("Error getting virtual media: %s\n", err)
        return
    }

    fmt.Printf("Value of WriteProtected after attach: %t\n", virtualMedia.WriteProtected)
    fmt.Printf("Value of inserted after attach: %t\n", virtualMedia.Inserted)
}

Output:

Value of inserted before attach: false
Value of WriteProtected from config: false
Inserting...
Value of WriteProtected after attach: true
Value of inserted after attach: true
stmcginnis commented 4 months ago

I took a look through the spec again to confirm, and this action does not return any kind of result other than pass/fail. So it would be up the the iDRAC Redfish implementation to fail the request if there is a property being set that it can't tolerate. So there's not much we can do from the Gofish side, at least as far as the spec is strictly concerned.

stmcginnis commented 4 months ago

Filing an issue with the vendor to change this behavior may be the best approach.