feat: Add S3 bucket configuration for path-style URL support.

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<std::string>(kEndPoint, "");
-  VELOX_CHECK(
-      !this->endPoint_.empty(), fmt::format("{} cannot be empty", kEndPoint));
-  if ('/' == this->endPoint_.back()) {
-    this->endPoint_.pop_back();
+  endPoint_ = config_->get<std::string>(kEndPoint, "");
+  VELOX_CHECK(!endPoint_.empty(), fmt::format("{} cannot be empty", kEndPoint));
+  if ('/' == endPoint_.back()) {
+    endPoint_.pop_back();
   }
 
+  bucket_ = config_->get<std::string>(kBucket, "");
+
   auto accessKeyId = config_->get<std::string>(kAccessKeyId, "");
   auto secretAccessKey = config_->get<std::string>(kSecretAccessKey, "");
   auto sessionToken = config_->get<std::string>(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 {

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_;
 };
 

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<std::string, std::string> 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<std::string, std::string> 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<std::string, std::string> 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