multitenant: add AdminUnsplitRequest capability#97717
multitenant: add AdminUnsplitRequest capability#97717craig[bot] merged 4 commits intocockroachdb:masterfrom ecwall:multitenant_unsplit
Conversation
|
This change is |
arulajmani
left a comment
There was a problem hiding this comment.
As we're adding new capabilities, we should continue to add tests for them in the tenantcapabilities package. For this one, I think all you need is to add more tests to the Authorizer's datadriven tests.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @knz, @mgartner, and @rafiss)
ecwall
left a comment
There was a problem hiding this comment.
To make adding those tests easier, can I replace the request method aliases (split, scan, cput) with the actual method names (AdminSplit, Scan, ConditionalPut)?
This will prevent new cases from needing to be added to this switch for each method:
switch z {
case "split":
ba.Add(&kvpb.AdminSplitRequest{})
case "scan":
ba.Add(&kvpb.ScanRequest{})
case "cput":
ba.Add(&kvpb.ConditionalPutRequest{})
default:
t.Fatalf("unsupported request type: %s", z)
}
I would like to minimize the overhead of using DSLs everywhere for what could be go tests.
Reviewable status:
Remove `TestGCTenant`'s reliance on `TenantCapabilities` struct field names to reduce code churn. Release note: None
Filter out non-target capabilities from output of capabilites_test.go testdata. Release note: None
knz
left a comment
There was a problem hiding this comment.
nice.
Reviewed 18 of 18 files at r3, 1 of 1 files at r4, 8 of 8 files at r5, 25 of 25 files at r6, all commit messages.
Reviewable status:
Use request methods instead of aliases to avoid needing to maintain alias -> request method mapping. Release note: None
Fixes #97716 1) Add a new `tenantcapabilitiespb.CanAdminUnsplit` capability. 2) Check capability in `Authorizer.HasCapabilityForBatch`. 3) Remove `ExecutorConfig.RequireSystemTenant` call from `execFactory.ConstructAlterTableUnsplit`, `execFactory.ConstructAlterTableUnsplitAll`. Release note: None
|
bors r=knz |
|
Build succeeded: |
Fixes #97716
tenantcapabilitiespb.CanAdminUnsplitcapability.Authorizer.HasCapabilityForBatch.ExecutorConfig.RequireSystemTenantcall fromexecFactory.ConstructAlterTableUnsplit,execFactory.ConstructAlterTableUnsplitAll.Release note: None