diff --git a/velox/connectors/clp/search_lib/ClpPackageS3AuthProvider.cpp b/velox/connectors/clp/search_lib/ClpPackageS3AuthProvider.cpp index f76b09c556a..c8a9842b99a 100644 --- a/velox/connectors/clp/search_lib/ClpPackageS3AuthProvider.cpp +++ b/velox/connectors/clp/search_lib/ClpPackageS3AuthProvider.cpp @@ -23,17 +23,23 @@ namespace facebook::velox::connector::clp { std::string ClpPackageS3AuthProvider::constructS3Url( std::string_view splitPath) { VELOX_CHECK(!splitPath.empty(), "splitPath cannot be empty"); - return fmt::format("{}/{}", this->endPoint_, splitPath); + // For URLs where the bucket is already encoded in the endpoint (e.g., AWS S3 + // virtual-hosted style: https://bucket.s3.region.amazonaws.com). + if (bucket_.empty()) { + return fmt::format("{}/{}", endPoint_, splitPath); + } + return fmt::format("{}/{}/{}", endPoint_, bucket_, splitPath); } bool ClpPackageS3AuthProvider::exportAuthEnvironmentVariables() { - this->endPoint_ = config_->get(kEndPoint, ""); - VELOX_CHECK( - !this->endPoint_.empty(), fmt::format("{} cannot be empty", kEndPoint)); - if ('/' == this->endPoint_.back()) { - this->endPoint_.pop_back(); + endPoint_ = config_->get(kEndPoint, ""); + VELOX_CHECK(!endPoint_.empty(), fmt::format("{} cannot be empty", kEndPoint)); + if ('/' == endPoint_.back()) { + endPoint_.pop_back(); } + bucket_ = config_->get(kBucket, ""); + auto accessKeyId = config_->get(kAccessKeyId, ""); auto secretAccessKey = config_->get(kSecretAccessKey, ""); auto sessionToken = config_->get(kSessionToken, ""); @@ -42,8 +48,10 @@ bool ClpPackageS3AuthProvider::exportAuthEnvironmentVariables() { VELOX_CHECK( !secretAccessKey.empty(), fmt::format("{} cannot be empty", kSecretAccessKey)); + setEnvironmentVariable(kEnvAwsAccessKeyId, accessKeyId); setEnvironmentVariable(kEnvAwsSecretAccessKey, secretAccessKey); + if (!sessionToken.empty()) { setEnvironmentVariable(kEnvAwsSessionToken, sessionToken); } else { diff --git a/velox/connectors/clp/search_lib/ClpPackageS3AuthProvider.h b/velox/connectors/clp/search_lib/ClpPackageS3AuthProvider.h index ead4df0cf3b..1a25a494d5e 100644 --- a/velox/connectors/clp/search_lib/ClpPackageS3AuthProvider.h +++ b/velox/connectors/clp/search_lib/ClpPackageS3AuthProvider.h @@ -31,6 +31,7 @@ class ClpPackageS3AuthProvider : public ClpS3AuthProviderBase { : ClpS3AuthProviderBase(config) {} static constexpr const char* kAccessKeyId = "clp.s3-access-key-id"; + static constexpr const char* kBucket = "clp.s3-bucket"; static constexpr const char* kEndPoint = "clp.s3-end-point"; static constexpr const char* kSecretAccessKey = "clp.s3-secret-access-key"; static constexpr const char* kSessionToken = "clp.s3-session-token"; @@ -44,6 +45,7 @@ class ClpPackageS3AuthProvider : public ClpS3AuthProviderBase { bool exportAuthEnvironmentVariables() override; private: + std::string bucket_; std::string endPoint_; }; diff --git a/velox/connectors/clp/tests/ClpConfigTest.cpp b/velox/connectors/clp/tests/ClpConfigTest.cpp index 4e53fbf8745..796f1b331c6 100644 --- a/velox/connectors/clp/tests/ClpConfigTest.cpp +++ b/velox/connectors/clp/tests/ClpConfigTest.cpp @@ -205,7 +205,8 @@ TEST_F(ClpS3AuthProviderBaseTest, caseInsensitiveAuthProvider) { TEST_F(ClpPackageS3AuthProviderTest, readAndExportAwsAuthEnvironmentVariables) { const std::string cTestAccessKeyId{"aaaaaa"}; - const std::string cTestEndPoint{"http://aaaaaa"}; + const std::string cTestBucket{"test-bucket"}; + const std::string cTestEndPoint{"http://localhost:9000"}; const std::string cTestSecretAccessKey{"bbbbbb"}; const std::string cTestSessionToken{"cccccc"}; @@ -214,6 +215,7 @@ TEST_F(ClpPackageS3AuthProviderTest, readAndExportAwsAuthEnvironmentVariables) { {{"clp.storage-type", "s3"}, {ClpConfig::kAuthProvider, "clp_package"}, {ClpPackageS3AuthProvider::kAccessKeyId, cTestAccessKeyId}, + {ClpPackageS3AuthProvider::kBucket, cTestBucket}, {ClpPackageS3AuthProvider::kEndPoint, cTestEndPoint}, {ClpPackageS3AuthProvider::kSecretAccessKey, cTestSecretAccessKey}, {ClpPackageS3AuthProvider::kSessionToken, cTestSessionToken}}); @@ -239,4 +241,72 @@ TEST_F(ClpPackageS3AuthProviderTest, readAndExportAwsAuthEnvironmentVariables) { ClpPackageS3AuthProvider::kEnvAwsSessionToken, std::nullopt)); } +// Tests URL construction for S3-compatible storage (e.g., MinIO) using +// path-style URLs with bucket. +TEST_F(ClpPackageS3AuthProviderTest, constructS3UrlForPathStyleWithBucket) { + const std::string cTestAccessKeyId{"aaaaaa"}; + const std::string cTestBucket{"logs"}; + const std::string cTestEndPoint{"http://172.26.105.44:9000"}; + const std::string cTestSecretAccessKey{"bbbbbb"}; + const std::string cTestSplitPath{"archives/default/abc123"}; + + std::unordered_map configMap( + {{"clp.storage-type", "s3"}, + {ClpConfig::kAuthProvider, "clp_package"}, + {ClpPackageS3AuthProvider::kAccessKeyId, cTestAccessKeyId}, + {ClpPackageS3AuthProvider::kBucket, cTestBucket}, + {ClpPackageS3AuthProvider::kEndPoint, cTestEndPoint}, + {ClpPackageS3AuthProvider::kSecretAccessKey, cTestSecretAccessKey}}); + auto clpPackageS3AuthProvider = buildClpPackageS3AuthProvider(configMap); + VELOX_CHECK(clpPackageS3AuthProvider->exportAuthEnvironmentVariables()); + + auto url = clpPackageS3AuthProvider->constructS3Url(cTestSplitPath); + VELOX_CHECK_EQ(url, "http://172.26.105.44:9000/logs/archives/default/abc123"); +} + +// Tests URL construction for AWS S3 using virtual-hosted style URLs without +// bucket config. +TEST_F(ClpPackageS3AuthProviderTest, constructS3UrlForAwsVirtualHostedStyle) { + const std::string cTestAccessKeyId{"aaaaaa"}; + const std::string cTestEndPoint{"https://logs.s3.us-east-1.amazonaws.com"}; + const std::string cTestSecretAccessKey{"bbbbbb"}; + const std::string cTestSplitPath{"archives/default/abc123"}; + + std::unordered_map configMap( + {{"clp.storage-type", "s3"}, + {ClpConfig::kAuthProvider, "clp_package"}, + {ClpPackageS3AuthProvider::kAccessKeyId, cTestAccessKeyId}, + {ClpPackageS3AuthProvider::kEndPoint, cTestEndPoint}, + {ClpPackageS3AuthProvider::kSecretAccessKey, cTestSecretAccessKey}}); + auto clpPackageS3AuthProvider = buildClpPackageS3AuthProvider(configMap); + VELOX_CHECK(clpPackageS3AuthProvider->exportAuthEnvironmentVariables()); + + auto url = clpPackageS3AuthProvider->constructS3Url(cTestSplitPath); + VELOX_CHECK_EQ( + url, "https://logs.s3.us-east-1.amazonaws.com/archives/default/abc123"); +} + +// Tests URL construction for AWS S3 using path-style URLs with bucket config. +TEST_F(ClpPackageS3AuthProviderTest, constructS3UrlForAwsPathStyleWithBucket) { + const std::string cTestAccessKeyId{"aaaaaa"}; + const std::string cTestBucket{"logs"}; + const std::string cTestEndPoint{"https://s3.us-east-1.amazonaws.com"}; + const std::string cTestSecretAccessKey{"bbbbbb"}; + const std::string cTestSplitPath{"archives/default/abc123"}; + + std::unordered_map configMap( + {{"clp.storage-type", "s3"}, + {ClpConfig::kAuthProvider, "clp_package"}, + {ClpPackageS3AuthProvider::kAccessKeyId, cTestAccessKeyId}, + {ClpPackageS3AuthProvider::kBucket, cTestBucket}, + {ClpPackageS3AuthProvider::kEndPoint, cTestEndPoint}, + {ClpPackageS3AuthProvider::kSecretAccessKey, cTestSecretAccessKey}}); + auto clpPackageS3AuthProvider = buildClpPackageS3AuthProvider(configMap); + VELOX_CHECK(clpPackageS3AuthProvider->exportAuthEnvironmentVariables()); + + auto url = clpPackageS3AuthProvider->constructS3Url(cTestSplitPath); + VELOX_CHECK_EQ( + url, "https://s3.us-east-1.amazonaws.com/logs/archives/default/abc123"); +} + } // namespace facebook::velox::connector::clp