fix: let explicit Hadoop Azure auth outrank ambient AZURE_* environment variables

[dwsmith1983]

Which issue does this PR close?

Closes #5542.

Rationale for this change

The native Azure store seeded its builder from the AZURE_* environment and then layered the translated Hadoop fs.azure.* keys on top. Same-key collisions resolved to Hadoop, but object_store's build() chooses the credential by a fixed order across keys, bearer token, then account key, then workload identity, then client secret, then SAS. An ambient AZURE_STORAGE_TOKEN or AZURE_STORAGE_ACCOUNT_KEY therefore outranked a configured Hadoop account key or service principal, and the AZURE_FEDERATED_TOKEN_FILE the AKS webhook injects into every annotated pod turned a configured client-secret principal into workload identity, dropping the secret. Hadoop's ABFS driver reads no environment variables, so the two readers could resolve different identities for the same table, surfacing as a 403 on tables the stock reader handles, or a read under a credential the job never configured.

The same fall-through existed inside object_store's own chain: a Hadoop mechanism Comet could not satisfy (a client secret without its client id and tenant, an auth.type it does not model, a blank value from a templated config) built a store anyway and object_store went on to the Azure CLI or the node's managed identity, again an identity the job never configured.

What changes are included in this PR?

• create_store builds through build_builder, which takes the environment as an explicit iterator and applies it under one of three policies decided from the translated Hadoop keys. When Hadoop names a mechanism for the account (an account key, a SAS token, a federated token file, a client secret, an MSI endpoint, an explicit fs.azure.account.auth.type, or a provider class ending in MsiTokenProvider), no AZURE_* variable and no IDENTITY_ENDPOINT is consulted at all, credentials and transport settings alike: an environment endpoint, proxy, invalid-certificate switch or emulator flag could otherwise redirect or intercept the configured identity. When Hadoop names only the identity (client id, tenant, authority host, or the WorkloadIdentityTokenProvider class with client id and tenant), the one variable read is AZURE_FEDERATED_TOKEN_FILE, and only when Hadoop supplies no token file itself; a principal with no token file from either source is an error naming both sources rather than a fall-through to managed identity. When Hadoop names nothing, the full environment applies as before, with IDENTITY_ENDPOINT applied last as MicrosoftAzureBuilder::from_env does.
• Hadoop configuration the native scan cannot honour is an error naming the exact key (account-scoped forms included) instead of a silent substitution: a client secret or token file without both client id and tenant; auth.type=SharedKey without an account key; auth.type=OAuth with no credential; a provider class other than the MSI, workload identity or client credentials providers, or one of those without the keys it needs (the class is validated whenever it is set, with or without auth.type); auth.type=Custom; auth.type=SAS without a SAS token; any other auth.type value; fs.azure.sas.token.provider.type, fs.azure.account.keyprovider, fs.azure.account.oauth2.refresh.token, fs.azure.account.oauth2.user.name and fs.azure.account.oauth2.user.password; and a blank or whitespace value for any translated credential or SAS key. Every such error carries the prefix Hadoop configuration for account <account>: and never includes a credential value.
• An explicit fs.azure.account.auth.type, resolved account-scoped over global as AbfsConfiguration.getAuthType does, also decides which keys are read. Only the selected mechanism's keys are translated and validated. An OAuth secret left in the configuration under an account-scoped SharedKey, or an account key under OAuth, is skipped the way Hadoop's initializeClient skips it, blank or not. fs.azure.account.keyprovider still belongs to SharedKey and the refresh token and user/password keys to OAuth, so each is rejected when its own mechanism is selected, and an incomplete credential for the selected mechanism remains an error. With no auth type and an account key present, Hadoop's default of SharedKey applies and only the key is read. With neither an auth type nor a key, every key is read as it was.
• fs.azure.sas.fixed.token (and its account-scoped forms) is translated to the SAS key, below the container-scoped fs.azure.sas.<container>.<account> key; a blank container-scoped key is an error and is not replaced by the fixed token.
• The storage account is taken from the URL host lowercased, since DNS resolves MyAcct.dfs.core.windows.net to the same account and Azure account names are lowercase, so a differently cased location can no longer miss an account-scoped key. The container is used as written, as Hadoop does.
• The module documentation and the Azure section of the data sources guide describe the precedence, the validated and rejected keys, and which environment variables apply under each policy.

