Skip to content
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

The optional:true parameter for AddAzureAppConfiguration does not work #136

Closed
zhenlan opened this issue Mar 3, 2020 · 7 comments · Fixed by #137
Closed

The optional:true parameter for AddAzureAppConfiguration does not work #136

zhenlan opened this issue Mar 3, 2020 · 7 comments · Fixed by #137
Assignees
Labels
bug Something isn't working
Milestone

Comments

@zhenlan
Copy link
Contributor

zhenlan commented Mar 3, 2020

I have code below and a key vault reference in the store.

builder.AddAzureAppConfiguration(options =>
{
    options.Connect(configuration["ConnectionString"])
}, optional:true);

The app does not have access to my key vault, but since I specified optional:true, the app is not expected to throw. However, it does:

Unhandled exception. Microsoft.Extensions.Configuration.AzureAppConfiguration.KeyVaultReferenceException: No key vault credential configured and no matching secret client could be found.. ErrorCode:, Key:foo, Label:, Etag:oyI5kwp9Dp68pxoux30v7G6lVQJ, SecretIdentifier:https://keyvaultreference.vault.azure.net/secrets/MySecret
 ---> System.UnauthorizedAccessException: No key vault credential configured and no matching secret client could be found.
   at Microsoft.Extensions.Configuration.AzureAppConfiguration.AzureKeyVault.AzureKeyVaultSecretProvider.GetSecretValue(Uri secretUri, CancellationToken cancellationToken)
   at Microsoft.Extensions.Configuration.AzureAppConfiguration.AzureKeyVault.AzureKeyVaultKeyValueAdapter.ProcessKeyValue(ConfigurationSetting setting, CancellationToken cancellationToken)
   --- End of inner exception stack trace ---
   at Microsoft.Extensions.Configuration.AzureAppConfiguration.AzureKeyVault.AzureKeyVaultKeyValueAdapter.ProcessKeyValue(ConfigurationSetting setting, CancellationToken cancellationToken)
   at Microsoft.Extensions.Configuration.AzureAppConfiguration.AzureAppConfigurationProvider.ProcessAdapters(ConfigurationSetting setting, CancellationToken cancellationToken)
   at Microsoft.Extensions.Configuration.AzureAppConfiguration.AzureAppConfigurationProvider.SetData(IDictionary`2 data, CancellationToken cancellationToken)
   at Microsoft.Extensions.Configuration.AzureAppConfiguration.AzureAppConfigurationProvider.LoadAll()
   at Microsoft.Extensions.Configuration.AzureAppConfiguration.AzureAppConfigurationProvider.Load()
   at Microsoft.Extensions.Configuration.ConfigurationRoot..ctor(IList`1 providers)
   at Microsoft.Extensions.Configuration.ConfigurationBuilder.Build()
   at Microsoft.Extensions.Configuration.AzureAppConfiguration.Examples.ConsoleApplication.Program.Configure()
   at Microsoft.Extensions.Configuration.AzureAppConfiguration.Examples.ConsoleApplication.Program.Main(String[] args)
@zhenlan zhenlan added the bug Something isn't working label Mar 3, 2020
@abhilasharora abhilasharora changed the title The optional:true parameter is not honored when failing to load key vault reference The optional:true parameter for AddAzureAppConfiguration does not work Mar 4, 2020
@abhilasharora
Copy link
Contributor

@zhenlan It looks like the optional: true parameter also does not work for other exceptions such as RequestFailedException, AggregateException etc. I have updated the title to reflect it. I will test out the different cases and send a fix.

@abhilasharora
Copy link
Contributor

The fix for this issue has been merged as a part of pull request #137.

@abhilasharora abhilasharora linked a pull request Mar 4, 2020 that will close this issue
@abhilasharora
Copy link
Contributor

The fix has been released as a part of the version 3.0.1 of the package.
https://www.nuget.org/packages/Microsoft.Extensions.Configuration.AzureAppConfiguration/3.0.1

@eetawil
Copy link

eetawil commented Apr 17, 2020

Are you sure it's fixed? I have the same problem as reported by @zhenlan with 3.0.1
==> Microsoft.Extensions.Configuration.AzureAppConfiguration.KeyVaultReferenceException

return new ConfigurationBuilder()
.AddAzureAppConfiguration(
options =>
{
options.Connect(connectionString)
.Select(KeyFilter.Any)
.Select(KeyFilter.Any, "label1")
.Select(KeyFilter.Any, "label2");
}
), optional:true
.Build();

@abhilasharora
Copy link
Contributor

@eetawil Thanks for reporting it. You're right, the change fixed the issue for other exceptions, but not for the KeyVaultReferenceException that was reported in the original issue. This will be fixed in the next release of the package.

@ghost
Copy link

ghost commented Sep 18, 2020

@abhilasharora
I see that the latest version 4.0 ignores exceptions thrown from keyvault. However, I think I may have misunderstood the use case here. From what I can see, right now the whole configuration is seen as optional, meaning that if at least one keyvault exception is thrown, no configuration values are loaded.

I had hoped that this would cover our use case, which is ignoring only configuration values from keyvault that are inaccessible (e.g. due to no access permissions), instead of ignoring all configuration values if one can't be loaded from keyvault.

Basically, we have hooked up multiple keyvaults in one app configuration, but not all apps have access to all keyvaults that are referenced there. So right now we can't load any configuration if we don't allow access to all keyvaults from all our apps.

In preview version 2, I could still create a custom keyvault delegating handler that ignores 401, 403 and 404 errors and return an empty string, but I also can't seem to find this option anymore in version 4.

Is there any way to support this use case?

@abhilasharora
Copy link
Contributor

@b10-dslappendel Thanks for your feedback. This issue tracks the ability to use optional: true to ignore all exceptions from this configuration source. I have created issue #203 to track your use case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants