OrchardCore icon indicating copy to clipboard operation
OrchardCore copied to clipboard

Invalid Uri error with version 1.8.2

Open vmahant opened this issue 1 year ago • 9 comments

Describe the bug

To Reproduce

Steps to reproduce the behaviour:

  1. upgrade to orchard core 1.7.2 to 1.8.2 as per the documentation

  2. connection method is azure key vault refer https://docs.orchardcore.net/en/latest/docs/reference/core/KeyVault.Azure/#configuration

  3. See error

Unhandled exception. System.UriFormatException: Invalid URI: The hostname could not be parsed. at System.Uri.CreateThis(String uri, Boolean dontEscape, UriKind uriKind, UriCreationOptions& creationOptions) at System.Uri..ctor(String uriString) at OrchardCore.Configuration.KeyVault.Extensions.AzureKeyVaultConfigurationExtension.AddOrchardCoreAzureKeyVault(IConfigurationBuilder builder, IConfiguration configuration, TokenCredential tokenCredential) at OrchardCore.Configuration.KeyVault.Extensions.AzureKeyVaultConfigurationExtension.<>c__DisplayClass0_0.<AddOrchardCoreAzureKeyVault>b__0(HostBuilderContext context, IConfigurationBuilder builder) at [Microsoft.Extensions.Hosting](http://microsoft.extensions.hosting/).HostBuilder.InitializeAppConfiguration()

Expected behaviour

Orchard core 1.8.2 should run successfully with azure key vault connection

Screenshots

image

vmahant avatar Jan 24 '24 16:01 vmahant

@vmahant the only way you would encounter this exception if you did not provide a value in KeyVaultName in your configuration.

Can you please check you settings. I'll submit a PR to provide a detailed exception. But you need to provide KeyVaultName for this to work.

MikeAlhayek avatar Jan 26 '24 21:01 MikeAlhayek

I have also encountered this issue when upgrading to 1.8.2. I had the KeyVaultName setting already configured and working prior to the update.

It looks like an issue may have been introduced by this change #14550 for applications that are using the older dotnet core 3.x/5 builder to bootstrap the application. Which is shown in the documentation

using OrchardCore.Configuration.KeyVault.Extensions;

public class Program
{
    public static Task Main(string[] args)
        => BuildHost(args).RunAsync();

    public static IHost BuildHost(string[] args) =>
        Host.CreateDefaultBuilder(args)
            .AddOrchardCoreAzureKeyVault()
            .ConfigureWebHostDefaults(webBuilder => webBuilder.UseStartup<Startup>())
            .Build();
}

https://docs.orchardcore.net/en/main/docs/reference/core/KeyVault.Azure/#configuration

I have since refactored my Program.cs to use the newer WebApplicationBuilder which has resolved the issue for me.

var builder = WebApplication.CreateBuilder(args);

builder.Host.AddOrchardCoreAzureKeyVault();

// Omitted for simplicity

var app = builder.Build();

// Omitted for simplicity

await app.RunAsync();

I am happy with the refactor of the Program.cs as a fix for me but I believe the older generic hosting model is still supported and is provided as the example in the documentation so an investigation into a fix or updated documentation may be appropriate?

@MikeAlhayek Unless I am missing something obvious would it be possible to reopen this issue?

From what I have been able to see when using the older generic hosting model the builder context doesn't include the appsettings configurations (or they aren't available yet) which causes an issue when creating the URI for KeyVault as the OrchardCore:OrchardCore_KeyVault_Azure:KeyVaultName is not in the IConfiguration used by AddOrchardCoreAzureKeyVault so results in a null KeyVaultName

buzzjames avatar Apr 04 '24 21:04 buzzjames

@buzzjames I reopen the issue. would you be interested in submitting a PR with the fix?

MikeAlhayek avatar Apr 04 '24 23:04 MikeAlhayek

@MikeAlhayek Happy to help where I can but if i am honest I don't know what the fix should be.

From what I have been able to see it might not be be possible to use the current AddOrchardCoreAzureKeyVault extension with the callback style generic host builder without relying on building a temporary configuration which it was doing before #14550.

I think it might be possible to load the appsetting manually so that they are available for AddOrchardCoreAzureKeyVault but that feels like a more complicated way building a temporary instance of the configuration

Perhaps the best option is to include a fail safe i.e. attempt to retrieve the 'KeyVaultName' from the context if that is null build a temporary version of the configuration and retrieve the value from that if not found after that it can fail.

@jtkech Having had previous experience with this do you have any ideas / suggestions on the best route forward?

buzzjames avatar Apr 05 '24 10:04 buzzjames

@buzzjames I don't have time at the moment to look into this. Maybe someone else can look @hishamco if you have time.

A sad note about @jtkech can be found in #14954

MikeAlhayek avatar Apr 05 '24 15:04 MikeAlhayek

I am very sorry to hear of Jean-Thierry passing. I haven't been a active member of Orchard Core long but reading the comments in the thread I can see that he was highly regarded member and one who will be missed. Very sad news.

No worries, we have a work around that suits us for now. I am more than happy to contribute and submit a PR but I think it would be good to get a new perspective on it if someone is available. If I get time to dive into it some more / have any new ideas will update here.

buzzjames avatar Apr 08 '24 10:04 buzzjames

+1 This is still an issue

NickSomsen avatar May 22 '25 12:05 NickSomsen

For Azure Key Vault, I might ping @Piedone or @BenedekFarkas because I don't work with Azure extensively

hishamco avatar May 22 '25 20:05 hishamco

We don't really work with Key Vault either.

Piedone avatar May 22 '25 20:05 Piedone