Infer azure tenant ID. by sreekanth-db · Pull Request #482 · databricks/databricks-sdk-java

sreekanth-db · 2025-07-11T08:35:09Z

What changes are proposed in this pull request?

This PR modifies Azure service principal credential provider to attempt to load the tenant ID of the workspace if not provided before authenticating. Tenant ID is indirectly exposed via the redirect URL used when logging into a workspace. In this PR, we fetch the tenant ID from this endpoint and configure it if not already set.

Reference PR: databricks/databricks-sdk-py#638

Key changes:

Added inferTenantId() method in AzureUtils that makes an HTTP request to {host}/aad/auth and extracts the tenant ID from the redirect URL
Modified AzureServicePrincipalCredentialsProvider to remove the explicit azureTenantId requirement and automatically call tenant ID discovery
Added comprehensive unit tests covering success scenarios, error handling, and edge cases

Technical implementation:

Makes HTTP GET request to https://<workspace-host>/aad/auth endpoint
Follows redirect chain to extract tenant ID from URLs like https://login.microsoftonline.com/{tenant-id}/oauth2/authorize
Handles various error scenarios gracefully (404, missing headers, malformed URLs)
Only works for Azure workspaces (skips for non-Azure hosts)
Respects existing tenant ID if already configured

Why are these changes needed?

Currently, Azure Databricks users must manually specify the tenant-id when using Service Principal authentication. With this feature, users don't need to manually specify tenant ID, thus improving the user experience.

How is this tested?

Unit Tests

Comprehensive unit tests in AzureUtilsTest covering:

✅ Happy path (successful tenant ID extraction)
✅ Error scenarios (404, missing Location header, malformed URLs)
✅ Edge cases (non-Azure workspaces, already set tenant ID, missing host)
✅ Integration with existing functionality

Manual Testing

✅ Locally built SDK JAR imported into JDBC driver and tested connection without passing azure_tenant_id
✅ Verified successful authentication using auto-discovered tenant ID
✅ Confirmed backward compatibility - still works when explicit tenant ID is provided

Signed-off-by: Sreekanth Vadigi <[email protected]>

databricks-sdk-java/src/main/java/com/databricks/sdk/core/DatabricksConfig.java

samikshya-db · 2025-07-31T19:21:27Z

...va/src/main/java/com/databricks/sdk/core/oauth/AzureServicePrincipalCredentialsProvider.java

    }
    AzureUtils.ensureHostPresent(
        config, mapper, AzureServicePrincipalCredentialsProvider::tokenSourceFor);
+    config.loadAzureTenantId();


can you return null if azure tenant id fails? to be consistent with line 26?
Also, can we keep the azureSPCredentials specific helpers for extracting tenant id here in credentialsProvider itself?

Have updated the code to return null if loading tenant id fails.

I think we should keep loadAzureTenantId() in DatabricksConfig for reusability. Looking at the Python SDK's implementation, load_azure_tenant_id() is called from multiple Azure credential providers (azure_service_principal and azure_cli), which demonstrates it's designed as a shared utility in the main Config class. This approach ensures consistency across our SDKs and allows for future reuse by other Azure authentication methods.

gopalldb · 2025-08-01T06:21:39Z

