fix: fall back to Spark when the driver cannot use the ABFS authentication by andygrove · Pull Request #7221 · apache/datafusion-comet

andygrove · 2026-10-25T15:31:29Z

Which issue does this PR close?

Closes #7202.

Part of #7204.

Rationale for this change

Since #6059, the native Parquet and CSV scans resolve ABFS authentication through Hadoop on the driver. When that resolution is something the native store cannot use, the scan still planned natively, and every task then failed with the native "unsupported" or driver error. 1.1.0 read these scans natively through the ambient identity, so this is a regression for 1.2.0.

All of these outcomes are known on the driver at planning time, so the scan can fall back to Spark's own reader, which authenticates through hadoop-azure, the same way it already does for scans that span more than one container or account.

What changes are included in this PR?

AbfsAuthResolver.nativeFallbackReason returns a fallback reason for:
- a mechanism Hadoop declines: Custom, UserboundSASWithOAuth, a custom SAS token provider, and Workload Identity with a custom client assertion provider;
- OAuth with RefreshTokenBasedTokenProvider or UserPasswordTokenProvider, which Hadoop resolves but object_store has no counterpart for;
- a client credentials endpoint the native store rejects, which is one that is not https or has no tenant path segment, and a Workload Identity authority that is not https;
- a resolution that fails on the driver, including a hadoop-azure that Comet cannot load or reflect into (missing, or a vendor fork). This also covers the environment-only configurations in the issue, since the handles load before the unconfigured check.
NativeConfig.extractScanObjectStoreOptions resolves once and returns that reason as a Left, which CometNativeScan and CometCsvNativeScanExec already turn into a fallback. Unconfigured accounts still read natively from the environment, and supported mechanisms still read natively with the same options as before.
The reason names the auth type and provider class, or carries Hadoop's already-redacted error. It names the URL as scheme, container and host, so a password in the URL stays out of it.
datasources.md now says these cases fall back instead of failing.
The native declined and error arms stay as a safety net. Native writes to ABFS (off by default) still fail with them.

#7127 touches the same files. Whichever lands second will need a small rebase.

How are these changes tested?

New NativeConfigSuite tests cover each case: Custom, a custom SAS provider, RefreshToken and UserPassword, an http or tenant-less client credentials endpoint, an http Workload Identity authority, an unknown auth type, a driver that cannot load hadoop-azure, and a URL password kept out of the reason. One more test checks that unconfigured and SharedKey scans still return native options. NativeConfigSuite, AbfsAuthResolverSuite and AbfsAuthParitySuite pass on the default Spark 4.1 profile.

…ation A native ABFS scan whose authentication Hadoop declines, resolves to a RefreshToken or UserPassword OAuth provider, or fails to resolve on the driver now falls back to Spark's own scan instead of planning natively and failing every task. Closes apache#7202

andygrove · 2026-10-25T15:33:40Z

@dwsmith1983 could you review?

dwsmith1983

Falling back on the driver is the right fix here. Every case in #7202 is known at planning time, and this uses the same Left path the multi-container decline already takes. I traced each case in the issue through resolve and nativeFallbackReason, and all of them fall back. Unconfigured and the supported mechanisms keep the options they had before.

A few things inline, and one more that doesn't attach to a line. The new tests call extractScanObjectStoreOptions directly, so nothing yet shows a plan over a single abfss:// container falling back with one of the new reasons. AbfsScanFallbackSuite already has the fake filesystem and checkSparkAnswerAndFallbackReason. One Parquet and one CSV case with fs.azure.account.auth.type=Custom, plus a NotAnAuthType case, would pin the end-to-end claim. They should read a single directory, since every new test except the password one uses two files.

On #7127: it changes the three Failed messages below to the password-free form and has a helper for the same authority rule as location here. Whichever of the two lands second can make nativeFallbackReason use it. I'll take care of it if this one goes in first.

dwsmith1983 · 2026-10-25T15:48:14Z

+            Some(c @ (Providers.REFRESH_TOKEN | Providers.USER_PASSWORD)),
+            _) =>
+        Some(unsupported(s"auth type ${AuthTypes.OAUTH} with provider $c"))
+      case Failed(_, message) =>


