From 7ac6362350f413ffd176a3993a37c1f945488b85 Mon Sep 17 00:00:00 2001 From: Drew Walters Date: Fri, 24 Apr 2020 14:49:11 +0000 Subject: [PATCH] Decode Redfish error responses as raw JSON Some BMCs return response formats that differ from the DMTF Redfish specification. When the response format varies, airshipctl is unable to decode the extended information field, leaving the user with the general, top-level error message "A general error has occurred. See ExtendedInfo for more information". This change introduces raw JSON processing for all Redfish error responses so both observed formats can be handled until this issue is addressed by the manufacturers. Change-Id: I6a6e87ecda65292b3cd58a0525985105db350ab8 Signed-off-by: Drew Walters --- pkg/remote/redfish/errors.go | 10 +++ pkg/remote/redfish/utils.go | 103 ++++++++++++++++++++++++------- pkg/remote/redfish/utils_test.go | 72 +++++++++++++++++++++ 3 files changed, 164 insertions(+), 21 deletions(-) diff --git a/pkg/remote/redfish/errors.go b/pkg/remote/redfish/errors.go index 0299a25ee..01c26a9d9 100644 --- a/pkg/remote/redfish/errors.go +++ b/pkg/remote/redfish/errors.go @@ -48,3 +48,13 @@ type ErrOperationRetriesExceeded struct { func (e ErrOperationRetriesExceeded) Error() string { return fmt.Sprintf("operation %s failed. Maximum retries (%d) exceeded", e.What, e.Retries) } + +// ErrUnrecognizedRedfishResponse is a debug error that describes unexpected formats in a Redfish error response. +type ErrUnrecognizedRedfishResponse struct { + aerror.AirshipError + Key string +} + +func (e ErrUnrecognizedRedfishResponse) Error() string { + return fmt.Sprintf("Unable to decode Redfish response. Key '%s' is missing or has unknown format.", e.Key) +} diff --git a/pkg/remote/redfish/utils.go b/pkg/remote/redfish/utils.go index 492332f14..37832bb19 100644 --- a/pkg/remote/redfish/utils.go +++ b/pkg/remote/redfish/utils.go @@ -30,6 +30,80 @@ const ( URLSchemeSeparator = "+" ) +// DecodeRawError decodes a raw Redfish HTTP response and retrieves the extended information and available resolutions +// returned by the BMC. +func DecodeRawError(rawResponse []byte) (string, error) { + processExtendedInfo := func(extendedInfo map[string]interface{}) (string, error) { + message, ok := extendedInfo["Message"] + if !ok { + return "", ErrUnrecognizedRedfishResponse{Key: "error.@Message.ExtendedInfo.Message"} + } + + messageContent, ok := message.(string) + if !ok { + return "", ErrUnrecognizedRedfishResponse{Key: "error.@Message.ExtendedInfo.Message"} + } + + // Resolution may be omitted in some responses + if resolution, ok := extendedInfo["Resolution"]; ok { + return fmt.Sprintf("%s %s", messageContent, resolution), nil + } + + return messageContent, nil + } + + // Unmarshal raw Redfish response as arbitrary JSON map + var arbitraryJSON map[string]interface{} + if err := json.Unmarshal(rawResponse, &arbitraryJSON); err != nil { + return "", ErrUnrecognizedRedfishResponse{Key: "error"} + } + + errObject, ok := arbitraryJSON["error"] + if !ok { + return "", ErrUnrecognizedRedfishResponse{Key: "error"} + } + + errContent, ok := errObject.(map[string]interface{}) + if !ok { + return "", ErrUnrecognizedRedfishResponse{Key: "error"} + } + + extendedInfoContent, ok := errContent["@Message.ExtendedInfo"] + if !ok { + return "", ErrUnrecognizedRedfishResponse{Key: "error.@Message.ExtendedInfo"} + } + + // NOTE(drewwalters96): The official specification dictates that "@Message.ExtendedInfo" should be a JSON array; + // however, some BMCs have returned a single JSON dictionary. Handle both types here. + switch extendedInfo := extendedInfoContent.(type) { + case []interface{}: + if len(extendedInfo) == 0 { + return "", ErrUnrecognizedRedfishResponse{Key: "error.@MessageExtendedInfo"} + } + + var errorMessage string + for _, info := range extendedInfo { + infoContent, ok := info.(map[string]interface{}) + if !ok { + return "", ErrUnrecognizedRedfishResponse{Key: "error.@Message.ExtendedInfo"} + } + + message, err := processExtendedInfo(infoContent) + if err != nil { + return "", err + } + + errorMessage = fmt.Sprintf("%s\n%s", message, errorMessage) + } + + return errorMessage, nil + case map[string]interface{}: + return processExtendedInfo(extendedInfo) + default: + return "", ErrUnrecognizedRedfishResponse{Key: "error.@Message.ExtendedInfo"} + } +} + // GetManagerID retrieves the manager ID for a redfish system. func GetManagerID(ctx context.Context, api redfishAPI.RedfishAPI, systemID string) (string, error) { system, httpResp, err := api.GetSystem(ctx, systemID) @@ -131,30 +205,17 @@ func ScreenRedfishError(httpResp *http.Response, clientErr error) error { httpResp.Status) } - // NOTE(drewwalters96) Check for error messages with extended information, as they often accompany more - // general error descriptions provided in "clientErr". Since there can be multiple errors, wrap them in a - // single error. - var redfishErr redfishClient.RedfishError - var additionalInfo string - + // Retrieve the raw HTTP response body oAPIErr, ok := clientErr.(redfishClient.GenericOpenAPIError) - if ok { - if err := json.Unmarshal(oAPIErr.Body(), &redfishErr); err == nil { - additionalInfo = "" - for _, extendedInfo := range redfishErr.Error.MessageExtendedInfo { - additionalInfo = fmt.Sprintf("%s %s %s", additionalInfo, extendedInfo.Message, - extendedInfo.Resolution) - } - } else { - log.Debugf("Unable to unmarshal the raw BMC error response. %s", err.Error()) - } + if !ok { + log.Debug("Unable to decode BMC response.") } - if strings.TrimSpace(additionalInfo) != "" { - finalError.Message = fmt.Sprintf("%s %s", finalError.Message, additionalInfo) - } else if redfishErr.Error.Message != "" { - // Provide the general error message when there were no messages containing extended information. - finalError.Message = fmt.Sprintf("%s %s", finalError.Message, redfishErr.Error.Message) + // Attempt to decode the BMC response from the raw HTTP response + if bmcResponse, err := DecodeRawError(oAPIErr.Body()); err == nil { + finalError.Message = fmt.Sprintf("%s BMC responded: '%s'", finalError.Message, bmcResponse) + } else { + log.Debugf("Unable to decode BMC response. %q", err) } return finalError diff --git a/pkg/remote/redfish/utils_test.go b/pkg/remote/redfish/utils_test.go index 98f9ec7ac..95efe0632 100644 --- a/pkg/remote/redfish/utils_test.go +++ b/pkg/remote/redfish/utils_test.go @@ -29,6 +29,78 @@ import ( testutil "opendev.org/airship/airshipctl/testutil/redfishutils/helpers" ) +const ( + redfishHTTPErrDMTF = ` +{ + "error": { + "message": "A general error has occurred. See Resolution for information on how to resolve the error.", + "@Message.ExtendedInfo": [ + { + "Message": "Extended error message.", + "Resolution": "Resolution message." + }, + { + "Message": "Extended message 2.", + "Resolution": "Resolution message 2." + } + ] + } +}` + redfishHTTPErrOther = ` +{ + "error": { + "message": "A general error has occurred. See Resolution for information on how to resolve the error.", + "@Message.ExtendedInfo": { + "Message": "Extended error message.", + "Resolution": "Resolution message." + } + } +}` + redfishHTTPErrMalformatted = ` +{ + "error": { + "message": "A general error has occurred. See Resolution for information on how to resolve the error.", + "@Message.ExtendedInfo": {} + } +}` + redfishHTTPErrEmptyList = ` +{ + "error": { + "message": "A general error has occurred. See Resolution for information on how to resolve the error.", + "@Message.ExtendedInfo": [] + } +}` +) + +func TestDecodeRawErrorEmptyInput(t *testing.T) { + _, err := redfish.DecodeRawError([]byte("{}")) + assert.Error(t, err) +} + +func TestDecodeRawErrorEmptyList(t *testing.T) { + _, err := redfish.DecodeRawError([]byte(redfishHTTPErrEmptyList)) + assert.Error(t, err) +} + +func TestDecodeRawErrorEmptyMalformatted(t *testing.T) { + _, err := redfish.DecodeRawError([]byte(redfishHTTPErrMalformatted)) + assert.Error(t, err) +} + +func TestDecodeRawErrorDMTF(t *testing.T) { + message, err := redfish.DecodeRawError([]byte(redfishHTTPErrDMTF)) + assert.NoError(t, err) + assert.Equal(t, "Extended message 2. Resolution message 2.\nExtended error message. Resolution message.\n", + message) +} + +func TestDecodeRawErrorOther(t *testing.T) { + message, err := redfish.DecodeRawError([]byte(redfishHTTPErrOther)) + assert.NoError(t, err) + assert.Equal(t, "Extended error message. Resolution message.", + message) +} + func TestRedfishErrorNoError(t *testing.T) { err := redfish.ScreenRedfishError(&http.Response{StatusCode: 200}, nil) assert.NoError(t, err)