Skip to content

Commit 968d073

Browse files
feat(rest): bind a per-table FileIO from vended credentials
1 parent 00ef76a commit 968d073

7 files changed

Lines changed: 105 additions & 12 deletions

File tree

‎src/iceberg/arrow/s3/arrow_s3_file_io.cc‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -100,15 +100,25 @@ Result<::arrow::fs::S3Options> ConfigureS3Options(
100100
}
101101

102102
// Configure region
103-
if (const auto* region = FindProperty(properties, S3Properties::kRegion);
104-
region != nullptr) {
103+
// Prefer the standard `client.region`; fall back to legacy `s3.region`.
104+
const auto* region = FindProperty(properties, S3Properties::kClientRegion);
105+
if (region == nullptr) {
106+
region = FindProperty(properties, S3Properties::kRegion);
107+
}
108+
if (region != nullptr) {
105109
options.region = *region;
106110
}
107111

108-
// Configure endpoint (for MinIO, LocalStack, etc.)
112+
// Configure endpoint (for MinIO, LocalStack, OSS, etc.)
109113
if (const auto* endpoint = FindProperty(properties, S3Properties::kEndpoint);
110114
endpoint != nullptr) {
111-
options.endpoint_override = *endpoint;
115+
// `s3.endpoint` may be a full URI; Arrow wants host[:port], so strip the scheme.
116+
std::string_view ep = *endpoint;
117+
if (const auto pos = ep.find("://"); pos != std::string_view::npos) {
118+
options.scheme = std::string(ep.substr(0, pos));
119+
ep = ep.substr(pos + 3);
120+
}
121+
options.endpoint_override = std::string(ep);
112122
} else {
113123
// Fall back to AWS standard environment variables for endpoint override
114124
const char* s3_endpoint_env = std::getenv("AWS_ENDPOINT_URL_S3");

‎src/iceberg/arrow/s3/s3_properties.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,10 @@ struct S3Properties {
3737
static constexpr std::string_view kSecretAccessKey = "s3.secret-access-key";
3838
/// AWS session token (for temporary credentials)
3939
static constexpr std::string_view kSessionToken = "s3.session-token";
40-
/// AWS region
40+
/// AWS region (legacy, non-standard key kept for compatibility)
4141
static constexpr std::string_view kRegion = "s3.region";
42+
/// AWS region, standard Iceberg client property (preferred over kRegion).
43+
static constexpr std::string_view kClientRegion = "client.region";
4244
/// Custom endpoint override (for MinIO, LocalStack, etc.)
4345
static constexpr std::string_view kEndpoint = "s3.endpoint";
4446
/// Whether to use path-style access (needed for MinIO)

‎src/iceberg/catalog/rest/rest_catalog.cc‎

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
#include "iceberg/catalog/rest/rest_file_io.h"
3939
#include "iceberg/catalog/rest/rest_util.h"
4040
#include "iceberg/catalog/rest/types.h"
41+
#include "iceberg/file_io_registry.h"
4142
#include "iceberg/json_serde_internal.h"
4243
#include "iceberg/partition_spec.h"
4344
#include "iceberg/result.h"
@@ -486,13 +487,36 @@ Result<LoadTableResult> RestCatalog::LoadTableResultFor(
486487
return LoadTableResultFromJson(json);
487488
}
488489

490+
Result<std::shared_ptr<FileIO>> RestCatalog::MakeFileIOForTable(
491+
const LoadTableResult& load_result) const {
492+
// Longest-prefix S3-family vended credential, matching the Java client's
493+
// VendedCredentialsProvider.
494+
const StorageCredential* s3_cred =
495+
SelectS3StorageCredential(load_result.storage_credentials);
496+
if (s3_cred == nullptr) {
497+
return file_io_;
498+
}
499+
500+
// Build an S3 FileIO from catalog + table config + the vended credential; its
501+
// ResolvePath resolves oss:// and other S3-compatible schemes.
502+
auto properties = config_.configs();
503+
for (const auto& [key, value] : load_result.config) {
504+
properties[key] = value;
505+
}
506+
for (const auto& [key, value] : s3_cred->config) {
507+
properties[key] = value;
508+
}
509+
ICEBERG_ASSIGN_OR_RAISE(
510+
auto io,
511+
FileIORegistry::Load(std::string(FileIORegistry::kArrowS3FileIO), properties));
512+
return std::shared_ptr<FileIO>(std::move(io));
513+
}
514+
489515
Result<std::shared_ptr<Table>> RestCatalog::LoadTable(const TableIdentifier& identifier) {
490516
ICEBERG_ASSIGN_OR_RAISE(auto load_result, LoadTableResultFor(identifier));
491-
// Binds the catalog's default FileIO; use LoadTableResultFor() for vended creds.
492-
// TODO: Support table-specific auth config and per-table FileIO from the REST
493-
// load response when table-scoped REST operations are introduced.
517+
ICEBERG_ASSIGN_OR_RAISE(auto io, MakeFileIOForTable(load_result));
494518
return Table::Make(identifier, std::move(load_result.metadata),
495-
std::move(load_result.metadata_location), file_io_,
519+
std::move(load_result.metadata_location), std::move(io),
496520
shared_from_this());
497521
}
498522

@@ -514,10 +538,9 @@ Result<std::shared_ptr<Table>> RestCatalog::RegisterTable(
514538

515539
ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.body()));
516540
ICEBERG_ASSIGN_OR_RAISE(auto load_result, LoadTableResultFromJson(json));
517-
// TODO: Support table-specific auth config and per-table FileIO from the REST
518-
// register response when table-scoped REST operations are introduced.
541+
ICEBERG_ASSIGN_OR_RAISE(auto io, MakeFileIOForTable(load_result));
519542
return Table::Make(identifier, std::move(load_result.metadata),
520-
std::move(load_result.metadata_location), file_io_,
543+
std::move(load_result.metadata_location), std::move(io),
521544
shared_from_this());
522545
}
523546

‎src/iceberg/catalog/rest/rest_catalog.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,11 @@ class ICEBERG_REST_EXPORT RestCatalog : public Catalog,
113113

114114
Result<std::string> LoadTableInternal(const TableIdentifier& identifier) const;
115115

116+
/// \brief Builds a per-table FileIO from the load response config and vended
117+
/// `storage-credentials`, or the catalog default FileIO when none apply.
118+
Result<std::shared_ptr<FileIO>> MakeFileIOForTable(
119+
const LoadTableResult& load_result) const;
120+
116121
Result<LoadTableResult> CreateTableInternal(
117122
const TableIdentifier& identifier, const std::shared_ptr<Schema>& schema,
118123
const std::shared_ptr<PartitionSpec>& spec, const std::shared_ptr<SortOrder>& order,

‎src/iceberg/catalog/rest/rest_file_io.cc‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,9 @@
2020
#include "iceberg/catalog/rest/rest_file_io.h"
2121

2222
#include <string>
23+
#include <vector>
2324

25+
#include "iceberg/catalog/rest/types.h"
2426
#include "iceberg/file_io_registry.h"
2527
#include "iceberg/util/macros.h"
2628

@@ -92,4 +94,16 @@ Result<std::unique_ptr<FileIO>> MakeCatalogFileIO(const RestCatalogProperties& c
9294
return FileIORegistry::Load(io_impl, config.configs());
9395
}
9496

97+
const StorageCredential* SelectS3StorageCredential(
98+
const std::vector<StorageCredential>& credentials) {
99+
const StorageCredential* best = nullptr;
100+
for (const auto& cred : credentials) {
101+
if (cred.prefix.starts_with("s3") &&
102+
(best == nullptr || cred.prefix.size() > best->prefix.size())) {
103+
best = &cred;
104+
}
105+
}
106+
return best;
107+
}
108+
95109
} // namespace iceberg::rest

‎src/iceberg/catalog/rest/rest_file_io.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,11 @@
2222
#include <cstdint>
2323
#include <memory>
2424
#include <string_view>
25+
#include <vector>
2526

2627
#include "iceberg/catalog/rest/catalog_properties.h"
2728
#include "iceberg/catalog/rest/iceberg_rest_export.h"
29+
#include "iceberg/catalog/rest/types.h"
2830
#include "iceberg/file_io.h"
2931
#include "iceberg/file_io_registry.h"
3032
#include "iceberg/result.h"
@@ -44,4 +46,9 @@ ICEBERG_REST_EXPORT std::string_view BuiltinFileIOName(BuiltinFileIOKind kind);
4446
ICEBERG_REST_EXPORT Result<std::unique_ptr<FileIO>> MakeCatalogFileIO(
4547
const RestCatalogProperties& config);
4648

49+
/// \brief Returns the longest-prefix S3-family vended credential (prefix
50+
/// starting with "s3"), or nullptr if none.
51+
ICEBERG_REST_EXPORT const StorageCredential* SelectS3StorageCredential(
52+
const std::vector<StorageCredential>& credentials);
53+
4754
} // namespace iceberg::rest

