Fix BCP139 cross-resource-group ACR AcrPull role in compute environments - #18118
Fix BCP139 cross-resource-group ACR AcrPull role in compute environments#18118David Fowler (davidfowl) wants to merge 1 commit into
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 18118Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 18118" |
There was a problem hiding this comment.
Pull request overview
This PR fixes #11256 — a BCP139 Bicep deployment error triggered when an Azure compute environment (ACA or App Service) is configured to pull from an existing Azure Container Registry in a different resource group. The inline AcrPull role assignment in env.bicep failed because Bicep requires modules for cross-scope deployments. The fix extracts the managed identity and its role assignment into separately deployable modules, reusing the existing AzureResourcePreparer cross-resource-group scoping machinery.
Changes:
- The ACR-pull identity (
env-mi) is now materialized as a first-classAzureUserAssignedIdentityResourcein publish mode, with the AcrPull role assignment emitted as a scoped module (env-mi-roles-env-acr) instead of being inlined inenv.bicep. - A new pipeline step (per compute package) runs before
azure-prepare-resourcesto resolve the final registry and attach theRoleAssignmentAnnotationto the generated identity, ensuring correct last-writer-wins semantics forWithAzureContainerRegistry. - Two new regression tests (ACA + App Service) verify cross-resource-group ACR scenarios produce correctly scoped modules.
Show a summary per file
| File | Description |
|---|---|
src/Aspire.Hosting.Azure.AppContainers/AzureContainerAppExtensions.cs |
Adds pipeline step, helper methods for identity creation, role assignment, and registry resolution (ACA path) |
src/Aspire.Hosting.Azure.AppContainers/AzureContainerAppEnvironmentAcrPullIdentityAnnotation.cs |
Adds assignAcrPullRole constructor parameter and AssignAcrPullRole property |
src/Aspire.Hosting.Azure.AppService/AzureAppServiceEnvironmentExtensions.cs |
Same pipeline step and helper methods (App Service path, near-identical to ACA) |
src/Aspire.Hosting.Azure.AppService/AzureAppServiceEnvironmentAcrPullIdentityAnnotation.cs |
Adds assignAcrPullRole constructor parameter and AssignAcrPullRole property |
tests/Aspire.Hosting.Azure.Tests/AzureContainerAppEnvironmentExtensionsTests.cs |
New regression test for cross-RG ACR with ACA |
tests/Aspire.Hosting.Azure.Tests/AzureAppServiceEnvironmentExtensionsTests.cs |
New regression test for cross-RG ACR with App Service |
tests/Aspire.Hosting.Azure.Tests/AzureUserAssignedIdentityTests.cs |
Updated resource collection assertions to account for new env-mi identity |
tests/Aspire.Hosting.Azure.Tests/AzureDeployerTests.cs |
Fake provisioner updated with env-mi module outputs |
tests/Aspire.Hosting.Azure.Tests/AzureContainerAppsTests.cs |
Updated resource count and collection assertions |
tests/Aspire.Hosting.Azure.Tests/AzureAppServiceTests.cs |
Updated resource count and collection assertions |
~63 .verified.bicep/.verified.json/.verified.txt snapshots |
Reflect identity moved to standalone module, role assignment moved to scoped module |
Copilot's findings
- Files reviewed: 72/72 changed files
- Comments generated: 1
1032edd to
e898c05
Compare
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
e898c05 to
c8baed9
Compare
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
❓ CLI E2E Tests unknown — 115 passed, 0 failed, 2 unknown (commit View all recordings
📹 Recordings uploaded automatically from CI run #27516379251 |
In publish mode, AddAzureContainerAppEnvironment and AddAzureAppServiceEnvironment
inlined an AcrPull role assignment into the environment's bicep file scoped to the
container registry. When the registry is an existing resource in a different resource
group (via PublishAsExisting), Bicep rejected the cross-scope role assignment with
BCP139 ("A resource's scope must match the scope of the Bicep file").
The fix reuses the existing model-resource + role-module machinery that the
user-supplied identity path (WithAcrPullIdentity) already uses, so the unmodified
AzureResourcePreparer emits a separate {identity}-roles-{registry} module and sets
its scope to the registry's resource group for an existing cross-RG registry.
- Add an internal AssignAcrPullRole flag to the existing per-package
AcrPullIdentityAnnotation types to mark Aspire-generated default identities.
- In publish mode, create a default AzureUserAssignedIdentityResource ({env}-mi)
and attach it via the existing annotation so the env callback reads the identity
id as a parameter and skips the inline identity + inline AcrPull role. Run mode
is unchanged.
- Add a per-package pre-prepare pipeline step that resolves the environment's final
configured registry (honoring a later WithAzureContainerRegistry swap) and adds a
RoleAssignmentAnnotation(registry, AcrPull) to the generated identity.
- WithAcrPullIdentity removes the generated identity when the user supplies their own.
- The ACR-pull helpers and pipeline step are intentionally duplicated across the
AppContainers and AppService packages (they don't share internals); reciprocal
keep-in-sync comments document this.
No public API changes, no AzureResourcePreparer changes, no AKS changes. Adds
regression tests for both ACA and App Service asserting the inline cross-scope role
is gone and a role module scoped to the existing registry's resource group is emitted.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1c7c6e1 to
15a2bac
Compare
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Description
Fixes #11256
When a compute environment (Azure Container Apps or Azure App Service) is configured to pull from an existing Azure Container Registry that lives in a different resource group (via
PublishAsExisting("myacr", "my-existing-resource-group")+env.WithAzureContainerRegistry(acr)), the generatedenv.bicepinlined anAcrPullrole assignment scoped directly to that cross-resource-group registry. Bicep rejects a resource whose scope doesn't match the file's scope with BCP139 ("A resource's scope must match the scope of the Bicep file ... use modules to deploy resources to a different scope"), soaspire publishproduced bicep that fails to deploy.Relationship to #17988
This is the companion fix for #17988. Live PR-build testing of #17988's Azure scope APIs found that the shared organization ACR scenario from #7514 still fails end to end for Azure Container Apps unless the
AcrPullrole assignment is moved out of the ACA environment module. #17988 provides the scope model/APIs, and this PR makes the ACA/App Service generated Bicep composition deployable for cross-resource-group existing registries.What changed for users
Publishing an ACA or App Service environment that pulls from a cross-resource-group existing ACR now succeeds: the
AcrPullrole assignment is emitted as a separately-scoped module instead of being inlined inenv.bicep, so it deploys without BCP139. The granted role is unchanged (AcrPullon the same identity and registry) — only where the assignment is declared moves. Run mode and same-resource-group scenarios are unaffected.Approach (minimal)
In publish mode, the ACR-pull identity is materialized as a first-class
AzureUserAssignedIdentityResource({env}-mi) added to the model, and the existing internal...AcrPullIdentityAnnotationis flagged with a new internalAssignAcrPullRole. This flips the environment's infrastructure callback onto its existing "bring-your-own identity" branch (the env module reads the identity id, and for App Service the client id, from a parameter), so the callback no longer emits the inline_miidentity or the inlineAcrPullrole.A per-package pre-prepare pipeline step (
requiredBy: PrepareResourcesStepName) resolves the environment's final registry — honoring any laterWithAzureContainerRegistryswap, last-writer-wins — and adds aRoleAssignmentAnnotation(registry, AcrPull)to the generated identity. The unchangedAzureResourcePreparerthen turns that annotation on the standalone identity into a separately-scoped{identity}-roles-{registry}module, settingScope = resourceGroup(...)when the registry is an existing cross-resource-group resource. That scoped module is the BCP139 fix.WithAcrPullIdentity(bring-your-own identity) removes the generated identity and skips the role assignment, preserving today's behavior where the caller owns the role via.WithRoleAssignments(acr, AcrPull).Constraints honored
api/*.cschanges).AzureResourcePreparer.csuntouched — the fix reuses its existing cross-resource-group role-module machinery.InternalsVisibleTo/.csprojchanges. Reuses the existing publicRoleAssignmentAnnotation.Validation
env.bicepcontains no inlineAcrPullrole assignment and thatmain.bicepemits a{env}-mi-roles-acrmodule scoped toresourceGroup('my-existing-resource-group').AzureDeployerTestsfake provisioner (now suppliesid/principalId/clientIdoutputs for the generated*-miidentity modules)._miidentity + inlineAcrPullrole removed fromenv.bicep; the env module now takes the identity id (and client id, for App Service) as a parameter; the role moves to a scoped module.Aspire.Hosting.Azure.Tests: 1477 passed, 5 skipped. The only failures (15) are pre-existing Docker-emulator functional tests (EventHubs/Storage/SignalR/Cosmos) unrelated to this change.Minimal alternative to #17992 (which refactors
AzureResourcePreparerand adds public API); opening this so both approaches can be compared. Not closing #17992.Checklist
<remarks />and<code />elements on your triple slash comments?