morningman opened a new pull request, #68117:
URL: https://github.com/apache/doris/pull/68117

   ### What problem does this PR solve?
   
   Issue Number: #68103
   
   Related PR: #67470
   
   Problem Summary:
   
   This is part 1 of the series tracked in #68103, which splits #67470 (route 
Azure data access
   through native Azure credentials, original implementation by @xylaaaaa) into 
independently
   reviewable PRs. Depends on nothing.
   
   Today the BE knows only one way to talk to Azure Blob Storage: a SharedKey 
pair carried in the
   AWS-shaped `AWS_ACCESS_KEY` / `AWS_SECRET_KEY` fields, with `provider=azure` 
as the only hint.
   There is no way to pass a SAS token or an OAuth2 client credential, `S3URI` 
cannot parse
   `abfs[s]://` / `wasb[s]://` locations, and a SAS token appended to a URL 
ends up in logs and
   error messages. This PR is the BE consumer side of the native Azure 
protocol. The FE producer
   side comes later in the series (FE-4), so nothing changes for existing 
deployments until a new
   FE starts emitting the native fields.
   
   What this PR does:
   
   - `AzureCredentialOptions` (new header 
`common/cpp/obj-client/auth/azure_credential_options.h`)
     is the provider-owned credential model: `SHARED_KEY` / `SAS` / `OAUTH2`, 
with `validate()`
     rejecting mixed credential groups. It has no Azure SDK dependency so 
`S3ClientConf` can carry
     it in builds with `BUILD_AZURE=OFF`.
   - `S3ClientConf` gains an `azure_credentials` member. `operator==`, 
`get_hash()` and
     `to_string()` are now provider-aware: for Azure the client identity is the 
Azure credential
     group plus endpoint / bucket / timeouts, and the AWS fields (region, role, 
signing settings)
     no longer participate. `to_string()` never prints a SAS token.
   - `S3ClientFactory::convert_properties_to_s3_conf` dispatches on 
`provider=azure`:
     - a map with `AZURE_AUTH_TYPE` is the native protocol (`AZURE_ENDPOINT`, 
`AZURE_ACCOUNT_NAME`,
       `AZURE_ACCOUNT_KEY`, `AZURE_SAS_TOKEN`, `AZURE_SAS_EXPIRY_MS`, 
`AZURE_CLIENT_ID`,
       `AZURE_CLIENT_SECRET`, `AZURE_TENANT_ID`, `AZURE_OAUTH_SERVER_URI`, 
`AZURE_CONTAINER`);
     - a map without it is the legacy SharedKey map (`AWS_ENDPOINT` / 
`AWS_ACCESS_KEY` /
       `AWS_SECRET_KEY`), handled by `convert_legacy_azure_properties` with 
unchanged behavior;
     - for native SharedKey the legacy `AWS_*` fields may also be present 
(rolling upgrade) and
       must agree with the `AZURE_*` fields.
     - `validate_azure_uri` checks that the data URI's container, account and 
host agree with the
       binding. OneLake hosts (`*.fabric.microsoft.com`) are refused by the 
native client and stay
       on their Hadoop binding.
   
     `S3Conf::get_s3_conf(ObjectStoreInfoPB)` / `get_s3_conf(TS3StorageParam)` 
(storage vault path)
     move ak/sk into `azure_credentials` for the Azure provider, so the vault 
path keeps working
     unchanged.
   - `AzureAuthFactory::create` builds the container client for all three 
credential types:
     SharedKey (`StorageSharedKeyCredential`), SAS (token appended to the 
container URL) and
     OAuth2 (`Azure::Identity::ClientSecretCredential`; the tenant comes from 
`AZURE_TENANT_ID` or
     is derived from the OAuth server URI). `AzureAuthFactory::validate` parses 
the SAS `se=`
     field and combines it with `AZURE_SAS_EXPIRY_MS` into the earliest 
effective expiry.
   - Azure clients live in a dedicated bounded cache (`_azure_cache`: capacity 
256, the least
     recently accessed entry is evicted, expired SAS entries are pruned on 
every access). Expiry
     is re-checked on every `create()`, so an expired SAS is refused before a 
new reader or writer
     is opened. The existing `_cache` for non-Azure providers is untouched.
     `S3ClientFactory::validate_credentials_for_access` is the entry point the 
next PR (BE-2)
     uses at reader/writer open time. `ObjClientHolder::reset` now copies 
`azure_credentials`, so a
     storage-vault credential rotation replaces the Azure client instead of 
being ignored.
   - `S3URI` parses `abfs[s]://[email protected]/key` and
     `wasb[s]://...`, recognizes Azure HTTP(S) hosts (`*.blob.core.windows.net`,
     `*.dfs.core.windows.net` and the China / US Gov / Germany cloud suffixes), 
exposes
     `get_endpoint()` / `get_account()` / `get_scheme()` / `is_azure()`, and 
