dwsmith1983 commented on code in PR #6059:
URL: https://github.com/apache/datafusion-comet/pull/6059#discussion_r4179987974
##########
native/core/src/parquet/objectstore/azure.rs:
##########
@@ -189,38 +1007,126 @@ fn account_scoped_value(
base_key: &str,
account: Option<&str>,
) -> Option<String> {
- if let Some(acc) = account {
- for suffix in ENDPOINT_SUFFIXES {
- let scoped = format!("{base_key}.{acc}.{suffix}");
- if let Some(v) = configs.get(&scoped) {
- return Some(v.clone());
- }
- }
- let bare = format!("{base_key}.{acc}");
- if let Some(v) = configs.get(&bare) {
- return Some(v.clone());
- }
+ account_scoped_entry(configs, base_key, account).map(|(_, value)| value)
+}
+
+/// Like `account_scoped_value`, but also returns the configuration key that
matched.
+fn account_scoped_entry(
+ configs: &HashMap<String, String>,
+ base_key: &str,
+ account: Option<&str>,
+) -> Option<(String, String)> {
+ let mut candidates = account_candidates(base_key, account);
+ candidates.push(base_key.to_string());
+ first_entry(configs, candidates)
+}
+
+/// The account-scoped forms of `base_key` in lookup order:
+/// `<base>.<account>.<endpoint-suffix>`, then `<base>.<account>`. Empty
without an account.
+fn account_candidates(base_key: &str, account: Option<&str>) -> Vec<String> {
+ let Some(acc) = account else {
+ return Vec::new();
+ };
+ ENDPOINT_SUFFIXES
+ .iter()
+ .map(|suffix| format!("{base_key}.{acc}.{suffix}"))
+ .chain(std::iter::once(format!("{base_key}.{acc}")))
+ .collect()
+}
+
+/// Look up a token provider class the way Hadoop's
`AbfsConfiguration.getTokenProviderClass`
+/// does: the account-scoped key first, then the global key only when the
global auth type
+/// is also the account's (`global_auth_type_matches`).
+fn token_provider_entry(
+ configs: &HashMap<String, String>,
+ base_key: &str,
+ account: Option<&str>,
+) -> Option<(String, String)> {
+ if let Some(entry) = first_entry(configs, account_candidates(base_key,
account)) {
+ return Some(entry);
+ }
+ if !global_auth_type_matches(configs, account) {
+ return None;
}
- configs.get(base_key).cloned()
+ configs
+ .get(base_key)
+ .map(|value| (base_key.to_string(), value.clone()))
}
-/// Resolve the SAS token for `(container, account)`, accepting any of the
-/// `fs.azure.sas.<container>.<account>[.<endpoint-suffix>]` variants.
-fn sas_value(configs: &HashMap<String, String>, container: &str, account:
&str) -> Option<String> {
- for suffix in ENDPOINT_SUFFIXES {
- let key = format!("{HADOOP_SAS_PREFIX}{container}.{account}.{suffix}");
- if let Some(v) = configs.get(&key) {
- return Some(v.clone());
- }
+/// Whether the global `fs.azure.account.auth.type` equals the one the account
resolves to,
+/// as `getTokenProviderClass` compares them. An unset global value is
`SharedKey`, the
+/// default of `getAccountAgnosticEnum`; an account without its own value
resolves to the
+/// global one, so the two always match then.
+fn global_auth_type_matches(configs: &HashMap<String, String>, account:
Option<&str>) -> bool {
+ let global = configs
+ .get(HADOOP_AUTH_TYPE)
+ .map_or("SharedKey", |value| value.trim());
+ first_entry(configs, account_candidates(HADOOP_AUTH_TYPE, account))
+ .is_none_or(|(_, scoped)| scoped.trim().eq_ignore_ascii_case(global))
+}
+
+/// The first `(key, value)` of `candidates` that is present in `configs`.
+fn first_entry(
+ configs: &HashMap<String, String>,
+ candidates: Vec<String>,
+) -> Option<(String, String)> {
+ candidates
+ .into_iter()
+ .find_map(|key| configs.get(&key).map(|v| (key, v.clone())))
+}
+
+/// `sas_token`, but only while SAS is the selected mechanism.
+fn active_sas_token(
+ configs: &HashMap<String, String>,
+ account: Option<&str>,
+ container: Option<UrlContainer<'_>>,
+) -> Option<(String, String)> {
+ if !mechanism_is_read(configs, account, AuthMechanism::Sas) {
+ return None;
}
- let bare = format!("{HADOOP_SAS_PREFIX}{container}.{account}");
- configs.get(&bare).cloned()
+ sas_token(configs, account, container)
+}
+
+/// Resolve the SAS token and the key it came from. Hadoop's
+/// `AbfsConfiguration.getSASTokenProvider` reads `fs.azure.sas.fixed.token`
through
+/// `getPasswordString`, which in Hadoop 3.4.2 and later probes
+/// `fs.azure.sas.fixed.token.<container>.<host>` first, with the URL's host
and port as
+/// `containerConf` builds it, then the account-scoped and global keys, as
+/// `account_scoped_entry` probes them. The WASB driver's
+/// `fs.azure.sas.<container>.<account>[.<endpoint-suffix>]` variants come
last as a fallback.
+fn sas_token(
+ configs: &HashMap<String, String>,
+ account: Option<&str>,
+ container: Option<UrlContainer<'_>>,
+) -> Option<(String, String)> {
+ let container_fixed = || {
+ let UrlContainer { name, host } = container?;
+ let key = format!("{HADOOP_SAS_FIXED_TOKEN}.{name}.{host}");
+ configs.get(&key).map(|value| (key, value.clone()))
+ };
+ let container_scoped = || match (container, account) {
+ (Some(UrlContainer { name, .. }), Some(account)) => {
+ let mut candidates: Vec<String> = ENDPOINT_SUFFIXES
+ .iter()
+ .map(|suffix|
format!("{HADOOP_SAS_PREFIX}{name}.{account}.{suffix}"))
+ .collect();
+ candidates.push(format!("{HADOOP_SAS_PREFIX}{name}.{account}"));
+ first_entry(configs, candidates)
+ }
+ _ => None,
+ };
+ container_fixed()
Review Comment:
> Could token selection and validation follow the runtime Hadoop behavior,
either through JVM resolution or a capability guard?
Done in 570b3753f with a capability guard. `NativeConfig` checks whether the
hadoop-azure on the classpath has `AbfsConfiguration.containerConf`, which
arrived with the container-scoped lookup in 3.4.2, and passes the answer to
native code as `comet.azure.containerScopedSasToken` in the abfs and abfss
object store options. The native scan reads and validates
`fs.azure.sas.fixed.token.<container>.<host>` only when that is `true`;
otherwise it follows 3.4.1 (account-scoped fixed token, then global) with the
WASB key as its own fallback. Your configuration now builds with the global
token on 3.4.1, and a non-blank container key no longer displaces it there. On
3.4.2 the blank key is still an error, as it is for Hadoop: `getPasswordString`
stops at the container key, `getTrimmedPasswordString` turns the blank value
into an empty token, and `getSASTokenProvider` throws.
Where the key is present but not read, it still keeps the environment out
while SAS keys are read, and if it is the only credential the scan fails
instead of building a store that would fall back to managed identity (the error
names the key when no auth type is set, and is the usual missing-token error
under `auth.type=SAS`); Hadoop 3.4.1 has no usable credential there either.
`NativeConfigSuite` checks the probe against the hadoop-azure version on each
profile: off for 3.3.4 and 3.4.1, on for 3.4.2 and 3.5.0.
--
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]
