add support for using elastic cloud api key for auth #33
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #33 +/- ##
==========================================
+ Coverage 92.36% 92.57% +0.21%
==========================================
Files 1 1
Lines 275 283 +8
==========================================
+ Hits 254 262 +8
Misses 21 21 ☔ View full report in Codecov by Sentry. |
| @@ -192,8 +205,13 @@ def __init__(self, config): | |||
| self.is_slave = False | |||
|
|
|||
| @property | |||
| def es_auth(self): | |||
| return self.es_username, self.es_password | |||
| def es_auth_args(self) -> dict[str, Any]: | |||
There was a problem hiding this comment.
IIUC, this function always returns a dict[str, dict[str, str]]. if that's the case, probably we can just use dict[str, dict[str, str]] here?
There was a problem hiding this comment.
it can return dict[str, tuple[str]], but is not important, it can return any request parameter and I don't want to document it all
| @@ -283,7 +301,7 @@ def report_test(self, item_report, outcome, old_report=None): | |||
| outcome=outcome, | |||
| duration=item_report.duration, | |||
| markers=item_report.keywords, | |||
| **self.session_data | |||
| **self.session_data, | |||
pytest_elk_reporter.py
Outdated
| def es_auth_args(self) -> dict[str, Any]: | ||
| output = {} | ||
| if self.es_api_key: | ||
| output.update(dict(headers={"Authorization": f"ApiKey {self.es_api_key}"})) |
There was a problem hiding this comment.
probably we can just return output here? IMHO, API key is more secure than username and password. if that's the case, we could just put
if self.es_api_key:
headers = {"Authorization": f"ApiKey {self.es_api_key}"}
return {"headers": headers}There was a problem hiding this comment.
that was I did in the beginning, pylint wasn't happy about multiple returns within if/else
yes we can end up with both headers, don't know what would be the behavior of ES in this case
but I'll test it
didn't yet integrated with dtest, I'll try this to see if it work, or we need to prevent user for supplying both key and user/pass
There was a problem hiding this comment.
I've change back, after testing, if both header passed down, and one is wrong you fail to connect
so api key comes first, if both are defined.
raising exception if both set, in __init__ would be enough, cause in most usages we change those on runtime, after we retrieve the credentials, and never passing them with command line or configuration file.
I think that's good enough, and no need to go overboard with guardrailing this.
fruch
force-pushed
the
add_api_key_support
branch
from
24b4962 to
1b17a10
Compare
|
Did integration test with a code base that uses this package, |
the introduces the option to use api keys Ref: https://www.elastic.co/guide/en/elasticsearch/reference/current/security-api-create-api-key.html
fruch
force-pushed
the
add_api_key_support
branch
from
1b17a10 to
acc04be
Compare
the introduces the option to use api keys
Ref: https://www.elastic.co/guide/en/elasticsearch/reference/current/security-api-create-api-key.html