‎src/iceberg/test/rest_file_io_test.cc‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,12 @@
1919

2020
#include "iceberg/catalog/rest/rest_file_io.h"
2121

22+
#include <vector>
23+
2224
#include <gmock/gmock.h>
2325
#include <gtest/gtest.h>
2426

27+
#include "iceberg/catalog/rest/types.h"
2528
#include "iceberg/file_io_registry.h"
2629
#include "iceberg/test/matchers.h"
2730

@@ -143,4 +146,33 @@ TEST(RestFileIOTest, MakeCatalogFileIOSkipsCheckWhenWarehouseAbsent) {
143146
ASSERT_THAT(result, IsOk());
144147
}
145148

149+
TEST(RestFileIOTest, SelectS3StorageCredentialPicksLongestS3Prefix) {
150+
std::vector<StorageCredential> credentials = {
151+
{.prefix = "s3", .config = {{"s3.access-key-id", "a"}}},
152+
{.prefix = "s3://bucket/data", .config = {{"s3.access-key-id", "b"}}},
153+
{.prefix = "s3://bucket", .config = {{"s3.access-key-id", "c"}}},
154+
};
155+
const auto* cred = SelectS3StorageCredential(credentials);
156+
ASSERT_NE(cred, nullptr);
157+
EXPECT_EQ(cred->prefix, "s3://bucket/data");
158+
}
159+
160+
TEST(RestFileIOTest, SelectS3StorageCredentialIgnoresNonS3Prefixes) {
161+
std::vector<StorageCredential> credentials = {
162+
{.prefix = "gs://bucket", .config = {{"k", "v"}}},
163+
{.prefix = "s3", .config = {{"s3.access-key-id", "a"}}},
164+
};
165+
const auto* cred = SelectS3StorageCredential(credentials);
166+
ASSERT_NE(cred, nullptr);
167+
EXPECT_EQ(cred->prefix, "s3");
168+
}
169+
170+
TEST(RestFileIOTest, SelectS3StorageCredentialReturnsNullWhenNoneMatch) {
171+
std::vector<StorageCredential> credentials = {
172+
{.prefix = "gs://bucket", .config = {{"k", "v"}}},
173+
};
174+
EXPECT_EQ(SelectS3StorageCredential(credentials), nullptr);
175+
EXPECT_EQ(SelectS3StorageCredential({}), nullptr);
176+
}
177+
146178
} // namespace iceberg::rest

0 commit comments

Comments
 (0)