andygrove commented on code in PR #7221:
URL: https://github.com/apache/datafusion-comet/pull/7221#discussion_r4371472718
##########
spark/src/main/scala/org/apache/comet/objectstore/AbfsAuthResolver.scala:
##########
@@ -467,6 +469,38 @@ private[comet] object AbfsAuthResolver extends Logging {
Markers.ERROR_MESSAGE -> message)
}
+ /**
+ * Why the native store cannot read `uri` under `outcome`, or None when it
can: a mechanism
+ * Hadoop named that runs Java code per request, a built-in OAuth provider
with no
+ * `object_store` counterpart, or a resolution that failed. All are known on
the driver, so the
+ * scan falls back to Spark's own reader, which authenticates through
hadoop-azure, instead of
+ * planning natively and failing every task.
+ */
+ def nativeFallbackReason(uri: URI, outcome: Outcome): Option[String] = {
+ // The container and host only: a password in the user info stays out of
the reason.
+ val authority = Option(uri.getRawAuthority).getOrElse("")
+ val at = authority.lastIndexOf('@')
+ val location = s"${uri.getScheme}://" +
+ (if (at < 0) authority
+ else authority.substring(0, at).takeWhile(_ != ':') +
authority.substring(at))
+ def unsupported(mechanism: String): String =
+ s"Native scan does not support ABFS authentication $mechanism for
$location"
+ 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("")
+ Some(unsupported(s"auth type $authType$classes"))
+ case Resolved(
+ AuthTypes.OAUTH,
+ Some(c @ (Providers.REFRESH_TOKEN | Providers.USER_PASSWORD)),
+ _) =>
+ Some(unsupported(s"auth type ${AuthTypes.OAUTH} with provider $c"))
+ case Failed(_, message) =>
Review Comment:
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 c2a7adb75f.
##########
spark/src/main/scala/org/apache/comet/objectstore/AbfsAuthResolver.scala:
##########
@@ -467,6 +469,38 @@ private[comet] object AbfsAuthResolver extends Logging {
Markers.ERROR_MESSAGE -> message)
}
+ /**
+ * Why the native store cannot read `uri` under `outcome`, or None when it
can: a mechanism
+ * Hadoop named that runs Java code per request, a built-in OAuth provider
with no
+ * `object_store` counterpart, or a resolution that failed. All are known on
the driver, so the
+ * scan falls back to Spark's own reader, which authenticates through
hadoop-azure, instead of
+ * planning natively and failing every task.
+ */
+ def nativeFallbackReason(uri: URI, outcome: Outcome): Option[String] = {
+ // The container and host only: a password in the user info stays out of
the reason.
+ val authority = Option(uri.getRawAuthority).getOrElse("")
+ val at = authority.lastIndexOf('@')
+ val location = s"${uri.getScheme}://" +
+ (if (at < 0) authority
+ else authority.substring(0, at).takeWhile(_ != ':') +
authority.substring(at))
+ def unsupported(mechanism: String): String =
+ s"Native scan does not support ABFS authentication $mechanism for
$location"
+ 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("")
+ Some(unsupported(s"auth type $authType$classes"))
+ case Resolved(
+ AuthTypes.OAUTH,
+ Some(c @ (Providers.REFRESH_TOKEN | Providers.USER_PASSWORD)),
+ _) =>
+ 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
Review Comment:
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.
##########
spark/src/main/scala/org/apache/comet/objectstore/AbfsAuthResolver.scala:
##########
@@ -467,6 +469,38 @@ private[comet] object AbfsAuthResolver extends Logging {
Markers.ERROR_MESSAGE -> message)
}
+ /**
+ * Why the native store cannot read `uri` under `outcome`, or None when it
can: a mechanism
+ * Hadoop named that runs Java code per request, a built-in OAuth provider
with no
+ * `object_store` counterpart, or a resolution that failed. All are known on
the driver, so the
+ * scan falls back to Spark's own reader, which authenticates through
hadoop-azure, instead of
+ * planning natively and failing every task.
+ */
+ def nativeFallbackReason(uri: URI, outcome: Outcome): Option[String] = {
+ // The container and host only: a password in the user info stays out of
the reason.
+ val authority = Option(uri.getRawAuthority).getOrElse("")
+ val at = authority.lastIndexOf('@')
+ val location = s"${uri.getScheme}://" +
+ (if (at < 0) authority
+ else authority.substring(0, at).takeWhile(_ != ':') +
authority.substring(at))
+ def unsupported(mechanism: String): String =
+ s"Native scan does not support ABFS authentication $mechanism for
$location"
+ 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("")
Review Comment:
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)`.
##########
docs/source/user-guide/latest/datasources.md:
##########
@@ -479,15 +479,15 @@ hadoop-azure must be on the driver classpath. It already
is whenever Spark can l
| `WorkloadIdentityTokenProvider` with a
`fs.azure.account.oauth2.client.assertion.provider.type` (hadoop-azure 3.5.0
and later) | Not supported |
|
| `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.
Review Comment:
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
