-
Notifications
You must be signed in to change notification settings - Fork 804
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
Azure Key Vault Health check package and Unit tests project #19
Changes from 5 commits
20c496b
9efb7a1
36d8a9d
3a2828e
560b44a
3ceb2c6
6e95b81
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
using Microsoft.Azure.KeyVault; | ||
using Microsoft.Extensions.Diagnostics.HealthChecks; | ||
using Microsoft.IdentityModel.Clients.ActiveDirectory; | ||
using System; | ||
using System.Collections.Generic; | ||
using System.Net.Http; | ||
using System.Text; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
|
||
namespace HealthChecks.AzureKeyVault | ||
{ | ||
public class AzureKeyVaultHealthCheck : IHealthCheck | ||
{ | ||
private readonly AzureKeyVaultOptions _keyVaultOptions; | ||
|
||
public AzureKeyVaultHealthCheck(AzureKeyVaultOptions keyVaultOptions) | ||
{ | ||
if (string.IsNullOrEmpty(keyVaultOptions.KeyVaultUrlBase)) throw new ArgumentNullException(keyVaultOptions.KeyVaultUrlBase); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please add backets |
||
if (string.IsNullOrEmpty(keyVaultOptions.ClientId)) throw new ArgumentNullException(keyVaultOptions.ClientId); | ||
if (string.IsNullOrEmpty(keyVaultOptions.ClientSecret)) throw new ArgumentNullException(keyVaultOptions.ClientSecret); | ||
|
||
_keyVaultOptions = keyVaultOptions; | ||
} | ||
public async Task<HealthCheckResult> CheckHealthAsync(HealthCheckContext context, CancellationToken cancellationToken = default) | ||
{ | ||
string currentSecret = string.Empty; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. var |
||
|
||
try | ||
{ | ||
var client = new KeyVaultClient(GetToken); | ||
foreach (var secretIdentifier in _keyVaultOptions.Secrets) | ||
{ | ||
currentSecret = secretIdentifier; | ||
await client.GetSecretAsync(_keyVaultOptions.KeyVaultUrlBase, secretIdentifier, cancellationToken); | ||
} | ||
|
||
return HealthCheckResult.Healthy(); | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove blank lines |
||
} | ||
catch (Exception ex) | ||
{ | ||
var secretException = new Exception($"{currentSecret} secret error - {ex.Message}", ex); | ||
return new HealthCheckResult(context.Registration.FailureStatus, exception: secretException); | ||
} | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. blank lines |
||
public async Task<string> GetToken(string authority, string resource, string scope) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what about Azure MSI? For Azure hosted applications KeyVaultClient can be created with AzureServiceTokenProvider and you don't need get token or specify clientId and client secret. Add support for Azure MSI var azureServiceTokenProvider = new AzureServiceTokenProvider(); |
||
{ | ||
var authContext = new AuthenticationContext(authority); | ||
ClientCredential clientCred = new ClientCredential(_keyVaultOptions.ClientId, _keyVaultOptions.ClientSecret); | ||
AuthenticationResult result = await authContext.AcquireTokenAsync(resource, clientCred); | ||
|
||
if (result == null) | ||
throw new InvalidOperationException($"[{nameof(AzureKeyVaultHealthCheck)}] - Failed to obtain the JWT token"); | ||
|
||
return result.AccessToken; | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
using System; | ||
using System.Collections.Generic; | ||
using System.Text; | ||
|
||
namespace HealthChecks.AzureKeyVault | ||
{ | ||
public class AzureKeyVaultOptions | ||
{ | ||
internal List<string> Secrets { get; } = new List<string>(); | ||
/// <summary> | ||
/// Azure Key Vault base url - https://[vaultname].vault.azure.net/ | ||
/// </summary> | ||
public string KeyVaultUrlBase { get; set; } | ||
/// <summary> | ||
/// Registered application Id | ||
/// </summary> | ||
public string ClientId { get; set; } | ||
/// <summary> | ||
/// Registered application secret | ||
/// </summary> | ||
public string ClientSecret { get; set; } | ||
|
||
/// <summary> | ||
/// Add a Azure Key Vault secret to be checked | ||
/// </summary> | ||
/// <param name="secretIdentifier"></param> | ||
/// <returns></returns> | ||
public AzureKeyVaultOptions AddSecret(string secretIdentifier) | ||
{ | ||
if(!Secrets.Contains(secretIdentifier)) | ||
{ | ||
Secrets.Add(secretIdentifier); | ||
} | ||
|
||
return this; | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
using HealthChecks.AzureKeyVault; | ||
using Microsoft.Extensions.DependencyInjection; | ||
using Microsoft.Extensions.Diagnostics.HealthChecks; | ||
using System; | ||
using System.Collections.Generic; | ||
using System.Text; | ||
|
||
namespace Microsoft.Extensions.DependencyInjection | ||
{ | ||
public static class AzureKeyVaultHealthChecksBuilderExtensions | ||
{ | ||
/// <summary> | ||
/// Add a health check for Azure Key Vault | ||
/// </summary> | ||
/// <param name="builder">The <see cref="IHealthChecksBuilder"/>.</param> | ||
/// <param name="setup"> Setup action to configure Azure Key Vault options </param> | ||
/// <param name="name">The health check name. Optional. If <c>null</c> the type name 'dynamodb' will be used for the name.</param> | ||
/// <param name="failureStatus"> | ||
/// The <see cref="HealthStatus"/> that should be reported when the health check fails. Optional. If <c>null</c> then | ||
/// the default status of <see cref="HealthStatus.Unhealthy"/> will be reported. | ||
/// </param> | ||
/// <param name="tags">A list of tags that can be used to filter sets of health checks. Optional.</param> | ||
/// <returns>The <see cref="IHealthChecksBuilder"/>.</returns></param> | ||
public static IHealthChecksBuilder AddAzureKeyVault(this IHealthChecksBuilder builder, Action<AzureKeyVaultOptions> setup, | ||
string name = default, HealthStatus? failureStatus = default, IEnumerable<string> tags = default) | ||
{ | ||
var azureKeyVaultOptions = new AzureKeyVaultOptions(); | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. blank line |
||
setup?.Invoke(azureKeyVaultOptions); | ||
|
||
return builder.Add(new HealthCheckRegistration( | ||
name ?? "azurekeyvault", | ||
sp => new AzureKeyVaultHealthCheck(azureKeyVaultOptions), | ||
failureStatus, | ||
tags)); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
<Project Sdk="Microsoft.NET.Sdk"> | ||
<PropertyGroup> | ||
<TargetFramework>$(NetStandardTargetVersion)</TargetFramework> | ||
<PackageLicenseUrl>$(PackageLicenseUrl)</PackageLicenseUrl> | ||
<PackageProjectUrl>$(PackageProjectUrl)</PackageProjectUrl> | ||
<PackageTags>HealthCheck;Azure Key Vault;Secrets</PackageTags> | ||
<Description>HealthChecks.AzureKeyVault is the health check package for Azure Key Vault secrets</Description> | ||
<Version>$(HealthCheckKeyVault)</Version> | ||
<RepositoryUrl>$(RepositoryUrl)</RepositoryUrl> | ||
<Company>$(Company)</Company> | ||
<Authors>$(Authors)</Authors> | ||
<LangVersion>latest</LangVersion> | ||
<PackageId>AspNetCore.HealthChecks.AzureKeyVault</PackageId> | ||
<PublishRepositoryUrl>$(PublishRepositoryUrl)</PublishRepositoryUrl> | ||
<AllowedOutputExtensionsInPackageBuildOutputFolder>$(AllowedOutputExtensionsInPackageBuildOutputFolder)</AllowedOutputExtensionsInPackageBuildOutputFolder> | ||
</PropertyGroup> | ||
<ItemGroup> | ||
<PackageReference Include="Microsoft.Extensions.Diagnostics.HealthChecks" Version="$(MicrosoftExtensionsDiagnosticsHealthChecks)" /> | ||
<PackageReference Include="Microsoft.Azure.KeyVault" Version="$(MicrosoftAzureKeyVault)" /> | ||
<PackageReference Include="Microsoft.IdentityModel.Clients.ActiveDirectory" Version="$(MicrosoftIdentityModelClientsActiveDirectory)" /> | ||
<PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.0.0-beta-63127-02"> | ||
<PrivateAssets>all</PrivateAssets> | ||
<IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets> | ||
</PackageReference> | ||
</ItemGroup> | ||
</Project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,8 +13,11 @@ public class AzureEventHubHealthCheck | |
private readonly string _eventHubName; | ||
public AzureEventHubHealthCheck(string connectionString, string eventHubName) | ||
{ | ||
_connectionString = connectionString ?? throw new ArgumentNullException(nameof(connectionString)); | ||
_eventHubName = eventHubName ?? throw new ArgumentNullException(nameof(eventHubName)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. probably mix features on different package is not a good idea for tracking project. Can you split in two different pull requests There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. New tests can't pass without this changes to the package. :/ |
||
if (string.IsNullOrEmpty(connectionString)) throw new ArgumentNullException(nameof(connectionString)); | ||
if (string.IsNullOrEmpty(eventHubName)) throw new ArgumentNullException(nameof(eventHubName)); | ||
|
||
_connectionString = connectionString; | ||
_eventHubName = eventHubName; | ||
} | ||
public async Task<HealthCheckResult> CheckHealthAsync(HealthCheckContext context, CancellationToken cancellationToken = default) | ||
{ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why a new version on service bus
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because health checks contructor validations changed. It does not make sense to check only for null and not for empty string when initializing configuration towards an azure service