Skip to content

Commit

Permalink
only enable the https for ssec related unit test cases, meanwhile, fo…
Browse files Browse the repository at this point in the history
…r http mionio server, rollback to use the random host ip
  • Loading branch information
Hang Zheng committed Oct 18, 2024
1 parent a69961b commit 0afad6f
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 16 deletions.
7 changes: 5 additions & 2 deletions cpp/src/arrow/filesystem/s3_test_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ const char* kEnvConnectString = "ARROW_TEST_S3_CONNECT_STRING";
const char* kEnvAccessKey = "ARROW_TEST_S3_ACCESS_KEY";
const char* kEnvSecretKey = "ARROW_TEST_S3_SECRET_KEY";

std::string GenerateConnectString() { return GetListenAddress("localhost"); }
std::string GenerateConnectString() { return GetListenAddress(); }

} // namespace

Expand Down Expand Up @@ -145,7 +145,10 @@ Status MinioTestServer::Start(bool enable_tls_if_supported) {
minio_args.push_back("--certs-dir");
minio_args.push_back(ca_dir_path());
impl_->scheme_ = "https";
#endif // MINIO_SERVER_WITH_TLS
impl_->connect_string_ =
GetListenAddress("localhost"); // for TLS enabled case, we need to use localhost
// which is the fixed hostname in the certificate
#endif // MINIO_SERVER_WITH_TLS
}

ARROW_RETURN_NOT_OK(impl_->server_process_->SetExecutable(kMinioExecutableName));
Expand Down
30 changes: 16 additions & 14 deletions cpp/src/arrow/filesystem/s3fs_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,9 @@ class S3TestMixin : public AwsTestMixin {
std::unique_ptr<Aws::Client::ClientConfiguration> client_config_;
Aws::Auth::AWSCredentials credentials_;
std::unique_ptr<Aws::S3::S3Client> client_;
bool enable_tls_if_supported_ = true;
bool enable_tls_if_supported_ =
false; // by default, use the HTTP for all the test cases, since there is the
// random failure when there is stress test against the minio HTTPS Server
};

void AssertGetObject(Aws::S3::Model::GetObjectResult& result,
Expand Down Expand Up @@ -940,15 +942,7 @@ TEST_F(TestS3FS, GetFileInfoGenerator) {
// Non-root dir case is tested by generic tests
}

class TestS3FSHTTP : public TestS3FS {
public:
void SetUp() override {
enable_tls_if_supported_ = true;
TestS3FS::SetUp();
}
};

TEST_F(TestS3FSHTTP, GetFileInfoGeneratorStress) {
TEST_F(TestS3FS, GetFileInfoGeneratorStress) {
// This test is slow because it needs to create a bunch of seed files. However, it is
// the only test that stresses listing and deleting when there are more than 1000
// files and paging is required.
Expand Down Expand Up @@ -1351,7 +1345,15 @@ TEST_F(TestS3FS, OpenInputFile) {

// Minio only allows Server Side Encryption on HTTPS client connections.
#ifdef MINIO_SERVER_WITH_TLS
TEST_F(TestS3FS, SSECustomerKeyMatch) {
class TestS3FSHTTPS : public TestS3FS {
public:
void SetUp() override {
enable_tls_if_supported_ = true;
TestS3FS::SetUp();
}
};

TEST_F(TestS3FSHTTPS, SSECustomerKeyMatch) {
// normal write/read with correct SSE-C key
std::shared_ptr<io::OutputStream> stream;
options_.sse_customer_key = "12345678123456781234567812345678";
Expand All @@ -1369,7 +1371,7 @@ TEST_F(TestS3FS, SSECustomerKeyMatch) {
}
}

TEST_F(TestS3FS, SSECustomerKeyMismatch) {
TEST_F(TestS3FSHTTPS, SSECustomerKeyMismatch) {
std::shared_ptr<io::OutputStream> stream;
for (const auto& allow_delayed_open : {false, true}) {
options_.allow_delayed_open = allow_delayed_open;
Expand All @@ -1385,7 +1387,7 @@ TEST_F(TestS3FS, SSECustomerKeyMismatch) {
}
}

TEST_F(TestS3FS, SSECustomerKeyMissing) {
TEST_F(TestS3FSHTTPS, SSECustomerKeyMissing) {
std::shared_ptr<io::OutputStream> stream;
for (const auto& allow_delayed_open : {false, true}) {
options_.allow_delayed_open = allow_delayed_open;
Expand All @@ -1402,7 +1404,7 @@ TEST_F(TestS3FS, SSECustomerKeyMissing) {
}
}

TEST_F(TestS3FS, SSECustomerKeyCopyFile) {
TEST_F(TestS3FSHTTPS, SSECustomerKeyCopyFile) {
ASSERT_OK_AND_ASSIGN(auto stream, fs_->OpenOutputStream("bucket/newfile_with_sse_c"));
ASSERT_OK(stream->Write("some"));
ASSERT_OK(stream->Close());
Expand Down

0 comments on commit 0afad6f

Please sign in to comment.