github-actions[bot] commented on code in PR #68540:
URL: https://github.com/apache/doris/pull/68540#discussion_r4231751519
##########
fe/fe-core/src/main/java/org/apache/doris/cloud/storage/ObjectInfo.java:
##########
@@ -56,19 +57,26 @@ public ObjectInfo(Cloud.ObjectStoreInfoPB
objectStoreInfoPB) {
this(objectStoreInfoPB.getProvider(), objectStoreInfoPB.getAk(),
objectStoreInfoPB.getSk(),
objectStoreInfoPB.getBucket(),
objectStoreInfoPB.getEndpoint(), objectStoreInfoPB.getRegion(),
objectStoreInfoPB.getPrefix(), null,
objectStoreInfoPB.getRoleArn(),
- objectStoreInfoPB.getExternalId(), null);
+ objectStoreInfoPB.getExternalId(), null,
nativeCredential(objectStoreInfoPB));
Review Comment:
[P2] Carry native GCP identity through internal-stage COPY. For a GCP
instance using COMPUTE_ENGINE with an impersonation account, Meta Service
get_stage rebuilds the internal StagePB using AK/SK, provider and location but
omits the credential envelope. This constructor then receives no native
credential, so COPY file discovery falls back to ADC and cannot list a private
bucket granted only to the impersonated account, even if files already exist.
CopyIntoInfo.validateStagePB also omits gs.credential_provider_type and
gs.impersonation_service_account from brokerProperties, so BE scans would still
use ADC after fixing get_stage. Carry the credential through both handoffs and
cover COPY from an impersonated internal stage.
##########
regression-test/suites/object_storage_iam_p0/test_alter_resource_with_role.groovy:
##########
@@ -0,0 +1,240 @@
+// 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.
+
+import groovy.json.JsonSlurper
+import org.apache.doris.regression.util.ObjectStorageIamTestUtils
+
+suite("test_alter_resource_with_role") {
+ if (!ObjectStorageIamTestUtils.isProvider(context.config.otherConfigs,
"AWS")) {
+ logger.info("skip ${name} because objectStorageIamProvider is not AWS")
+ return
+ }
+ def config =
ObjectStorageIamTestUtils.getConfig(context.config.otherConfigs)
+
+
+ if (isCloudMode()) {
+ logger.info("skip ${name} case, because it is cloud mode")
+ return
+ }
+
+ def tableName = "test_alter_resource_with_role"
+ def randomStr = UUID.randomUUID().toString().replace("-", "")
+ def resourceName = "alter_resource_${randomStr}"
+ def policyName = "alter_policy_${randomStr}"
+
+ def awsEndpoint = config.endpoint
+ def region = config.region
+ def bucket = config.bucket
+ def roleArn = config.roleArn
+ def externalId = config.externalId ?: ""
+ def prefix = config.prefix
+
+ def awsAccessKey = context.config.awsAccessKey
Review Comment:
[P2] Require static credentials before the AWS HMAC transition. The AWS
role-only configuration documented by ObjectStorageIamTestUtils does not set
awsAccessKey or awsSecretKey, and getConfig does not require them. Both this
suite and test_alter_storage_vault_iam later interpolate these null fields into
static-key ALTER statements and then expect remote writes to succeed. A valid
role-only test run therefore fails for reasons unrelated to the role feature.
Require and document real static keys for these transitions, or skip just the
HMAC portion when they are absent.
##########
fe/fe-paimon-common/src/main/java/org/apache/doris/paimon/NativeGcsFileIO.java:
##########
@@ -0,0 +1,121 @@
+// 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.paimon;
+
+import org.apache.paimon.fs.FileIO;
+import org.apache.paimon.fs.FileIOLoader;
+import org.apache.paimon.fs.FileStatus;
+import org.apache.paimon.fs.Path;
+import org.apache.paimon.fs.PositionOutputStream;
+import org.apache.paimon.fs.RemoteIterator;
+import org.apache.paimon.fs.SeekableInputStream;
+import org.apache.paimon.fs.hadoop.HadoopFileIO;
+
+import java.io.IOException;
+
+/** Routes legacy GCS aliases through the native Hadoop GCS connector,
including persisted file paths. */
+public class NativeGcsFileIO extends HadoopFileIO {
+ private static final long serialVersionUID = 1L;
+
+ public NativeGcsFileIO(Path path) {
+ super(normalize(path));
+ }
+
+ public static Path normalize(Path path) {
+ String scheme = path.toUri().getScheme();
+ if ("s3".equalsIgnoreCase(scheme) || "s3a".equalsIgnoreCase(scheme)) {
Review Comment:
[P2] Limit alias rewriting to paths known to belong to GCS. Paimon calls
this loader as preferIO before scheme lookup, and checkAccess keeps it even
when exists returns false. Thus a catalog with a gs:// GCS warehouse and an
independent s3a:// AWS external table or HMS database location rewrites the
table path to gs://aws-bucket/... here. Metadata reads then miss the S3A
filesystem configured for that table, and writes may address the wrong store.
Preserve S3A routing for external S3 locations and cover a mixed-location
catalog through FileIO.get.
##########
fe/fe-connector/fe-connector-hudi/src/main/java/org/apache/doris/connector/hudi/HudiConnectorMetadata.java:
##########
@@ -222,6 +222,19 @@ public Optional<ConnectorTableHandle> getTableHandle(
.build());
}
+ private String hadoopTableLocation(String location) {
+ // Native GCP auth configures fs.gs.*, while HMS may retain an
S3-compatible location.
+ // Normalize the handle so metadata, split planning and the JNI reader
share the same base path.
+ if
("com.google.cloud.hadoop.fs.gcs.GoogleHadoopFileSystem".equals(storageHadoopConfig.get("fs.gs.impl")))
{
Review Comment:
[P2] Keep unrelated S3 Hudi table locations on S3A. A catalog with native
GCS storage can also have an independent AWS table in the same HMS, with
fs.s3a.* credentials accepted by buildHadoopConf. This catalog-wide check
rewrites that table's s3a://aws-bucket/table location to gs://aws-bucket/table.
The resulting HudiTableHandle sends schema, timeline, and JNI reads to GCS
rather than the configured S3A filesystem, so the AWS table fails. Restrict the
rewrite to locations known to represent the GCS binding and cover a mixed HMS
catalog. This is separate from the earlier GCS-alias-on-S3A issue.
--
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]
