Repository navigation
fix(workflow, hub): treat a workflow's owner as WRITE wherever access is read - #9190
Conversation
… is read Workflow access checks, search, the workflow list and its filters, the quota and admin execution pages now take the owner's privilege from workflow_of_user, not from their workflow_user_access row. Hub workflow cards join only the viewer's own access row, so a shared workflow yields one card with the viewer's level. Closes apache#9150
Backport auto-label reportThis
|
Automated Reviewer SuggestionsBased on the
|
…access # Conflicts: # amber/src/main/scala/org/apache/texera/web/resource/dashboard/WorkflowSearchQueryBuilder.scala
|
| config | throughput | MB/s | latency | max Δ latest / 7d | |
|---|---|---|---|---|---|
| 🔴 | bs=10 sw=10 sl=64 | 405 | 0.247 | 23,397/32,703/32,703 us | 🔴 +10.4% / 🔴 +137.4% |
| 🔴 | bs=100 sw=10 sl=64 | 825 | 0.504 | 117,878/153,137/153,137 us | 🔴 +6.0% / 🔴 +64.4% |
| ⚪ | bs=1000 sw=10 sl=64 | 961 | 0.587 | 1,032,959/1,170,315/1,170,315 us | ⚪ within ±5% / 🔴 +32.8% |
Baseline details
Latest main 6659eec from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 405 tuples/sec | 443 tuples/sec | 941.58 tuples/sec | -8.6% | -57.0% |
| bs=10 sw=10 sl=64 | MB/s | 0.247 MB/s | 0.27 MB/s | 0.575 MB/s | -8.5% | -57.0% |
| bs=10 sw=10 sl=64 | p50 | 23,397 us | 21,195 us | 10,991 us | +10.4% | +112.9% |
| bs=10 sw=10 sl=64 | p95 | 32,703 us | 34,533 us | 13,776 us | -5.3% | +137.4% |
| bs=10 sw=10 sl=64 | p99 | 32,703 us | 34,533 us | 16,872 us | -5.3% | +93.8% |
| bs=100 sw=10 sl=64 | throughput | 825 tuples/sec | 870 tuples/sec | 1,264 tuples/sec | -5.2% | -34.7% |
| bs=100 sw=10 sl=64 | MB/s | 0.504 MB/s | 0.531 MB/s | 0.772 MB/s | -5.1% | -34.7% |
| bs=100 sw=10 sl=64 | p50 | 117,878 us | 111,242 us | 86,073 us | +6.0% | +37.0% |
| bs=100 sw=10 sl=64 | p95 | 153,137 us | 144,955 us | 93,135 us | +5.6% | +64.4% |
| bs=100 sw=10 sl=64 | p99 | 153,137 us | 144,955 us | 111,124 us | +5.6% | +37.8% |
| bs=1000 sw=10 sl=64 | throughput | 961 tuples/sec | 967 tuples/sec | 1,304 tuples/sec | -0.6% | -26.3% |
| bs=1000 sw=10 sl=64 | MB/s | 0.587 MB/s | 0.59 MB/s | 0.796 MB/s | -0.5% | -26.3% |
| bs=1000 sw=10 sl=64 | p50 | 1,032,959 us | 1,031,003 us | 841,986 us | +0.2% | +22.7% |
| bs=1000 sw=10 sl=64 | p95 | 1,170,315 us | 1,132,158 us | 881,107 us | +3.4% | +32.8% |
| bs=1000 sw=10 sl=64 | p99 | 1,170,315 us | 1,132,158 us | 918,208 us | +3.4% | +27.5% |
Raw CSV
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,493.88,200,128000,405,0.247,23396.66,32703.34,32703.34
1,100,10,64,20,2423.90,2000,1280000,825,0.504,117878.25,153137.43,153137.43
2,1000,10,64,20,20805.67,20000,12800000,961,0.587,1032959.11,1170314.59,1170314.59…ok service and trim access lookups The notebook service's write check now lets the workflow owner through. getPrivilege reads the owner and the caller's grant in one query, hasReadAccess looks it up once, grant/revoke reuse isOwner, and the admin execution listing checks access against a set. Adds owner-without-a-row tests for the quota, admin user, admin execution and notebook paths.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9190 +/- ##
=========================================
Coverage 91.98% 91.98%
+ Complexity 5085 5084 -1
=========================================
Files 1281 1281
Lines 55812 55794 -18
Branches 7076 7077 +1
=========================================
- Hits 51338 51322 -16
Misses 2734 2734
+ Partials 1740 1738 -2
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…eave the notebook service as on main A workflow can have several owner rows, so the owner join is narrowed to the caller; the unnarrowed join made fetchOne() throw. The notebook service's own access check is reverted to main's direct-grant behaviour, which is outside this fix.
|
/request-review @kunwp1 |
…eld updates on write access alone
What changes were proposed in this PR?
Workflows took the owner's privilege from their
workflow_user_accessrow alone. If that row was missing or downgraded, the owner lost access to their own workflow, or it was hidden from them. This makes workflows match datasets, models and computing units (#8890): the owner is always WRITE.WorkflowAccessResource.getPrivilegereads the caller's own owner row and access row in one query (so a workflow with several owners still yields one row) and returns WRITE for the owner (viaOwnerAccess.privilege).hasReadAccess,hasWriteAccess,hasGrantedAccessand sharing all go through it.WorkflowAccessResource.accessibleWids(uid)is the single source for the ID and owner filters, the quota page and the admin executions page. Each of these used to read the access table alone.baseWorkflowSelect(uid)joins only the viewer's access row, the same as the dataset/model hub fix in fix(resource, computing-unit): keep owners' full access to their datasets, models and computing units #8890. A workflow shared with several users now gives one card with the viewer's own level, instead of duplicates or someone else's level.Any related issues, documentation, discussions?
Closes #9150. Follow-up to #8890.
How was this PR tested?
New tests:
WorkflowAccessResourceSpec: the owner is WRITE with no access row, and with a READ row. A workflow with several owner rows doesn't fail the check.accessibleWidslists owned and granted workflows once each.WorkflowResourceSpec: after the owner's access row is deleted, they still see the workflow as WRITE in the list, search, the ID and owner filters, and when opening it.HubResourceSpec: a workflow shared with two other users gives one entry with the viewer's own level (NONE for an ungranted or anonymous viewer). The owner without an access row gets WRITE.UserQuotaResourceSpec,AdminUserResourceSpec,AdminExecutionResourceSpec: an owner without an access row still gets their workflow listed or marked accessible.WorkflowSearchQueryBuilderSpec: the owner is WRITE whatever their access row says, and sees their live name rather than the pinned one. An anonymous viewer of a workflow with no owner row is not mistaken for the owner.Three existing tests assumed the old behaviour and were updated. One asserted NONE for an owner without an access row, which is the bug this PR fixes. The other two read the privilege as the owner, so they now read it as a non-owner.
With the main-code changes reverted, the 6 new owner and hub tests fail. The affected amber specs pass (373), and
scalafmtCheckAllandscalafixAll --checkare clean.Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5.5)