Skip to content

Commit 694521d

Browse files
Refactor CliTokenSource to use an ordered attempt chain
Replace ad-hoc forceCmd/profileCmd/fallbackCmd fields with a generic ordered attempt model: each CliCommand describes its command, the flags it uses (for detecting "unknown flag" errors), and an optional warning message. getToken() is now a simple loop over the attempt chain instead of nested if-else branches. This makes adding future CLI flag fallbacks straightforward without introducing more fields. DatabricksCliCredentialsProvider builds the attempt chain explicitly: - with profile: force+profile -> profile -> host - without profile: force+host -> host Old constructors delegate to the new model. Azure CLI callers use the 5-arg constructor which creates a single-attempt chain, so their behavior is unchanged. Signed-off-by: Mihai Mitrea <mihai.mitrea@databricks.com>
1 parent 6b8a57f commit 694521d

File tree

5 files changed

+233
-84
lines changed

5 files changed

+233
-84
lines changed

NEXT_CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
### Documentation
1515

1616
### Internal Changes
17+
* Generalize CLI token source into a progressive command list for forward-compatible flag support.
1718

1819
### API Changes
1920
* Add `createCatalog()`, `createSyncedTable()`, `deleteCatalog()`, `deleteSyncedTable()`, `getCatalog()` and `getSyncedTable()` methods for `workspaceClient.postgres()` service.

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