User-facing changes: with a Hadoop mechanism configured, AZURE_ALLOW_HTTP, AZURE_PROXY_URL, AZURE_STORAGE_ENDPOINT, AZURE_USE_AZURE_CLI, AZURE_SKIP_SIGNATURE and the emulator flag no longer apply, and no fs.azure.* key is translated to them. With only an identity configured, only the token file is borrowed from the environment. Configurations that previously built a store and then ran under a substituted identity now fail at scan time with an error naming the key: an incomplete principal, an explicit auth.type the translated keys do not satisfy, the unsupported keys above, and blank values. A Hadoop-keyed way to set a proxy or emulator endpoint can be a follow-up if anyone needs it.

A ClientCredsTokenProvider configuration with a client id and secret but no fs.azure.account.oauth2.client.endpoint, whose tenant came only from fs.azure.account.oauth2.msi.tenant, used to build and now fails, which matches Hadoop, where AbfsConfiguration.getTokenProvider reads the endpoint with getMandatoryPasswordString(FS_AZURE_ACCOUNT_OAUTH_CLIENT_ENDPOINT) for that class.

How are these changes tested?

Unit tests in azure.rs drive build_builder and create_store_with_env with an explicit environment and inspect the builder with get_config_value before any store is built, so nothing reads or mutates the process environment and nothing touches the network. They map to the issue's three scenarios (env_bearer_token_is_ignored_when_hadoop_sets_account_key, env_account_key_is_ignored_when_hadoop_sets_client_secret_principal, env_federated_token_file_is_ignored_when_hadoop_sets_client_secret_principal), keep workload identity working from the environment when Hadoop names nothing (env_workload_identity_is_used_when_hadoop_has_no_auth) and borrow only the token file when Hadoop names the identity or the workload identity provider, with an ambient account key, bearer token and transport switch present and left unused. They pin the boundaries: transport settings ignored with Hadoop auth and applied without it, IDENTITY_ENDPOINT applied only without Hadoop auth and winning over AZURE_MSI_ENDPOINT in either order, the MSI provider class counting as a mechanism with and without surrounding whitespace, every explicit auth.type value accepted or rejected as documented, each unsupported key rejected in its global and account-scoped forms without borrowing the environment, the fixed SAS token translated and outranked by the container-scoped key, blank values rejected by exact key, the three-way account-scoped key precedence, and a mixed-case host resolving the account-scoped key. Every rejection test asserts the error names the key and the account and does not contain the credential value, and no test formats a built store. create_store is built end to end with an account key, SharedKey, SAS plus a fixed token, OAuth with a client-secret principal, and OAuth with the workload identity provider and a token file.

Before the fix, the "ignored" tests fail against unconditional environment seeding, the ordering and provider-class tests fail against the previous behaviour, and the rejection tests fail by building a store that carries object_store's managed identity or an unsigned client.

No Scala test is added: CI has no Azure account and the decision is entirely native; the Delta contrib declines abfss scans, so only the plain Parquet path is affected. No CI label is needed: no serde, planner, shim or diff-file change.

Run locally: cargo test -p datafusion-comet --lib azure, cargo clippy --all-targets -- -D warnings, cargo fmt, prettier on the docs page.

[sunchao]

Correctness

The existing builder loaded ambient Azure settings before overlaying Hadoop options. Same-key overrides worked, but object_store chooses among credential mechanisms in a fixed order. An ambient bearer token, account key or federated token file could therefore select a different mechanism from the one configured by the job. This change starts from a fresh builder when translated Hadoop settings select a mechanism and preserves environment loading when no mechanism is present.

The three reported conflicts are addressed. Complete Hadoop account-key, client-secret, workload-identity and SAS configurations exclude competing environment fields. Explicit MsiTokenProvider also suppresses them. Account-scoped translation and URL extraction are unchanged. Maintained Spark 3.5 and 4.0 sources confirm that spark.hadoop.* settings enter Hadoop configuration, and Comet forwards the relevant fs.azure.* options.

