feat(go/adbc/databricks): add azure-client-secret (Entra ID service principal) auth#143
feat(go/adbc/databricks): add azure-client-secret (Entra ID service principal) auth#143r-jais wants to merge 2 commits into
Conversation
Add Azure service-principal (Microsoft Entra ID) authentication to the Databricks driver via a new "azure-client-secret" auth_type and databricks.azure.client_id / client_secret / tenant_id options. The new auth case builds a databricks-sdk-go Config with AzureClientSecretCredentials and passes it to dbsql.WithAuthenticator; *config.Config satisfies databricks-sql-go's auth.Authenticator, so the SDK attaches both the AAD bearer token and the X-Databricks-Azure-SP-Management-Token header. azure_tenant_id is optional and discovered from the workspace's /aad/auth redirect when absent. databricks-sdk-go promoted to a direct dependency. Additive: pat / oauth-m2m / external-browser paths are unchanged. For dbt-core#13986.
|
|
||
| // newAzureClientSecretAuthenticator builds a databricks-sdk-go Config configured | ||
| // for Azure service-principal (Microsoft Entra ID) authentication. *config.Config | ||
| // implements databricks-sql-go's auth.Authenticator via its Authenticate method, so |
There was a problem hiding this comment.
a lot of this is boilerplate we can remove. keep the super specific domain lore though
|
|
||
| // discoverAzureTenantID resolves the Microsoft Entra ID tenant for an Azure | ||
| // Databricks workspace from the unauthenticated <host>/aad/auth redirect, mirroring | ||
| // databricks-sdk's tenant discovery. Only used when azure_tenant_id is not supplied. |
There was a problem hiding this comment.
should this logic actually be arrow adbc? What's the rationale?
I don't love the idea of making adhoc runtime asks for connection either. Might not be able to be helped though I suppose.
There was a problem hiding this comment.
removed the discovery from the driver here. Tenant resolution (explicit or /aad/auth discovery) now lives upstream in fs dbt-auth
| return "", err | ||
| } | ||
| defer resp.Body.Close() | ||
| if resp.StatusCode/100 != 3 { |
There was a problem hiding this comment.
is this safe (I'm bad at Go and don't have all the coercion rules in my brain)
| AzureClientID: d.azureClientID, | ||
| AzureClientSecret: d.azureClientSecret, | ||
| AzureTenantID: tenantID, | ||
| Credentials: sdkconfig.AzureClientSecretCredentials{}, |
There was a problem hiding this comment.
does this work / fail for different users who have special overrides? Any live infra testing here so we can know how this performs?
There was a problem hiding this comment.
Posted on channel for DBX Azure SP credentials (could only find PAT ones in vault that I have access to)
4f10400 to
fb5423e
Compare
| } | ||
| } | ||
|
|
||
| cfg := &sdkconfig.Config{ |
There was a problem hiding this comment.
nice!
There's quite a few additional fields, can we permalink these docs?
https://github.com/databricks/databricks-sdk-go/blob/main/README.md#azure-native-authentication
3 participants
What
Adds Azure/Microsoft Entra ID service-principal authentication to the Databricks driver: a new
azure-client-secretauth_type anddatabricks.azure.client_id/databricks.azure.client_secret/databricks.azure.tenant_idoptions.How
The new auth case builds a
databricks-sdk-goConfigwithAzureClientSecretCredentials{}and passes it todbsql.WithAuthenticator.*config.Configsatisfies databricks-sql-go'sauth.Authenticator, so the SDK attaches the AAD bearer token (Authorization) and theX-Databricks-Azure-SP-Management-Tokenheader on every request — mirroring dbt-databricks/databricks-sdk-py.azure_tenant_idis optional and discovered from the workspace's/aad/authredirect when absent.databricks-sdk-gois promoted to a direct dependency.Compatibility
Additive:
pat/oauth-m2m/external-browserdispatch is unchanged;IsAzure()gates the new path to Azure workspaces. No changes to the databricks-sql-go fork.Tests
New
azure_auth_test.go(option wiring, missing-field errors, tenant parsing across Azure clouds). Existing driver tests pass (the pre-existingTestIPCReaderAdapterpanic is unrelated — fails on clean main).For dbt-core#13986.