-
Notifications
You must be signed in to change notification settings - Fork 807
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 health cheks vault #2249
base: master
Are you sure you want to change the base?
Add health cheks vault #2249
Conversation
@dotnet-policy-service agree |
@adamsitnik, can you add ci/cd workflows for this? |
HealthCheks.Vault.Test/HealthChecks.Vault.HealthAndConnectionTest.cs
Outdated
Show resolved
Hide resolved
@Alirexaa please review code and tell me change if need. |
@Alirexaa ptl |
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.
@SaeedSafi1999 thank you for your contribution! PTAL at my comments.
CallerArgumentExpressionAttribute.cs
Outdated
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.
What is the purpose of having this empty file?
|
||
public VaultContainerFixture() | ||
{ | ||
_container = new ContainerBuilder() |
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.
in this repo we typically configure the containers from github workflow level, here you can find some examples of how we do that:
https://github.com/Xabaril/AspNetCore.Diagnostics.HealthChecks/blob/master/.github/workflows/healthchecks_redis_cd.yml
https://github.com/Xabaril/AspNetCore.Diagnostics.HealthChecks/blob/master/.github/workflows/healthchecks_redis_cd_preview.yml
https://github.com/Xabaril/AspNetCore.Diagnostics.HealthChecks/blob/master/.github/workflows/healthchecks_redis_ci.yml
could you please do that for this project as well (we need that to be able to run the tests in the CI) and remove the dependency to Testcontainers
?
I am not saying that Testcontainers
are bad (it actually looks very neat), we just use a different approach in this repo and I would prefer for all of the health checks to implement the same pattern.
What this PR does / why we need it:
add hashicorp vault to reposiotiry
Which issue(s) this PR fixes:
add all of the Common authentication methods like LDAP,OKTA,RADIUS,Vault token
Please reference the issue this PR will close: #2239
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
Please make sure you've completed the relevant tasks for this PR, out of the following list: