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.

Sign up for GitHub

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

Jump to bottom

Added BlobFileStoreFactory to allow DefaultAzureCredential or Connectionstring authentication for Azure Blob storage #12874

Closed

rikbosch wants to merge 3 commits into OrchardCMS:main from rikbosch:features/add-defaultazurecredential-support-to-blobfile-storage

+128 −16

rikbosch commented Nov 23, 2022 •

edited by agriffard

Loading

Fixes #12639

This PR introduces a BlobFileStoreFactory which is used by the OrchardCore.Shells.Azure and OrchardCore.Media.Azure module.

It allows to specify a reference to a configured BlobServiceClient through the Microsoft.Extensions.Azure nuget package.

see https://learn.microsoft.com/en-us/dotnet/azure/sdk/dependency-injection#configure-multiple-service-clients-with-different-names about configuring multiple blob clients and also: https://devblogs.microsoft.com/azure-sdk/best-practices-for-using-azure-sdk-with-asp-net-core/ for best-practices

for example:

program.cs

builder.Services.AddAzureClients(clientBuilder =>
{
    clientBuilder.AddBlobServiceClient(builder.Configuration.GetSection("PublicStorage"));
    clientBuilder.AddBlobServiceClient(builder.Configuration.GetSection("PrivateStorage"))
        .WithName("PrivateStorage");
});

appsettings.json

{
  "OrchardCore": {
    "OrchardCore_Shells_Azure": {
      "BlobServiceName": "PrivateStorage", //<<< ! this is where the magic happens
      "ContainerName": "a-container-name", 
      "BasePath": "shells"
    }
  }
}


          fixes OrchardCMS#12639 added BlobFileStoreFactory

a3b198c

This allows to use registered clients from Azure.Configuration.Extensions  (and DefaultAzureCredential )or
specify a connectionstring

rikbosch changed the title ~~fixes #12639 added BlobFileStoreFactory to allow DefaultAzureCredential or Connectionstring authentication for Azure Blob storage~~ Fixes #12639: Added BlobFileStoreFactory to allow DefaultAzureCredential or Connectionstring authentication for Azure Blob storage

rikbosch added 2 commits

November 24, 2022 09:10


          Add fallback to Default storage for simplicity

c596910


          register the AzureBlobFileStorage components on the ApplicationServic…

e3b0afd

…es collection and not on each tenant.

agriffard changed the title ~~Fixes #12639: Added BlobFileStoreFactory to allow DefaultAzureCredential or Connectionstring authentication for Azure Blob storage~~ Added BlobFileStoreFactory to allow DefaultAzureCredential or Connectionstring authentication for Azure Blob storage

MikeAlhayek reviewed

View reviewed changes

src/OrchardCore/OrchardCore.FileStorage.AzureBlob/BlobFileStoreFactory.cs

+              using Microsoft.Extensions.DependencyInjection;
+              using OrchardCore.Modules;
+              namespace OrchardCore.FileStorage.AzureBlob;

Member

MikeAlhayek Feb 4, 2023

Add new line after the namespace.

src/OrchardCore/OrchardCore.FileStorage.AzureBlob/BlobFileStoreFactory.cs

+                  private readonly IAzureClientFactory<BlobServiceClient> _clientFactory;
+                  private readonly IServiceProvider _serviceProvider;
+                  private const string DefaultStorageName = "Default";

Member

MikeAlhayek Feb 4, 2023

rename the variable to _defaultStorageName

src/OrchardCore/OrchardCore.FileStorage.AzureBlob/BlobFileStoreFactory.cs

+                  private BlobContainerClient CreateContainerClientFromOptions(BlobStorageOptions options)
+                  {
+                      // return a container client from connectionstring, if specified

Member

MikeAlhayek Feb 4, 2023

Suggested change

      
                    // return a container client from connectionstring, if specified
          
                    // Create a container using the specified connection string.

src/OrchardCore/OrchardCore.FileStorage.AzureBlob/BlobFileStoreFactory.cs

+                          return new BlobContainerClient(options.ConnectionString, options.ContainerName);
+                      }
+                      // else use the clientFactory to lookup a BlobService with the specified name