One P2 remains when a valid Hadoop OAuth configuration becomes incomplete during native option extraction. Hadoop can resolve the client ID and endpoint from a credential provider while the secret is a normal configuration entry. NativeConfig copies ordinary properties, so those provider-only fields do not reach native code. With matching ambient client and tenant values, the previous builder could use the configured service principal. The new suppression path instead falls through to the node's managed identity. Rejecting the incomplete translated credential would preserve the intended precedence rule without selecting another identity. The inline comment describes this specific case.

Validation

I checked both changed files, the native call path, the maintained Spark configuration paths, the corresponding Hadoop provider implementations and the checksum-verified object_store 0.13.2 source. Six standalone synthetic Rust checks passed. They execute the three extracted Comet helpers with a recording builder and a source-derived model of final credential selection. This is focused helper evidence, not execution of object_store or an end-to-end Comet test. The credential-provider configuration and its path through Hadoop listing and Comet option extraction were checked in source only. An earlier candidate with genuinely missing mandatory Hadoop fields was withheld because Hadoop rejects it before native execution. git diff --check passed. No real credential values, token files or storage endpoints were accessed.

Comet CI and CodeQL require approval and have zero jobs. The successful label job checked out the base commit and provides no product-test evidence. The PR's reported 25 Azure tests and lint results are author-reported. Required maintained Spark 3.4 and 4.1 branches were unavailable, so this review makes no compatibility claim for those versions.

Performance

The changed work occurs during Azure store construction. The new mechanism check scans the small translated option list, and the existing store cache and scan path are unchanged. I found no changed per-row or per-batch cost. Skipping environment application also avoids storing unused ambient fields in the builder, though env_pairs() still creates the process-environment iterator. No performance measurements were run.

Design

Separating environment application from Hadoop translation makes the precedence rule clear and permits deterministic tests without mutating process-wide environment variables. Preserving IDENTITY_ENDPOINT precedence in either iteration order matches the dependency. Suppressing transport settings alongside credentials is a deliberate behavior change, and the documentation calls out the affected endpoint, proxy, HTTP and emulator settings.

The remaining design boundary is an explicit OAuth credential whose fields are not all available to the native builder. Detecting that mechanism should ensure the builder either uses it or reports a configuration error. The documented IMDS fallback can instead select an identity that the Hadoop-side reader did not use.

Abstraction & complexity

The three private helpers are a reasonable size for this change. They keep environment parsing, mechanism detection and builder assembly separate without changing the shared object-store interface. The tests cover the principal precedence and ordering boundaries. I suggest extending the partial-configuration regression to assert rejection before an IMDS provider can be selected, rather than only inspecting which fields remain in the builder.

[sunchao]

Correctness

[P2] Reject incomplete translated OAuth credentials before selecting IMDS

Could we reject an incomplete translated OAuth credential before build() can select IMDS? A reachable case is Hadoop's ClientCredsTokenProvider with fs.azure.account.oauth2.client.secret in normal configuration, while the fs.azure.account.oauth2.client.id and fs.azure.account.oauth2.client.endpoint aliases live in a configured JCEKS credential provider. Hadoop's ABFS reads all three through getPassword(), so its credential is complete and file listing can succeed. NativeConfig.extractObjectStoreOptions only copies Configuration.iterator()/get(), so native receives the secret but neither the client ID nor tenant.

With matching AZURE_CLIENT_ID and AZURE_TENANT_ID, the previous native builder selects the same client-secret principal. This branch now removes those ambient fields. In the pinned object_store 0.13.2 builder, the incomplete tuple falls through to ImdsManagedIdentityProvider without a client-ID selector. The native read can then use the node's identity if it has access, or fail through IMDS, instead of reporting that the configured OAuth credential cannot be represented.

Ending environment completion is the intended policy. Could we return a configuration error at that boundary and add a regression checking the final provider/error selection? This only needs a fail-closed check, not native JCEKS support. I traced this case in source and checked the native selection boundary with the synthetic helper tests; I did not run a live Azure query.

[dwsmith1983]

Could we return a configuration error at that boundary and add a regression checking the final provider/error selection?

