Skip to content

[api] Add REST permission management - #9393

Merged
JingsongLi merged 2 commits into
apache:masterfrom
JingsongLi:codex/rest-permission-management-api
Aug 26, 2026
Merged

[api] Add REST permission management#9393
JingsongLi merged 2 commits into
apache:masterfrom
JingsongLi:codex/rest-permission-management-api

Conversation

@JingsongLi

Copy link
Copy Markdown
Contributor

Purpose

Introduce experimental Java and REST client contracts for managing catalog permissions.

Changes

  • Add resource, access, assignment, expiry, and pagination models.
  • Support catalog/database descendant scopes and column allowlists or denylists.
  • Add grant, revoke, and list operations to the REST client.
  • Expose permission management directly from RESTCatalog.
  • Cover validation, shaded/external Jackson compatibility, request paths, authentication, pagination, and wire shapes.

Scope

This is part 1 of a planned split. Data policy management, the management OpenAPI/documentation, and Spark SQL procedures will follow in separate PRs.

Tests

  • mvn -pl paimon-api -DskipITs -Dfast -Dtest=PermissionManagementJsonTest,RESTPermissionManagementTest,RequestJacksonCompatibilityTest test
  • mvn -pl paimon-core -am -DskipITs -Dfast -Dtest=ResourcePathsTest -Dsurefire.failIfNoSpecifiedTests=false test

@JingsongLi
JingsongLi marked this pull request as ready for review August 26, 2026 03:52
@JsonInclude(JsonInclude.Include.NON_NULL)
private final String expireTime;

@JsonCreator

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This @JsonCreator is also the strict-validation constructor, and it deserializes ListPermissionsResponse.permissions — so anything the client doesn't recognize fails the whole page, not one row. E.g. Instant.now() is microsecond-precision on JDK 9+, so a server returning ...00.123456Z breaks every list call; such a record can't be revoked either, since RevokePermissionRequest runs the same canonicalize.

Since the server decides anyway, could we keep validation on the request path and let the response path be lenient — e.g. move @JsonCreator to a package-private unvalidated constructor?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 8f9b504. The public constructor remains strict for grant requests, while Jackson response deserialization now uses a package-private unvalidated constructor. Added shaded and external Jackson coverage for a microsecond-precision expireTime response, while verifying the same value is still rejected on the grant request path.

MAX_PAGE_SIZE);
this.principal = isBlank(principal) ? null : principal;
this.access = isBlank(access) ? null : PermissionAccess.canonicalize(resource, access);
this.pageToken = isBlank(pageToken) ? null : pageToken;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When a server emits a nonempty whitespace-only continuation token, withPageToken routes it through this constructor and converts it to null; the next GET then omits pageToken and fetches the first page again, and PagedList.listAllFromPagedApi can loop indefinitely if that page keeps returning the token. Page tokens are
opaque and the paging API uses null as the first-page marker, so retain the returned token unchanged.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 8f9b504. ListPermissionsRequest now preserves pageToken verbatim, including nonempty whitespace-only opaque tokens. Added a regression assertion through withPageToken.

@plusplusjiajia

Copy link
Copy Markdown
Member

Overall shape looks right — filling the management-plane gap next to the existing authTableQuery enforcement path, and staying on the existing conventions. Left two questions inline above.

@plusplusjiajia

Copy link
Copy Markdown
Member

LGTM +1

@JingsongLi
JingsongLi merged commit d99b376 into apache:masterAug 26, 2026
14 checks passed
Sign up for freeto 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.

2 participants

@JingsongLi@plusplusjiajia