Fix ACR pull role assignments for existing registries

src/Aspire.Hosting.Azure.AppContainers/Aspire.Hosting.Azure.AppContainers.csproj

@@ -13,6 +13,7 @@
     <Compile Include="$(SharedDir)BicepFormattingHelpers.cs" LinkBase="Shared\BicepFormattingHelpers.cs" />
     <Compile Include="$(SharedDir)ComputeEnvironmentEndpointResolver.cs" LinkBase="Shared\ComputeEnvironmentEndpointResolver.cs" />
     <Compile Include="$(SharedDir)ContainerRegistryInfrastructure.cs" LinkBase="Shared\ContainerRegistryInfrastructure.cs" />
+    <Compile Include="$(SharedDir)GeneratedContainerRegistryAnnotation.cs" LinkBase="Shared\GeneratedContainerRegistryAnnotation.cs" />
     <Compile Include="$(SharedDir)ResourceNameComparer.cs" LinkBase="Shared\ResourceNameComparer.cs" />
   </ItemGroup>
 

src/Aspire.Hosting.Azure.AppContainers/AzureContainerAppEnvironmentAcrPullIdentityAnnotation.cs

@@ -11,11 +11,19 @@ namespace Aspire.Hosting.Azure;
 /// configured container registry, instead of having Aspire create a new identity and a new <c>AcrPull</c>
 /// role assignment.
 /// </summary>
-/// <param name="identity">The user-assigned identity resource to use for the <c>AcrPull</c> role.</param>
-internal sealed class AzureContainerAppEnvironmentAcrPullIdentityAnnotation(AzureUserAssignedIdentityResource identity) : IResourceAnnotation
+/// <param name="identity">The user-assigned identity resource to use for container registry pulls.</param>
+/// <param name="assignAcrPullRole">Indicates whether Aspire should assign the <c>AcrPull</c> role to the identity.</param>
+internal sealed class AzureContainerAppEnvironmentAcrPullIdentityAnnotation(
+    AzureUserAssignedIdentityResource identity,
+    bool assignAcrPullRole) : IResourceAnnotation
 {
     /// <summary>
-    /// Gets the user-assigned identity resource that holds the <c>AcrPull</c> role.
+    /// Gets the user-assigned identity resource used for container registry pulls.
     /// </summary>
     public AzureUserAssignedIdentityResource Identity { get; } = identity;
+
+    /// <summary>
+    /// Gets a value indicating whether Aspire should assign the <c>AcrPull</c> role to the identity.
+    /// </summary>
+    public bool AssignAcrPullRole { get; } = assignAcrPullRole;
 }

src/Aspire.Hosting.Azure.AppContainers/AzureContainerAppEnvironmentResource.cs

@@ -182,13 +182,7 @@ private async Task PrepareDeploymentTargetsAsync(PipelineStepContext context)
             return;
         }
 
-        // Remove the default container registry from the model if an explicit registry is configured
-        if (this.HasAnnotationOfType<ContainerRegistryReferenceAnnotation>() &&
-            DefaultContainerRegistry is not null)
-        {
-            appModel.Resources.Remove(DefaultContainerRegistry);
-            DefaultContainerRegistry = null;
-        }
+        RemoveGeneratedContainerRegistryIfReplaced(appModel);
 
         var logger = services.GetRequiredService<ILogger<AzureContainerAppEnvironmentResource>>();
         var options = services.GetRequiredService<IOptions<AzureProvisioningOptions>>();
@@ -199,6 +193,18 @@ private async Task PrepareDeploymentTargetsAsync(PipelineStepContext context)
             this,
             services);
 
+        // Deployment prerequisites such as the AcrPull role assignment are owned by the environment,
+        // not by each generated container app. The role-assignment module does not produce a value that
+        // the environment or container app Bicep naturally references, so there is no implicit data-flow
+        // dependency for Bicep to infer. References carries the explicit ordering edge: the environment
+        // module and each generated deployment target must wait for the environment-owned prerequisite
+        // modules before image push/deploy steps can rely on the permission being present.
+        var environmentDeploymentPrerequisites = GetEnvironmentDeploymentPrerequisites();
+        foreach (var prerequisite in environmentDeploymentPrerequisites)
+        {
+            References.Add(prerequisite);
+        }
+
         foreach (var r in appModel.GetComputeResources())
         {
             // Skip resources that are explicitly targeted to a different compute environment
@@ -208,6 +214,7 @@ private async Task PrepareDeploymentTargetsAsync(PipelineStepContext context)
                 continue;
             }
 