In 2b044b4. A client secret or token file without the client id and tenant is a configuration error naming the missing keys, raised before the builder can select IMDS, and the tests pin both the error text and that no provider is chosen for that configuration.

[sunchao]

Re-reviewed 2b044b48 against a7bdbe4f, including the three environment policies, explicit-provider validation, fixed-SAS translation, blank-value handling, and lowercase account lookup.

The previous P2 is fixed: the valid-Hadoop JCEKS case whose client ID and endpoint are absent from NativeConfig's map now fails with a configuration error before the builder can select IMDS.

One new P2 remains: validation also checks inactive OAuth settings after an account explicitly selects SharedKey, rejecting configurations that Hadoop and the previous native builder accept. Details and the concrete configuration are inline.

Validation: eight focused standalone Rust checks passed, using 25 helpers copied verbatim from the old/new revisions, a recording builder, and a selection model checked against the checksum-verified object_store 0.13.2 source. Hadoop dispatch and NativeConfig reachability were traced in source. This did not run a Comet/Spark integration test, Hadoop/JCEKS runtime, or Azure credential/network operation. Canonical Spark 3.4/4.1 branches were unavailable; no compatibility claim is made for those versions.

Current-head Comet CI, CodeQL, and title validation require approval and have zero jobs. The green label job checked out the base revision and provides no build/test coverage.

[sunchao]

Correctness

[P2] Validate only the authentication mechanism selected for this account

This now rejects a valid SharedKey configuration when inactive global OAuth settings remain in the Hadoop configuration. For example:

fs.azure.account.auth.type=OAuth
fs.azure.account.auth.type.myacct.dfs.core.windows.net=SharedKey
fs.azure.account.key.myacct.dfs.core.windows.net=<valid account key>
fs.azure.account.oauth.provider.type=org.apache.hadoop.fs.azurebfs.oauth2.ClientCredsTokenProvider
fs.azure.account.oauth2.client.secret=<unused OAuth secret>

Hadoop's getAuthType selects the account-scoped SharedKey override, and AzureBlobFileSystemStore.initializeClient calls only getStorageAccountKey, so the missing OAuth client ID/endpoint are irrelevant to this account. NativeConfig nevertheless forwards the global secret. The old native builder uses the supplied account key; here, after SharedKey validation succeeds, the presence of that inactive secret makes validate_translated require OAuth's client ID and tenant and fail before constructing the store.

I reproduced this with the exact old/new translation and validation helpers and a synthetic valid-base64 account key: old selection is AccessKey, while the new validator returns the missing client-ID/tenant error. Could we resolve the effective auth mechanism first and validate only its applicable credential fields/provider? The same rule should cover the earlier blank/unsupported-provider checks, while preserving rejection of incomplete active OAuth credentials.

[dwsmith1983]

Could we resolve the effective auth mechanism first and validate only its applicable credential fields/provider? The same rule should cover the earlier blank/unsupported-provider checks, while preserving rejection of incomplete active OAuth credentials.

Done in c127178. explicit_mechanism reads fs.azure.account.auth.type the way AbfsConfiguration.getAuthType does, with the account-scoped key winning over the global one. Translation, the blank-value check, the provider class check and the unsupported-key check then look only at the keys that belong to the selected mechanism. Your configuration builds with the account key, and the global secret never reaches the builder. fs.azure.account.keyprovider still belongs to SharedKey, and the refresh token and user/password keys to OAuth, so each is rejected as before when its own mechanism is selected. An incomplete credential for the selected mechanism remains an error, and with no auth type set every key is read as it was. The tests cover the scoped type overriding the global one in both directions, blank and unsupported keys of the other mechanisms under each type, the environment under an explicit type, and both rejections.

[dwsmith1983]

Could we cover Hadoop's default SharedKey case as well, while keeping environment-only configuration working?

Done in c0aeca8. With no fs.azure.account.auth.type, an account key entry now selects SharedKey, as AbfsConfiguration.getAuthType defaults to, so the unused client secret in your configuration is neither translated nor validated and the store builds with the key. A blank key is still reported as blank rather than skipped, fs.azure.account.keyprovider beside a key is still rejected, and the environment contributes nothing once a key is present. With neither an auth type nor a key, every mechanism's keys are read as before, so an OAuth-only or environment-only configuration keeps working. The tests cover your case, the SAS token and SAS provider class beside a key, the environment beside a key, the blank key, the key provider class, and the key-free OAuth case.

