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

Issues with VersionTag in AzureKeyVaultConfigBuilder #20

Open
rahulpnath opened this Issue Jun 15, 2018 · 4 comments

Comments

Projects
None yet
2 participants
@rahulpnath
Contributor

rahulpnath commented Jun 15, 2018

When specifying the version identifier, config builder replaces only the specific secret value and does not replace other secrets from Key Vault.

Repro Steps

  1. Setup a sample project using AzureKeyVaultConfigBuilder
  2. Create multiple Secrets in the Key Vault. (MySecret and VersionedSecret). Create two versions for VersionedSecret
  3. Specify versionTag in KeyVaultConfigBuilder to be the version id of VersiondSecret.
  4. Run the application, and only VersionedSecret gets replaced. Value for MySecret does not get replaced

Further technical details

At the moment the version tag is specified along with the setting up of AzureKeyVault ConfigBuilder

version - Azure Key Vault provides a versioning feature for secrets. If this is specified, the builder will only retrieve secrets matching this version.

In Key Vault the Version is specific to an object (Key, Secret or Certificate). So when you have multiple Secrets and create multiple versions for each of them, each gets a different version identifier. So specifying the version tag at the config builder level does not make sense. It should be part of specifying the key under appSettings/connectionStrings.

The issue needs some discussion on how and where the version needs to be specified. Happy to discuss further.

@rahulpnath

This comment has been minimized.

Contributor

rahulpnath commented Jun 16, 2018

I can think of some options here

  1. Specify the version along with the key
    <add key="VersionedSecret/bc55d5cf01ba4567af470d4626bc6377" value="dummy2" />

Passing in version identifier like this does work when preload is set to false because the keys dictionary does not get pre-populated which makes individual key requests to go through and the URL formed is similar to the one as if we passed name and version separately.
But this does not feel like a good option as it would fail when preload is enabled, and also it does hinder usability

  • In code, we will need to start referring to the config key along with the identifier, which makes it no longer a configurable value.
  • This also does not work well with Release Management tools (like Octopus Deploy, VSTS) variable replacement features. We might have different version identifiers in different environments (vaults)
  1. Specify the version identifier as part of the value
    <add key="VersionedSecret" value="version:bc55d5cf01ba4567af470d4626bc6377" />

If we can come up with some version identifier ('version:') to be used along with the identifier and specify that as part of the value. But at present, the GetValue calls only get the key and not the value. Not sure if it will be possible/make sense to pass in the value as well to the abstract methods.

I might be missing out on some things here though, so happy to discuss this further :)

@rahulpnath

This comment has been minimized.

Contributor

rahulpnath commented Jun 22, 2018

@StephenMolloy Any thoughts on this (and on the other issues as well)?

@StephenMolloy

This comment has been minimized.

Contributor

StephenMolloy commented Jun 29, 2018

I would favor the first approach. However, it does have problems as you mentioned. I don't see an easy fix here for the impending 1.0.2 release, but I've got some thoughts for a potential 1.1 release that could address this situation.

@rahulpnath

This comment has been minimized.

Contributor

rahulpnath commented Jun 29, 2018

@StephenMolloy It would be related to how we fix #27 as well. Looks like that might need a way where we have VaultName/Uri as part of the configuration and the VaultConfigurationBuilder can read the default values. Is that a much larger change to make the config value also to be passed in along with the key? (TBH I have not looked into the code where that happens, Will look it up some time this week assuming that is part of this same repository)

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