Azure / azure-libraries-for-net

Azure libraries for .Net
MIT License
377 stars 190 forks source link

[BUG] Regex pattern error in model CsmMoveResourceEnvelope. #1293

Closed PeteCrayne closed 2 years ago

PeteCrayne commented 2 years ago

Describe the bug The model for CsmMoveResourceEnvelope seems to contain a bad regular expression. The first character is a space, followed by the start-of-line character. https://github.com/Azure/azure-libraries-for-net/blob/master/src/ResourceManagement/AppService/Generated/Models/CsmMoveResourceEnvelope.cs Lines 74, 76

I believe this expression cannot be matched, and the function will always throw an exception. https://regex101.com/r/qeb5z3/1

Exception or Stack Trace "'TargetResourceGroup' does not match expected pattern ' ^[-\w._()]+[^.]$'."

To Reproduce Attempt to call "MoveAsync"

Code Snippet

{
    using (WebSiteManagementClient client = new WebSiteManagementClient(this.RestCredentials) { SubscriptionId = this.SubscriptionID })
    {
        var moveResources = new CsmMoveResourceEnvelope() { Resources = resourceList, TargetResourceGroup = targetResourceGroup };
        await client.MoveAsync(resourceGroupName: sourceResourceGroup, moveResourceEnvelope: moveResources);
    }
}

Setup (please complete the following information):

Additional context I believe that the error originates in the swagger definition: https://github.com/Azure/azure-rest-api-specs/blob/main/specification/web/resource-manager/Microsoft.Web/stable/2021-03-01/ResourceProvider.json Line 785

weidongxu-microsoft commented 2 years ago

Code is generated from openapi spec here https://github.com/Azure/azure-rest-api-specs/blob/main/specification/web/resource-manager/Microsoft.Web/stable/2021-03-01/ResourceProvider.json#L782-L787

Maybe this is wrong?

PS: please consider migration to new SDK here https://aka.ms/azsdk/dotnet/mgmt

PS2: lib "Microsoft.Azure.Management.Websites" is in this repo https://github.com/Azure/azure-sdk-for-net

PeteCrayne commented 2 years ago

After further testing, I see that there is more wrong with the MoveAsync function than just the validation on the CsmMoveResourceEnvelope model. In my local project, I made a new class file derived from CsmMoveResourceEnvelope so that I could change the validator. After removing the extra space, Azure returned a "BadRequest", and the response said that it was expecting a full Resource Identifier. So I tried disabling the RegEx validator entirely and passed in a full ResourceID.

The function returned an exception: "Operation returned an invalid status code 'Accepted'". It throws an exception for any response code other than 204 (No Content). As you would expect, Azure did actually move the resource.

It seems that the "MoveAsync" function in Microsoft.Azure.Management.Websites 3.1.2 is out-of-date. Maybe the space was added to the spec intentionally to disable the function? (Just a guess)

In any case I'll try a different approach. Thanks.

weidongxu-microsoft commented 2 years ago

@PeteCrayne

SDK is actually not out-of-date. The spec is wrong and hence any SDK that derived from it is also wrong.

If you got 201, the whole spec around moveResource om Microsoft.Web is wrong. The spec says it must be 204 https://github.com/Azure/azure-rest-api-specs/blob/main/specification/web/resource-manager/Microsoft.Web/stable/2021-03-01/ResourceProvider.json#L573-L575 And looks like this to be a LRO (as 201).

If you only need to move resource, maybe you can use Microsoft.Azure.Management.Resources (or better the new Azure.ResourceManager.Resources). This is a generic REST API that actually not belong to the Microsoft.Web lib.