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

Add Azure KeyVault cryptographic keys check #368

Merged

Conversation

Riff451
Copy link

@Riff451 Riff451 commented Dec 16, 2019

Hi, this a proposed PR to slightly extend the already existing HealthChecks.AzureKeyVault created with PR #19 .
The idea is to support the check also for the cryptographic keys that can be stored in KeyVault. I noticed that another user suggested the change in the above PR, but I couldn't find any ongoing work.

Usage:

Default usage with Managed Service Identity:

services.AddHealthChecks()
    .AddAzureKeyVault(setup =>
    {
        setup
            .UseKeyVaultUrl("https://[vaultname].vault.azure.net/")                  
            .AddKey("keyname");
    });

Usage usage client id and client secret:

services.AddHealthChecks()
    .AddAzureKeyVault(setup =>
    {
        setup
            .UseKeyVaultUrl("https://[vaultname].vault.azure.net/")    
            .UseClientSecrets("clientid", "clientsecret")
            .AddKey("keyname");
    });

Thanks.

@CarlosLanderas
Copy link
Contributor

Thanks @Riff451, let me test it and I'll merge it

@CarlosLanderas
Copy link
Contributor

Tested and working. Thanks for the PR @Riff451

@CarlosLanderas CarlosLanderas merged commit 17961d8 into Xabaril:master Dec 17, 2019
@CarlosLanderas
Copy link
Contributor

Adding certificates feature as well:

#369

@Riff451
Copy link
Author

Riff451 commented Dec 17, 2019

Thanks @CarlosLanderas for the quick review and for the new certificates feature as well :)

@sungam3r sungam3r mentioned this pull request Jul 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants