[Backport 2.x] Forward port integ test fixes to main branch (#4867) (#4868)

cwperks · opensearch-trigger-bot[bot] · github-actions[bot] · web-flow · commit 5d67fd2f250d · 2024-11-01T10:26:15.000-04:00
Signed-off-by: Craig Perkins &lt;cwperx@amazon.com&gt;
Signed-off-by: github-actions[bot] &lt;github-actions[bot]@users.noreply.github.com&gt;
Co-authored-by: opensearch-trigger-bot[bot] &lt;98922864+opensearch-trigger-bot[bot]@users.noreply.github.com&gt;
Co-authored-by: github-actions[bot] &lt;github-actions[bot]@users.noreply.github.com&gt;
diff --git a/README.md b/README.md
@@ -76,6 +76,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
@@ -569,7 +569,11 @@ task integrationTest(type: Test) {
     }
 }
 
-tasks.integTest.dependsOn(integrationTest)
+tasks.named("integrationTest") {
+    minHeapSize = "512m"
+    maxHeapSize = "2g"
+}
+
 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
@@ -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
diff --git 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
@@ -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<String, Object> clusterSettings = ImmutableMap.builder();
-
     protected static TestSecurityConfig testSecurityConfig = new TestSecurityConfig();
 
     public static LocalCluster localCluster;
 
-    @BeforeClass
-    public static void startCluster() throws IOException {
+    private Class<? extends AbstractApiIntegrationTest> 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<String, Object> getClusterSettings() {
+        Map<String, Object> 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
@@ -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<String, Object> getClusterSettings() {
+        Map<String, Object> 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
@@ -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<String, Object> getClusterSettings() {
+        Map<String, Object> 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
@@ -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<String, Object> getClusterSettings() {
+        Map<String, Object> 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
@@ -12,6 +12,7 @@
 package org.opensearch.security.api;
 
 import java.util.List;
+import java.util.Map;
 
 import org.junit.Test;
 
@@ -32,15 +33,21 @@ 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")
             )
         );
     }
 
+    @Override
+    protected Map<String, Object> getClusterSettings() {
+        Map<String, Object> 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
@@ -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<String, Object> getClusterSettings() {
+        Map<String, Object> 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
@@ -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<String, Object> getClusterSettings() {
+        Map<String, Object> 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
diff --git a/src/test/java/org/opensearch/security/SystemIntegratorsTests.java b/src/test/java/org/opensearch/security/SystemIntegratorsTests.java
diff --git a/src/test/java/org/opensearch/security/system_indices/AbstractSystemIndicesTests.java b/src/test/java/org/opensearch/security/system_indices/AbstractSystemIndicesTests.java

Original file line number	Diff line number	Diff line change
`@@ -569,7 +569,11 @@ task integrationTest(type: Test) {`
`569`	`569`	`}`
`570`	`570`	`}`
`571`	`571`
`572`		`-tasks.integTest.dependsOn(integrationTest)`
	`572`	`+tasks.named("integrationTest") {`
	`573`	`+ minHeapSize = "512m"`
	`574`	`+ maxHeapSize = "2g"`
	`575`	`+}`
	`576`	`+`
`573`	`577`	`tasks.integrationTest.finalizedBy(jacocoTestReport) // report is always generated after integration tests run`
`574`	`578`
`575`	`579`	`//run the integrationTest task before the check task`