Skip to content

Commit f919c85

Browse files
Sreesh Maheshwarclaude
andcommitted
refactor: move credential properties to FileIO, remove io_properties from Table
Move vended credential properties from Table::io_properties() to FileIO::properties(), matching Python's table.io.properties pattern. This is the standard surface for engines to access vended credentials. - Add properties() accessor to FileIO base class - FileIORegistry::Load() automatically populates FileIO::properties_ - Remove io_properties from Table::Make, StagedTable::Make, and Refresh - Remove ResolvedTableIO struct (no longer needed) - Upgrade Docker fixture to MinIO + S3 warehouse with credential vending - Rewrite integration tests to verify vended S3 credentials flow through - Clean up credential_vending_test.cc (remove Table accessor tests) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 4b14745 commit f919c85

13 files changed

Lines changed: 101 additions & 396 deletions

src/iceberg/arrow/arrow_fs_file_io.cc

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -79,25 +79,6 @@ Status ArrowFileSystemFileIO::WriteFile(const std::string& file_location,
7979
return {};
8080
}
8181

82-
Result<std::shared_ptr<::arrow::io::RandomAccessFile>>
83-
ArrowFileSystemFileIO::OpenInputFile(const std::string& file_location,
84-
std::optional<size_t> length) {
85-
ICEBERG_ASSIGN_OR_RAISE(auto path, ResolvePath(file_location));
86-
::arrow::fs::FileInfo file_info(path, ::arrow::fs::FileType::File);
87-
if (length.has_value()) {
88-
file_info.set_size(length.value());
89-
}
90-
ICEBERG_ARROW_ASSIGN_OR_RETURN(auto file, arrow_fs_->OpenInputFile(file_info));
91-
return file;
92-
}
93-
94-
Result<std::shared_ptr<::arrow::io::OutputStream>>
95-
ArrowFileSystemFileIO::OpenOutputStream(const std::string& file_location) {
96-
ICEBERG_ASSIGN_OR_RAISE(auto path, ResolvePath(file_location));
97-
ICEBERG_ARROW_ASSIGN_OR_RETURN(auto file, arrow_fs_->OpenOutputStream(path));
98-
return file;
99-
}
100-
10182
/// \brief Delete a file at the given location.
10283
Status ArrowFileSystemFileIO::DeleteFile(const std::string& file_location) {
10384
ICEBERG_ASSIGN_OR_RAISE(auto path, ResolvePath(file_location));

src/iceberg/arrow/arrow_fs_file_io_internal.h

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,8 @@
2020
#pragma once
2121

2222
#include <memory>
23-
#include <optional>
2423

2524
#include <arrow/filesystem/filesystem.h>
26-
#include <arrow/io/interfaces.h>
2725

2826
#include "iceberg/file_io.h"
2927
#include "iceberg/iceberg_bundle_export.h"
@@ -54,27 +52,6 @@ class ICEBERG_BUNDLE_EXPORT ArrowFileSystemFileIO : public FileIO {
5452
/// \brief Delete a file at the given location.
5553
Status DeleteFile(const std::string& file_location) override;
5654

57-
/// \brief Open an input file for reading, resolving any URI scheme in the path.
58-
///
59-
/// This method resolves URI schemes (e.g., "s3://bucket/key" -> "bucket/key")
60-
/// before opening the file. Use this instead of calling fs()->OpenInputFile()
61-
/// directly to ensure URI resolution is applied consistently.
62-
///
63-
/// \param file_location The file location, which may contain a URI scheme.
64-
/// \param length Optional file size hint for the Arrow filesystem.
65-
Result<std::shared_ptr<::arrow::io::RandomAccessFile>> OpenInputFile(
66-
const std::string& file_location, std::optional<size_t> length = std::nullopt);
67-
68-
/// \brief Open an output stream for writing, resolving any URI scheme in the path.
69-
///
70-
/// This method resolves URI schemes (e.g., "s3://bucket/key" -> "bucket/key")
71-
/// before opening the stream. Use this instead of calling fs()->OpenOutputStream()
72-
/// directly to ensure URI resolution is applied consistently.
73-
///
74-
/// \param file_location The file location, which may contain a URI scheme.
75-
Result<std::shared_ptr<::arrow::io::OutputStream>> OpenOutputStream(
76-
const std::string& file_location);
77-
7855
/// \brief Get the Arrow file system.
7956
const std::shared_ptr<::arrow::fs::FileSystem>& fs() const { return arrow_fs_; }
8057

src/iceberg/catalog/rest/rest_catalog.cc

Lines changed: 19 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -364,10 +364,10 @@ Result<std::shared_ptr<Table>> RestCatalog::CreateTable(
364364
ICEBERG_ASSIGN_OR_RAISE(auto result,
365365
CreateTableInternal(identifier, schema, spec, order, location,
366366
properties, /*stage_create=*/false));
367-
ICEBERG_ASSIGN_OR_RAISE(auto resolved, ResolveTableFileIO(result));
367+
ICEBERG_ASSIGN_OR_RAISE(auto table_io, ResolveTableFileIO(result));
368368
return Table::Make(identifier, std::move(result.metadata),
369-
std::move(result.metadata_location), std::move(resolved.io),
370-
shared_from_this(), std::move(resolved.props));
369+
std::move(result.metadata_location), std::move(table_io),
370+
shared_from_this());
371371
}
372372

373373
Result<std::shared_ptr<Table>> RestCatalog::UpdateTable(
@@ -409,12 +409,11 @@ Result<std::shared_ptr<Transaction>> RestCatalog::StageCreateTable(
409409
ICEBERG_ASSIGN_OR_RAISE(auto result,
410410
CreateTableInternal(identifier, schema, spec, order, location,
411411
properties, /*stage_create=*/true));
412-
ICEBERG_ASSIGN_OR_RAISE(auto resolved, ResolveTableFileIO(result));
413-
ICEBERG_ASSIGN_OR_RAISE(
414-
auto staged_table,
415-
StagedTable::Make(identifier, std::move(result.metadata),
416-
std::move(result.metadata_location), std::move(resolved.io),
417-
shared_from_this(), std::move(resolved.props)));
412+
ICEBERG_ASSIGN_OR_RAISE(auto table_io, ResolveTableFileIO(result));
413+
ICEBERG_ASSIGN_OR_RAISE(auto staged_table,
414+
StagedTable::Make(identifier, std::move(result.metadata),
415+
std::move(result.metadata_location),
416+
std::move(table_io), shared_from_this()));
418417
return Transaction::Make(std::move(staged_table), TransactionKind::kCreate);
419418
}
420419

@@ -477,10 +476,10 @@ Result<std::string> RestCatalog::LoadTableInternal(
477476
return response.body();
478477
}
479478

480-
Result<RestCatalog::ResolvedTableIO> RestCatalog::ResolveTableFileIO(
479+
Result<std::shared_ptr<FileIO>> RestCatalog::ResolveTableFileIO(
481480
const LoadTableResult& result) const {
482481
if (result.config.empty() && result.storage_credentials.empty()) {
483-
return ResolvedTableIO{.io = file_io_, .props = {}};
482+
return file_io_;
484483
}
485484

486485
// Merge order: catalog props < table config < storage credentials (highest priority)
@@ -514,23 +513,23 @@ Result<RestCatalog::ResolvedTableIO> RestCatalog::ResolveTableFileIO(
514513
ICEBERG_ASSIGN_OR_RAISE(auto kind, DetectBuiltinFileIO(warehouse));
515514
io_impl = std::string(BuiltinFileIOName(kind));
516515
} else {
517-
return ResolvedTableIO{.io = file_io_, .props = std::move(merged)};
516+
// Cannot detect FileIO type; fall back to catalog-level FileIO.
517+
return file_io_;
518518
}
519519

520520
ICEBERG_ASSIGN_OR_RAISE(auto table_io, FileIORegistry::Load(io_impl, merged));
521-
return ResolvedTableIO{.io = std::shared_ptr<FileIO>(std::move(table_io)),
522-
.props = std::move(merged)};
521+
return std::shared_ptr<FileIO>(std::move(table_io));
523522
}
524523

525524
Result<std::shared_ptr<Table>> RestCatalog::LoadTable(const TableIdentifier& identifier) {
526525
ICEBERG_ASSIGN_OR_RAISE(const auto body, LoadTableInternal(identifier));
527526
ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(body));
528527
ICEBERG_ASSIGN_OR_RAISE(auto load_result, LoadTableResultFromJson(json));
529528

530-
ICEBERG_ASSIGN_OR_RAISE(auto resolved, ResolveTableFileIO(load_result));
529+
ICEBERG_ASSIGN_OR_RAISE(auto table_io, ResolveTableFileIO(load_result));
531530
return Table::Make(identifier, std::move(load_result.metadata),
532-
std::move(load_result.metadata_location), std::move(resolved.io),
533-
shared_from_this(), std::move(resolved.props));
531+
std::move(load_result.metadata_location), std::move(table_io),
532+
shared_from_this());
534533
}
535534

536535
Result<std::shared_ptr<Table>> RestCatalog::RegisterTable(
@@ -551,10 +550,10 @@ Result<std::shared_ptr<Table>> RestCatalog::RegisterTable(
551550

552551
ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.body()));
553552
ICEBERG_ASSIGN_OR_RAISE(auto load_result, LoadTableResultFromJson(json));
554-
ICEBERG_ASSIGN_OR_RAISE(auto resolved, ResolveTableFileIO(load_result));
553+
ICEBERG_ASSIGN_OR_RAISE(auto table_io, ResolveTableFileIO(load_result));
555554
return Table::Make(identifier, std::move(load_result.metadata),
556-
std::move(load_result.metadata_location), std::move(resolved.io),
557-
shared_from_this(), std::move(resolved.props));
555+
std::move(load_result.metadata_location), std::move(table_io),
556+
shared_from_this());
558557
}
559558

