AzureStorage auto create blob containers#9008
Conversation
github-actions
bot
added
the
area-integrations
There was a problem hiding this comment.
Pull Request Overview
This PR introduces functionality to automatically create blob containers in Azure Storage and includes relevant updates to test coverage, resource configuration, and provisioning logic. Key changes include:
- Adding tests to verify Bicep generation and functional behavior of blob container creation on an Azure Storage emulator.
- Extending resource and extensions classes (AzureStorageResource, AzureBlobStorageResource, and AzureBlobStorageContainerResource) with new APIs for blob container handling and provisioning.
- Updating the end-to-end playground to reference and wait for the new blob container resource.
Reviewed Changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Hosting.Azure.Tests/AzureStorageExtensionsTests.cs | Added a test verifying Bicep manifest generation for blob container-related resources. |
| tests/Aspire.Hosting.Azure.Tests/AzureStorageEmulatorFunctionalTests.cs | Introduced a new functional test to verify blob container creation and blob upload behavior on an emulator. |
| src/Aspire.Hosting.Azure.Storage/AzureStorageResource.cs | Added a Blobs property for aggregating blob resources and adjusted constant declarations. |
| src/Aspire.Hosting.Azure.Storage/AzureStorageExtensions.cs | Added logic to integrate blob containers into provisioning and new methods for adding and configuring blob container resources. |
| src/Aspire.Hosting.Azure.Storage/AzureStorageEmulatorConnectionString.cs | Adjusted connection string formatting by removing a trailing semicolon. |
| src/Aspire.Hosting.Azure.Storage/AzureBlobStorageResource.cs | Updated connection string composition and added a method for converting to a provisioning entity; note minor comment typo. |
| src/Aspire.Hosting.Azure.Storage/AzureBlobStorageContainerResource.cs | Added a new resource class for Azure Blob Storage container with associated provisioning conversion. |
| playground/AzureStorageEndToEnd/... | Updated end-to-end projects to reference and wait for blob container resources accordingly. |
Files not reviewed (1)
- tests/Aspire.Hosting.Azure.Tests/AzureStorageExtensionsTests.ResourceNamesBicepValid.verified.bicep: Language not supported
Comments suppressed due to low confidence (2)
src/Aspire.Hosting.Azure.Storage/AzureStorageEmulatorConnectionString.cs:15
- Removing the trailing semicolon from the connection string may affect how connection properties are parsed. Please verify that the new format is compatible with downstream consumers expecting a semicolon delimiter.
builder.Append("${key}=http://{endpoint.Property(EndpointProperty.IPV4Host)}:{endpoint.Property(EndpointProperty.Port)}/devstoreaccount1")
src/Aspire.Hosting.Azure.Storage/AzureBlobStorageResource.cs:68
- There is a typo in the comment: 'on;y' should be 'only'.
// We don't inject the queue resource here since we on;y want it to
|
Are you doing the client integration side of this as well? |
src/Aspire.Hosting.Azure.Storage/AzureBlobStorageContainerResource.cs
Outdated
Show resolved
Hide resolved
src/Aspire.Hosting.Azure.Storage/AzureBlobStorageContainerResource.cs
Outdated
Show resolved
Hide resolved
|
For the most part this changeset copies the patterns implemented for AzureServiceBus. I tested it locally in the emulator and deploying to Azure (configuring the Azure subscription details in secrets.json) Emulator
Azurevar builder = DistributedApplication.CreateBuilder(args);
var storage = builder.AddAzureStorage("storage");
var blobs = storage.AddBlobs("blobs");
var blobContainer = blobs.AddBlobContainer("mycontainer");
var queues = storage.AddQueues("queues");
builder.AddProject<Projects.AzureStorageEndToEnd_ApiService>("api")
.WithExternalHttpEndpoints()
.WithReference(blobs).WaitFor(blobs)
.WithReference(blobContainer).WaitFor(blobContainer)
.WithReference(queues).WaitFor(queues);
|
This comment was marked as outdated.
This comment was marked as outdated.
playground/AzureStorageEndToEnd/AzureStorageEndToEnd.AppHost/Program.cs
Outdated
Show resolved
Hide resolved
|
|
||
| partial class AspireBlobStorageExtensions | ||
| { | ||
| private sealed class BlobStorageComponent : AzureComponent<AzureStorageBlobsSettings, BlobServiceClient, BlobClientOptions> |
There was a problem hiding this comment.
Moved this verbatim from AspireBlobStorageExtensions.cs
| /// Azure SDK recommends treating clients as singletons <see href="https://devblogs.microsoft.com/azure-sdk/lifetime-management-and-thread-safety-guarantees-of-azure-sdk-net-clients/"/>, | ||
| /// so this should be the exact same instance used by other parts of the application. | ||
| /// </param> | ||
| private sealed class AzureBlobStorageContainerHealthCheck(BlobContainerClient blobContainerClient) : IHealthCheck |
There was a problem hiding this comment.
There's AzureBlobStorageHealthCheck for BlobServiceClient (comes from https://www.nuget.org/packages/AspNetCore.HealthChecks.Azure.Storage.Blobs). But there are no healthchecks for the containers.
Given that we're creating a blob container, I think checking that the container exists is sufficient.
src/Components/Aspire.Azure.Storage.Blobs/AzureBlobStorageContainerSettings.cs
Outdated
Show resolved
Hide resolved
This comment was marked as outdated.
This comment was marked as outdated.
src/Components/Aspire.Azure.Storage.Blobs/AspireBlobStorageBuilder.cs
Outdated
Show resolved
Hide resolved
|
@eerhardt @sebastienros I added tests but I'm really unsure if or what more tests are needed. Implementation-wise, I think, this is ready. |
|
|
||
| partial class AspireBlobStorageExtensions | ||
| { | ||
| private sealed partial class BlobStorageContainerComponent : AzureComponent<AzureBlobStorageContainerSettings, BlobContainerClient, BlobClientOptions> |
There was a problem hiding this comment.
(nit) if we are going to break these out into their own files, do they need to be nested classes? Why not just make them internal and not nested?
There was a problem hiding this comment.
There are several reasons why I've done this. This style is present in our guidelines (https://github.com/dotnet/runtime/blob/main/docs/coding-guidelines/coding-style.md#example-file); and it's a common practice in the other repos I've been involved with. IMO, it improves the ability to navigate the codebase.
I can put all of these in the same file, if you insist.
But if these types are not nested, then the visibility needs to be increased which, essentially, will break the encapsulation (making the private implementation no longer private). It would also make the implementation inconsistent with the rest.
|
@eerhardt please let me know if there are any tweaks you want to make it and I'll action those as a follow up. |
| /// <param name="name">The name of the resource.</param> | ||
| /// <param name="blobContainerName">The name of the blob container.</param> | ||
| /// <param name="parent">The <see cref="AzureBlobStorageResource"/> that the resource is stored in.</param> | ||
| public class AzureBlobStorageContainerResource(string name, string blobContainerName, AzureBlobStorageResource parent) : Resource(name), |
There was a problem hiding this comment.
Should this implement IResourceWithAzureFunctionsConfig? cc @captainsafia
There was a problem hiding this comment.
My tests (per @captainsafia's guidance) didn't suggest any AF-specific implementations were necessary. As far as my limited knowledge goes, the functions require connections to the storage but not individual components within.
Because it's no longer OSS-friendly. |
Resolves #5167
Description
Checklist
<remarks />and<code />elements on your triple slash comments?doc-ideatemplatebreaking-changetemplatediagnostictemplate