[sunchao]

Re-reviewed c127178c against 5ca14992. The new mechanism filtering fixes the explicit account-scoped SharedKey example, and the earlier JCEKS fail-closed fix still holds.

[P2] remains in the existing inactive-auth thread: with a valid fs.azure.account.key.myacct.dfs.core.windows.net, an unused fs.azure.account.oauth2.client.secret, and no fs.azure.account.auth.type, Hadoop defaults to SharedKey. Comet still treats the unset type as selecting every mechanism and rejects the configuration for missing OAuth client ID and tenant/endpoint. The new unset_auth_type_still_validates_every_mechanism test preserves this behavior. Could we cover Hadoop's default SharedKey case as well, while keeping environment-only configuration working?

Twelve isolated tests using exact copied helpers passed and confirmed both the explicit fix and this remaining case. I checked the configuration path against maintained Spark 3.5/4.0 and their Hadoop sources. These were helper tests and source checks, with no live storage authentication. Comet CI and CodeQL still require workflow approval and have run no jobs. No additional findings.

[andygrove]

The env gating in build_builder and env_policy fixes the three cases in #5542, and the env_*_is_ignored_* tests would fail against unconditional seeding, so I'm happy with the core change. Two things in the new Hadoop-matching logic go the other way from Hadoop, though.

extract_account now lowercases the account before the key lookup, and Hadoop doesn't. AzureBlobFileSystemStore.authorityParts takes uri.getRawAuthority() as written and AbfsConfiguration.accountConf appends it, so for abfss://data@MyAcct.dfs.core.windows.net Hadoop reads fs.azure.account.key.MyAcct.dfs.core.windows.net. With this change the native scan misses that key, finds no mechanism and applies the full environment, which is the same kind of identity split this PR is fixing. Could we keep the account as written, the way main did, and update mixed_case_host_resolves_account_scoped_key_and_borrows_no_env and the sentence in datasources.md to match?

blank_value_problem rejects a blank fs.azure.account.oauth2.client.id or fs.azure.account.oauth2.msi.tenant under MsiTokenProvider. On Hadoop 3.3.4, which Spark 3.4 and 3.5 ship, getTokenProvider reads both with getMandatoryPasswordString, which only rejects null, so a system-assigned identity has to set them to empty strings. Could those two be treated as absent under MsiTokenProvider, so the builder falls through to IMDS with no client id the way Hadoop does?

[dwsmith1983]

Could we keep the account as written, the way main did, and update mixed_case_host_resolves_account_scoped_key_and_borrows_no_env and the sentence in datasources.md to match?

extract_account keeps the first host label as written. For abfss://data@MyAcct.dfs.core.windows.net the native scan now looks up fs.azure.account.key.MyAcct.dfs.core.windows.net, the key Hadoop reads, and a key spelled myacct is not found for that host, which matches Hadoop too. The test is renamed to say that, a second one pins the lowercase key not matching, and the sentence in datasources.md now says the account is taken as written.

Could those two be treated as absent under MsiTokenProvider, so the builder falls through to IMDS with no client id the way Hadoop does?

blank_value_problem skips fs.azure.account.oauth2.client.id and fs.azure.account.oauth2.msi.tenant when the active provider is MsiTokenProvider, so empty strings there are absent and the builder goes to the managed identity endpoint with no client id. A blank client id under ClientCredsTokenProvider is still an error naming the key. The new test sets both to empty strings, with and without fs.azure.account.auth.type=OAuth, and checks the store builds and that AZURE_CLIENT_ID, AZURE_TENANT_ID, AZURE_STORAGE_TOKEN and AZURE_FEDERATED_TOKEN_FILE are all ignored. The user guide paragraph on blank values names the exception. Checked against both Hadoop versions Spark ships: 3.3.4 reads the two keys with getMandatoryPasswordString, which only rejects null, and 3.4.1 reads them with getPasswordString, so it does not even require them to be set. Hadoop's own abfs.md sample for MsiTokenProvider sets both to empty values.

