github-actions[bot] commented on code in PR #68540: URL: https://github.com/apache/doris/pull/68540#discussion_r4226856804
########## fe/fe-filesystem/fe-filesystem-api/src/main/java/org/apache/doris/filesystem/auth/GcsAuthResolver.java: ########## @@ -0,0 +1,169 @@ +// 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. + +package org.apache.doris.filesystem.auth; + + +import java.util.Locale; +import java.util.Map; +import java.util.Optional; + +/** + * Resolves GCS authentication without creating clients or loading credentials. + * Vault ALTER still preserves field presence when constructing its patch from this result. + */ +public final class GcsAuthResolver { + private static final String CREDENTIAL_PROVIDER_TYPE = GcpCredential.CREDENTIAL_PROVIDER_TYPE; + private static final String IMPERSONATION_SERVICE_ACCOUNT = GcpCredential.IMPERSONATION_SERVICE_ACCOUNT; + private static final String[] ACCESS_KEYS = {"gs.access_key", "s3.access_key", "AWS_ACCESS_KEY", "access_key"}; + private static final String[] SECRET_KEYS = {"gs.secret_key", "s3.secret_key", "AWS_SECRET_KEY", "secret_key"}; + private static final String[] TOKENS = {"gs.session_token", "s3.session_token", "AWS_TOKEN", "session_token"}; + private static final String[] AWS_PROVIDERS = {"s3.credentials_provider_type", "AWS_CREDENTIALS_PROVIDER_TYPE"}; + private static final String[] AWS_ROLES = {"s3.role_arn", "AWS_ROLE_ARN", "s3.external_id", "AWS_EXTERNAL_ID"}; + + private GcsAuthResolver() { + } + + public static Optional<GcsAuth> resolve(Map<String, String> properties) { + boolean hasNativeProperties = hasNativeCredentialProperties(properties); + boolean hasGcsSelector = guessIsGcs(properties) + || "true".equalsIgnoreCase(getPropertyIgnoreCase(properties, "fs.gcs.support")); + boolean hasAccessKey = hasNonBlankProperty(properties, ACCESS_KEYS); + boolean hasSecretKey = hasNonBlankProperty(properties, SECRET_KEYS); + boolean hasLegacyAnonymous = hasLegacyAnonymousProvider(properties); + String provider = getPropertyIgnoreCase(properties, "provider"); + boolean useLegacyAnonymousDefault = "S3".equalsIgnoreCase(provider) Review Comment: [P1] Preserve explicit S3 authentication when the endpoint is GCS. A `provider=S3` resource pointed at `storage.googleapis.com`, with no AK/SK properties but HMAC keys in the AWS environment, can pass the FE validity ping through the explicit S3 provider's default chain. This heuristic classifies the same map as GCS anonymous: `S3ThriftAdapter` then pushes ANONYMOUS to current BEs, breaking private cold-tier reads/writes, and `CloudObjectStoreAdapter` rejects a vault using the same map. Preserve explicit S3's default chain unless anonymous was explicitly selected, and cover ping, policy push, and vault creation together. ########## fe/fe-core/src/main/java/org/apache/doris/catalog/S3Resource.java: ########## @@ -94,6 +112,7 @@ public String getProperty(String propertyKey) { protected void setProperties(ImmutableMap<String, String> newProperties) throws DdlException { Preconditions.checkState(newProperties != null); this.properties = Maps.newHashMap(newProperties); + normalizeProperties(this.properties, this.properties.get("provider")); Review Comment: [P2] Normalize inferred GCP aliases before validating resource creation. A `CREATE RESOURCE` or `CREATE STORAGE VAULT` with `gs.endpoint=https://storage.googleapis.com`, native `gs.credential_provider_type=DEFAULT`, and no explicit `provider` is recognized as GCS by the filesystem resolver, but this call passes null to `normalizeProperties`. The alias stays as `gs.endpoint`, then `requiredS3PingProperties` throws `Missing [s3.endpoint]` before either creation can proceed. Infer GCP for this normalization (or make the public property contract consistently require `provider`) and cover the omitted-provider create path. ########## fe/fe-filesystem/fe-filesystem-gcs/src/main/java/org/apache/doris/filesystem/gcs/GcsFileSystemProperties.java: ########## @@ -134,10 +169,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"); Review Comment: [P2] Normalize Paimon warehouse aliases before using native GCS Hadoop auth. This branch configures OAuth only for `fs.gs.*`, while this storage binding still advertises `s3a` and Paimon passes `warehouse=s3a://bucket/warehouse` unchanged to its catalog/FileIO. [Paimon 1.3.1 selects FileIO by the URI scheme](https://github.com/apache/paimon/blob/release-1.3.1/paimon-common/src/main/java/org/apache/paimon/fs/FileIO.java), so a private GCS warehouse selects S3A without the OAuth identity and metadata or JNI reads fail. Rewrite Paimon's Hadoop-facing warehouse and file paths to `gs://` when native GCP auth is selected, and cover a private alias warehouse. -- 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]
