Fix Blob Container Connection String Format Exception#9472
Fix Blob Container Connection String Format Exception#9472sebastienros merged 5 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes how AzureBlobStorageContainerSettings distinguishes between a service URI and an emulator connection string when parsing, ensuring the correct client constructor is used.
- Introduce
ParseConnectionStringInternalto centralize parsing logic. - Update
AzureBlobStorageContainerSettingsto call the internal parser on the endpoint value. - Add unit tests covering service URI, emulator connection string, and invalid URI scenarios.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/Aspire.Azure.Storage.Blobs.Tests/AzureBlobStorageContainerSettingsTests.cs | Added tests for parsing service URI, emulator connection string, and non-URI endpoints. |
| src/Components/Aspire.Azure.Storage.Blobs/AzureStorageBlobsSettings.cs | Exposed ParseConnectionStringInternal and hooked up the interface implementation. |
| src/Components/Aspire.Azure.Storage.Blobs/AzureBlobStorageContainerSettings.cs | Changed logic to call ParseConnectionStringInternal instead of assigning directly to ConnectionString. |
| playground/AzureStorageEndToEnd/AzureStorageEndToEnd.AppHost/Program.cs | Commented-out RunAsEmulator calls in example host to reflect new defaults. |
Comments suppressed due to low confidence (2)
src/Components/Aspire.Azure.Storage.Blobs/AzureBlobStorageContainerSettings.cs:29
- [nitpick] The constant 'ContainerName' is shadowing the property name and can be confusing; consider renaming it to 'ContainerNameKey' or similar to clarify its purpose.
const string ContainerName = nameof(ContainerName);
tests/Aspire.Azure.Storage.Blobs.Tests/AzureBlobStorageContainerSettingsTests.cs:49
- Add an assertion that
settings.ConnectionStringis null when a service URI is provided to ensure onlyServiceUriis set andConnectionStringremains unset.
public void ParseConnectionString_With_ServiceUri(string connectionString)
playground/AzureStorageEndToEnd/AzureStorageEndToEnd.AppHost/Program.cs
Outdated
Show resolved
Hide resolved
src/Components/Aspire.Azure.Storage.Blobs/AzureBlobStorageContainerSettings.cs
Outdated
Show resolved
Hide resolved
| builder.Append($"Endpoint={ConnectionStringExpression}"); | ||
| } | ||
|
|
||
| if (!string.IsNullOrEmpty(blobContainerName)) |
There was a problem hiding this comment.
This can be removed cuz it is already checked above.
| @@ -231,10 +231,11 @@ public async Task AddBlobContainer_ConnectionString_resolved_expected_RunAsEmula | |||
| var blobs = storage.AddBlobs("blob"); | |||
There was a problem hiding this comment.
Can we also update this test to verify blob containers work end-to-end?
There was a problem hiding this comment.
I don't understand, update AddBlobContainer_ConnectionString_resolved_expected_RunAsEmulator or VerifyAzureStorageEmulatorResource. The first one is a unit test, the second is already testing the container client with the emulator.
There was a problem hiding this comment.
Update the VerifyAzureStorageEmulatorResource functional e2e test to also ensure that Hosting builder.AddBlobContainer("container"); and Client side builder.AddAzureBlobContainerClient("container"); work together end-to-end.
|
/backport to release/9.3 |
|
Started backporting to release/9.3: https://github.com/dotnet/aspire/actions/runs/15220833760 |
Description
The
AzureBlobStorageContainerSettingsspecialized class is preventing the distinction betweenConnectionStringandServiceUrifrom happening. In the case of Azure deployment it would then assign a service uri to theConnectionStringproperty and the wrong client constructor was invoked.Fixes #9454
Checklist
<remarks />and<code />elements on your triple slash comments?doc-ideatemplatebreaking-changetemplatediagnostictemplate