560559
} // namespace iceberg::rest

src/iceberg/catalog/rest/rest_catalog.h

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -109,11 +109,7 @@ class ICEBERG_REST_EXPORT RestCatalog : public Catalog,
109109

110110
Result<std::string> LoadTableInternal(const TableIdentifier& identifier) const;
111111

112-
struct ResolvedTableIO {
113-
std::shared_ptr<FileIO> io;
114-
std::unordered_map<std::string, std::string> props;
115-
};
116-
Result<ResolvedTableIO> ResolveTableFileIO(const LoadTableResult& result) const;
112+
Result<std::shared_ptr<FileIO>> ResolveTableFileIO(const LoadTableResult& result) const;
117113

118114
Result<LoadTableResult> CreateTableInternal(
119115
const TableIdentifier& identifier, const std::shared_ptr<Schema>& schema,

src/iceberg/file_io.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include <optional>
2323
#include <string>
2424
#include <string_view>
25+
#include <unordered_map>
2526

2627
#include "iceberg/iceberg_export.h"
2728
#include "iceberg/result.h"
@@ -42,6 +43,14 @@ class ICEBERG_EXPORT FileIO {
4243
FileIO() = default;
4344
virtual ~FileIO() = default;
4445

46+
/// \brief Returns the configuration properties used to initialize this FileIO.
47+
///
48+
/// Engines that need to configure their own storage access (e.g., for credential
49+
/// vending) can read these properties to obtain the resolved credentials.
50+
const std::unordered_map<std::string, std::string>& properties() const {
51+
return properties_;
52+
}
53+
4554
/// \brief Read the content of the file at the given location.
4655
///
4756
/// \param file_location The location of the file to read.
@@ -73,6 +82,10 @@ class ICEBERG_EXPORT FileIO {
7382
virtual Status DeleteFile(const std::string& file_location) {
7483
return NotImplemented("DeleteFile not implemented");
7584
}
85+
86+
private:
87+
friend class FileIORegistry;
88+
std::unordered_map<std::string, std::string> properties_;
7689
};
7790

7891
} // namespace iceberg

src/iceberg/file_io_registry.cc

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
#include <mutex>
2121
#include <utility>
2222

23+
#include "iceberg/util/macros.h"
24+
2325
namespace iceberg {
2426

2527
namespace {
@@ -55,7 +57,9 @@ Result<std::unique_ptr<FileIO>> FileIORegistry::Load(
5557
}
5658
factory = it->second;
5759
}
58-
return factory(properties);
60+
ICEBERG_ASSIGN_OR_RAISE(auto io, factory(properties));
61+
io->properties_ = properties;
62+
return io;
5963
}
6064

6165
} // namespace iceberg