+            AddDeploymentPrerequisites(r, environmentDeploymentPrerequisites);
             var containerApp = await containerAppEnvironmentContext.CreateContainerAppAsync(r, options.Value, cancellationToken).ConfigureAwait(false);
 
             // Capture information about the container registry used by the
@@ -224,6 +231,41 @@ private async Task PrepareDeploymentTargetsAsync(PipelineStepContext context)
         containerAppEnvironmentContext.LogHttpsUpgradeIfNeeded();
     }
 
+    private IReadOnlySet<AzureBicepResource> GetEnvironmentDeploymentPrerequisites()
+    {
+        if (!this.TryGetAnnotationsOfType<DeploymentPrerequisitesAnnotation>(out var prerequisiteAnnotations))
+        {
+            return new HashSet<AzureBicepResource>();
+        }
+
+        // The preparer writes environment-owned prerequisites after it materializes generated
+        // role-assignment modules. Collapse all annotations here so future environment-owned
+        // infrastructure can participate in the same deployment ordering contract.
+        return prerequisiteAnnotations.SelectMany(a => a.Resources).ToHashSet();
+    }
+
+    private static void AddDeploymentPrerequisites(IResource resource, IReadOnlySet<AzureBicepResource> prerequisites)
+    {
+        if (prerequisites.Count == 0)
+        {
+            return;
+        }
+
+        var newPrerequisites = prerequisites.ToHashSet();
+        if (resource.TryGetAnnotationsOfType<DeploymentPrerequisitesAnnotation>(out var existingAnnotations))
+        {
+            // A resource can already have prerequisites from direct Azure references. Only add the
+            // environment-level resources it does not already wait on to avoid duplicate References
+            // when deployment targets are generated.
+            newPrerequisites.ExceptWith(existingAnnotations.SelectMany(a => a.Resources));
+        }
+
+        if (newPrerequisites.Count > 0)
+        {
+            resource.Annotations.Add(new DeploymentPrerequisitesAnnotation(newPrerequisites));
+        }
+    }
+
     internal bool UseAzdNamingConvention { get; set; }
 
     internal bool UseCompactResourceNaming { get; set; }
@@ -273,10 +315,8 @@ private async Task PrepareDeploymentTargetsAsync(PipelineStepContext context)
     internal Dictionary<string, (IResource resource, ContainerMountAnnotation volume, int index, BicepOutputReference outputReference)> VolumeNames { get; } = [];
 
     /// <summary>
-    /// Gets the default container registry for this environment.
+    /// Gets the configured container registry for this environment.
     /// </summary>
-    internal AzureContainerRegistryResource? DefaultContainerRegistry { get; set; }
-
     ReferenceExpression IContainerRegistry.Name => GetContainerRegistry()?.Name ?? ReferenceExpression.Create($"{ContainerRegistryName}");
 
     ReferenceExpression IContainerRegistry.Endpoint => GetContainerRegistry()?.Endpoint ?? ReferenceExpression.Create($"{ContainerRegistryUrl}");
@@ -291,8 +331,24 @@ private async Task PrepareDeploymentTargetsAsync(PipelineStepContext context)
             return annotation.Registry;
         }
 
-        // Fall back to default container registry
-        return DefaultContainerRegistry;
+        return null;
+    }
+
+    private void RemoveGeneratedContainerRegistryIfReplaced(DistributedApplicationModel appModel)
+    {
+        if (!this.TryGetLastAnnotation<GeneratedContainerRegistryAnnotation>(out var generatedRegistryAnnotation))
+        {
+            return;
+        }
+
+        if (this.TryGetLastAnnotation<ContainerRegistryReferenceAnnotation>(out var currentRegistryAnnotation) &&
+            ReferenceEquals(currentRegistryAnnotation.Registry, generatedRegistryAnnotation.Registry))
+        {
+            return;
+        }
+
+        appModel.Resources.Remove(generatedRegistryAnnotation.Registry);
+        Annotations.Remove(generatedRegistryAnnotation);
     }
 
     /// <summary>