percent-decodes only
     HTTP(S) Azure paths (ABFS/WASB object names are literal, matching Iceberg 
`ADLSLocation`).
     Parse errors for Azure locations no longer echo the location.
   - SAS redaction: the Azure SDK logger listener and every 
`AzureObjStorageClient` error path
     strip the query component of URLs before the message is logged or returned.
   - `AzureObjStorageClient::generate_presigned_url` signs only with a 
SharedKey credential and
     returns an empty string for SAS/OAuth2; 
`RuntimeState::get_error_log_file_path` then keeps
     the local error log path instead of publishing an empty URL. The signed 
URL is now built from
     the SDK blob client URL, so a literal `%2F` in the object name signs the 
right blob.
   - `AzureObjStorageClient::abort_multipart_upload` becomes a no-op instead of 
deleting the blob.
   
   Behavior / compatibility:
   
   - Old FE, new BE: `AWS_ENDPOINT` / `AWS_ACCESS_KEY` / `AWS_SECRET_KEY` + 
`provider=azure` keeps
     working through `convert_legacy_azure_properties`; Azure storage vault 
configs are unchanged.
   - An Azure host URI (`abfs://...`, `https://x.blob.core.windows.net/...`) 
without
     `provider=azure` is now rejected instead of being treated as S3.
   - `abort_multipart_upload` on Azure no longer deletes the target blob. Azure 
has no server-side
     multipart session; blocks are isolated per upload UUID, and deleting the 
blob could remove
     previously committed data or another writer's staged blocks.
   - Presigned URL generation is refused for SAS/OAuth2 credentials (a 
container-scoped SAS would
     otherwise be republished), and the error-log path falls back to the local 
file.
   - Native OAuth2: the BE builds the client with 
`Azure::Identity::ClientSecretCredential` from
     `AZURE_CLIENT_ID` / `AZURE_CLIENT_SECRET` / `AZURE_TENANT_ID` (or the 
tenant in
     `AZURE_OAUTH_SERVER_URI`). The description of #67470 said native OAuth2 
"fails closed"; this
     PR describes what the code does. User-delegation SAS signing for OAuth2 is 
not implemented, so
     presigned URLs are unavailable in that mode.
   
   Rolling upgrade: a new FE emits both the native `AZURE_*` fields and the 
matching legacy `AWS_*`
   group for SharedKey, so old BEs keep working; new BEs verify that the two 
groups agree. SAS and
   OAuth2 have no legacy wire form: upgrade all BEs before enabling native 
SAS/OAuth2 from a new FE,
   and stop native SAS/OAuth2 use before rolling back.
   
   Review focus:
   
   1. `S3ClientConf::operator==` / `get_hash()` are now per-provider.
   2. `_azure_cache` (capacity 256, access-ordered eviction, expiry pruning) 
coexists with `_cache`.
   3. `abort_multipart_upload` semantic change (no-op instead of delete).
   4. `generate_presigned_url` signs only under SharedKey.
   5. `convert_properties_to_s3_conf` rejects Azure host URIs without 
`provider=azure`.
   6. OAuth2 wording versus code, see above.
   
   ### Release note
   
   None
   
   ### Check List (For Author)
   
   - Test
       - [x] Unit Test
           - macOS Debug BE UT, filter 
`AzureAuthFactoryTest.*:AzureObjStorageClient*Test.*:AzureClientFactoryCacheTest.*:S3ClientFactoryTest.*:S3URITest.*:S3FileSystemTest.*`:
 121 tests, 84 passed, 37 skipped (34 `S3FileSystemTest` need a live bucket, 3 
need Azurite), 0 failed
           - second pass `*S3*:*Azure*:*Rowset*` (guards every `S3ClientConf` 
designated-initializer site): 341 tests, 296 passed, 44 skipped, 1 failed. The 
failure (`S3ObjStorageClientMockTest.test_ca_cert`) and an exit-time abort from 
`S3FileWriterTest` are pre-existing macOS-only issues in files this PR does not 
touch (Linux CA bundle paths; a leaked static `S3FileSystem` destroyed after 
`Aws::ShutdownAPI`)
           - `build-support/check-format.sh` clean; the four `common/cpp` files 
clang-format 16 clean
       - [ ] Regression test
       - [ ] Manual test (add detailed scripts or steps below)
   
   - Behavior changed:
       - [ ] No.
       - [x] Yes. Azure `abort_multipart_upload` no longer deletes the blob; 
presigned URLs are only generated for SharedKey; Azure host URIs require 
`provider=azure`.
   
   - Does this need documentation?
       - [x] No.
       - [ ] Yes.
   
   ### Check List (For Reviewer who merge this PR)
   
   - [ ] Confirm the release note
   - [ ] Confirm test cases
   - [ ] Confirm document
   - [ ] Add branch pick label
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
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]

Reply via email to