API review fixes for 13.4 (PR #17700) - #17706
Conversation
Addresses several issues found during API surface review:
1. Rename NetworkID -> NetworkId (and networkID -> networkId) on
AllocatedEndpoint, EndpointAnnotation, EndpointReference,
EndpointReferenceAnnotation, NetworkEndpointSnapshot,
NetworkEndpointSnapshotList, and related methods/parameters.
2. Add [Experimental("ASPIREAZURE003")] to AzureRoleAssignmentResource.
3. Change EndpointReferenceAnnotation.EndpointNames from HashSet<string>
to ISet<string> (backing field stays HashSet).
4. Add 'sealed' to new public resource classes that are not subclassed
in the repo: KubernetesHelmChartResource, BlazorWasmAppResource,
BunAppResource, NextJsAppResource, ViteAppResource,
AzureNatGatewayResource, AzureNetworkSecurityGroupResource,
AzureNetworkSecurityPerimeterResource, AzurePrivateEndpointResource,
AzurePublicIPAddressResource, AzureSubnetResource,
AzureVirtualNetworkResource. (GoAppResource and NodeAppResource left
non-sealed because they are used as generic type constraints in the
same assembly.)
5. Disambiguate WithHiddenOnCompletion overloads by removing the
'= 0' default from the int overload, so calls with no argument
resolve to the params overload.
api/*.cs and api/*.ats.txt are intentionally not updated here - the API
surface PR will regenerate them.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 17706Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 17706" |
There was a problem hiding this comment.
Pull request overview
This PR applies API-review cleanup for the 13.4 surface by aligning naming with .NET/Aspire conventions, marking the role-assignment resource experimental, narrowing exposed collection types, sealing new public resource classes, and removing an ambiguous default parameter from WithHiddenOnCompletion.
Changes:
- Renames
NetworkID/networkIDAPIs and call sites toNetworkId/networkId. - Adds experimental annotation/suppressions for
AzureRoleAssignmentResource. - Seals new public resource types and adjusts
WithHiddenOnCompletionoverload resolution.
A non-blocking spelling nit was stored on ResourceExtensions.cs.
Show a summary per file
| File | Description |
|---|---|
src/Shared/StringComparers.cs |
Renames network comparer members to NetworkId. |
src/Aspire.Hosting/ResourceBuilderExtensions.cs |
Updates network parameter/member names and WithHiddenOnCompletion overload docs/signature. |
src/Aspire.Hosting/Orchestrator/ApplicationOrchestrator.cs |
Uses renamed endpoint default network property. |
src/Aspire.Hosting/Dcp/DcpModelUtilities.cs |
Uses renamed allocated endpoint network members/locals. |
src/Aspire.Hosting/Dcp/DcpExecutor.cs |
Uses renamed endpoint default network property. |
src/Aspire.Hosting/ApplicationModel/ResourceUrlsCallbackContext.cs |
Renames context network parameter. |
src/Aspire.Hosting/ApplicationModel/ResourceExtensions.cs |
Renames context network parameters in endpoint helpers. |
src/Aspire.Hosting/ApplicationModel/HostUrl.cs |
Uses renamed endpoint default network property. |
src/Aspire.Hosting/ApplicationModel/EndpointReferenceAnnotation.cs |
Changes EndpointNames to ISet<string> and renames context network property. |
src/Aspire.Hosting/ApplicationModel/EndpointReference.cs |
Renames context network field/property/parameters. |
src/Aspire.Hosting/ApplicationModel/EndpointAnnotation.cs |
Renames default network members and snapshot record field. |
src/Aspire.Hosting/ApplicationModel/AllocatedEndpoint.cs |
Renames constructor parameter/property to NetworkId. |
src/Aspire.Hosting.Kubernetes/KubernetesHelmChartResource.cs |
Seals the Helm chart resource type. |
src/Aspire.Hosting.JavaScript/ViteAppResource.cs |
Seals the Vite resource type. |
src/Aspire.Hosting.JavaScript/NextJsAppResource.cs |
Seals the Next.js resource type. |
src/Aspire.Hosting.JavaScript/BunAppResource.cs |
Seals the Bun resource type. |
src/Aspire.Hosting.Blazor/Resources/BlazorWasmAppResource.cs |
Seals the Blazor WASM resource type. |
src/Aspire.Hosting.Azure/RoleAssignmentResourceAnnotation.cs |
Adds suppression for experimental role assignment usage. |
src/Aspire.Hosting.Azure/AzureRoleAssignmentResource.cs |
Marks the role assignment resource experimental. |
src/Aspire.Hosting.Azure.Network/AzureVirtualNetworkResource.cs |
Seals the virtual network resource type. |
src/Aspire.Hosting.Azure.Network/AzureSubnetResource.cs |
Seals the subnet resource type. |
src/Aspire.Hosting.Azure.Network/AzurePublicIPAddressResource.cs |
Seals the public IP resource type. |
src/Aspire.Hosting.Azure.Network/AzurePrivateEndpointResource.cs |
Seals the private endpoint resource type. |
src/Aspire.Hosting.Azure.Network/AzureNetworkSecurityPerimeterResource.cs |
Seals the NSP resource type. |
src/Aspire.Hosting.Azure.Network/AzureNetworkSecurityGroupResource.cs |
Seals the NSG resource type. |
src/Aspire.Hosting.Azure.Network/AzureNatGatewayResource.cs |
Seals the NAT gateway resource type. |
tests/Aspire.Hosting.Tests/WithEnvironmentTests.cs |
Updates named argument usage to networkId. |
tests/Aspire.Hosting.Tests/EndpointReferenceTests.cs |
Updates named argument usage to networkId. |
tests/Aspire.Hosting.Tests/Dcp/DcpExecutorTests.cs |
Updates assertions to use NetworkId. |
tests/Aspire.Hosting.Redis.Tests/AddRedisTests.cs |
Updates named argument usage to networkId. |
tests/Aspire.Hosting.Qdrant.Tests/AddQdrantTests.cs |
Updates named argument usage to networkId. |
tests/Aspire.Hosting.PostgreSQL.Tests/PostgresMcpBuilderTests.cs |
Updates named argument usage to networkId. |
tests/Aspire.Hosting.PostgreSQL.Tests/AddPostgresTests.cs |
Updates named argument usage to networkId. |
tests/Aspire.Hosting.Milvus.Tests/AddMilvusTests.cs |
Updates named argument usage to networkId. |
tests/Aspire.Hosting.JavaScript.Tests/AddNodeAppTests.cs |
Updates named argument usage to networkId. |
tests/Aspire.Hosting.Azure.Tests/RoleAssignmentTests.cs |
Suppresses experimental role assignment diagnostics in tests. |
tests/Aspire.Hosting.Azure.Tests/AzureUserAssignedIdentityTests.cs |
Suppresses experimental role assignment diagnostics in tests. |
tests/Aspire.Hosting.Azure.Tests/AzurePostgresExtensionsTests.cs |
Updates named argument usage to networkId. |
Copilot's findings
- Files reviewed: 38/38 changed files
- Comments generated: 1
| /// </summary> | ||
| /// <param name="resource">The <see cref="IResourceWithEndpoints"/> which contains <see cref="EndpointAnnotation"/> annotations.</param> | ||
| /// <param name="contextNetworkID">The ID of the network that serves as the context context for the endpoint references.</param> | ||
| /// <param name="contextNetworkId">The ID of the network that serves as the context context for the endpoint references.</param> |
Reverts the subset of API changes from the previous commit that would be
binary-breaking against 13.3.5, and bumps PackageValidationBaselineVersion
from 13.2.2 to 13.3.5 so pack validation runs against the latest shipped
release.
Reverts (binary-breaking against 13.3.5):
- AllocatedEndpoint.NetworkID (kept ctor param 'networkId' - not breaking)
- EndpointAnnotation.DefaultNetworkID (kept ctor param 'networkId')
- EndpointReference.ContextNetworkID (kept ctor param 'contextNetworkId')
- NetworkEndpointSnapshot.NetworkID record positional param
- Removed 'sealed' from 9 shipped resource classes:
- NextJsAppResource, ViteAppResource
- AzureNatGatewayResource, AzureNetworkSecurityGroupResource,
AzureNetworkSecurityPerimeterResource, AzurePrivateEndpointResource,
AzurePublicIPAddressResource, AzureSubnetResource, AzureVirtualNetworkResource
Kept (not binary-breaking):
- All constructor/method parameter renames (networkID->networkId, etc.)
- EndpointReferenceAnnotation.ContextNetworkId (new in 13.4)
- EndpointNames type change (HashSet -> ISet)
- WithHiddenOnCompletion overload disambiguation
- [Experimental("ASPIREAZURE003")] on AzureRoleAssignmentResource
- 'sealed' on KubernetesHelmChartResource, BlazorWasmAppResource, BunAppResource (new in 13.4)
Package validation:
- Bumped PackageValidationBaselineVersion 13.2.2 -> 13.3.5
- Regenerated CompatibilitySuppressions.xml in 4 projects: most legacy
entries against 13.2.2 are no longer needed because those APIs already
shipped in 13.3.x. The remaining suppression is PublishAsNpmScript in
Aspire.Hosting.JavaScript (documented removal from PR #17382).
|
❓ CLI E2E Tests unknown — 110 passed, 0 failed, 2 unknown (commit View all recordings
📹 Recordings uploaded automatically from CI run #26674466350 |
|
Validated that the failure here is unrelated to the changes. Merging. |
da473d2
into
release/13.4
AzureRoleAssignmentResource is now marked [Experimental("ASPIREAZURE003")]
as part of the 13.4 API review fixes (microsoft/aspire#17706). Update the
diagnostic page to list all APIs that trigger ASPIREAZURE003 and add an
experimental caution to the role-assignments programmatic inspection section.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Pull request created: #1140
|
|
📝 Documentation has been drafted in microsoft/aspire.dev#1140 targeting Updated docs for two triggered signals:
Files changed: Note This draft PR needs human review before merging. |
Addresses API design feedback from PR #17700 review while preserving the public API that has already shipped.
Changes
Network identifier naming
Aspire convention is
Id(one word), so new/implementation-facing members useNetworkId/ContextNetworkIdwhere possible. The shipped public members are kept for compatibility with the 13.x baseline:AllocatedEndpoint.NetworkIDEndpointAnnotation.DefaultNetworkIDEndpointReference.ContextNetworkIDNetworkEndpointSnapshot.NetworkIDConstructor and method parameter renames such as
networkID->networkIdare kept because they do not affect binary compatibility.Azure role assignment experimental marker
AzureRoleAssignmentResourceis marked[Experimental("ASPIREAZURE003")], reusing the existing diagnostic. Suppressions were added where existing code references the type.EndpointReferenceAnnotation endpoint names
EndpointReferenceAnnotation.EndpointNamesusesISet<string>on the public surface while keeping aHashSet<string>backing field with the case-insensitive comparer.Sealed resource classes
Classes that are new in 13.4 and not used as generic constraints remain sealed, including:
KubernetesHelmChartResourceBlazorWasmAppResourceBunAppResourceClasses that already shipped unsealed remain unsealed to avoid APICompat CP0009 breaks. This includes the resource types reported by the Build packages check:
ViteAppResourceAzureNatGatewayResourceAzureNetworkSecurityGroupResourceAzurePrivateEndpointResourceAzurePublicIPAddressResourceAzureSubnetResourceAzureVirtualNetworkResourceGoAppResourceandNodeAppResourcealso remain unsealed because C# does not allow sealed types as generic constraints.WithHiddenOnCompletion overload disambiguation
The
= 0default was removed fromWithHiddenOnCompletion<T>(builder, int exitCode)so calls bind unambiguously:WithHiddenOnCompletion(builder)->params int[]overloadWithHiddenOnCompletion(builder, 5)->intoverloadWithHiddenOnCompletion(builder, 3, 7)->params int[]overloadAPI compatibility
The Build packages check reported breaking changes from removed members and newly sealed shipped classes. This PR now undoes those check-reported breaks rather than reverting unrelated changes.
The targeted compatibility fixes are:
NetworkID,DefaultNetworkID,ContextNetworkID, andNetworkEndpointSnapshot.NetworkIDpublic members.sealedfrom the shipped JavaScript/Azure.Network resource classes listed above.api/*.cs surface files
Intentionally not updated in this PR. Those files are regenerated as part of the API surface update process.
Validation
dotnet build src\Aspire.Hosting\Aspire.Hosting.csproj /p:SkipNativeBuild=truedotnet build src\Aspire.Hosting.JavaScript\Aspire.Hosting.JavaScript.csproj /p:SkipNativeBuild=truedotnet build src\Aspire.Hosting.Azure.Network\Aspire.Hosting.Azure.Network.csproj /p:SkipNativeBuild=truedotnet packfor the same three projects