Copilot commented on code in PR #16170:
URL: https://github.com/apache/pinot/pull/16170#discussion_r2160276797
##########
pinot-broker/src/main/java/org/apache/pinot/broker/broker/BasicAuthAccessControlFactory.java:
##########
@@ -136,10 +137,27 @@ public TableAuthorizationResult
authorize(RequesterIdentity requesterIdentity, S
}
private Optional<BasicAuthPrincipal> getPrincipalOpt(RequesterIdentity
requesterIdentity) {
- Preconditions.checkArgument(requesterIdentity instanceof
HttpRequesterIdentity, "HttpRequesterIdentity required");
- HttpRequesterIdentity identity = (HttpRequesterIdentity)
requesterIdentity;
-
- Collection<String> tokens =
identity.getHttpHeaders().get(HEADER_AUTHORIZATION);
+ Preconditions.checkArgument(
+ requesterIdentity instanceof HttpRequesterIdentity ||
requesterIdentity instanceof GrpcRequesterIdentity,
+ "BasicAuthAccessControl only supports HttpRequesterIdentity or
GrpcRequesterIdentity, got %s",
+ requesterIdentity.getClass().getName());
+ Collection<String> tokens = null;
Review Comment:
[nitpick] There is duplicated logic for extracting HTTP and gRPC tokens;
consider refactoring into a shared helper to simplify `getPrincipalOpt` and
reduce code duplication.
##########
pinot-clients/pinot-jdbc-client/src/main/java/org/apache/pinot/client/examples/PinotJdbcExample.java:
##########
@@ -85,9 +102,23 @@ public static void main(String[] args)
String query;
// query = "SELECT count(*) FROM airlineStats";
query = "SELECT * FROM airlineStats limit 1000";
+
+ // Test with JDBC connection methods: http(default), grpc.
testPinotJdbcForQuickStart("jdbc:pinot://localhost:9000", query);
testPinotJdbcForQuickStart("jdbc:pinotgrpc://localhost:9000?blockRowSize=100",
query);
+ // Test with JDBC connection using grpc with authentication.
+ // Note: Run below with {@link org.apache.pinot.tools.AuthQuickstart} to
enable authentication.
+ System.out.println("\n\nTesting JDBC connection with authentication");
+
testPinotJdbcForQuickStart("jdbc:pinotgrpc://localhost:9000?blockRowSize=100",
query, "admin", "verysecret");
+
+ // Test with JDBC connection using grpc with authentication properties.
+ System.out.println("\n\nTesting JDBC connection with authentication
properties");
+ Properties authProperties = new Properties();
+ authProperties.put("Authorization", "Basic YWRtaW46dmVyeXNlY3JldA==");
+
testPinotJdbcForQuickStart("jdbc:pinotgrpc://localhost:9000?blockRowSize=100",
query, "admin", "verysecret");
Review Comment:
The `authProperties` defined above is never used; this call mistakenly
reuses the username/password overload instead of passing `authProperties`.
Replace this with `testPinotJdbcForQuickStart(jdbcUrl, query, authProperties)`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