location leaves out the password, but message goes in as is. Three Failed messages in this file are built from the full URI: $uri has invalid authority. and Invalid URI '$uri' has a malformed authority... in authorityParts, and Invalid URI $uri - account name is not fully qualified. in sharedKey. For abfss://data:pw@accta/x.parquet with a key set for accta, sharedKey returns that last message, and the reason reads ... for abfss://data@accta: Invalid URI abfss://data:pw@accta/x.parquet - account name is not fully qualified. Hadoop reads data:pw as the container, so that URL doesn't read for Spark either, but the password still lands in text people share. Before this change the same text was in the task error. Now it is also in the extended explain output and in the fallback warning when spark.comet.explain.fallback.log.enabled is on.

The password test at line 785 uses Custom, so it takes the Declined arm and doesn't reach these messages. Could the Failed arm strip the user info as location does, or the three messages use the same form? A test with a SharedKey key, an account name without a dot and a password in the URL would cover it.

Good catch. The Failed arm of nativeFallbackReason now replaces the raw authority in Hadoop's message with the password-free one, so the sharedKey and authorityParts messages no longer carry it into the reason. I did it there and left the three messages alone because AbfsAuthParitySuite compares them with Hadoop's. The new test in NativeConfigSuite is the case you described: a SharedKey key for accta, a short account name and a password in the URL. It checks the reason has abfss://data@accta and not the password. Pushed in c2a7adb.

dwsmith1983 · 2026-10-25T15:49:55Z

+        Some(unsupported(s"auth type ${AuthTypes.OAUTH} with provider $c"))
+      case Failed(_, message) =>
+        Some(s"Hadoop ABFS authentication failed on the driver for $location: $message")
+      case _: Resolved | Unconfigured => None


This line decides between native and Spark for every OAuth account, but the "stay native" test at line 836 covers only Unconfigured and SharedKey. If the RefreshToken/UserPassword arm above were widened to Resolved(AuthTypes.OAUTH, Some(_), _), every ClientCreds, MSI and Workload Identity scan would fall back. Spark reads them fine, so nothing would fail. Could the test add one Resolved case per supported provider, plus SAS with a fixed token, each asserting Right with comet.azure.resolution=resolved? AbfsAuthResolverSuite already has the key sets for each.

Added. NativeConfigSuite now has a test with one Resolved case per supported mechanism (ClientCreds, MSI, Workload Identity, fixed SAS token), each asserting Right with comet.azure.resolution=resolved. Workload Identity and the fixed SAS token only run where hadoop-azure has them (3.4.1 and later). Widening the RefreshToken/UserPassword arm would now fail it.

dwsmith1983 · 2026-10-25T15:51:24Z

