Skip to content

Conversation

@captainsafia
Copy link
Member

Closes #8143.

Prior to this change, we still maintained a RoleAssignmentAnnotation for the implicit host storage resource even when WithHostStorage was called to set it explicitly. As a result of this, the AzureResourcePreparer would emit role assignments against this stale resource pointer. This declaration would be emitted into the manifest and result in a Bicep module for the stale implicit host storage still being created.

Copilot AI review requested due to automatic review settings March 19, 2025 18:44
@github-actions github-actions bot added the area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication label Mar 19, 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 pull request removes role assignments on an implicit storage account when an explicit host storage is provided, preventing the deployment of a stale implicit host storage resource in the manifest. It introduces additional tests to verify proper behavior with and without role assignments and updates the resource extension logic to remove legacy role assignment annotations.

Reviewed Changes

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

File Description
tests/Aspire.Hosting.Azure.Tests/AzureFunctionsTests.cs Added tests to validate the WithHostStorage functionality and role assignment handling
src/Aspire.Hosting.Azure.Functions/AzureFunctionsProjectResourceExtensions.cs Updated annotation removal logic and comments to remove stale role assignments for implicit storage
Comments suppressed due to low confidence (1)

src/Aspire.Hosting.Azure.Functions/AzureFunctionsProjectResourceExtensions.cs:67

  • Using SingleOrDefault assumes that there will be at most one role assignment for the storage resource. If multiple annotations exist, this could throw an exception; consider using FirstOrDefault or adding validation to ensure uniqueness.
if (item.TryGetAnnotationsOfType<RoleAssignmentAnnotation>(out var roleAssignments) && roleAssignments.SingleOrDefault(assignment => assignment.Target == storage) is { } roleAssignmentAnnotation)

Copy link
Member

@eerhardt eerhardt left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@captainsafia captainsafia merged commit 72f1dd9 into main Mar 19, 2025
166 checks passed
@captainsafia captainsafia deleted the cs/funcs-aca branch March 19, 2025 20:37
eerhardt added a commit to eerhardt/aspire that referenced this pull request Mar 20, 2025
eerhardt added a commit that referenced this pull request Mar 20, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Apr 19, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Functions project provisioning storage even when WithHostStorage is set in Aspire 9.2

3 participants