aspire icon indicating copy to clipboard operation
aspire copied to clipboard

Fix Azure Container App Environment to respect AsExisting and WithComputeEnvironment

Open TheEagleByte opened this issue 2 months ago • 2 comments

Summary

Fixes two critical bugs preventing Azure Container App deployments from using existing Container App Environments, causing permission failures when the deployment attempted to create new infrastructure instead of referencing existing resources.

Problems Fixed

Issue #12977

  1. Infrastructure Generation Ignores AsExisting Annotation

When a Container App Environment was marked with .AsExisting(), the infrastructure callback still created a brand new environment with all child resources (Container Registry, Log Analytics Workspace, User Assigned Identity, Dashboard, etc.) instead of referencing the existing environment. This caused deployment failures due to ACR access permission issues.

Location: src/Aspire.Hosting.Azure.AppContainers/AzureContainerAppExtensions.cs:60-324

  1. Resource Assignment Ignores WithComputeEnvironment Binding

When multiple Container App Environments existed, resources were assigned to ALL environments instead of only their specified environment via WithComputeEnvironment(). The last environment processed would win, causing resources to be deployed to the wrong environment.

Location: src/Aspire.Hosting.Azure.AppContainers/AzureContainerAppsInfrastructure.cs:39-52

Changes Made

  1. AzureContainerAppExtensions.cs - Added early-return logic in the infrastructure callback to detect ExistingAzureResourceAnnotation and call AddAsExistingResource() instead of creating new infrastructure
  2. AzureContainerAppsInfrastructure.cs - Added logic to check ComputeEnvironmentAnnotation and only assign resources to their matching environment
  3. ComputeEnvironmentAnnotation.cs- Made the class public (was internal) with XML documentation to allow access from theAppContainers` package
  4. AzureContainerAppsTests.cs - Added comprehensive tests:
  • AsExistingEnvironmentWithWithComputeEnvironmentBindsCorrectly - Validates single existing environment scenario
  • MultipleEnvironmentsWithAsExistingAndWithComputeEnvironmentBindsCorrectly - Validates multiple existing environments with correct binding

Test Plan

  • New tests added for both single and multiple existing environment scenarios
  • Validates correct environment assignment in DeploymentTargetAnnotation
  • Verifies Bicep output uses FromExisting references instead of creating new resources
  • Existing Container Apps tests pass without regression (requires manual verification due to unrelated schema validation issues)

Breaking Changes

None - this is a bug fix that makes existing functionality work as documented.

Related Issues

Fixes scenarios where:

  • .AsExisting(environmentName, sharedResourceGroupName) creates new infrastructure
  • .WithComputeEnvironment(env) doesn't bind resources to the correct environment
  • Deployments fail with ACR permission errors when using existing infrastructure

Note: The ComputeEnvironmentAnnotation visibility change from internal to public is intentional and necessary for the fix to work across assembly boundaries.

Checklist

  • Is this feature complete?
    • [x] Yes. Ready to ship.
    • [ ] No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • [x] Yes
    • [ ] No
  • Did you add public API?
    • [x] Yes
      • If yes, did you have an API Review for it?
        • [ ] Yes
        • [x] No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • [ ] Yes
        • [x] No
    • [ ] No
  • Does the change make any security assumptions or guarantees?
    • [ ] Yes
      • If yes, have you done a threat model and had a security review?
        • [ ] Yes
        • [ ] No
    • [x] No
  • Does the change require an update in our Aspire docs?

TheEagleByte avatar Nov 15 '25 15:11 TheEagleByte

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 12978

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 12978"

github-actions[bot] avatar Nov 15 '25 15:11 github-actions[bot]

@dotnet-policy-service agree

TheEagleByte avatar Nov 15 '25 15:11 TheEagleByte

I think we'll need to go with an approach like this trust fall approach. All of the outpts need to be satisfied though so it means that we have to somehow force the user to fullfill the existing contract or make it possible to supply a partial one (subset of the resources and outputs are available).

davidfowl avatar Nov 21 '25 04:11 davidfowl