Conversation
Adds spec/polaris-shares-api.yaml, a draft OpenAPI specification for a sharing control plane, and registers it in spec/README.md. It is specification only: no server code and no generated resources; the whole API is marked beta per the evolution guidelines and is meant to be served only behind a feature flag that defaults to off. A share is an explicit, named container enumerating shared objects, bound to exactly one catalog in this version; an external consumer is a restricted, first-class identity for a party outside the catalog's identity domain; a listing is the only binding between a consumer and a share and the unit of audit; a listing's endpoint configuration is the connection material a consumer pastes into any standard Iceberg REST client; consumer credentials are OAuth2 client-credential pairs with issue, list, revoke and rotate operations. Consumers never call this API: they read shared tables through the Iceberg REST Catalog protocol unchanged, one prefix per share, read-only by construction. Share management mounts under the existing management root (/api/management/v1/shares and /api/management/v1/external-consumers), beside catalogs, principals and roles, following the split Polaris already has between its management root and its protocol root. The consumer-facing Iceberg REST root is not part of this file; it is named only through the endpoint configuration. Open questions for reviewers are recorded in the specification's own descriptions: the one-catalog rule is a rule of this version rather than of the addressing; member updates compare-and-set on the share's version; member identities are pinned to stable ids; the exposed identity is reserved and must equal the canonical one; credential rotation keeps the previous credential valid for a service-defined overlap window; the token endpoint is deployment-defined.
| operationId: listShares | ||
| summary: List shares | ||
| parameters: | ||
| - name: pageToken |
There was a problem hiding this comment.
This introduces pageToken/pageSize pagination on list endpoints. I think it's great, but it's a big surprising to have it as the existing management API (listCatalogs, listPrincipals, ...) has no pagination at all.
I'm fully support this, it's great, just a note for reviewer comparing to other endpoints.
There was a problem hiding this comment.
Yeah good callout, mostly had it here just due to symmetry with the other operations in this same management surface. In practice usually the count of Shares shouldn't be too high to need pagination, and implementation-wise maybe we'd defer pagination support to a later phase. But I suppose nowadays the persistence-layer pagination should be generalized enough that we could also go the other way and just bring all APIs up to the standard of having uniform pagination mechanics.
| not support | ||
| canonical: | ||
| $ref: "#/components/schemas/MemberIdentity" | ||
| exposed: |
There was a problem hiding this comment.
Since a service must reject any exposed value that differs from canonical, this field has no observable effect yet. Since it's optional, maybe we could add this later (without breaking change) with the purpose to keep things simple for the initial version.
There was a problem hiding this comment.
Good idea; this was some plumbing boilerplate that came from experimenting with the aliasing we discussed in the doc, but actually for v1 it's probably best to scope down to remove any premature hooks for the aliasing that would complicate the code with unused scaffolding at this phase.
| "**Beta:** this API is under active development and may change in a | ||
| backward-incompatible way. Create an OAuth2 client-credentials pair for this | ||
| consumer. The client secret is returned exactly once, in this response, and | ||
| cannot be retrieved later. A service MAY limit a consumer to one active |
There was a problem hiding this comment.
I believe this silent-replace behavior is a bit ambiguous for users: a client calling POST .../credentials has no way to know in advance whether it got a second credential or a replacement. Maybe we should clarify this in this comment: distinct response code/field indicating "replaced" vs "created", or an idempotency-style flag the user passes to opt in to replacement?
There was a problem hiding this comment.
Yeah I guess I was just letting the can get kicked down the road for now :)
But I agree we could be more explicit. The idea of silent-invalidation has some precedent; for example minting a new OAuth refresh token on Google says: https://developers.google.com/identity/protocols/oauth2#expiration
There is currently a limit of 100 refresh tokens per Google Account per OAuth 2.0 client ID.
If the limit is reached, creating a new refresh token automatically invalidates the oldest refresh token without warning.
This limit does not apply to service accounts.
I guess it's arguable whether it's a better user experience to hard-block once a limit is hit and require a revokeExternalConsumerCredential to manually revoke an old one vs letting the oldest one drop off automatically.
The creation flow might not be the same persona who knows about where older credentials were distributed to be able to re-distribute them.
I guess right now there's kind of a two-layered overlap of intent between creating multiple credentials per ExternalConsumer vs being able to rotateCredentials for a single credential-id of a given ExternalConsumer.
If our goal with having multiple credentials per ExternalConsumer is for ease-of-use to have one logical "partner organization" be the single "ExternalConsumer" but to mint a set of distinct credentials for them to distribute to subteams, then maybe it's best to remove the A service MAY limit a consumer to one active credential... statement and instead say a service may impose a max number of credentials per consumer, but lifecycle of each credential should be manually managed.
| 404: | ||
| description: "The share or the external consumer does not exist" | ||
| 409: | ||
| description: "A listing with the specified name already exists on this share, or a listing already binds this consumer to this share (at most one listing binds a given consumer to a given share)" |
There was a problem hiding this comment.
This 409 collapses two different conflict conditions (name collision vs consumer-already-listed) into one response with no way to distinguish them programmatically, only via the free-text description.
Maybe worth considering whether these should be distinguishable (via an error code/body, or a different status), since an user retrying with a new name won't help in the second case.
There was a problem hiding this comment.
Yeah good point. I guess expanding on the details for your suggestions, the possibilities are:
- Have body content on 409 for heavyweight error-content defs like iceberg-rest - but these can sometimes be annoying by causing JSON parse exceptions in e.g. pyspark when something returns an empty response body, and then we might want to re-work all the Polaris error-responses to return error-response bodies. TBH I didn't think about this too much before, but it looks like there's already inconsistency between policy service returning the structured error responses vs the basic management-service just returning empty responses
- Handle it the way we do grants/assignRole, where IIRC we just let it be an idempotent no-op if you already have the "equivalent" grant/listing (return 200)
- Return something like 412 Precondition Failed for already-existing listing - the idea being semantically we "imply" an expectation that the same logical listing-mapping doesn't already exist (by any name), so that "condition" is violated
| properties: | ||
| catalog: | ||
| type: string | ||
| description: |
There was a problem hiding this comment.
If this field is constrained to always equal the share's bound catalog when present, what does it add over omitting it?
Since Share.catalogName already pins the catalog, this reads as a field that can only ever hold one legal value (or be absent). I believe it could be dropped from MemberIdentity now and reintroduced if/when a share can span multiple catalogs (my understanding is that "one-catalog-per-share" is the approach here, and it makes perfect sense to me, I'm not convince by the multi-catalogs scenario).
There was a problem hiding this comment.
Same as the "exposed/canonical" stuff above, I agree we should just remove it for now - it's basically just to "prove" the structure of the API can cleanly accommodate the discussed ambitious plans for aliasing/cross-catalog, but we can trim it down a lot more tightly for v1.
| version: 0.0.3 | ||
| description: | ||
| Defines the management APIs for sharing catalog content with consumers outside the | ||
| catalog's identity domain. A share enumerates what is shared; an external consumer is |
There was a problem hiding this comment.
I'm not sure what the outside the catalog's identity domain sentence means 🤔 Shouldn't Polaris always assert the client's identity and enforce access controls?
| - warehouse | ||
| - oauth2ServerUri | ||
|
|
||
| ConsumerCredential: |
There was a problem hiding this comment.
I'm not sure we had consensus about "external consumers" identification in earlier discussions.
I believe this should connect with the existing Polaris Authentication frameworks.
clientId / secret maps well to Polaris local principals, but does not map to external IdPs.
I do not think shares should be limited to internal Polaris principals (consumers).
@adutra : WDYT?
| openapi: 3.0.3 | ||
| info: | ||
| title: Polaris Shares API | ||
| version: 0.0.3 |
| state and may change in a backward-incompatible way; it is served only when the | ||
| shares feature flag is enabled, and it is disabled by default. | ||
| servers: | ||
| - url: "{scheme}://{host}/api/management/v1" |
There was a problem hiding this comment.
api/management/v1 is used by polaris-management-service.yml. I think this spec, being a new API, should use a distinct prefix.
| schema: | ||
| type: integer | ||
| description: | ||
| "**Beta:** this API is under active development and may change in a |
There was a problem hiding this comment.
nit: It should be sufficient to flag "beta" in the top-level description, I guess 🤔
| type: object | ||
| properties: | ||
| currentEntityVersion: | ||
| type: integer |
There was a problem hiding this comment.
Do we have to force the int type? From the client's perspective this is just an opaque unique ID without ordering connotations.
Servers should be able to represent integers as a string without much overhead.
| The catalog containing the object; optional, and when present it must equal | ||
| the share's bound catalog | ||
| namespace: | ||
| type: array |
There was a problem hiding this comment.
How will client actually use this name to access the member? Why API? (apologies if I missed it),
| type: string | ||
| createTimestamp: | ||
| type: integer | ||
| format: int64 |
| format: int64 | ||
| entityVersion: | ||
| type: integer | ||
| description: The version of the share object used to determine if the share metadata has changed |
There was a problem hiding this comment.
used to determine if the share metadata has changed - I assume the intention is to support optimistic locking / avoid concurrent update overwrites... Could you clarify the description in that sense?
| description: Updates to apply to a share's description or properties | ||
| type: object | ||
| properties: | ||
| currentEntityVersion: |
There was a problem hiding this comment.
"Current" in what sense / at what moment? I believe it something like "base" entity version, meaning the version on which the update is based.... or perhaps "reference" entity version.
| description: | ||
| The share version this update was computed against; the request conflicts | ||
| when the share has moved past it. The member list is a read-modify-write, | ||
| so compare-and-set is recommended for every writer |
There was a problem hiding this comment.
The request is a collection of ADD / REMOVE entries with specific identities. Each of the changes can be resolved against the current state of the Share member on the server side without requiring a string sequence of changes.
Is an ADD invalidated by a concurrent ADD of another identity?
| description: An ordered list of membership updates, applied atomically | ||
| type: object | ||
| properties: | ||
| currentEntityVersion: |
There was a problem hiding this comment.
nit: this has a bit of an API skew - the payload mentions "entity version", but the entity is identified outside the context of this object.
If we go with strict previous entity version enforcement, it might be more consistent to put this on the URL as a query parameter.
3 participants
This adds an OpenAPI specification for a sharing control plane, as discussed in the dev-list thread
[DISCUSS] Adding support for new "Open Sharing" APIs in Polaris(https://lists.apache.org/thread/bsh4m3hvob1z21l14rd81r602mmmw2qz). It is specification only: no server code, no generated resources, and the whole API is marked beta per the evolution guidelines and would be served only behind a feature flag that defaults to off.What it defines, in one paragraph: a share is an explicit, named container enumerating shared objects, bound to exactly one catalog in this version; an external consumer is a restricted, first-class identity for a party outside the catalog's identity domain; a listing is the only binding between a consumer and a share and is the unit of audit; a listing's endpoint configuration is the connection material a consumer pastes into any standard Iceberg REST client (catalog endpoint, warehouse, token endpoint, scope, required headers); consumer credentials are OAuth2 client-credential pairs with issue, list, revoke and rotate operations. Consumers never call this API: they read shared tables through the Iceberg REST Catalog protocol unchanged, one prefix per share, read-only by construction.
Where it mounts: share management sits under the existing management root (
/api/management/v1/shares,/api/management/v1/external-consumers), beside catalogs, principals and roles, following the split Polaris already has between its management root and its protocol root. The consumer-facing Iceberg REST root is not part of this file; it is named only through the endpoint configuration, so the specification does not constrain where a deployment serves shares.Design notes for reviewers, each an open question rather than a settled position:
currentEntityVersion), so every writer can compare-and-set.exposedidentity on a member is reserved for a future aliasing capability and must equal the canonical identity in this version; response-only renaming was found unsound during design, so aliasing is all-or-nothing and deferred.An accompanying design document — recording the decisions considered (share addressing, the consumer-facing root, the entity representation) with the working implementations that informed them — will follow on the dev-list thread.
Checklist items: the specification passes
redocly lintstructurally (schema validity; the repository's default rule set is not clean on the existing specifications either); no code changes; no documentation changes beyond thespec/README.mdentry.Not in this PR: server code, the data-plane serving surface, the feature flag, and documentation pages — those follow the community's reaction to the specification and design document, on separate PRs.
See https://github.com/dennishuo/dhuo-public-provenance/blob/main/polaris-sharing/design/shares-journeys.md for examples of user journeys across different design combinations.
Per CONTRIBUTING's guidelines for AI-assisted contributions: this specification was drafted with AI assistance and reviewed and revised by the author, who owns every line.