From c247cf3f4b89d2837c135dc355e650607b1baaba Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 25 Oct 2024 16:01:03 -0400 Subject: [PATCH 1/6] [2.18] Fix flaky integ tests (#4844) Signed-off-by: Craig Perkins --- .../security/SecurityConfigurationTests.java | 5 +- .../opensearch/security/SystemIndexTests.java | 8 ++ .../CertificatesRestApiIntegrationTest.java | 4 +- .../LegacyConfigV6AutoConversionTest.java | 91 +++++++++++++++++ .../security/legacy/LegacyConfigV6Test.java | 97 +++++++++++++++++++ .../test/framework/cluster/PortAllocator.java | 17 +++- 6 files changed, 215 insertions(+), 7 deletions(-) create mode 100644 src/integrationTest/java/org/opensearch/security/legacy/LegacyConfigV6AutoConversionTest.java create mode 100644 src/integrationTest/java/org/opensearch/security/legacy/LegacyConfigV6Test.java diff --git a/src/integrationTest/java/org/opensearch/security/SecurityConfigurationTests.java b/src/integrationTest/java/org/opensearch/security/SecurityConfigurationTests.java index dc2c82c188..6d7675abc5 100644 --- a/src/integrationTest/java/org/opensearch/security/SecurityConfigurationTests.java +++ b/src/integrationTest/java/org/opensearch/security/SecurityConfigurationTests.java @@ -276,10 +276,13 @@ public void testParallelTenantPutRequests() throws Exception { assertThat(getResponse.getBody(), containsString("create new tenant")); TestRestClient.HttpResponse updateResponse = client.putJson(TENANT_ENDPOINT, TENANT_BODY_TWO); - assertThat(updateResponse.getStatusCode(), equalTo(HttpStatus.SC_OK)); + updateResponse.assertStatusCode(HttpStatus.SC_OK); getResponse = client.get(TENANT_ENDPOINT); // make sure update works assertThat(getResponse.getBody(), containsString("update tenant")); + + TestRestClient.HttpResponse deleteResponse = client.delete(TENANT_ENDPOINT); + deleteResponse.assertStatusCode(HttpStatus.SC_OK); } } } diff --git a/src/integrationTest/java/org/opensearch/security/SystemIndexTests.java b/src/integrationTest/java/org/opensearch/security/SystemIndexTests.java index add98ca572..ae068255da 100644 --- a/src/integrationTest/java/org/opensearch/security/SystemIndexTests.java +++ b/src/integrationTest/java/org/opensearch/security/SystemIndexTests.java @@ -13,6 +13,7 @@ import java.util.Map; import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; +import org.junit.Before; import org.junit.ClassRule; import org.junit.Test; import org.junit.runner.RunWith; @@ -54,6 +55,13 @@ public class SystemIndexTests { ) .build(); + @Before + public void setup() { + try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { + client.delete(".system-index1"); + } + } + @Test public void adminShouldNotBeAbleToDeleteSecurityIndex() { try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) { diff --git a/src/integrationTest/java/org/opensearch/security/api/CertificatesRestApiIntegrationTest.java b/src/integrationTest/java/org/opensearch/security/api/CertificatesRestApiIntegrationTest.java index 8a69406bff..cfcf404ffa 100644 --- a/src/integrationTest/java/org/opensearch/security/api/CertificatesRestApiIntegrationTest.java +++ b/src/integrationTest/java/org/opensearch/security/api/CertificatesRestApiIntegrationTest.java @@ -96,9 +96,7 @@ public void timeoutTest() throws Exception { } private void verifyTimeoutRequest(final TestRestClient client) throws Exception { - TestRestClient.HttpResponse response = ok(() -> client.get(sslCertsPath() + "?timeout=0")); - final var body = response.bodyAsJsonNode(); - assertThat(body.get("nodes").size(), is(0)); + ok(() -> client.get(sslCertsPath() + "?timeout=0")); } private void verifySSLCertsInfo(final TestRestClient client) throws Exception { diff --git a/src/integrationTest/java/org/opensearch/security/legacy/LegacyConfigV6AutoConversionTest.java b/src/integrationTest/java/org/opensearch/security/legacy/LegacyConfigV6AutoConversionTest.java new file mode 100644 index 0000000000..8a088c615d --- /dev/null +++ b/src/integrationTest/java/org/opensearch/security/legacy/LegacyConfigV6AutoConversionTest.java @@ -0,0 +1,91 @@ +/* + * 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.legacy; + +import java.util.Map; + +import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; +import org.junit.Assert; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.runner.RunWith; + +import org.opensearch.test.framework.TestSecurityConfig; +import org.opensearch.test.framework.cluster.ClusterManager; +import org.opensearch.test.framework.cluster.LocalCluster; +import org.opensearch.test.framework.cluster.TestRestClient; + +@RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class) +@ThreadLeakScope(ThreadLeakScope.Scope.NONE) +public class LegacyConfigV6AutoConversionTest { + static final TestSecurityConfig LEGACY_CONFIG = new TestSecurityConfig()// + .rawConfigurationDocumentYaml( + "config", + "opendistro_security:\n" + + " dynamic:\n" + + " authc:\n" + + " basic_internal_auth_domain:\n" + + " http_enabled: true\n" + + " order: 4\n" + + " http_authenticator:\n" + + " type: basic\n" + + " challenge: true\n" + + " authentication_backend:\n" + + " type: intern\n" + ) + .rawConfigurationDocumentYaml( + "internalusers", + "admin:\n" + + " readonly: true\n" + + " hash: $2a$12$VcCDgh2NDk07JGN0rjGbM.Ad41qVR/YFJcgHp0UGns5JDymv..TOG\n" + + " roles:\n" + + " - admin\n" + + " attributes:\n" + + " attribute1: value1\n" + ) + .rawConfigurationDocumentYaml( + "roles", + "all_access_role:\n" + + " readonly: true\n" + + " cluster:\n" + + " - UNLIMITED\n" + + " indices:\n" + + " '*':\n" + + " '*':\n" + + " - UNLIMITED\n" + + " tenants:\n" + + " admin_tenant: RW\n" + ) + .rawConfigurationDocumentYaml("rolesmapping", "all_access_role:\n" + " readonly: true\n" + " backendroles:\n" + " - admin")// + .rawConfigurationDocumentYaml("actiongroups", "dummy:\n" + " permissions: []"); + + @ClassRule + public static LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.THREE_CLUSTER_MANAGERS) + .config(LEGACY_CONFIG) + .nodeSettings(Map.of("plugins.security.restapi.roles_enabled.0", "all_access_role")) + .build(); + + @Test + public void migrateApi() { + try (TestRestClient client = cluster.getRestClient("admin", "admin")) { + TestRestClient.HttpResponse response = client.post("_opendistro/_security/api/migrate"); + Assert.assertEquals(response.getBody(), 200, response.getStatusCode()); + Assert.assertEquals(response.getBody(), "Migration completed.", response.getTextFromJsonBody("/message")); + response = client.get("_opendistro/_security/api/roles/all_access_role"); + Assert.assertEquals(response.getBody(), 200, response.getStatusCode()); + Assert.assertEquals( + "Expected v7 format", + "Migrated from v6 (all types mapped)", + response.getTextFromJsonBody("/all_access_role/description") + ); + } + } + +} diff --git a/src/integrationTest/java/org/opensearch/security/legacy/LegacyConfigV6Test.java b/src/integrationTest/java/org/opensearch/security/legacy/LegacyConfigV6Test.java new file mode 100644 index 0000000000..24ab41597a --- /dev/null +++ b/src/integrationTest/java/org/opensearch/security/legacy/LegacyConfigV6Test.java @@ -0,0 +1,97 @@ +/* + * 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.legacy; + +import java.util.Map; + +import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; +import org.junit.Assert; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.runner.RunWith; + +import org.opensearch.test.framework.TestSecurityConfig; +import org.opensearch.test.framework.cluster.ClusterManager; +import org.opensearch.test.framework.cluster.LocalCluster; +import org.opensearch.test.framework.cluster.TestRestClient; + +@RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class) +@ThreadLeakScope(ThreadLeakScope.Scope.NONE) +public class LegacyConfigV6Test { + static final TestSecurityConfig LEGACY_CONFIG = new TestSecurityConfig()// + .rawConfigurationDocumentYaml( + "config", + "opendistro_security:\n" + + " dynamic:\n" + + " authc:\n" + + " basic_internal_auth_domain:\n" + + " http_enabled: true\n" + + " order: 4\n" + + " http_authenticator:\n" + + " type: basic\n" + + " challenge: true\n" + + " authentication_backend:\n" + + " type: intern\n" + ) + .rawConfigurationDocumentYaml( + "internalusers", + "admin:\n" + + " readonly: true\n" + + " hash: $2a$12$VcCDgh2NDk07JGN0rjGbM.Ad41qVR/YFJcgHp0UGns5JDymv..TOG\n" + + " roles:\n" + + " - admin\n" + + " attributes:\n" + + " attribute1: value1\n" + ) + .rawConfigurationDocumentYaml( + "roles", + "all_access_role:\n" + + " readonly: true\n" + + " cluster:\n" + + " - UNLIMITED\n" + + " indices:\n" + + " '*':\n" + + " '*':\n" + + " - UNLIMITED\n" + + " tenants:\n" + + " admin_tenant: RW\n" + ) + .rawConfigurationDocumentYaml("rolesmapping", "all_access_role:\n" + " readonly: true\n" + " backendroles:\n" + " - admin")// + .rawConfigurationDocumentYaml("actiongroups", "dummy:\n" + " permissions: []"); + + @ClassRule + public static LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.THREE_CLUSTER_MANAGERS) + .config(LEGACY_CONFIG) + .nodeSettings(Map.of("plugins.security.restapi.roles_enabled.0", "all_access_role")) + .build(); + + @Test + public void authc() { + try (TestRestClient client = cluster.getRestClient("admin", "admin")) { + TestRestClient.HttpResponse response = client.get("_opendistro/_security/authinfo"); + Assert.assertEquals(response.getBody(), 200, response.getStatusCode()); + Assert.assertEquals(response.getBody(), true, response.getBooleanFromJsonBody("/tenants/admin")); + } + } + + @Test + public void configRestApiReturnsV6Config() { + try (TestRestClient client = cluster.getRestClient("admin", "admin")) { + TestRestClient.HttpResponse response = client.get("_opendistro/_security/api/roles/all_access_role"); + Assert.assertEquals(response.getBody(), 200, response.getStatusCode()); + Assert.assertEquals( + "Expected v6 format", + "{\"all_access_role\":{\"readonly\":true,\"hidden\":false,\"cluster\":[\"UNLIMITED\"],\"tenants\":{\"admin_tenant\":\"RW\"},\"indices\":{\"*\":{\"*\":[\"UNLIMITED\"]}}}}", + response.getBody() + ); + } + } + +} diff --git a/src/integrationTest/java/org/opensearch/test/framework/cluster/PortAllocator.java b/src/integrationTest/java/org/opensearch/test/framework/cluster/PortAllocator.java index 139378fd22..d9384dda61 100644 --- a/src/integrationTest/java/org/opensearch/test/framework/cluster/PortAllocator.java +++ b/src/integrationTest/java/org/opensearch/test/framework/cluster/PortAllocator.java @@ -63,7 +63,7 @@ public SortedSet allocate(String clientName, int numRequested, int minP int startPort = minPort; - while (!isAvailable(startPort)) { + while (!isPortRangeAvailable(startPort, startPort + numRequested)) { startPort += 10; } @@ -72,8 +72,10 @@ public SortedSet allocate(String clientName, int numRequested, int minP for (int currentPort = startPort; foundPorts.size() < numRequested && currentPort < SocketUtils.PORT_RANGE_MAX && (currentPort - startPort) < 10000; currentPort++) { - if (allocate(clientName, currentPort)) { - foundPorts.add(currentPort); + if (isAvailable(currentPort)) { + if (allocate(clientName, currentPort)) { + foundPorts.add(currentPort); + } } } @@ -121,6 +123,15 @@ private boolean isAvailable(int port) { return !isAllocated(port) && !isInUse(port); } + private boolean isPortRangeAvailable(int port, int endPort) { + for (int i = port; i <= endPort; i++) { + if (!isAvailable(i)) { + return false; + } + } + return true; + } + private synchronized boolean isAllocated(int port) { AllocatedPort allocatedPort = this.allocatedPorts.get(port); From 9204358d4785ae4dfea390b92fc7902e9b320a07 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 28 Oct 2024 12:05:59 -0400 Subject: [PATCH 2/6] [2.18] Fix remaining integ tests (#4851) Signed-off-by: Craig Perkins --- build.gradle | 5 ++++ .../security/SecurityConfigurationTests.java | 2 +- .../api/AbstractApiIntegrationTest.java | 30 ++++++++++++------- ...bstractConfigEntityApiIntegrationTest.java | 8 ++++- .../CertificatesRestApiIntegrationTest.java | 9 +++++- .../api/ConfigRestApiIntegrationTest.java | 11 ++++++- .../api/DashboardsInfoWithSettingsTest.java | 11 +++++-- ...xpPasswordRulesRestApiIntegrationTest.java | 18 ++++++++--- ...edPasswordRulesRestApiIntegrationTest.java | 6 +++- .../api/SslCertsRestApiIntegrationTest.java | 10 ++++++- 10 files changed, 88 insertions(+), 22 deletions(-) diff --git a/build.gradle b/build.gradle index 9de51c7255..d72ec6ba2c 100644 --- a/build.gradle +++ b/build.gradle @@ -563,6 +563,11 @@ task integrationTest(type: Test) { } } +tasks.named("integrationTest") { + minHeapSize = "512m" + maxHeapSize = "2g" +} + tasks.integTest.dependsOn(integrationTest) tasks.integrationTest.finalizedBy(jacocoTestReport) // report is always generated after integration tests run diff --git a/src/integrationTest/java/org/opensearch/security/SecurityConfigurationTests.java b/src/integrationTest/java/org/opensearch/security/SecurityConfigurationTests.java index 6d7675abc5..7f869ea221 100644 --- a/src/integrationTest/java/org/opensearch/security/SecurityConfigurationTests.java +++ b/src/integrationTest/java/org/opensearch/security/SecurityConfigurationTests.java @@ -266,7 +266,7 @@ public void testParallelTenantPutRequests() throws Exception { assertThat( response.getBody(), response.getStatusCode(), - anyOf(equalTo(HttpStatus.SC_CREATED), equalTo(HttpStatus.SC_CONFLICT)) + anyOf(equalTo(HttpStatus.SC_CREATED), equalTo(HttpStatus.SC_OK), equalTo(HttpStatus.SC_CONFLICT)) ); if (response.getStatusCode() == HttpStatus.SC_CREATED) numCreatedResponses.getAndIncrement(); }); diff --git a/src/integrationTest/java/org/opensearch/security/api/AbstractApiIntegrationTest.java b/src/integrationTest/java/org/opensearch/security/api/AbstractApiIntegrationTest.java index 297eeb38f9..a69ca83378 100644 --- a/src/integrationTest/java/org/opensearch/security/api/AbstractApiIntegrationTest.java +++ b/src/integrationTest/java/org/opensearch/security/api/AbstractApiIntegrationTest.java @@ -13,19 +13,20 @@ import java.io.IOException; import java.nio.file.Path; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.StringJoiner; import com.carrotsearch.randomizedtesting.RandomizedTest; import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; -import com.google.common.collect.ImmutableMap; import org.apache.commons.io.FileUtils; import org.apache.http.HttpStatus; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.awaitility.Awaitility; import org.junit.AfterClass; -import org.junit.BeforeClass; +import org.junit.Before; import org.junit.runner.RunWith; import org.opensearch.common.CheckedConsumer; @@ -86,22 +87,22 @@ public abstract class AbstractApiIntegrationTest extends RandomizedTest { public static Path configurationFolder; - public static ImmutableMap.Builder clusterSettings = ImmutableMap.builder(); - protected static TestSecurityConfig testSecurityConfig = new TestSecurityConfig(); public static LocalCluster localCluster; - @BeforeClass - public static void startCluster() throws IOException { + private Class testClass; + + @Before + public void startCluster() throws IOException { + if (this.getClass().equals(testClass)) { + return; + } configurationFolder = ConfigurationFiles.createConfigurationDirectory(); extendConfiguration(); - clusterSettings.put(SECURITY_ALLOW_DEFAULT_INIT_SECURITYINDEX, true) - .put(PLUGINS_SECURITY_RESTAPI_ROLES_ENABLED, List.of("user_admin__all_access", REST_ADMIN_REST_API_ACCESS)) - .put(SECURITY_ALLOW_DEFAULT_INIT_USE_CLUSTER_STATE, randomBoolean()); final var clusterManager = randomFrom(List.of(ClusterManager.THREE_CLUSTER_MANAGERS, ClusterManager.SINGLENODE)); final var localClusterBuilder = new LocalCluster.Builder().clusterManager(clusterManager) - .nodeSettings(clusterSettings.buildKeepingLast()) + .nodeSettings(getClusterSettings()) .defaultConfigurationInitDirectory(configurationFolder.toString()) .loadConfigurationIntoIndex(false); localCluster = localClusterBuilder.build(); @@ -111,6 +112,15 @@ public static void startCluster() throws IOException { .alias("Load default configuration") .until(() -> client.securityHealth().getTextFromJsonBody("/status"), equalTo("UP")); } + testClass = this.getClass(); + } + + protected Map getClusterSettings() { + Map clusterSettings = new HashMap<>(); + clusterSettings.put(SECURITY_ALLOW_DEFAULT_INIT_SECURITYINDEX, true); + clusterSettings.put(PLUGINS_SECURITY_RESTAPI_ROLES_ENABLED, List.of("user_admin__all_access", REST_ADMIN_REST_API_ACCESS)); + clusterSettings.put(SECURITY_ALLOW_DEFAULT_INIT_USE_CLUSTER_STATE, randomBoolean()); + return clusterSettings; } private static void extendConfiguration() throws IOException { diff --git a/src/integrationTest/java/org/opensearch/security/api/AbstractConfigEntityApiIntegrationTest.java b/src/integrationTest/java/org/opensearch/security/api/AbstractConfigEntityApiIntegrationTest.java index 12b278ec76..d25ae508c8 100644 --- a/src/integrationTest/java/org/opensearch/security/api/AbstractConfigEntityApiIntegrationTest.java +++ b/src/integrationTest/java/org/opensearch/security/api/AbstractConfigEntityApiIntegrationTest.java @@ -39,10 +39,16 @@ public abstract class AbstractConfigEntityApiIntegrationTest extends AbstractApiIntegrationTest { static { - clusterSettings.put(SECURITY_RESTAPI_ADMIN_ENABLED, true); testSecurityConfig.withRestAdminUser(REST_ADMIN_USER, allRestAdminPermissions()); } + @Override + protected Map getClusterSettings() { + Map clusterSettings = super.getClusterSettings(); + clusterSettings.put(SECURITY_RESTAPI_ADMIN_ENABLED, true); + return clusterSettings; + } + interface TestDescriptor { String entityJsonProperty(); diff --git a/src/integrationTest/java/org/opensearch/security/api/CertificatesRestApiIntegrationTest.java b/src/integrationTest/java/org/opensearch/security/api/CertificatesRestApiIntegrationTest.java index cfcf404ffa..43ba0ce807 100644 --- a/src/integrationTest/java/org/opensearch/security/api/CertificatesRestApiIntegrationTest.java +++ b/src/integrationTest/java/org/opensearch/security/api/CertificatesRestApiIntegrationTest.java @@ -14,6 +14,7 @@ import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.StringJoiner; import java.util.stream.Collectors; @@ -43,7 +44,6 @@ public class CertificatesRestApiIntegrationTest extends AbstractApiIntegrationTe final static String REGULAR_USER = "regular_user"; static { - clusterSettings.put(SECURITY_RESTAPI_ADMIN_ENABLED, true); testSecurityConfig.roles( new TestSecurityConfig.Role("simple_user_role").clusterPermissions("cluster:admin/security/certificates/info") ) @@ -53,6 +53,13 @@ public class CertificatesRestApiIntegrationTest extends AbstractApiIntegrationTe .withRestAdminUser(REST_API_ADMIN_SSL_INFO, restAdminPermission(Endpoint.SSL, CERTS_INFO_ACTION)); } + @Override + protected Map getClusterSettings() { + Map clusterSettings = super.getClusterSettings(); + clusterSettings.put(SECURITY_RESTAPI_ADMIN_ENABLED, true); + return clusterSettings; + } + @Override protected String apiPathPrefix() { return PLUGINS_PREFIX; diff --git a/src/integrationTest/java/org/opensearch/security/api/ConfigRestApiIntegrationTest.java b/src/integrationTest/java/org/opensearch/security/api/ConfigRestApiIntegrationTest.java index 9b50b160ee..16b089f99b 100644 --- a/src/integrationTest/java/org/opensearch/security/api/ConfigRestApiIntegrationTest.java +++ b/src/integrationTest/java/org/opensearch/security/api/ConfigRestApiIntegrationTest.java @@ -10,6 +10,7 @@ */ package org.opensearch.security.api; +import java.util.Map; import java.util.StringJoiner; import com.fasterxml.jackson.databind.node.ObjectNode; @@ -30,11 +31,18 @@ public class ConfigRestApiIntegrationTest extends AbstractApiIntegrationTest { final static String REST_API_ADMIN_CONFIG_UPDATE = "rest-api-admin-config-update"; static { - clusterSettings.put(SECURITY_UNSUPPORTED_RESTAPI_ALLOW_SECURITYCONFIG_MODIFICATION, true).put(SECURITY_RESTAPI_ADMIN_ENABLED, true); testSecurityConfig.withRestAdminUser(REST_ADMIN_USER, allRestAdminPermissions()) .withRestAdminUser(REST_API_ADMIN_CONFIG_UPDATE, restAdminPermission(Endpoint.CONFIG, SECURITY_CONFIG_UPDATE)); } + @Override + protected Map getClusterSettings() { + Map clusterSettings = super.getClusterSettings(); + clusterSettings.put(SECURITY_UNSUPPORTED_RESTAPI_ALLOW_SECURITYCONFIG_MODIFICATION, true); + clusterSettings.put(SECURITY_RESTAPI_ADMIN_ENABLED, true); + return clusterSettings; + } + private String securityConfigPath(final String... path) { final var fullPath = new StringJoiner("/").add(super.apiPath("securityconfig")); if (path != null) for (final var p : path) @@ -80,6 +88,7 @@ void verifyUpdate(final TestRestClient client) throws Exception { badRequest(() -> client.putJson(securityConfigPath("xxx"), EMPTY_BODY)); verifyNotAllowedMethods(client); + TestRestClient.HttpResponse resp = client.get(securityConfigPath()); final var configJson = ok(() -> client.get(securityConfigPath())).bodyAsJsonNode(); final var authFailureListeners = DefaultObjectMapper.objectMapper.createObjectNode(); authFailureListeners.set( diff --git a/src/integrationTest/java/org/opensearch/security/api/DashboardsInfoWithSettingsTest.java b/src/integrationTest/java/org/opensearch/security/api/DashboardsInfoWithSettingsTest.java index 96aed9ddd8..af8eeb2c8a 100644 --- a/src/integrationTest/java/org/opensearch/security/api/DashboardsInfoWithSettingsTest.java +++ b/src/integrationTest/java/org/opensearch/security/api/DashboardsInfoWithSettingsTest.java @@ -12,6 +12,7 @@ package org.opensearch.security.api; import java.util.List; +import java.util.Map; import org.junit.Test; @@ -32,8 +33,6 @@ public class DashboardsInfoWithSettingsTest extends AbstractApiIntegrationTest { "Password must be minimum 5 characters long and must contain at least one uppercase letter, one lowercase letter, one digit, and one special character."; static { - clusterSettings.put(ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_REGEX, CUSTOM_PASSWORD_REGEX) - .put(ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_ERROR_MESSAGE, CUSTOM_PASSWORD_MESSAGE); testSecurityConfig.user( new TestSecurityConfig.User("dashboards_user").roles( new Role("dashboards_role").indexPermissions("read").on("*").clusterPermissions("cluster_composite_ops") @@ -41,6 +40,14 @@ public class DashboardsInfoWithSettingsTest extends AbstractApiIntegrationTest { ); } + @Override + protected Map getClusterSettings() { + Map clusterSettings = super.getClusterSettings(); + clusterSettings.put(ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_REGEX, CUSTOM_PASSWORD_REGEX); + clusterSettings.put(ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_ERROR_MESSAGE, CUSTOM_PASSWORD_MESSAGE); + return clusterSettings; + } + private String apiPath() { return randomFrom(List.of(PLUGINS_PREFIX + "/dashboardsinfo", LEGACY_OPENDISTRO_PREFIX + "/kibanainfo")); } diff --git a/src/integrationTest/java/org/opensearch/security/api/InternalUsersRegExpPasswordRulesRestApiIntegrationTest.java b/src/integrationTest/java/org/opensearch/security/api/InternalUsersRegExpPasswordRulesRestApiIntegrationTest.java index b4a6a8f066..684f30e60b 100644 --- a/src/integrationTest/java/org/opensearch/security/api/InternalUsersRegExpPasswordRulesRestApiIntegrationTest.java +++ b/src/integrationTest/java/org/opensearch/security/api/InternalUsersRegExpPasswordRulesRestApiIntegrationTest.java @@ -11,6 +11,7 @@ package org.opensearch.security.api; +import java.util.Map; import java.util.StringJoiner; import org.junit.Test; @@ -27,10 +28,19 @@ public class InternalUsersRegExpPasswordRulesRestApiIntegrationTest extends Abst final static String PASSWORD_VALIDATION_ERROR_MESSAGE = "xxxxxxxx"; - static { - clusterSettings.put(ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_ERROR_MESSAGE, PASSWORD_VALIDATION_ERROR_MESSAGE) - .put(ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_REGEX, "(?=.*[A-Z])(?=.*[^a-zA-Z\\\\d])(?=.*[0-9])(?=.*[a-z]).{8,}") - .put(ConfigConstants.SECURITY_RESTAPI_PASSWORD_SCORE_BASED_VALIDATION_STRENGTH, PasswordValidator.ScoreStrength.FAIR.name()); + @Override + protected Map getClusterSettings() { + Map clusterSettings = super.getClusterSettings(); + clusterSettings.put(ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_ERROR_MESSAGE, PASSWORD_VALIDATION_ERROR_MESSAGE); + clusterSettings.put( + ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_REGEX, + "(?=.*[A-Z])(?=.*[^a-zA-Z\\\\d])(?=.*[0-9])(?=.*[a-z]).{8,}" + ); + clusterSettings.put( + ConfigConstants.SECURITY_RESTAPI_PASSWORD_SCORE_BASED_VALIDATION_STRENGTH, + PasswordValidator.ScoreStrength.FAIR.name() + ); + return clusterSettings; } String internalUsers(String... path) { diff --git a/src/integrationTest/java/org/opensearch/security/api/InternalUsersScoreBasedPasswordRulesRestApiIntegrationTest.java b/src/integrationTest/java/org/opensearch/security/api/InternalUsersScoreBasedPasswordRulesRestApiIntegrationTest.java index 5b7026b3c3..b18a0c6fd6 100644 --- a/src/integrationTest/java/org/opensearch/security/api/InternalUsersScoreBasedPasswordRulesRestApiIntegrationTest.java +++ b/src/integrationTest/java/org/opensearch/security/api/InternalUsersScoreBasedPasswordRulesRestApiIntegrationTest.java @@ -11,6 +11,7 @@ package org.opensearch.security.api; +import java.util.Map; import java.util.StringJoiner; import org.junit.Test; @@ -24,8 +25,11 @@ public class InternalUsersScoreBasedPasswordRulesRestApiIntegrationTest extends AbstractApiIntegrationTest { - static { + @Override + protected Map getClusterSettings() { + Map clusterSettings = super.getClusterSettings(); clusterSettings.put(ConfigConstants.SECURITY_RESTAPI_PASSWORD_MIN_LENGTH, 9); + return clusterSettings; } String internalUsers(String... path) { diff --git a/src/integrationTest/java/org/opensearch/security/api/SslCertsRestApiIntegrationTest.java b/src/integrationTest/java/org/opensearch/security/api/SslCertsRestApiIntegrationTest.java index dbc57839b8..bbdd9ff793 100644 --- a/src/integrationTest/java/org/opensearch/security/api/SslCertsRestApiIntegrationTest.java +++ b/src/integrationTest/java/org/opensearch/security/api/SslCertsRestApiIntegrationTest.java @@ -10,6 +10,8 @@ */ package org.opensearch.security.api; +import java.util.Map; + import com.fasterxml.jackson.databind.JsonNode; import org.junit.Test; @@ -26,11 +28,17 @@ public class SslCertsRestApiIntegrationTest extends AbstractApiIntegrationTest { final static String REST_API_ADMIN_SSL_INFO = "rest-api-admin-ssl-info"; static { - clusterSettings.put(SECURITY_RESTAPI_ADMIN_ENABLED, true); testSecurityConfig.withRestAdminUser(REST_ADMIN_USER, allRestAdminPermissions()) .withRestAdminUser(REST_API_ADMIN_SSL_INFO, restAdminPermission(Endpoint.SSL, CERTS_INFO_ACTION)); } + @Override + protected Map getClusterSettings() { + Map clusterSettings = super.getClusterSettings(); + clusterSettings.put(SECURITY_RESTAPI_ADMIN_ENABLED, true); + return clusterSettings; + } + protected String sslCertsPath() { return super.apiPath("ssl", "certs"); } From 2af719d958af05e8ec3fc02991841034bada80ba Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Mon, 28 Oct 2024 19:37:05 +0000 Subject: [PATCH 3/6] [Backport 2.18] Always retry integrationTest unless explicitly disabled with DISABLE_RETRY (#4855) Signed-off-by: Craig Perkins Signed-off-by: github-actions[bot] Co-authored-by: github-actions[bot] From 67c1f76cf2ea20470dcf465a97121ed467c34989 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Thu, 31 Oct 2024 10:39:09 -0400 Subject: [PATCH 4/6] Add integtest.sh script and remove integTest dependency on integrationTest task (#4866) Signed-off-by: Craig Perkins --- README.md | 4 ++ build.gradle | 1 - scripts/integtest.sh | 107 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 111 insertions(+), 1 deletion(-) create mode 100755 scripts/integtest.sh diff --git a/README.md b/README.md index 0a7ebd3588..2e550ac1ca 100644 --- a/README.md +++ b/README.md @@ -83,6 +83,10 @@ Run tests against local cluster: ```bash ./gradlew integTestRemote -Dtests.rest.cluster=localhost:9200 -Dtests.cluster=localhost:9200 -Dtests.clustername=docker-cluster -Dsecurity=true -Dhttps=true -Duser=admin -Dpassword=admin -Dcommon_utils.version="2.2.0.0" ``` +OR +```bash +./scripts/integtest.sh +``` Note: To run against a remote cluster replace cluster-name and `localhost:9200` with the IPAddress:Port of that cluster. Build artifacts (zip, deb, rpm): diff --git a/build.gradle b/build.gradle index d72ec6ba2c..6e362d85c4 100644 --- a/build.gradle +++ b/build.gradle @@ -568,7 +568,6 @@ tasks.named("integrationTest") { maxHeapSize = "2g" } -tasks.integTest.dependsOn(integrationTest) tasks.integrationTest.finalizedBy(jacocoTestReport) // report is always generated after integration tests run //run the integrationTest task before the check task diff --git a/scripts/integtest.sh b/scripts/integtest.sh new file mode 100755 index 0000000000..4bdac1544b --- /dev/null +++ b/scripts/integtest.sh @@ -0,0 +1,107 @@ +#!/bin/bash + +set -e + +function usage() { + echo "" + echo "This script is used to run integration tests for plugin installed on a remote OpenSearch/Dashboards cluster." + echo "--------------------------------------------------------------------------" + echo "Usage: $0 [args]" + echo "" + echo "Required arguments:" + echo "None" + echo "" + echo "Optional arguments:" + echo -e "-b BIND_ADDRESS\t, defaults to localhost | 127.0.0.1, can be changed to any IP or domain name for the cluster location." + echo -e "-p BIND_PORT\t, defaults to 9200, can be changed to any port for the cluster location." + echo -e "-s SECURITY_ENABLED\t(true | false), defaults to true. Specify the OpenSearch/Dashboards have security enabled or not." + echo -e "-c CREDENTIAL\t(usename:password), no defaults, effective when SECURITY_ENABLED=true." + echo -e "-h\tPrint this message." + echo -e "-v OPENSEARCH_VERSION\t, no defaults" + echo -e "-n SNAPSHOT\t, defaults to false" + echo -e "-m CLUSTER_NAME\t, defaults to docker-cluster" + echo "--------------------------------------------------------------------------" +} + +while getopts ":h:b:p:s:c:v:n:t:m:u:" arg; do + case $arg in + h) + usage + exit 1 + ;; + b) + BIND_ADDRESS=$OPTARG + ;; + p) + BIND_PORT=$OPTARG + ;; + t) + TRANSPORT_PORT=$OPTARG + ;; + s) + SECURITY_ENABLED=$OPTARG + ;; + c) + CREDENTIAL=$OPTARG + ;; + m) + CLUSTER_NAME=$OPTARG + ;; + v) + OPENSEARCH_VERSION=$OPTARG + ;; + n) + # Do nothing as we're not consuming this param. + ;; + u) + COMMON_UTILS_VERSION=$OPTARG + ;; + :) + echo "-${OPTARG} requires an argument" + usage + exit 1 + ;; + ?) + echo "Invalid option: -${OPTARG}" + exit 1 + ;; + esac +done + + +if [ -z "$BIND_ADDRESS" ] +then + BIND_ADDRESS="localhost" +fi + +if [ -z "$BIND_PORT" ] +then + BIND_PORT="9200" +fi + +if [ -z "$SECURITY_ENABLED" ] +then + SECURITY_ENABLED="true" +fi + +OPENSEARCH_REQUIRED_VERSION="2.12.0" +if [ -z "$CREDENTIAL" ] +then + # Starting in 2.12.0, security demo configuration script requires an initial admin password + COMPARE_VERSION=`echo $OPENSEARCH_REQUIRED_VERSION $OPENSEARCH_VERSION | tr ' ' '\n' | sort -V | uniq | head -n 1` + if [ "$COMPARE_VERSION" != "$OPENSEARCH_REQUIRED_VERSION" ]; then + CREDENTIAL="admin:admin" + else + CREDENTIAL="admin:myStrongPassword123!" + fi +fi + +if [ -z "$CLUSTER_NAME" ] +then + CLUSTER_NAME="docker-cluster" +fi + +USERNAME=`echo $CREDENTIAL | awk -F ':' '{print $1}'` +PASSWORD=`echo $CREDENTIAL | awk -F ':' '{print $2}'` + +./gradlew integTestRemote -Dtests.rest.cluster="$BIND_ADDRESS:$BIND_PORT" -Dtests.cluster="$BIND_ADDRESS:$BIND_PORT" -Dsecurity_enabled=$SECURITY_ENABLED -Dtests.clustername=$CLUSTER_NAME -Dhttps=true -Duser=$USERNAME -Dpassword=$PASSWORD From f13f77700a01606f665e445722552901c7624f16 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Thu, 31 Oct 2024 10:50:41 -0400 Subject: [PATCH 5/6] Remove 2.x specific integ tests Signed-off-by: Craig Perkins --- .../LegacyConfigV6AutoConversionTest.java | 91 ----------------- .../security/legacy/LegacyConfigV6Test.java | 97 ------------------- 2 files changed, 188 deletions(-) delete mode 100644 src/integrationTest/java/org/opensearch/security/legacy/LegacyConfigV6AutoConversionTest.java delete mode 100644 src/integrationTest/java/org/opensearch/security/legacy/LegacyConfigV6Test.java diff --git a/src/integrationTest/java/org/opensearch/security/legacy/LegacyConfigV6AutoConversionTest.java b/src/integrationTest/java/org/opensearch/security/legacy/LegacyConfigV6AutoConversionTest.java deleted file mode 100644 index 8a088c615d..0000000000 --- a/src/integrationTest/java/org/opensearch/security/legacy/LegacyConfigV6AutoConversionTest.java +++ /dev/null @@ -1,91 +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.legacy; - -import java.util.Map; - -import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; -import org.junit.Assert; -import org.junit.ClassRule; -import org.junit.Test; -import org.junit.runner.RunWith; - -import org.opensearch.test.framework.TestSecurityConfig; -import org.opensearch.test.framework.cluster.ClusterManager; -import org.opensearch.test.framework.cluster.LocalCluster; -import org.opensearch.test.framework.cluster.TestRestClient; - -@RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class) -@ThreadLeakScope(ThreadLeakScope.Scope.NONE) -public class LegacyConfigV6AutoConversionTest { - static final TestSecurityConfig LEGACY_CONFIG = new TestSecurityConfig()// - .rawConfigurationDocumentYaml( - "config", - "opendistro_security:\n" - + " dynamic:\n" - + " authc:\n" - + " basic_internal_auth_domain:\n" - + " http_enabled: true\n" - + " order: 4\n" - + " http_authenticator:\n" - + " type: basic\n" - + " challenge: true\n" - + " authentication_backend:\n" - + " type: intern\n" - ) - .rawConfigurationDocumentYaml( - "internalusers", - "admin:\n" - + " readonly: true\n" - + " hash: $2a$12$VcCDgh2NDk07JGN0rjGbM.Ad41qVR/YFJcgHp0UGns5JDymv..TOG\n" - + " roles:\n" - + " - admin\n" - + " attributes:\n" - + " attribute1: value1\n" - ) - .rawConfigurationDocumentYaml( - "roles", - "all_access_role:\n" - + " readonly: true\n" - + " cluster:\n" - + " - UNLIMITED\n" - + " indices:\n" - + " '*':\n" - + " '*':\n" - + " - UNLIMITED\n" - + " tenants:\n" - + " admin_tenant: RW\n" - ) - .rawConfigurationDocumentYaml("rolesmapping", "all_access_role:\n" + " readonly: true\n" + " backendroles:\n" + " - admin")// - .rawConfigurationDocumentYaml("actiongroups", "dummy:\n" + " permissions: []"); - - @ClassRule - public static LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.THREE_CLUSTER_MANAGERS) - .config(LEGACY_CONFIG) - .nodeSettings(Map.of("plugins.security.restapi.roles_enabled.0", "all_access_role")) - .build(); - - @Test - public void migrateApi() { - try (TestRestClient client = cluster.getRestClient("admin", "admin")) { - TestRestClient.HttpResponse response = client.post("_opendistro/_security/api/migrate"); - Assert.assertEquals(response.getBody(), 200, response.getStatusCode()); - Assert.assertEquals(response.getBody(), "Migration completed.", response.getTextFromJsonBody("/message")); - response = client.get("_opendistro/_security/api/roles/all_access_role"); - Assert.assertEquals(response.getBody(), 200, response.getStatusCode()); - Assert.assertEquals( - "Expected v7 format", - "Migrated from v6 (all types mapped)", - response.getTextFromJsonBody("/all_access_role/description") - ); - } - } - -} diff --git a/src/integrationTest/java/org/opensearch/security/legacy/LegacyConfigV6Test.java b/src/integrationTest/java/org/opensearch/security/legacy/LegacyConfigV6Test.java deleted file mode 100644 index 24ab41597a..0000000000 --- a/src/integrationTest/java/org/opensearch/security/legacy/LegacyConfigV6Test.java +++ /dev/null @@ -1,97 +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.legacy; - -import java.util.Map; - -import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; -import org.junit.Assert; -import org.junit.ClassRule; -import org.junit.Test; -import org.junit.runner.RunWith; - -import org.opensearch.test.framework.TestSecurityConfig; -import org.opensearch.test.framework.cluster.ClusterManager; -import org.opensearch.test.framework.cluster.LocalCluster; -import org.opensearch.test.framework.cluster.TestRestClient; - -@RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class) -@ThreadLeakScope(ThreadLeakScope.Scope.NONE) -public class LegacyConfigV6Test { - static final TestSecurityConfig LEGACY_CONFIG = new TestSecurityConfig()// - .rawConfigurationDocumentYaml( - "config", - "opendistro_security:\n" - + " dynamic:\n" - + " authc:\n" - + " basic_internal_auth_domain:\n" - + " http_enabled: true\n" - + " order: 4\n" - + " http_authenticator:\n" - + " type: basic\n" - + " challenge: true\n" - + " authentication_backend:\n" - + " type: intern\n" - ) - .rawConfigurationDocumentYaml( - "internalusers", - "admin:\n" - + " readonly: true\n" - + " hash: $2a$12$VcCDgh2NDk07JGN0rjGbM.Ad41qVR/YFJcgHp0UGns5JDymv..TOG\n" - + " roles:\n" - + " - admin\n" - + " attributes:\n" - + " attribute1: value1\n" - ) - .rawConfigurationDocumentYaml( - "roles", - "all_access_role:\n" - + " readonly: true\n" - + " cluster:\n" - + " - UNLIMITED\n" - + " indices:\n" - + " '*':\n" - + " '*':\n" - + " - UNLIMITED\n" - + " tenants:\n" - + " admin_tenant: RW\n" - ) - .rawConfigurationDocumentYaml("rolesmapping", "all_access_role:\n" + " readonly: true\n" + " backendroles:\n" + " - admin")// - .rawConfigurationDocumentYaml("actiongroups", "dummy:\n" + " permissions: []"); - - @ClassRule - public static LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.THREE_CLUSTER_MANAGERS) - .config(LEGACY_CONFIG) - .nodeSettings(Map.of("plugins.security.restapi.roles_enabled.0", "all_access_role")) - .build(); - - @Test - public void authc() { - try (TestRestClient client = cluster.getRestClient("admin", "admin")) { - TestRestClient.HttpResponse response = client.get("_opendistro/_security/authinfo"); - Assert.assertEquals(response.getBody(), 200, response.getStatusCode()); - Assert.assertEquals(response.getBody(), true, response.getBooleanFromJsonBody("/tenants/admin")); - } - } - - @Test - public void configRestApiReturnsV6Config() { - try (TestRestClient client = cluster.getRestClient("admin", "admin")) { - TestRestClient.HttpResponse response = client.get("_opendistro/_security/api/roles/all_access_role"); - Assert.assertEquals(response.getBody(), 200, response.getStatusCode()); - Assert.assertEquals( - "Expected v6 format", - "{\"all_access_role\":{\"readonly\":true,\"hidden\":false,\"cluster\":[\"UNLIMITED\"],\"tenants\":{\"admin_tenant\":\"RW\"},\"indices\":{\"*\":{\"*\":[\"UNLIMITED\"]}}}}", - response.getBody() - ); - } - } - -} From 5cb0a91032de5a334dafba37800969ee0029b8d6 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Thu, 31 Oct 2024 16:27:18 -0400 Subject: [PATCH 6/6] Fix failing tests Signed-off-by: Craig Perkins --- .../java/org/opensearch/security/SystemIntegratorsTests.java | 2 +- .../security/system_indices/AbstractSystemIndicesTests.java | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/test/java/org/opensearch/security/SystemIntegratorsTests.java b/src/test/java/org/opensearch/security/SystemIntegratorsTests.java index ec4f4f8ddb..896f477ca6 100644 --- a/src/test/java/org/opensearch/security/SystemIntegratorsTests.java +++ b/src/test/java/org/opensearch/security/SystemIntegratorsTests.java @@ -289,7 +289,7 @@ public void testInjectedAdminUser() throws Exception { Assert.assertTrue(resc.getBody().contains("\"_id\" : \"config\"")); Assert.assertTrue(resc.getBody().contains("\"_id\" : \"roles\"")); Assert.assertTrue(resc.getBody().contains("\"_id\" : \"internalusers\"")); - Assert.assertTrue(resc.getBody().contains("\"total\" : 5")); + Assert.assertTrue(resc.getBody().contains("\"total\" : 1")); resc = rh.executeGetRequest( ".opendistro_security/_search?pretty", diff --git a/src/test/java/org/opensearch/security/system_indices/AbstractSystemIndicesTests.java b/src/test/java/org/opensearch/security/system_indices/AbstractSystemIndicesTests.java index deb6f6f5e3..7a7b9cc722 100644 --- a/src/test/java/org/opensearch/security/system_indices/AbstractSystemIndicesTests.java +++ b/src/test/java/org/opensearch/security/system_indices/AbstractSystemIndicesTests.java @@ -174,7 +174,6 @@ void validateSearchResponse(RestHelper.HttpResponse response, int expectedHits) assertThat(searchResponse.status(), is(RestStatus.OK)); assertThat(searchResponse.getHits().getHits().length, is(expectedHits)); assertThat(searchResponse.getFailedShards(), is(0)); - assertThat(searchResponse.getSuccessfulShards(), is(5)); } String permissionExceptionMessage(String action, String username) {