[sunchao]

Summary

• Prior state and problem: Ambient Azure credentials could outrank explicitly configured Hadoop authentication.
• Design approach: Select an environment policy from Hadoop configuration, validate credentials, then construct the Azure store.
• Correctness / compatibility analysis: The original precedence conflicts and previously reported cases are addressed. Two new P2 regressions reject valid Hadoop configurations, detailed below.
• Key design decisions: Authentication also gates environment transport settings. Explicit auth types filter inactive mechanisms, and incomplete credentials fail before provider selection.
• Implementation sketch: Private helpers handle selection, translation, validation and builder assembly. The store cache remains unchanged. Added work occurs during store construction, with no changed per-row path. No performance benchmark was run.
• Behavioral changes worth calling out: Previously accepted configurations can now fail during store construction. Documentation describes the stricter policy, but validation still needs to distinguish individual OAuth providers and Hadoop’s built-in key provider.
• Suggested improvements: Validate only fields used by the selected OAuth provider and preserve explicitly configured SimpleKeyProvider behavior.

Reviewed the full two-file diff from 4453c57249aa4bf9c47fd4ff88bd13a09cd2a9f8 to 37036dba49cbf9141e03716bf12833b8ea2defe6. The PR remains non-draft. Routed skill: review-comet-pr. No sibling skill applies. Existing reviews, issue comments, inline comments and threads were read.

Validation: All 86 Azure module tests passed in an isolated harness using the actual object_store 0.13.2 dependency. Five additional base/head comparisons passed and reproduced the regressions. git diff --check passed. Checked Spark configuration forwarding for 3.4.3, 3.5.9, 4.0.4, 4.1.3 and 4.2.0, plus their corresponding Hadoop ABFS implementations.

Exact-head CI: Comet CI and CodeQL require approval and have zero jobs. The successful label workflow checked out the base commit. Validation did not include a full Comet build, Spark integration suite or live Azure authentication.

[sunchao]

[P2] Validate fields against the selected OAuth provider. A complete ClientCredsTokenProvider configuration now fails when an unused global fs.azure.account.oauth2.user.password remains in the configuration. Hadoop selects the provider class and reads only its client ID, secret and endpoint, so that password is irrelevant. The base revision builds the client-secret store, but this filter treats every OAuth field as active and rejects the configuration before scanning. The blank-value check similarly rejects an unused blank MSI endpoint. Could both checks use the resolved OAuth provider’s applicable fields, with regressions for shared configurations containing inactive OAuth settings?

Evidence: Executed exact base/head create_store implementations with object_store 0.13.2, an empty process environment and synthetic configuration: fs.azure.account.auth.type=OAuth, fs.azure.account.oauth.provider.type=org.apache.hadoop.fs.azurebfs.oauth2.ClientCredsTokenProvider, client ID synthetic-client, client secret synthetic-secret, client endpoint https://login.microsoftonline.com/synthetic-tenant/oauth2/token, and fs.azure.account.oauth2.user.password=unused-synthetic-password. Base returns Ok. Head returns fs.azure.account.oauth2.user.password selects an authentication mechanism the native scan does not support. Removing the unused field succeeds on both revisions. A separate comparison reproduces rejection of an unused empty fs.azure.account.oauth2.msi.endpoint. Hadoop 3.3.4, 3.4.1, 3.4.2 and 3.5.0 AbfsConfiguration.getTokenProvider() confirm neither field is read for ClientCredsTokenProvider. No network requests were made.

[dwsmith1983]

Done. Once fs.azure.account.oauth.provider.type resolves to ClientCredsTokenProvider, MsiTokenProvider or WorkloadIdentityTokenProvider, only the OAuth keys that class reads in AbfsConfiguration.getTokenProvider are translated or checked. Client credentials read the client endpoint, id and secret. MSI reads the MSI endpoint, tenant, client id and authority. Workload Identity reads the authority, tenant, client id and token file. Every other fs.azure.account.oauth2.* key is inactive for that account. It is neither validated nor handed to the builder, so an unused user.password or a blank msi.endpoint no longer rejects a complete client-credentials configuration. With no provider class, or one the scan rejects anyway, every OAuth key is still read as before.

Scoping the translation as well closes the same gap on the builder side. Under client credentials the tenant now comes from client.endpoint alone, as in Hadoop, so a global msi.tenant kept for another provider no longer replaces it. Under MSI an unused client secret no longer turns the managed identity into a client-secret credential. Under Workload Identity an unused MSI endpoint no longer stops the token file from being taken from AZURE_FEDERATED_TOKEN_FILE.

A ClientCredsTokenProvider configuration with a client id and secret but no fs.azure.account.oauth2.client.endpoint, whose tenant came only from fs.azure.account.oauth2.msi.tenant, used to build and now fails, which matches Hadoop, where AbfsConfiguration.getTokenProvider reads the endpoint with getMandatoryPasswordString(FS_AZURE_ACCOUNT_OAUTH_CLIENT_ENDPOINT) for that class.

The new tests cover each of those cases and a shared configuration carrying user-password, refresh-token and MSI settings beside an active client-credentials or Workload Identity provider. A blank secret under ClientCredsTokenProvider itself is still rejected.

[sunchao]

[P2] Preserve explicitly configured SimpleKeyProvider. Rejecting every fs.azure.account.keyprovider also rejects org.apache.hadoop.fs.azurebfs.services.SimpleKeyProvider, which is the same built-in provider Hadoop uses when this setting is omitted. With a normal account-scoped key, its behavior is already represented by the native AccessKey translation. Such configurations work with Hadoop and the base revision but now fail every native scan as unsupported. Could this built-in class be accepted when the account key is available, while retaining rejection of unsupported custom providers?

Evidence: A direct base/head comparison used fs.azure.account.auth.type=SharedKey, fs.azure.account.key.myacct.dfs.core.windows.net=AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=, and fs.azure.account.keyprovider=org.apache.hadoop.fs.azurebfs.services.SimpleKeyProvider. With an empty process environment, base store construction succeeds and head returns the unsupported-keyprovider error. Omitting only the provider setting succeeds on both revisions. Hadoop 3.3.4, 3.4.1, 3.4.2 and 3.5.0 getStorageAccountKey() instantiate this same class by default. Its implementation reads the configured account key through getPasswordString(). The reproduction used synthetic values and made no Azure requests.

[dwsmith1983]

Done. fs.azure.account.keyprovider=org.apache.hadoop.fs.azurebfs.services.SimpleKeyProvider is now treated the same as leaving the setting out, since getStorageAccountKey falls back to that class and it reads fs.azure.account.key. With the account key present it builds as SharedKey, with or without an explicit auth type and with the provider set globally or per account. Without the key it is an error that names fs.azure.account.key, because Hadoop's SimpleKeyProvider fails there too and the scan must not fall through to another credential.

The class name must match exactly. Hadoop passes the value untrimmed to Configuration.getClassByName, so a padded name is a class it cannot load and is rejected here as a custom provider. Any other class, including ShellDecryptionKeyProvider, a bare SimpleKeyProvider or a lookalike in another package, is still rejected. The user guide row for fs.azure.account.keyprovider now says this.

[sunchao]

Summary

• Prior state and problem: Ambient Azure credentials could outrank the authentication configured through Hadoop.
• Design approach: Resolve Hadoop authentication, validate its configuration, and apply one of three environment policies before building the store.
• Correctness / compatibility analysis: The original precedence conflicts are addressed. Two existing P2 concerns remain reproducible: valid client credentials fail when inactive OAuth fields are present, and a valid account key fails with explicitly configured SimpleKeyProvider. No additional introduced P1/P2 issues found within this review.
• Key design decisions: Explicit mechanisms suppress ambient credentials and transport settings. A named workload identity may borrow only its token file. Incomplete credentials fail before provider selection.
• Implementation sketch: Private helpers separate selection, translation, validation and builder assembly. The public interface and store cache remain unchanged. Added configuration lookups and environment collection occur during store construction, with no changed per-row path. No performance benchmark was run.
• Behavioral changes worth calling out: The PR adds fixed-SAS translation and stricter configuration errors. Documentation explains the environment restrictions. Validation still groups distinct OAuth providers too broadly.
• Suggested improvements: Address the existing OAuth-field validation concern and built-in key-provider concern. Validate fields used by the selected OAuth provider and accept SimpleKeyProvider when its account key is available.

