Backport to branch(3.13) : Add Cassandra permission test #2837

feeblefakie · 2025-07-08T05:47:47Z

This is an automated backport of the following:

Original PR: Add Cassandra permission test #2822
Backported commit: 29e0a41

Please merge this PR after all checks have passed.

gemini-code-assist

Summary of Changes

Hello @feeblefakie, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request backports a crucial set of integration tests designed to verify Cassandra's permission handling capabilities. It establishes a robust, extensible framework for testing user privileges across various storage operations and administrative tasks, ensuring that our Cassandra integration correctly enforces security policies.

Highlights

New Permission Testing Framework: I've introduced abstract base classes (DistributedStorageAdminPermissionIntegrationTestBase, DistributedStoragePermissionIntegrationTestBase) and a utility interface (PermissionTestUtils) to standardize how we test user permissions across different storage backends.
Cassandra-Specific Permission Tests: I've added concrete integration tests (CassandraPermissionIntegrationTest, CassandraAdminPermissionIntegrationTest) and a Cassandra-specific utility (CassandraPermissionTestUtils) to thoroughly validate permission enforcement for both data manipulation and administrative operations in Cassandra.
Build Configuration Update: I've modified core/build.gradle to define a dedicated source set and Gradle task (integrationTestCassandraPermission) for these new Cassandra permission tests, ensuring they are run separately and reliably.
Enhanced Cassandra Test Environment Setup: I've updated CassandraEnv to support the creation and use of a 'normal user' for permission testing, including retry logic in the Cassandra-specific tests to account for eventual consistency in schema changes.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request backports the addition of Cassandra permission tests. The changes involve adding new integration test configurations in Gradle, new base test classes, and new Cassandra-specific test implementations. I've identified a few areas for improvement, primarily concerning potential resource leaks in test code and a security vulnerability related to CQL injection in test utilities.

gemini-code-assist · 2025-07-08T05:51:34Z

...ntegration-test/java/com/scalar/db/storage/cassandra/CassandraPermissionIntegrationTest.java

+  protected void waitForTableCreation() {
+    try {
+      AdminTestUtils utils = getAdminTestUtils(TEST_NAME);
+      int retryCount = 0;
+      while (retryCount < MAX_RETRY_COUNT) {
+        if (utils.tableExists(NAMESPACE, TABLE)) {
+          utils.close();
+          return;
+        }
+        Uninterruptibles.sleepUninterruptibly(SLEEP_BETWEEN_RETRIES_SECONDS, TimeUnit.SECONDS);
+        retryCount++;
+      }
+      utils.close();
+      throw new RuntimeException("Table was not created after " + MAX_RETRY_COUNT + " retries");
+    } catch (Exception e) {
+      throw new RuntimeException("Failed to wait for table creation", e);
+    }
+  }


The AdminTestUtils instance utils is not closed in a finally block. If utils.tableExists() throws an exception, utils.close() will not be called, leading to a resource leak. This can cause test flakiness.

try { AdminTestUtils utils = getAdminTestUtils(TEST_NAME); try { int retryCount = 0; while (retryCount < MAX_RETRY_COUNT) { if (utils.tableExists(NAMESPACE, TABLE)) { return; } Uninterruptibles.sleepUninterruptibly(SLEEP_BETWEEN_RETRIES_SECONDS, TimeUnit.SECONDS); retryCount++; } throw new RuntimeException("Table was not created after " + MAX_RETRY_COUNT + " retries"); } finally { utils.close(); } } catch (Exception e) { throw new RuntimeException("Failed to wait for table creation", e); }

gemini-code-assist · 2025-07-08T05:51:34Z

...ation-test/java/com/scalar/db/storage/cassandra/CassandraAdminPermissionIntegrationTest.java

+  protected void waitForNamespaceCreation() {
+    try {
+      AdminTestUtils utils = getAdminTestUtils(TEST_NAME);
+      int retryCount = 0;
+      while (retryCount < MAX_RETRY_COUNT) {
+        if (utils.namespaceExists(NAMESPACE)) {
+          utils.close();
+          return;
+        }
+        Uninterruptibles.sleepUninterruptibly(
+            SLEEP_BETWEEN_RETRIES_SECONDS, java.util.concurrent.TimeUnit.SECONDS);
+        retryCount++;
+      }
+      utils.close();
+      throw new RuntimeException("Namespace was not created after " + MAX_RETRY_COUNT + " retries");
+    } catch (Exception e) {
+      throw new RuntimeException("Failed to wait for namespace creation", e);
+    }
+  }


