-
Notifications
You must be signed in to change notification settings - Fork 667
Fix Azure Functions Role Assignments to Host Storage #8290
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
With dotnet#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.
There was a problem hiding this 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' = {
src/Aspire.Hosting.Azure.Functions/AzureFunctionsProjectResourceExtensions.cs
Show resolved
Hide resolved
There was a problem hiding this 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!
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