[PECOBLR-727] Add kerberos support for proxy auth #675
+690
−98
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
There was a problem hiding this comment.
Pull Request Overview
This PR adds Kerberos/SPNEGO authentication support for proxy authentication by consolidating proxy handling logic and introducing a new authentication method system.
- Consolidates proxy detection and authentication logic into a shared utility module
- Replaces specific proxy credential parameters with a generic
proxy_auth_methodparameter - Implements Kerberos/SPNEGO authentication support using the
requests-kerberoslibrary
Reviewed Changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/databricks/sql/common/http_utils.py |
New utility module containing shared proxy detection and authentication logic with support for basic and Kerberos authentication methods |
src/databricks/sql/common/unified_http_client.py |
Updated to use shared proxy utilities and support per-request proxy decisions with dual pool managers |
src/databricks/sql/auth/thrift_http_client.py |
Refactored to use shared proxy utilities and removed duplicate authentication logic |
src/databricks/sql/backend/sea/utils/http_client.py |
Updated to use shared proxy utilities and cleaned up variable names |
src/databricks/sql/auth/common.py |
Replaced specific proxy credential parameters with generic proxy_auth_method parameter |
src/databricks/sql/utils.py |
Updated to pass new proxy_auth_method parameter instead of individual credentials |
src/databricks/sql/backend/thrift_backend.py |
Added support for passing proxy authentication method to transport layer |
tests/unit/test_thrift_backend.py |
Updated test to use new shared utility function |
tests/unit/test_telemetry.py |
Fixed method name references to match renamed methods |
pyproject.toml |
Added optional requests-kerberos dependency for Kerberos authentication support |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
jprakash-db
approved these changes
Aug 18, 2025
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
CarlosW1998
reviewed
Aug 18, 2025
| @@ -141,7 +222,7 @@ def request_context( | |||
| url: str, | |||
| headers: Optional[Dict[str, str]] = None, | |||
| **kwargs, | |||
| ) -> Generator[urllib3.HTTPResponse, None, None]: | |||
| ) -> Generator[urllib3.BaseHTTPResponse, None, None]: | |||
There was a problem hiding this comment.
Here urllib3 has no BaseHTTPResponse this class is in urllib3.response
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
Description
This PR adds Kerberos/SPNEGO authentication support for proxy authentication by consolidating proxy handling logic and introducing a new authentication method system.
Consolidates proxy detection and authentication logic into a shared utility module
Replaces specific proxy credential parameters with a generic proxy_auth_method parameter
Implements Kerberos/SPNEGO authentication support using the requests-kerberos library
How is this tested?
Related Tickets & Documents