Add App Service delegated subnet support - #18738
Conversation
Configure App Service sites, slots, and the dashboard for regional VNet integration. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 25c353af-7555-4b46-ac6f-8e78f3cfeea1
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 18738Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 18738" |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Adds delegated-subnet support to Azure App Service deployment environments, including upgrade-safe VNet integration.
Changes:
- Propagates delegated subnets to websites, slots, and the dashboard.
- Emits an upgrade-safe
networkConfigchild resource. - Adds documentation, snapshots, unit tests, and Azure E2E coverage.
Show a summary per file
| File | Description |
|---|---|
tests/...CanOverrideEnvironmentDelegatedSubnet.verified.bicep |
Verifies per-site subnet overrides. |
tests/...AddAppServiceWithDelegatedSubnet.verified.bicep |
Captures complete generated infrastructure. |
AzureVirtualNetworkExtensionsTests.cs |
Tests App Service delegation annotations. |
AzureAppServiceTests.cs |
Tests Bicep generation and overrides. |
Aspire.Deployment.EndToEnd.Tests.csproj |
Extends deployment-test timeouts. |
AppServiceReactDeploymentTests.cs |
Tests live VNet upgrade behavior. |
AzureVirtualNetworkExtensions.cs |
Documents App Service delegation. |
README.md |
Documents regional VNet integration. |
AzureAppServiceWebsiteContext.cs |
Applies subnet configuration to sites and slots. |
AzureAppServiceEnvironmentResource.cs |
Adds delegated-subnet capability. |
AzureAppServiceEnvironmentExtensions.cs |
Configures the dashboard subnet. |
AspireSiteContainer.cs |
Adds the named networkConfig resource workaround. |
Review details
- Files reviewed: 12/12 changed files
- Comments generated: 3
- Review effort level: Medium
PR Testing ReportPR Information
Artifact Version Verification
Scenarios ExecutedFresh AppHost from the PR artifactStatus: Passed
Generated Azure artifactsStatus: Passed
Experimental API diagnostic and recoveryStatus: Passed
Local AppHost lifecycleStatus: Passed Started the fresh AppHost with the PR CLI in isolated mode, waited for the web resource to reach Summary
Overall ResultPR artifact verified. This report contains no Azure account, tenant, subscription, resource-group, hostname, credential, token, local-path, or raw-log information. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
…nner The two-phase App Service VNet test runs two consecutive 'aspire deploy' commands in one terminal session. WaitForPipelineSuccessAsync matches the 'pipeline succeeded' banner anywhere on the visible screen, so the second deploy's wait matched the first deploy's leftover banner immediately and returned without waiting for the redeploy. The subsequent success-prompt wait then had to absorb the entire second deployment and timed out. Clear the terminal before the second deploy, mirroring the existing pattern in CliTelemetryTests, so the pipeline wait only observes the redeploy's own completion banner. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Pushed a fix for the What failed: In the Root cause (test sync, not a product bug): The two-phase test runs two consecutive Fix: Clear the terminal before the second deploy (mirroring the existing Worth re-running |
|
/deployment-test |
|
🚀 Deployment tests starting on PR #18738... This will deploy to real Azure infrastructure. Results will be posted here when complete. |
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Tests selector (audit mode)The full test matrix and all jobs still run in audit mode. The tests and jobs below are what selective CI would run under enforcement. 5 / 100 test projects · 4 jobs, from 15 changed files. Selected test projects (5 / 100)
Selected jobs (4)
How these were chosen — grouped by what changed🔧 🔧 🧪 🔧 🔧 🔧 🧪 🧪 🧪 🧪 🧪 🧪 🧪 Job reasons
Selection computed for commit |
Mitch Denny (mitchdenny)
left a comment
There was a problem hiding this comment.
Approving after a full review pass.
Reviewed the shared Aspire.Hosting.Azure.Network change closely since it's a common library: WithDelegatedSubnet<T> is generalized (not App-Service-specific) and hardened with idempotency + conflict guards (one subnet per resource, one delegation service per subnet) using ResourceAnnotationMutationBehavior.Replace, plus case-insensitive delegation comparison. No App Service leakage into the shared API.
App Service side: environment implements IAzureDelegatedSubnetResource (Microsoft.Web/serverFarms), and the subnet is applied to the dashboard site, each website, and its slot. The @onlyIfNotExists() + Microsoft.Web/sites/networkConfig (virtualNetwork) interplay is the highest-risk area but is correctly reasoned, well-commented, and verified on the production-upgrade path. The AspireSiteNetworkConfig BCP035 workaround is properly link-tracked (Azure/azure-sdk-for-net#54629).
README correctly documents that regional VNet integration is outbound-only. Both App Service deployment E2E tests (React two-phase upgrade + storage private-endpoint connectivity) passed together against real Azure on current HEAD.
Two follow-ups are already captured: the throw-on-re-delegation API-design consideration (author confirmed intentional) and the pre-existing App Service site-name tail-truncation weakness (#18750, not caused by this PR).
This comment has been minimized.
This comment has been minimized.
|
The CI build failed due to test failure(s) that appear unrelated to the PR changes. These may be flaky tests. Suspected flaky test(s):
Suggested actions:
You can re-run the failed jobs from the workflow run page. |
|
📝 Documentation has been drafted in microsoft/aspire.dev#1441 targeting Drafted a docs PR updating Note This draft PR needs human review before merging. |
Description
Adds regional virtual network integration for Azure App Service environments, addressing the App Service configuration gap reported in discussion #18734. AppHost authors can now apply an Azure delegated subnet to an App Service environment, and Aspire configures workload sites, deployment slots, and the default dashboard consistently.
User-facing usage
The subnet is delegated to
Microsoft.Web/serverFarms. This is regional VNet integration for outbound traffic only: it does not make site or dashboard ingress private and does not enable Route All.Implementation details
Slot-enabled deployments preserve the production
Microsoft.Web/sitesresource with@onlyIfNotExists(). The change emits the requiredMicrosoft.Web/sites/networkConfigchild namedvirtualNetworkso an existing production site can be upgraded safely without bypassing that guard. A narrow custom Azure.Provisioning resource is used until the upstream library can generate the required fixed child resource name.Validation
dotnet test --project tests/Aspire.Hosting.Azure.Tests/Aspire.Hosting.Azure.Tests.csproj --no-launch-profile -- --filter-class "*.AzureAppServiceTests" --filter-class "*.AzureVirtualNetworkExtensionsTests" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"(84 passed)dotnet test --project tests/Aspire.Hosting.CodeGeneration.TypeScript.Tests/Aspire.Hosting.CodeGeneration.TypeScript.Tests.csproj --no-launch-profile -- --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"(99 passed).withDelegatedSubnet(subnet)call.N/A - prompted by discussion #18734.
Checklist
<remarks />and<code />elements on your triple slash comments?