diff --git a/.github/workflows/MainDistributionPipeline.yml b/.github/workflows/MainDistributionPipeline.yml index ed1de59..d455a81 100644 --- a/.github/workflows/MainDistributionPipeline.yml +++ b/.github/workflows/MainDistributionPipeline.yml @@ -19,20 +19,20 @@ concurrency: jobs: duckdb-stable-build: name: Build extension binaries - uses: duckdb/extension-ci-tools/.github/workflows/_extension_distribution.yml@v1.5.1 + uses: duckdb/extension-ci-tools/.github/workflows/_extension_distribution.yml@v1.5.5 with: - duckdb_version: v1.5.1 - ci_tools_version: v1.5.1 + duckdb_version: v1.5.5 + ci_tools_version: v1.5.5 extension_name: proto_iceberg - # iceberg-cpp does not build for Windows or WebAssembly yet. - exclude_archs: 'windows_amd64;windows_amd64_mingw;wasm_mvp;wasm_eh;wasm_threads' + # windows_amd64_mingw builds with rtools42's GCC, which predates C++23; iceberg-cpp does not build for WebAssembly. + exclude_archs: 'windows_amd64_mingw;wasm_mvp;wasm_eh;wasm_threads' code-quality-check: name: Code Quality Check - uses: duckdb/extension-ci-tools/.github/workflows/_extension_code_quality.yml@v1.5.1 + uses: duckdb/extension-ci-tools/.github/workflows/_extension_code_quality.yml@v1.5.5 with: - duckdb_version: v1.5.1 - ci_tools_version: v1.5.1 + duckdb_version: v1.5.5 + ci_tools_version: v1.5.5 extension_name: proto_iceberg # FIXME: Disable tidy to prevent full compilation format_checks: 'format' diff --git a/.gitmodules b/.gitmodules index e70425f..28c9b8c 100644 --- a/.gitmodules +++ b/.gitmodules @@ -5,9 +5,9 @@ [submodule "extension-ci-tools"] path = extension-ci-tools url = https://github.com/duckdb/extension-ci-tools - branch = v1.5.1 + branch = v1.5.5 # TODO: Switch to upstream apache/iceberg-cpp once requirements merged [submodule "third_party/iceberg-cpp"] path = third_party/iceberg-cpp url = https://github.com/smaheshwar-pltr/iceberg-cpp.git - branch = duckdb-iceberg + branch = duckdb-proto-iceberg diff --git a/README.md b/README.md index d928564..62eb4f2 100644 --- a/README.md +++ b/README.md @@ -39,7 +39,7 @@ GEN=ninja make release # or: GEN=ninja make debug The first build takes longer: CMake downloads and builds iceberg-cpp and its vendored dependencies (Arrow, the AWS SDK and others) while configuring. Later builds skip this unless the submodule or its build settings change. Set `CMAKE_BUILD_PARALLEL_LEVEL` to limit build parallelism on machines with little memory. -CI also builds loadable binaries for Linux (amd64, arm64) and macOS 13.3+ (amd64, arm64) with DuckDB's [extension-ci-tools](https://github.com/duckdb/extension-ci-tools); each run of the Main Extension Distribution Pipeline workflow attaches them as artifacts. +CI also builds loadable binaries for Linux (amd64, arm64), macOS 13.3+ (amd64, arm64) and Windows (amd64) with DuckDB's [extension-ci-tools](https://github.com/duckdb/extension-ci-tools); each run of the Main Extension Distribution Pipeline workflow attaches them as artifacts. See [docs/UPDATING.md](docs/UPDATING.md) for updating DuckDB or iceberg-cpp. diff --git a/cmake/duckdb_cxx23_patch.cmake b/cmake/duckdb_cxx23_patch.cmake index ff32c8d..79a1b08 100644 --- a/cmake/duckdb_cxx23_patch.cmake +++ b/cmake/duckdb_cxx23_patch.cmake @@ -1,4 +1,4 @@ -# DuckDB v1.5.1's profiling_utils.hpp does not compile in a C++23 translation unit under libc++ +# DuckDB v1.5's profiling_utils.hpp does not compile in a C++23 translation unit under libc++ # (Apple's standard library): QueryMetrics resets a unique_ptr in an inline member # function before ActiveTimer is complete, and libc++ instantiates the constexpr unique_ptr members # eagerly in C++23. The patch moves those member functions below ActiveTimer's definition. It is @@ -17,7 +17,7 @@ if(APPLE) RESULT_VARIABLE result) if(NOT result EQUAL 0) message(FATAL_ERROR "[proto_iceberg] Cannot apply ${patch} to the DuckDB tree at " - "${CMAKE_SOURCE_DIR}. Is it still DuckDB v1.5.1?") + "${CMAKE_SOURCE_DIR}. Is it still DuckDB v1.5?") endif() message(STATUS "[proto_iceberg] Applied ${patch}") endif() diff --git a/cmake/iceberg_cpp.cmake b/cmake/iceberg_cpp.cmake index 4c51ba1..f4994da 100644 --- a/cmake/iceberg_cpp.cmake +++ b/cmake/iceberg_cpp.cmake @@ -18,6 +18,8 @@ block() set(init_cache [==[ set(CMAKE_POSITION_INDEPENDENT_CODE ON CACHE BOOL "") set(CMAKE_INSTALL_LIBDIR lib CACHE PATH "") +# Lets the forwarded CMAKE_MSVC_RUNTIME_LIBRARY reach bundled dependencies that predate CMake 3.15. +set(CMAKE_POLICY_DEFAULT_CMP0091 NEW CACHE STRING "") # Build every dependency from source rather than picking up system packages. set(FETCHCONTENT_TRY_FIND_PACKAGE_MODE NEVER CACHE STRING "") set(ICEBERG_BUILD_STATIC ON CACHE BOOL "") @@ -40,6 +42,8 @@ set(ICEBERG_S3 ON CACHE BOOL "") CMAKE_OSX_ARCHITECTURES CMAKE_OSX_DEPLOYMENT_TARGET CMAKE_OSX_SYSROOT + # DuckDB links the static MSVC runtime, and all objects in the extension must agree. + CMAKE_MSVC_RUNTIME_LIBRARY # Set by the vcpkg toolchain, so that the sub-build uses the same installed packages. VCPKG_TARGET_TRIPLET VCPKG_HOST_TRIPLET @@ -49,6 +53,11 @@ set(ICEBERG_S3 ON CACHE BOOL "") endif() endforeach() + # The AWS libraries Arrow bundles choose their MSVC runtime from their own option instead. + if(MSVC AND CMAKE_MSVC_RUNTIME_LIBRARY AND NOT CMAKE_MSVC_RUNTIME_LIBRARY MATCHES "DLL") + string(APPEND init_cache "set(AWS_STATIC_MSVC_RUNTIME_LIBRARY ON CACHE BOOL \"\")\n") + endif() + execute_process(COMMAND git rev-parse HEAD WORKING_DIRECTORY "${source_dir}" OUTPUT_VARIABLE revision diff --git a/docs/UPDATING.md b/docs/UPDATING.md index 7a5f9b8..c006433 100644 --- a/docs/UPDATING.md +++ b/docs/UPDATING.md @@ -5,7 +5,7 @@ The extension targets a DuckDB release, which is pinned in several places that must move together. Check out submodule commits explicitly rather than using `make update` or `make pull`, which move every submodule to the tip of its tracked branch. - The `duckdb` submodule: check out the release tag, and set its `branch` in `.gitmodules` to the release branch. -- The `extension-ci-tools` submodule: check out the branch named after the release (e.g. `v1.5.1`), and set its `branch` in `.gitmodules` to match. +- The `extension-ci-tools` submodule: check out the branch named after the release (e.g. `v1.5.5`), and set its `branch` in `.gitmodules` to match. - `.github/workflows/MainDistributionPipeline.yml`: the reusable workflow refs and the `duckdb_version` and `ci_tools_version` inputs. - `extension_config.cmake`: the httpfs `GIT_TAG`, which should match `duckdb/.github/config/extensions/httpfs.cmake`. - `cmake/duckdb_cxx23.patch`: check whether it is still needed. On macOS, configuring fails if it no longer applies. diff --git a/duckdb b/duckdb index 7dbb2e6..d8cdaa3 160000 --- a/duckdb +++ b/duckdb @@ -1 +1 @@ -Subproject commit 7dbb2e646fea939a89f10a55aa98c474cbb0c098 +Subproject commit d8cdaa33fda8df955cc76ef58a280f68f4cd43fa diff --git a/extension-ci-tools b/extension-ci-tools index ef15a2a..72e76e9 160000 --- a/extension-ci-tools +++ b/extension-ci-tools @@ -1 +1 @@ -Subproject commit ef15a2a7453db5b4f85b7c668a545ae2f1193ff6 +Subproject commit 72e76e99cd7fee45a99739cd118ec2db64e034ec diff --git a/extension_config.cmake b/extension_config.cmake index f5faf5d..25436b3 100644 --- a/extension_config.cmake +++ b/extension_config.cmake @@ -1,5 +1,5 @@ # Extensions built by `make`: this one, plus httpfs, which the SQL tests use to read table data from S3. -# httpfs is pinned to the commit DuckDB v1.5.1 builds against. +# httpfs is pinned to the commit DuckDB v1.5.5 builds against. duckdb_extension_load(proto_iceberg SOURCE_DIR ${CMAKE_CURRENT_LIST_DIR} @@ -8,5 +8,5 @@ duckdb_extension_load(proto_iceberg duckdb_extension_load(httpfs GIT_URL https://github.com/duckdb/duckdb-httpfs - GIT_TAG 7e86e7a5e5a1f01f458361bebdfa9b0a9a73a619 + GIT_TAG 827222fb45a043a7a852d1f7aae46901492a3cda ) diff --git a/src/include/proto_iceberg_catalog.hpp b/src/include/proto_iceberg_catalog.hpp index f352eac..8cd4988 100644 --- a/src/include/proto_iceberg_catalog.hpp +++ b/src/include/proto_iceberg_catalog.hpp @@ -7,7 +7,7 @@ #include "duckdb/parser/parsed_data/attach_info.hpp" #include "duckdb/storage/storage_extension.hpp" -#include "iceberg/catalog/rest/rest_catalog.h" +#include "iceberg/catalog.h" namespace duckdb { @@ -15,8 +15,8 @@ class ProtoIcebergSchemaEntry; class ProtoIcebergCatalog : public Catalog { public: - ProtoIcebergCatalog(AttachedDatabase &db_p, string catalog_uri, - std::shared_ptr rest_catalog, string default_schema); + ProtoIcebergCatalog(AttachedDatabase &db_p, string catalog_uri, std::shared_ptr rest_catalog, + string default_schema); static unique_ptr Attach(optional_ptr storage_info, ClientContext &context, AttachedDatabase &db, const string &name, AttachInfo &info, @@ -32,7 +32,7 @@ class ProtoIcebergCatalog : public Catalog { /// Acquires exclusive use of the REST catalog. Every call into it, including through a loaded iceberg::Table (e.g. /// Table::Refresh()), must hold the returned guard, and only for the duration of that call. - Mutex>::Guard LockRestCatalog() { + Mutex>::Guard LockRestCatalog() { return rest_catalog_.Lock(); } @@ -93,7 +93,7 @@ class ProtoIcebergCatalog : public Catalog { string default_schema_; // TODO: Drop the lock once iceberg-cpp documents RestCatalog as thread-safe. It currently isn't: its HttpClient // shares one libcurl connection cache across all requests. - Mutex> rest_catalog_; + Mutex> rest_catalog_; }; } // namespace duckdb diff --git a/src/include/s3_conversion.hpp b/src/include/s3_conversion.hpp index 78b18b4..6f58262 100644 --- a/src/include/s3_conversion.hpp +++ b/src/include/s3_conversion.hpp @@ -4,8 +4,11 @@ #include "duckdb/common/case_insensitive_map.hpp" #include "duckdb/common/types/value.hpp" +#include "iceberg/storage_credential.h" +#include #include +#include #include namespace duckdb::conversion { @@ -21,4 +24,13 @@ ConvertS3SecretToIcebergProperties(const case_insensitive_tree_t &secret) [[nodiscard]] case_insensitive_map_t ConvertIcebergPropertiesToS3Secret(const std::unordered_map &properties); +/// Overlays the vended S3 storage credential whose prefix is the longest match for location onto FileIO properties. +/// +/// REST catalogs vend credentials either in the table config, which iceberg-cpp merges into the FileIO properties, +/// or as storage credentials scoped to location prefixes, which it keeps separately. Matching follows iceberg-cpp: +/// non-S3 prefixes are skipped and S3 scheme aliases such as s3a:// compare equal to s3://. +[[nodiscard]] std::unordered_map +MergeStorageCredential(std::unordered_map properties, + std::span credentials, std::string_view location); + } // namespace duckdb::conversion diff --git a/src/proto_iceberg_catalog.cpp b/src/proto_iceberg_catalog.cpp index 2a4dbe1..1a6b630 100644 --- a/src/proto_iceberg_catalog.cpp +++ b/src/proto_iceberg_catalog.cpp @@ -11,8 +11,7 @@ namespace duckdb { ProtoIcebergCatalog::ProtoIcebergCatalog(AttachedDatabase &db_p, string catalog_uri, - std::shared_ptr rest_catalog, - string default_schema) + std::shared_ptr rest_catalog, string default_schema) : Catalog(db_p), catalog_uri_(std::move(catalog_uri)), default_schema_(std::move(default_schema)), rest_catalog_(std::in_place, std::move(rest_catalog)) { } diff --git a/src/proto_iceberg_catalog_attach.cpp b/src/proto_iceberg_catalog_attach.cpp index c9d9aa7..e2d8ef8 100644 --- a/src/proto_iceberg_catalog_attach.cpp +++ b/src/proto_iceberg_catalog_attach.cpp @@ -9,6 +9,7 @@ #include "duckdb/main/secret/secret.hpp" #include "iceberg/catalog/rest/catalog_properties.h" +#include "iceberg/catalog/rest/rest_catalog.h" #include @@ -27,6 +28,9 @@ using constants::kWarehouse; const string kUri = "uri"; const string kHeaderAuthorization = "header.Authorization"; +/// Asks the catalog to vend storage credentials with each loaded table. Some catalogs only vend when asked. +const string kHeaderAccessDelegation = "header.X-Iceberg-Access-Delegation"; +const string kVendedCredentials = "vended-credentials"; /// Path to look up the user's S3 secret at: only secrets scoped to all S3 paths match it, not per-bucket ones or the /// per-table secrets this extension creates. const string kS3RootPath = "s3://"; @@ -134,10 +138,15 @@ unique_ptr ProtoIcebergCatalog::Attach(optional_ptrAsCatalog(), "Failed to open Iceberg REST catalog at %s", params.uri); auto catalog = make_uniq(db, std::move(params.uri), std::move(rest_catalog), std::move(params.default_schema)); diff --git a/src/proto_iceberg_extension.cpp b/src/proto_iceberg_extension.cpp index 7e74db7..1facae3 100644 --- a/src/proto_iceberg_extension.cpp +++ b/src/proto_iceberg_extension.cpp @@ -13,6 +13,8 @@ #include "iceberg/arrow/arrow_register.h" #include "iceberg/avro/avro_register.h" +#include + namespace duckdb { namespace { @@ -53,6 +55,23 @@ void RegisterIcebergFileIO() { iceberg::avro::RegisterAll(); } +/// Turns off the AWS SDK's logging unless the user configured it, before iceberg-cpp initializes Arrow's S3 support. +/// +/// When Arrow finalizes S3 at exit, the AWS CRT can still be shutting down an event loop on another thread, which +/// logs through the SDK's logger after the SDK has torn it down and crashes. With logging off the SDK installs no +/// logger. Arrow reads ARROW_S3_LOG_LEVEL when S3 is first initialized. The variable is process-wide, so it also +/// applies to other Arrow S3 users in the process and to child processes. +void DisableAwsSdkLogging() { + static constexpr const char *kArrowS3LogLevel = "ARROW_S3_LOG_LEVEL"; +#ifdef _WIN32 + if (!std::getenv(kArrowS3LogLevel)) { + _putenv_s(kArrowS3LogLevel, "off"); + } +#else + setenv(kArrowS3LogLevel, "off", /*overwrite=*/0); +#endif +} + void RegisterIcebergSecretType(ExtensionLoader &loader) { SecretType iceberg_secret_type; iceberg_secret_type.name = kIcebergSecretType; @@ -74,6 +93,7 @@ void RegisterIcebergSecretFunction(ExtensionLoader &loader) { void LoadInternal(ExtensionLoader &loader) { auto &instance = loader.GetDatabaseInstance(); + DisableAwsSdkLogging(); RegisterIcebergFileIO(); RegisterIcebergSecretType(loader); diff --git a/src/proto_iceberg_table_entry.cpp b/src/proto_iceberg_table_entry.cpp index 0979a4b..a1fe7ae 100644 --- a/src/proto_iceberg_table_entry.cpp +++ b/src/proto_iceberg_table_entry.cpp @@ -67,15 +67,20 @@ CreateSecretInput BuildScopedS3Secret(const string &catalog_name, const iceberg: auto input = MakeBaseS3SecretInput(); input.name = GenerateScopedSecretName(catalog_name, table.name(), txn_id); - input.scope.push_back(std::move(scope_prefix)); + input.scope.push_back(scope_prefix); // Scope to write.data.path too, that may live outside the table's location if (string write_data_path {table.properties().Get(iceberg::TableProperties::kWriteDataLocation)}; !write_data_path.empty()) { input.scope.push_back(scope_with_slash(std::move(write_data_path))); } - // TODO: Respect storage credentials REST field, not just the credentials in IO properties - input.options = conversion::ConvertIcebergPropertiesToS3Secret(io->properties()); + std::span credentials; + if (const auto *credentialed = io->AsSupportsStorageCredentials()) { + credentials = credentialed->credentials(); + } + // Match on the slash-terminated table location, so that a credential for "/" applies. + input.options = conversion::ConvertIcebergPropertiesToS3Secret( + conversion::MergeStorageCredential(io->properties(), credentials, scope_prefix)); return input; } diff --git a/src/s3_conversion.cpp b/src/s3_conversion.cpp index c9dab18..47423f2 100644 --- a/src/s3_conversion.cpp +++ b/src/s3_conversion.cpp @@ -1,6 +1,7 @@ #include "s3_conversion.hpp" #include "iceberg/arrow/s3/s3_properties.h" +#include "iceberg/util/property_util.h" #include #include @@ -31,20 +32,9 @@ constexpr std::array kPlainPropertyMappings = {{ {"key_id", S3Properties::kAccessKeyId}, {"secret", S3Properties::kSecretAccessKey}, {"session_token", S3Properties::kSessionToken}, - {"region", S3Properties::kRegion}, + {"region", S3Properties::kClientRegion}, }}; -/// Parses an iceberg-cpp boolean property, which is exactly "true" or "false". -std::optional ParseBool(std::string_view value) { - if (value == "true") { - return true; - } - if (value == "false") { - return false; - } - return std::nullopt; -} - /// Returns whether an Iceberg S3 endpoint's http(s) scheme implies SSL, or nullopt if it has no such scheme. std::optional SchemeUsesSsl(std::string_view endpoint) { if (endpoint.starts_with(kHttpsScheme)) { @@ -56,6 +46,16 @@ std::optional SchemeUsesSsl(std::string_view endpoint) { return std::nullopt; } +/// Rewrites an S3 scheme alias such as s3a:// or S3:// to s3://, as iceberg-cpp does before matching credential +/// prefixes. +std::string CanonicalizeS3Scheme(std::string_view location) { + if (auto separator = location.find("://"); + separator != std::string_view::npos && iceberg::arrow::IsS3Scheme(location.substr(0, separator))) { + return "s3://" + std::string(location.substr(separator + 3)); + } + return std::string(location); +} + /// Returns DuckDB's scheme-less form of an Iceberg S3 endpoint, e.g. host:9000/path for http://host:9000/path/. std::string_view ToDuckDBEndpoint(std::string_view endpoint) { if (endpoint.starts_with(kHttpsScheme)) { @@ -123,19 +123,47 @@ ConvertIcebergPropertiesToS3Secret(const std::unordered_map +MergeStorageCredential(std::unordered_map properties, + std::span credentials, std::string_view location) { + const auto canonical_location = CanonicalizeS3Scheme(location); + const iceberg::StorageCredential *best = nullptr; + size_t best_length = 0; + for (const auto &credential : credentials) { + if (!iceberg::arrow::IsS3CredentialPrefix(credential.prefix)) { + continue; + } + auto prefix = CanonicalizeS3Scheme(credential.prefix); + if (prefix.size() > best_length && canonical_location.starts_with(prefix)) { + best = &credential; + best_length = prefix.size(); + } + } + if (best) { + for (const auto &[key, value] : best->config) { + properties.insert_or_assign(key, value); + } + } + return properties; +} + } // namespace duckdb::conversion diff --git a/test/test_s3_conversion.cpp b/test/test_s3_conversion.cpp index 97b2ab2..0827f7e 100644 --- a/test/test_s3_conversion.cpp +++ b/test/test_s3_conversion.cpp @@ -4,6 +4,7 @@ #include #include #include +#include using namespace duckdb; using namespace duckdb::conversion; @@ -48,7 +49,7 @@ TEST_CASE("secret to properties: plain keys pass through", "[s3_conversion]") { {"secret", Value("shh")}, {"session_token", Value("tok")}, {"region", Value("eu-west-1")}})) == - "s3.access-key-id=AKIA, s3.region=eu-west-1, s3.secret-access-key=shh, s3.session-token=tok"); + "client.region=eu-west-1, s3.access-key-id=AKIA, s3.secret-access-key=shh, s3.session-token=tok"); } TEST_CASE("secret to properties: empty, null and unrelated keys are omitted", "[s3_conversion]") { @@ -89,7 +90,7 @@ TEST_CASE("properties to secret: plain keys pass through", "[s3_conversion]") { REQUIRE(Render(ConvertIcebergPropertiesToS3Secret({{"s3.access-key-id", "AKIA"}, {"s3.secret-access-key", "shh"}, {"s3.session-token", "tok"}, - {"s3.region", "eu-west-1"}})) == + {"client.region", "eu-west-1"}})) == "key_id='AKIA', region='eu-west-1', secret='shh', session_token='tok'"); } @@ -97,12 +98,14 @@ TEST_CASE("properties to secret: empty and unrelated properties are omitted", "[ REQUIRE(Render(ConvertIcebergPropertiesToS3Secret({{"s3.access-key-id", ""}, {"s3.endpoint", ""}, {"s3.connect-timeout-ms", "1000"}, - {"client.region", "us-east-1"}})) == ""); + {"s3.region", "us-east-1"}})) == ""); } -TEST_CASE("properties to secret: non-boolean path-style access and SSL enabled are ignored", "[s3_conversion]") { +TEST_CASE("properties to secret: booleans are true only for true in any case", "[s3_conversion]") { REQUIRE(Render(ConvertIcebergPropertiesToS3Secret( - {{"s3.path-style-access", "True"}, {"s3.ssl.enabled", "FALSE"}})) == ""); + {{"s3.path-style-access", "True"}, {"s3.ssl.enabled", "TRUE"}})) == "url_style='path', use_ssl=true"); + REQUIRE(Render(ConvertIcebergPropertiesToS3Secret({{"s3.path-style-access", "yes"}, {"s3.ssl.enabled", ""}})) == + "url_style='vhost', use_ssl=false"); } TEST_CASE("properties to secret: path-style access maps to URL style", "[s3_conversion]") { @@ -125,13 +128,13 @@ TEST_CASE("properties to secret: https endpoint drops the scheme and enables SSL "endpoint='s3.eu-west-1.amazonaws.com', use_ssl=true"); } -TEST_CASE("properties to secret: endpoint scheme takes precedence over SSL enabled", "[s3_conversion]") { +TEST_CASE("properties to secret: SSL enabled takes precedence over the endpoint scheme", "[s3_conversion]") { REQUIRE(Render(ConvertIcebergPropertiesToS3Secret( {{"s3.endpoint", "http://minio:9000"}, {"s3.ssl.enabled", "true"}})) == - "endpoint='minio:9000', use_ssl=false"); + "endpoint='minio:9000', use_ssl=true"); REQUIRE( Render(ConvertIcebergPropertiesToS3Secret({{"s3.endpoint", "https://storage"}, {"s3.ssl.enabled", "false"}})) == - "endpoint='storage', use_ssl=true"); + "endpoint='storage', use_ssl=false"); } TEST_CASE("properties to secret: scheme-less endpoint takes SSL from SSL enabled", "[s3_conversion]") { @@ -170,3 +173,49 @@ TEST_CASE("secret round-trips through iceberg-cpp properties", "[s3_conversion]" {"use_ssl", Value::BOOLEAN(false)}}; REQUIRE(Render(ConvertIcebergPropertiesToS3Secret(ConvertS3SecretToIcebergProperties(secret))) == Render(secret)); } + +TEST_CASE("storage credential: without credentials the properties are unchanged", "[s3_conversion]") { + REQUIRE(Render(MergeStorageCredential({{"s3.access-key-id", "AKIA"}}, {}, "s3://bucket/table/")) == + "s3.access-key-id=AKIA"); +} + +TEST_CASE("storage credential: the longest matching prefix overrides the properties", "[s3_conversion]") { + std::vector credentials = { + {.prefix = "s3://bucket/", .config = {{"s3.access-key-id", "BUCKET"}, {"s3.session-token", "bucket-token"}}}, + {.prefix = "s3://bucket/table", .config = {{"s3.access-key-id", "TABLE"}}}, + {.prefix = "s3://other/", .config = {{"s3.access-key-id", "OTHER"}}}, + }; + REQUIRE(Render(MergeStorageCredential({{"s3.access-key-id", "CATALOG"}, {"client.region", "eu-west-1"}}, + credentials, "s3://bucket/table/")) == + "client.region=eu-west-1, s3.access-key-id=TABLE"); +} + +TEST_CASE("storage credential: credentials for other locations are ignored", "[s3_conversion]") { + std::vector credentials = { + {.prefix = "s3://other/", .config = {{"s3.access-key-id", "OTHER"}}}, + }; + REQUIRE(Render(MergeStorageCredential({{"s3.access-key-id", "CATALOG"}}, credentials, "s3://bucket/table/")) == + "s3.access-key-id=CATALOG"); +} + +TEST_CASE("storage credential: a prefix ending in a slash matches the table location", "[s3_conversion]") { + std::vector credentials = { + {.prefix = "s3://bucket/table/", .config = {{"s3.access-key-id", "TABLE"}}}, + }; + REQUIRE(Render(MergeStorageCredential({}, credentials, "s3://bucket/table/")) == "s3.access-key-id=TABLE"); +} + +TEST_CASE("storage credential: S3 scheme aliases compare equal", "[s3_conversion]") { + std::vector credentials = { + {.prefix = "s3a://bucket/", .config = {{"s3.access-key-id", "BUCKET"}}}, + }; + REQUIRE(Render(MergeStorageCredential({}, credentials, "S3://bucket/table/")) == "s3.access-key-id=BUCKET"); +} + +TEST_CASE("storage credential: non-S3 prefixes are skipped", "[s3_conversion]") { + std::vector credentials = { + {.prefix = "gs://bucket/", .config = {{"s3.access-key-id", "GCS"}}}, + {.prefix = "s3", .config = {{"s3.access-key-id", "S3"}}}, + }; + REQUIRE(Render(MergeStorageCredential({}, credentials, "s3://bucket/table/")) == "s3.access-key-id=S3"); +} diff --git a/third_party/iceberg-cpp b/third_party/iceberg-cpp index 5cbe377..d307d0c 160000 --- a/third_party/iceberg-cpp +++ b/third_party/iceberg-cpp @@ -1 +1 @@ -Subproject commit 5cbe377ba26e046c7970abae386fb310be2f8c56 +Subproject commit d307d0ced0bf0376da7ca6e1658cf1f3a0308e7c