Add support for custom OAuth functions #1925
Conversation
|
🎉 All Contributor License Agreements have been signed. Ready to merge. |
There was a problem hiding this comment.
PR Overview
This PR adds support for custom OAuth functionality by introducing a common _BearerFieldProvider interface with implementations for OAuth, static tokens, and custom logic. It also updates existing tests and configuration handling to work with the new provider abstraction while removing legacy OAuth client tests.
Reviewed Changes
| File | Description |
|---|---|
| tests/schema_registry/test_bearer_field_provider.py | Adds tests covering expiration, token retrieval, static and custom OAuth implementations. |
| tests/schema_registry/test_config.py | Adds tests to validate custom bearer configuration handling. |
| src/confluent_kafka/schema_registry/schema_registry_client.py | Refactors bearer auth handling by introducing _BearerFieldProvider and related classes, and adapts configuration accordingly. |
| tests/schema_registry/test_oauth_client.py | Removes legacy OAuth client tests in favor of the new provider approach. |
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (2)
tests/schema_registry/test_bearer_field_provider.py:95
- [nitpick] The test for the custom OAuth client only asserts that get_bearer_fields() returns the same value on successive calls. Consider asserting specific expected fields from the custom function to improve test coverage.
def test_custom_oauth_client():
src/confluent_kafka/schema_registry/schema_registry_client.py:84
- [nitpick] Consider renaming '_OAuthClient' to 'OAuthBearerFieldProvider' to better reflect its role as an implementation of _BearerFieldProvider for OAuth logic.
class _OAuthClient(_BearerFieldProvider):
Analysis Details6 Issues
Coverage and Duplications
Project ID: confluent-kafka-python |
|
Hi, a potential user of this functionality here 😁 Do you already have a (rough) estimate on when you plan to release this? |
There was a problem hiding this comment.
Thanks @Claimundefine , left some nits, otherwise looking good
| @@ -62,7 +62,26 @@ def _urlencode(value: str) -> str: | |||
| VALID_AUTH_PROVIDERS = ['URL', 'USER_INFO'] | |||
|
|
|||
|
|
|||
| class _OAuthClient: | |||
| class _BearerFieldProvider: | |||
There was a problem hiding this comment.
| class _BearerFieldProvider: | |
| class _BearerFieldProvider(metaclass=abc.ABCMeta): |
| @@ -62,7 +62,26 @@ def _urlencode(value: str) -> str: | |||
| VALID_AUTH_PROVIDERS = ['URL', 'USER_INFO'] | |||
|
|
|||
|
|
|||
| class _OAuthClient: | |||
| class _BearerFieldProvider: | |||
| def get_bearer_fields(self) -> dict: | |||
There was a problem hiding this comment.
@abc.abstractmethod
def get_bearer_fields(self) -> dict:
| raise NotImplementedError | ||
|
|
||
|
|
||
| class _StaticFieldProvider(_BearerFieldProvider): |
There was a problem hiding this comment.
Add __init__ that takes a token and sets a local bearer_token member variable. Then you can remove the bearer_token member variable in the SR client.
|
|
||
| class _StaticFieldProvider(_BearerFieldProvider): | ||
| def get_bearer_fields(self) -> dict: | ||
| return {} |
There was a problem hiding this comment.
| return {} | |
| return {'bearer.auth.token': self.bearer_token} |
| @@ -206,23 +228,28 @@ def __init__(self, conf: dict): | |||
| + str(type(retries_max_wait_ms))) | |||
| self.retries_max_wait_ms = retries_max_wait_ms | |||
|
|
|||
| self.oauth_client = None | |||
| self.bearer_field_provider = None | |||
| self.bearer_token = None | |||
| elif self.bearer_auth_credentials_source == 'STATIC_TOKEN': | ||
| if 'bearer.auth.token' not in conf_copy: | ||
| raise ValueError("Missing bearer.auth.token") | ||
| self.bearer_token = conf_copy.pop('bearer.auth.token') | ||
| self.bearer_field_provider = _StaticFieldProvider() |
There was a problem hiding this comment.
| self.bearer_field_provider = _StaticFieldProvider() | |
| self.bearer_field_provider = _StaticFieldProvider(bearer_token) |
| if not isinstance(self.bearer_token, string_type): | ||
| raise TypeError("bearer.auth.token must be a str, not " + str(type(self.bearer_token))) | ||
| elif self.bearer_auth_credentials_source == "CUSTOM": |
There was a problem hiding this comment.
| elif self.bearer_auth_credentials_source == "CUSTOM": | |
| elif self.bearer_auth_credentials_source == 'CUSTOM': |
HI @Elmerboul, the projected release date is end of March. |
What
Add custom OAuth implementation. Created new base class for bearer field provider, which will be called to retrieve all fields that need to be modified in the header.
Checklist
References
JIRA:
https://confluentinc.atlassian.net/browse/DGS-17473
Test & Review
Tested locally with OAuth for custom.
Open questions / Follow-ups