Skip to content

[tests] Wait until users, roles and grants reach all cluster replicas #3277

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
polyglotAI-bot wants to merge 1 commit into from
Closed

[tests] Wait until users, roles and grants reach all cluster replicas #3277

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions clickhouse-client/src/test/java/com/clickhouse/client/BaseIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package com.clickhouse.client;

import com.clickhouse.client.config.ClickHouseClientOption;
import org.testng.Assert;
import org.testng.annotations.AfterTest;
import org.testng.annotations.BeforeTest;

import java.io.IOException;
import java.net.InetAddress;
import java.net.UnknownHostException;
import java.util.Locale;
Expand All @@ -13,6 +15,9 @@
* Base class for all integration tests.
*/
public abstract class BaseIntegrationTest {
private static final int REPLICA_CHECK_ATTEMPTS = 100;
private static final long REPLICA_CHECK_INTERVAL_MS = 100L;

@BeforeTest(groups = {"integration"})
public static void setupClickHouseContainer() {
ClickHouseServerForTest.beforeSuite();
Expand Down Expand Up @@ -86,6 +91,53 @@
return ClickHouseServerForTest.isCluster() ? " ON CLUSTER 'test_cluster'" : "";
}

/**
* Waits until each replica of the test cluster returns {@code expectedRows} rows of
* {@code system.<table>} that match {@code condition}, so that the next request sees a
* created or changed user, role, settings profile or grant on any replica.
* Does nothing when the tests do not run on a cluster.
*/
protected static void waitForAllReplicas(String table, String condition, int expectedRows) {
if (!isCluster()) {
return;
}
String sql = "SELECT hostName() AS host, countIf(" + condition + ") AS matching_rows FROM clusterAllReplicas('"
+ ClusterClickHouseContainerStarter.CLUSTER_NAME + "', system." + table + ") GROUP BY host";
String state = "";
for (int attempt = 1; attempt <= REPLICA_CHECK_ATTEMPTS; attempt++) {
try {
state = ClickHouseServerForTest.queryForResult(sql).trim();
if (hasRowsOnAllReplicas(state, expectedRows)) {
return;
}
} catch (IOException e) {
state = e.getMessage();
}
try {
Thread.sleep(REPLICA_CHECK_INTERVAL_MS);

Check warning on line 117 in clickhouse-client/src/test/java/com/clickhouse/client/BaseIntegrationTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove this use of "Thread.sleep()". 


See more on https://sonarcloud.io/project/issues?id=ClickHouse_clickhouse-java&issues=AaGJs9dyEVU3YmPccQ2I&open=AaGJs9dyEVU3YmPccQ2I&pullRequest=3277
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
break;
}
}
Assert.fail("Expected " + expectedRows + " rows matching [" + condition + "] in system." + table + " on all "
+ ClusterClickHouseContainerStarter.NODE_COUNT + " replicas, but got [host, rows]:\n" + state);
}

private static boolean hasRowsOnAllReplicas(String result, int expectedRows) {
String[] lines = result.split("\n");
if (lines.length != ClusterClickHouseContainerStarter.NODE_COUNT) {
return false;
}
for (String line : lines) {
String[] columns = line.split("\t");
if (columns.length != 2 || !columns[1].equals(String.valueOf(expectedRows))) {
return false;
}
}
return true;
}

protected boolean createDatabase(String dbName) {
return ClickHouseServerForTest.runQuery("CREATE DATABASE IF NOT EXISTS `" + dbName + "`" + onCluster());
}
Expand Down
45 changes: 45 additions & 0 deletions clickhouse-client/src/test/java/com/clickhouse/client/ClickHouseServerForTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@

import com.clickhouse.client.ClickHouseTestEnvironment.ContainerMode;

import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.net.HttpURLConnection;
import java.net.InetSocketAddress;
Expand Down Expand Up @@ -279,6 +282,48 @@
return false;
}

