Repository navigation
[feature](storage) Support native GCP IAM authentication - #68540
Merged
Merged
Conversation
### What problem does this PR solve? Problem Summary: GCS access currently depends on HMAC credentials or unsigned requests. A deployment using Application Default Credentials or a Compute Engine service account cannot consistently use the same identity for object reads and writes, Iceberg metadata operations, storage vaults, and backend error-log downloads. Add native GCP OAuth authentication across FE, BE, and Cloud using `gs.credential_provider_type=DEFAULT` or `COMPUTE_ENGINE`, with optional `gs.impersonation_service_account`. GCP storage without explicit authentication defaults to Application Default Credentials. Public-bucket access requires explicit `ANONYMOUS`; existing HMAC configuration remains supported. Reject conflicting native, HMAC, and AWS-only authentication properties before creating clients. The implementation follows the current master architecture: - Keep credential resolution in the filesystem API, OAuth client construction in the GCS plugin, and Iceberg client integration in the Iceberg connector. Use the native Hadoop GCS connector for namespace operations and bearer-authenticated S3 XML clients for object operations. - Carry typed credentials through additive Thrift and Protobuf fields, Cloud object information, and the shared C++ S3 client factory. Preserve AWS authentication, client caching, and rate limiting. - Normalize GCS aliases for resources, serialize resource ALTER operations, and validate effective properties before publication. Preserve omitted vault credential fields during partial ALTER, support clearing impersonation explicitly, and reject anonymous storage vaults. Apply vault changes to candidate metadata so a failed update does not persist a partial rename. - Support `gs://` in EXPORT and OUTFILE while preserving HTTP paths. Generate backend GCS V4 error-log URLs through IAM `signBlob`, falling back to the local error-log path if signing fails. - Add unit coverage and IAM regression suites for catalogs, TVFs, load, export, outfile, backup/restore, resources, vaults, property validation, and anonymous access. Export tests read back the written data and isolate cleanup by run directory. Example native authentication properties: ```properties provider=GCP gs.credential_provider_type=DEFAULT gs.impersonation_service_account=target@project.iam.gserviceaccount.com ``` The impersonation property is optional. Use `COMPUTE_ENGINE` to select the attached VM identity explicitly, or `ANONYMOUS` for public object access. Compatibility and deployment: - Existing AWS/HMAC protocol fields and authentication paths remain available. Native GCP credentials require updated consumers; upgrade MetaService and the relevant FE/BE processes before enabling the feature during a rolling upgrade. - Package google-cloud-cpp OAuth libraries for BE/Cloud and the shaded Hadoop GCS connector for FE/Iceberg. - FE presigned uploads with native OAuth remain unsupported and fail explicitly. Backend error-log downloads use the dedicated GCS V4 signing path. - Native OAuth uses the existing configurable object-store endpoint, so this remains an outbound-request/SSRF surface. Catalog creation requires catalog CREATE privilege; vault creation requires global ADMIN privilege. Google token and signing operations use the credential library and Google IAM APIs. No remote code-loading path is introduced. ### Release note Add native GCP IAM authentication with Application Default Credentials, Compute Engine identities, and optional service-account impersonation for GCS-backed storage. GCP configurations without HMAC or an explicit authentication mode now use ADC; public access must select `ANONYMOUS`. Improve resource/vault credential updates and support GCS error-log signing. ### Validation Passed locally: - GCS filesystem reactor unit tests, including 88 filesystem API tests, 152 S3-base tests, and 36 GCS tests; zero failures, errors, or skips in these modules. - FE Core and Iceberg Checkstyle, plus the GCS reactor's style checks. - clang-format 16 checks for all 33 changed C++ files, BE build hygiene, connector import boundaries, and FE metadata-funnel checks. - Parsing of all 41 changed Java files; Protobuf descriptor generation and Thrift C++ generation for the changed definitions. - `git diff --check`. Not completed locally: - Full FE unit tests: the standard runner requires JDK 17; the available test runtime is JDK 20. - Iceberg reactor tests: generation stops because the installed Thrift compiler is 0.16.0 while master requires 0.24.0. The standalone Thrift syntax check above does not replace this build. - BE/Cloud compilation, unit tests, and clang-tidy: the installed thirdparty bundle lacks the required google-cloud-cpp libraries. - Live IAM regression suites: no configured Doris cluster or GCP test environment was available. ### Check List (For Author) - Test: - [x] Unit tests added; filesystem API/S3/GCS tests executed successfully. - [x] Regression tests added. - [ ] Full FE, Iceberg, BE/Cloud, and live IAM tests completed. - Behavior changed: - [x] Yes: native GCP authentication, ADC default, explicit anonymous mode, and validated partial credential updates as described above. - Does this need documentation? - [x] Yes: document credential properties, ADC deployment, impersonation permissions, explicit anonymous access, and upgrade requirements.
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
…ures ### What problem does this PR solve? Related PR: apache#68540 Problem Summary: Native GCP Hadoop access ignored the configured endpoint and HadoopCatalog selected S3A for s3/s3a warehouse aliases. Propagate the endpoint to the Google connector and normalize native HadoopCatalog warehouses to gs. The C++ client also sent anonymous object requests when token acquisition failed. Acquire bearer tokens before each object-storage SDK operation and return a non-retryable authentication error on missing or empty tokens. Add endpoint, warehouse alias, and authentication-failure coverage. ### Release note Native GCP catalogs honor custom endpoints and compatible warehouse schemes. Missing GCP tokens now fail locally instead of issuing anonymous requests. ### Check List (For Author) - Test: GCS module and upstream unit tests passed (37 GCS tests); FE Checkstyle, clang-format 16 on changed C++ files, build hygiene, and git diff --check passed. Iceberg tests stopped at the Thrift 0.16/0.24 mismatch; BE test setup stopped at the JDK 17 requirement. Further compilation and tests skipped at user request. Live GCP regression tests were not run. - Behavior changed: Yes, native Hadoop endpoint/scheme routing and token failure handling. - Does this need documentation: No
### What problem does this PR solve? Related PR: apache#68540 Problem Summary: Resources saved before GCS alias normalization can retain old gs.* properties. Normalizing only after merging an ALTER lets an old alias override a new s3.* value, silently ignoring credential rotation or restoring a session token that the user cleared. Normalize stored properties and the patch separately before merging, so the patch takes precedence while preserving gs.* priority within the same input map. Update the regression unit test to expect the new access/secret keys and the cleared session token. ### Release note GCP resource ALTER updates now override persisted gs.* aliases correctly. ### Check List (For Author) - Test: FE Core Checkstyle and git diff --check passed. Updated unit coverage for credential replacement, token clearing, unchanged properties, and same-input alias precedence. Compilation and tests skipped at user request. - Behavior changed: Yes, explicit resource updates override old alias values. - Does this need documentation: No
sollhui
marked this pull request as ready for review
September 28, 2026 07:38
sollhui
requested review from
924060929,
CalvinKirs,
Gabriel39,
englefly,
gavinchou,
liaoxin01,
luwei16,
morningman,
morrySnow and
starocean999
as code owners
September 28, 2026 07:38
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Request changes: five inline findings remain after two static review rounds. Three affect native GCP identity across Cloud stage retrieval, COPY INTO scan planning, and legacy instance AK/SK updates; two break IAM regression suites. The stage findings are independent: repairing the stage response alone does not repair COPY's separately built broker map. There was no additional user-provided review focus.
Critical checkpoints (code-review skill 1.3):
- Goal and proof: Native GCP OAuth is wired through FE, BE, Cloud, Iceberg, and error-log signing, but the three credential handoffs above prevent consistent identity use. Added builder and validation tests do not cover these end-to-end paths.
- Scope and clarity: The broad cross-module change generally reuses shared credential adapters. The Syncer helper removal also breaks an unchanged AWS suite.
- Concurrency: FE resource ALTER serializes snapshot and publication; the C++ token provider protects its cached generator. I found no separate lock-order, thread-context, or atomic-order defect.
- Lifecycle: Cached clients include the credential configuration and release the signing callback with the client. No cross-translation-unit static initialization or ownership-cycle issue was found.
- Configuration: The new GCP mode and impersonation properties are carried as persisted or transport fields. Vault partial ALTER preserves omitted fields; legacy instance key updates retain the native envelope incorrectly (inline finding). No new dynamic configuration knob needs a refresh path.
- Compatibility: The new Thrift and Protobuf fields are optional, and the PR declares that updated consumers are required before enabling native mode. The removed regression helper is an incompatible test-framework symbol (inline finding).
- Parallel paths and conditionals: MetaService's stage response and COPY's broker map each lose credential settings; both instance AK/SK update entry points retain native auth. Other auth conflict and anonymous-mode checks matched their documented behavior on static trace.
- Test coverage and results: Unit tests cover direct adapters, token/header behavior, and vault patching; no stage RPC/COPY scan or native-to-AK/SK instance transition test covers the findings. Two regression suites have deterministic failures under their supported AWS configurations. Builds, tests, and live IAM requests were prohibited by the review prompt, so this review makes no runtime validation claim; author-reported checks were not independently rerun.
- Observability and errors: Token and IAM signing failures are propagated or logged, with a local error-log fallback. I found no separate ignored Status, exception-boundary, or metric defect; wrong identity selection in the inline findings can still surface only downstream.
- Persistence, writes, and crashes: Vault ALTER validates a candidate and commits vault/instance metadata together. Legacy instance key updates persist a mixed auth state (inline finding). No other new write-atomicity or crash-recovery defect was substantiated.
- FE/BE propagation: Optional credential fields reach the regular adapters, but COPY's manually built scan map drops GCP mode and impersonation. The provider-less resource concern raised during review was dismissed after tracing GcpCredentialAdapter's downstream Thrift setter.
- Performance, memory, and BE nullable handling: No material new hot-path cost or memory ownership/reservation issue was substantiated. Vectorized ColumnNullable and MoW/versioned-read code are untouched by this PR.
Review complete after the final changed-file and duplicate sweep. Five distinct issues are left inline; no unresolved candidate remains. Static inspection only.
gavinchou
previously approved these changes
Sep 30, 2026
This was referenced Sep 30, 2026
### What problem does this PR solve? Related PR: apache#68540 Problem Summary: Clearing s3.role_arn previously erased static keys and native GCP credentials before persisting the vault. Only replace other credentials when selecting a nonempty AWS role or an explicit AWS credential provider. Cover encrypted static credentials, native GCP credentials, and repeated clears at the RPC/persistence boundary. Also use TableNameInfo in ExportCommandTest and restore the role repository helper still used by the legacy AWS suite. ### Release note Clearing an unused AWS role preserves the vault's active static or GCP credentials. ### Check List (For Author) - Test: clang-format 16 check, build hygiene, Java Checkstyle (JDK 20), git diff --check, and Groovy helper smoke checks passed. Added cloud RPC/persistence regression coverage. FE UT and cloud build could not start because the configured JDK 17 lacks lib/modules; cloud third-party build dependencies and a live Doris/AWS environment are also unavailable. Full unit/integration tests were not run. - Behavior changed: Yes, clearing an unused role preserves existing credentials. - Does this need documentation: No
…keys ### What problem does this PR solve? Related PR: apache#68540 Problem Summary: Legacy instance key updates retained native GCP credentials, so clients kept using OAuth after a successful AK/SK update. Clear the native envelope in both instance update helpers, including both user-id branches, and allow unchanged keys to repair an already persisted mixed auth state. Reject empty replacement keys before removing native authentication. Add RPC/persistence coverage for native-to-key transitions, mixed-state repair, and rejection without mutation. Guard optional AWS external-ID assertions so an omitted external ID does not fail the resource ALTER regression suite. ### Release note Replacing native GCP authentication with instance access keys now makes the new keys effective. AWS IAM resource regression tests accept an omitted external ID. ### Check List (For Author) - Test: Added regression coverage; compilation and test execution skipped at user request. Applied repository clang-format 16; git diff --check passed. - Behavior changed: Yes, instance key updates remove native authentication. - Does this need documentation: No
### What problem does this PR solve? Related PR: apache#68540 Problem Summary: Merge apache/doris master at 7ac0eb2 into the GCP IAM branch. Resolve concurrent test additions in OutFileTest and ExternalFileTableValuedFunctionTest by retaining both the GCP credential coverage and upstream UUID coverage as separate methods. Preserve the previous credential-transition and regression fixes. ### Release note None ### Check List (For Author) - Test: Targeted Java Checkstyle passed with zero violations. Verified both parents' test methods remain, no unresolved merge entries, and prior repair patches still apply in reverse. Conflict-file whitespace checks passed; upstream generated .out whitespace was retained. Compilation and test execution skipped at user request. - Behavior changed: No additional behavior change beyond merging master. - Does this need documentation: No
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
There was a problem hiding this comment.
Review outcome: request changes. This exact-head static review found six distinct new issues in the inline comments: two P1 and four P2. Existing P1 comments 4119930342 (internal-stage credential lost in get_stage) and 4119930347 (COPY INTO broker properties omit native GCP credentials) still apply and are carried as blockers without reposting. The other three existing threads were rechecked and their issues are fixed at this head.
Goal and test evidence: The change adds native GCP OAuth across FE, BE, Cloud object storage, Iceberg, export, and signed URLs. The implementation does not yet meet that goal on the token-boundary and Cloud mutation paths identified inline. Added BE, Cloud, FE, and regression test sources were inspected; builds and tests were prohibited by this review contract and were not run, so their results are not independently verified.
Scope, configuration, and compatibility: The change mostly uses the existing storage adapters and factories, and no unrelated edit was substantiated as an issue. The inspected FE-to-BE and FE-to-Cloud mappings retain native credential fields for supported Resource, Vault, Iceberg, LOAD, TVF, OUTFILE, EXPORT, and recycler paths. The role-only ADD_S3_VAULT RPC loses its previous default-provider compatibility. No new dynamically reloaded process setting was identified. Native credential protocol compatibility during rolling upgrades remains dependent on updated consumers as described in the PR; this static review cannot validate a mixed-version deployment.
Concurrency and lifecycle: Client cache misses construct outside the factory lock, and the new GCP consumers read the shared CA path without its lock. Token refresh ownership and synchronization, presigning callbacks, release paths, and cross-translation-unit initialization were inspected; no other substantiated lifecycle issue remained. Heavy client construction occurs outside the cache lock, and no new lock-order issue was found.
Validation, errors, and observability: The auth-mode predicates and caller-selected endpoint boundary were traced through request construction, including the concrete path-style SQL case. Cloud admission conditions are inconsistent for native ADD_OBJ_INFO and role-only vault ADD. Signed URL error handling, IAM response parsing, and existing logging were inspected; no further distinct status-propagation or observability issue was substantiated.
Persistence, writes, and parallel paths: Cloud root and derived credential changes use separate transactions; a derived failure followed by the same request cannot finish the cascade. The two existing stage/COPY paths still drop native credentials. Other inspected vault persistence, recycler reads, and FE/BE transport paths preserve the selected identity. Table visible-version, MoW delete bitmap, and rowset publication logic are not changed by this PR. No additional confirmed memory-accounting or hot-path performance issue was found; final glibc 2.17 binary compatibility cannot be established without a build artifact.
Completion: All 107 changed paths, the prior inline threads, and the unresolved-candidate list were swept after two review rounds; all round-two agents returned NO_NEW_VALUABLE_FINDINGS. The user focus file specified no additional focus.
Existing P0/P1 findings confirmed for this head: #68540 (comment), #68540 (comment)
### What problem does this PR solve? Related PR: apache#68540 Problem Summary: Native GCP clients attached process bearer tokens to arbitrary storage endpoints. Require Google Cloud Storage HTTPS endpoints on port 443 in FE properties and request interception, and in the shared BE/Cloud client before fetching tokens. Keep TLS verification enabled, disable native-client redirects, and reject AWS access-point ARNs in GCS bucket fields. HMAC and anonymous endpoint handling retain their existing behavior. Exclude the Doris cluster Auth-Token header from IAM signBlob and metadata HTTP requests. Admit validated native credentials for legacy ADD_OBJ_INFO requests, preserve role-only ADD_S3_VAULT requests with an omitted provider type, and pass the locked CA-path snapshot to both GCP consumers. ### Release note Native GCP OAuth requires a Google Cloud Storage HTTPS endpoint on port 443; untrusted custom endpoints and plaintext endpoints are rejected. Legacy object storage ADD accepts native GCP credentials, and role-only vault ADD continues to default to INSTANCE_PROFILE. ### Check List (For Author) - Test: Added FE, BE, and Cloud unit coverage. Java Checkstyle, clang-format 16, build hygiene, and git diff --check passed. An isolated check compiled the actual C++ endpoint validator and passed 14 endpoint cases. Full unit tests and BE/Cloud builds were not run: standard FE/BE test entry points stopped because JDK 17 is unavailable, and this checkout has no thirdparty bundle. - Behavior changed: Yes, endpoint restrictions and corrected credential admission. - Does this need documentation: Yes, native OAuth endpoint restrictions are described in the release note above.
### What problem does this PR solve? Related PR: apache#68540 Problem Summary: The pinned AWS SDK does not expose the four-argument constructor on S3Error, so GCP authentication failures prevent BE compilation. Construct AWSError<S3Errors> first and convert it to S3Error, preserving the error details and response codes. FE test compilation also fails because new S3 resource and vault tests use removed JMockit APIs. Migrate those mocks to scoped Mockito instances and retain the credential and no-RPC assertions. ### Release note None ### Check List (For Author) - Test: FE-core Checkstyle, clang-format 16, git diff --check, and PR-wide legacy JMockit scan passed. Constructor APIs verified against AWS SDK 1.11.221 headers. Full build and unit tests unavailable locally because JDK 17 and thirdparty dependencies are missing. - Behavior changed: No - Does this need documentation: No
Contributor
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
TPC-H: Total hot run time: 28355 ms |
Contributor
TPC-DS: Total hot run time: 150778 ms |
Contributor
ClickBench: Total hot run time: 25.37 s |
Contributor
FE Regression Coverage ReportIncrement line coverage |
### What problem does this PR solve? Related PR: apache#68540 The GCP IAM implementation used two public routing hooks and changed every filesystem selection entry point. Remove that generic routing dependency and restore the existing manager. Reuse dependency-free GCS property recognition in S3/MinIO predicates and reject native GCP keys when AWS/HMAC properties bind, including forced selection and operation without the GCS plugin. ### Release note GCP matching uses the existing filesystem SPI. Explicit S3 authentication and multiple filesystem selections are preserved. Remove the draft routing hooks and update the filesystem API surface/version to 6.0; native identity hooks remain and plugins must be rebuilt against the matching host API. ### Check List (For Author) - Test: 11 GcsAuthResolverTest cases passed through run-fe-ut.sh; Checkstyle passed for all affected modules; 14 Java files parsed; git diff --check passed. Added routing/binding regressions. The remaining standard FE run is blocked at fe-thrift by compiler 0.16.0 versus required 0.24.0; those tests did not run. - Behavior changed: Yes, GCP-specific selection/validation stays in plugins. - Does this need documentation: Yes, filesystem README updated.
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
There was a problem hiding this comment.
Request changes. Static review covered all 149 files at 0d4255d168e1eb96b43151b741dcbe87d23275a3, including the aggregate ALTER prerequisite changes. Two new P2 comments identify deterministic failures in added tests.
Three existing P1 findings remain applicable and are retained without duplicate comments: internal-stage credential loss, COPY scan credential loss, and derived-instance credential cascade retry. All 19 prior P1 comments were revalidated: the other reports are repaired or fall under the PR's explicitly documented support and rollout constraints.
Critical checkpoint conclusions:
- Goal and proof: ADC, Compute Engine and impersonation are carried through the shared authentication design. Existing stage/COPY/cascade defects prevent complete identity preservation. Tests are included; this invocation did not execute them.
- Focus and reuse: the changes share provider rules, typed identity descriptions, a common C++ factory and host-side wire translation. The broad scope follows the feature's FE/BE/Cloud paths.
- Concurrency: token access is serialized; resource ALTER serializes snapshot, validation and publication; policy serialization releases read locks in
finally. Remote validation occurs outside the resource write lock. No concrete new lock-order defect was established. - Lifecycle: callbacks own credential/configuration snapshots; client creation and reset carry native identity; TCCL restoration uses
finally. No new ownership cycle or cross-translation-unit initialization dependency was found. - Configuration: new credential properties preserve omitted versus explicitly cleared impersonation values. Resource versions and vault synchronization propagate successful updates. Added regression configuration is test-only.
- Compatibility: legacy static keys, explicit DEFAULT and provider aliases were checked. Omitted GCS authentication now selects ADC, as documented. Native activation requires upgrading every relevant consumer first; native FE presigned uploads and native Hudi/Paimon support are outside the stated support scope.
- Shared schemas: every touched definition was compared with branches 3.1, 4.0, 4.1 and 4.2. Parent field IDs 20 (PB) and 16 (Thrift), child IDs and enum values are compatible; new fields are optional.
- Parallel paths: TVF, LOAD, OUTFILE, EXPORT, backup, policy resources, vaults, stages, recycler and Iceberg were traced. The existing COPY map omission remains an exception.
- Conditional logic: CREATE defaults and presence-preserving ALTER patches were checked. Existing threads still cover native/HMAC resource transitions, empty session-token aliases and static-key vaults with dormant DEFAULT fields.
- Test coverage: added tests cover auth modes, validation, replay, batched policy pushes, concurrent ALTER, aliases, token exchange, signing, serialization and metadata/storage identity separation. The new MinIO and validation-message expectations contradict current code and are reported inline.
- Expected results: no
.outfiles changed. Assertions were checked statically; no build, test or live-service success is claimed. - Observability: token/client/signing failures have error paths and diagnostic logs; token values are not logged. No further concrete observability defect was established.
- Persistence and failover: resource journaling/replay and vault candidate validation were traced. Existing internal-stage serialization and derived-instance retry findings remain; rowset, visible-version and delete-bitmap semantics are untouched.
- Writes and atomicity: failed resource validation preserves the prior published state; rejected vault updates preserve authentication and rename state. The separately committed cascade remains the existing failure case.
- FE/BE transport: native fields reach property maps, policy Thrift, Cloud PB and serialized Iceberg FileIO; COPY's manually assembled broker map still omits them.
- Performance and memory: token refresh is cached, credential resolution scans bounded property sets, and the changes add no large data buffers or unbounded metadata loop. SDK closure/link additions were inspected statically.
- Further risks: existing threads already cover C++ ADC construction/refresh exceptions, HMAC Glue FileIO, mixed AWS/GCS locations and explicit HadoopFileIO aliases. They were verified and deduplicated; no unresolved candidate remains in this static review.
There was no additional user-provided focus. Runtime limits: builds/tests, live GCP authorization, packaged Java dependency closure and final ELF/glibc 2.17 compatibility were not validated. Review convergence describes completion of this bounded static review, not correctness of the PR or resolution of its existing findings.
Review converged after 2 round(s).
Existing P0/P1 findings confirmed for this head: #68540 (comment), #68540 (comment), #68540 (comment)
### What problem does this PR solve? Related PR: apache#68540 Problem Summary: The MinIO provider test expected a regional GCS endpoint to be claimed even though GCS endpoint detection correctly rejects that binding. Split private MinIO explicit-S3 coverage from global and regional GCS rejection checks. The GCP validation regression selected provider=S3 but expected an error from the GCS resolver. Match the error emitted by the selected S3 provider instead. ### Release note None ### Check List (For Author) - Test: 78 focused MinIO, S3, and GCS unit tests passed, with Checkstyle and git diff --check passing. Standard FE UT runner blocked by installed Thrift 0.16.0 versus required 0.24.0; live regression suite not run. - Behavior changed: No - Does this need documentation: No
Contributor
TPC-H: Total hot run time: 28279 ms |
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
There was a problem hiding this comment.
Static review of 7ebb8b33a19423b609f40a0f9e3d9955e62659b2: request changes for three independently revalidated existing P1 findings. No additional distinct finding survived verification and duplicate checks; no duplicate inline comments are posted.
Existing blockers retained:
- 4119930342:
get_stagereconstructs internal-stage object information without its native credential envelope (meta_service_resource.cpp:4250-4272), losing COMPUTE_ENGINE/impersonation selection. - 4119930347:
CopyIntoInfo.validateStagePB:418-439independently omits native identity from the broker map, so fixing the stage response alone does not fix backend COPY reads. - 4208967423: root key updates commit before derived-instance updates; equal-key retries return before cascade discovery, leaving a failed descendant on its old identity.
Known P2 concerns remain covered by existing comments: empty gs.session_token clearing (4246097912), resource HMAC/native transitions (4217149882), omitted-auth public configurations changing to ADC (4244585756), mixed AWS/GCS Iceberg S3 locations and explicit HadoopFileIO aliases (4236791176/4248820614), HMAC GCS with Glue role authentication (4358312794), ADC construction/refresh exceptions (4248820599/4356259111), and unrelated ALTER of a static-key vault with dormant DEFAULT (4248820622). These are verified existing issues, not unresolved new candidates. The pinned SDK still has throwing credential parsing/signing paths: ADC loader, OAuth refresh.
Other prior P1 dispositions: 4119930352, 4208967416, 4217149855, 4226469645, 4226856804, 4232295828, 4236791171, 4246097904, 4334086186, 4334086192 and 4357972632 are repaired for their reported triggers. Native FE presigned uploads (4215142986) and native Hudi/Paimon scanners (4226469640) remain explicitly deferred in the current PR description; no supported HMAC regression was substantiated there. The older-consumer concerns 4217722279/4217722285/4217722293 remain deployment restrictions: native authentication must be enabled only after MetaService and all relevant FE/BE/recycler consumers are upgraded. This review does not assert that native authentication works with old consumers.
Critical checkpoint conclusions:
- Goal and proof: ADC, Compute Engine and impersonation are wired through current FE/BE/Cloud object clients and error-log signing. The stage/COPY blockers mean the supported end-to-end identity flow is incomplete. Added tests were inspected; none were executed.
- Scope and reuse: the broad change is coherent with a cross-module feature; pure GCS rules, API identity descriptions, host wire codecs and shared C++ auth retain separate ownership. No additional unrelated change was identified.
- Concurrency: resource ALTER serializes snapshots/validation/publication; remote ping stays outside its read/write metadata lock. Policy pushes unlock in finally. Token refresh uses a dedicated mutex and client publication uses the holder lock; no concrete new lock inversion was found.
- Lifecycle and initialization: clients/providers/callbacks have bounded shared ownership, FileIO retains serializable configuration, and no new ownership cycle or cross-translation-unit static initialization dependency was found.
- Configuration: new native properties are per-resource/catalog/vault settings; explicit ALTER presence and impersonation clears are carried separately. No new production process setting requires dynamic-config propagation. Regression configuration is opt-in.
- Compatibility: current map, Thrift and protobuf consumers carry native identities; plugin major versions/baselines change. Enabling native authentication with older consumers remains unsupported under the documented upgrade procedure. Final linked ABI was not tested.
- Wire definitions: complete touched definitions were compared with live branch-4.2, branch-4.1, branch-4.0 and branch-3.1. Branch-4.2 matches the new definitions; older branches end before the new IDs. No ID reuse, changed enum meaning or new required field was found.
- Parallel paths: reviewed CREATE/ALTER/replay, map/PB/Thrift, new/reset clients, Cloud/shared-nothing, all used object operations, Iceberg metadata versus object authentication, sync/async FileIO and JNI packaging. Existing exceptions and documented exclusions are identified above.
- Conditional checks: provider precedence, incompatible credentials, endpoint rules and partial-field checks have concrete triggers. Alias-clear and mode-transition defects remain in existing threads.
- Test coverage: substantial unit and opt-in IAM regression coverage is added, including service-account OAuth exchange, scopes, token failures, credential transitions, rename rejection, URI handling and isolated factory reconstruction. Live cloud, mixed-version and final artifact behavior remain unverified.
- Expected results: no
.outfiles change. Inspected new assertions and repaired provider/error expectations; no additional deterministic expected-result defect was found. Some Groovy tests retain assertions/dynamic names/cleanup patterns contrary to preferred test conventions. - Observability and errors: token/IAM failures and object SDK outcomes provide diagnostics; signing failure retains the local log path. Existing uncaught ADC construction/refresh exceptions remain material error-handling defects. No additional critical missing metric/log was proved.
- Persistence and failover: resource properties remain journaled, native envelopes are persisted, and vault rename/auth validation uses candidate state before the KV commit. Cross-instance cascade retry is the retained persistence blocker. No rowset version or delete-bitmap contract changes.
- Writes and atomicity: failed validation/ping does not publish a new resource snapshot; failed vault credential changes do not persist the candidate rename. Native/static key replacement clears the old envelope. Existing worker-exception and cascade issues constrain failure recovery.
- FE-to-BE propagation: normal backend maps, policy Thrift and Cloud PB paths preserve provider/source/account and optional presence. Manual stage/COPY reconstruction is the retained exception.
- Performance: credentials and clients are cached; refresh is serialized separately from metadata locks. No additional substantiated asymptotic regression was found; no performance measurements were made.
- Final recheck: all 149 changed paths and the five initial risk mechanisms were covered by the main review, complete FE/native reviews and a separate credential-boundary review. Every candidate was deduplicated or dismissed with evidence; no new unresolved code question remains.
User focus: no additional focus was supplied. Validation limits: static source review only; no builds, tests, live GCS/IAM experiments, packaging execution, runtime classloader checks or final ELF/glibc 2.17 audit. Convergence describes review completeness, not approval or runtime correctness. Converged after two review rounds; the three existing P1 blockers remain outstanding.
Review converged after 2 round(s).
Existing P0/P1 findings confirmed for this head: #68540 (comment), #68540 (comment), #68540 (comment)
Contributor
TPC-H: Total hot run time: 35667 ms |
Contributor
ClickBench: Total hot run time: 24.94 s |
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
1 similar comment
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
FE Regression Coverage ReportIncrement line coverage |
1 similar comment
Contributor
FE Regression Coverage ReportIncrement line coverage |
gavinchou
approved these changes
Oct 26, 2026
CalvinKirs
approved these changes
Oct 26, 2026
englefly
approved these changes
Oct 26, 2026
5 of 16 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
GCS access currently relies on HMAC keys or unsigned requests. This PR adds native GCP IAM authentication across FE, BE, and Cloud, allowing Doris to access GCS using Application Default Credentials (ADC), a Compute Engine service account, or an impersonated service account.
Authentication and configuration
DEFAULTuses Google's Application Default Credentials chain.COMPUTE_ENGINEuses the service account attached to the VM.gs.impersonation_service_accountoptionally selects a target service account using the source identity's impersonation permissions.ANONYMOUS.Existing GCS HMAC authentication remains supported. Native IAM authentication rejects conflicting HMAC or AWS credential properties and requires a supported Google Storage HTTPS service endpoint.
How it works
gs://paths. Backend error-log downloads use GCS V4 URLs signed through IAMsignBlob.FE/Iceberg package the Hadoop GCS connector; BE/Cloud package the google-cloud-cpp OAuth libraries. The participating FE, BE, and MetaService processes must support the new credential fields before native IAM is enabled. Native IAM support for Paimon/Hudi and FE presigned uploads is outside this PR's scope.
Release note
Support native GCP IAM authentication for GCS using ADC, Compute Engine service accounts, and optional service-account impersonation. GCP configurations without HMAC credentials default to ADC; anonymous access must be selected explicitly.
Validation
git diff --checkpassed.Check List (For Author)