The AdminTestUtils instance utils is not closed in a finally block. If an exception is thrown from methods like utils.namespaceExists(), the utils.close() call will be skipped, leading to a resource leak. This can cause test flakiness.

try { AdminTestUtils utils = getAdminTestUtils(TEST_NAME); try { int retryCount = 0; while (retryCount < MAX_RETRY_COUNT) { if (utils.namespaceExists(NAMESPACE)) { return; } Uninterruptibles.sleepUninterruptibly( SLEEP_BETWEEN_RETRIES_SECONDS, java.util.concurrent.TimeUnit.SECONDS); retryCount++; } throw new RuntimeException("Namespace was not created after " + MAX_RETRY_COUNT + " retries"); } finally { utils.close(); } } catch (Exception e) { throw new RuntimeException("Failed to wait for namespace creation", e); }

gemini-code-assist · 2025-07-08T05:51:34Z

...ation-test/java/com/scalar/db/storage/cassandra/CassandraAdminPermissionIntegrationTest.java

+  protected void waitForTableCreation() {
+    try {
+      AdminTestUtils utils = getAdminTestUtils(TEST_NAME);
+      int retryCount = 0;
+      while (retryCount < MAX_RETRY_COUNT) {
+        if (utils.tableExists(NAMESPACE, TABLE)) {
+          utils.close();
+          return;
+        }
+        Uninterruptibles.sleepUninterruptibly(
+            SLEEP_BETWEEN_RETRIES_SECONDS, java.util.concurrent.TimeUnit.SECONDS);
+        retryCount++;
+      }
+      utils.close();
+      throw new RuntimeException("Table was not created after " + MAX_RETRY_COUNT + " retries");
+    } catch (Exception e) {
+      throw new RuntimeException("Failed to wait for table creation", e);
+    }
+  }


The AdminTestUtils instance utils is not closed in a finally block. If an exception is thrown from methods like utils.tableExists(), the utils.close() call will be skipped, leading to a resource leak. This can cause test flakiness.

try { AdminTestUtils utils = getAdminTestUtils(TEST_NAME); try { int retryCount = 0; while (retryCount < MAX_RETRY_COUNT) { if (utils.tableExists(NAMESPACE, TABLE)) { return; } Uninterruptibles.sleepUninterruptibly( SLEEP_BETWEEN_RETRIES_SECONDS, java.util.concurrent.TimeUnit.SECONDS); retryCount++; } throw new RuntimeException("Table was not created after " + MAX_RETRY_COUNT + " retries"); } finally { utils.close(); } } catch (Exception e) { throw new RuntimeException("Failed to wait for table creation", e); }

gemini-code-assist · 2025-07-08T05:51:34Z

...ation-test/java/com/scalar/db/storage/cassandra/CassandraAdminPermissionIntegrationTest.java

+  protected void waitForNamespaceDeletion() {
+    try {
+      AdminTestUtils utils = getAdminTestUtils(TEST_NAME);
+      int retryCount = 0;
+      while (retryCount < MAX_RETRY_COUNT) {
+        if (!utils.namespaceExists(NAMESPACE)) {
+          utils.close();
+          return;
+        }
+        Uninterruptibles.sleepUninterruptibly(
+            SLEEP_BETWEEN_RETRIES_SECONDS, java.util.concurrent.TimeUnit.SECONDS);
+        retryCount++;
+      }
+      utils.close();
+      throw new RuntimeException("Namespace was not deleted after " + MAX_RETRY_COUNT + " retries");
+    } catch (Exception e) {
+      throw new RuntimeException("Failed to wait for namespace deletion", e);
+    }
+  }


The AdminTestUtils instance utils is not closed in a finally block. If an exception is thrown from methods like utils.namespaceExists(), the utils.close() call will be skipped, leading to a resource leak. This can cause test flakiness.

try { AdminTestUtils utils = getAdminTestUtils(TEST_NAME); try { int retryCount = 0; while (retryCount < MAX_RETRY_COUNT) { if (!utils.namespaceExists(NAMESPACE)) { return; } Uninterruptibles.sleepUninterruptibly( SLEEP_BETWEEN_RETRIES_SECONDS, java.util.concurrent.TimeUnit.SECONDS); retryCount++; } throw new RuntimeException("Namespace was not deleted after " + MAX_RETRY_COUNT + " retries"); } finally { utils.close(); } } catch (Exception e) { throw new RuntimeException("Failed to wait for namespace deletion", e); }

gemini-code-assist · 2025-07-08T05:51:34Z

...ation-test/java/com/scalar/db/storage/cassandra/CassandraAdminPermissionIntegrationTest.java

