Skip to content

Conversation

@eerhardt
Copy link
Member

Description

Adding role assignment support for PostgreSQL following the pattern set in #8140

Fix #6161

Checklist

  • Is this feature complete?
    • Yes. Ready to ship!!
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
  • Did you add public API?
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • ongoing
  • Does the change require an update in our Aspire docs?

…tity

Adding role assignment support for PostgreSQL following the pattern set in dotnet#8140

Fix dotnet#6161
@github-actions github-actions bot added the area-integrations Issues pertaining to Aspire Integrations packages label Mar 20, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for Azure SQL Server role assignments on app-specific managed identities by extending existing resource patterns and updating tests accordingly.

  • Introduces a new NameOutputReference and AddRoleAssignments method in AzureSqlServerResource.
  • Adjusts the AzureSqlExtensions logic to generate role assignments and outputs the server name.
  • Updates tests to verify the new role assignment outputs in generated Bicep manifests.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

File Description
src/Aspire.Hosting.Azure.Sql/AzureSqlServerResource.cs Added role assignment support and a NameOutputReference property for exporting the server name.
src/Aspire.Hosting.Azure.Sql/AzureSqlExtensions.cs Updated resource creation logic and annotation to support role assignments.
tests/Aspire.Hosting.Azure.Tests/* Extended tests to check for server name outputs in the Bicep manifests.
Comments suppressed due to low confidence (1)

src/Aspire.Hosting.Azure.Sql/AzureSqlServerResource.cs:111

  • [nitpick] The variable name 'postgres' is misleading as it represents an SQL Server resource. Consider renaming it to 'sqlServer' for improved clarity.
var postgres = (SqlServer)AddAsExistingResource(infra);

var resource = new AzureSqlServerResource(name, configureInfrastructure);
var azureSqlServer = builder.AddResource(resource);
var azureSqlServer = builder.AddResource(resource)
.WithAnnotation(new DefaultRoleAssignmentsAnnotation(new HashSet<RoleDefinition>()));
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.WithAnnotation(new DefaultRoleAssignmentsAnnotation(new HashSet<RoleDefinition>()));
.WithAnnotation(new DefaultRoleAssignmentsAnnotation([]));

Copy link
Member Author

Choose a reason for hiding this comment

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

Can't because of [API Proposal]: Allow collections expression for other collections (dotnet/runtime#108457). The parameter is of type IReadOnlySet which doesn't support collection expressions.

Copy link
Member

@davidfowl davidfowl left a comment

Choose a reason for hiding this comment

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

Clean

@eerhardt eerhardt merged commit b5752c2 into dotnet:main Mar 21, 2025
167 checks passed
@eerhardt eerhardt deleted the AzureSqlRoleAssignment branch March 21, 2025 17:17
@github-actions github-actions bot locked and limited conversation to collaborators Apr 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-integrations Issues pertaining to Aspire Integrations packages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve Azure Managed Identity Defaults

2 participants