github-actions[bot] commented on code in PR #65834: URL: https://github.com/apache/doris/pull/65834#discussion_r3642740357
########## fe/fe-filesystem/fe-filesystem-gcs/src/main/java/org/apache/doris/filesystem/gcs/GcsFileSystemProvider.java: ########## @@ -0,0 +1,85 @@ +// 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.gcs; + +import org.apache.doris.filesystem.FileSystem; +import org.apache.doris.filesystem.s3.S3CompatSignals; +import org.apache.doris.filesystem.s3.S3FileSystem; +import org.apache.doris.filesystem.s3.S3FileSystemProperties; +import org.apache.doris.filesystem.s3.S3ObjStorage; +import org.apache.doris.filesystem.spi.FileSystemProvider; +import org.apache.doris.foundation.property.ConnectorPropertiesUtils; + +import java.io.IOException; +import java.util.Map; +import java.util.Set; + +/** + * SPI provider for Google Cloud Storage over the S3 interoperability endpoint (HMAC keys). + * + * <p>Registered via META-INF/services/org.apache.doris.filesystem.spi.FileSystemProvider. + * + * <p>Selected explicitly by {@code provider=GCS}/{@code GCP} or {@code fs.gcs.support=true}; + * otherwise, when the user declared no filesystem explicitly, by + * {@link S3CompatSignals#guessIsGcs} (a {@code gs.endpoint}, or an endpoint on + * {@code storage.googleapis.com}). Delegates I/O to {@link S3FileSystem}. + */ +public class GcsFileSystemProvider implements FileSystemProvider<GcsFileSystemProperties> { + + private static final String STORAGE_TYPE_GCS = "GCS"; + private static final String FS_GCS_SUPPORT = "fs.gcs.support"; + + @Override + public boolean supports(Map<String, String> properties) { + String providerValue = properties.get(S3CompatSignals.PROVIDER_KEY); Review Comment: [P1] Preserve GCP identity for custom-endpoint stages `ObjectInfoAdapter.toStorageProperties()` maps an authoritative `Provider.GCP` stage to `S3Properties`, and its builder emits only `s3.*` keys. After conversion this branch therefore receives no `provider=GCP`; if the supported GCS endpoint is custom rather than ending in `storage.googleapis.com`, the guess is false and generic S3 wins. External stage/copy operations then bypass the dedicated GCS property and HMAC contract. Please preserve the GCP marker (or construct `GCSProperties`) through the adapter/converter and cover `ObjectInfo(GCP, custom-endpoint)` through real provider arbitration. ########## fe/fe-filesystem/fe-filesystem-s3-base/src/main/java/org/apache/doris/filesystem/s3/AbstractDelegatingS3Properties.java: ########## @@ -0,0 +1,246 @@ +// 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.s3; + +import org.apache.doris.filesystem.FileSystemType; +import org.apache.doris.filesystem.properties.BackendStorageKind; +import org.apache.doris.filesystem.properties.BackendStorageProperties; +import org.apache.doris.filesystem.properties.FileSystemProperties; +import org.apache.doris.filesystem.properties.HadoopStorageProperties; +import org.apache.doris.filesystem.properties.S3CompatibleFileSystemProperties; +import org.apache.doris.filesystem.properties.StorageKind; +import org.apache.doris.foundation.property.ConnectorPropertiesUtils; +import org.apache.doris.foundation.property.ConnectorProperty; +import org.apache.doris.foundation.property.ParamRules; + +import org.apache.commons.lang3.StringUtils; + +import java.lang.reflect.Field; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.Optional; + +/** + * Base class for S3-compatible dialect properties. Concrete dialects live in their own plugin + * modules (fe-filesystem-gcs, fe-filesystem-minio, fe-filesystem-ozone) and depend on this one. + * + * <p>A dialect differs from plain S3 only in property spelling (aliases), defaults, and the + * allowed credential surface (static HMAC keys only). Subclasses declare the annotated alias + * fields and dialect-specific validation; this class owns the shared plumbing: binding, + * matched-property collection, the canonical {@code AWS_*} delegation map consumed by + * {@link S3FileSystemProperties#of(Map)}, the Hadoop s3a configuration, and masked rendering. + */ +public abstract class AbstractDelegatingS3Properties + implements FileSystemProperties, BackendStorageProperties, HadoopStorageProperties, + S3CompatibleFileSystemProperties { + + // Alias order matches legacy AbstractS3CompatibleProperties.getBucket(), which reads + // s3.bucket first and falls back to AWS_BUCKET, for every S3-compatible dialect. + @ConnectorProperty(names = {"s3.bucket", "AWS_BUCKET"}, + required = false, + description = "The default bucket name.") + protected String bucket = ""; + + @ConnectorProperty(names = {"AWS_ROOT_PATH"}, + required = false, + description = "The root path prefix inside the bucket.") + protected String rootPath = ""; + + private final Map<String, String> rawProperties; + private Map<String, String> matchedProperties = Collections.emptyMap(); + + protected AbstractDelegatingS3Properties(Map<String, String> rawProperties) { + this.rawProperties = Collections.unmodifiableMap(new HashMap<>(rawProperties)); + } + + /** + * Binds raw properties onto the annotated fields (this class and the concrete subclass; + * {@link ConnectorPropertiesUtils} walks the hierarchy) and collects matched keys. + * + * <p>MUST be called from the subclass's static {@code of()} factory AFTER construction: + * subclass field initializers run after this base constructor, so binding inside the + * constructor would be silently overwritten by the dialect defaults. + */ + protected final void bindAndCollect() { + ConnectorPropertiesUtils.bindConnectorProperties(this, rawProperties); + Map<String, String> matched = new HashMap<>(); + for (Field field : ConnectorPropertiesUtils.getConnectorProperties(getClass())) { + String matchedName = ConnectorPropertiesUtils.getMatchedPropertyName(field, rawProperties); + if (StringUtils.isNotBlank(matchedName)) { + matched.put(matchedName, rawProperties.get(matchedName)); + } + } + this.matchedProperties = Collections.unmodifiableMap(matched); + } + + /** + * Shared validation for HMAC-only dialects. {@code aliasPrefix} is the dialect key prefix + * without the dot (e.g. {@code gs}); {@code displayName} the human-readable storage name. + */ + protected final void validateHmacDialect(String aliasPrefix, String displayName) { + new ParamRules() + .requireTogether(new String[] {getAccessKey(), getSecretKey()}, + aliasPrefix + ".access_key and " + aliasPrefix + + ".secret_key must be set together") + .requireAllIfPresent(getSessionToken(), new String[] {getAccessKey(), getSecretKey()}, + aliasPrefix + ".session_token requires " + aliasPrefix + ".access_key and " + + aliasPrefix + ".secret_key") + .check(() -> StringUtils.isBlank(getEndpoint()), + "Property " + aliasPrefix + ".endpoint is required.") + .check(this::hasInvalidUsePathStyle, + "use_path_style must be true or false, got: '" + getUsePathStyle() + "'") + .check(() -> S3CompatSignals.hasAwsOnlyCredentialOptions(rawProperties), + displayName + " supports only HMAC access_key/secret_key credentials; " + + "role_arn/instance-profile style options are not supported") + .validate("Invalid " + displayName + " filesystem properties"); + } + + @Override + public StorageKind kind() { + return StorageKind.OBJECT_STORAGE; + } + + @Override + public FileSystemType type() { + return FileSystemType.S3; + } + + @Override + public BackendStorageKind backendKind() { + return BackendStorageKind.S3_COMPATIBLE; + } + + @Override + public Map<String, String> rawProperties() { + return rawProperties; + } + + @Override + public Map<String, String> matchedProperties() { + return matchedProperties; + } + + @Override + public Optional<BackendStorageProperties> toBackendProperties() { + return Optional.of(this); + } + + @Override + public Optional<HadoopStorageProperties> toHadoopProperties() { + return Optional.of(this); + } + + @Override + public Map<String, String> toMap() { + return toS3CompatibleKv(); + } + + /** + * Canonical {@code AWS_*} keys accepted by {@link S3FileSystemProperties#of(Map)}; this is + * how a dialect delegates to the shared S3 interoperability client. + */ + public final Map<String, String> toS3CompatibleKv() { + Map<String, String> kv = new HashMap<>(); + kv.put("AWS_ENDPOINT", getEndpoint()); + kv.put("AWS_REGION", getRegion()); + putIfNotBlank(kv, "AWS_ACCESS_KEY", getAccessKey()); + putIfNotBlank(kv, "AWS_SECRET_KEY", getSecretKey()); + putIfNotBlank(kv, "AWS_TOKEN", getSessionToken()); + putIfNotBlank(kv, "AWS_BUCKET", bucket); + putIfNotBlank(kv, "AWS_ROOT_PATH", rootPath); + kv.put("AWS_MAX_CONNECTIONS", getMaxConnections()); + kv.put("AWS_REQUEST_TIMEOUT_MS", getRequestTimeoutMs()); + kv.put("AWS_CONNECTION_TIMEOUT_MS", getConnectionTimeoutMs()); + kv.put("use_path_style", getUsePathStyle()); + // With no HMAC pair configured, legacy AbstractS3CompatibleProperties defaults to the + // ANONYMOUS marker; without it the shared S3 client falls back to the AWS default provider + // chain and signs requests with ambient env credentials. But preserve an explicitly + // requested mode (validation restricts it to DEFAULT/ANONYMOUS): a user asking for DEFAULT + // keeps the credential chain instead of being silently downgraded to unsigned ANONYMOUS. + if (!hasStaticCredentials()) { + String requested = S3CompatSignals.requestedCredentialsProviderType(rawProperties); + kv.put("AWS_CREDENTIALS_PROVIDER_TYPE", requested != null ? requested : "ANONYMOUS"); + } + customizeS3CompatibleKv(kv); + return Collections.unmodifiableMap(kv); + } + + /** Hook for dialect-specific canonical keys. */ + protected void customizeS3CompatibleKv(Map<String, String> kv) { + } + + @Override + public Map<String, String> toHadoopConfigurationMap() { + Map<String, String> cfg = new HashMap<>(); + cfg.put("fs.s3.impl", "org.apache.hadoop.fs.s3a.S3AFileSystem"); + cfg.put("fs.s3a.impl", "org.apache.hadoop.fs.s3a.S3AFileSystem"); + cfg.put("fs.s3.impl.disable.cache", "true"); + cfg.put("fs.s3a.impl.disable.cache", "true"); + cfg.put("fs.s3a.endpoint", getEndpoint()); + cfg.put("fs.s3a.endpoint.region", getRegion()); + if (StringUtils.isNotBlank(getAccessKey())) { Review Comment: [P2] Preserve the no-static credential mode in the Hadoop export The validated properties distinguish `DEFAULT` from `ANONYMOUS` and `toS3CompatibleKv()` now preserves that choice, but this export emits no provider whenever AK/SK is absent. MinIO/Ozone therefore turn explicit or implicit `ANONYMOUS` into Hadoop's ambient chain, while the GCS hook does the reverse and overwrites explicit `DEFAULT` with `AnonymousAWSCredentialsProvider`. Since these objects advertise `HadoopStorageProperties`, please map the resolved mode consistently (for example with `S3CredentialsProviderFactory.hadoopClassName`) and let GCS only add `fs.gs.impl`, with `DEFAULT`/`ANONYMOUS` tests for each dialect. -- 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]
