armresources.DeploymentsClient Validate always returns "the operation failed or was cancelled" not the actual error

[matthchr]

Bug Report

The BeginValidate function of DeploymentsClient doesn't correctly return errors.

• import path of package in question, e.g. "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources"
• SDK version: 1.2.0

I believe that this is because the service defines the following in its Swagger for this operation:

        "responses": {
          "200": {
            "description": "OK - Returns the validation result.",
            "schema": {
              "$ref": "#/definitions/DeploymentValidateResult"
            }
          },
          "202": {
            "description": "Accepted - The request has been accepted for processing and the operation will complete asynchronously."
          },
          "400": {
            "description": "Returns the validation result.",
            "schema": {
              "$ref": "#/definitions/DeploymentValidateResult"
            }
          },

This results in HTTP 400 not producing an error, here

Which then ends up hitting this code:

	// this is a back-stop in case the swagger is incorrect (i.e. missing one or more status codes for success).
	// ideally the codegen should return an error if the initial response failed and not even create a poller.
	if !poller.StatusCodeValid(resp) {
		return nil, errors.New("the operation failed or was cancelled")
	}

The result: Any failed deployment validate will never report any error except "the operation failed or was cancelled".

[matthchr]

I'm not clear if the problem here is that the service Swagger shouldn't document HTTP 400, or if the Go SDK should allow them to document it and define it as a "success case"

[jhendrixMSFT]

The responses section document the HTTP status codes returned by the service for a successful request/response. Presumably the service doesn't actually return a 400 as one of the success cases, in which case it should be removed from the swagger.

[matthchr]

What's the best way to report this and get it fixed? If this gets changed in the upstream Swagger, will the Go SDK automatically be regenerated w/ the fix or is there anything else that needs to be done to trigger taking the fix?

[matthchr]

Looking at this a bit more, this behavior seems to differ from that of the Track 1 SDK (shown below), which tolerates the 400:

azure-sdk-for-go/services/resources/mgmt/2018-05-01/resources/deployments.go

Lines 1515 to 1525 in 2187aca

	// ValidateResponder handles the response to the Validate request. The method always
	// closes the http.Response Body.
	func (client DeploymentsClient) ValidateResponder(resp *http.Response) (result DeploymentValidateResult, err error) {
	err = autorest.Respond(
	resp,
	azure.WithErrorUnlessStatusCode(http.StatusOK, http.StatusBadRequest),
	autorest.ByUnmarshallingJSON(&result),
	autorest.ByClosing())
	result.Response = autorest.Response{Response: resp}
	return
	}

The shape of the "success" response indicates that the intent here is that regardless of if the service returns 2xx or 400, the user doesn't get an error return, they just get a return of the type DeploymentValidateResult, which has error as a field. Changing the service Swagger would alter this behavior.

Looking at the Python SDK, it supports this pattern:
https://github.com/Azure/azure-sdk-for-python/blob/e9e626f6d1373723c91d9b2b55fc128372091cea/sdk/resources/azure-mgmt-resource/azure/mgmt/resource/resources/v2021_04_01/operations/_operations.py#L7295-7301

So I don't think we can just change the upstream Swagger - the issue seems to be that this pattern (documenting a "success" 400 response) isn't supported in the Go SDK, at least for LROs (it might work just fine for non-LROs because they don't use the poller and so bypass the !poller.StatusCodeValid(resp) check), but it is supported for other SDKs.

[jhendrixMSFT]

@JeffreyRichter for ARM LROs that are POST, is it acceptable that the terminal success includes a 400? I'm looking at the latest design guidelines for ARM and it only mentions 200/204.

For context, the way this API works is as follows.

• template is submitted to validate it's syntactically correct. this happens async
• if the template is valid, the terminal success case returns a 200 with a populated DeploymentsClientValidateResponse
  • the DeploymentsClientValidateResponse.Error field is nil in this case
• if the template is invalid, the terminal success case returns a 400 with the DeploymentsClientValidateResponse.Error field populated (the other fields are nil).

Shouldn't this always return a 200 with the appropriate fields set?

[JeffreyRichter]

Polling a status monitor resource with GET should return 200 with the status monitor. A field in the status monitor could indicate failure or an error field. What I say is true for data plane. And I think it should be true for ARM, but I'm not involved with ARM guidelines. So, you might want to ask the ARM reviewers alias.

…

________________________________ From: Joel Hendrix ***@***.***> Sent: Tuesday, March 18, 2025 1:14:49 PM To: Azure/azure-sdk-for-go ***@***.***> Cc: Mention ***@***.***> Subject: Re: [Azure/azure-sdk-for-go] armresources.DeploymentsClient Validate always returns "the operation failed or was cancelled" not the actual error (Issue #24274) @JeffreyRichter<https://github.com/JeffreyRichter> for ARM LROs that are POST, is it acceptable that the terminal success includes a 400? I'm looking at the latest design guidelines for ARM<https://github.com/cloud-and-ai-microsoft/resource-provider-contract/blob/master/v1.0/async-api-reference.md#call-action-POST-asynchronously> and it only mentions 200/204. For context, the way this API works is as follows. * template is submitted to validate it's syntactically correct. this happens async * if the template is valid, the terminal success case returns a 200 with a populated DeploymentsClientValidateResponse * the DeploymentsClientValidateResponse.Error field is nil in this case * if the template is invalid, the terminal success case returns a 400 with the DeploymentsClientValidateResponse.Error field populated (the other fields are nil). Shouldn't this always return a 200 with the appropriate fields set? — Reply to this email directly, view it on GitHub<#24274 (comment)> or unsubscribe<https://github.com/notifications/unsubscribe-auth/AARLJP6Z3IEY4ZCGZ6MVTKL2VBIAVBFKMF2HI4TJMJ2XIZLTSSBKK5TBNR2WLJDUOJ2WLJDOMFWWLO3UNBZGKYLEL5YGC4TUNFRWS4DBNZ2F6YLDORUXM2LUPGBKK5TBNR2WLJDUOJ2WLJDOMFWWLLTXMF2GG2C7MFRXI2LWNF2HTAVFOZQWY5LFUVUXG43VMWSG4YLNMWVXI2DSMVQWIX3UPFYGLAVFOZQWY5LFVIZDAMBWG4YTSNJWGOSG4YLNMWUWQYLTL5WGCYTFNSWHG5LCNJSWG5C7OR4XAZNMJFZXG5LFINXW23LFNZ2KM5DPOBUWG44TQKSHI6LQMWVHEZLQN5ZWS5DPOJ42K5TBNR2WLKBSGI4DKNJSGQYIFJDUPFYGLJLJONZXKZNFOZQWY5LFVIZDSMRRGM4TEMJVG6BKI5DZOBS2K3DBMJSWZJLWMFWHKZNKGIYDANRXGE4TKNRTU52HE2LHM5SXFJTDOJSWC5DF>. You are receiving this email because you were mentioned. Triage notifications on the go with GitHub Mobile for iOS<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[matthchr]

FWIW, I tend to agree with the point @jhendrixMSFT is making - why not just have this API return a success error code with an error body in the success response if that's the experience they want.

With that said, one problem here is that as far as I can tell all the existing SDKs (and the Go track 1 SDK) allow this pattern, and the Track2 Go SDK doesn't. I actually found this when trying to move some old code using the Track1 Go SDK to Track2, but I can't actually complete that migration for this package due to this behavioral difference. It would be nice if we could get this API to be changed/fixed to make a bit more sense, but at the same time from a practical perspective:

1. That might take a while.
2. It's not clear to me that this is the only instance of this problem - are there other APIs where this pattern was used in ARM (even if we don't like it)? Are we tracking those down and driving fixes to them?