From 1bed5fd22f489860a446618550dee1d8bebf14db Mon Sep 17 00:00:00 2001 From: Matt Primrose Date: Mon, 16 Sep 2024 17:58:35 -0700 Subject: [PATCH] fix: centralized error handling to be more central --- pkg/amterror/error.go | 87 +++++++++++++++++++++++++++++ pkg/amterror/error_test.go | 40 +++++++++++++ pkg/wsman/amt/boot/settingdata.go | 10 ---- pkg/wsman/cim/kvm/redirectionsap.go | 10 ---- pkg/wsman/client/wsman.go | 28 +++++----- pkg/wsman/common/constants.go | 24 -------- pkg/wsman/common/constants_test.go | 73 ------------------------ pkg/wsman/common/types.go | 43 -------------- 8 files changed, 140 insertions(+), 175 deletions(-) create mode 100644 pkg/amterror/error.go create mode 100644 pkg/amterror/error_test.go delete mode 100644 pkg/wsman/common/constants_test.go diff --git a/pkg/amterror/error.go b/pkg/amterror/error.go new file mode 100644 index 00000000..741c3ccc --- /dev/null +++ b/pkg/amterror/error.go @@ -0,0 +1,87 @@ +package amterror + +import ( + "encoding/xml" + "fmt" +) + +func (e *AMTError) Error() string { + return fmt.Sprintf("Error [SubCode: %s] Message: %s, Detail: %s", e.SubCode, e.Message, e.Detail) +} + +func NewAMTError(subCode, message, detail string) *AMTError { + return &AMTError{ + SubCode: subCode, + Message: message, + Detail: detail, + } +} + +func DecodeAMTErrorString(s string) error { + checkForErrorResponse := ErrorResponse{} + + err := xml.Unmarshal([]byte(s), &checkForErrorResponse) + if err != nil { + return err + } + + return NewAMTError(checkForErrorResponse.Body.Fault.Code.SubCode.Value, checkForErrorResponse.Body.Fault.Reason.Text, checkForErrorResponse.Body.Fault.Detail) +} + +// AMT WSMAN Error Response Types. +type ( + AMTError struct { + SubCode string + Message string + Detail string + } + + Header struct { + XMLName xml.Name `xml:"Header"` + To string `xml:"To"` + RelatesTo int `xml:"RelatesTo"` + Action Action `xml:"Action"` + MessageID string `xml:"MessageID"` + ResourceURI string `xml:"ResourceURI"` + } + + Action struct { + XMLName xml.Name `xml:"Action"` + MustUnderstand string `xml:"mustUnderstand,attr"` + Value string `xml:",chardata"` + } + + ErrorResponse struct { + XMLName xml.Name `xml:"Envelope"` + Header Header `xml:"Header"` + Body ErrorBody `xml:"Body"` + } + + ErrorBody struct { + XMLName xml.Name `xml:"Body"` + Fault Fault `xml:"Fault"` + } + + Fault struct { + XMLName xml.Name `xml:"Fault"` + Code Code `xml:"Code"` + Reason Reason `xml:"Reason"` + Detail string `xml:"Detail"` + } + + Code struct { + XMLName xml.Name `xml:"Code"` + Value string `xml:"Value"` + SubCode SubCode `xml:"Subcode"` + } + + SubCode struct { + XMLName xml.Name `xml:"Subcode"` + Value string `xml:"Value"` + } + + Reason struct { + XMLName xml.Name `xml:"Reason"` + Text string `xml:"Text"` + } +) diff --git a/pkg/amterror/error_test.go b/pkg/amterror/error_test.go new file mode 100644 index 00000000..1d1753dc --- /dev/null +++ b/pkg/amterror/error_test.go @@ -0,0 +1,40 @@ +/********************************************************************* + * Copyright (c) Intel Corporation 2024 + * SPDX-License-Identifier: Apache-2.0 + **********************************************************************/ + +package amterror + +import ( + "encoding/xml" + "testing" +) + +func TestDecodeWSMANError(t *testing.T) { + tests := []struct { + input string + expected error + }{ + { + "http://schemas.xmlsoap.org/ws/2004/08/addressing/role/anonymous0http://schemas.xmlsoap.org/ws/2004/08/addressing/faultuuid:00000000-8086-8086-8086-000000000061a:Senderb:DestinationUnreachableNo route can be determined to reach the destination role defined by the WSAddressing To.", + NewAMTError("b:DestinationUnreachable", "No route can be determined to reach the destination role defined by the WSAddressing To.", ""), + }, + { + "bad xml", + xml.Unmarshal([]byte("bad xml"), &ErrorResponse{}), + }, + } + + for _, test := range tests { + result := DecodeAMTErrorString(test.input) + if result == nil { + if test.expected != nil { + t.Errorf("Expected %s, but got nil", test.expected) + } + } else { + if result.Error() != test.expected.Error() { + t.Errorf("Expected %s, but got %s", test.expected, result) + } + } + } +} diff --git a/pkg/wsman/amt/boot/settingdata.go b/pkg/wsman/amt/boot/settingdata.go index a3a143d2..23a88c27 100644 --- a/pkg/wsman/amt/boot/settingdata.go +++ b/pkg/wsman/amt/boot/settingdata.go @@ -11,7 +11,6 @@ import ( "github.com/open-amt-cloud-toolkit/go-wsman-messages/v2/internal/message" "github.com/open-amt-cloud-toolkit/go-wsman-messages/v2/pkg/wsman/client" - "github.com/open-amt-cloud-toolkit/go-wsman-messages/v2/pkg/wsman/common" ) // Instantiates a new Boot Setting Data service. @@ -148,14 +147,5 @@ func (settingData SettingData) Put(bootSettingData BootSettingDataRequest) (resp return response, err } - checkForErrorResponse := common.ErrorResponse{} - - err = xml.Unmarshal([]byte(response.XMLOutput), &checkForErrorResponse) - if err != nil { - return response, err - } - - err = common.DecodeAMTError(checkForErrorResponse) - return response, err } diff --git a/pkg/wsman/cim/kvm/redirectionsap.go b/pkg/wsman/cim/kvm/redirectionsap.go index 6f066446..c5854087 100644 --- a/pkg/wsman/cim/kvm/redirectionsap.go +++ b/pkg/wsman/cim/kvm/redirectionsap.go @@ -12,7 +12,6 @@ import ( "github.com/open-amt-cloud-toolkit/go-wsman-messages/v2/internal/message" "github.com/open-amt-cloud-toolkit/go-wsman-messages/v2/pkg/wsman/cim/methods" "github.com/open-amt-cloud-toolkit/go-wsman-messages/v2/pkg/wsman/client" - "github.com/open-amt-cloud-toolkit/go-wsman-messages/v2/pkg/wsman/common" ) // NewKVMRedirectionSAP returns a new instance of the KVMRedirectionSAP struct. @@ -103,14 +102,5 @@ func (redirectionSAP RedirectionSAP) Pull(enumerationContext string) (response R return } - checkForErrorResponse := common.ErrorResponse{} - - err = xml.Unmarshal([]byte(response.XMLOutput), &checkForErrorResponse) - if err != nil { - return - } - - err = common.DecodeAMTError(checkForErrorResponse) - return } diff --git a/pkg/wsman/client/wsman.go b/pkg/wsman/client/wsman.go index ff16e6f6..5a8aef6e 100644 --- a/pkg/wsman/client/wsman.go +++ b/pkg/wsman/client/wsman.go @@ -21,6 +21,7 @@ import ( "sync" "time" + "github.com/open-amt-cloud-toolkit/go-wsman-messages/v2/pkg/amterror" "github.com/sirupsen/logrus" ) @@ -256,21 +257,6 @@ func (t *Target) Post(msg string) (response []byte, err error) { defer res.Body.Close() - if res.StatusCode >= 400 { - b, err := io.ReadAll(res.Body) - if err != nil { - return nil, err - } - - if t.logAMTMessages { - logrus.Trace(string(b)) - } - - errPostResponse := errors.New("wsman.Client post received") - - return nil, fmt.Errorf("%w: %v\n%v", errPostResponse, res.Status, string(b)) - } - response, err = io.ReadAll(res.Body) if t.logAMTMessages { @@ -281,6 +267,18 @@ func (t *Target) Post(msg string) (response []byte, err error) { return nil, err } + if res.StatusCode == 400 { + amterr := amterror.DecodeAMTErrorString(string(response)) + + return nil, amterr + } + + if res.StatusCode >= 401 { + errPostResponse := errors.New("wsman.Client post received") + + return nil, fmt.Errorf("%w: %v\n%v", errPostResponse, res.Status, string(response)) + } + return response, nil } diff --git a/pkg/wsman/common/constants.go b/pkg/wsman/common/constants.go index ef421248..3ca60443 100644 --- a/pkg/wsman/common/constants.go +++ b/pkg/wsman/common/constants.go @@ -5,10 +5,6 @@ package common -import ( - "fmt" -) - // TODO: Review if this file is still necessary. const ValueNotFound string = "Value not found in map" @@ -235,23 +231,3 @@ func ConvertPackageTypeToString(value int) string { return ValueNotFound } - -func (e *AMTError) Error() string { - return fmt.Sprintf("Error [SubCode: %s] Message: %s, Detail: %s", e.SubCode, e.Message, e.Detail) -} - -func NewAMTError(subCode, message, detail string) *AMTError { - return &AMTError{ - SubCode: subCode, - Message: message, - Detail: detail, - } -} - -func DecodeAMTError(er ErrorResponse) error { - if er.Body.Fault.Code.SubCode.Value != "" { - return NewAMTError(er.Body.Fault.Code.SubCode.Value, er.Body.Fault.Reason.Text, er.Body.Fault.Detail) - } - - return nil -} diff --git a/pkg/wsman/common/constants_test.go b/pkg/wsman/common/constants_test.go deleted file mode 100644 index d5aaa977..00000000 --- a/pkg/wsman/common/constants_test.go +++ /dev/null @@ -1,73 +0,0 @@ -/********************************************************************* - * Copyright (c) Intel Corporation 2024 - * SPDX-License-Identifier: Apache-2.0 - **********************************************************************/ - -package common - -import ( - "testing" -) - -func TestDecodeWSMANError(t *testing.T) { - tests := []struct { - input ErrorResponse - expected error - }{ - { - ErrorResponse{ - Body: ErrorBody{ - Fault: Fault{ - Code: Code{ - SubCode: SubCode{ - Value: "b:AccessDenied", - }, - }, - Reason: Reason{ - Text: "The sender was not authorized to access the resource.", - }, - Detail: "", - }, - }, - }, - &AMTError{ - SubCode: "b:AccessDenied", Message: "The sender was not authorized to access the resource.", Detail: "", - }, - }, { - ErrorResponse{ - Body: ErrorBody{ - Fault: Fault{ - Code: Code{ - SubCode: SubCode{ - Value: "e:DestinationUnreachable", - }, - }, - Reason: Reason{ - Text: "No route can be determined to reach the destination role defined by the WSAddressing To.", - }, - Detail: "", - }, - }, - }, - &AMTError{ - SubCode: "e:DestinationUnreachable", Message: "No route can be determined to reach the destination role defined by the WSAddressing To.", Detail: "", - }, - }, { - ErrorResponse{}, - nil, - }, - } - - for _, test := range tests { - result := DecodeAMTError(test.input) - if result == nil { - if test.expected != nil { - t.Errorf("Expected %s, but got nil", test.expected) - } - } else { - if result.Error() != test.expected.Error() { - t.Errorf("Expected %s, but got %s", test.expected, result) - } - } - } -} diff --git a/pkg/wsman/common/types.go b/pkg/wsman/common/types.go index d01e699e..16c4096d 100644 --- a/pkg/wsman/common/types.go +++ b/pkg/wsman/common/types.go @@ -30,46 +30,3 @@ type ReturnValue struct { ReturnValue int `xml:"ReturnValue,omitempty"` ReturnValueStr string `xml:"ReturnValueStr,omitempty"` } - -// AMT WSMAN Error Response Types. -type ( - AMTError struct { - SubCode string - Message string - Detail string - } - - ErrorResponse struct { - XMLName xml.Name `xml:"Envelope"` - Header message.Header `xml:"Header"` - Body ErrorBody `xml:"Body"` - } - - ErrorBody struct { - XMLName xml.Name `xml:"Body"` - Fault Fault `xml:"Fault"` - } - - Fault struct { - XMLName xml.Name `xml:"Fault"` - Code Code `xml:"Code"` - Reason Reason `xml:"Reason"` - Detail string `xml:"Detail"` - } - - Code struct { - XMLName xml.Name `xml:"Code"` - Value string `xml:"Value"` - SubCode SubCode `xml:"Subcode"` - } - - SubCode struct { - XMLName xml.Name `xml:"Subcode"` - Value string `xml:"Value"` - } - - Reason struct { - XMLName xml.Name `xml:"Reason"` - Text string `xml:"Text"` - } -)