Copilot commented on code in PR #17167:
URL: https://github.com/apache/pinot/pull/17167#discussion_r3041368338
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/TlsIntegrationTest.java:
##########
@@ -499,8 +536,12 @@ public void testControllerBrokerQueryForward()
@Test(expectedExceptions = IOException.class)
public void testUnauthenticatedFailure()
throws IOException {
- sendDeleteRequest(
-
_controllerRequestURLBuilder.forTableDelete(TableNameBuilder.REALTIME.tableNameWithType("mytable")));
+ HttpDelete request = new HttpDelete(
+ "https://localhost:"; + _externalControllerPort + "/tables/" +
TableNameBuilder.REALTIME.tableNameWithType(
+ getTableName()));
+ try (CloseableHttpClient client = HttpClientBuilder.create().build()) {
+ client.execute(request);
+ }
Review Comment:
This unauthenticated TLS test now uses a default Apache HttpClient without
the test SSLContext, so the expected IOException is likely coming from TLS
handshake/trust failure rather than an authentication/authorization failure. To
keep the test asserting the intended behavior, make the request using the
configured SSLContext (or PinotAdminClient without auth headers) and assert the
controller returns/raises an auth failure (e.g.,
401/PinotAdminAuthenticationException).
##########
pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotAccessControlUserRestletResourceTest.java:
##########
@@ -91,17 +61,17 @@ public void testUpdateUserConfig()
String username = "updateTC";
String userConfigString =
_userConfigBuilder.setUsername(username).setComponentType(ComponentType.CONTROLLER).build().toJsonString();
- ControllerTest.sendPostRequest(_createUserUrl, userConfigString);
+
TEST_INSTANCE.getOrCreateAdminClient().getUserClient().createUser(userConfigString);
// user creation should succeed
Review Comment:
This test class no longer covers user creation failure scenarios (e.g.,
rejecting usernames with '.'/spaces, or creating an already-existing user).
Since the PR is a client migration, it would be better to preserve those
assertions using PinotUserAdminClient (expecting the appropriate
exception/status) to avoid losing coverage of controller validation behavior.
--
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]
