Skip to content

Fix Azure Functions Role Assignments to Host Storage#8290

Merged
eerhardt merged 2 commits intomicrosoft:mainfrom
eerhardt:FixAzureFunctions
Mar 25, 2025
Merged

Fix Azure Functions Role Assignments to Host Storage#8290
eerhardt merged 2 commits intomicrosoft:mainfrom
eerhardt:FixAzureFunctions

Conversation

@eerhardt
Copy link
Copy Markdown
Member

Description

With #8127, a mistake was made in that WithRoleAssignments only works if the app is using ACA infrastructure. If the app isn't using our ACA infrastructure, WithRoleAssignments throws an exception at startup.

To make Azure Functions work correctly both with and without ACA infrastructure we use the internal WithDefaultRoleAssignments method to set the default role assignments on the Host Storage, which will be respected for both.

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?
    • No
  • Does the change require an update in our Aspire docs?
    • No

With microsoft#8127, a mistake was made in that WithRoleAssignments only
works if the app is using ACA infrastructure. If the app isn't using our
ACA infrastructure, WithRoleAssignments throws an exception at startup.

To make Azure Functions work correctly both with and without ACA infrastructure
we use the internal WithDefaultRoleAssignments method to set the default
role assignments on the Host Storage, which will be respected for both.
@github-actions github-actions bot added the area-integrations Issues pertaining to Aspire Integrations packages label Mar 25, 2025
Copy link
Copy Markdown
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 fixes the role assignment issue for Azure Functions host storage so that default role assignments are applied when not using ACA infrastructure. The changes adjust how storage resources are created and update tests to reflect the correct built-in role assignments.

  • Updated storage resource creation to use WithDefaultRoleAssignments when the storage resource is null.
  • Removed the block that removed role assignments for explicit storage resources.
  • Updated test resources to match the expected built-in role IDs and resource names.

Reviewed Changes

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

File Description
src/Aspire.Hosting.Azure.Functions/AzureFunctionsProjectResourceExtensions.cs Change storage creation logic to conditionally add default role assignments without removing explicit role assignments.
tests/Aspire.Hosting.Azure.Tests/AzureFunctionsTests.cs Swapped test resource names and role definition IDs to match updated role assignments.
Files not reviewed (2)
  • playground/AzureFunctionsEndToEnd/AzureFunctionsEndToEnd.AppHost/aspire-manifest.json: Language not supported
  • src/Aspire.Hosting.Azure.Functions/Aspire.Hosting.Azure.Functions.csproj: Language not supported
Comments suppressed due to low confidence (2)

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

  • The new conditional block applies default role assignments only when storage is implicitly created. Please verify that this change does not lead to unintended role conflicts when an explicit storage resource is provided.
if (storage is null)

tests/Aspire.Hosting.Azure.Tests/AzureFunctionsTests.cs:315

  • The role assignment resource names and roleDefinition IDs have been swapped relative to previous tests. Ensure that the intended built-in role (StorageTableDataContributor vs StorageQueueDataContributor) is consistently referenced between the code and tests.
resource funcstorage634f8_StorageTableDataContributor 'Microsoft.Authorization/roleAssignments@2022-04-01' = {

Copy link
Copy Markdown
Contributor

@captainsafia captainsafia left a comment

Choose a reason for hiding this comment

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

Thanks for adding the test!

@eerhardt eerhardt merged commit 6900eeb into microsoft:main Mar 25, 2025
164 checks passed
@eerhardt eerhardt deleted the FixAzureFunctions branch March 25, 2025 20:00
@github-actions github-actions bot locked and limited conversation to collaborators Apr 25, 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.

3 participants