From 926dc78ab3eef473e96d5a14289e049a7d8ea246 Mon Sep 17 00:00:00 2001 From: llilyy Date: Mon, 5 Jan 2026 20:46:35 +0900 Subject: [PATCH 1/4] Refactor plugin system index tests to use parameterized test pattern Signed-off-by: llilyy --- CHANGELOG.md | 1 + ...ts.java => PluginSystemIndexIntTests.java} | 85 +++++++++++++------ ...SystemIndexPermissionDisabledIntTests.java | 56 ------------ ...nSystemIndexPermissionEnabledIntTests.java | 58 ------------- 4 files changed, 58 insertions(+), 142 deletions(-) rename src/integrationTest/java/org/opensearch/security/privileges/int_tests/{AbstractPluginSystemIndexIntTests.java => PluginSystemIndexIntTests.java} (71%) delete mode 100644 src/integrationTest/java/org/opensearch/security/privileges/int_tests/PluginSystemIndexPermissionDisabledIntTests.java delete mode 100644 src/integrationTest/java/org/opensearch/security/privileges/int_tests/PluginSystemIndexPermissionEnabledIntTests.java diff --git a/CHANGELOG.md b/CHANGELOG.md index ee9bf1fb44..68b7d1d4b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Skip hasExplicitIndexPrivilege check for plugin users accessing their own system indices ([#5858](https://github.com/opensearch-project/security/pull/5858)) ### 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)) diff --git a/src/integrationTest/java/org/opensearch/security/privileges/int_tests/AbstractPluginSystemIndexIntTests.java b/src/integrationTest/java/org/opensearch/security/privileges/int_tests/PluginSystemIndexIntTests.java similarity index 71% rename from src/integrationTest/java/org/opensearch/security/privileges/int_tests/AbstractPluginSystemIndexIntTests.java rename to src/integrationTest/java/org/opensearch/security/privileges/int_tests/PluginSystemIndexIntTests.java index 4b78fd8d14..7e6e24966b 100644 --- a/src/integrationTest/java/org/opensearch/security/privileges/int_tests/AbstractPluginSystemIndexIntTests.java +++ b/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.AfterClass; import org.junit.Before; 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,54 @@ 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. - *

- * 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. - *

- * 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 { - protected abstract LocalCluster getCluster(); + 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); + } + + @AfterClass + public static void stopClusters() { + for (ClusterConfig clusterConfig : ClusterConfig.values()) { + clusterConfig.shutdown(); + } + } + + @Parameters(name = "{0}") + public static Collection params() { + List result = new ArrayList<>(); + + for (ClusterConfig clusterConfig : ClusterConfig.values()) { + result.add(new Object[] { clusterConfig }); + } + return result; + } + + public PluginSystemIndexIntTests(ClusterConfig clusterConfig) { + this.clusterConfig = clusterConfig; + this.cluster = clusterConfig.cluster(PluginSystemIndexIntTests::clusterBuilder); + } @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 +90,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 +105,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 +120,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 +129,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 +138,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 +152,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 +172,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 +192,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 +210,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( diff --git a/src/integrationTest/java/org/opensearch/security/privileges/int_tests/PluginSystemIndexPermissionDisabledIntTests.java b/src/integrationTest/java/org/opensearch/security/privileges/int_tests/PluginSystemIndexPermissionDisabledIntTests.java deleted file mode 100644 index 37b8a938db..0000000000 --- a/src/integrationTest/java/org/opensearch/security/privileges/int_tests/PluginSystemIndexPermissionDisabledIntTests.java +++ /dev/null @@ -1,56 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - * - */ -package org.opensearch.security.privileges.int_tests; - -import java.util.List; -import java.util.Map; - -import org.junit.ClassRule; - -import org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin1; -import org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin2; -import org.opensearch.test.framework.TestSecurityConfig.AuthcDomain; -import org.opensearch.test.framework.cluster.ClusterManager; -import org.opensearch.test.framework.cluster.LocalCluster; - -import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ROLES_ENABLED; -import static org.opensearch.security.support.ConfigConstants.SECURITY_SYSTEM_INDICES_ENABLED_KEY; -import static org.opensearch.test.framework.TestSecurityConfig.Role.ALL_ACCESS; -import static org.opensearch.test.framework.TestSecurityConfig.User.USER_ADMIN; - -/** - * plugins.security.system_indices.permission.enabled is not set or false - */ -public class PluginSystemIndexPermissionDisabledIntTests extends AbstractPluginSystemIndexIntTests { - - public static final AuthcDomain AUTHC_DOMAIN = new AuthcDomain("basic", 0).httpAuthenticatorWithChallenge("basic").backend("internal"); - - @ClassRule - public static final LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.SINGLENODE) - .anonymousAuth(false) - .authc(AUTHC_DOMAIN) - .users(USER_ADMIN) - .plugin(SystemIndexPlugin1.class, SystemIndexPlugin2.class) - .nodeSettings( - Map.of( - SECURITY_RESTAPI_ROLES_ENABLED, - List.of("user_" + USER_ADMIN.getName() + "__" + ALL_ACCESS.getName()), - SECURITY_SYSTEM_INDICES_ENABLED_KEY, - true - // SECURITY_SYSTEM_INDICES_PERMISSIONS_ENABLED_KEY is not set (defaults to false) - ) - ) - .build(); - - @Override - protected LocalCluster getCluster() { - return cluster; - } -} diff --git a/src/integrationTest/java/org/opensearch/security/privileges/int_tests/PluginSystemIndexPermissionEnabledIntTests.java b/src/integrationTest/java/org/opensearch/security/privileges/int_tests/PluginSystemIndexPermissionEnabledIntTests.java deleted file mode 100644 index 001fd2c007..0000000000 --- a/src/integrationTest/java/org/opensearch/security/privileges/int_tests/PluginSystemIndexPermissionEnabledIntTests.java +++ /dev/null @@ -1,58 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - * - */ -package org.opensearch.security.privileges.int_tests; - -import java.util.List; -import java.util.Map; - -import org.junit.ClassRule; - -import org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin1; -import org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin2; -import org.opensearch.test.framework.TestSecurityConfig.AuthcDomain; -import org.opensearch.test.framework.cluster.ClusterManager; -import org.opensearch.test.framework.cluster.LocalCluster; - -import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ROLES_ENABLED; -import static org.opensearch.security.support.ConfigConstants.SECURITY_SYSTEM_INDICES_ENABLED_KEY; -import static org.opensearch.security.support.ConfigConstants.SECURITY_SYSTEM_INDICES_PERMISSIONS_ENABLED_KEY; -import static org.opensearch.test.framework.TestSecurityConfig.Role.ALL_ACCESS; -import static org.opensearch.test.framework.TestSecurityConfig.User.USER_ADMIN; - -/** - * plugins.security.system_indices.permission.enabled = true - */ -public class PluginSystemIndexPermissionEnabledIntTests extends AbstractPluginSystemIndexIntTests { - - public static final AuthcDomain AUTHC_DOMAIN = new AuthcDomain("basic", 0).httpAuthenticatorWithChallenge("basic").backend("internal"); - - @ClassRule - public static final LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.SINGLENODE) - .anonymousAuth(false) - .authc(AUTHC_DOMAIN) - .users(USER_ADMIN) - .plugin(SystemIndexPlugin1.class, SystemIndexPlugin2.class) - .nodeSettings( - Map.of( - SECURITY_RESTAPI_ROLES_ENABLED, - List.of("user_" + USER_ADMIN.getName() + "__" + ALL_ACCESS.getName()), - SECURITY_SYSTEM_INDICES_ENABLED_KEY, - true, - SECURITY_SYSTEM_INDICES_PERMISSIONS_ENABLED_KEY, - true - ) - ) - .build(); - - @Override - protected LocalCluster getCluster() { - return cluster; - } -} From acae5f151981117670b1dc9dd98625c5c52b3307 Mon Sep 17 00:00:00 2001 From: llilyy Date: Thu, 8 Jan 2026 11:24:00 +0900 Subject: [PATCH 2/4] fix break Signed-off-by: llilyy --- .../int_tests/PluginSystemIndexIntTests.java | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/integrationTest/java/org/opensearch/security/privileges/int_tests/PluginSystemIndexIntTests.java b/src/integrationTest/java/org/opensearch/security/privileges/int_tests/PluginSystemIndexIntTests.java index 7e6e24966b..5540907543 100644 --- a/src/integrationTest/java/org/opensearch/security/privileges/int_tests/PluginSystemIndexIntTests.java +++ b/src/integrationTest/java/org/opensearch/security/privileges/int_tests/PluginSystemIndexIntTests.java @@ -13,8 +13,8 @@ import java.util.Collection; import java.util.List; -import org.junit.AfterClass; import org.junit.Before; +import org.junit.ClassRule; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; @@ -49,12 +49,10 @@ static LocalCluster.Builder clusterBuilder() { .plugin(SystemIndexPlugin1.class, SystemIndexPlugin2.class); } - @AfterClass - public static void stopClusters() { - for (ClusterConfig clusterConfig : ClusterConfig.values()) { - clusterConfig.shutdown(); - } - } + @ClassRule + public static final ClusterConfig.ClusterInstances clusters = new ClusterConfig.ClusterInstances( + PluginSystemIndexIntTests::clusterBuilder + ); @Parameters(name = "{0}") public static Collection params() { @@ -68,7 +66,7 @@ public static Collection params() { public PluginSystemIndexIntTests(ClusterConfig clusterConfig) { this.clusterConfig = clusterConfig; - this.cluster = clusterConfig.cluster(PluginSystemIndexIntTests::clusterBuilder); + this.cluster = clusters.get(clusterConfig); } @Before From 7bf674e0d32c693c384cb693939c7540a29386e9 Mon Sep 17 00:00:00 2001 From: llilyy Date: Thu, 8 Jan 2026 11:27:09 +0900 Subject: [PATCH 3/4] spotlessApply Signed-off-by: llilyy --- .../privileges/int_tests/PluginSystemIndexIntTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/integrationTest/java/org/opensearch/security/privileges/int_tests/PluginSystemIndexIntTests.java b/src/integrationTest/java/org/opensearch/security/privileges/int_tests/PluginSystemIndexIntTests.java index 5540907543..e2de6e7409 100644 --- a/src/integrationTest/java/org/opensearch/security/privileges/int_tests/PluginSystemIndexIntTests.java +++ b/src/integrationTest/java/org/opensearch/security/privileges/int_tests/PluginSystemIndexIntTests.java @@ -51,7 +51,7 @@ static LocalCluster.Builder clusterBuilder() { @ClassRule public static final ClusterConfig.ClusterInstances clusters = new ClusterConfig.ClusterInstances( - PluginSystemIndexIntTests::clusterBuilder + PluginSystemIndexIntTests::clusterBuilder ); @Parameters(name = "{0}") From 81e617cc0c7b10db3a1c2dd0ef0f4246e8341849 Mon Sep 17 00:00:00 2001 From: llilyy Date: Fri, 9 Jan 2026 10:02:20 +0900 Subject: [PATCH 4/4] add javadoc Signed-off-by: llilyy --- .../privileges/int_tests/PluginSystemIndexIntTests.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/integrationTest/java/org/opensearch/security/privileges/int_tests/PluginSystemIndexIntTests.java b/src/integrationTest/java/org/opensearch/security/privileges/int_tests/PluginSystemIndexIntTests.java index e2de6e7409..e626c57ecf 100644 --- a/src/integrationTest/java/org/opensearch/security/privileges/int_tests/PluginSystemIndexIntTests.java +++ b/src/integrationTest/java/org/opensearch/security/privileges/int_tests/PluginSystemIndexIntTests.java @@ -36,6 +36,12 @@ import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL; import static org.opensearch.test.framework.TestSecurityConfig.User.USER_ADMIN; +/* + * 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. + */ @RunWith(Parameterized.class) public class PluginSystemIndexIntTests {