AspNetCore.Diagnostics.HealthChecks icon indicating copy to clipboard operation
AspNetCore.Diagnostics.HealthChecks copied to clipboard

Feature/elastic auth tests

Open bCamba opened this issue 2 years ago • 1 comments

What this PR does / why we need it: Added tests for elastic search authentication by api key and by username and password

Which issue(s) this PR fixes: #1117

Please reference the issue this PR will close: #[issue number]

Special notes for your reviewer: I recently created a PR (#1118) to add api key authentication to elastic search health check. I have now created tests for it and also for the username and password which you did not have tests for. I tried also testing the authentication by certificate but for some reason I was not able to make it work. Bear in mind that the tests I have created spin up and tear down docker containers automatically so I am not sure you would want those to run in your default CI/CD.

Does this PR introduce a user-facing change?: NO

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • [x] Code compiles correctly
  • [x] Created/updated tests
  • [x] Unit tests passing
  • [x] End-to-end tests passing
  • [x] Extended the documentation
  • [x] Provided sample for the feature

bCamba avatar Apr 26 '22 12:04 bCamba

@bCamba Formatting check failed

sungam3r avatar Aug 02 '22 11:08 sungam3r

@bCamba I'm fine to merge after pulling changes from master.

sungam3r avatar Dec 22 '22 20:12 sungam3r

Hi @sungam3r, I could refresh/revamp this PR to make it mergeable.

Could you shortly describe, what should be done within the scope of this PR?

I'm not sure, if I can interact with someone else's branch/fork?

cieciurm avatar Jul 03 '23 09:07 cieciurm

@cieciurm Just take the code from bCamba:feature/elastic-auth-tests branch into your local branch, then merge it with master and make new PR. Then I can review, merge your PR and close this PR as obsolete.

sungam3r avatar Jul 05 '23 06:07 sungam3r

Thanks @sungam3r, I already tried that - see #1849.

The tests against docker are failing and I had hard time trying to fix them.

Should we keep the tests for Authentication (user/password & api key) and wait for someone knowledgeable with integration tests in .NET for Docker or can I just keep the basic tests?

cieciurm avatar Jul 05 '23 07:07 cieciurm

I have no experience either in Docker or .NET for Docker, ping @bCamba again.

sungam3r avatar Jul 05 '23 07:07 sungam3r

Hi again,

It's been fixed in #1857 cc @sungam3r

cieciurm avatar Jul 05 '23 09:07 cieciurm