Make app provisioning wait for role assignments and private endpoints#14483
Make app provisioning wait for role assignments and private endpoints#14483eerhardt merged 3 commits intorelease/13.2from
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 14483Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 14483" |
🎬 CLI E2E Test RecordingsThe following terminal recordings are available for commit
📹 Recordings uploaded automatically from CI run #22641869722 |
There was a problem hiding this comment.
Pull request overview
Ensures Azure app provisioning (Container Apps / App Service Web Sites) waits for role-assignment resources to be provisioned first, preventing startup connectivity failures caused by role assignments not being ready.
Changes:
- Introduces
ComputedRoleAssignmentsAnnotationand attaches it during Azure resource preparation after role assignment resources are created. - Updates Azure Container App and App Service Web Site provisioning steps to depend on the computed role-assignment resources (instead of name-based matching).
- Updates deployment snapshot tests to reflect the corrected dependency ordering.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Hosting.Azure.Tests/Snapshots/AzureDeployerTests.DeployAsync_WithRedisAccessKeyAuthentication_CreatesCorrectDependencies.verified.txt | Snapshot updated to show website provisioning depends on role-assignment steps. |
| tests/Aspire.Hosting.Azure.Tests/Snapshots/AzureDeployerTests.DeployAsync_WithAzureResourceDependencies_DoesNotHang_step=diagnostics.verified.txt | Snapshot updated to reflect dependency ordering in diagnostics output. |
| src/Aspire.Hosting.Azure/ComputedRoleAssignmentsAnnotation.cs | Adds public annotation to surface computed role assignment resources to downstream pipeline configuration. |
| src/Aspire.Hosting.Azure/AzureResourcePreparer.cs | Attaches the computed role assignment annotation after creating role-assignment resources in publish mode. |
| src/Aspire.Hosting.Azure.AppService/AzureAppServiceWebSiteResource.cs | Makes website provisioning depend on computed role-assignment provisioning steps. |
| src/Aspire.Hosting.Azure.AppContainers/AzureContainerAppResource.cs | Makes container app provisioning depend on computed role-assignment provisioning steps. |
src/Aspire.Hosting.Azure.AppService/AzureAppServiceWebSiteResource.cs
Outdated
Show resolved
Hide resolved
src/Aspire.Hosting.Azure.AppContainers/AzureContainerAppResource.cs
Outdated
Show resolved
Hide resolved
src/Aspire.Hosting.Azure.AppContainers/AzureContainerAppResource.cs
Outdated
Show resolved
Hide resolved
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
e2776ab to
6f16e97
Compare
|
@davidfowl - I believe this is ready for review now. |
| @@ -29,26 +29,26 @@ Steps with no dependencies run first, followed by steps that depend on them. | |||
| 13. login-to-acr-aca-env-acr | |||
| 14. push-prereq | |||
| 15. push-api-service | |||
| 16. update-api-service-provisionable-resource | |||
There was a problem hiding this comment.
FYI - this test is currently broken (quarantined). This baseline change wasn't caused by my changes, but I'm putting it here so the test passes.
| @@ -124,7 +124,6 @@ public async Task DeployAsync_PromptsViaInteractionService() | |||
| /// the containers and does not attempt to push them. | |||
| /// </summary> | |||
| [Fact] | |||
| [RequiresTools(["az"])] // Requires Azure CLI to compile Bicep templates | |||
There was a problem hiding this comment.
I changed these tests to use a the ProvisioningTestHelpers.CreateBicepCompiler(), which mocks out compiling bicep. It speeds the tests up tremendously.
| @@ -569,7 +565,7 @@ public async Task DeployAsync_WithUnresolvedParameters_PromptsForParameterValues | |||
| ConfigureTestServices(builder, interactionService: testInteractionService, bicepProvisioner: new NoOpBicepProvisioner()); | |||
|
|
|||
| // Add a parameter that will be unresolved | |||
| var param = builder.AddParameter("test-param"); | |||
| var param = builder.AddParameter("unresolved-test-param"); | |||
There was a problem hiding this comment.
This is necessary because there is another test using this same parameter name, and saving a value into the deployment state file. Having a value in the deployment state file causes this test to hang.
The easy fix was to use a different parameter name.
src/Aspire.Hosting.Azure.AppContainers/ContainerAppEnvironmentContext.cs
Show resolved
Hide resolved
…#14483) * Make app provisioning wait for role assignments and private endpoints * Update code to use DeploymentPrerequisitesAnnotation * Add private endpoints to tests --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
Description
Container apps and web sites can be provisioned before their role assignments and private endpoints are ready, causing connectivity failures on startup. The pipeline ordering didn't enforce that prerequisite infrastructure steps (role assignments, private endpoints) complete before app provisioning steps like
provision-{app}-containerapp/provision-{app}-website.Before:
provision-api-websiteruns concurrently withprovision-api-roles-kvAfter:
provision-api-websitedepends onprovision-api-roles-kvand any private endpointsApproach
ComputedDeploymentPrerequisitesAnnotation(public) thatAzureResourcePreparerattaches to a compute resource after discovering its deployment prerequisites — both role assignment resources and private endpoint resourcesPrivateEndpointResourceAnnotation(internal) to link Azure resources to their private endpoint resources.AddPrivateEndpoint()annotates the target's root Azure resource, enablingAzureResourcePreparerto discover private endpoints without cross-project type referencesAzureContainerAppResourceandAzureAppServiceWebSiteResourcenow queryComputedDeploymentPrerequisitesAnnotationon the target resource to set up pipeline dependenciesChecklist
<remarks />and<code />elements on your triple slash comments?💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.