github-actions[bot] commented on code in PR #68540: URL: https://github.com/apache/doris/pull/68540#discussion_r4334086186
########## common/cpp/obj-client/auth/gcp/gcp_token_provider.cpp: ########## @@ -0,0 +1,102 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#include "cpp/obj-client/auth/gcp/gcp_token_provider.h" + +#include <glog/logging.h> +#include <google/cloud/credentials.h> +#include <google/cloud/internal/unified_rest_credentials.h> + +#include <utility> + +namespace doris { +namespace { + +constexpr char GCS_READ_WRITE_SCOPE[] = "https://www.googleapis.com/auth/devstorage.read_write";; +constexpr char CLOUD_PLATFORM_SCOPE[] = "https://www.googleapis.com/auth/cloud-platform";; + +std::shared_ptr<google::cloud::Credentials> make_credentials(const GcpCredentialConfig& credential, + const std::string& ca_cert_path, + GcpTokenScope token_scope) { + google::cloud::Options options; + if (!ca_cert_path.empty()) { + options.set<google::cloud::CARootsFilePathOption>(ca_cert_path); + } + + // A source credential used for impersonation calls the IAM Credentials API + // and therefore needs cloud-platform. The resulting target token uses the + // caller-selected scope: object read/write for storage requests, or + // cloud-platform when it will call IAM signBlob. + const char* requested_scope = token_scope == GcpTokenScope::CloudPlatform + ? CLOUD_PLATFORM_SCOPE + : GCS_READ_WRITE_SCOPE; + options.set<google::cloud::ScopesOption>({credential.impersonation_service_account.empty() + ? requested_scope + : CLOUD_PLATFORM_SCOPE}); + + std::shared_ptr<google::cloud::Credentials> credentials; + switch (credential.provider_type) { + case GcpCredentialProviderType::Default: + credentials = google::cloud::MakeGoogleDefaultCredentials(options); + break; + case GcpCredentialProviderType::ComputeEngine: + credentials = google::cloud::MakeComputeEngineCredentials(options); + break; + } + if (!credential.impersonation_service_account.empty()) { + auto impersonation_options = options; + impersonation_options.set<google::cloud::ScopesOption>({requested_scope}); + credentials = google::cloud::MakeImpersonateServiceAccountCredentials( + std::move(credentials), credential.impersonation_service_account, + std::move(impersonation_options)); + } + return credentials; +} + +} // namespace + +GcpTokenProvider::GcpTokenProvider(const GcpCredentialConfig& credential, + const std::string& ca_cert_path, GcpTokenScope token_scope) + : GcpTokenProvider(google::cloud::rest_internal::MapCredentials( Review Comment: [P1] Exchange service-account ADC for GCS-compatible OAuth tokens With `gs.credential_provider_type=DEFAULT` and `GOOGLE_APPLICATION_CREDENTIALS` pointing to a valid service-account JSON key, without impersonation or the experimental JWT-disable override, google-cloud-cpp 2.45.0's generic `MapCredentials` enables self-signed JWTs. `get_token()` returns that scoped JWT and `GcpS3Client` sends it directly as `Bearer`; GCS rejects scoped self-signed JWTs, which its [Storage-specific credential adapter explicitly disables](https://github.com/googleapis/google-cloud-cpp/blob/v2.45.0/google/cloud/storage/oauth2/service_account_credentials.cc#L127-L139). FE's Java OAuth connectivity check can therefore succeed while BE reads/writes and recycler requests fail for the same identity. Force OAuth access-token exchange for this ADC path and add coverage through actual service-account ADC construction. ########## fe/fe-core/src/main/java/org/apache/doris/datasource/storage/S3ThriftAdapter.java: ########## @@ -66,7 +70,13 @@ private S3ThriftAdapter() { /** Direct move of legacy {@code S3Properties.getS3TStorageParam}. */ public static TS3StorageParam getS3TStorageParam(Map<String, String> properties) { + Optional<ObjectStorageAuthentication> authentication = StorageAdapter.resolveAuthentication(properties); TS3StorageParam s3Info = new TS3StorageParam(); + if (StringUtils.isNotBlank(properties.get("provider"))) { + String provider = properties.get("provider").trim().toUpperCase(Locale.ROOT); + s3Info.setProvider("S3".equals(provider) ? TObjStorageType.AWS + : "GCS".equals(provider) ? TObjStorageType.GCP : TObjStorageType.valueOf(provider)); Review Comment: [P1] Preserve policy pushes for accepted MinIO provider aliases An existing S3 resource with a MinIO endpoint and static keys can be ALTERed with `"provider"="MINIO", "s3_validity_check"="false"`; this is accepted and persisted, including on this head. `MINIO` is a filesystem provider but is absent from `TObjStorageType`, so this `valueOf` throws on the next backend report. `ReportHandler` includes every changed S3 resource in one `PushStoragePolicyTask`, so even an unreferenced resource prevents that entire policy/resource update from being sent, repeatedly blocking unrelated updates. Previously this adapter left the provider unset and BE used its AWS-compatible fallback. Map accepted filesystem aliases to their wire backend and cover replay plus a batched policy push. -- 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]
