Add Entra ID auth manager #2974
Conversation
| gcp-auth = ["google-auth>=2.4.0"] | ||
| entra-auth = ["azure-identity>=1.25.1"] |
There was a problem hiding this comment.
There doesnt seem to be docs on these extra groups? Should we add some information about these eventually?
There was a problem hiding this comment.
Good point, they are mentioned here: https://github.com/apache/iceberg-python/blob/5fca4ac7982c825750a034eb032e948e80a83d0b/mkdocs/docs/index.md#installation
rambleraptor
left a comment
rambleraptor
left a comment
There was a problem hiding this comment.
I've got one suggestion for a test, but otherwise this looks great!
|
|
||
| def __init__( | ||
| self, | ||
| scopes: list[str] | None = None, |
There was a problem hiding this comment.
nit: consider making the default for this [self.DEFAULT_SCOPE]. That's basically the default anyways (line 287)
There was a problem hiding this comment.
that would make the param a mutable list (and shared across calls) which is generally discouraged in python
|
|
||
|
|
||
| @patch("azure.identity.DefaultAzureCredential") | ||
| def test_entra_auth_manager_default_credential(mock_default_cred: MagicMock, rest_mock: Mocker) -> None: |
There was a problem hiding this comment.
Could you add a test where the azure-default library can't successfully get a token? I'd expect that we'd throw an exception and not try to start sending Iceberg requests with a blank auth header.
Fokko
left a comment
Fokko
left a comment
There was a problem hiding this comment.
Agree with @jayceslesar's comment, but apart from that, it looks good to me 👍
4 participants
Closes #2800
Rationale for this change
Adds
EntraAuthManagerfor Microsoft Entra ID (Azure AD) authentication usingDefaultAzureCredentialfrom the Azure Identity library.Supported auth methods
AZURE_TENANT_ID,AZURE_CLIENT_ID,AZURE_CLIENT_SECRET)az login)Are these changes tested?
Yes, unit tests, tested locally with OneLake and az cli,
Preview for docs:
Are there any user-facing changes?