add-table refactor for clean auth #9228
Conversation
apucher
added
incompatible
Codecov Report
@@ Coverage Diff @@
## master #9228 +/- ##
=============================================
- Coverage 70.01% 28.53% -41.48%
+ Complexity 4762 53 -4709
=============================================
Files 1857 1845 -12
Lines 99136 98782 -354
Branches 15076 15040 -36
=============================================
- Hits 69406 28185 -41221
- Misses 24833 67890 +43057
+ Partials 4897 2707 -2190
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
apucher
force-pushed
the
add-table-auth-cleanup
branch
from
81812ea to
3613589
Compare
apucher
force-pushed
the
add-table-auth-cleanup
branch
from
e3bd481 to
379766a
Compare
| @@ -100,11 +91,15 @@ public void validatePermission(Optional<String> tableNameOpt, AccessType accessT | |||
| * @param httpHeaders HTTP headers containing requester identity required by access control object | |||
There was a problem hiding this comment.
Update the javadoc. It's not a table level.
| public static void validatePermission(@Nullable String tableName, AccessType accessType, | ||
| @Nullable HttpHeaders httpHeaders, @Nullable String endpointUrl, AccessControl accessControl) { | ||
| String accessTypeToEndpointMsg = | ||
| String.format("access type '%s' to the endpoint '%s' for table '%s'", accessType, endpointUrl, tableName); |
There was a problem hiding this comment.
If tableName is null, the log message doesn't look good. Previous message was handling this case better IMO.
|
Looks like with this PR, we've lost the ability to upload REALTIME tables using AddTable which was supported before? |
An oss user stumbled upon this after upgrading to 0.11 https://apache-pinot.slack.com/archives/C011C9JHN7R/p1664525974774459 |
We modify the
AddTableadmin command to use the new-ish/tableConfigsendpoint rather than/schemaand/tablesto fix an undesirable auth effect. This change enables use-cases where non-admin users can self-serve table creation, assuming the table names have been whitelisted ahead of time.Previously, the
/schemaendpoint wasn't able to gracefully extract the table/schema name on POST, triggering a generic (i.e. non-table-specific) CREATE request against the cluster which required admin privileges.The label has been assigned since the change makes pinot-admin
incompatiblewith very old versions of pinot which don't support/tableConfigsyet