Skip to content

Commit

Permalink
Merge pull request #345 from bmc-toolbox/redfish-stream-uploads
Browse files Browse the repository at this point in the history 
Redfish stream multipart uploads
  • Loading branch information
@joelrebel
joelrebel authored Sep 11, 2023
2 parents a1b87e2 + 08172f2 commit f99e587
Show file tree
Hide file tree
Showing 3 changed files with 218 additions and 30 deletions.
2 changes: 1 addition & 1 deletion examples/install-firmware/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func main() {
}

switch state {
case constants.FirmwareInstallRunning:
case constants.FirmwareInstallRunning, constants.FirmwareInstallInitializing:
l.WithFields(logrus.Fields{"state": state, "component": *component}).Info("firmware install running")

case constants.FirmwareInstallFailed:
Expand Down
164 changes: 138 additions & 26 deletions providers/redfish/firmware.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (

var (
errInsufficientCtxTimeout = errors.New("remaining context timeout insufficient to install firmware")
errMultiPartPayload = errors.New("error preparing multipart payload")
)

// SupportedFirmwareApplyAtValues returns the supported redfish firmware applyAt values
Expand All @@ -37,10 +38,16 @@ func SupportedFirmwareApplyAtValues() []string {

// FirmwareInstall uploads and initiates the firmware install process
func (c *Conn) FirmwareInstall(ctx context.Context, component, applyAt string, forceInstall bool, reader io.Reader) (taskID string, err error) {
// limit to *os.File until theres a need for other types of readers
updateFile, ok := reader.(*os.File)
if !ok {
return "", errors.Wrap(bmclibErrs.ErrFirmwareInstall, "method expects an *os.File object")
}

// validate firmware update mechanism is supported
err = c.firmwareUpdateCompatible(ctx)
if err != nil {
return "", err
return "", errors.Wrap(bmclibErrs.ErrFirmwareInstall, err.Error())
}

// validate applyAt parameter
Expand All @@ -59,7 +66,7 @@ func (c *Conn) FirmwareInstall(ctx context.Context, component, applyAt string, f
// list redfish firmware install task if theres one present
task, err := c.GetFirmwareInstallTaskQueued(ctx, component)
if err != nil {
return "", err
return "", errors.Wrap(bmclibErrs.ErrFirmwareInstall, err.Error())
}

if task != nil {
Expand Down Expand Up @@ -93,17 +100,17 @@ func (c *Conn) FirmwareInstall(ctx context.Context, component, applyAt string, f
// override the gofish HTTP client timeout,
// since the context timeout is set at Open() and is at a lower value than required for this operation.
//
// record the http client time to be restored
// record the http client timeout to be restored
httpClientTimeout := c.redfishwrapper.HttpClientTimeout()
defer func() {
c.redfishwrapper.SetHttpClientTimeout(httpClientTimeout)
}()

c.redfishwrapper.SetHttpClientTimeout(time.Until(ctxDeadline))

payload := map[string]io.Reader{
"UpdateParameters": bytes.NewReader(updateParameters),
"UpdateFile": reader,
payload := &multipartPayload{
updateParameters: updateParameters,
updateFile: updateFile,
}

resp, err := c.runRequestWithMultipartPayload(http.MethodPost, "/redfish/v1/UpdateService/MultipartUpload", payload)
Expand All @@ -127,6 +134,11 @@ func (c *Conn) FirmwareInstall(ctx context.Context, component, applyAt string, f
return taskID, nil
}

type multipartPayload struct {
updateParameters []byte
updateFile *os.File
}

// FirmwareInstallStatus returns the status of the firmware install task queued
func (c *Conn) FirmwareInstallStatus(ctx context.Context, installVersion, component, taskID string) (state string, err error) {
vendor, _, err := c.DeviceVendorModel(ctx)
Expand Down Expand Up @@ -197,6 +209,59 @@ func (c *Conn) firmwareUpdateCompatible(ctx context.Context) (err error) {
return nil
}

// pipeReaderFakeSeeker wraps the io.PipeReader and implements the io.Seeker interface
// to meet the API requirements for the Gofish client https://github.com/stmcginnis/gofish/blob/46b1b33645ed1802727dc4df28f5d3c3da722b15/client.go#L434
//
// The Gofish method linked does not currently perform seeks and so a PR will be suggested
// to change the method signature to accept an io.Reader instead.
type pipeReaderFakeSeeker struct {
*io.PipeReader
}

// Seek impelements the io.Seeker interface only to panic if called
func (p pipeReaderFakeSeeker) Seek(offset int64, whence int) (int64, error) {
return 0, errors.New("Seek() not implemented for fake pipe reader seeker.")
}

// multipartPayloadSize prepares a temporary multipart form to determine the form size
//
// 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) {
body := &bytes.Buffer{}
form := multipart.NewWriter(body)

// Add UpdateParameters field part
part, err := updateParametersFormField("UpdateParameters", form)
if err != nil {
return 0, body, err
}

if _, err = io.Copy(part, bytes.NewReader(payload.updateParameters)); err != nil {
return 0, body, err
}

// Add updateFile form
_, err = form.CreateFormFile("UpdateFile", filepath.Base(payload.updateFile.Name()))
if err != nil {
return 0, body, err
}

// determine update file size
finfo, err := payload.updateFile.Stat()
if err != nil {
return 0, body, err
}

// add terminating boundary to multipart form
err = form.Close()
if err != nil {
return 0, body, err
}

return int64(body.Len()) + finfo.Size(), body, nil
}

// runRequestWithMultipartPayload is a copy of https://github.com/stmcginnis/gofish/blob/main/client.go#L349
// with a change to add the UpdateParameters multipart form field with a json content type header
// the resulting form ends up in this format
Expand All @@ -218,34 +283,81 @@ func (c *Conn) firmwareUpdateCompatible(ctx context.Context) (err error) {

// hey.
// --------------------------1771f60800cb2801--
func (c *Conn) runRequestWithMultipartPayload(method, url string, payload map[string]io.Reader) (*http.Response, error) {
func (c *Conn) runRequestWithMultipartPayload(method, url string, payload *multipartPayload) (*http.Response, error) {
if url == "" {
return nil, fmt.Errorf("unable to execute request, no target provided")
}

var payloadBuffer bytes.Buffer
var err error
payloadWriter := multipart.NewWriter(&payloadBuffer)
for key, reader := range payload {
var partWriter io.Writer
if file, ok := reader.(*os.File); ok {
// Add a file stream
if partWriter, err = payloadWriter.CreateFormFile(key, filepath.Base(file.Name())); err != nil {
return nil, err
}
} else {
// Add other fields
if partWriter, err = updateParametersFormField(key, payloadWriter); err != nil {
return nil, err
// A content-length header is passed in to indicate the payload size
//
// The Content-length is set explicitly since the payload is an io.Reader,
// https://github.com/golang/go/blob/ddad9b618cce0ed91d66f0470ddb3e12cfd7eeac/src/net/http/request.go#L861
//
// 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)
if err != nil {
return nil, errors.Wrap(err, "error determining multipart payload size")
}

headers := map[string]string{
"Content-Length": strconv.FormatInt(contentLength, 10),
}

// setup pipe
pipeReader, pipeWriter := io.Pipe()
defer pipeReader.Close()

// initiate a mulitpart writer
form := multipart.NewWriter(pipeWriter)

// go routine blocks on the io.Copy until the http request is made
go func() {
var err error
defer func() {
if err != nil {
c.Log.Error(err, "multipart upload error occurred")
}
}()

defer pipeWriter.Close()

// Add UpdateParameters part
parametersPart, err := updateParametersFormField("UpdateParameters", form)
if err != nil {
c.Log.Error(errMultiPartPayload, err.Error()+": UpdateParameters part copy error")

return
}
if _, err = io.Copy(partWriter, reader); err != nil {
return nil, err

if _, err = io.Copy(parametersPart, bytes.NewReader(payload.updateParameters)); err != nil {
c.Log.Error(errMultiPartPayload, err.Error()+": UpdateParameters part copy error")

return
}
}
payloadWriter.Close()

return c.redfishwrapper.RunRawRequestWithHeaders(method, url, bytes.NewReader(payloadBuffer.Bytes()), payloadWriter.FormDataContentType(), nil)
// Add UpdateFile part
updateFilePart, err := form.CreateFormFile("UpdateFile", filepath.Base(payload.updateFile.Name()))
if err != nil {
c.Log.Error(errMultiPartPayload, err.Error()+": UpdateFile part create error")

return
}

if _, err = io.Copy(updateFilePart, payload.updateFile); err != nil {
c.Log.Error(errMultiPartPayload, err.Error()+": UpdateFile part copy error")

return
}

// add terminating boundary to multipart form
form.Close()
}()

// pipeReader wrapped as a io.ReadSeeker to satisfy the gofish method signature
reader := pipeReaderFakeSeeker{pipeReader}

return c.redfishwrapper.RunRawRequestWithHeaders(method, url, reader, form.FormDataContentType(), headers)
}

// sets up the UpdateParameters MIMEHeader for the multipart form
Expand Down
82 changes: 79 additions & 3 deletions providers/redfish/firmware_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package redfish

import (
"context"
"encoding/json"
"fmt"
"io"
"log"
Expand Down Expand Up @@ -30,6 +31,9 @@ func multipartUpload(w http.ResponseWriter, r *http.Request) {
log.Fatal(err)
}

// payload size
expectedContentLength := "476"

expected := []string{
`Content-Disposition: form-data; name="UpdateParameters"`,
`Content-Type: application/json`,
Expand All @@ -46,11 +50,15 @@ func multipartUpload(w http.ResponseWriter, r *http.Request) {
}
}

if r.Header.Get("Content-Length") != expectedContentLength {
log.Fatal("Header Content-Length does not match expected")
}

w.Header().Add("Location", "/redfish/v1/TaskService/Tasks/JID_467696020275")
w.WriteHeader(http.StatusAccepted)
}

func Test_FirmwareInstall(t *testing.T) {
func TestFirmwareInstall(t *testing.T) {
// curl -Lv -s -k -u root:calvin \
// -F 'UpdateParameters={"Targets": [], "@Redfish.OperationApplyTime": "OnReset", "Oem": {}};type=application/json' \
// -F'foo.bin=@/tmp/dummyfile;application/octet-stream'
Expand Down Expand Up @@ -88,6 +96,17 @@ func Test_FirmwareInstall(t *testing.T) {
false,
nil,
"",
bmclibErrs.ErrFirmwareInstall,
"method expects an *os.File object",
"expect *os.File object",
},
{
common.SlugBIOS,
constants.FirmwareApplyOnReset,
false,
false,
&os.File{},
"",
errInsufficientCtxTimeout,
"",
"remaining context deadline",
Expand All @@ -97,7 +116,7 @@ func Test_FirmwareInstall(t *testing.T) {
"invalidApplyAt",
false,
true,
nil,
&os.File{},
"",
bmclibErrs.ErrFirmwareInstall,
"invalid applyAt parameter",
Expand Down Expand Up @@ -151,7 +170,64 @@ func Test_FirmwareInstall(t *testing.T) {

}

func Test_firmwareUpdateCompatible(t *testing.T) {
func TestMultipartPayloadSize(t *testing.T) {
updateParameters, err := json.Marshal(struct {
Targets []string `json:"Targets"`
RedfishOpApplyTime string `json:"@Redfish.OperationApplyTime"`
Oem struct{} `json:"Oem"`
}{
[]string{},
"foobar",
struct{}{},
})

if err != nil {
t.Fatal(err)
}

tmpdir := t.TempDir()
binPath := filepath.Join(tmpdir, "test.bin")
err = os.WriteFile(binPath, []byte(`HELLOWORLD`), 0600)
if err != nil {
t.Fatal(err)
}

testfileFH, err := os.Open(binPath)
if err != nil {
t.Fatalf("%s -> %s", err.Error(), binPath)
}

testCases := []struct {
testName string
payload *multipartPayload
expectedSize int64
errorMsg string
}{
{
"content length as expected",
&multipartPayload{
updateParameters: updateParameters,
updateFile: testfileFH,
},
475,
"",
},
}

for _, tc := range testCases {
t.Run(tc.testName, func(t *testing.T) {
gotSize, _, err := multipartPayloadSize(tc.payload)
if tc.errorMsg != "" {
assert.Contains(t, err.Error(), tc.errorMsg)
}

assert.Nil(t, err)
assert.Equal(t, tc.expectedSize, gotSize)
})
}
}

func TestFirmwareUpdateCompatible(t *testing.T) {
err := mockClient.firmwareUpdateCompatible(context.TODO())
if err != nil {
t.Fatal(err)
Expand Down

0 comments on commit f99e587

Please sign in to comment.