dwsmith1983 commented on PR #6059: URL: https://github.com/apache/datafusion-comet/pull/6059#issuecomment-5888996821
> Could those count as absent under the providers that default them? Yes, in bc2136e. Under `MsiTokenProvider` a blank `msi.endpoint` or `msi.authority`, and under `WorkloadIdentityTokenProvider` a blank `msi.authority` or `token.file`, now count as unset, the way `getTrimmedPasswordString(key, default)` treats them; object_store's IMDS endpoint and authority host defaults are the same URLs. A blank token file falls to `AZURE_FEDERATED_TOKEN_FILE` as an unset one does (Hadoop's default is the fixed path `/var/run/secrets/azure/tokens/azure-identity-token`, which AKS sets that variable to). With no provider class the same blank keys are still errors. The tests also put `IDENTITY_ENDPOINT`, `AZURE_MSI_ENDPOINT` and `AZURE_AUTHORITY_HOST` in the environment and check none of them stands in for the blank key. > Would it make sense to prefer the fixed token when it is set and keep the container key as the fallback? Done. `sas_token` reads `fs.azure.sas.fixed.token` first and the container-scoped key only when no fixed token is set. Whichever key is selected must be non-blank: a blank fixed token is an error even with a container-scoped token, since Hadoop treats it as unset and fails SAS, and a blank container-scoped key is an error only without a fixed token. Example 4 now shows `auth.type=SAS` with the fixed token and notes the container-scoped key as a native-scan-only fallback. > Could that section say the provider class decides? It does now: no class, both keys read and `msi.tenant` wins; `ClientCredsTokenProvider`, `client.endpoint` alone; `MsiTokenProvider` and `WorkloadIdentityTokenProvider`, `msi.tenant` alone. One thing I noticed while checking `getPasswordString`: in 3.4.2 it probes `<key>.<container>.<account>` before `<key>.<account>` (3.4.1 and earlier do not), and that release's ABFS docs list `fs.azure.sas.fixed.token.CONTAINER_NAME.ACCOUNT_NAME`. The native scan's account-scoped lookup does not probe that container form for the fixed token. I have left that out of this PR; happy to add it here or file it separately. -- 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]