databricks-sdk-java/src/main/java/com/databricks/sdk/core/DatabricksConfig.java


  public DatabricksConfig clone() {
-    return clone(new HashSet<>());
+    return clone(new HashSet<>(Collections.singletonList("logger")));


why is this needed?

The logger field is excluded from cloning because it's a static final field. The clone(Set<String> fieldsToSkip) method uses reflection to copy field values with f.set(newConfig, f.get(this)).

Attempting to clone a static field causes an IllegalAccessException with the message: "Can not set static final org.slf4j.Logger field com.databricks.sdk.core.DatabricksConfig.logger to org.slf4j.reload4j.Reload4jLoggerAdapter"

databricks-sdk-java/src/main/java/com/databricks/sdk/core/DatabricksConfig.java

Signed-off-by: Sreekanth Vadigi <[email protected]>

nikhilsuri-db · 2025-08-04T19:46:47Z

databricks-sdk-java/src/main/java/com/databricks/sdk/core/DatabricksConfig.java

@@ -726,7 +735,7 @@ private DatabricksConfig clone(Set<String> fieldsToSkip) {
  }

  public DatabricksConfig clone() {


Do we have to clone this method for all our config classes?

Could you elaborate on this comment? Are you asking whether we need to implement similar clone() methods in other configuration classes, or are you suggesting a different architectural approach for handling object cloning?

nikhilsuri-db · 2025-08-04T19:47:48Z

databricks-sdk-java/src/main/java/com/databricks/sdk/core/DatabricksConfig.java

+      return true; // Tenant ID already available - success
+    }
+
+    if (!isAzure() || host == null) {


Should not the isAzure check be the first check in this method & return false even if azureTenantId is having some value?

The purpose of this method is to load the tenant ID, so if it's already set (explicitly by the user), we should respect that and return true. The isAzure() check is only needed when we need to discover the tenant ID from the workspace host. If the tenant ID is set and the host is not Azure, it should get caught at appropriate layers - this method should not be responsible for returning false and blocking the flow.

renaudhartert-db · 2025-08-07T15:54:59Z

databricks-sdk-java/src/main/java/com/databricks/sdk/core/DatabricksConfig.java

+   * 
+   * @return true if tenant ID is available (either was already set or successfully loaded), false otherwise
+   */
+  public boolean loadAzureTenantId() {


I understand that there's a lot of exceptions in the current code but I would recommend treating the config as an immutable object. That is, do not change the tenantId in the config and rather have a mutable copy in the AzureServicePrincipalCredentialsProvider.

Thanks for the suggestion, I have updated the code to not make any changes to the state of DatabricksConfig

renaudhartert-db · 2025-08-07T15:56:40Z

...va/src/main/java/com/databricks/sdk/core/oauth/AzureServicePrincipalCredentialsProvider.java

+    boolean tenantIdLoaded = config.loadAzureTenantId();
+    if (!tenantIdLoaded) {
+      return null;
+    }


As mentioned in another comment, I would recommend implementing the loadAzureTenantId as an immutable function that would return the tenantId or fail. The tenantId can be stored in this class.

Something like this:

try { this.tenantId = inferTenantId(config) } catch (Exception e) { logger.debug("Failed to extract tenant ID: {}", e.getMessage()) }

Updated the code as per your suggestions:

Not changing the state of DatabricksConfig

Maintaining local variable for tenantId in AzureServicePrincipalCredentialsProvider

Moved the tenant id inferring logic to AzureUtils for reusability

Signed-off-by: Sreekanth Vadigi <[email protected]>

renaudhartert-db · 2025-08-13T08:32:10Z

databricks-sdk-java/src/main/java/com/databricks/sdk/core/utils/AzureUtils.java

+        logger.warn("Failed to get redirect location from Azure auth endpoint: {}", loginUrl);
+        return null;


Let's throw an exception instead, the caller should be the one deciding whether these should be logged or not. Same in other places.

Updated the code in AzureUtils to throw exception, caller will handle it now and logs the message

Signed-off-by: Sreekanth Vadigi <[email protected]>

renaudhartert-db

LGTM modulo remaining comment to remove the logger from AzureUtils.

renaudhartert-db · 2025-08-13T11:04:56Z

databricks-sdk-java/src/main/java/com/databricks/sdk/core/utils/AzureUtils.java

+    try {
+      String redirectLocation = getRedirectLocation(config, loginUrl);
+      String extractedTenantId = extractTenantIdFromUrl(redirectLocation);
+      logger.info("Successfully discovered Azure tenant ID: {}", extractedTenantId);


Also remove private static final Logger logger = LoggerFactory.getLogger(AzureUtils.class);

Suggested change

logger.info("Successfully discovered Azure tenant ID: {}", extractedTenantId);

Signed-off-by: Sreekanth Vadigi <[email protected]>

github-actions · 2025-08-18T10:33:59Z

If integration tests don't run automatically, an authorized user can run them manually by following the instructions below:

Trigger:
go/deco-tests-run/sdk-java

Inputs:

PR number: 482
Commit SHA: ea7ebc302324e8e9306780a23e055ebf364d179f

Checks will be approved automatically on success.

sreekanth-db temporarily deployed to test-trigger-is July 11, 2025 08:35 — with GitHub Actions Inactive

sreekanth-db temporarily deployed to test-trigger-is July 11, 2025 08:51 — with GitHub Actions Inactive

sreekanth-db added 2 commits July 30, 2025 16:26

infer azure tenant id

9c7ae67

Signed-off-by: Sreekanth Vadigi <[email protected]>

unit test fix

56859bf

Signed-off-by: Sreekanth Vadigi <[email protected]>

sreekanth-db force-pushed the infer-azure-tenant-id branch from 0426acf to 56859bf Compare July 30, 2025 10:56

sreekanth-db temporarily deployed to test-trigger-is July 30, 2025 10:57 — with GitHub Actions Inactive

changelog

312c32a

Signed-off-by: Sreekanth Vadigi <[email protected]>

sreekanth-db temporarily deployed to test-trigger-is July 31, 2025 13:27 — with GitHub Actions Inactive

gopalldb reviewed Jul 31, 2025

View reviewed changes

databricks-sdk-java/src/main/java/com/databricks/sdk/core/DatabricksConfig.java Outdated Show resolved Hide resolved

samikshya-db reviewed Jul 31, 2025

View reviewed changes

gopalldb reviewed Aug 1, 2025

View reviewed changes

databricks-sdk-java/src/main/java/com/databricks/sdk/core/DatabricksConfig.java Outdated Show resolved Hide resolved

gopalldb reviewed Aug 1, 2025

View reviewed changes

databricks-sdk-java/src/main/java/com/databricks/sdk/core/DatabricksConfig.java Outdated Show resolved Hide resolved

addressing review comments

f06c006

Signed-off-by: Sreekanth Vadigi <[email protected]>

sreekanth-db temporarily deployed to test-trigger-is August 3, 2025 12:59 — with GitHub Actions Inactive

nikhilsuri-db reviewed Aug 4, 2025

View reviewed changes

renaudhartert-db requested changes Aug 7, 2025

View reviewed changes

sreekanth-db added 2 commits August 11, 2025 18:39

moved inferring logic to azure utils

cf281fd

Signed-off-by: Sreekanth Vadigi <[email protected]>

code formatting

9f9f7a1

Signed-off-by: Sreekanth Vadigi <[email protected]>

sreekanth-db temporarily deployed to test-trigger-is August 11, 2025 13:40 — with GitHub Actions Inactive

sreekanth-db requested a review from renaudhartert-db August 11, 2025 14:03

renaudhartert-db requested changes Aug 13, 2025

View reviewed changes

azure utils throwing exception

22adeed

Signed-off-by: Sreekanth Vadigi <[email protected]>

sreekanth-db temporarily deployed to test-trigger-is August 13, 2025 10:51 — with GitHub Actions Inactive

renaudhartert-db approved these changes Aug 13, 2025

View reviewed changes

Merge branch 'main' into infer-azure-tenant-id

21ba14f

renaudhartert-db temporarily deployed to test-trigger-is August 13, 2025 11:09 — with GitHub Actions Inactive

renaudhartert-db changed the title ~~infer azure tenant id~~ Infer azure tenant ID. Aug 13, 2025

removing logger from azureutils

0bb96a2

Signed-off-by: Sreekanth Vadigi <[email protected]>

sreekanth-db temporarily deployed to test-trigger-is August 13, 2025 11:31 — with GitHub Actions Inactive

Merge branch 'main' into infer-azure-tenant-id

ea7ebc3

renaudhartert-db temporarily deployed to test-trigger-is August 18, 2025 10:33 — with GitHub Actions Inactive

renaudhartert-db enabled auto-merge August 18, 2025 10:33

renaudhartert-db added this pull request to the merge queue Aug 19, 2025

Merged via the queue into databricks:main with commit fe28261 Aug 19, 2025
15 checks passed

		@@ -726,7 +735,7 @@ private DatabricksConfig clone(Set<String> fieldsToSkip) {
		}

		public DatabricksConfig clone() {

		logger.warn("Failed to get redirect location from Azure auth endpoint: {}", loginUrl);
		return null;

Conversation

sreekanth-db commented Jul 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes are proposed in this pull request?

Why are these changes needed?

How is this tested?

Unit Tests

Manual Testing

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sreekanth-db Aug 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

renaudhartert-db left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Aug 18, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sreekanth-db commented Jul 11, 2025 •

edited

Loading

sreekanth-db Aug 3, 2025 •

edited

Loading

renaudhartert-db left a comment •

edited

Loading