Skip to content

Commit

Permalink
Fix "missing error information" error on 409s (#4530)
Browse files Browse the repository at this point in the history
This can happen if the Azure RP being called returns a JSON error
response payload that doesn't match the Azure contract. Instead of
overwriting the underlying error, just return it as-is to aid in
debugging.
  • Loading branch information
matthchr authored Jan 17, 2025
1 parent 1b05811 commit d8c6ccd
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 3 deletions.
59 changes: 59 additions & 0 deletions v2/internal/genericarmclient/generic_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,3 +219,62 @@ func Test_NewResourceGroup_SubscriptionNotRegisteredError(t *testing.T) {
g.Expect(err).To(HaveOccurred())
g.Expect(err.Error()).To(Equal("registering Resource Provider Microsoft.Fake with subscription. Try again later"))
}

var rpMalformedConflictError = `
{
"code": "OperationNotAllowed",
"message": "The operation is not allowed."
}`

func Test_NewResourceGroup_ConflictWithMalformedErrorShape(t *testing.T) {
t.Parallel()
g := NewGomegaWithT(t)
ctx := context.Background()

server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method == http.MethodPut {
if r.URL.Path == "/subscriptions/12345/resourceGroups/myrg/providers/Microsoft.Fake/fakeResource/fake" {
w.WriteHeader(http.StatusConflict)
g.Expect(w.Write([]byte(rpMalformedConflictError))).ToNot(BeZero())
return
}
}

g.Fail(fmt.Sprintf("unknown request attempted. Method: %s, URL: %s", r.Method, r.URL))
}))
defer server.Close()

cfg := cloud.Configuration{
Services: map[cloud.ServiceName]cloud.ServiceConfiguration{
cloud.ResourceManager: {
Endpoint: server.URL,
Audience: cloud.AzurePublic.Services[cloud.ResourceManager].Audience,
},
},
}
subscriptionId := "12345"

metrics := asometrics.NewARMClientMetrics()
options := &genericarmclient.GenericClientOptions{
HttpClient: server.Client(),
Metrics: metrics,
}
client, err := genericarmclient.NewGenericClient(cfg, creds.MockTokenCredential{}, options)
g.Expect(err).ToNot(HaveOccurred())

resourceURI := fmt.Sprintf("/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Fake/fakeResource/fake", subscriptionId, "myrg")
apiVersion := "2019-01-01"
resource := &resources.ResourceGroup{
ObjectMeta: metav1.ObjectMeta{
Name: "name",
},
Spec: resources.ResourceGroup_Spec{
Location: to.Ptr("westus"),
},
}

_, err = client.BeginCreateOrUpdateByID(ctx, resourceURI, apiVersion, resource)
g.Expect(err).To(MatchError(ContainSubstring("409 Conflict")))
g.Expect(err).To(MatchError(ContainSubstring("OperationNotAllowed")))
g.Expect(err).To(MatchError(ContainSubstring("The operation is not allowed")))
}
11 changes: 8 additions & 3 deletions v2/internal/genericarmclient/policy_register_rp.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,16 @@ func (r *rpRegistrationPolicy) Do(req *azpolicy.Request) (*http.Response, error)
return resp, err
}
var reqErr requestError
if err = runtime.UnmarshalAsJSON(resp, &reqErr); err != nil {
return resp, err
if unmarshalErr := runtime.UnmarshalAsJSON(resp, &reqErr); unmarshalErr != nil {
return resp, unmarshalErr
}
if reqErr.ServiceError == nil {
return resp, errors.New("missing error information")
// This is unexpected and likely due to Azure RPs failing to adhere to the expected
// error shape contract. The good news is that if this happens we know this isn't
// an unregistered RP because that API will have the right shape. We just return
// the resp/err directly in this case to let the caller deal with whatever the problem
// was as we know it wasn't unregistered RP.
return resp, err
}
if !strings.EqualFold(reqErr.ServiceError.Code, unregisteredRPCode) {
// not a 409 due to unregistered RP
Expand Down

0 comments on commit d8c6ccd

Please sign in to comment.