Skip to content

Commit

Permalink
Fix azure.IsNotFound for new azcore error type
Browse files Browse the repository at this point in the history
  • Loading branch information
thomas11 committed Jan 9, 2025
1 parent a8ea0a6 commit 3c330f6
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 9 deletions.
5 changes: 5 additions & 0 deletions provider/pkg/azure/azure.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
39 changes: 39 additions & 0 deletions provider/pkg/azure/azure_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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",
}))
})
}
24 changes: 18 additions & 6 deletions provider/pkg/azure/client_azcore.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}
14 changes: 11 additions & 3 deletions provider/pkg/azure/client_azcore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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)))
})
}

0 comments on commit 3c330f6

Please sign in to comment.