Reviewed the full two-file diff from 88a1f48cc8a5c8f017016737c86f0912bbaefbe9 to 0f817daa75888bd771d4c98dde2dd752f58d8c28. The PR remains non-draft. Read AGENTS.md, existing reviews, issue comments, inline comments and threads. Routed skill: review-comet-pr. No sibling skill applies.

Validation: All 86 Azure module tests passed in an isolated harness importing the exact-head source with actual object_store 0.13.2. Five base/head comparisons passed, reproducing both existing blockers and confirming their control configurations succeed. git diff --check passed. Cross-checked Spark configuration forwarding for 3.4.3, 3.5.9, 4.0.4, 4.1.3 and 4.2.0 against their Hadoop ABFS sources.

Exact-head CI: Comet CI and CodeQL require approval and have zero jobs. The successful label workflow checked out the base commit. Validation did not include a full Comet build, Spark integration suite or live Azure authentication.

[sunchao]

Summary

• Prior state and problem: Ambient Azure credentials could outrank authentication configured through Hadoop.
• Design approach: Resolve the Hadoop mechanism and provider, validate their applicable fields, then apply one of three environment policies.
• Correctness / compatibility analysis: No introduced P1/P2 issues found within this review. The previously reported inactive-OAuth-field and explicit SimpleKeyProvider regressions are fixed. No substantiated existing blocker remains.
• Key design decisions: Explicit mechanisms suppress ambient credentials and transport settings. A named workload identity may borrow only its token file. Incomplete credentials fail before provider selection.
• Implementation sketch: Private helpers separate selection, translation, validation and builder assembly. The abstractions remain local to Azure configuration. Additional lookups and environment collection occur during store construction behind the unchanged cache, with no changed per-row path. No performance benchmark was run.
• Behavioral changes worth calling out: Fixed-SAS translation and stricter configuration errors are added. Provider-specific filtering ignores inactive OAuth fields, and the built-in SimpleKeyProvider is accepted with its account key. Documentation describes the environment restrictions.
• Suggested improvements: No additional P1/P2 changes requested.

Reviewed the full two-file base-relative diff from 88a1f48cc8a5c8f017016737c86f0912bbaefbe9 to 959a42c61fb551fed3ab496c3c7dec7de6596c91. The PR remains non-draft. Read AGENTS.md, existing reviews, issue comments, inline comments and review threads. Routed skill: review-comet-pr. No sibling skill applies.

Validation: All 98 Azure module tests passed in an isolated harness importing the exact-head source with actual object_store 0.13.2. Five independent base/head comparisons passed, confirming the recent regression cases and their controls now succeed. git diff --check passed. Cross-checked Spark configuration forwarding for 3.4.3, 3.5.9, 4.0.4, 4.1.3 and 4.2.0 against their corresponding Hadoop ABFS sources, verified against upstream.

Exact-head CI: Comet CI and CodeQL require approval and have zero jobs. The successful label workflow checked out the base commit. Validation did not include a full Comet build, Spark integration suite or live Azure authentication.

[dwsmith1983]

The OAuth handling now follows the resolved provider class. Only the keys that class reads are translated and checked, so a shared configuration with settings for other providers builds for the one the account selects, and the store sees only what Hadoop would read. The explicit SimpleKeyProvider is accepted with the account key and still rejected without it, and custom key providers are still rejected. The module docs and the user guide describe both rules.

A ClientCredsTokenProvider configuration with a client id and secret but no fs.azure.account.oauth2.client.endpoint, whose tenant came only from fs.azure.account.oauth2.msi.tenant, used to build and now fails, which matches Hadoop, where AbfsConfiguration.getTokenProvider reads the endpoint with getMandatoryPasswordString(FS_AZURE_ACCOUNT_OAUTH_CLIENT_ENDPOINT) for that class.

[dwsmith1983]

@andygrove could you approve a CI run on 8752c46ea? sunchao approved it after the provider-scoped OAuth checks and the SimpleKeyProvider change. CI has not run on this PR yet.