+  @Override
+  protected void waitForTableDeletion() {
+    try {
+      AdminTestUtils utils = getAdminTestUtils(TEST_NAME);
+      int retryCount = 0;
+      while (retryCount < MAX_RETRY_COUNT) {
+        if (!utils.tableExists(NAMESPACE, TABLE)) {
+          utils.close();
+          return;
+        }
+        Uninterruptibles.sleepUninterruptibly(
+            SLEEP_BETWEEN_RETRIES_SECONDS, java.util.concurrent.TimeUnit.SECONDS);
+        retryCount++;
+      }
+      utils.close();
+      throw new RuntimeException("Table was not deleted after " + MAX_RETRY_COUNT + " retries");
+    } catch (Exception e) {
+      throw new RuntimeException("Failed to wait for table deletion", e);
+    }
+  }


The AdminTestUtils instance utils is not closed in a finally block. If an exception is thrown from methods like utils.tableExists(), the utils.close() call will be skipped, leading to a resource leak. This can cause test flakiness.

try { AdminTestUtils utils = getAdminTestUtils(TEST_NAME); try { int retryCount = 0; while (retryCount < MAX_RETRY_COUNT) { if (!utils.tableExists(NAMESPACE, TABLE)) { return; } Uninterruptibles.sleepUninterruptibly( SLEEP_BETWEEN_RETRIES_SECONDS, java.util.concurrent.TimeUnit.SECONDS); retryCount++; } throw new RuntimeException("Table was not deleted after " + MAX_RETRY_COUNT + " retries"); } finally { utils.close(); } } catch (Exception e) { throw new RuntimeException("Failed to wait for table deletion", e); }

gemini-code-assist · 2025-07-08T05:51:34Z

.../src/integration-test/java/com/scalar/db/storage/cassandra/CassandraPermissionTestUtils.java

+  public void createNormalUser(String userName, String password) {
+    clusterManager
+        .getSession()
+        .execute(
+            String.format(
+                "CREATE ROLE %s WITH PASSWORD = '%s' AND LOGIN = true", userName, password));
+  }
+
+  @Override
+  public void dropNormalUser(String userName) {
+    clusterManager.getSession().execute(String.format("DROP ROLE %s", userName));
+  }
+
+  @Override
+  public void grantRequiredPermission(String userName) {
+    Session session = clusterManager.getSession();
+    for (String grantStatement : getGrantPermissionStatements(userName)) {
+      session.execute(grantStatement);
+    }
+  }
+
+  private String[] getGrantPermissionStatements(String userName) {
+    return new String[] {
+      String.format("GRANT CREATE ON ALL KEYSPACES TO %s", userName),
+      String.format("GRANT DROP ON ALL KEYSPACES TO %s", userName),
+      String.format("GRANT ALTER ON ALL KEYSPACES TO %s", userName),
+      String.format("GRANT SELECT ON ALL KEYSPACES TO %s", userName),
+      String.format("GRANT MODIFY ON ALL KEYSPACES TO %s", userName)
+    };
+  }


The methods createNormalUser, dropNormalUser, and grantRequiredPermission use String.format to construct CQL queries with user-provided userName and password. This is susceptible to CQL injection if the inputs contain malicious characters.

Add Cassandra permission test (#2822)

6a1d306

feeblefakie assigned KodaiD Jul 8, 2025

gemini-code-assist bot reviewed Jul 8, 2025

View reviewed changes

KodaiD and others added 3 commits July 10, 2025 18:02

Update AdminTestUtils

5ee0940

Fix ServerAdminTestUtils

3245dcc

Merge branch '3.13' into 3.13-pull-2822

bed54a9

brfrn169 merged commit 3746f1b into 3.13 Jul 11, 2025
52 checks passed

brfrn169 deleted the 3.13-pull-2822 branch July 11, 2025 05:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Backport to branch(3.13) : Add Cassandra permission test #2837

Backport to branch(3.13) : Add Cassandra permission test #2837

Uh oh!

feeblefakie commented Jul 8, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Jul 8, 2025

Uh oh!

gemini-code-assist bot Jul 8, 2025

Uh oh!

gemini-code-assist bot Jul 8, 2025

Uh oh!

gemini-code-assist bot Jul 8, 2025

Uh oh!

gemini-code-assist bot Jul 8, 2025

Uh oh!

gemini-code-assist bot Jul 8, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Backport to branch(3.13) : Add Cassandra permission test #2837

Backport to branch(3.13) : Add Cassandra permission test #2837

Uh oh!

Conversation

feeblefakie commented Jul 8, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants