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

[FEATURE REQ] - Azure Keyvault AddAzureKeyVault.ReloadInterval needs to raise an Event #11671

Closed
JohnGalt1717 opened this issue Apr 29, 2020 · 7 comments
Assignees
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. Extensions ASP.NET Core extensions needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that

Comments

@JohnGalt1717
Copy link

Library or service name.
Microsoft.Extensions.Configuration.AzureKeyVault

Is your feature request related to a problem? Please describe.
Recently the ReloadInterval property was added which is great, but it doesn't go far enough because we have no way of knowing that it occured and respond and update the strongly bound configuration singletons etc in our DI.

What feature would you like to get added? What problem is it solving?

Please add an event to the AddAzureKeyVault method that allows us to specify a function/lambda that can reload these singletons as a result of a reload from Azure.

@ghost ghost added needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. customer-reported Issues that are reported by GitHub users external to the Azure organization. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Apr 29, 2020
@jsquire jsquire added Client This issue points to a problem in the data-plane of the library. Extensions ASP.NET Core extensions feature-request This issue requires a new behavior in the product in order be resolved. labels Apr 29, 2020
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Apr 29, 2020
@jsquire jsquire added needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team and removed question The issue doesn't require a change to the product in order to be resolved. Most issues start as that needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. labels Apr 29, 2020
@pakrym pakrym added question The issue doesn't require a change to the product in order to be resolved. Most issues start as that and removed feature-request This issue requires a new behavior in the product in order be resolved. labels Apr 29, 2020
@JohnGalt1717
Copy link
Author

Thanks. Bizzarely complex, but it works! Thanks!

openapi-sdkautomation bot pushed a commit to AzureSDKAutomation/azure-sdk-for-net that referenced this issue Nov 18, 2020
20201113 streamanalytics (Azure#11671)

* sdk-no-flatten

* use unique jobName is examples
@ezgambac
Copy link

@JohnGalt1717 Could you share how you configured azure keyvault configuration provider to reload stuff on change?

@JohnGalt1717
Copy link
Author

Here's my complete implementation:

		public static void ApplyKeyVaultToConfig(IConfigurationBuilder config)
		{
			var builtConfig = config.Build();
			var connectionStrings = ConnectionStrings.GetSettings(builtConfig);
			if (connectionStrings.AzureKeyVault == null)
				return;


			var client = connectionStrings.AzureKeyVault.GetSecretClient();

			var options = new AzureKeyVaultConfigurationOptions
			{
				ReloadInterval = TimeSpan.FromMinutes(5),
			};

			//HACK: This is here until microsoft gives us a better way to make sure that Azure Keyvault doesn't override environment variables etc.
			var tKeyVaultSource = options.GetType().Assembly.GetType("Azure.Extensions.AspNetCore.Configuration.Secrets.AzureKeyVaultConfigurationSource")!;

			var source = Activator.CreateInstance(tKeyVaultSource, new object[] { client, options }) as IConfigurationSource;

			config.Sources.Insert(0, source);
		}

ReloadInterval = is the key to it just doing it automatically from what I can tell. In the past I had to do WAY more work to get it to reload using the old library.

@ezgambac
Copy link

Why are these needed?

var tKeyVaultSource = options.GetType().Assembly.GetType("Azure.Extensions.AspNetCore.Configuration.Secrets.AzureKeyVaultConfigurationSource")!;

var source = Activator.CreateInstance(tKeyVaultSource, new object[] { client, options }) as IConfigurationSource;

config.Sources.Insert(0, source);

@JohnGalt1717
Copy link
Author

JohnGalt1717 commented Jul 23, 2021

That puts Azure Keyvault first instead of last in the order of configuration providers because literally everything else should come after it for overriding your keyvault. If you don't do this, then command line, environment variables and even .config files will be ignored.

Typically you'll want to have specific settings in environment variables in docker as an example that override certain values per environment. (and Microsoft made the class you need internal so you have to use reflection to get at it)

@ezgambac
Copy link

Thanks @JohnGalt1717 !

@github-actions github-actions bot locked and limited conversation to collaborators Mar 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. Extensions ASP.NET Core extensions needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that
Projects
None yet
Development

No branches or pull requests

4 participants