Skip to content

Commit 0189bfe

Browse files
hectorcast-dbclaude
andcommitted
Move cloud-based credential filtering into DefaultCredentialsProvider
Azure strategies (azure-cli, azure-client-secret, github-oidc-azure) are now skipped on GCP/AWS hosts in auto-detect mode. GCP strategies (google-credentials, google-id) are skipped on Azure/AWS hosts. When authType is explicitly set, cloud filtering is bypassed so the named strategy is always attempted regardless of host cloud. Individual credential providers no longer perform their own isAzure()/isGcp() checks — that responsibility now lives centrally in DefaultCredentialsProvider via a CLOUD_REQUIREMENTS map, mirroring the approach taken in databricks/databricks-sdk-go#1505. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent a6044f4 commit 0189bfe

File tree

7 files changed

+117
-14
lines changed

7 files changed

+117
-14
lines changed

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,6 @@ private Optional<String> getSubscription(DatabricksConfig config) {
7777

7878
@Override
7979
public OAuthHeaderFactory configure(DatabricksConfig config) {
80-
if (!config.isAzure()) {
81-
return null;
82-
}
83-
8480
try {
8581
AzureUtils.ensureHostPresent(config, mapper, this::tokenSourceFor);
8682
String resource = config.getEffectiveAzureLoginAppId();

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

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
11
package com.databricks.sdk.core;
22

33
import com.databricks.sdk.core.oauth.*;
4+
import com.databricks.sdk.core.utils.Cloud;
45
import com.databricks.sdk.support.InternalApi;
56
import com.google.common.base.Strings;
67
import java.util.ArrayList;
8+
import java.util.Collections;
9+
import java.util.HashMap;
710
import java.util.List;
11+
import java.util.Map;
812
import org.slf4j.Logger;
913
import org.slf4j.LoggerFactory;
1014

@@ -18,6 +22,21 @@
1822
public class DefaultCredentialsProvider implements CredentialsProvider {
1923
private static final Logger LOG = LoggerFactory.getLogger(DefaultCredentialsProvider.class);
2024

25+
// cloudRequirements declares the cloud each strategy requires. DefaultCredentialsProvider uses
26+
// this to skip cloud-specific strategies in auto-detect mode when the host cloud does not match.
27+
// Cloud filtering is bypassed when authType is explicitly set.
28+
private static final Map<String, Cloud> CLOUD_REQUIREMENTS;
29+
30+
static {
31+
Map<String, Cloud> m = new HashMap<>();
32+
m.put("github-oidc-azure", Cloud.AZURE);
33+
m.put("azure-client-secret", Cloud.AZURE);
34+
m.put("azure-cli", Cloud.AZURE);
35+
m.put("google-credentials", Cloud.GCP);
36+
m.put("google-id", Cloud.GCP);
37+
CLOUD_REQUIREMENTS = Collections.unmodifiableMap(m);
38+
}
39+
2140
/* List of credential providers that will be tried in sequence */
2241
private List<CredentialsProvider> providers = new ArrayList<>();
2342

@@ -40,6 +59,11 @@ public NamedIDTokenSource(String name, IDTokenSource idTokenSource) {
4059

4160
public DefaultCredentialsProvider() {}
4261

62+
/** For testing: creates a provider with a fixed list of credential providers. */
63+
DefaultCredentialsProvider(List<CredentialsProvider> providers) {
64+
this.providers = new ArrayList<>(providers);
65+
}
66+
4367
/**
4468
* Returns the current authentication type being used
4569
*
@@ -60,14 +84,25 @@ public String authType() {
6084
@Override
6185
public synchronized HeaderFactory configure(DatabricksConfig config) {
6286
addDefaultCredentialsProviders(config);
87+
boolean explicitAuthType = config.getAuthType() != null && !config.getAuthType().isEmpty();
6388
for (CredentialsProvider provider : providers) {
64-
if (config.getAuthType() != null
65-
&& !config.getAuthType().isEmpty()
66-
&& !provider.authType().equals(config.getAuthType())) {
89+
if (explicitAuthType && !provider.authType().equals(config.getAuthType())) {
6790
LOG.info(
6891
"Ignoring {} auth, because {} is preferred", provider.authType(), config.getAuthType());
6992
continue;
7093
}
94+
// In auto-detect mode, skip cloud-specific strategies whose required cloud does not match
95+
// the detected host cloud. When authType is explicitly set, cloud filtering is bypassed so
96+
// that users can request any strategy regardless of detected cloud (e.g. "azure-cli" on GCP).
97+
if (!explicitAuthType) {
98+
Cloud requiredCloud = CLOUD_REQUIREMENTS.get(provider.authType());
99+
if (requiredCloud != null
100+
&& config.getDatabricksEnvironment().getCloud() != requiredCloud) {
101+
LOG.debug(
102+
"Skipping \"{}\" auth: not configured for {}", provider.authType(), requiredCloud);
103+
continue;
104+
}
105+
}
71106
try {
72107
LOG.info("Trying {} auth", provider.authType());
73108
HeaderFactory headerFactory = provider.configure(config);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public String authType() {
3030
public HeaderFactory configure(DatabricksConfig config) {
3131
String host = config.getHost();
3232
String googleCredentials = config.getGoogleCredentials();
33-
if (host == null || googleCredentials == null || !config.isGcp()) {
33+
if (host == null || googleCredentials == null) {
3434
return null;
3535
}
3636

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public String authType() {
2727
public HeaderFactory configure(DatabricksConfig config) {
2828
String host = config.getHost();
2929
String googleServiceAccount = config.getGoogleServiceAccount();
30-
if (host == null || googleServiceAccount == null || !config.isGcp()) {
30+
if (host == null || googleServiceAccount == null) {
3131
return null;
3232
}
3333

databricks-sdk-java/src/main/java/com/databricks/sdk/core/oauth/AzureGithubOidcCredentialsProvider.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,7 @@ public String authType() {
2626

2727
@Override
2828
public OAuthHeaderFactory configure(DatabricksConfig config) {
29-
if (!config.isAzure()
30-
|| config.getAzureClientId() == null
29+
if (config.getAzureClientId() == null
3130
|| config.getAzureTenantId() == null
3231
|| config.getHost() == null) {
3332
return null;

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,7 @@ public String authType() {
2727

2828
@Override
2929
public OAuthHeaderFactory configure(DatabricksConfig config) {
30-
if (!config.isAzure()
31-
|| config.getAzureClientId() == null
32-
|| config.getAzureClientSecret() == null) {
30+
if (config.getAzureClientId() == null || config.getAzureClientSecret() == null) {
3331
return null;
3432
}
3533

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
package com.databricks.sdk.core;
2+
3+
import static org.junit.jupiter.api.Assertions.*;
4+
5+
import java.util.Collections;
6+
import org.junit.jupiter.api.Test;
7+
8+
class DefaultCredentialsProviderTest {
9+
10+
/** A credentials provider that records whether configure() was called. */
11+
private static class RecordingCredentialsProvider implements CredentialsProvider {
12+
private final String name;
13+
private boolean called = false;
14+
15+
RecordingCredentialsProvider(String name) {
16+
this.name = name;
17+
}
18+
19+
@Override
20+
public String authType() {
21+
return name;
22+
}
23+
24+
@Override
25+
public HeaderFactory configure(DatabricksConfig config) {
26+
called = true;
27+
return null;
28+
}
29+
30+
public boolean wasCalled() {
31+
return called;
32+
}
33+
}
34+
35+
/**
36+
* In auto-detect mode (authType not set), azure-cli must be skipped on a GCP host because the
37+
* detected cloud does not match the strategy's required cloud (AZURE).
38+
*/
39+
@Test
40+
void testCloudFiltering_SkipsOnCloudMismatch() {
41+
RecordingCredentialsProvider azureCli = new RecordingCredentialsProvider("azure-cli");
42+
DefaultCredentialsProvider provider =
43+
new DefaultCredentialsProvider(Collections.singletonList(azureCli));
44+
45+
DatabricksConfig config = new DatabricksConfig().setHost("https://xyz.gcp.databricks.com/");
46+
// configure() throws because no provider succeeds; that's expected
47+
assertThrows(DatabricksException.class, () -> provider.configure(config));
48+
49+
assertFalse(
50+
azureCli.wasCalled(),
51+
"azure-cli should be skipped on a GCP host in auto-detect mode");
52+
}
53+
54+
/**
55+
* When authType is explicitly set, cloud filtering is bypassed so that the named strategy is
56+
* always attempted regardless of the detected host cloud.
57+
*/
58+
@Test
59+
void testCloudFiltering_BypassesOnExplicitAuthType() {
60+
RecordingCredentialsProvider azureCli = new RecordingCredentialsProvider("azure-cli");
61+
DefaultCredentialsProvider provider =
62+
new DefaultCredentialsProvider(Collections.singletonList(azureCli));
63+
64+
DatabricksConfig config =
65+
new DatabricksConfig()
66+
.setHost("https://xyz.gcp.databricks.com/")
67+
.setAuthType("azure-cli");
68+
// configure() throws because azure-cli returns null; that's expected
69+
assertThrows(DatabricksException.class, () -> provider.configure(config));
70+
71+
assertTrue(
72+
azureCli.wasCalled(),
73+
"azure-cli should be called when authType is explicitly set, even on a GCP host");
74+
}
75+
}

0 commit comments

Comments
 (0)