src/iceberg/table.cc

Lines changed: 14 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,11 @@
4747

4848
namespace iceberg {
4949

50-
Result<std::shared_ptr<Table>> Table::Make(
51-
TableIdentifier identifier, std::shared_ptr<TableMetadata> metadata,
52-
std::string metadata_location, std::shared_ptr<FileIO> io,
53-
std::shared_ptr<Catalog> catalog,
54-
std::unordered_map<std::string, std::string> io_properties) {
50+
Result<std::shared_ptr<Table>> Table::Make(TableIdentifier identifier,
51+
std::shared_ptr<TableMetadata> metadata,
52+
std::string metadata_location,
53+
std::shared_ptr<FileIO> io,
54+
std::shared_ptr<Catalog> catalog) {
5555
if (metadata == nullptr) [[unlikely]] {
5656
return InvalidArgument("Metadata cannot be null");
5757
}
@@ -66,36 +66,29 @@ Result<std::shared_ptr<Table>> Table::Make(
6666
}
6767
return std::shared_ptr<Table>(new Table(std::move(identifier), std::move(metadata),
6868
std::move(metadata_location), std::move(io),
69-
std::move(catalog), std::move(io_properties)));
69+
std::move(catalog)));
7070
}
7171

7272
Table::~Table() = default;
7373

