From 70f7b1974ef94f5235efee993e79702cce6805b9 Mon Sep 17 00:00:00 2001 From: Hang Zheng Date: Wed, 9 Oct 2024 04:48:49 -0400 Subject: [PATCH] support the fromurl for the new added tls options --- cpp/src/arrow/filesystem/filesystem.h | 2 +- cpp/src/arrow/filesystem/s3_internal.h | 7 ++++--- cpp/src/arrow/filesystem/s3fs.cc | 7 +++++++ cpp/src/arrow/filesystem/s3fs.h | 2 +- cpp/src/arrow/filesystem/s3fs_test.cc | 13 +++++++++++++ 5 files changed, 26 insertions(+), 5 deletions(-) diff --git a/cpp/src/arrow/filesystem/filesystem.h b/cpp/src/arrow/filesystem/filesystem.h index 9bbabb3ea07c1..4ee5d8bbb15fa 100644 --- a/cpp/src/arrow/filesystem/filesystem.h +++ b/cpp/src/arrow/filesystem/filesystem.h @@ -711,7 +711,7 @@ struct FileSystemGlobalOptions { /// If empty, the underlying TLS library's defaults will be used. std::string tls_ca_dir_path; - /// Controls whether to verify SSL certificates, Default to true + /// Controls whether to verify TLS certificates. Defaults to true. bool tls_verify_certificates = true; }; diff --git a/cpp/src/arrow/filesystem/s3_internal.h b/cpp/src/arrow/filesystem/s3_internal.h index 3c2e94c119663..fc4b4288ae6dd 100644 --- a/cpp/src/arrow/filesystem/s3_internal.h +++ b/cpp/src/arrow/filesystem/s3_internal.h @@ -323,14 +323,15 @@ Status SetSSECustomerKey(S3RequestType& request, const std::string& sse_customer if (sse_customer_key.empty()) { return Status::OK(); // do nothing if the sse_customer_key is not configured } -#ifndef ARROW_S3_SUPPORT_SSEC - return Status::NotImplemented("SSE-C is not supported"); -#endif +#ifdef ARROW_S3_SUPPORT_SSEC ARROW_ASSIGN_OR_RAISE(auto md5, internal::CalculateSSECustomerKeyMD5(sse_customer_key)); request.SetSSECustomerKeyMD5(md5); request.SetSSECustomerKey(arrow::util::base64_encode(sse_customer_key)); request.SetSSECustomerAlgorithm("AES256"); return Status::OK(); +#else + return Status::NotImplemented("SSE-C is not supported"); +#endif } } // namespace internal diff --git a/cpp/src/arrow/filesystem/s3fs.cc b/cpp/src/arrow/filesystem/s3fs.cc index 827f900f3bbe9..ddc77927a3b61 100644 --- a/cpp/src/arrow/filesystem/s3fs.cc +++ b/cpp/src/arrow/filesystem/s3fs.cc @@ -408,6 +408,13 @@ Result S3Options::FromUri(const Uri& uri, std::string* out_path) { } else if (kv.first == "allow_bucket_deletion") { ARROW_ASSIGN_OR_RAISE(options.allow_bucket_deletion, ::arrow::internal::ParseBoolean(kv.second)); + } else if (kv.first == "tls_ca_file_path") { + options.tls_ca_file_path = kv.second; + } else if (kv.first == "tls_ca_dir_path") { + options.tls_ca_dir_path = kv.second; + } else if (kv.first == "tls_verify_certificates") { + ARROW_ASSIGN_OR_RAISE(options.tls_verify_certificates, + ::arrow::internal::ParseBoolean(kv.second)); } else { return Status::Invalid("Unexpected query parameter in S3 URI: '", kv.first, "'"); } diff --git a/cpp/src/arrow/filesystem/s3fs.h b/cpp/src/arrow/filesystem/s3fs.h index dbde76f05813b..a1c0a98a8001e 100644 --- a/cpp/src/arrow/filesystem/s3fs.h +++ b/cpp/src/arrow/filesystem/s3fs.h @@ -210,7 +210,7 @@ struct ARROW_EXPORT S3Options { /// If empty, the underlying TLS library's defaults will be used. std::string tls_ca_dir_path; - /// Controls whether to verify SSL certificates, Default to true + /// Controls whether to verify TLS certificates. Defaults to true. bool tls_verify_certificates = true; S3Options(); diff --git a/cpp/src/arrow/filesystem/s3fs_test.cc b/cpp/src/arrow/filesystem/s3fs_test.cc index 0aff942baf966..2a7db31ac4d5c 100644 --- a/cpp/src/arrow/filesystem/s3fs_test.cc +++ b/cpp/src/arrow/filesystem/s3fs_test.cc @@ -307,6 +307,16 @@ TEST_F(S3OptionsTest, FromUri) { ASSERT_EQ(options.endpoint_override, "localhost"); ASSERT_EQ(path, "mybucket/foo/bar"); + // Explicit tls related configuration + ASSERT_OK_AND_ASSIGN( + options, + S3Options::FromUri("s3://mybucket/foo/bar/?tls_ca_dir_path=/test&tls_ca_file_path=/" + "test/test.pem&tls_verify_certificates=false", + &path)); + ASSERT_EQ(options.tls_ca_dir_path, "/test"); + ASSERT_EQ(options.tls_ca_file_path, "/test/test.pem"); + ASSERT_EQ(options.tls_verify_certificates, true); + // Missing bucket name ASSERT_RAISES(Invalid, S3Options::FromUri("s3:///foo/bar/", &path)); @@ -449,6 +459,7 @@ class TestS3FS : public S3TestMixin { // Most tests will create buckets options_.allow_bucket_creation = true; options_.allow_bucket_deletion = true; + options_.tls_verify_certificates = false; MakeFileSystem(); // Set up test bucket { @@ -1303,6 +1314,7 @@ TEST_F(TestS3FS, OpenInputFile) { ASSERT_RAISES(IOError, file->Seek(10)); } +#ifdef MINIO_SERVER_WITH_TLS TEST_F(TestS3FS, SSECustomerKeyMatch) { // normal write/read with correct SSEC key std::shared_ptr stream; @@ -1330,6 +1342,7 @@ TEST_F(TestS3FS, SSECustomerKeyMismatch) { ASSERT_RAISES(IOError, fs_->OpenInputFile("bucket/newfile_with_sse_c")); ASSERT_OK(RestoreTestBucket()); } +#endif // MINIO_SERVER_WITH_TLS struct S3OptionsTestParameters { bool background_writes{false};