Fix: Skip resources targeted to different compute environments#14177
Fix: Skip resources targeted to different compute environments#14177
Conversation
Co-authored-by: eerhardt <8291187+eerhardt@users.noreply.github.com>
Co-authored-by: eerhardt <8291187+eerhardt@users.noreply.github.com>
Co-authored-by: eerhardt <8291187+eerhardt@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 14177Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 14177" |
There was a problem hiding this comment.
Pull request overview
This PR fixes an issue where multiple compute environments (e.g., Azure Container Apps + AppService) would incorrectly process all resources regardless of WithComputeEnvironment() targeting. Previously, ACA would validate resources intended for AppService, causing errors due to port validation mismatches. The fix adds filtering logic to skip resources explicitly targeted to different compute environments.
Changes:
- Added a public
GetComputeEnvironment()helper method inResourceExtensions.csto retrieve the compute environment a resource is bound to - Added filtering logic in all four infrastructure implementations (ACA, AppService, DockerCompose, Kubernetes) to skip resources with
ComputeEnvironmentAnnotationtargeting different environments - Added comprehensive test coverage across three test projects validating the multi-environment resource targeting behavior
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/Aspire.Hosting/ApplicationModel/ResourceExtensions.cs |
Added public GetComputeEnvironment() extension method to retrieve the compute environment bound to a resource |
src/Aspire.Hosting.Azure.AppContainers/AzureContainerAppsInfrastructure.cs |
Added filtering to skip resources targeted to different compute environments before processing |
src/Aspire.Hosting.Azure.AppService/AzureAppServiceInfrastructure.cs |
Added filtering to skip resources targeted to different compute environments before processing |
src/Aspire.Hosting.Docker/DockerComposeInfrastructure.cs |
Added filtering to skip resources targeted to different compute environments before processing |
src/Aspire.Hosting.Kubernetes/KubernetesInfrastructure.cs |
Added filtering to skip resources targeted to different compute environments before processing |
tests/Aspire.Hosting.Azure.Tests/AzureContainerAppsTests.cs |
Added test validating ACA and AppService multi-environment targeting with port validation scenario |
tests/Aspire.Hosting.Docker.Tests/DockerComposeTests.cs |
Added test validating DockerCompose and Kubernetes multi-environment targeting |
tests/Aspire.Hosting.Docker.Tests/Aspire.Hosting.Docker.Tests.csproj |
Added project reference to Kubernetes for test scenarios |
tests/Aspire.Hosting.Kubernetes.Tests/KubernetesEnvironmentResourceTests.cs |
Added test validating Kubernetes and DockerCompose multi-environment targeting |
tests/Aspire.Hosting.Kubernetes.Tests/Aspire.Hosting.Kubernetes.Tests.csproj |
Added project references to Docker and TestUtilities for test scenarios |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
PR Testing ReportCLI Version Verification
Test Results
Multi-Compute Environment Publish TestCreated an AppHost with two compute environments (ACA + App Service) and resources targeted to each: var aca = builder.AddAzureContainerAppEnvironment("aca");
var appService = builder.AddAzureAppServiceEnvironment("appservice");
var apiService = builder.AddProject<Projects.MultiEnv_ApiService>("apiservice")
.WithComputeEnvironment(aca);
builder.AddProject<Projects.MultiEnv_Web>("webfrontend")
.WithComputeEnvironment(appService);Result: �spire publish succeeded with all 6 steps passing:
Note on Cross-Environment ReferencesWhen testing with .WithReference(apiservice) on a resource in a different compute environment, the publish failed with:
This appears to be expected behavior / a separate issue - cross-environment resource references aren't supported. The PR's fix for filtering resources by compute environment is working correctly. Tested with CLI from PR artifacts on Windows |
Updated PR Testing ReportCLI Version Verification
Exact Scenario from PR DescriptionTested the exact scenario that was failing before this fix: var aca = builder.AddAzureContainerAppEnvironment("aca");
var appService = builder.AddAzureAppServiceEnvironment("appservice");
builder.AddProject<Projects.ExactTest_Web>("webappaca")
.WithExternalHttpEndpoints()
.WithComputeEnvironment(aca);
// Container with custom port targeted to AppService
// Previously: ACA would error "endpoint 'http' is an http endpoint and must use port 80"
// Now: ACA should skip it entirely
builder.AddRedis("redis")
.WithHttpEndpoint(port: 8123, targetPort: 8123, name: "http")
.WithExternalHttpEndpoints()
.WithComputeEnvironment(appService);Result: ✅ PASSEDKey finding: The Redis container with custom port 8123 (targeted to AppService) was correctly skipped by ACA infrastructure - no port 80 validation error occurred. The fix is working as intended. Tested with CLI from PR artifacts on Windows |
🎬 CLI E2E Test RecordingsThe following terminal recordings are available for commit
📹 Recordings uploaded automatically from CI run #21455482372 |
Description
When using multiple compute environments (e.g., ACA + AppService), all environments processed all resources regardless of
WithComputeEnvironment()targeting. This caused ACA to reject resources intended for AppService due to port validation mismatches, and similar cross-environment validation failures.Example scenario that now works:
Changes:
AzureContainerAppsInfrastructure,AzureAppServiceInfrastructure,DockerComposeInfrastructure, andKubernetesInfrastructureto skip resources withComputeEnvironmentAnnotationtargeting a different environmentComputeEnvironmentAnnotationto infrastructure projects viaInternalsVisibleToResourceNameComparer,DashboardConfigNames, etc.) from infrastructure projects now using internal types directlyChecklist
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.