Add API version 2025-05-01 for dnsresolver #49921
Conversation
github-actions
bot
added
the
Mgmt
API Change CheckAPIView identified API level changes in this PR and created the following API reviews |
jamesvoongms
force-pushed
the
dns-resolver-ga-version-2025-05-01
branch
from
82e5412 to
e82004e
Compare
jamesvoongms
changed the title
|
Currently having issues with the virtual network resource in the generation: Autorest.md This seems to cause the generated examples to use a function that is only internal. eg. |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces API version 2025-05-01 for the dnsresolver service to support a new POST bulk API for large domain list usage, along with several supporting changes.
- Removed the BlockResponseCode property from DNS security rule actions in tests and API definitions.
- Updated the creation of DnsResolverDomainListData by moving from a constructor parameter to using the Domains collection.
- Changed the DefaultLocation in tests from AustraliaEast to WestUS2, and updated metadata in autorest.md, assets.json, and CHANGELOG.md accordingly.
Reviewed Changes
Copilot reviewed 81 out of 81 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| VirtualNetworkLinkTests.cs, OutboundEndpointTests.cs, InboundEndpointTests.cs, DnsForwardingRulesetTests.cs | Adjusted constructor comments for consistent formatting. |
| DnsSecurityRuleTests.cs | Updated tests to remove the BlockResponseCode property from action creation. |
| DnsResolverDomainListTests.cs | Modified domain list initialization to populate the Domains collection after construction; added tests for empty domain lists and bulk operations. |
| DnsResolverTestBase.cs | Changed DefaultLocation from AustraliaEast to WestUS2. |
| autorest.md, assets.json | Updated API version tags and related metadata. |
| API files (netstandard2.0.cs, net8.0.cs) | Removed the BlockResponseCode property and replaced action property with ActionType, as well as added bulk API endpoints. |
| CHANGELOG.md | Documented the new API version and bulk API feature. |
Comments suppressed due to low confidence (1)
sdk/dnsresolver/Azure.ResourceManager.DnsResolver/api/Azure.ResourceManager.DnsResolver.netstandard2.0.cs:883
- The BlockResponseCode property has been removed in favor of using ActionType only. Confirm that all client code and documentation have been updated accordingly to reflect this breaking change.
public Azure.ResourceManager.DnsResolver.Models.BlockResponseCode? BlockResponseCode { get { throw null; } set { } }
| @@ -58,7 +59,7 @@ public async Task CreateDnsSecurityRule() | |||
| // ARRANGE | |||
| var dnsSecurityRuleName = Recording.GenerateAssetName("dnsSecurityRule-"); | |||
| await CreateDnsResolverCollection(); | |||
There was a problem hiding this comment.
The removal of the BlockResponseCode property from the DnsSecurityRuleAction requires that both tests and documentation explicitly acknowledge this breaking change. Consider updating any relevant comments or documentation to clarify the new usage.
| await CreateDnsResolverCollection(); | |
| await CreateDnsResolverCollection(); | |
| // The BlockResponseCode property has been removed from DnsSecurityRuleAction. | |
| // The ActionType property should now be used to specify the desired action (e.g., Block). |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Added breaking change comment, customers can no longer specify the blockresponsecode, tests also reflect this.
|
@ArthurMa1978 , could you help confirm the compilation error seen is the same as Azure/autorest.csharp#5134 ? Edit: I have moved the bad generation to the customized folder and made the relevant function public instead of internal as a temporary fix. |
Moving down the dnsresolver sections Co-authored-by: Jesse Squire <[email protected]>
There was a problem hiding this comment.
CODEOWNERS updates look good. @ArthurMa1978's team will need to review the library implementation.
sdk/dnsresolver/Azure.ResourceManager.DnsResolver/src/Azure.ResourceManager.DnsResolver.csproj
Outdated
Show resolved
Hide resolved
| @@ -571,7 +574,7 @@ protected DnsSecurityRuleCollection() { } | |||
| public partial class DnsSecurityRuleData : Azure.ResourceManager.Models.TrackedResourceData, System.ClientModel.Primitives.IJsonModel<Azure.ResourceManager.DnsResolver.DnsSecurityRuleData>, System.ClientModel.Primitives.IPersistableModel<Azure.ResourceManager.DnsResolver.DnsSecurityRuleData> | |||
| { | |||
| public DnsSecurityRuleData(Azure.Core.AzureLocation location, int priority, Azure.ResourceManager.DnsResolver.Models.DnsSecurityRuleAction action, System.Collections.Generic.IEnumerable<Azure.ResourceManager.Resources.Models.WritableSubResource> dnsResolverDomainLists) { } | |||
| public Azure.ResourceManager.DnsResolver.Models.DnsSecurityRuleAction Action { get { throw null; } set { } } | |||
| public Azure.ResourceManager.DnsResolver.Models.DnsSecurityRuleActionType? ActionType { get { throw null; } set { } } | |||
There was a problem hiding this comment.
as far as I could see here, this property changes its name, and become an optional property.
This is a breaking change that is simple to mitigate therefore we do not need to bump the major version for such a minor change.
There was a problem hiding this comment.
so by offline sync, I see this change was caused by that we removed a property on the DnsSecurityRuleAction model.
And now this model only has one property which triggers the mechanism in the generator to "flatten" it. In the meantime, the generator will hide the original property.
Therefore we saw this change, which is actually: the previous DnsSecurityRuleAction Action property is changed to internal, and a new public property DnsSecurityRuleActionType? ActionType was added.
...solver/Azure.ResourceManager.DnsResolver/src/Customized/VirtualNetworkDnsResolverResource.cs
Outdated
Show resolved
Hide resolved
sdk/dnsresolver/Azure.ResourceManager.DnsResolver/src/autorest.md
Outdated
Show resolved
Hide resolved
...nsresolver/Azure.ResourceManager.DnsResolver/api/Azure.ResourceManager.DnsResolver.net8.0.cs
Outdated
Show resolved
Hide resolved
| @@ -895,7 +911,7 @@ protected virtual void JsonModelWriteCore(System.Text.Json.Utf8JsonWriter writer | |||
| public partial class DnsSecurityRulePatch : System.ClientModel.Primitives.IJsonModel<Azure.ResourceManager.DnsResolver.Models.DnsSecurityRulePatch>, System.ClientModel.Primitives.IPersistableModel<Azure.ResourceManager.DnsResolver.Models.DnsSecurityRulePatch> | |||
| { | |||
| public DnsSecurityRulePatch() { } | |||
| public Azure.ResourceManager.DnsResolver.Models.DnsSecurityRuleAction Action { get { throw null; } set { } } | |||
| public Azure.ResourceManager.DnsResolver.Models.DnsSecurityRuleActionType? ActionType { get { throw null; } set { } } | |||
There was a problem hiding this comment.
if this is triggering the breaking change, we just need to change it back on the SDK side.
There was a problem hiding this comment.
To resolve this, I think you need to do this following this step:
- in your autorest.md, find an empty place and add this:
mgmt-debug:
show-serialized-names: true
- regenerate the library.
- now you should be able to see a lot of debugging information showing up in the xml doc of every member and type, in this generated library, for this
ActionTypeinDnsSecurityRulePathclass, you should see in its xml doc, we have (for instance, you need to see its real value by actually running it)Serialized Name: XXX.YYY - put that
XXX.YYYinto therename-mappingsection inautorest.mdfile like this:
rename-mapping:
XXX.YYY: Action
- regenerate the library. Now you should see the property name has been changed by to
Action.
And we could do the same for the type DnsSecurityRuleActionType, by adding a similar entry to rename-mapping to change its name back to DnsSecurityRuleAction.
This also leaves the breaking change of this being changed to nullable - let us figure out the changes above and then I could show you how to do it
There was a problem hiding this comment.
This seems to cause issues with the the dotnet build, is it possible I need to make modifications to the existing rename-mappings instead?\
rename-mapping:
ProvisioningState: DnsResolverProvisioningState
ForwardingRule: DnsForwardingRule
ForwardingRuleState: DnsForwardingRuleState
ForwardingRule.properties.forwardingRuleState: DnsForwardingRuleState
ForwardingRulePatch.properties.forwardingRuleState: DnsForwardingRuleState
InboundEndpoint: DnsResolverInboundEndpoint
IpConfiguration: InboundEndpointIpConfiguration
IpAllocationMethod: InboundEndpointIPAllocationMethod
OutboundEndpoint: DnsResolverOutboundEndpoint
VirtualNetworkLink: DnsForwardingRulesetVirtualNetworkLink
ActionType: DnsSecurityRuleActionType
Action: DnsResolverDomainListBulkAction
Error when adding
DnsSecurityRuleAction.actionType: Action
PS C:\Git\jamesvoong-azure-sdk-for-net\sdk\dnsresolver\Azure.ResourceManager.DnsResolver> dotnet build /t:GenerateCode
Restore complete (0.9s)
Azure.ResourceManager.DnsResolver failed with 2 error(s) (10.8s)
EXEC : error | error : Plugin csharpgen reported failure.
Q:.tools.nuget\packages\microsoft.azure.autorest.csharp\3.0.0-beta.20250512.2\buildMultiTargeting\Microsoft.Azure.AutoRest.CSharp.targets(59,5): error MSB3073: The command "npx autorest@ --max-memory-size=8192 --skip-csproj --skip-upgrade-check --version=3.9.7 C:\Git\jamesvoong-azure-sdk-for-net\sdk\dnsresolver\Azure.ResourceManager.DnsResolver\src/autorest.md --use=Q:.tools.nuget\packages\microsoft.azure.autorest.csharp\3.0.0-beta.20250512.2\buildMultiTargeting../tools/net9.0/any/ --clear-output-folder=true --shared-source-folders="C:\Git\jamesvoong-azure-sdk-for-net\eng/../sdk/core/Azure.Core/src/Shared/;Q:.tools.nuget\packages\microsoft.azure.autorest.csharp\3.0.0-beta.20250512.2\buildMultiTargeting../content/Generator.Shared/" --output-folder=C:\Git\jamesvoong-azure-sdk-for-net\sdk\dnsresolver\Azure.ResourceManager.DnsResolver\src/Generated --namespace=Azure.ResourceManager.DnsResolver" exited with code 1.
| @@ -865,7 +882,6 @@ public partial class DnsSecurityRuleAction : System.ClientModel.Primitives.IJson | |||
| { | |||
| public DnsSecurityRuleAction() { } | |||
| public Azure.ResourceManager.DnsResolver.Models.DnsSecurityRuleActionType? ActionType { get { throw null; } set { } } | |||
| public Azure.ResourceManager.DnsResolver.Models.BlockResponseCode? BlockResponseCode { get { throw null; } set { } } | |||
There was a problem hiding this comment.
this is a breaking change.
In order to find a way to mitigate it, I need to know what happened to this property. Could you share some information about how this property changed in your spec?
We removed it in the latest api-version, I know this for sure.
Would the older existing api-versions still support this property? Or this property is a mistake from its first day.
The answer to these questions could change the way how we mitigate this.
There was a problem hiding this comment.
Yes the older existing api-versions will support this property.
For more details, this property essentially is ignored on the control plane side, whatever the customer provides in this field is ignored and defaulted to a different value, and so the intention is to remove it.
Even on the old API versions, the value is being ignored on the old api versions but we are allowing customers to provide the parameter on old API versions.
Ideally we would like to remove it on all API versions as it being ignored in all API versions in the control plane standpoint.
There was a problem hiding this comment.
If this is the case, we need to introduce this property back to mitigate the breaking change:
Please create a partial class in src/Customization or src/Custom directory for this class:
namespace Azure.ResourceManager.DnsResolver.Models
{
[CodeGenSerialization(nameof(BlockResponseCode), "blockResponseCode")]
public partial class DnsSecurityRuleAction
{
[EditorBrowsable(EditorBrowsableState.Never)]
public BlockResponseCode? BlockResponseCode { get; set; }
}
}
The EditorBrowsable attribute hides this property from the intellisence so that new users would not see it, and the CodeGenSerialization attribute lets the generator know about how to serialize/deserialize this property into/from the payload to make sure the generated SDK could work with previous API versions.
During this process, the type of this property BlockResponseCode may also be deleted in this update, please find the removed file from git history and move it to the src/Customization or src/Custom directory.
After you have introduced the above code, please run the dotnet build /t:GenerateCode command to regenerate everything.
There was a problem hiding this comment.
I have added the deleted blockresponsecode.cs back into customization and the dnsSecurityRuleACtion but these seems to cause issues in the generator
PS C:\Git\jamesvoong-azure-sdk-for-net\sdk\dnsresolver\Azure.ResourceManager.DnsResolver> dotnet build /t:GenerateCode
Restore complete (1.1s)
Azure.ResourceManager.DnsResolver failed with 2 error(s) (11.3s)
EXEC : error | error : Plugin csharpgen reported failure.
Q:.tools.nuget\packages\microsoft.azure.autorest.csharp\3.0.0-beta.20250512.2\buildMultiTargeting\Microsoft.Azure.AutoRest.CSharp.targets(59,5): error MSB3073: The command "npx autorest@ --max-memory-size=8192 --skip-csproj --skip-upgrade-check --version=3.9.7 C:\Git\jamesvoong-azure-sdk-for-net\sdk\dnsresolver\Azure.ResourceManager.DnsResolver\src/autorest.md --use=Q:.tools.nuget\packages\microsoft.azure.autorest.csharp\3.0.0-beta.20250512.2\buildMultiTargeting../tools/net9.0/any/ --clear-output-folder=true --shared-source-folders="C:\Git\jamesvoong-azure-sdk-for-net\eng/../sdk/core/Azure.Core/src/Shared/;Q:.tools.nuget\packages\microsoft.azure.autorest.csharp\3.0.0-beta.20250512.2\buildMultiTargeting../content/Generator.Shared/" --output-folder=C:\Git\jamesvoong-azure-sdk-for-net\sdk\dnsresolver\Azure.ResourceManager.DnsResolver\src/Generated --namespace=Azure.ResourceManager.DnsResolver" exited with code 1.
Build failed with 2 error(s) in 12.7s
Workload updates are available. Run dotnet workload list for more information.
Contributing to the Azure SDK
Please see our CONTRIBUTING.md if you are not familiar with contributing to this repository or have questions.
For specific information about pull request etiquette and best practices, see this section.
This is current draft PR for the API changes in dns-resolver-ga-version-2025-05-01
Azure/azure-rest-api-specs#34457
The PR adds upon the public preview with a new POST request for the domain list resource. Also smaller changes such as removing required properties and the blockresponsecode of the DNS security rule.
Release Plan: (https://apps.powerapps.com/play/e/ed2ffefd-774d-40dd-ab23-7fff01aeec9f/a/821ab569-ae60-420d-8264-d7b5d5ef734c?tenantId=72f988bf-86f1-41af-91ab-2d7cd011db47)