Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 0 additions & 31 deletions src/Aspire.Hosting.Azure/AppIdentityResource.cs

This file was deleted.

36 changes: 24 additions & 12 deletions src/Aspire.Hosting.Azure/AzureResourcePreparer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ private async Task BuildRoleAssignmentAnnotations(DistributedApplicationModel ap
continue;
}

if (!resource.IsContainer() && resource is not ProjectResource)
if (!IsResourceValidForRoleAssignments(resource))
{
continue;
}
Expand Down Expand Up @@ -179,10 +179,13 @@ private async Task BuildRoleAssignmentAnnotations(DistributedApplicationModel ap
{
var (identityResource, roleAssignmentResources) = CreateIdentityAndRoleAssignmentResources(options, resource, roleAssignments);

// attach the identity resource to compute resource so it can be used by the compute environment
resource.Annotations.Add(new AppIdentityAnnotation(identityResource));

appModel.Resources.Add(identityResource);
if (resource != identityResource)
{
// attach the identity resource to compute resource so it can be used by the compute environment
resource.Annotations.Add(new AppIdentityAnnotation(identityResource));
// add the identity resource to the resource collection so it can be provisioned
appModel.Resources.Add(identityResource);
}
foreach (var roleAssignmentResource in roleAssignmentResources)
{
appModel.Resources.Add(roleAssignmentResource);
Expand All @@ -209,6 +212,13 @@ private async Task BuildRoleAssignmentAnnotations(DistributedApplicationModel ap
{
await CreateGlobalRoleAssignments(appModel, globalRoleAssignments, options).ConfigureAwait(false);
}

// We can derive role assignments for compute resources and declared
// AzureUserAssignedIdentityResources
static bool IsResourceValidForRoleAssignments(IResource resource)
{
return resource.IsContainer() || resource is ProjectResource || resource is AzureUserAssignedIdentityResource;
}
}

private static Dictionary<AzureProvisioningResource, IEnumerable<RoleDefinition>> GetAllRoleAssignments(IResource resource)
Expand All @@ -224,15 +234,17 @@ private static Dictionary<AzureProvisioningResource, IEnumerable<RoleDefinition>
return result;
}

private static (AppIdentityResource IdentityResource, List<AzureBicepResource> RoleAssignmentResources) CreateIdentityAndRoleAssignmentResources(
private static (AzureUserAssignedIdentityResource IdentityResource, List<AzureBicepResource> RoleAssignmentResources) CreateIdentityAndRoleAssignmentResources(
AzureProvisioningOptions provisioningOptions,
IResource resource,
Dictionary<AzureProvisioningResource, IEnumerable<RoleDefinition>> roleAssignments)
{
var identityResource = new AppIdentityResource($"{resource.Name}-identity")
{
ProvisioningBuildOptions = provisioningOptions.ProvisioningBuildOptions
};
var identityResource = resource is AzureUserAssignedIdentityResource existingIdentityResource
? existingIdentityResource
: new AzureUserAssignedIdentityResource($"{resource.Name}-identity")
{
ProvisioningBuildOptions = provisioningOptions.ProvisioningBuildOptions
};

var roleAssignmentResources = CreateRoleAssignmentsResources(provisioningOptions, resource, roleAssignments, identityResource);
return (identityResource, roleAssignmentResources);
Expand All @@ -242,7 +254,7 @@ private static List<AzureBicepResource> CreateRoleAssignmentsResources(
AzureProvisioningOptions provisioningOptions,
IResource resource,
Dictionary<AzureProvisioningResource, IEnumerable<RoleDefinition>> roleAssignments,
AppIdentityResource appIdentityResource)
AzureUserAssignedIdentityResource appIdentityResource)
{
var roleAssignmentResources = new List<AzureBicepResource>();
foreach (var (targetResource, roles) in roleAssignments)
Expand Down Expand Up @@ -271,7 +283,7 @@ private static void AddRoleAssignmentsInfrastructure(
AzureResourceInfrastructure infra,
AzureProvisioningResource azureResource,
IEnumerable<RoleDefinition> roles,
AppIdentityResource appIdentityResource)
AzureUserAssignedIdentityResource appIdentityResource)
{
var context = new AddRoleAssignmentsContext(
infra,
Expand Down
44 changes: 44 additions & 0 deletions src/Aspire.Hosting.Azure/AzureUserAssignedIdentityExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Aspire.Hosting.ApplicationModel;

namespace Aspire.Hosting.Azure;

/// <summary>
/// Provides extension methods for working with Azure user‑assigned identities.
/// </summary>
public static class AzureUserAssignedIdentityExtensions
{
/// <summary>
/// Adds an Azure user‑assigned identity resource to the application model.
/// </summary>
/// <param name="builder">The builder for the distributed application.</param>
/// <param name="name">The name of the resource.</param>
/// <exception cref="ArgumentNullException">Thrown when <paramref name="builder"/> is null.</exception>
/// <exception cref="ArgumentException">Thrown when <paramref name="name"/> is null or empty.</exception>
/// <remarks>
/// This method adds an Azure user‑assigned identity resource to the application model. It configures the
/// infrastructure for the resource and returns a builder for the resource.
/// The resource is added to the infrastructure only if the application is not in run mode.
/// </remarks>
/// <returns>A reference to the <see cref="IResourceBuilder{AzureUserAssignedIdentityResource}"/> builder.</returns>
public static IResourceBuilder<AzureUserAssignedIdentityResource> AddAzureUserAssignedIdentity(
this IDistributedApplicationBuilder builder,
string name)
{
ArgumentNullException.ThrowIfNull(builder);
ArgumentException.ThrowIfNullOrEmpty(name);

builder.AddAzureProvisioning();

var resource = new AzureUserAssignedIdentityResource(name);
// Don't add the resource to the infrastructure if we're in run mode.
if (builder.ExecutionContext.IsRunMode)
{
return builder.CreateResourceBuilder(resource);
}

return builder.AddResource(resource);
}
}
68 changes: 68 additions & 0 deletions src/Aspire.Hosting.Azure/AzureUserAssignedIdentityResource.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Azure.Provisioning;
using Azure.Provisioning.Primitives;
using Azure.Provisioning.Roles;

namespace Aspire.Hosting.Azure;

/// <summary>
/// An Azure Provisioning resource that represents an Azure user assigned managed identity.
/// </summary>
public sealed class AzureUserAssignedIdentityResource(string name)
: AzureProvisioningResource(name, ConfigureAppIdentityInfrastructure), IAppIdentityResource
{
/// <summary>
/// The identifier associated with the user assigned identity.
/// </summary>
public BicepOutputReference Id => new("id", this);

/// <summary>
/// The client ID of the user assigned identity.
/// </summary>
public BicepOutputReference ClientId => new("clientId", this);

/// <summary>
/// The principal ID of the user assigned identity.
/// </summary>
public BicepOutputReference PrincipalId => new("principalId", this);

/// <summary>
/// The principal name of the user assigned identity.
/// </summary>
public BicepOutputReference PrincipalName => new("principalName", this);

private static void ConfigureAppIdentityInfrastructure(AzureResourceInfrastructure infrastructure)
{
var userAssignedIdentity = CreateExistingOrNewProvisionableResource(infrastructure,
(identifier, name) =>
{
var resource = UserAssignedIdentity.FromExisting(identifier);
resource.Name = name;
return resource;
},
(infrastructure) =>
{
var identityName = Infrastructure.NormalizeBicepIdentifier(infrastructure.AspireResource.Name);
var resource = new UserAssignedIdentity(identityName);
return resource;
});

infrastructure.Add(userAssignedIdentity);

infrastructure.Add(new ProvisioningOutput("id", typeof(string)) { Value = userAssignedIdentity.Id });
infrastructure.Add(new ProvisioningOutput("clientId", typeof(string)) { Value = userAssignedIdentity.ClientId });
infrastructure.Add(new ProvisioningOutput("principalId", typeof(string)) { Value = userAssignedIdentity.PrincipalId });
infrastructure.Add(new ProvisioningOutput("principalName", typeof(string)) { Value = userAssignedIdentity.Name });
}

/// <inheritdoc/>
public override ProvisionableResource AddAsExistingResource(AzureResourceInfrastructure infra)
{
var store = UserAssignedIdentity.FromExisting(this.GetBicepIdentifier());
store.Name = PrincipalName.AsProvisioningParameter(infra);
Copy link
Member

@eerhardt eerhardt May 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does PrincipalName always work? Is that always the same as Name? Oh, we assign "Name" to the "principalName" output.

infra.Add(store);
return store;
}
}
102 changes: 102 additions & 0 deletions tests/Aspire.Hosting.Azure.Tests/AzureUserAssignedIdentityTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Aspire.Hosting.ApplicationModel;
using Aspire.Hosting.Azure.AppContainers;
using Aspire.Hosting.Azure.ContainerRegistry;
using Aspire.Hosting.Utils;
using Azure.Provisioning.ContainerRegistry;
using Microsoft.Extensions.DependencyInjection;
using static Aspire.Hosting.Utils.AzureManifestUtils;

namespace Aspire.Hosting.Azure.Tests;

public class AzureUserAssignedIdentityTests
{
[Fact]
public async Task AddAzureUserAssignedIdentity_GeneratesExpectedResourcesAndBicep()
{
// Arrange
var builder = TestDistributedApplicationBuilder.Create(DistributedApplicationOperation.Publish);

builder.AddAzureUserAssignedIdentity("myidentity");

using var app = builder.Build();
await ExecuteBeforeStartHooksAsync(app, default);

var model = app.Services.GetRequiredService<DistributedApplicationModel>();

// Act
var resource = Assert.Single(model.Resources.OfType<AzureUserAssignedIdentityResource>());

var (_, bicep) = await GetManifestWithBicep(resource);

await Verifier.Verify(bicep, extension: "bicep")
.UseHelixAwareDirectory("Snapshots")
.AutoVerify();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is something we should be doing; auto-accepting changes in the tests undermines the value of the tests - we won't be alerted when the snapshot changes, and thus, may inadvertently let a regression slip through.

For local dev there are tools to accept all the pending snapshots, which is useful when the snapshots first created --> https://github.com/VerifyTests/Verify/blob/main/readme.md#snapshot-management (I've been using DiffEngineTray).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes @captainsafia mentioned that she forgot to revert it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I noticed a branch in the trunk :)

To add - there's an optional parameter to autoaccept locally, but fail on a CI (https://github.com/VerifyTests/DiffEngine/blob/main/src/DiffEngine/BuildServerDetector.cs), but I don't think we should consider that either - some things we assert may contain random/generated information (like guid, port numbers, etc.). And auto-acceptance may hide those until after the changes are pushed.

Copy link
Member Author

@captainsafia captainsafia May 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, AutoVerify and I have a long history of getting in trouble in the ASP.NET Core repo. 😅

One thing we were able to do in the manual snapshot strategy we used for RDG is use a boolean that would regenerate the files but fail the test (see https://github.com/dotnet/aspnetcore/blob/754e56eea0d88f36653af0da1f9c00186d93cec8/src/Http/Http.Extensions/test/RequestDelegateGenerator/RequestDelegateCreationTestBase.cs#L363). This makes it easy to bootstrap or iterate on new snapshots without getting into trouble with accidentally merging. It seems like the BuildServerDetector helps with that.

but I don't think we should consider that either - some things we assert may contain random/generated information (like guid, port numbers, etc.).

I think for the most part, the GUIDs/port numbers we rely on might be consistent across tests, like a GUID associated with a role identity. For everything else, we probably want to use scrubbers.

Revert PR is #9134.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing we were able to do in the manual snapshot strategy we used for RDG is use a boolean that would regenerate the files but fail the test (see dotnet/aspnetcore@754e56e/src/Http/Http.Extensions/test/RequestDelegateGenerator/RequestDelegateCreationTestBase.cs#L363). This makes it easy to bootstrap or iterate on new snapshots without getting into trouble with accidentally merging. It seems like the BuildServerDetector helps with that.

DiffEngineTray can accept all pending snapshots in one go 😉

And yes for scrubbers.

}

[Fact]
public async Task AddAzureUserAssignedIdentity_PublishAsExisting_Works()
{
var builder = TestDistributedApplicationBuilder.Create(DistributedApplicationOperation.Publish);

builder.AddAzureUserAssignedIdentity("myidentity")
.PublishAsExisting("existingidentity", "my-rg");

using var app = builder.Build();
var model = app.Services.GetRequiredService<DistributedApplicationModel>();

var resource = Assert.Single(model.Resources.OfType<AzureUserAssignedIdentityResource>());

var (_, bicep) = await GetManifestWithBicep(resource);

await Verifier.Verify(bicep, extension: "bicep")
.UseHelixAwareDirectory("Snapshots")
.AutoVerify();
}

[Fact]
public async Task AddAzureUserAssignedIdentity_WithRoleAssignments_Works()
{
var builder = TestDistributedApplicationBuilder.Create(DistributedApplicationOperation.Publish);

builder.AddAzureContainerAppEnvironment("cae");

var registry = builder.AddAzureContainerRegistry("myregistry");
builder.AddAzureUserAssignedIdentity("myidentity")
.WithRoleAssignments(registry, [ContainerRegistryBuiltInRole.AcrPush]);

using var app = builder.Build();
var model = app.Services.GetRequiredService<DistributedApplicationModel>();
await ExecuteBeforeStartHooksAsync(app, default);

Assert.Collection(model.Resources.OrderBy(r => r.Name),
r => Assert.IsType<AzureContainerAppEnvironmentResource>(r),
r => Assert.IsType<AzureUserAssignedIdentityResource>(r),
r =>
{
Assert.IsType<AzureProvisioningResource>(r);
Assert.Equal("myidentity-roles-myregistry", r.Name);
},
r => Assert.IsType<AzureContainerRegistryResource>(r));

var identityResource = Assert.Single(model.Resources.OfType<AzureUserAssignedIdentityResource>());
var (_, identityBicep) = await GetManifestWithBicep(identityResource, skipPreparer: true);

var registryResource = Assert.Single(model.Resources.OfType<AzureContainerRegistryResource>());
var (_, registryBicep) = await GetManifestWithBicep(registryResource, skipPreparer: true);

var identityRoleAssignments = Assert.Single(model.Resources.OfType<AzureProvisioningResource>(), r => r.Name == "myidentity-roles-myregistry");
var (_, identityRoleAssignmentsBicep) = await GetManifestWithBicep(identityRoleAssignments, skipPreparer: true);

Target[] targets = [
new Target("bicep", identityBicep),
new Target("bicep", registryBicep),
new Target("bicep", identityRoleAssignmentsBicep)
];
await Verifier.Verify(targets)
.UseHelixAwareDirectory("Snapshots")
.AutoVerify();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
@description('The location for the resource(s) to be deployed.')
param location string = resourceGroup().location

resource myidentity 'Microsoft.ManagedIdentity/userAssignedIdentities@2023-01-31' = {
name: take('myidentity-${uniqueString(resourceGroup().id)}', 128)
location: location
}

output id string = myidentity.id

output clientId string = myidentity.properties.clientId

output principalId string = myidentity.properties.principalId

output principalName string = myidentity.name
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
@description('The location for the resource(s) to be deployed.')
param location string = resourceGroup().location

resource myidentity 'Microsoft.ManagedIdentity/userAssignedIdentities@2023-01-31' existing = {
name: 'existingidentity'
}

output id string = myidentity.id

output clientId string = myidentity.properties.clientId

output principalId string = myidentity.properties.principalId

output principalName string = myidentity.name
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
@description('The location for the resource(s) to be deployed.')
param location string = resourceGroup().location

resource myidentity 'Microsoft.ManagedIdentity/userAssignedIdentities@2023-01-31' = {
name: take('myidentity-${uniqueString(resourceGroup().id)}', 128)
location: location
}

output id string = myidentity.id

output clientId string = myidentity.properties.clientId

output principalId string = myidentity.properties.principalId

output principalName string = myidentity.name
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
@description('The location for the resource(s) to be deployed.')
param location string = resourceGroup().location

resource myregistry 'Microsoft.ContainerRegistry/registries@2023-07-01' = {
name: take('myregistry${uniqueString(resourceGroup().id)}', 50)
location: location
sku: {
name: 'Basic'
}
tags: {
'aspire-resource-name': 'myregistry'
}
}

output name string = myregistry.name

output loginServer string = myregistry.properties.loginServer
Loading