github-actions[bot] commented on code in PR #68540: URL: https://github.com/apache/doris/pull/68540#discussion_r4370186914
########## regression-test/suites/object_storage_iam_p0/test_gcp_iam_property_validation.groovy: ########## @@ -0,0 +1,131 @@ +// 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. + +suite("test_gcp_iam_property_validation") { + ["ADC", "", "unknown"].each { providerType -> + test { + sql """ + SELECT * FROM s3( + "uri" = "gs://validation-only-bucket/file.csv", + "provider" = "GCP", + "gs.credential_provider_type" = "${providerType}", + "format" = "csv" + ) LIMIT 1 + """ + exception "gs.credential_provider_type" + } + } + + [" ", "invalid-account"].each { account -> + test { + sql """ + SELECT * FROM s3( + "uri" = "gs://validation-only-bucket/file.csv", + "provider" = "GCP", + "gs.impersonation_service_account" = "${account}", + "format" = "csv" + ) LIMIT 1 + """ + exception "Invalid GCP service account email" + } + } + + [ + "gs.access_key": "gcp-access-key", + "gs.secret_key": "gcp-secret-key", + "gs.session_token": "gcp-session-token", + "s3.access_key": "access-key", + "s3.secret_key": "secret-key", + "s3.session_token": "session-token", + "s3.role_arn": "arn:aws:iam::123456789012:role/test-role", + "s3.external_id": "external-id", + "s3.credentials_provider_type": "INSTANCE_PROFILE" + ].each { key, value -> + test { + sql """ + SELECT * FROM s3( + "uri" = "gs://validation-only-bucket/file.csv", + "provider" = "GCP", + "gs.credential_provider_type" = "DEFAULT", + "${key}" = "${value}", + "format" = "csv" + ) LIMIT 1 + """ + exception "cannot be used together" + } + } + + test { + sql """ + SELECT * FROM s3( + "uri" = "gs://validation-only-bucket/file.csv", + "provider" = "S3", + "gs.credential_provider_type" = "DEFAULT", + "format" = "csv" + ) LIMIT 1 + """ + exception "requires provider=GCP" Review Comment: [P2] Match the error emitted by selected S3 binding This case explicitly selects `provider=S3`, so TVF construction goes through `StorageAdapter.of` and the S3 provider's binding before any GCS authentication resolver runs. `S3FileSystemProperties.validate` calls `S3CompatSignals.rejectNativeGcpCredentials`, which throws `Native GCP authentication requires the GCS filesystem provider`. The regression framework checks `message.contains(expected)`, so `requires provider=GCP` never matches and this unconditional validation suite fails even though the configuration is correctly rejected. Update the expectation to the selected-provider error or a shared stable substring. ########## fe/fe-filesystem/fe-filesystem-minio/src/test/java/org/apache/doris/filesystem/minio/MinioFileSystemProviderTest.java: ########## @@ -108,6 +108,19 @@ void supports_rejectsForeignDialects() { Assertions.assertFalse(provider.supports(plainS3)); } + @Test + void supportsGuess_doesNotAlsoBindExplicitS3() { + for (String endpoint : new String[] {"https://storage.us-east1.rep.googleapis.com";, + "http://minio.local:9000"}) { + Map<String, String> props = new HashMap<>(); + props.put("s3.endpoint", endpoint); + Assertions.assertTrue(provider.supportsGuess(props), endpoint); Review Comment: [P2] Expect MinIO to reject the regional GCS endpoint The first iteration contains only `s3.endpoint=https://storage.us-east1.rep.googleapis.com`. `GcsEndpoint` now recognizes that regional host, so `GcsAuthResolver.guessIsGcs` returns true and the new guard in `MinioFileSystemProvider.supportsGuess` returns false. This `assertTrue` therefore fails before the explicit S3 checks run, making the added unit test fail deterministically. Expect false for the Google endpoint and retain the true-before/false-after checks for the private MinIO endpoint. -- 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]