/**
* Runs a query once and returns the response body (TabSeparated).
*
* @throws IOException when the request fails or the server returns an error
*/
public static String queryForResult(String sql) throws IOException {

@chernser chernser Oct 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, no - we should use own client - it is capable enough for this. this code just add extra to tests and hard to maintain.
In V1 move tests to http client tests - this will be much easier to use V1 within tests
In V2 use V2 instance of client - if required create aux instance just for tests queries.

@chernser chernser Oct 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@polyglotAI-bot please fix:

  • we should use own client - it is capable enough for this. this code just add extra to tests and hard to maintain.
    In V1 move tests to http client tests - this will be much easier to use V1 within tests
    In V2 use V2 instance of client - if required create aux instance just for tests queries.

ClickHouseNode server = getClickHouseNode(ClickHouseProtocol.HTTP, isCloud(), ClickHouseNode.builder().build());
byte[] postData = sql.getBytes(StandardCharsets.UTF_8);
String authorization = Base64.getEncoder()
.encodeToString((getUsername() + ":" + getPassword()).getBytes(StandardCharsets.UTF_8));
HttpURLConnection httpConn = (HttpURLConnection) new URL(server.getBaseUri()).openConnection();
try {

Check failure on line 296 in clickhouse-client/src/test/java/com/clickhouse/client/ClickHouseServerForTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Change this "try" to a try-with-resources. 


See more on https://sonarcloud.io/project/issues?id=ClickHouse_clickhouse-java&issues=AaGJs9b-EVU3YmPccQ2H&open=AaGJs9b-EVU3YmPccQ2H&pullRequest=3277
httpConn.setConnectTimeout(10000);
httpConn.setReadTimeout(10000);
httpConn.setRequestMethod("POST");
httpConn.setDoOutput(true);
httpConn.setRequestProperty("Authorization", "Basic " + authorization);
httpConn.setFixedLengthStreamingMode(postData.length);
try (OutputStream out = httpConn.getOutputStream()) {
out.write(postData, 0, postData.length);
}

boolean ok = httpConn.getResponseCode() == HttpURLConnection.HTTP_OK;
ByteArrayOutputStream body = new ByteArrayOutputStream();
try (InputStream in = ok ? httpConn.getInputStream() : httpConn.getErrorStream()) {
if (in != null) {
byte[] buffer = new byte[4096];
for (int read; (read = in.read(buffer)) != -1; ) {
body.write(buffer, 0, read);
}
}
}
String result = new String(body.toByteArray(), StandardCharsets.UTF_8);
if (!ok) {
throw new IOException("HTTP " + httpConn.getResponseCode() + ": " + result);
}
return result;
} finally {
httpConn.disconnect();
}
}

private static Endpoint resolve(ClickHouseProtocol protocol, int port, boolean applyOverride) {
if (starter != null) {
int containerPort = (port == ClickHouseProtocol.HTTP.getDefaultPort())
Expand Down
102 changes: 49 additions & 53 deletions client-v2/src/test/java/com/clickhouse/client/ClientTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,28 +434,43 @@ private static Object[][] sessionRoles() {
}

@Test(groups = {"integration"}, dataProvider = "sessionRoles")
public void testOperationCustomRoles(String[] roles) throws Exception {
public void testOperationCustomRoles(String[] roleNames) throws Exception {
if (isVersionMatch("(,24.3]", newClient().build())) {
return;
}

String suffix = "_" + RandomStringUtils.insecure().next(8, true, true);
String user = "some_user" + suffix;
String[] roles = Arrays.stream(roleNames).map(role -> role + suffix).toArray(String[]::new);
String password = "^1A" + RandomStringUtils.random(12, true, true) + "3b$";
final String rolesList = "\"" + Strings.join("\",\"", roles) + "\"";
try (Client client = newClient().build()) {
client.execute("DROP ROLE IF EXISTS " + rolesList).get().close();
client.execute("CREATE ROLE " + rolesList).get().close();
client.execute("DROP USER IF EXISTS some_user").get().close();
client.execute("CREATE USER some_user IDENTIFIED BY '" + password + "'").get().close();
client.execute("GRANT " + rolesList + " TO some_user").get().close();
try (Client client = newClient().build();
Client userClient = newClient().setUsername(user).setPassword(password).build()) {
try {
createUserWithRoles(client, user, "IDENTIFIED BY '" + password + "'", roles);
QuerySettings settings = new QuerySettings().setDBRoles(Arrays.asList(roles));
List<GenericRecord> resp = userClient.queryAll("SELECT currentRoles()", settings);
Set<String> roleSet = new HashSet<>(Arrays.asList(roles));
Set<String> currentRoles = new HashSet<String> (resp.get(0).getList(1));
Assert.assertEquals(currentRoles, roleSet, "Roles " + roleSet + " not found in " + currentRoles);
} finally {
client.execute("DROP USER IF EXISTS " + user).get().close();
client.execute("DROP ROLE IF EXISTS " + rolesList).get().close();
}
}
}

try (Client userClient = newClient().setUsername("some_user").setPassword(password).build()) {
QuerySettings settings = new QuerySettings().setDBRoles(Arrays.asList(roles));
List<GenericRecord> resp = userClient.queryAll("SELECT currentRoles()", settings);
Set<String> roleSet = new HashSet<>(Arrays.asList(roles));
Set<String> currentRoles = new HashSet<String> (resp.get(0).getList(1));
Assert.assertEquals(currentRoles, roleSet, "Roles " + roleSet + " not found in " + currentRoles);
}
private static void createUserWithRoles(Client client, String user, String identification, String[] roles)
throws Exception {
final String rolesList = "\"" + Strings.join("\",\"", roles) + "\"";
final String roleNames = "'" + Strings.join("','", roles) + "'";
client.execute("CREATE ROLE " + rolesList).get().close();
client.execute("CREATE USER " + user + " " + identification).get().close();
waitForAllReplicas("roles", "name IN (" + roleNames + ")", roles.length);
waitForAllReplicas("users", "name = '" + user + "'", 1);
client.execute("GRANT " + rolesList + " TO " + user).get().close();
waitForAllReplicas("role_grants", "user_name = '" + user + "' AND granted_role_name IN (" + roleNames + ")",
roles.length);
}

@DataProvider(name = "clientSessionRoles")
Expand All @@ -466,27 +481,29 @@ private static Object[][] clientSessionRoles() {
};
}
@Test(groups = {"integration"}, dataProvider = "clientSessionRoles")
public void testClientCustomRoles(String[] roles) throws Exception {
public void testClientCustomRoles(String[] roleNames) throws Exception {
if (isVersionMatch("(,24.3]", newClient().build())) {
return;
}

String suffix = "_" + RandomStringUtils.insecure().next(8, true, true);
String user = "some_user" + suffix;
String[] roles = Arrays.stream(roleNames).map(role -> role + suffix).toArray(String[]::new);
String password = "^1A" + RandomStringUtils.insecure().next(12, true, true) + "3B$";
final String rolesList = "\"" + Strings.join("\",\"", roles) + "\"";
try (Client client = newClient().build()) {
client.execute("DROP ROLE IF EXISTS " + rolesList).get().close();
client.execute("CREATE ROLE " + rolesList).get().close();
client.execute("DROP USER IF EXISTS some_user").get().close();
client.execute("CREATE USER some_user IDENTIFIED WITH sha256_password BY '" + password + "'").get().close();
client.execute("GRANT " + rolesList + " TO some_user").get().close();
}

try (Client userClient = newClient().setUsername("some_user").setPassword(password).build()) {
userClient.setDBRoles(Arrays.asList(roles));
List<GenericRecord> resp = userClient.queryAll("SELECT currentRoles()");
Set<String> roleSet = new HashSet<>(Arrays.asList(roles));
Set<String> currentRoles = new HashSet<String> (resp.get(0).getList(1));
Assert.assertEquals(currentRoles, roleSet, "Roles " + roleSet + " not found in " + currentRoles);
try (Client client = newClient().build();
Client userClient = newClient().setUsername(user).setPassword(password).build()) {
try {
createUserWithRoles(client, user, "IDENTIFIED WITH sha256_password BY '" + password + "'", roles);
userClient.setDBRoles(Arrays.asList(roles));
List<GenericRecord> resp = userClient.queryAll("SELECT currentRoles()");
Set<String> roleSet = new HashSet<>(Arrays.asList(roles));
Set<String> currentRoles = new HashSet<String> (resp.get(0).getList(1));
Assert.assertEquals(currentRoles, roleSet, "Roles " + roleSet + " not found in " + currentRoles);
} finally {
client.execute("DROP USER IF EXISTS " + user).get().close();
client.execute("DROP ROLE IF EXISTS " + rolesList).get().close();
}
}
}

Expand All @@ -507,36 +524,15 @@ public void testRuntimeCredentialChange() throws Exception {
adminClient.execute("DROP USER IF EXISTS " + user2).get().close();
adminClient.execute("CREATE USER " + user1 + " IDENTIFIED BY '" + password1 + "'").get().close();
adminClient.execute("CREATE USER " + user2 + " IDENTIFIED BY '" + password2 + "'").get().close();
waitForAllReplicas("users", "name IN ('" + user1 + "', '" + user2 + "')", 2);

try (Client userClient = newClient().setUsername(user1).setPassword(password1).build()) {
List<GenericRecord> firstResponse = null;
for (int i = 0; i < 20; i++) {
try {
firstResponse = userClient.queryAll("SELECT currentUser() AS user");
break;
} catch (ClientException e) {
if (i == 19) {
throw e;
}
Thread.sleep(250);
}
}
List<GenericRecord> firstResponse = userClient.queryAll("SELECT currentUser() AS user");
Assert.assertEquals(firstResponse.get(0).getString("user"), user1);

userClient.updateUserAndPassword(user2, password2);

List<GenericRecord> secondResponse = null;
for (int i = 0; i < 20; i++) {
try {
secondResponse = userClient.queryAll("SELECT currentUser() AS user");
break;
} catch (ClientException e) {
if (i == 19) {
throw e;
}
Thread.sleep(250);
}
}
List<GenericRecord> secondResponse = userClient.queryAll("SELECT currentUser() AS user");
Assert.assertEquals(secondResponse.get(0).getString("user"), user2);
}
} finally {
Expand Down
Loading
Loading