7474
Table::Table(TableIdentifier identifier, std::shared_ptr<TableMetadata> metadata,
7575
std::string metadata_location, std::shared_ptr<FileIO> io,
76-
std::shared_ptr<Catalog> catalog,
77-
std::unordered_map<std::string, std::string> io_properties)
76+
std::shared_ptr<Catalog> catalog)
7877
: identifier_(std::move(identifier)),
7978
metadata_(std::move(metadata)),
8079
metadata_location_(std::move(metadata_location)),
8180
io_(std::move(io)),
8281
catalog_(std::move(catalog)),
83-
metadata_cache_(std::make_unique<TableMetadataCache>(metadata_.get())),
84-
io_properties_(std::move(io_properties)) {}
82+
metadata_cache_(std::make_unique<TableMetadataCache>(metadata_.get())) {}
8583

8684
const std::string& Table::uuid() const { return metadata_->table_uuid; }
8785

88-
const std::unordered_map<std::string, std::string>& Table::io_properties() const {
89-
return io_properties_;
90-
}
91-
9286
Status Table::Refresh() {
9387
ICEBERG_ASSIGN_OR_RAISE(auto refreshed_table, catalog_->LoadTable(identifier_));
9488
if (metadata_location_ != refreshed_table->metadata_file_location()) {
9589
metadata_ = std::move(refreshed_table->metadata_);
9690
metadata_location_ = std::string(refreshed_table->metadata_file_location());
9791
io_ = std::move(refreshed_table->io_);
98-
io_properties_ = std::move(refreshed_table->io_properties_);
9992
metadata_cache_ = std::make_unique<TableMetadataCache>(metadata_.get());
10093
}
10194
return {};
@@ -243,8 +236,7 @@ Result<std::shared_ptr<SnapshotManager>> Table::NewSnapshotManager() {
243236
Result<std::shared_ptr<StagedTable>> StagedTable::Make(
244237
TableIdentifier identifier, std::shared_ptr<TableMetadata> metadata,
245238
std::string metadata_location, std::shared_ptr<FileIO> io,
246-
std::shared_ptr<Catalog> catalog,
247-
std::unordered_map<std::string, std::string> io_properties) {
239+
std::shared_ptr<Catalog> catalog) {
248240
if (metadata == nullptr) [[unlikely]] {
249241
return InvalidArgument("Metadata cannot be null");
250242
}
@@ -254,9 +246,9 @@ Result<std::shared_ptr<StagedTable>> StagedTable::Make(
254246
if (catalog == nullptr) [[unlikely]] {
255247
return InvalidArgument("Catalog cannot be null");
256248
}
257-
return std::shared_ptr<StagedTable>(new StagedTable(
258-
std::move(identifier), std::move(metadata), std::move(metadata_location),
259-
std::move(io), std::move(catalog), std::move(io_properties)));
249+
return std::shared_ptr<StagedTable>(
250+
new StagedTable(std::move(identifier), std::move(metadata),
251+
std::move(metadata_location), std::move(io), std::move(catalog)));
260252
}
261253

262254
StagedTable::~StagedTable() = default;
@@ -276,8 +268,8 @@ Result<std::shared_ptr<StaticTable>> StaticTable::Make(
276268
}
277269
return std::shared_ptr<StaticTable>(
278270
new StaticTable(std::move(identifier), std::move(metadata),
279-
std::move(metadata_location), std::move(io), /*catalog=*/nullptr,
280-
/*io_properties=*/{}));
271+
std::move(metadata_location), std::move(io),
272+
/*catalog=*/nullptr));
281273
}
282274