Lines changed: 86 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@
1616
import java.time.format.DateTimeFormatter;
1717
import java.time.format.DateTimeParseException;
1818
import java.util.Arrays;
19+
import java.util.Collections;
1920
import java.util.List;
21+
import java.util.stream.Collectors;
2022
import org.apache.commons.io.IOUtils;
2123
import org.slf4j.Logger;
2224
import org.slf4j.LoggerFactory;
@@ -25,18 +27,25 @@
2527
public class CliTokenSource implements TokenSource {
2628
private static final Logger LOG = LoggerFactory.getLogger(CliTokenSource.class);
2729

28-
// forceCmd is tried before profileCmd when non-null. If the CLI rejects
29-
// --force-refresh or --profile, execution falls through to profileCmd.
30-
private List<String> forceCmd;
30+
/**
31+
* Describes a CLI command with an optional warning message emitted when falling through to the
32+
* next command in the chain.
33+
*/
34+
static class CliCommand {
35+
final List<String> cmd;
36+
37+
// Flags used by this command (e.g. "--force-refresh", "--profile"). Used to distinguish
38+
// "unknown flag" errors (which trigger fallback) from real auth errors (which propagate).
39+
final List<String> usedFlags;
3140

32-
private List<String> profileCmd;
33-
private String tokenTypeField;
34-
private String accessTokenField;
35-
private String expiryField;
36-
private Environment env;
37-
// fallbackCmd is tried when profileCmd fails with "unknown flag: --profile",
38-
// indicating the CLI is too old to support --profile.
39-
private List<String> fallbackCmd;
41+
final String fallbackMessage;
42+
43+
CliCommand(List<String> cmd, List<String> usedFlags, String fallbackMessage) {
44+
this.cmd = cmd;
45+
this.usedFlags = usedFlags != null ? usedFlags : Collections.emptyList();
46+
this.fallbackMessage = fallbackMessage;
47+
}
48+
}
4049

4150
/**
4251
* Internal exception that carries the clean stderr message but exposes full output for checks.
@@ -54,42 +63,65 @@ String getFullOutput() {
5463
}
5564
}
5665

66+
private final List<CliCommand> attempts;
67+
private final String tokenTypeField;
68+
private final String accessTokenField;
69+
private final String expiryField;
70+
private final Environment env;
71+
72+
/** Constructs a single-attempt source. Used by Azure CLI and simple callers. */
5773
public CliTokenSource(
5874
List<String> cmd,
5975
String tokenTypeField,
6076
String accessTokenField,
6177
String expiryField,
6278
Environment env) {
63-
this(cmd, tokenTypeField, accessTokenField, expiryField, env, null, null);
79+
this(
80+
Collections.singletonList(
81+
new CliCommand(
82+
OSUtils.get(env).getCliExecutableCommand(cmd), Collections.emptyList(), null)),
83+
tokenTypeField,
84+
accessTokenField,
85+
expiryField,
86+
env,
87+
true);
6488
}
6589

66-
public CliTokenSource(
67-
List<String> cmd,
90+
/** Creates a CliTokenSource from a pre-built attempt chain. */
91+
static CliTokenSource fromAttempts(
92+
List<CliCommand> attempts,
6893
String tokenTypeField,
6994
String accessTokenField,
7095
String expiryField,
71-
Environment env,
72-
List<String> fallbackCmd) {
73-
this(cmd, tokenTypeField, accessTokenField, expiryField, env, fallbackCmd, null);
96+
Environment env) {
97+
return new CliTokenSource(
98+
attempts.stream()
99+
.map(
100+
a ->
101+
new CliCommand(
102+
OSUtils.get(env).getCliExecutableCommand(a.cmd),
103+
a.usedFlags,
104+
a.fallbackMessage))
105+
.collect(Collectors.toList()),
106+
tokenTypeField,
107+
accessTokenField,
108+
expiryField,
109+
env,
110+
true);
74111
}
75112

76-
public CliTokenSource(
77-
List<String> cmd,
113+
private CliTokenSource(
114+
List<CliCommand> attempts,
78115
String tokenTypeField,
79116
String accessTokenField,
80117
String expiryField,
81118
Environment env,
82-
List<String> fallbackCmd,
83-
List<String> forceCmd) {
84-
super();
85-
this.profileCmd = OSUtils.get(env).getCliExecutableCommand(cmd);
119+
boolean alreadyResolved) {
120+
this.attempts = attempts;
86121
this.tokenTypeField = tokenTypeField;
87122
this.accessTokenField = accessTokenField;
88123
this.expiryField = expiryField;
89124
this.env = env;
90-
this.fallbackCmd =
91-
fallbackCmd != null ? OSUtils.get(env).getCliExecutableCommand(fallbackCmd) : null;
92-
this.forceCmd = forceCmd != null ? OSUtils.get(env).getCliExecutableCommand(forceCmd) : null;
93125
}
94126

95127
/**
@@ -150,8 +182,6 @@ private Token execCliCommand(List<String> cmdToRun) throws IOException {
150182
if (stderr.contains("not found")) {
151183
throw new DatabricksException(stderr);
152184
}
153-
// getMessage() returns the clean stderr-based message; getFullOutput() exposes
154-
// both streams so the caller can check for "unknown flag: --profile" in either.
155185
throw new CliCommandException("cannot get access token: " + stderr, stdout + "\n" + stderr);
156186
}
157187
JsonNode jsonNode = new ObjectMapper().readTree(stdout);
@@ -167,54 +197,48 @@ private Token execCliCommand(List<String> cmdToRun) throws IOException {
167197
}
168198
}
169199

170-
private String getErrorText(IOException e) {
200+
private static String getErrorText(IOException e) {
171201
return e instanceof CliCommandException
172202
? ((CliCommandException) e).getFullOutput()
173203
: e.getMessage();
174204
}
175205

176-
private boolean isUnknownFlagError(String errorText, String flag) {
177-
return errorText != null && errorText.contains("unknown flag: " + flag);
178-
}
179-
180-
private Token execProfileCmdWithFallback() {
181-
try {
182-
return execCliCommand(this.profileCmd);
183-
} catch (IOException e) {
184-
String textToCheck = getErrorText(e);
185-
if (fallbackCmd != null && isUnknownFlagError(textToCheck, "--profile")) {
186-
LOG.warn(
187-
"Databricks CLI does not support --profile flag. Falling back to --host. "
188-
+ "Please upgrade your CLI to the latest version.");
189-
try {
190-
return execCliCommand(this.fallbackCmd);
191-
} catch (IOException fallbackException) {
192-
throw new DatabricksException(fallbackException.getMessage(), fallbackException);
193-
}
206+
private static boolean isUnknownFlagError(String errorText, List<String> flags) {
207+
if (errorText == null) {
208+
return false;
209+
}
210+
for (String flag : flags) {
211+
if (errorText.contains("unknown flag: " + flag)) {
212+
return true;
194213
}
195-
throw new DatabricksException(e.getMessage(), e);
196214
}
215+
return false;
197216
}
198217

199218
@Override
200219
public Token getToken() {
201-
if (forceCmd == null) {
202-
return execProfileCmdWithFallback();
220+
if (attempts.isEmpty()) {
221+
throw new DatabricksException("cannot get access token: no CLI commands configured");
203222
}
204223

205-
try {
206-
return execCliCommand(this.forceCmd);
207-
} catch (IOException e) {
208-
String textToCheck = getErrorText(e);
209-
if (isUnknownFlagError(textToCheck, "--force-refresh")
210-
|| isUnknownFlagError(textToCheck, "--profile")) {
211-
LOG.warn(
212-
"Databricks CLI does not support --force-refresh flag. "
213-
+ "Falling back to regular token fetch. "
214-
+ "Please upgrade your CLI to the latest version.");
215-
return execProfileCmdWithFallback();
224+
IOException lastException = null;
225+
226+
for (int i = 0; i < attempts.size(); i++) {
227+
CliCommand attempt = attempts.get(i);
228+
try {
229+
return execCliCommand(attempt.cmd);
230+
} catch (IOException e) {
231+
if (i + 1 < attempts.size() && isUnknownFlagError(getErrorText(e), attempt.usedFlags)) {
232+
if (attempt.fallbackMessage != null) {
233+
LOG.warn(attempt.fallbackMessage);
234+
}
235+
lastException = e;
236+
continue;
237+
}
238+
throw new DatabricksException(e.getMessage(), e);
216239
}
217-
throw new DatabricksException(e.getMessage(), e);
218240
}
241+
242+
throw new DatabricksException(lastException.getMessage(), lastException);
219243
}
220244
}

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

Lines changed: 43 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,47 @@ List<String> buildProfileArgs(String cliPath, DatabricksConfig config) {
7575
}
7676

7777
private static List<String> withForceRefresh(List<String> cmd) {
78-
List<String> forceCmd = new ArrayList<>(cmd);
79-
forceCmd.add("--force-refresh");
80-
return forceCmd;
78+
List<String> result = new ArrayList<>(cmd);
79+
result.add("--force-refresh");
80+
return result;
81+
}
82+
83+
List<CliTokenSource.CliCommand> buildAttempts(String cliPath, DatabricksConfig config) {
84+
List<CliTokenSource.CliCommand> attempts = new ArrayList<>();
85+
86+
boolean hasProfile = config.getProfile() != null;
87+
88+
if (hasProfile) {
89+
List<String> profileCmd = buildProfileArgs(cliPath, config);
90+
91+
attempts.add(
92+
new CliTokenSource.CliCommand(
93+
withForceRefresh(profileCmd),
94+
Arrays.asList("--force-refresh", "--profile"),
95+
"Databricks CLI does not support --force-refresh flag. "
96+
+ "Falling back to regular token fetch. "
97+
+ "Please upgrade your CLI to the latest version."));
98+
99+
if (config.getHost() != null) {
100+
attempts.add(
101+
new CliTokenSource.CliCommand(
102+
profileCmd,
103+
Collections.singletonList("--profile"),
104+
"Databricks CLI does not support --profile flag. Falling back to --host. "
105+
+ "Please upgrade your CLI to the latest version."));
106+
attempts.add(
107+
new CliTokenSource.CliCommand(
108+
buildHostArgs(cliPath, config), Collections.emptyList(), null));
109+
} else {
110+
attempts.add(new CliTokenSource.CliCommand(profileCmd, Collections.emptyList(), null));
111+
}
112+
} else {
113+
attempts.add(
114+
new CliTokenSource.CliCommand(
115+
buildHostArgs(cliPath, config), Collections.emptyList(), null));
116+
}
117+
118+
return attempts;
81119
}
82120

83121
private CliTokenSource getDatabricksCliTokenSource(DatabricksConfig config) {
@@ -90,23 +128,8 @@ private CliTokenSource getDatabricksCliTokenSource(DatabricksConfig config) {
90128
return null;
91129
}
92130

93-
List<String> profileCmd;
94-
List<String> fallbackCmd = null;
95-
List<String> forceCmd;
96-
97-
if (config.getProfile() != null) {
98-
profileCmd = buildProfileArgs(cliPath, config);
99-
forceCmd = withForceRefresh(profileCmd);
100-
if (config.getHost() != null) {
101-
fallbackCmd = buildHostArgs(cliPath, config);
102-
}
103-
} else {
104-
profileCmd = buildHostArgs(cliPath, config);
105-
forceCmd = withForceRefresh(profileCmd);
106-
}
107-
108-
return new CliTokenSource(
109-
profileCmd, "token_type", "access_token", "expiry", config.getEnv(), fallbackCmd, forceCmd);
131+
return CliTokenSource.fromAttempts(
132+
buildAttempts(cliPath, config), "token_type", "access_token", "expiry", config.getEnv());
110133
}
111134

112135
@Override

databricks-sdk-java/src/test/java/com/databricks/sdk/core/CliTokenSourceTest.java

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import java.time.format.DateTimeParseException;
2525
import java.util.ArrayList;
2626
import java.util.Arrays;
27+
import java.util.Collections;
2728
import java.util.HashMap;
2829
import java.util.List;
2930
import java.util.Map;
@@ -226,12 +227,28 @@ private CliTokenSource makeTokenSource(
226227

227228
private CliTokenSource makeTokenSource(
228229
Environment env, List<String> primaryCmd, List<String> fallbackCmd, List<String> forceCmd) {
230+
List<CliTokenSource.CliCommand> attempts = new ArrayList<>();
231+
232+
if (forceCmd != null) {
233+
attempts.add(
234+
new CliTokenSource.CliCommand(
235+
forceCmd, Arrays.asList("--force-refresh", "--profile"), "force-refresh fallback"));
236+
}
237+
238+
if (fallbackCmd != null) {
239+
attempts.add(
240+
new CliTokenSource.CliCommand(
241+
primaryCmd, Collections.singletonList("--profile"), "profile fallback"));
242+
attempts.add(new CliTokenSource.CliCommand(fallbackCmd, Collections.emptyList(), null));
243+
} else {
244+
attempts.add(new CliTokenSource.CliCommand(primaryCmd, Collections.emptyList(), null));
245+
}
246+
229247
OSUtilities osUtils = mock(OSUtilities.class);
230248
when(osUtils.getCliExecutableCommand(any())).thenAnswer(inv -> inv.getArgument(0));
231249
try (MockedStatic<OSUtils> mockedOSUtils = mockStatic(OSUtils.class)) {
232250
mockedOSUtils.when(() -> OSUtils.get(any())).thenReturn(osUtils);
233-
return new CliTokenSource(
234-
primaryCmd, "token_type", "access_token", "expiry", env, fallbackCmd, forceCmd);
251+
return CliTokenSource.fromAttempts(attempts, "token_type", "access_token", "expiry", env);
235252
}
236253
}
237254

0 commit comments

Comments
 (0)