Skip to content

Commit c1216fa

Browse files
feat(rest): parse storage-credentials from LoadTableResponse
1 parent d3b02bb commit c1216fa

6 files changed

Lines changed: 109 additions & 7 deletions

File tree

src/iceberg/catalog/rest/json_serde.cc

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ constexpr std::string_view kSource = "source";
7171
constexpr std::string_view kDestination = "destination";
7272
constexpr std::string_view kMetadata = "metadata";
7373
constexpr std::string_view kConfig = "config";
74+
constexpr std::string_view kStorageCredentials = "storage-credentials";
75+
constexpr std::string_view kPrefix = "prefix";
7476
constexpr std::string_view kIdentifiers = "identifiers";
7577
constexpr std::string_view kOverrides = "overrides";
7678
constexpr std::string_view kDefaults = "defaults";
@@ -695,6 +697,16 @@ nlohmann::json ToJson(const LoadTableResult& result) {
695697
SetOptionalStringField(json, kMetadataLocation, result.metadata_location);
696698
json[kMetadata] = ToJson(*result.metadata);
697699
SetContainerField(json, kConfig, result.config);
700+
if (!result.storage_credentials.empty()) {
701+
nlohmann::json creds = nlohmann::json::array();
702+
for (const auto& cred : result.storage_credentials) {
703+
nlohmann::json entry;
704+
entry[kPrefix] = cred.prefix;
705+
SetContainerField(entry, kConfig, cred.config);
706+
creds.push_back(std::move(entry));
707+
}
708+
json[kStorageCredentials] = std::move(creds);
709+
}
698710
return json;
699711
}
700712

@@ -707,6 +719,27 @@ Result<LoadTableResult> LoadTableResultFromJson(const nlohmann::json& json) {
707719
ICEBERG_ASSIGN_OR_RAISE(result.metadata, TableMetadataFromJson(metadata_json));
708720
ICEBERG_ASSIGN_OR_RAISE(result.config,
709721
GetJsonValueOrDefault<decltype(result.config)>(json, kConfig));
722+
if (auto it = json.find(kStorageCredentials); it != json.end() && !it->is_null()) {
723+
if (!it->is_array()) {
724+
return JsonParseError("Cannot parse storage credentials from non-array: {}",
725+
SafeDumpJson(*it));
726+
}
727+
for (const auto& entry : *it) {
728+
StorageCredential cred;
729+
ICEBERG_ASSIGN_OR_RAISE(cred.prefix, GetJsonValue<std::string>(entry, kPrefix));
730+
ICEBERG_ASSIGN_OR_RAISE(cred.config,
731+
GetJsonValue<decltype(cred.config)>(entry, kConfig));
732+
// prefix and config are required by the REST spec; non-empty matches the
733+
// Java reference implementation (Credential.validate()).
734+
if (cred.prefix.empty()) {
735+
return JsonParseError("Invalid storage credential: prefix must be non-empty");
736+
}
737+
if (cred.config.empty()) {
738+
return JsonParseError("Invalid storage credential: config must be non-empty");
739+
}
740+
result.storage_credentials.push_back(std::move(cred));
741+
}
742+
}
710743
ICEBERG_RETURN_UNEXPECTED(result.Validate());
711744
return result;
712745
}

src/iceberg/catalog/rest/rest_catalog.cc

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -479,10 +479,16 @@ Result<std::string> RestCatalog::LoadTableInternal(
479479
return response.body();
480480
}
481481

482-
Result<std::shared_ptr<Table>> RestCatalog::LoadTable(const TableIdentifier& identifier) {
482+
Result<LoadTableResult> RestCatalog::LoadTableResultFor(
483+
const TableIdentifier& identifier) {
483484
ICEBERG_ASSIGN_OR_RAISE(const auto body, LoadTableInternal(identifier));
484485
ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(body));
485-
ICEBERG_ASSIGN_OR_RAISE(auto load_result, LoadTableResultFromJson(json));
486+
return LoadTableResultFromJson(json);
487+
}
488+
489+
Result<std::shared_ptr<Table>> RestCatalog::LoadTable(const TableIdentifier& identifier) {
490+
ICEBERG_ASSIGN_OR_RAISE(auto load_result, LoadTableResultFor(identifier));
491+
// Binds the catalog's default FileIO; use LoadTableResultFor() for vended creds.
486492
// TODO: Support table-specific auth config and per-table FileIO from the REST
487493
// load response when table-scoped REST operations are introduced.
488494
return Table::Make(identifier, std::move(load_result.metadata),

src/iceberg/catalog/rest/rest_catalog.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,10 @@ class ICEBERG_REST_EXPORT RestCatalog : public Catalog,
9595

9696
Result<std::shared_ptr<Table>> LoadTable(const TableIdentifier& identifier) override;
9797

98+
/// \brief Like LoadTable() but returns the raw LoadTableResult with any vended
99+
/// `storage-credentials`, without binding a FileIO.
100+
Result<LoadTableResult> LoadTableResultFor(const TableIdentifier& identifier);
101+
98102
Result<std::shared_ptr<Table>> RegisterTable(
99103
const TableIdentifier& identifier,
100104
const std::string& metadata_file_location) override;

src/iceberg/catalog/rest/types.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,8 @@ bool CreateTableRequest::operator==(const CreateTableRequest& other) const {
8686
}
8787

8888
bool LoadTableResult::operator==(const LoadTableResult& other) const {
89-
if (metadata_location != other.metadata_location || config != other.config) {
89+
if (metadata_location != other.metadata_location || config != other.config ||
90+
storage_credentials != other.storage_credentials) {
9091
return false;
9192
}
9293

src/iceberg/catalog/rest/types.h

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,12 +180,23 @@ struct ICEBERG_REST_EXPORT CreateTableRequest {
180180
/// \brief An opaque token that allows clients to make use of pagination for list APIs.
181181
using PageToken = std::string;
182182

183+
/// \brief A short-lived credential vended by a REST catalog for a storage
184+
/// location ``prefix`` (clients pick the longest matching prefix); ``config``
185+
/// holds backend properties such as ``"s3.access-key-id"`` (Iceberg REST spec).
186+
struct ICEBERG_REST_EXPORT StorageCredential {
187+
std::string prefix;
188+
std::unordered_map<std::string, std::string> config;
189+
190+
bool operator==(const StorageCredential& other) const = default;
191+
};
192+
183193
/// \brief Result body for table create/load/register APIs.
184194
struct ICEBERG_REST_EXPORT LoadTableResult {
185195
std::string metadata_location;
186196
std::shared_ptr<TableMetadata> metadata; // required
187197
std::unordered_map<std::string, std::string> config;
188-
// TODO(Li Feiyang): Add std::shared_ptr<StorageCredential> storage_credential;
198+
/// \brief Vended storage credentials, one per URI prefix; empty if none.
199+
std::vector<StorageCredential> storage_credentials;
189200

190201
/// \brief Validates the LoadTableResult.
191202
Status Validate() const {

src/iceberg/test/rest_json_serde_test.cc

Lines changed: 50 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,11 @@ static std::shared_ptr<TableMetadata> MakeSimpleTableMetadata() {
8282
});
8383
}
8484

85+
std::string LoadTableJsonWithCredentials(std::string_view storage_credentials) {
86+
return std::string(R"({"storage-credentials":)") + std::string(storage_credentials) +
87+
R"(,"metadata":{"format-version":2,"table-uuid":"test","location":"s3://test","last-sequence-number":0,"last-column-id":1,"last-updated-ms":0,"schemas":[{"type":"struct","schema-id":1,"fields":[{"id":1,"name":"id","type":"int","required":true}]}],"current-schema-id":1,"partition-specs":[{"spec-id":0,"fields":[]}],"default-spec-id":0,"last-partition-id":0,"sort-orders":[{"order-id":0,"fields":[]}],"default-sort-order-id":0}})";
88+
}
89+
8590
// Test parameter structure for roundtrip tests
8691
template <typename Model>
8792
struct JsonRoundTripParam {
@@ -1116,7 +1121,17 @@ INSTANTIATE_TEST_SUITE_P(
11161121
.model = {.metadata_location = "s3://bucket/metadata/v1.json",
11171122
.metadata = MakeSimpleTableMetadata(),
11181123
.config = {{"warehouse", "s3://bucket/warehouse"},
1119-
{"foo", "bar"}}}}),
1124+
{"foo", "bar"}}}},
1125+
LoadTableResultParam{
1126+
.test_name = "WithStorageCredentials",
1127+
.expected_json_str =
1128+
R"({"metadata":{"current-schema-id":1,"current-snapshot-id":null,"default-sort-order-id":0,"default-spec-id":0,"format-version":2,"last-column-id":1,"last-partition-id":0,"last-sequence-number":0,"last-updated-ms":0,"location":"s3://bucket/test","metadata-log":[],"partition-specs":[{"fields":[],"spec-id":0}],"partition-statistics":[],"properties":{},"refs":{},"schemas":[{"fields":[{"id":1,"name":"id","required":true,"type":"int"}],"schema-id":1,"type":"struct"}],"snapshot-log":[],"snapshots":[],"sort-orders":[{"fields":[],"order-id":0}],"statistics":[],"table-uuid":"test-uuid-1234"},"storage-credentials":[{"config":{"s3.access-key-id":"AKIAtest","s3.region":"us-east-1","s3.secret-access-key":"secret"},"prefix":"s3"}]})",
1129+
.model =
1130+
{.metadata = MakeSimpleTableMetadata(),
1131+
.storage_credentials = {{.prefix = "s3",
1132+
.config = {{"s3.access-key-id", "AKIAtest"},
1133+
{"s3.secret-access-key", "secret"},
1134+
{"s3.region", "us-east-1"}}}}}}),
11201135
[](const ::testing::TestParamInfo<LoadTableResultParam>& info) {
11211136
return info.param.test_name;
11221137
});
@@ -1145,7 +1160,18 @@ INSTANTIATE_TEST_SUITE_P(
11451160
.json_str =
11461161
R"({"metadata":{"format-version":2,"table-uuid":"test-uuid-1234","location":"s3://bucket/test","last-sequence-number":0,"last-updated-ms":0,"last-column-id":1,"schemas":[{"type":"struct","schema-id":1,"fields":[{"id":1,"name":"id","type":"int","required":true}]}],"current-schema-id":1,"partition-specs":[{"spec-id":0,"fields":[]}],"default-spec-id":0,"last-partition-id":0,"sort-orders":[{"order-id":0,"fields":[]}],"default-sort-order-id":0,"properties":{}},"config":{"warehouse":"s3://bucket/warehouse"}})",
11471162
.expected_model = {.metadata = MakeSimpleTableMetadata(),
1148-
.config = {{"warehouse", "s3://bucket/warehouse"}}}}),
1163+
.config = {{"warehouse", "s3://bucket/warehouse"}}}},
1164+
LoadTableResultDeserializeParam{
1165+
.test_name = "WithStorageCredentials",
1166+
.json_str =
1167+
R"({"metadata":{"format-version":2,"table-uuid":"test-uuid-1234","location":"s3://bucket/test","last-sequence-number":0,"last-updated-ms":0,"last-column-id":1,"schemas":[{"type":"struct","schema-id":1,"fields":[{"id":1,"name":"id","type":"int","required":true}]}],"current-schema-id":1,"partition-specs":[{"spec-id":0,"fields":[]}],"default-spec-id":0,"last-partition-id":0,"sort-orders":[{"order-id":0,"fields":[]}],"default-sort-order-id":0,"properties":{}},"storage-credentials":[{"prefix":"s3","config":{"s3.access-key-id":"AKIAtest","s3.secret-access-key":"secret","s3.session-token":"token","s3.region":"us-east-1"}}]})",
1168+
.expected_model =
1169+
{.metadata = MakeSimpleTableMetadata(),
1170+
.storage_credentials = {{.prefix = "s3",
1171+
.config = {{"s3.access-key-id", "AKIAtest"},
1172+
{"s3.secret-access-key", "secret"},
1173+
{"s3.session-token", "token"},
1174+
{"s3.region", "us-east-1"}}}}}}),
11491175
[](const ::testing::TestParamInfo<LoadTableResultDeserializeParam>& info) {
11501176
return info.param.test_name;
11511177
});
@@ -1184,7 +1210,28 @@ INSTANTIATE_TEST_SUITE_P(
11841210
LoadTableResultInvalidParam{
11851211
.test_name = "InvalidMetadataContent",
11861212
.invalid_json_str = R"({"metadata":{"format-version":"invalid"}})",
1187-
.expected_error_message = "type must be number, but is string"}),
1213+
.expected_error_message = "type must be number, but is string"},
1214+
LoadTableResultInvalidParam{
1215+
.test_name = "StorageCredentialsNotArray",
1216+
.invalid_json_str = LoadTableJsonWithCredentials(R"("oops")"),
1217+
.expected_error_message = "Cannot parse storage credentials from non-array"},
1218+
LoadTableResultInvalidParam{
1219+
.test_name = "StorageCredentialMissingPrefix",
1220+
.invalid_json_str = LoadTableJsonWithCredentials(R"([{"config":{"k":"v"}}])"),
1221+
.expected_error_message = "Missing 'prefix'"},
1222+
LoadTableResultInvalidParam{
1223+
.test_name = "StorageCredentialMissingConfig",
1224+
.invalid_json_str = LoadTableJsonWithCredentials(R"([{"prefix":"s3"}])"),
1225+
.expected_error_message = "Missing 'config'"},
1226+
LoadTableResultInvalidParam{.test_name = "StorageCredentialEmptyPrefix",
1227+
.invalid_json_str = LoadTableJsonWithCredentials(
1228+
R"([{"prefix":"","config":{"k":"v"}}])"),
1229+
.expected_error_message = "prefix must be non-empty"},
1230+
LoadTableResultInvalidParam{
1231+
.test_name = "StorageCredentialEmptyConfig",
1232+
.invalid_json_str =
1233+
LoadTableJsonWithCredentials(R"([{"prefix":"s3","config":{}}])"),
1234+
.expected_error_message = "config must be non-empty"}),
11881235
[](const ::testing::TestParamInfo<LoadTableResultInvalidParam>& info) {
11891236
return info.param.test_name;
11901237
});

0 commit comments

Comments
 (0)