Fix disabled use virtual addressing option for import from S3 (#9162)

pixcc · web-flow · commit c1760bd5c593 · 2024-09-13T00:51:59.000+03:00
diff --git a/ydb/core/wrappers/s3_storage_config.cpp b/ydb/core/wrappers/s3_storage_config.cpp
@@ -171,13 +171,15 @@ IExternalStorageOperator::TPtr TS3ExternalStorageConfig::DoConstructStorageOpera
 TS3ExternalStorageConfig::TS3ExternalStorageConfig(const Ydb::Import::ImportFromS3Settings& settings)
     : Config(ConfigFromSettings(settings))
     , Credentials(CredentialsFromSettings(settings))
+    , UseVirtualAddressing(!settings.disable_virtual_addressing())
 {
     Bucket = settings.bucket();
 }
 
 TS3ExternalStorageConfig::TS3ExternalStorageConfig(const Ydb::Export::ExportToS3Settings& settings)
     : Config(ConfigFromSettings(settings))
     , Credentials(CredentialsFromSettings(settings))
+    , UseVirtualAddressing(!settings.disable_virtual_addressing())
 {
     Bucket = settings.bucket();
 }
diff --git a/ydb/public/lib/ydb_cli/commands/ydb_service_import.cpp b/ydb/public/lib/ydb_cli/commands/ydb_service_import.cpp
@@ -128,6 +128,7 @@ int TCommandImportFromS3::Run(TConfig& config) {
     settings.Bucket(AwsBucket);
     settings.AccessKey(AwsAccessKey);
     settings.SecretKey(AwsSecretKey);
+    settings.UseVirtualAddressing(UseVirtualAddressing);
 
     if (Description) {
         settings.Description(Description);
diff --git a/ydb/public/lib/ydb_cli/common/aws.cpp b/ydb/public/lib/ydb_cli/common/aws.cpp
@@ -70,12 +70,13 @@ class TS3ClientWrapper : public IS3ClientWrapper {
         } else {
             throw TMisuseException() << "\"" << settings.Scheme_ << "\" scheme type is not supported";
         }
+        config.useVirtualAddressing = settings.UseVirtualAddressing_;
 
         Client = std::make_unique<Aws::S3::S3Client>(
             Aws::Auth::AWSCredentials(settings.AccessKey_, settings.SecretKey_),
             config,
             Aws::Client::AWSAuthV4Signer::PayloadSigningPolicy::Never,
-            true);
+            settings.UseVirtualAddressing_);
     }
 
     TListS3Result ListObjectKeys(const TString& prefix, const std::optional<TString>& token) override {
diff --git a/ydb/tests/functional/backup/backup_ut.cpp b/ydb/tests/functional/backup/backup_ut.cpp
diff --git a/ydb/tests/functional/backup/s3_path_style/s3_path_style_backup_ut.cpp b/ydb/tests/functional/backup/s3_path_style/s3_path_style_backup_ut.cpp
@@ -0,0 +1,110 @@
+#include <ydb/public/sdk/cpp/client/ydb_driver/driver.h>
+#include <ydb/public/sdk/cpp/client/ydb_export/export.h>
+#include <ydb/public/sdk/cpp/client/ydb_import/import.h>
+#include <ydb/public/sdk/cpp/client/ydb_operation/operation.h>
+#include <ydb/public/sdk/cpp/client/ydb_table/table.h>
+#include <ydb/library/testlib/s3_recipe_helper/s3_recipe_helper.h>
+
+#include <library/cpp/testing/unittest/registar.h>
+#include <util/system/env.h>
+
+using namespace NYdb;
+using namespace NYdb::NTable;
+
+namespace {
+    template<typename TOp>
+    void WaitOp(TMaybe<TOperation>& op, NOperation::TOperationClient& opClient) {
+        int attempt = 20;
+        while (--attempt) {
+            op = opClient.Get<TOp>(op->Id()).GetValueSync();
+            if (op->Ready()) {
+                break;
+            } 
+            Sleep(TDuration::Seconds(1));
+        }
+        UNIT_ASSERT_C(attempt, "Unable to wait completion of backup");
+    }
+}
+
+Y_UNIT_TEST_SUITE(S3PathStyleBackup)
+{
+    Y_UNIT_TEST(DisableVirtualAddressing)
+    {
+        TString connectionString = GetEnv("YDB_ENDPOINT") + "/?database=" + GetEnv("YDB_DATABASE");
+        auto config = TDriverConfig(connectionString);
+        auto driver = TDriver(config);
+        auto tableClient = TTableClient(driver);
+        auto session = tableClient.GetSession().GetValueSync().GetSession();
+
+        {
+            auto res = session.ExecuteSchemeQuery(R"(
+                CREATE TABLE `/local/Table` (
+                    Key Uint32,
+                    PRIMARY KEY (Key)
+                );
+            )").GetValueSync();
+            UNIT_ASSERT_C(res.IsSuccess(), res.GetIssues().ToString());
+        }
+
+        Aws::Client::ClientConfiguration s3ClientConfig;
+        s3ClientConfig.endpointOverride = GetEnv("S3_ENDPOINT");
+        s3ClientConfig.scheme = Aws::Http::Scheme::HTTP;
+        auto s3Client = Aws::S3::S3Client(
+            std::make_shared<Aws::Auth::AnonymousAWSCredentialsProvider>(),
+            s3ClientConfig,
+            Aws::Client::AWSAuthV4Signer::PayloadSigningPolicy::Never,
+            false
+        );
+        const TString bucketName = "my-bucket";
+        NTestUtils::CreateBucket(bucketName, s3Client);
+
+        auto fillS3Settings = [bucketName](auto& settings) {
+            settings.Endpoint(GetEnv("S3_ENDPOINT"));
+            settings.Bucket(bucketName);
+            settings.AccessKey("minio");
+            settings.SecretKey("minio123");
+            settings.UseVirtualAddressing(false);
+        };
+
+        {
+            NExport::TExportToS3Settings settings;
+            fillS3Settings(settings);
+
+            settings.AppendItem({"/local/Table", "Table"});
+
+            auto exportClient = NExport::TExportClient(driver);
+            auto operationClient = NOperation::TOperationClient(driver);
+
+            const auto backupOp = exportClient.ExportToS3(settings).GetValueSync();
+
+            if (backupOp.Ready()) {
+                UNIT_ASSERT_C(backupOp.Status().IsSuccess(), backupOp.Status().GetIssues().ToString());
+            } else {
+                TMaybe<TOperation> op = backupOp;
+                WaitOp<NExport::TExportToS3Response>(op, operationClient);
+                UNIT_ASSERT_C(op->Status().IsSuccess(), op->Status().GetIssues().ToString());
+            }
+        }
+
+        {
+            NImport::TImportFromS3Settings settings;
+            fillS3Settings(settings);
+
+            settings.AppendItem({"Table", "/local/Restored"});
+
+            auto importClient = NImport::TImportClient(driver);
+            auto operationClient = NOperation::TOperationClient(driver);
+
+            const auto restoreOp = importClient.ImportFromS3(settings).GetValueSync();
+
+            if (restoreOp.Ready()) {
+                UNIT_ASSERT_C(restoreOp.Status().IsSuccess(), restoreOp.Status().GetIssues().ToString());
+            } else {
+                TMaybe<TOperation> op = restoreOp;
+                WaitOp<NImport::TImportFromS3Response>(op, operationClient);
+                UNIT_ASSERT_C(op->Status().IsSuccess(), op->Status().GetIssues().ToString());
+            }
+        }
+    }
+}
+
diff --git a/ydb/tests/functional/backup/s3_path_style/ya.make b/ydb/tests/functional/backup/s3_path_style/ya.make
@@ -0,0 +1,29 @@
+UNITTEST()
+
+ENV(S3_IGNORE_SUBDOMAIN_BUCKETNAME=true)
+ENV(YDB_USE_IN_MEMORY_PDISKS=true)
+
+ENV(YDB_ERASURE=block_4-2)
+
+PEERDIR(
+    ydb/library/testlib/s3_recipe_helper
+    ydb/public/sdk/cpp/client/ydb_export
+    ydb/public/sdk/cpp/client/ydb_table
+    ydb/public/sdk/cpp/client/ydb_operation
+    ydb/public/sdk/cpp/client/draft
+)
+
+SRCS(
+    s3_path_style_backup_ut.cpp
+)
+
+INCLUDE(${ARCADIA_ROOT}/ydb/public/tools/ydb_recipe/recipe.inc)
+INCLUDE(${ARCADIA_ROOT}/ydb/tests/tools/s3_recipe/recipe.inc)
+
+SIZE(MEDIUM)
+
+IF (SANITIZER_TYPE)
+    REQUIREMENTS(ram:16)
+ENDIF()
+
+END()
diff --git a/ydb/tests/functional/backup/ya.make b/ydb/tests/functional/backup/ya.make
@@ -16,7 +16,7 @@ PEERDIR(
 )
 
 SRCS(
-    main.cpp
+    backup_ut.cpp
 )
 
 INCLUDE(${ARCADIA_ROOT}/ydb/public/tools/ydb_recipe/recipe.inc)
@@ -29,3 +29,7 @@ IF (SANITIZER_TYPE)
 ENDIF()
 
 END()
+
+RECURSE(
+    s3_path_style
+)

Original file line number	Diff line number	Diff line change
`@@ -171,13 +171,15 @@ IExternalStorageOperator::TPtr TS3ExternalStorageConfig::DoConstructStorageOpera`
`171`	`171`	`TS3ExternalStorageConfig::TS3ExternalStorageConfig(const Ydb::Import::ImportFromS3Settings& settings)`
`172`	`172`	`: Config(ConfigFromSettings(settings))`
`173`	`173`	`, Credentials(CredentialsFromSettings(settings))`
	`174`	`+ , UseVirtualAddressing(!settings.disable_virtual_addressing())`
`174`	`175`	`{`
`175`	`176`	`Bucket = settings.bucket();`
`176`	`177`	`}`
`177`	`178`
`178`	`179`	`TS3ExternalStorageConfig::TS3ExternalStorageConfig(const Ydb::Export::ExportToS3Settings& settings)`
`179`	`180`	`: Config(ConfigFromSettings(settings))`
`180`	`181`	`, Credentials(CredentialsFromSettings(settings))`
	`182`	`+ , UseVirtualAddressing(!settings.disable_virtual_addressing())`
`181`	`183`	`{`
`182`	`184`	`Bucket = settings.bucket();`
`183`	`185`	`}`
Original file line number	Diff line number	Diff line change
`@@ -70,12 +70,13 @@ class TS3ClientWrapper : public IS3ClientWrapper {`
`70`	`70`	`} else {`
`71`	`71`	`throw TMisuseException() << "\"" << settings.Scheme_ << "\" scheme type is not supported";`
`72`	`72`	`}`
	`73`	`+ config.useVirtualAddressing = settings.UseVirtualAddressing_;`
`73`	`74`
`74`	`75`	`Client = std::make_unique<Aws::S3::S3Client>(`
`75`	`76`	`Aws::Auth::AWSCredentials(settings.AccessKey_, settings.SecretKey_),`
`76`	`77`	`config,`
`77`	`78`	`Aws::Client::AWSAuthV4Signer::PayloadSigningPolicy::Never,`
`78`		`- true);`
	`79`	`+ settings.UseVirtualAddressing_);`
`79`	`80`	`}`
`80`	`81`
`81`	`82`	`TListS3Result ListObjectKeys(const TString& prefix, const std::optional<TString>& token) override {`
Original file line number	Diff line number	Diff line change
`@@ -16,7 +16,7 @@ PEERDIR(`
`16`	`16`	`)`
`17`	`17`
`18`	`18`	`SRCS(`
`19`		`- main.cpp`
	`19`	`+ backup_ut.cpp`
`20`	`20`	`)`
`21`	`21`
`22`	`22`	`INCLUDE(${ARCADIA_ROOT}/ydb/public/tools/ydb_recipe/recipe.inc)`
`@@ -29,3 +29,7 @@ IF (SANITIZER_TYPE)`
`29`	`29`	`ENDIF()`
`30`	`30`
`31`	`31`	`END()`
	`32`	`+`
	`33`	`+RECURSE(`
	`34`	`+ s3_path_style`
	`35`	`+)`