Skip to content

Backport to branch(3.15) : Add JDBC database permission test (Part 2) #2941

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

Merged
merged 2 commits into from
Aug 5, 2025
Merged

Backport to branch(3.15) : Add JDBC database permission test (Part 2) #2941

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
88810cf
Add JDBC database permission test (Part 2) (#2923)
KodaiD Aug 5, 2025
a51eba1
Remove Db2 handling
KodaiD Aug 5, 2025
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
198 changes: 198 additions & 0 deletions .github/workflows/permission-check.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -619,3 +619,201 @@ jobs:
with:
name: sqlserver_2022_permission_integration_test_reports
path: core/build/reports/tests/integrationTestJdbcPermission

integration-test-permission-jdbc-mariadb-10-11:
name: MariaDB 10.11 Permission Integration Test
runs-on: ubuntu-latest

steps:
- name: Run MariaDB 10.11
run: |
docker run -e MYSQL_ROOT_PASSWORD=mysql -p 3306:3306 -d mariadb:10.11 --character-set-server=utf8mb4 --collation-server=utf8mb4_bin

- uses: actions/checkout@v4

- name: Set up JDK ${{ env.JAVA_VERSION }} (${{ env.JAVA_VENDOR }})
uses: actions/setup-java@v4
with:
java-version: ${{ env.JAVA_VERSION }}
distribution: ${{ env.JAVA_VENDOR }}

- name: Setup Gradle
uses: gradle/actions/setup-gradle@v4

- name: Execute Gradle 'integrationTestJdbcPermission' task
run: ./gradlew integrationTestJdbcPermission -Dscalardb.jdbc.url=jdbc:mariadb://localhost:3306 -Dscalardb.jdbc.username=root -Dscalardb.jdbc.password=mysql

- name: Upload Gradle test reports
if: always()
uses: actions/upload-artifact@v4
with:
name: mariadb_10.11_permission_integration_test_reports
path: core/build/reports/tests/integrationTestJdbcPermission

integration-test-permission-jdbc-mariadb-11-4:
name: MariaDB 11.4 Permission Integration Test
runs-on: ubuntu-latest

steps:
- name: Run MariaDB 11.4
run: |
docker run -e MYSQL_ROOT_PASSWORD=mysql -p 3306:3306 -d mariadb:11.4 --character-set-server=utf8mb4 --collation-server=utf8mb4_bin

- uses: actions/checkout@v4

- name: Set up JDK ${{ env.JAVA_VERSION }} (${{ env.JAVA_VENDOR }})
uses: actions/setup-java@v4
with:
java-version: ${{ env.JAVA_VERSION }}
distribution: ${{ env.JAVA_VENDOR }}

- name: Setup Gradle
uses: gradle/actions/setup-gradle@v4

- name: Execute Gradle 'integrationTestJdbcPermission' task
run: ./gradlew integrationTestJdbcPermission -Dscalardb.jdbc.url=jdbc:mariadb://localhost:3306 -Dscalardb.jdbc.username=root -Dscalardb.jdbc.password=mysql

- name: Upload Gradle test reports
if: always()
uses: actions/upload-artifact@v4
with:
name: mariadb_11.4_permission_integration_test_reports
path: core/build/reports/tests/integrationTestJdbcPermission

integration-test-permission-jdbc-yugabytedb-2:
name: YugabyteDB 2 Permission Integration Test
runs-on: ubuntu-latest

steps:
- name: Run YugabyteDB 2
run: |
docker run -p 5433:5433 -e YSQL_USER=yugabyte -e YSQL_PASSWORD=yugabyte -d yugabytedb/yugabyte:2.20.4.0-b50 bin/yugabyted start --background=false --master_flag="ysql_enable_db_catalog_version_mode=false" --tserver_flags="ysql_enable_db_catalog_version_mode=false"

- uses: actions/checkout@v4

- name: Set up JDK ${{ env.JAVA_VERSION }} (${{ env.JAVA_VENDOR }})
uses: actions/setup-java@v4
with:
java-version: ${{ env.JAVA_VERSION }}
distribution: ${{ env.JAVA_VENDOR }}

- name: Setup Gradle
uses: gradle/actions/setup-gradle@v4

- name: Execute Gradle 'integrationTestJdbcPermission' task
run: ./gradlew integrationTestJdbcPermission -Dscalardb.jdbc.url=jdbc:yugabytedb://localhost:5433/yugabyte?load-balance=any -Dscalardb.jdbc.username=yugabyte -Dscalardb.jdbc.password=yugabyte -Dscalar.db.jdbc.connection_pool.max_total=12 -Dscalar.db.jdbc.table_metadata.connection_pool.max_total=4 -Dscalar.db.jdbc.admin.connection_pool.max_total=4

- name: Upload Gradle test reports
if: always()
uses: actions/upload-artifact@v4
with:
name: yugabytedb_2_permission_integration_test_reports
path: core/build/reports/tests/integrationTestJdbcPermission

integration-test-permission-jdbc-db2-11-5:
name: Db2 11.5 Permission Integration Test
runs-on: ubuntu-latest

services:
db2:
image: icr.io/db2_community/db2:11.5.9.0
env:
DB2INSTANCE: db2inst1
DB2INST1_PASSWORD: db2inst1
DBNAME: test_db
LICENSE: accept
ports:
- 50000:50000
options: --privileged --name db2

steps:
- uses: actions/checkout@v4

- name: Set up JDK ${{ env.JAVA_VERSION }} (${{ env.JAVA_VENDOR }})
uses: actions/setup-java@v4
with:
java-version: ${{ env.JAVA_VERSION }}
distribution: ${{ env.JAVA_VENDOR }}

- name: Setup Gradle
uses: gradle/actions/setup-gradle@v4

- name: Wait for the container to be ready
timeout-minutes: 10
run: |
while ! docker logs db2 2>&1 | grep -q "Setup has completed"
do
echo "Container is not yet ready"
sleep 5s
done
echo "Container is ready"

- name: Create OS user for Db2
run: |
docker exec db2 bash -c "useradd -m -s /bin/bash test"
docker exec db2 bash -c "echo \"test:test\" | sudo chpasswd"
docker exec db2 bash -c "usermod -aG db2iadm1 test"

- name: Execute Gradle 'integrationTestJdbcPermission' task
run: ./gradlew integrationTestJdbcPermission -Dscalardb.jdbc.url="jdbc:db2://localhost:50000/test_db" -Dscalardb.jdbc.username=db2inst1 -Dscalardb.jdbc.password=db2inst1

- name: Upload Gradle test reports
if: always()
uses: actions/upload-artifact@v4
with:
name: db2_11.5_permission_integration_test_reports
path: core/build/reports/tests/integrationTestJdbcPermission

integration-test-permission-jdbc-db2-12-1:
name: Db2 12.1 Permission Integration Test
runs-on: ubuntu-latest

services:
db2:
image: icr.io/db2_community/db2:12.1.1.0
env:
DB2INSTANCE: db2inst1
DB2INST1_PASSWORD: db2inst1
DBNAME: test_db
LICENSE: accept
ports:
- 50000:50000
options: --privileged --name db2

steps:
- uses: actions/checkout@v4

- name: Set up JDK ${{ env.JAVA_VERSION }} (${{ env.JAVA_VENDOR }})
uses: actions/setup-java@v4
with:
java-version: ${{ env.JAVA_VERSION }}
distribution: ${{ env.JAVA_VENDOR }}

- name: Setup Gradle
uses: gradle/actions/setup-gradle@v4

- name: Wait for the container to be ready
timeout-minutes: 10
run: |
while ! docker logs db2 2>&1 | grep -q "Setup has completed"
do
echo "Container is not yet ready"
sleep 5s
done
echo "Container is ready"

- name: Create OS user for Db2
run: |
docker exec db2 bash -c "useradd -m -s /bin/bash test"
docker exec db2 bash -c "echo \"test:test\" | sudo chpasswd"
docker exec db2 bash -c "usermod -aG db2iadm1 test"

- name: Execute Gradle 'integrationTestJdbcPermission' task
run: ./gradlew integrationTestJdbcPermission -Dscalardb.jdbc.url="jdbc:db2://localhost:50000/test_db" -Dscalardb.jdbc.username=db2inst1 -Dscalardb.jdbc.password=db2inst1

- name: Upload Gradle test reports
if: always()
uses: actions/upload-artifact@v4
with:
name: db2_12.1_permission_integration_test_reports
path: core/build/reports/tests/integrationTestJdbcPermission
2 changes: 1 addition & 1 deletion ...gration-test/java/com/scalar/db/storage/cassandra/CassandraPermissionIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ protected Map<String, String> getCreationOptions() {
}

@Override
protected void waitForTableCreation() {
protected void waitForDdlCompletion() {
try {
AdminTestUtils utils = getAdminTestUtils(TEST_NAME);
int retryCount = 0;
Expand Down
44 changes: 43 additions & 1 deletion .../integration-test/java/com/scalar/db/storage/jdbc/JdbcAdminPermissionIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,24 @@
package com.scalar.db.storage.jdbc;

import static com.scalar.db.storage.jdbc.JdbcPermissionTestUtils.DDL_WAIT_SECONDS;

import com.google.common.util.concurrent.Uninterruptibles;
import com.scalar.db.api.DistributedStorageAdminPermissionIntegrationTestBase;
import com.scalar.db.config.DatabaseConfig;
import com.scalar.db.util.AdminTestUtils;
import com.scalar.db.util.PermissionTestUtils;
import java.util.Properties;
import java.util.concurrent.TimeUnit;

public class JdbcAdminPermissionIntegrationTest
extends DistributedStorageAdminPermissionIntegrationTestBase {
private RdbEngineStrategy rdbEngine;

@Override
protected Properties getProperties(String testName) {
return JdbcEnv.getProperties(testName);
Properties properties = JdbcEnv.getProperties(testName);
rdbEngine = RdbEngineFactory.create(new JdbcConfig(new DatabaseConfig(properties)));
return properties;
Comment on lines 18 to +21
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Aug 5, 2025

Choose a reason for hiding this comment

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

medium

The rdbEngine field is re-initialized every time getProperties() is called. This method can be invoked multiple times during the test setup, leading to unnecessary object creation. Consider initializing rdbEngine only once using a null check.

Properties properties = JdbcEnv.getProperties(testName);
if (rdbEngine == null) {
  rdbEngine = RdbEngineFactory.create(new JdbcConfig(new DatabaseConfig(properties)));
}
return properties;

}

@Override
Expand All @@ -26,4 +35,37 @@ protected AdminTestUtils getAdminTestUtils(String testName) {
protected PermissionTestUtils getPermissionTestUtils(String testName) {
return new JdbcPermissionTestUtils(getProperties(testName));
}

@Override
protected void waitForTableCreation() {
waitForDdlCompletion();
}

@Override
protected void waitForNamespaceCreation() {
waitForDdlCompletion();
}

@Override
protected void waitForTableDeletion() {
waitForDdlCompletion();
}

@Override
protected void waitForNamespaceDeletion() {
waitForDdlCompletion();
}

@Override
protected void sleepBetweenTests() {
// Sleep to ensure the DDL operations executed as ACT are completed before the next setup.
waitForDdlCompletion();
}

private void waitForDdlCompletion() {
if (JdbcTestUtils.isYugabyte(rdbEngine)) {
// This is needed to avoid schema or catalog version mismatch database errors.
Uninterruptibles.sleepUninterruptibly(DDL_WAIT_SECONDS, TimeUnit.SECONDS);
}
}
}
19 changes: 18 additions & 1 deletion core/src/integration-test/java/com/scalar/db/storage/jdbc/JdbcPermissionIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,23 @@
package com.scalar.db.storage.jdbc;

import static com.scalar.db.storage.jdbc.JdbcPermissionTestUtils.DDL_WAIT_SECONDS;

import com.google.common.util.concurrent.Uninterruptibles;
import com.scalar.db.api.DistributedStoragePermissionIntegrationTestBase;
import com.scalar.db.config.DatabaseConfig;
import com.scalar.db.util.AdminTestUtils;
import com.scalar.db.util.PermissionTestUtils;
import java.util.Properties;
import java.util.concurrent.TimeUnit;

public class JdbcPermissionIntegrationTest extends DistributedStoragePermissionIntegrationTestBase {
private RdbEngineStrategy rdbEngine;

@Override
protected Properties getProperties(String testName) {
return JdbcEnv.getProperties(testName);
Properties properties = JdbcEnv.getProperties(testName);
rdbEngine = RdbEngineFactory.create(new JdbcConfig(new DatabaseConfig(properties)));
return properties;
Comment on lines 17 to +20
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Aug 5, 2025

Choose a reason for hiding this comment

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

medium

The rdbEngine field is re-initialized every time getProperties() is called. This method can be invoked multiple times during the test setup, leading to unnecessary object creation. Consider initializing rdbEngine only once using a null check.

Properties properties = JdbcEnv.getProperties(testName);
if (rdbEngine == null) {
  rdbEngine = RdbEngineFactory.create(new JdbcConfig(new DatabaseConfig(properties)));
}
return properties;

}

@Override
Expand All @@ -25,4 +34,12 @@ protected PermissionTestUtils getPermissionTestUtils(String testName) {
protected AdminTestUtils getAdminTestUtils(String testName) {
return new JdbcAdminTestUtils(getProperties(testName));
}

@Override
protected void waitForDdlCompletion() {
if (JdbcTestUtils.isYugabyte(rdbEngine)) {
// This is needed to avoid schema or catalog version mismatch database errors.
Uninterruptibles.sleepUninterruptibly(DDL_WAIT_SECONDS, TimeUnit.SECONDS);
}
}
}
34 changes: 22 additions & 12 deletions core/src/integration-test/java/com/scalar/db/storage/jdbc/JdbcPermissionTestUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import org.apache.commons.dbcp2.BasicDataSource;

public class JdbcPermissionTestUtils implements PermissionTestUtils {
public static final int DDL_WAIT_SECONDS = 1;
private final RdbEngineStrategy rdbEngine;
private final BasicDataSource dataSource;

Expand All @@ -20,25 +21,29 @@ public JdbcPermissionTestUtils(Properties properties) {

@Override
public void createNormalUser(String userName, String password) {
try (Connection connection = dataSource.getConnection()) {
String createUserSql = getCreateUserSql(userName, password);
try (Statement statement = connection.createStatement()) {
statement.execute(createUserSql);
if (!JdbcTestUtils.isDb2(rdbEngine)) {
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Aug 5, 2025

Choose a reason for hiding this comment

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

critical

The method JdbcTestUtils.isDb2(rdbEngine) is used here and in other places in this file, but it doesn't seem to be defined in JdbcTestUtils based on the provided file context. Similarly, RdbEngineFactory doesn't appear to support DB2.

This will cause a compilation error. It seems that some related changes for DB2 support might be missing from this backport. Please verify that all necessary dependencies and definitions for DB2 are included.

try (Connection connection = dataSource.getConnection()) {
String createUserSql = getCreateUserSql(userName, password);
try (Statement statement = connection.createStatement()) {
statement.execute(createUserSql);
}
} catch (SQLException e) {
throw new RuntimeException("Failed to create user: " + userName, e);
}
} catch (SQLException e) {
throw new RuntimeException("Failed to create user: " + userName, e);
}
}

@Override
public void dropNormalUser(String userName) {
try (Connection connection = dataSource.getConnection()) {
String dropUserSql = getDropUserSql(userName);
try (Statement statement = connection.createStatement()) {
statement.execute(dropUserSql);
if (!JdbcTestUtils.isDb2(rdbEngine)) {
try (Connection connection = dataSource.getConnection()) {
String dropUserSql = getDropUserSql(userName);
try (Statement statement = connection.createStatement()) {
statement.execute(dropUserSql);
}
} catch (SQLException e) {
throw new RuntimeException("Failed to drop user: " + userName, e);
}
} catch (SQLException e) {
throw new RuntimeException("Failed to drop user: " + userName, e);
}
}

Expand Down Expand Up @@ -128,6 +133,11 @@ private String[] getGrantPermissionStatements(String userName) {
String.format("ALTER ROLE [db_datareader] ADD MEMBER %s", userName),
String.format("ALTER ROLE [db_datawriter] ADD MEMBER %s", userName)
};
} else if (JdbcTestUtils.isDb2(rdbEngine)) {
return new String[] {
String.format("GRANT DBADM ON DATABASE TO USER %s", userName),
String.format("GRANT DATAACCESS ON DATABASE TO USER %s", userName)
};
} else {
throw new UnsupportedOperationException(
"Granting permissions is not supported for " + rdbEngine);
Expand Down
2 changes: 1 addition & 1 deletion ...src/main/java/com/scalar/db/api/DistributedStorageAdminPermissionIntegrationTestBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public abstract class DistributedStorageAdminPermissionIntegrationTestBase {
TableMetadata.newBuilder()
.addColumn(COL_NAME1, DataType.INT)
.addColumn(COL_NAME2, DataType.TEXT)
.addColumn(COL_NAME3, DataType.TEXT)
.addColumn(COL_NAME3, DataType.INT)
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Aug 5, 2025

Choose a reason for hiding this comment

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

high

The column COL_NAME3 is being redefined from TEXT to INT. Ensure that this change is intentional and does not break compatibility or existing data.

.addColumn(COL_NAME4, DataType.INT)
.addPartitionKey(COL_NAME1)
.addClusteringKey(COL_NAME2, Scan.Ordering.Order.ASC)
Expand Down
Loading
Loading