Add WithUniqueResourceNaming to opt into collision-free managed environment names#18737
Add WithUniqueResourceNaming to opt into collision-free managed environment names#18737mitchdenny wants to merge 14 commits into
Conversation
Multiple AddAzureContainerAppEnvironment resources in one resource group generated identical managed-environment names because Azure.Provisioning's default name sanitizer strips digits, collapsing e.g. cae1/cae2 onto a single physical environment and racing with ManagedEnvironmentOperationInProgress. Default naming now preserves digits so distinct resource names produce distinct environment names. WithSingletonResourceNaming() opts back into the pre-fix name for already-deployed single-environment resource groups. Fixes #18722 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Real Azure deployment validation for #18722I validated this fix with real Test appsTwo minimal AppHosts, each attaching public-image containers to Container App Environments (no image build/push needed): // Single
var cae1 = builder.AddAzureContainerAppEnvironment("cae1");
builder.AddContainer("web1", "mcr.microsoft.com/dotnet/samples", "aspnetapp")
.WithHttpEndpoint(targetPort: 8080).WithComputeEnvironment(cae1);
// Multi
var cae1 = builder.AddAzureContainerAppEnvironment("cae1");
var cae2 = builder.AddAzureContainerAppEnvironment("cae2");
builder.AddContainer("web1", ...).WithComputeEnvironment(cae1);
builder.AddContainer("web2", ...).WithComputeEnvironment(cae2);CLI: Phase 1 — no fix (reproduce the bug)Both CAEs generate the identical managed-environment name because the Azure.Provisioning name sanitizer strips digits, so Multi (bug): deploy failed exactly as reported in the issue — Resulting Azure state: one managed environment ( Single (baseline): deployed fine, env Phase 2 — with fix (distinct naming)The fix preserves digits in the name prefix, so the environments become distinct. Notably the Multi: both environments provisioned distinctly and Azure state after: Single: same behaviour — env renamed This is the key finding that motivates the opt-out: the default naming change is correct for new deployments, but existing deployments cannot silently migrate a container app to a renamed environment. Phase 3 —
|
| Phase | Scenario | Outcome |
|---|---|---|
| 1 no-fix | single | ✅ baseline env caekzeq3vbgappqw |
| 1 no-fix | multi | ❌ ManagedEnvironmentOperationInProgress — bug reproduced, only 1 env / web2 lost |
| 2 fix | single | env renamed cae…→cae1…; web1 mismatch (expected redeploy churn) |
| 2 fix | multi | ✅ distinct envs, web2 deploys; web1 mismatch (expected redeploy churn) |
| 3 pin | single | ✅ exit 0 — legacy name restored, clean redeploy |
| 3 pin | multi | ✅ exit 0 — cae1 pinned legacy + cae2 distinct, both apps deploy |
Conclusion: the default fix eliminates the colliding-name bug for new deployments, the redeploy churn on existing deployments is real and expected (Azure won't move a container app between environments), and WithSingletonResourceNaming() is the correct, verified escape hatch for that case.
Note: the orphaned environments left behind in phase 2 (
cae1kzeq3vbgappqw,cae1iucdjryuegdpu) are artifacts of the deliberate same-RG rename test, not of normal usage. Test resource groups were cleaned up after validation.
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 18737Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 18737" |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Fixes Azure Container Apps managed environment name collisions when multiple AddAzureContainerAppEnvironment(...) resources are deployed into the same resource group by explicitly setting the managed environment name (preserving digits) in the default naming path, and adds an opt-out API to preserve legacy singleton naming.
Changes:
- Explicitly sets
ContainerAppManagedEnvironment.Name(non-azd, non-singleton) to include the resource name prefix to avoid collisions. - Adds
WithSingletonResourceNaming()(experimental) to preserve the pre-fix managed environment naming behavior. - Adds unit tests that assert the generated managed environment
name:expressions in emitted Bicep.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Hosting.Azure.Tests/AzureContainerAppsTests.cs | Adds regression tests asserting distinct managed environment names by default and legacy naming when opting out. |
| src/Aspire.Hosting.Azure.AppContainers/AzureContainerAppExtensions.cs | Sets managed environment Name explicitly to prevent collisions; introduces WithSingletonResourceNaming() API and helper prefix builder. |
| src/Aspire.Hosting.Azure.AppContainers/AzureContainerAppEnvironmentResource.cs | Adds internal flag to support singleton naming opt-out behavior. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Low
Root-cause analysis: why this was latent since #8606This is not a fresh regression — it's a latent gap introduced when managed environment naming was first made resource-name-aware, which the test suite structurally couldn't catch. Origin: PR #8606Before #8606, the managed environment was created with a fixed literal identifier: var containerAppEnvironment = new ContainerAppManagedEnvironment("cae")There was only ever one environment, so its default name #8606 ("AddAzureContainerAppEnvironment should use the environment name as a prefix") deliberately changed it to derive the name from the resource, with the explicit intent of "allow[ing] for multiple environments to be in a single distributed application in the future": var containerAppEnvironment = new ContainerAppManagedEnvironment(appEnvResource.GetBicepIdentifier())Why it silently broke#8606 only changed the bicep symbolic identifier (the That is why it went unnoticed:
It's the third fix in this naming-collision family
Residual constraint worth notingBecause Why the new test closes the gapThe suite previously asserted on bicep shape rather than name distinctness. The new |
…on note - Look up environments by resource name instead of enumeration order in the multi-environment distinctness test (order of model.Resources isn't guaranteed). - Add WithCompactResourceNamingGeneratesDistinctManagedEnvironmentNames to cover the compact-naming + default-naming interaction. - Document that uniqueString is identical per resource group, so distinctness relies on the name prefix surviving the 24-char truncation (#14427 class). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…2-multiple-azurecontainerappenvironments-i-fd59af
This comment has been minimized.
This comment has been minimized.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
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>
…den E2E test Match the opted-in ContainerAppManagedEnvironment by reference identity rather than by Bicep identifier, since Bicep identifiers are only unique within a module and could otherwise rename an unrelated environment that shares the same name in a different module. Also dispose the TemporaryWorkspace and assert the AppHost entry point exists before rewriting it in the multi-environment deployment E2E test. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: fd0bc08a-9ca0-4612-b9bd-284c7ab547a6
This comment has been minimized.
This comment has been minimized.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: fd0bc08a-9ca0-4612-b9bd-284c7ab547a6
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: fd0bc08a-9ca0-4612-b9bd-284c7ab547a6
This comment has been minimized.
This comment has been minimized.
PR Testing ReportPR Information
Artifact Version Verification
Changes AnalyzedChange Categories
The PR adds an opt-in, non-breaking MethodologyValidation targets publish-time Bicep generation, which is where the naming contract is Test Scenarios ExecutedScenario 1: Default behavior (no opt-in) — regression guardObjective: Two environments Generated managed-environment name: take('cae${uniqueString(resourceGroup().id)}', 24)Both are identical — the legacy behavior is preserved exactly, and the underlying #18722 Scenario 2: Both environments opted in — distinct namesObjective: // cae1/cae1.bicep
name: take('cae1-${uniqueString(resourceGroup().id)}', 32)
// cae2/cae2.bicep
name: take('cae2-${uniqueString(resourceGroup().id)}', 32)Digits are preserved, a hyphen separator is used, and max length is 32 — matching the Scenario 3: Hyphenated name boundary (
|
| Scenario | Status | Result |
|---|---|---|
| 1. Default (no opt-in) | Passed | Both cae${uniqueString} (24) — legacy preserved, collision reproduced |
| 2. Both opted in | Passed | Distinct cae1-... / cae2-... (32) |
3. Hyphen boundary my-cae1 |
Passed | mycae1-... (32) |
| 4. Mixed opt-in | Passed | cae1-... (32) + cae${uniqueString} (24), no collision |
Overall Result
PR VERIFIED
- Default path is byte-identical to shipped behavior (no breaking change, no forced
environment re-creation on redeploy). - Opt-in produces distinct, standard-algorithm-consistent names that resolve the Multiple AzureContainerAppEnvironments in one resource group generate colliding managed-environment names #18722
collision. - Opt-in is correctly scoped per environment.
Notes / Recommendations
- Validation was performed at the
aspire publishBicep-generation layer (no live Azure
deploy in this run). The repository additionally carries live deployment E2E coverage in
tests/Aspire.Deployment.EndToEnd.Tests/AcaCompactNamingDeploymentTests.cs. - Follow-up (tracked separately): the polyglot fixtures under
tests/PolyglotAppHosts/Aspire.Hosting.Azure.AppContainers/(Go/Java/TypeScript) could be
extended to exercisewithUniqueResourceNaming()for cross-language coverage.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: fd0bc08a-9ca0-4612-b9bd-284c7ab547a6 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: fd0bc08a-9ca0-4612-b9bd-284c7ab547a6
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. 9 / 100 test projects · 3 jobs, from 4 changed files. Selected test projects (9 / 100)
Selected jobs (3)
How these were chosen — grouped by what changed🔧 🧪 🔧 🧪 Job reasons
Selection computed for commit |
| /// <para> | ||
| /// This option has no effect when <see cref="WithAzdResourceNaming"/> is also used, since azd naming sets | ||
| /// the environment name explicitly. | ||
| /// </para> |
| /// </para> | ||
| /// </remarks> | ||
| [AspireExport] | ||
| [Experimental("ASPIREACANAMING002", UrlFormat = "https://aka.ms/aspire/diagnostics/{0}")] |
| [Experimental("ASPIREACANAMING002", UrlFormat = "https://aka.ms/aspire/diagnostics/{0}")] | ||
| public static IResourceBuilder<AzureContainerAppEnvironmentResource> WithUniqueResourceNaming(this IResourceBuilder<AzureContainerAppEnvironmentResource> builder) |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Description
Multiple
AddAzureContainerAppEnvironment(...)resources deployed to the same resource group generate identical managed-environment names, collapsing onto a single physical Azure environment and racing withManagedEnvironmentOperationInProgress.Root cause:
ContainerAppManagedEnvironmentinherits Azure.Provisioning'sResourceNameRequirements(1, 24, LowercaseLetters), whose sanitizer strips digits. So environments named e.g.cae1/cae2both resolve totake('cae${uniqueString(resourceGroup().id)}', 24)— the same name in a shared resource group.Approach: opt-in, non-breaking
Changing the managed environment name by default would rename already-deployed environments and force Azure to recreate them, which is far too dangerous to ship as a default. So the fix is opt-in, mirroring how
WithCompactResourceNamingshipped:WithUniqueResourceNaming()is a new per-environment extension method that opts into digit-preserving managed environment names, so distinct resource names produce distinct environment names. Apply it to eachAddAzureContainerAppEnvironment(...)that needs to coexist with others in the same resource group.The method is marked
[Experimental("ASPIREACANAMING001")]. It is implemented as a scoped fallbackResourceNamePropertyResolver, so any name resolver explicitly configured by the caller (e.g.AspireV8ResourceNamePropertyResolver) still takes precedence.Fixes #18722
Validation
Unit tests assert the generated Bicep names: collision is preserved by default, and
WithUniqueResourceNaming()produces distinct digit-preserving names. The fix was also validated with realaspire deployruns against live Azure for both single-CAE and multi-CAE scenarios — see the detailed testing report in the PR comments. A deployment E2E test deploys two environments into one resource group and asserts Azure contains two distinct managed environments.Checklist
<remarks />and<code />elements on your triple slash comments?