+    outcome match {
+      case Declined(authType, providerClass, assertionProviderClass) =>
+        val classes = providerClass.map(c => s" with provider $c").getOrElse("") +
+          assertionProviderClass.map(c => s" and client assertion provider $c").getOrElse("")


No test fails if this suffix is dropped. The resolver test for a Workload Identity client assertion provider runs only where hasClientAssertionProvider holds (hadoop-azure 3.5.0, the 4.2 profile), and it checks toOptions, not the reason. nativeFallbackReason only takes a URI and an Outcome. So a direct test on Declined("OAuth", Some(Providers.WORKLOAD_IDENTITY), Some("com.example.Assertion")) would run on every profile. The same goes for Declined("UserboundSASWithOAuth", None), which no NativeConfigSuite test names.

Added two direct nativeFallbackReason tests that run on every profile: Declined("OAuth", Some(WORKLOAD_IDENTITY), Some("com.example.Assertion")), which checks the provider and the client assertion provider suffix, and Declined("UserboundSASWithOAuth", None).

dwsmith1983 · 2026-10-25T15:52:54Z

 | `UserboundSASWithOAuth` (hadoop-azure 3.5.0 and later)                                                                         | Not supported |                                                                                                                                                                                                                                                                              |

-Custom provider classes hand each token request to Java code the native scan cannot call, and the refresh token and user password flows have no `object_store` counterpart. When Hadoop selects one of these, the scan fails with an error naming the auth type and, where one is configured, the class. Configure a supported mechanism for that account, or keep the path on Spark's own scan. Values arrive as Hadoop resolved them, trimmed and defaulted where the ABFS driver trims and defaults them, so the store sees what the driver would have used.
+Custom provider classes hand each token request to Java code the native scan cannot call, and the refresh token and user password flows have no `object_store` counterpart. When Hadoop selects one of these, the scan falls back to Spark's own scan, which authenticates through hadoop-azure, and the fallback reason names the auth type and, where one is configured, the class. Configure a supported mechanism for that account to read it natively. Values arrive as Hadoop resolved them, trimmed and defaulted where the ABFS driver trims and defaults them, so the store sees what the driver would have used.


"names the auth type and, where one is configured, the class": for Custom, resolve returns Declined(other, None), so the reason is only auth type Custom even with fs.azure.account.oauth.provider.type set. The Custom test sets that class but checks only the auth type. Either name the class for Custom too, or say here that the class is named for SAS and Workload Identity.

You're right. resolve returns Declined(Custom, None), so there is no class to name. I changed the docs instead of the code: the reason names the provider class for SAS, OAuth and Workload Identity, and Custom names only the auth type.

…reason Strip the password from the user info wherever a Failed message quotes it, cover the supported OAuth and SAS mechanisms staying native, and add end-to-end fallback tests for Parquet and CSV scans.

andygrove · 2026-10-25T16:12:20Z

Thanks @dwsmith1983. I added the end-to-end cases you asked for in AbfsScanFallbackSuite: Parquet and CSV over a single abfss:// directory, each with fs.azure.account.auth.type=Custom and with NotAnAuthType. They check the fallback reason and compare the answer with Spark's. Everything else is answered inline. Pushed in c2a7adb. And thanks for offering to switch nativeFallbackReason over to the #7127 helper if this lands first.

dwsmith1983 · 2026-10-25T16:21:42Z

Thanks @andygrove, these all look good to me.

comphead

Two points on the head after the first round. The second is a question about the endpoints the native store still rejects after a Resolved outcome.

comphead · 2026-10-26T15:17:07Z

-    }
-    val s3CompliantSchemes = parseSchemeSet(s3CompliantRaw)
+    val options = scala.collection.mutable.Map[String, String]() ++= schemeListOptions(hadoopConf)
+    val s3CompliantSchemes = parseSchemeSet(hadoopConf.get(COMET_S3_COMPLIANT_SCHEMES_KEY))


Now that the raw value is no longer kept here, this is the body of resolveS3CompliantSchemes at line 91, so it can call resolveS3CompliantSchemes(hadoopConf).

Done. objectStoreOptionsAndAliasOptions now calls resolveS3CompliantSchemes(hadoopConf). Pushed in 8c7c35d.

comphead · 2026-10-26T15:17:07Z

+      case Resolved(
+            AuthTypes.OAUTH,
+            Some(c @ (Providers.REFRESH_TOKEN | Providers.USER_PASSWORD)),
+            _) =>


This arm mirrors the provider check in resolved_settings in azure.rs, but that file still rejects some Resolved client credentials accounts. client_creds_settings errors when oauth_endpoint_parts finds no tenant segment, and datasources.md says an http:// client endpoint fails, since the store is built without allow_http. Hadoop posts to fs.azure.account.oauth2.client.endpoint as given, so Spark can read such an account while the native scan still plans and every task fails. That is the failure this PR removes for the other cases.

Could nativeFallbackReason also decline a client credentials endpoint that is not https or has no tenant segment? If those are left out on purpose, the new azure.rs doc line could say that scans can still reach these errors. I traced this from the code and have not run it.

I agree, and I traced the same two cases. nativeFallbackReason now declines a Resolved ClientCredsTokenProvider whose endpoint is not https or has no tenant segment, using the same tenant rule as oauth_endpoint_parts (the segment before the last oauth2, else the first). The check is conservative where java.net.URI and the native URL parser could disagree: a percent escape, a backslash or a dot segment falls back rather than being guessed at. The reason names the provider but not the endpoint, since the endpoint can carry user info.

NativeConfigSuite now has the fallback cases for an http endpoint and one with no tenant, a password-in-endpoint check, and a table of endpoints taken from the azure.rs unit test. datasources.md and the azure.rs doc line say these cases fall back. NativeConfigSuite, AbfsAuthResolverSuite and AbfsAuthParitySuite pass on the default Spark 4.1 profile. Pushed in 8c7c35d.

