github-actions[bot] commented on code in PR #68540:
URL: https://github.com/apache/doris/pull/68540#discussion_r4217722279
##########
gensrc/proto/cloud.proto:
##########
@@ -320,6 +338,8 @@ message ObjectStoreInfoPB {
optional CredProviderTypePB cred_provider_type = 17;
optional string role_arn = 18; // aws assumed role's arn
optional string external_id = 19; // aws assumed role's external_id if
configure
+
+ optional ObjectStoreCredentialPB credential = 20;
Review Comment:
[P1] Gate native GCP vault creation on consumer support. This new field is
the only OAuth auth material stored for ADC/impersonation vaults, with empty
AK/SK. During a rolling upgrade, older BE and recycler binaries ignore field 20
and build ordinary AWS S3 clients; a new FE/MS can publish such a vault without
a capability gate, so private GCS reads/writes and recycling fail on those
nodes. Gate native vault/instance publication until all consumers understand
this field, or provide a compatible rollout path.
##########
gensrc/thrift/AgentService.thrift:
##########
@@ -127,6 +144,7 @@ struct TS3StorageParam {
13: optional TCredProviderType cred_provider_type
14: optional string role_arn // aws assumed role's arn
15: optional string external_id // aws assumed role's external_id if
configure
+ 16: optional TCredential credential
Review Comment:
[P1] Gate native GCP storage-policy resources on BE support. A new FE can
create an explicit `provider=GCP` resource using only ADC/impersonation, then
`PushStoragePolicyTask` sends this field to every reporting BE. An older BE
ignores field 16, accepts empty AK/SK, and installs an ordinary AWS S3 client;
FE ping succeeds, but private GCS cold-tier reads and writes fail during a
rolling upgrade. This shared-nothing Thrift path needs a capability gate
separate from Cloud vault publication.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/storage/StorageAdapter.java:
##########
@@ -504,7 +504,8 @@ private Map<String, String>
alignS3FamilyBackendMap(S3CompatibleFileSystemProper
// AWS_CREDENTIALS_PROVIDER_TYPE=ANONYMOUS exactly when both AK
and SK are blank.
aligned.remove("AWS_ROLE_ARN");
aligned.remove("AWS_EXTERNAL_ID");
- if (s3.hasStaticCredentials()) {
+ if (s3.hasStaticCredentials()
+ ||
aligned.containsKey(org.apache.doris.filesystem.auth.GcpCredential.CREDENTIAL_PROVIDER_TYPE))
{
Review Comment:
[P1] Route native GCP map requests only to capable BEs. This backend map
carries `gs.credential_provider_type` with empty AK/SK, but an older BE ignores
the selector and builds an AWS S3 client instead of OAuth.
`S3TableValuedFunction` sends the map to any alive BE for schema fetch and then
to scan BEs, so private GCS TVFs fail during a rolling upgrade even without a
storage policy; LOAD/OUTFILE/EXPORT share the BE map converter. Gate or route
these jobs by BE capability independently of the policy push.
##########
fe/fe-filesystem/fe-filesystem-gcs/src/main/java/org/apache/doris/filesystem/gcs/GcsFileSystemProperties.java:
##########
@@ -134,10 +161,63 @@ public Map<String, String> toMap() {
return Collections.unmodifiableMap(kv);
}
+ private static Map<String, String> withGcpProvider(Map<String, String>
properties) {
+ Map<String, String> selected = new HashMap<>(properties);
+ selected.put("provider", "GCP");
+ return selected;
+ }
+
+ public GcsAuth getAuth() {
+ return auth;
+ }
+
+ @Override
+ public Map<String, String> matchedProperties() {
+ Map<String, String> matched = new HashMap<>(super.matchedProperties());
+ if (auth.getMode() != GcsAuth.Mode.HMAC) {
+ matched.put(GcpCredential.CREDENTIAL_PROVIDER_TYPE,
auth.isAnonymous() ? "ANONYMOUS"
+ :
auth.getNativeCredential().orElseThrow().getCredentialProviderType().name());
+ matched.put(GcpCredential.IMPERSONATION_SERVICE_ACCOUNT,
+
auth.getNativeCredential().map(GcpCredential::getImpersonationServiceAccount).orElse(""));
+ }
+ return Collections.unmodifiableMap(matched);
+ }
+
+ @Override
+ protected void customizeS3CompatibleKv(Map<String, String> kv) {
+ if (auth.getNativeCredential().isPresent()) {
+ GcpCredential credential = auth.getNativeCredential().get();
+ kv.remove("AWS_CREDENTIALS_PROVIDER_TYPE");
+ kv.put(GcpCredential.CREDENTIAL_PROVIDER_TYPE,
credential.getCredentialProviderType().name());
+ putIfNotBlank(kv, GcpCredential.IMPERSONATION_SERVICE_ACCOUNT,
+ credential.getImpersonationServiceAccount());
+ }
+ }
+
+ @Override
+ public Map<String, String> toHadoopConfigurationMap() {
+ if (auth.getNativeCredential().isEmpty()) {
+ return super.toHadoopConfigurationMap();
+ }
+ GcpCredential credential = auth.getNativeCredential().get();
+ Map<String, String> cfg = new HashMap<>();
+ cfg.put("fs.gs.impl",
"com.google.cloud.hadoop.fs.gcs.GoogleHadoopFileSystem");
+ String storageRoot = getEndpoint();
+ if (!storageRoot.contains("://")) {
+ storageRoot = "https://"; + storageRoot;
+ }
+ cfg.put("fs.gs.storage.root.url", storageRoot.endsWith("/") ?
storageRoot : storageRoot + "/");
+ cfg.put("fs.gs.auth.type", credential.getCredentialProviderType() ==
GcpCredentialProviderType.DEFAULT
Review Comment:
[P2] Keep native GCP auth effective for Hadoop catalog operations. This map
selects auth from `gs.credential_provider_type`, but
`IcebergCatalogFactory.buildHadoopConfiguration()` overlays raw `fs.*` keys
afterward. For a private `gs://` Hadoop Iceberg warehouse,
`fs.gs.auth.type=UNAUTHENTICATED` replaces ADC, so namespace creation/listing
fails while S3FileIO still uses ADC; overriding
`fs.gs.auth.impersonation.service.account` similarly splits identities. The
endpoint-only raw-key check above does not cover auth keys. Reject conflicting
raw `fs.gs.auth.*` keys or apply the selected auth after the overlay.
--
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]