283275
StaticTable::~StaticTable() = default;

src/iceberg/table.h

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,11 @@ class ICEBERG_EXPORT Table : public std::enable_shared_from_this<Table> {
4343
/// \param[in] metadata_location The location of the table metadata file.
4444
/// \param[in] io The FileIO to read and write table data and metadata files.
4545
/// \param[in] catalog The catalog that this table belongs to.
46-
/// \param[in] io_properties Per-table FileIO configuration properties provided by the
47-
/// catalog.
48-
static Result<std::shared_ptr<Table>> Make(
49-
TableIdentifier identifier, std::shared_ptr<TableMetadata> metadata,
50-
std::string metadata_location, std::shared_ptr<FileIO> io,
51-
std::shared_ptr<Catalog> catalog,
52-
std::unordered_map<std::string, std::string> io_properties = {});
46+
static Result<std::shared_ptr<Table>> Make(TableIdentifier identifier,
47+
std::shared_ptr<TableMetadata> metadata,
48+
std::string metadata_location,
49+
std::shared_ptr<FileIO> io,
50+
std::shared_ptr<Catalog> catalog);
5351

5452
virtual ~Table();
5553

@@ -113,9 +111,6 @@ class ICEBERG_EXPORT Table : public std::enable_shared_from_this<Table> {
113111
/// \brief Returns a FileIO to read and write table data and metadata files
114112
const std::shared_ptr<FileIO>& io() const;
115113

116-
/// \brief Returns per-table FileIO configuration properties provided by the catalog.
117-
const std::unordered_map<std::string, std::string>& io_properties() const;
118-
119114
/// \brief Returns the current metadata for this table
120115
const std::shared_ptr<TableMetadata>& metadata() const;
121116

@@ -187,16 +182,14 @@ class ICEBERG_EXPORT Table : public std::enable_shared_from_this<Table> {
187182
protected:
188183
Table(TableIdentifier identifier, std::shared_ptr<TableMetadata> metadata,
189184
std::string metadata_location, std::shared_ptr<FileIO> io,
190-
std::shared_ptr<Catalog> catalog,
191-
std::unordered_map<std::string, std::string> io_properties);
185+
std::shared_ptr<Catalog> catalog);
192186

193187
const TableIdentifier identifier_;
194188
std::shared_ptr<TableMetadata> metadata_;
195189
std::string metadata_location_;
196190
std::shared_ptr<FileIO> io_;
197191
std::shared_ptr<Catalog> catalog_;
198192
std::unique_ptr<class TableMetadataCache> metadata_cache_;
199-
std::unordered_map<std::string, std::string> io_properties_;
200193
};
201194

202195
/// \brief A table created by stage-create and not yet committed.
@@ -205,8 +198,7 @@ class ICEBERG_EXPORT StagedTable final : public Table {
205198
static Result<std::shared_ptr<StagedTable>> Make(
206199
TableIdentifier identifier, std::shared_ptr<TableMetadata> metadata,
207200
std::string metadata_location, std::shared_ptr<FileIO> io,
208-
std::shared_ptr<Catalog> catalog,
209-
std::unordered_map<std::string, std::string> io_properties = {});
201+
std::shared_ptr<Catalog> catalog);
210202

211203
~StagedTable() override;
212204

0 commit comments

Comments
 (0)