One related thing I left alone: Workload Identity passes fs.azure.account.oauth2.msi.authority to the store the same way, so an http:// authority there would fail natively too. I did not trace that case, so it is not in this change.

I traced the Workload Identity authority case I left out above, and it fails the same way. With an http:// fs.azure.account.oauth2.msi.authority, the store builds and then every token request fails with reqwest ... BadScheme, because object_store builds its client with https_only unless allow_http is set. I saw this in a throwaway azure.rs test, not just from reading the code, and the http:// client credentials endpoint failed identically. Hadoop posts to that authority over http, so Spark reads the account.

nativeFallbackReason now declines a Resolved WorkloadIdentityTokenProvider whose authority is not an https URL with a host. The reason leaves the authority out, as it does for the client credentials endpoint. The MSI provider is unaffected: object_store does not use the authority on that path. The new tests cover the fallback, a password in the authority staying out of the reason, and an https authority with a path staying native, and datasources.md and the azure.rs doc line now include it. Pushed in 5ee970e.

The check is conservative where java.net.URI and the native URL parser could disagree: a percent escape, a backslash or a dot segment falls back rather than being guessed at.

Should the fallback reason say so too? An endpoint like https://h/a/%2e%2e/t/oauth2/token is https and has a tenant segment, but it fails the check at line 535. The reason then tells the user it is "not an https URL with a tenant path segment", which sends them looking for the wrong problem. The native store builds for the percent escape and backslash cases, per the oauth_endpoint_parts test. Something like "an endpoint that is not an https URL with a tenant path segment and no percent escape, backslash or dot segment" would match the check. datasources.md could list these cases next to the http:// and no-tenant ones.

nativeFallbackReason now declines a Resolved ClientCredsTokenProvider whose endpoint is not https or has no tenant segment, using the same tenant rule as oauth_endpoint_parts

As I read it, nothing keeps the two copies of that rule in step. The test table is copied from the azure.rs cases, and the new azure.rs doc comment says scans never reach these errors. If oauth_endpoint_parts later rejects a shape that clientCredsEndpointUsable accepts, the scan plans natively and every task fails again, and neither suite notices.

Could the driver ask native instead of reimplementing the rule? There is precedent for a planning-time JNI probe in NativeBase.icebergStorageSchemes. A probe that runs resolved_settings plus an explicit https check would also give native ABFS writes a clear error up front instead of the BadScheme on each token request you saw. If that's more than this PR should take on, what do you think about one fixture file that both the Rust test and the Scala test read?

…e rejects nativeFallbackReason now declines a Resolved ClientCredsTokenProvider whose endpoint is not https or has no tenant segment, which azure.rs rejects, so the scan reads through Spark instead of failing every task. The reason leaves the endpoint out because it can carry user info. Also reuse resolveS3CompliantSchemes in objectStoreOptionsAndAliasOptions.

…ABFS scan object_store's token client refuses http, so a Resolved WorkloadIdentityTokenProvider whose fs.azure.account.oauth2.msi.authority is not https plans natively and fails every task, while Hadoop posts to it. nativeFallbackReason now declines it. The reason leaves the authority out because it can carry user info.

andygrove added 2 commits October 25, 2026 09:20

fix: keep a URL password out of the ABFS fallback reason

c33edf3

github-actions Bot added bug Something isn't working area:scan Parquet scan / data reading labels Oct 25, 2026

dwsmith1983 reviewed Oct 25, 2026

View reviewed changes

fix: keep a URL password out of Hadoop messages in the ABFS fallback …

c2a7adb

…reason Strip the password from the user info wherever a Failed message quotes it, cover the supported OAuth and SAS mechanisms staying native, and add end-to-end fallback tests for Parquet and CSV scans.

comphead reviewed Oct 26, 2026

View reviewed changes

andygrove added 2 commits October 26, 2026 09:29

Conversation

andygrove commented Oct 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

How are these changes tested?

Uh oh!

andygrove commented Oct 25, 2026

Uh oh!

dwsmith1983 left a comment

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!

andygrove commented Oct 25, 2026

Uh oh!

dwsmith1983 commented Oct 25, 2026

Uh oh!

comphead left a comment

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!

mbutrovich Oct 26, 2026 • 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!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

andygrove commented Oct 25, 2026 •

edited

Loading

mbutrovich Oct 26, 2026 •

edited

Loading