From 2dfd03cb4b5d063370cd6a7ab28374db1e845802 Mon Sep 17 00:00:00 2001 From: Thomas Kappler Date: Thu, 9 Jan 2025 17:53:49 +0100 Subject: [PATCH] Fix azure.IsNotFound for new azcore error type --- provider/pkg/azure/azure.go | 5 +++ provider/pkg/azure/azure_test.go | 39 ++++++++++++++++++++++++ provider/pkg/azure/client_azcore.go | 24 +++++++++++---- provider/pkg/azure/client_azcore_test.go | 14 +++++++-- 4 files changed, 73 insertions(+), 9 deletions(-) diff --git a/provider/pkg/azure/azure.go b/provider/pkg/azure/azure.go index 8800ccdcfc4e..fcf560b98844 100644 --- a/provider/pkg/azure/azure.go +++ b/provider/pkg/azure/azure.go @@ -49,6 +49,11 @@ func IsNotFound(err error) bool { if responseError, ok := err.(*azcore.ResponseError); ok { return responseError.StatusCode == http.StatusNotFound } + + if responseError, ok := err.(*PulumiAzcoreResponseError); ok { + return responseError.StatusCode == http.StatusNotFound + } + return false } diff --git a/provider/pkg/azure/azure_test.go b/provider/pkg/azure/azure_test.go index d7cf2a27fc7e..a9d96bf1b637 100644 --- a/provider/pkg/azure/azure_test.go +++ b/provider/pkg/azure/azure_test.go @@ -3,9 +3,13 @@ package azure import ( + "net/http" "testing" + "github.com/Azure/azure-sdk-for-go/sdk/azcore" "github.com/Azure/azure-sdk-for-go/sdk/azcore/cloud" + "github.com/Azure/go-autorest/autorest" + autorestAzure "github.com/Azure/go-autorest/autorest/azure" "github.com/stretchr/testify/assert" ) @@ -23,3 +27,38 @@ func TestGetCloudByName(t *testing.T) { assert.Equal(t, tc.expected, GetCloudByName(tc.name), tc.name) } } + +func TestIsNotFound(t *testing.T) { + t.Run("autorest", func(t *testing.T) { + assert.True(t, IsNotFound(&autorestAzure.RequestError{ + DetailedError: autorest.DetailedError{ + StatusCode: http.StatusNotFound, + }, + })) + assert.False(t, IsNotFound(&autorestAzure.RequestError{ + DetailedError: autorest.DetailedError{ + StatusCode: http.StatusForbidden, + }, + })) + }) + + t.Run("azcore", func(t *testing.T) { + assert.True(t, IsNotFound(&azcore.ResponseError{ + StatusCode: http.StatusNotFound, + })) + assert.False(t, IsNotFound(&azcore.ResponseError{ + StatusCode: http.StatusForbidden, + })) + }) + + t.Run("provider", func(t *testing.T) { + assert.True(t, IsNotFound(&PulumiAzcoreResponseError{ + StatusCode: http.StatusNotFound, + ErrorCode: "ResourceNotFound", + })) + assert.False(t, IsNotFound(&PulumiAzcoreResponseError{ + StatusCode: http.StatusForbidden, + ErrorCode: "Unauthorized", + })) + }) +} diff --git a/provider/pkg/azure/client_azcore.go b/provider/pkg/azure/client_azcore.go index 8fbe1a9e70b0..d5b37a7ab9cb 100644 --- a/provider/pkg/azure/client_azcore.go +++ b/provider/pkg/azure/client_azcore.go @@ -529,6 +529,7 @@ func (r *requestAssertingTransporter) Do(req *http.Request) (*http.Response, err return nil, nil } +// newResponseError replaces an azcore.ResponseError, created from the given response, with a more concise error type (#3778). func newResponseError(resp *http.Response) error { err := runtime.NewResponseError(resp) azcoreErr, ok := err.(*azcore.ResponseError) @@ -553,12 +554,23 @@ func newResponseError(resp *http.Response) error { } } - errCode := azcoreErr.ErrorCode - if errCode == "" { - errCode = fmt.Sprintf("%d", resp.StatusCode) + return &PulumiAzcoreResponseError{ + StatusCode: resp.StatusCode, + ErrorCode: azcoreErr.ErrorCode, + Message: errMsg, } +} + +// We use this error type instead of azcore.ResponseError because the latter has a very verbose error message (#3778). +type PulumiAzcoreResponseError struct { + StatusCode int + ErrorCode string + Message string +} - // The capitalized message replicates the error message format of the previous autorest client. - //nolint:ST1005 - return fmt.Errorf(`Code="%s" Message="%s"`, errCode, errMsg) +func (e *PulumiAzcoreResponseError) Error() string { + if e.ErrorCode == "" { + return fmt.Sprintf(`Status=%d Message="%s"`, e.StatusCode, e.Message) + } + return fmt.Sprintf(`Status=%d Code="%s" Message="%s"`, e.StatusCode, e.ErrorCode, e.Message) } diff --git a/provider/pkg/azure/client_azcore_test.go b/provider/pkg/azure/client_azcore_test.go index 0c06535ec6e7..b378980ff69f 100644 --- a/provider/pkg/azure/client_azcore_test.go +++ b/provider/pkg/azure/client_azcore_test.go @@ -781,7 +781,7 @@ func TestNewResponseError(t *testing.T) { } err := newResponseError(resp) require.Error(t, err) - assert.Equal(t, "Code=\"Conflict\" Message=\"Foo already exists\"", err.Error()) + assert.Equal(t, "Status=409 Code=\"Conflict\" Message=\"Foo already exists\"", err.Error()) }) t.Run("no message", func(t *testing.T) { @@ -792,7 +792,7 @@ func TestNewResponseError(t *testing.T) { } err := newResponseError(resp) require.Error(t, err) - assert.Equal(t, `Code="Conflict" Message="{"error": {"code": "Conflict"}}"`, err.Error()) + assert.Equal(t, `Status=409 Code="Conflict" Message="{"error": {"code": "Conflict"}}"`, err.Error()) }) t.Run("unknown error", func(t *testing.T) { @@ -802,6 +802,14 @@ func TestNewResponseError(t *testing.T) { } err := newResponseError(resp) require.Error(t, err) - assert.Equal(t, `Code="409" Message="{"foo": "bar"}"`, err.Error()) + assert.Equal(t, `Status=409 Message="{"foo": "bar"}"`, err.Error()) + }) + + t.Run("404 is recognized by IsNotFound", func(t *testing.T) { + resp := &http.Response{ + StatusCode: 404, + Body: io.NopCloser(strings.NewReader(`not found`)), + } + assert.True(t, IsNotFound(newResponseError(resp))) }) }