feat: Add Azure Key Vault Variable Provider

[ThorstenHans]

This PR adds Azure Key Vault as Variable provider.

Users can provide a runtime config using necessary information for Client Credential Flow:

[[config_provider]]
type = "azure_key_vault"
vault_url = "https://some.vault.azure.net/"
client_id = "11111111-1111-1111-1111-111111111111"
client_secret = "11111111-1111-1111-1111-111111111111"
tenant_id = "11111111-1111-1111-1111-111111111111"
# authority_host = "AzurePublicCloud"

If not specified, authority_host will default to AzurePublicCloud.

Unfortunately, I wasn't able to run make lint and make test on my machine. Both commands ran into errors shown below.

Am I missing something on my machine?

Output from make lint

    Checking spin-expressions v2.5.0-pre0 (/Users/thorsten/dev/thorstenhans/spin/crates/expressions)
error: you are explicitly cloning with `.map()`
  --> crates/plugins/src/badger/store.rs:89:26
   |
89 |             badgered_to: to.iter().map(|v| <semver::Version>::clone(v)).collect(),
   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `cloned` method: `to.iter().cloned()`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_clone
   = note: `-D clippy::map-clone` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(clippy::map_clone)]`

    Checking spin-outbound-networking v2.5.0-pre0 (/Users/thorsten/dev/thorstenhans/spin/crates/outbound-networking)
    Checking spin-doctor v2.5.0-pre0 (/Users/thorsten/dev/thorstenhans/spin/crates/doctor)
    Checking spin-build v2.5.0-pre0 (/Users/thorsten/dev/thorstenhans/spin/crates/build)
    Checking spin-templates v2.5.0-pre0 (/Users/thorsten/dev/thorstenhans/spin/crates/templates)
error: could not compile `spin-plugins` (lib) due to 1 previous error
warning: build failed, waiting for other jobs to finish...
error: could not compile `spin-plugins` (lib test) due to 1 previous error
make: *** [lint] Error 101

Output from make test

Checking spin-key-value v2.5.0-pre0 (/Users/thorsten/dev/thorstenhans/spin/crates/key-value)
error: you are explicitly cloning with `.map()`
  --> crates/plugins/src/badger/store.rs:89:26
   |
89 |             badgered_to: to.iter().map(|v| <semver::Version>::clone(v)).collect(),
   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `cloned` method: `to.iter().cloned()`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_clone
   = note: `-D clippy::map-clone` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(clippy::map_clone)]`

    Checking outbound-http v2.5.0-pre0 (/Users/thorsten/dev/thorstenhans/spin/crates/outbound-http)
error: could not compile `spin-plugins` (lib) due to 1 previous error
warning: build failed, waiting for other jobs to finish...
error: could not compile `spin-plugins` (lib test) due to 1 previous error
error: field `0` is never read
   --> crates/templates/src/template.rs:649:21
    |
649 |     struct TempFile(tempfile::TempDir, PathBuf);
    |            -------- ^^^^^^^^^^^^^^^^^
    |            |
    |            field in this struct
    |
    = note: `-D dead-code` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(dead_code)]`
help: consider changing the field to be of unit type to suppress this warning while preserving the field numbering, or remove the field
    |
649 |     struct TempFile((), PathBuf);
    |                     ~~

error: could not compile `spin-templates` (lib test) due to 1 previous error
make: *** [lint] Error 101

[rylev]

If you rebase against the latest main branch, the errors you are seeing should be fixed. They were address here.

[rylev]

Looks great overall! I did not actually run the code, but it looks pretty straight forward to me. I think we can merge this. The only points I had were nitpicks.

[ThorstenHans]

Looks great overall! I did not actually run the code, but it looks pretty straight forward to me. I think we can merge this. The only points I had were nitpicks.

Awesome feedback @rylev I refactored the PR as part of 0a3e792

When implementing FromStr, I ended-up using bail! to fail early. Is that considered a valid practice here, or is there a better alternative?

[rylev]

I'm confused actually - what are the FromStr and From<String> impls for AzureAuthorityHost used for? It seems like they could just be deleted.

[ThorstenHans]

Hmm, it's used in the AzureKeyVaultProvider::new function, to construct AzureAuthorityHost from raw strings provided as part of runtime config file

[rylev]

Hmm... AzureKeyVaultProvider::new only seems to be called in AzureKeyVaultVariablesProviderOpts::build_provider which passes a AzureAuthorityHost directly (not a string representation). That means that the only valid AzureAuthorityHost string representations in the runtime config file is whatever matches AzureAuthorityHost's deserialize implementation which is just a one-to-one mapping with the Rust identifier.

In other words, the runtime config will only accept the string "AzurePublicCloud" for the public cloud option. Other options like "public" and "publiccloud" won't work. Are you seeing something different in testing?

[ThorstenHans]

Makes sense. I tried without FromStr and From<String>. When users provide a wrong value for AzureAuthorityHost a corresponding error is presented and showing allowed values to the user - which is good.

I rebased on most recent main and pushed the change once again