Member

MikeAlhayek Feb 4, 2023

Suggested change

      
                    // else use the clientFactory to lookup a BlobService with the specified name
          
                    // Create a container using the specified Blob service.

src/OrchardCore/OrchardCore.FileStorage.AzureBlob/BlobFileStoreFactory.cs

+                              .GetBlobContainerClient(options.ContainerName);
+                      }
+                      // fall-back to the default registered blob storage

Member

MikeAlhayek Feb 4, 2023

Suggested change

      
                    // fall-back to the default registered blob storage
          
                    // Create a container using the Default client.

src/OrchardCore/OrchardCore.Shells.Azure/Extensions/BlobShellsOrchardCoreBuilderExtensions.cs

		@@ -24,6 +24,9 @@ public static OrchardCoreBuilder AddAzureShellsConfiguration(this OrchardCoreBui

		services.TryAddSingleton<IContentTypeProvider, FileExtensionContentTypeProvider>();

		// register the azure blob file storage services

Member

MikeAlhayek Feb 4, 2023

I would just remove this comment.

...ardCore/OrchardCore.FileStorage.AzureBlob/AzureBlobFileStorageServiceCollectionExtensions.cs

+                      // always use TryXXX methods because this method can be called multiple times
+                      // by different modules (currently: Azure Media and Azure Shells module)
+                      services.TryAddSingleton<BlobFileStoreFactory>();
+                      return services;

Member

MikeAlhayek Feb 4, 2023

New line before return

Then either remove the comment or change the comment block to something like this

// TryAddSingleton is used to prevent registering the service again.

...ardCore/OrchardCore.FileStorage.AzureBlob/AzureBlobFileStorageServiceCollectionExtensions.cs

+              {
+                  /// <summary>
+                  /// Registers the BlobFileStorage services in the ServiceCollection.
+                  /// Note: this method can be called multiple times

Member

MikeAlhayek Feb 4, 2023

I would remove the comments here.

...rdCore/OrchardCore.FileStorage.AzureBlob/AzureBlobFileStorageOrchardCoreBuilderExtensions.cs

+                  /// <returns></returns>
+                  public static OrchardCoreBuilder AddAzureBlobFileStorage(this OrchardCoreBuilder builder)
+                  {
+                      builder.ApplicationServices.AddAzureBlobFileStorage();

Member

MikeAlhayek Feb 4, 2023

new line before return.

src/OrchardCore.Modules/OrchardCore.Media.Azure/Startup.cs

                               services.AddSingleton<IMediaFileStoreCache>(serviceProvider =>
                                   serviceProvider.GetRequiredService<IMediaFileStoreCacheFileProvider>());
+                              // Register the BlobFileStorageFactory

Member

MikeAlhayek Feb 4, 2023

Comment isn't needed here

Member

MikeAlhayek commented Feb 4, 2023

Additionally, please

Fix conflict.
Update the appsettings.json file to add BlobServiceName in there.
Update the docs file with the update setting variable.

Contributor

rjpowers10 commented Sep 5, 2023

It looks like the trail has gone cold on this. Just commenting to say that I'm looking for a way to use Azure Managed Identity for OrchardCore.Shells.Azure and OrchardCore.Media.Azure and this PR seems to be exactly what I'm looking for.

github-actions bot added the merge conflict label

Contributor

github-actions bot commented Jan 29, 2024

This pull request has merge conflicts. Please resolve those before requesting a review.

Member

Piedone commented Mar 14, 2024

Is this something you'd like to revisit any time soon @rikbosch or should we close?

rjpowers10 mentioned this pull request

Add option to use DefaultAzureCredential for Azure BlobFileStore #15663

Closed

Member

Piedone commented Apr 8, 2024

Would you like to perhaps take over this, @mariojsnunes?

Contributor

mariojsnunes commented Apr 8, 2024

Yes, I'll look into it towards the end of the week

Member

Piedone commented Apr 15, 2024

Superseded by #15753.

Piedone closed this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels