Repository navigation
[python] Test permissions and query auth across PyPaimon and DataFusion SQL - #10503
plusplusjiajia wants to merge 1 commit into
Conversation
7918ba4 to
3e1697e
Compare
JingsongLi
left a comment
JingsongLi
left a comment
There was a problem hiding this comment.
The permission/procedure interoperability coverage has clear end-to-end value. I found one mock-server contract issue below.
Validation at 3e1697e: 208 focused permission/query-auth tests passed; the broader REST run had 378 passes and one environment failure because the optional lance package is absent. Changed Python files passed the repository Flake8 configuration, and license validation passed. The exact head's Native CI is green. My local installed Rust binary predates read-time query auth, so its SQL failures are not attributed to this PR.
| return set().union(*(_referenced_columns(item) for item in node)) | ||
| if not isinstance(node, dict): | ||
| return set() | ||
| found = {node["name"]} if "index" in node and "name" in node else set() |
There was a problem hiding this comment.
[P2] Collect name-only FIELD_REF references before validating schema changes
A valid row-filter fieldRef can omit index, e.g. {"kind":"LEAF","transform":{"name":"FIELD_REF","fieldRef":{"name":"id","type":"BIGINT"}},"function":"EQUAL","literals":[1]}. Both the existing Python predicate parser and Java's FieldRef/remapping support this. Requiring index here makes _policy_columns return an empty set. I reproduced through HTTP: create this policy on id, then rename id to user_id; the rename succeeds and the stored policy still references id. This bypasses the new policy-preserving schema guard and leaves subsequent authorized reads with a stale policy. Please recognize field references by their transform structure/name, and cover the name-only form for policy creation and column rename/drop.
There was a problem hiding this comment.
Thanks for catching this! The mock now reads policy columns from every fieldRef and transform input, with or without index, and the tests cover name-only refs for policy creation and column rename/drop.
3e1697e to
b091545
Compare
…on SQL The mock REST server gains the permission, policy and AuthTableQuery endpoints, after Java RESTCatalogServer: - Grants are removed or moved with the table, database or function they name, and COLUMN grants follow column renames. - Per-table policies need query-auth.enabled, conflict as POLICY, must name existing columns and go away with their table. Their columns are read from every fieldRef and transform input, with or without an index. - A schema change is refused before it is committed if it would disable query authorization or drop or rename a column that a policy or the last allowed column of a COLUMN grant needs. - The auth endpoint checks the selected columns against the COLUMN grants of the principals the caller acts as, and answers with their policies; two masks on one column deny the query. sql_permission_test.py runs in the Rust main job and checks that the two clients agree: grants and policies made with the CALL sys.* procedures are listed by PyPaimon and the other way round, and both readers return the same rows and values under a row filter, column masks and a COLUMN grant. rest_permission_server_test.py covers the mock server's rules.
b091545 to
0359045
Compare
JingsongLi
left a comment
JingsongLi
left a comment
There was a problem hiding this comment.
Requirement fit: SUPPORTED. The cross-client permission/read coverage has end-to-end value. The previous name-only fieldRef finding is fixed: both direct fieldRef and transform inputs now protect referenced columns before schema changes are committed.
I re-reviewed the full diff and reran the permission/policy/mock tests (49 passed), auth/JSON/validation tests (262 passed), and the broader REST suite (418 passed initially; the sole missing-DuckDB environment failure passed after installing the declared optional dependency). Changed files passed the repository Flake8 configuration. I also verified the exact-head Native CI log: all five SQL permission tests passed with Rust 0.5.0 at 3bc53b4d5f2fcd54d4f4bcbc2dec2aea1c4357a6; I did not rerun that native SQL lane locally.
One remaining mock/API contract mismatch is reproduced below. Two added JDK 8 Java reference probes passed without fast-build, while the equivalent Python HTTP rejection probe failed because the mock accepts the invalid mask.
| "Data policies require the target table option query-auth.enabled=true.", | ||
| 409), 409) | ||
| identity = _policy_identity(policy) | ||
| missing = _policy_columns(policy) - {field.name for field in metadata.schema.fields} |
There was a problem hiding this comment.
[P2] Validate mask output types as well as referenced column names
This validation accepts a mask for STRING email with {'name':'YEAR','fieldRef':{'name':'joined','type':'DATE'}}, because both column names exist. I reproduced through catalog.policy_management().create_policy: it succeeds, while an assertion expecting BadRequestException fails. YEAR produces INT; the documented management contract requires a mask's output type to match its protected column, and Java RESTCatalogServerUtils.parseColumnMask rejects this mismatch. Direct Java probes confirmed the rejection.
The new successful fixtures also depend on this looser mock: NULL_MASK is used on STRING email, whereas Java's NullTransform.outputType() is BOOLEAN and its reference server rejects that policy too. Thus the cross-client pass can exercise policies the reference service cannot create. Validate schema-bound mask transforms before storing them and when validating schema changes; add a wrong-output-type negative case. Use a valid STRING mask for email, and a nullable BOOLEAN column for NULL-mask coverage (or separately establish and synchronize a broader NULL policy contract). This finding concerns the new mock/test validity, not an unchanged Java implementation regression.
2 participants
Purpose
PyPaimon's
SQLContextis pypaimon-rust's DataFusion context. Both clients manage permissions and data policies through the same REST endpoints, and both read query-auth tables through AuthTableQuery, but no test checks that they agree. This adds those tests.The mock REST server gains the permission, policy and AuthTableQuery endpoints, after Java
RESTCatalogServer:query-auth.enabledand existing columns, and stops applying at its expire time, read with the same instant rules asPermissionAssignment.query-auth.enabledand existing columns, conflict asPOLICY, and are removed with their table. The columns a policy refers to are read from everyfieldRefand transform input, with or without anindex.set_query_principals), then answers with their policies. Two masks on one column deny the query.sql_permission_test.pychecks that grants and policies made with theCALL sys.*procedures are listed by PyPaimon and the other way round, and that both readers return the same rows and values under a row filter, column masks (NULLandCONCAT('***')) and a COLUMN grant. Both readers also refuse a column that the grant excludes. These tests are markednative_plan, so they run only in the Native CI job, which builds paimon-rust from main. The released pypaimon-rust 0.3.0 has neither the procedures nor read-time query auth.rest_permission_server_test.pycovers the mock server's rules and runs in every job.Tests
With pypaimon-rust built from apache/paimon-rust main (
eaa64bc) on Python 3.12 and all fivePYPAIMON_TEST_NATIVE_*switches set, the 5 tests insql_permission_test.pypass. Without the switches they are skipped.pypaimon/tests/restpasses (380 tests, excluding one that needs the optionallancepackage), including the mock server tests. The native REST plan tests still pass, and on Python 3.6 thepy36tests that use the mock server pass.