Adding Managed Identity for Access Token #48
Conversation
|
Hi! Thank you for your contribution! ✨ |
| } | ||
|
|
||
| """ | ||
| @spec smart_connect() :: Client.t() | {:error, any} |
There was a problem hiding this comment.
This looks out of scope for this feature. Could you move this piece to a separate PR for further discussion?
| Client.t() | {:error, any} | ||
| def msi_connect(vault_name \\ nil) do | ||
| vault_name = get_env(:azure_vault_name, vault_name) | ||
| endpoint = System.get_env("IDENTITY_ENDPOINT") |
There was a problem hiding this comment.
I'd suggest you move the IDENTITY_ENDPOINT and IDENTITY_HEADER env variables to the config files for each environment. This way you can leverage the get_env() function for them as well.
| @@ -91,6 +194,17 @@ defmodule ExAzureKeyVault.Client do | |||
| } | |||
|
|
|||
| """ | |||
| @spec connect!() :: Client.t() | {:error, any} | |||
There was a problem hiding this comment.
This seems out of scope as well. Also, although I understand the motivation for the ! function, the current connect() already raises exceptions.
Please open a PR or issue with this suggestion if you want to discuss more.
| @@ -151,6 +265,28 @@ defmodule ExAzureKeyVault.Client do | |||
| } | |||
|
|
|||
| """ | |||
| @spec cert_connect!() :: Client.t() | {:error, any} | |||
There was a problem hiding this comment.
Same as connect!(). I'm happy to discuss more on a separate issue/PR.
|
This project is configured to expect 100% coverage. Could you include some tests? |
I added code to support managed identity authentication.
I have not tested this! Dont complete right away - Can you walk me through the process for testing the changes and/or any patterns that I did that don't align to yours?