Refactor plugin system index tests to use parameterized test pattern

CHANGELOG.md

@@ -24,6 +24,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Fix test failure related to change in core to add content-encoding to response headers ([#5897](https://github.com/opensearch-project/security/pull/5897))
 
 ### Refactoring
+- Refactor plugin system index tests to use parameterized test pattern ([#5895](https://github.com/opensearch-project/security/pull/5895))
 
 ### Maintenance
 - Bump `spring_version` from 7.0.1 to 7.0.2 ([#5852](https://github.com/opensearch-project/security/pull/5852))

src/integrationTest/java/org/opensearch/security/privileges/int_tests/AbstractPluginSystemIndexIntTests.java → src/integrationTest/java/org/opensearch/security/privileges/int_tests/PluginSystemIndexIntTests.java

@@ -9,10 +9,20 @@
  */
 package org.opensearch.security.privileges.int_tests;
 
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+
 import org.junit.Before;
+import org.junit.ClassRule;
 import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
 
 import org.opensearch.core.rest.RestStatus;
+import org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin1;
+import org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin2;
 import org.opensearch.test.framework.cluster.LocalCluster;
 import org.opensearch.test.framework.cluster.TestRestClient;
 import org.opensearch.test.framework.cluster.TestRestClient.HttpResponse;
@@ -23,35 +33,58 @@
 import static org.hamcrest.Matchers.equalTo;
 import static org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin1.SYSTEM_INDEX_1;
 import static org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin2.SYSTEM_INDEX_2;
+import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL;
 import static org.opensearch.test.framework.TestSecurityConfig.User.USER_ADMIN;
 
-/**
- * Abstract base class defining the contract for plugin system index access privileges.
- * <p>
+/*
  * This class serves as a single source of truth for the privilege rules governing plugin users'
  * access to their own system indices. It enforces consistency across different cluster configurations
  * (e.g., with and without explicit system index permission enabled) by defining the complete set of
  * tests that must pass in any environment.
- * <p>
- * Concrete implementations, such as {@link PluginSystemIndexPermissionEnabledIntTests} and
- * {@link PluginSystemIndexPermissionDisabledIntTests}, provide the specific {@link LocalCluster}
- * environments in which these contract tests are executed. This design leverages polymorphism
- * to dynamically bind the test logic to different runtime configurations.
  */
-public abstract class AbstractPluginSystemIndexIntTests {
+@RunWith(Parameterized.class)
+public class PluginSystemIndexIntTests {
+
+    final LocalCluster cluster;
+    final ClusterConfig clusterConfig;
+
+    static LocalCluster.Builder clusterBuilder() {
+        return new LocalCluster.Builder().singleNode()
+            .authc(AUTHC_HTTPBASIC_INTERNAL)
+            .users(USER_ADMIN)
+            .plugin(SystemIndexPlugin1.class, SystemIndexPlugin2.class);
+    }
+
+    @ClassRule
+    public static final ClusterConfig.ClusterInstances clusters = new ClusterConfig.ClusterInstances(
+        PluginSystemIndexIntTests::clusterBuilder
+    );
 
-    protected abstract LocalCluster getCluster();
+    @Parameters(name = "{0}")
+    public static Collection<Object[]> params() {
+        List<Object[]> result = new ArrayList<>();
+
+        for (ClusterConfig clusterConfig : ClusterConfig.values()) {
+            result.add(new Object[] { clusterConfig });
+        }
+        return result;
+    }
+
+    public PluginSystemIndexIntTests(ClusterConfig clusterConfig) {
+        this.clusterConfig = clusterConfig;
+        this.cluster = clusters.get(clusterConfig);
+    }
 
     @Before
     public void setup() {
-        try (TestRestClient client = getCluster().getRestClient(getCluster().getAdminCertificate())) {
+        try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) {
             client.delete(SYSTEM_INDEX_1);
         }
     }
 
     @Test
     public void testPluginShouldBeAbleToIndexDocumentIntoItsSystemIndex() {
-        try (TestRestClient client = getCluster().getRestClient(USER_ADMIN)) {
+        try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
             HttpResponse response = client.put("try-create-and-index/" + SYSTEM_INDEX_1);
 
             assertThat(response.getStatusCode(), equalTo(RestStatus.OK.getStatus()));
@@ -61,7 +94,7 @@ public void testPluginShouldBeAbleToIndexDocumentIntoItsSystemIndex() {
 
     @Test
     public void testPluginShouldNotBeAbleToIndexDocumentIntoSystemIndexRegisteredByOtherPlugin() {
-        try (TestRestClient client = getCluster().getRestClient(USER_ADMIN)) {
+        try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
             HttpResponse response = client.put("try-create-and-index/" + SYSTEM_INDEX_2);
 
             assertThat(
@@ -76,7 +109,7 @@ public void testPluginShouldNotBeAbleToIndexDocumentIntoSystemIndexRegisteredByO
 
     @Test
     public void testPluginShouldNotBeAbleToRunClusterActions() {
-        try (TestRestClient client = getCluster().getRestClient(USER_ADMIN)) {
+        try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
             HttpResponse response = client.get("try-cluster-health/plugin");
 
             assertThat(
@@ -91,7 +124,7 @@ public void testPluginShouldNotBeAbleToRunClusterActions() {
 
     @Test
     public void testPluginShouldBeAbleToCreateSystemIndexButUserShouldNotBeAbleToIndex() {
-        try (TestRestClient client = getCluster().getRestClient(USER_ADMIN)) {
+        try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
             HttpResponse response = client.put("try-create-and-index/" + SYSTEM_INDEX_1 + "?runAs=user");
 
             assertThat(response, RestMatchers.isForbidden("/error/root_cause/0/reason", "no permissions for [] and User [name=admin"));
@@ -100,7 +133,7 @@ public void testPluginShouldBeAbleToCreateSystemIndexButUserShouldNotBeAbleToInd
 
     @Test
     public void testPluginShouldBeAbleToBulkIndexDocumentIntoItsSystemIndex() {
-        try (TestRestClient client = getCluster().getRestClient(USER_ADMIN)) {
+        try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
             HttpResponse response = client.put("try-create-and-bulk-index/" + SYSTEM_INDEX_1);
 
             assertThat(response.getStatusCode(), equalTo(RestStatus.OK.getStatus()));
@@ -109,7 +142,7 @@ public void testPluginShouldBeAbleToBulkIndexDocumentIntoItsSystemIndex() {
 
     @Test
     public void testPluginShouldBeAbleSearchOnItsSystemIndex() {
-        try (TestRestClient client = getCluster().getRestClient(USER_ADMIN)) {
+        try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
             HttpResponse response = client.put("try-create-and-bulk-index/" + SYSTEM_INDEX_1);
 
             assertThat(response.getStatusCode(), equalTo(RestStatus.OK.getStatus()));
@@ -123,7 +156,7 @@ public void testPluginShouldBeAbleSearchOnItsSystemIndex() {
 
     @Test
     public void testPluginShouldBeAbleGetOnItsSystemIndex() {
-        try (TestRestClient client = getCluster().getRestClient(USER_ADMIN)) {
+        try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
             HttpResponse response = client.put("try-create-and-bulk-index/" + SYSTEM_INDEX_1);
 
             assertThat(response.getStatusCode(), equalTo(RestStatus.OK.getStatus()));
@@ -143,7 +176,7 @@ public void testPluginShouldBeAbleGetOnItsSystemIndex() {
 
     @Test
     public void testPluginShouldBeAbleUpdateOnItsSystemIndex() {
-        try (TestRestClient client = getCluster().getRestClient(USER_ADMIN)) {
+        try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
             HttpResponse response = client.put("try-create-and-bulk-index/" + SYSTEM_INDEX_1);
 
             assertThat(response.getStatusCode(), equalTo(RestStatus.OK.getStatus()));
@@ -163,11 +196,11 @@ public void testPluginShouldBeAbleUpdateOnItsSystemIndex() {
 
     @Test
     public void testPluginShouldNotBeAbleToBulkIndexDocumentIntoMixOfSystemIndexWhereAtLeastOneDoesNotBelongToPlugin() {
-        try (TestRestClient client = getCluster().getRestClient(getCluster().getAdminCertificate())) {
+        try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) {
             client.put(SYSTEM_INDEX_1);
             client.put(SYSTEM_INDEX_2);
         }
-        try (TestRestClient client = getCluster().getRestClient(USER_ADMIN)) {
+        try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
             HttpResponse response = client.put("try-create-and-bulk-mixed-index");
 
             assertThat(
@@ -181,11 +214,11 @@ public void testPluginShouldNotBeAbleToBulkIndexDocumentIntoMixOfSystemIndexWher
 
     @Test
     public void testPluginShouldNotBeAbleToSearchOnMixOfSystemIndexWhereAtLeastOneDoesNotBelongToPlugin() {
-        try (TestRestClient client = getCluster().getRestClient(getCluster().getAdminCertificate())) {
+        try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) {
             client.put(SYSTEM_INDEX_1);
             client.put(SYSTEM_INDEX_2);
         }
-        try (TestRestClient client = getCluster().getRestClient(USER_ADMIN)) {
+        try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
             HttpResponse response = client.get("search-on-mixed-system-index");
 
             assertThat(