Skip to content

Iceberg REST Phase 1 [2/3]: Runtime facade and unit coverage - #499

Closed
cbb330 wants to merge 2 commits into
linkedin:chbush/iceberg-rest-1-codegenfrom
cbb330:chbush/iceberg-rest-2-controller
Closed

cbb330 wants to merge 2 commits into
linkedin:chbush/iceberg-rest-1-codegenfrom
cbb330:chbush/iceberg-rest-2-controller

Conversation

@cbb330

@cbb330 cbb330 commented Mar 11, 2026

Copy link
Copy Markdown
Collaborator

Summary

Part 2 of 3 for a read-only Iceberg REST Catalog facade.

This PR implements the generated contract as a thin Spring MVC adapter. Protocol translation lives in IcebergRestApiHandler; existing OpenHouse table behavior is reused, not rewritten. The facade is disabled by default.

Changes

  • Client-facing API Changes
  • Internal API Changes
  • Bug Fixes
  • New Features
  • Performance Improvements
  • Code Style
  • Refactoring
  • Documentation
  • Tests

Client-facing API Changes

When cluster.tables.iceberg-rest.enabled=true:

  • GET /v1/config returns prefix iceberg and advertises only the implemented endpoints.
  • GET /v1/iceberg/namespaces/{namespace}/tables lists tables with opaque pagination.
  • GET /v1/iceberg/namespaces/{namespace}/tables/{table} loads Iceberg table metadata.
  • HEAD /v1/iceberg/namespaces/{namespace}/tables/{table} checks existence.

Existing OpenHouse /v1/databases/... APIs are unchanged. Unsupported Iceberg REST operations are not generated or advertised.

Internal API Changes

  • IcebergRestCatalogController implements the generated interfaces and delegates to IcebergRestApiHandler.
  • OpenHouseIcebergRestApiHandler reuses TablesApiHandler for authorization/validation and OpenHouseInternalCatalog for Iceberg LoadTableResponse.
  • TablesService is unchanged. The earlier principal-aware list overload is removed; listing now uses the existing paginated handler path.
  • Iceberg JSON is serialized by a write-only converter that does not affect OpenHouse JSON.
  • Controller-scoped advice returns Iceberg ErrorResponse envelopes and sanitizes 403/500 messages.

New Features

  • Feature flag cluster.tables.iceberg-rest.enabled (default false), also on ClusterProperties.
  • Local docker-compose enables the flag for smoke testing.
  • Capabilities advertisement, prefix validation, single-level namespace decoding, opaque page tokens, and explicit rejection of snapshots=refs.

Refactoring

  • Moves protocol logic out of the controller so the generated interface remains a transport adapter.

Documentation

  • Adds docs/iceberg-rest-catalog.md covering enablement, architecture, limitations, observability, and spec maintenance.

Tests

  • Controller tests cover typed responses, sanitized 403s, and delegation.
  • Handler tests cover capabilities, pagination tokens, prefix/page-token validation, snapshot projection rejection, and reuse of the existing read handler.
  • Feature-flag tests confirm the controller is absent by default and present when enabled.

Testing Done

  • Manually Tested on local docker setup. Please include commands ran, and their output.
  • Added new tests for the changes made.
  • Updated existing tests to reflect the changes made.
  • No tests added or updated. Please explain why. If unsure, please feel free to ask for help.
  • Some other form of testing like staging or soak time in production. Please explain.

Validated locally with JDK 17:

./gradlew :services:tables:check

That includes the new controller, handler, and feature-flag tests. Client round-trips and the PyIceberg CI smoke test are in #500.

Review this PR's two runtime commits after #498. Because the GitHub files tab is against main, it also includes #498.

Additional Information

  • Breaking Changes
  • Deprecations
  • Large PR broken into smaller PRs, and PR plan linked in the description.

Merge in order:

  1. Iceberg REST Phase 1 [1/3]: Read-only contract and codegen #498 — Read-only contract and codegen
  2. Iceberg REST Phase 1 [2/3]: Runtime facade and unit coverage #499 — Runtime facade and unit coverage ← you are here
  3. Iceberg REST Phase 1 [3/3]: Client and integration validation #500 — Client and integration validation

cbb330 added 2 commits August 20, 2026 22:27
Implement the read-only Iceberg REST Catalog surface on top of the
generated OpenAPI interfaces from PR1:

Controller (IcebergRestCatalogController):
- Implements CatalogApiApi + ConfigurationApiApi generated interfaces
- GET /v1/config returns prefix override for route isolation
- GET /v1/{prefix}/namespaces/{ns}/tables lists tables via TablesService
- GET /v1/{prefix}/namespaces/{ns}/tables/{t} loads table via CatalogHandlers
- HEAD /v1/{prefix}/namespaces/{ns}/tables/{t} checks table existence
- All unimplemented endpoints return 501 via generated defaults

Serialization:
- IcebergRestHttpMessageConverter for Iceberg REST types (kebab-case JSON)
- IcebergRestSerde with Iceberg RESTSerializers + kebab-case ObjectMapper
- IcebergRestSerdeConfig registers converter without affecting existing Jackson

Error handling:
- IcebergRestExceptionHandler scoped to controller, returns Iceberg ErrorResponse

Service:
- TablesService.searchTables(databaseId, actingPrincipal) overload for auth
Keep the generated controller transport-only while delegating through a protocol handler that preserves existing table behavior, errors, auditing, and rollout controls.
@cbb330
cbb330 force-pushed the chbush/iceberg-rest-2-controller branch from c2161a8 to 1872e5d Compare August 21, 2026 05:30
@cbb330 cbb330 changed the title Iceberg REST Phase 1 [2/3]: Controller, serde, and exception handling Iceberg REST Phase 1 [2/3]: Runtime facade and unit coverage Aug 21, 2026
@cbb330
cbb330 changed the base branch from main to chbush/iceberg-rest-1-codegen August 21, 2026 05:44
@cbb330

cbb330 commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator Author

Superseded by the same-repo GitHub stack (fork heads cannot be added to GitHub stacks):

  1. Iceberg REST Phase 1 [1/3]: Read-only contract and codegen #689
  2. Iceberg REST Phase 1 [2/3]: Runtime facade and unit coverage #690
  3. Iceberg REST Phase 1 [3/3]: Client and integration validation #691

Closing in favor of that stack.

@cbb330

cbb330 commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator Author

Closing in favor of stacked PRs #689 / #690 / #691.

@cbb330 cbb330 closed this Aug 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant