spin icon indicating copy to clipboard operation
spin copied to clipboard

feat: Add Azure Key Vault Variable Provider

Open ThorstenHans opened this issue 2 months ago • 2 comments

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

ThorstenHans avatar Apr 25 '24 16:04 ThorstenHans

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

rylev avatar Apr 26 '24 12:04 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.

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?

ThorstenHans avatar Apr 29 '24 07:04 ThorstenHans