Run IT tests with security plugin (#335) (opensearch-project#1986)

Yury-Fridlyand · acarbonetto · web-flow · commit 7e3a718f1b7d · 2023-08-21T10:25:12.000-07:00
* Run IT tests with security plugin (#335) * Add extra IT flow. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Remove unneeded files. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Typo fix. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Fix GHA matrix syntax. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Fix GHA matrix syntax. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Code clean up. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Optimize downloading. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Apply suggestions from code review Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> Co-authored-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Update integ-test/build.gradle Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> Co-authored-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Typo fix. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Rework implementation. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Address PR review. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Address PR feedback + some fixes. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> --------- Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> Co-authored-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Minor fix. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Address PR feedback. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Typo fix. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> --------- Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> Co-authored-by: Andrew Carbonetto <andrewc@bitquilltech.com>
diff --git a/.github/workflows/integ-tests-with-security.yml b/.github/workflows/integ-tests-with-security.yml
@@ -0,0 +1,43 @@
+name: Security Plugin IT
+
+on:
+  pull_request:
+  push:
+    branches-ignore:
+      - 'dependabot/**'
+    paths:
+      - 'integ-test/**'
+      - '.github/workflows/integ-tests-with-security.yml'
+
+jobs:
+  security-it:
+    strategy:
+      fail-fast: false
+      matrix:
+        os: [ ubuntu-latest, windows-latest, macos-latest ]
+        java: [ 11, 17 ]
+
+    runs-on: ${{ matrix.os }}
+
+    steps:
+    - uses: actions/checkout@v3
+
+    - name: Set up JDK ${{ matrix.java }}
+      uses: actions/setup-java@v3
+      with:
+        distribution: 'temurin'
+        java-version: ${{ matrix.java }}
+
+    - name: Build with Gradle
+      run: ./gradlew integTestWithSecurity
+
+    - name: Upload test reports
+      if: ${{ always() }}
+      uses: actions/upload-artifact@v2
+      continue-on-error: true
+      with:
+        name: test-reports-${{ matrix.os }}-${{ matrix.java }}
+        path: |
+          integ-test/build/reports/**
+          integ-test/build/testclusters/*/logs/*
+          integ-test/build/testclusters/*/config/*
diff --git a/integ-test/build.gradle b/integ-test/build.gradle
@@ -24,7 +24,10 @@
 
 import org.opensearch.gradle.test.RestIntegTestTask
 import org.opensearch.gradle.testclusters.StandaloneRestIntegTestTask
+import org.opensearch.gradle.testclusters.OpenSearchCluster
 
+import groovy.xml.XmlParser
+import java.nio.file.Paths
 import java.util.concurrent.Callable
 import java.util.stream.Collectors
 
@@ -62,6 +65,81 @@ ext {
     projectSubstitutions = [:]
     licenseFile = rootProject.file('LICENSE.TXT')
     noticeFile = rootProject.file('NOTICE')
+
+    getSecurityPluginDownloadLink = { ->
+        var repo = "https://aws.oss.sonatype.org/content/repositories/snapshots/org/opensearch/plugin/" +
+                   "opensearch-security/$opensearch_build/"
+        var metadataFile = Paths.get(projectDir.toString(), "build", "maven-metadata.xml").toAbsolutePath().toFile()
+        download.run {
+            src repo + "maven-metadata.xml"
+            dest metadataFile
+        }
+        def metadata = new XmlParser().parse(metadataFile)
+        def securitySnapshotVersion = metadata.versioning.snapshotVersions[0].snapshotVersion[0].value[0].text()
+
+        return repo + "opensearch-security-${securitySnapshotVersion}.zip"
+    }
+
+    File downloadedSecurityPlugin = null
+
+    configureSecurityPlugin = { OpenSearchCluster cluster ->
+
+        cluster.getNodes().forEach { node ->
+            var creds = node.getCredentials()
+            if (creds.isEmpty()) {
+                creds.add(Map.of('useradd', 'admin', '-p', 'admin'))
+            } else {
+                creds.get(0).putAll(Map.of('useradd', 'admin', '-p', 'admin'))
+            }
+        }
+
+        var projectAbsPath = projectDir.getAbsolutePath()
+
+        // add a check to avoid re-downloading multiple times during single test run
+        if (downloadedSecurityPlugin == null) {
+            downloadedSecurityPlugin = Paths.get(projectAbsPath, 'bin', 'opensearch-security-snapshot.zip').toFile()
+            download.run {
+                src getSecurityPluginDownloadLink()
+                dest downloadedSecurityPlugin
+            }
+        }
+
+        // Config below including files are copied from security demo configuration
+        ['esnode.pem', 'esnode-key.pem', 'root-ca.pem'].forEach { file ->
+            File local = Paths.get(projectAbsPath, 'bin', file).toFile()
+            download.run {
+                src "https://raw.githubusercontent.com/opensearch-project/security/main/bwc-test/src/test/resources/security/" + file
+                dest local
+                overwrite false
+            }
+            cluster.extraConfigFile file, local
+        }
+        [
+            // config copied from security plugin demo configuration
+            'plugins.security.ssl.transport.pemcert_filepath' : 'esnode.pem',
+            'plugins.security.ssl.transport.pemkey_filepath' : 'esnode-key.pem',
+            'plugins.security.ssl.transport.pemtrustedcas_filepath' : 'root-ca.pem',
+            'plugins.security.ssl.transport.enforce_hostname_verification' : 'false',
+            // https is disabled to simplify test debugging
+            'plugins.security.ssl.http.enabled' : 'false',
+            'plugins.security.ssl.http.pemcert_filepath' : 'esnode.pem',
+            'plugins.security.ssl.http.pemkey_filepath' : 'esnode-key.pem',
+            'plugins.security.ssl.http.pemtrustedcas_filepath' : 'root-ca.pem',
+            'plugins.security.allow_unsafe_democertificates' : 'true',
+
+            'plugins.security.allow_default_init_securityindex' : 'true',
+            'plugins.security.authcz.admin_dn' : 'CN=kirk,OU=client,O=client,L=test,C=de',
+            'plugins.security.audit.type' : 'internal_opensearch',
+            'plugins.security.enable_snapshot_restore_privilege' : 'true',
+            'plugins.security.check_snapshot_restore_write_privileges' : 'true',
+            'plugins.security.restapi.roles_enabled' : '["all_access", "security_rest_api_access"]',
+            'plugins.security.system_indices.enabled' : 'true'
+        ].forEach { name, value ->
+            cluster.setting name, value
+        }
+
+        cluster.plugin provider((Callable<RegularFile>) (() -> (RegularFile) (() -> downloadedSecurityPlugin)))
+    }
 }
 
 tasks.withType(licenseHeaders.class) {
@@ -108,6 +186,7 @@ dependencies {
     testImplementation group: 'com.h2database', name: 'h2', version: '2.2.220'
     testImplementation group: 'org.xerial', name: 'sqlite-jdbc', version: '3.41.2.2'
     testImplementation group: 'com.google.code.gson', name: 'gson', version: '2.8.9'
+    testCompileOnly 'org.apiguardian:apiguardian-api:1.1.2'
 
     // Needed for BWC tests
     zipArchive group: 'org.opensearch.plugin', name:'opensearch-sql-plugin', version: "${bwcVersion}-SNAPSHOT"
@@ -128,21 +207,28 @@ compileTestJava {
 }
 
 testClusters.all {
-    testDistribution = 'archive'
-
     // debug with command, ./gradlew opensearch-sql:run -DdebugJVM. --debug-jvm does not work with keystore.
     if (System.getProperty("debugJVM") != null) {
         jvmArgs '-agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=*:5005'
     }
 }
 
-testClusters.integTest {
-    plugin ":opensearch-sql-plugin"
-    setting "plugins.query.datasources.encryption.masterkey", "1234567812345678"
-}
-
 testClusters {
+    integTest {
+        testDistribution = 'archive'
+        plugin ":opensearch-sql-plugin"
+        setting "plugins.query.datasources.encryption.masterkey", "1234567812345678"
+    }
     remoteCluster {
+        testDistribution = 'archive'
+        plugin ":opensearch-sql-plugin"
+    }
+    integTestWithSecurity {
+        testDistribution = 'archive'
+        plugin ":opensearch-sql-plugin"
+    }
+    remoteIntegTestWithSecurity {
+        testDistribution = 'archive'
         plugin ":opensearch-sql-plugin"
     }
 }
@@ -223,6 +309,65 @@ task integJdbcTest(type: RestIntegTestTask) {
     }
 }
 
+task integTestWithSecurity(type: RestIntegTestTask) {
+    useCluster testClusters.integTestWithSecurity
+    useCluster testClusters.remoteIntegTestWithSecurity
+
+    systemProperty "cluster.names",
+        getClusters().stream().map(cluster -> cluster.getName()).collect(Collectors.joining(","))
+
+    getClusters().forEach { cluster ->
+        configureSecurityPlugin(cluster)
+    }
+
+    useJUnitPlatform()
+    dependsOn ':opensearch-sql-plugin:bundlePlugin'
+    testLogging {
+        events "passed", "skipped", "failed"
+    }
+    afterTest { desc, result ->
+        logger.quiet "${desc.className}.${desc.name}: ${result.resultType} ${(result.getEndTime() - result.getStartTime())/1000}s"
+    }
+
+    systemProperty 'tests.security.manager', 'false'
+    systemProperty 'project.root', project.projectDir.absolutePath
+
+    // Set default query size limit
+    systemProperty 'defaultQuerySizeLimit', '10000'
+
+    // Tell the test JVM if the cluster JVM is running under a debugger so that tests can use longer timeouts for
+    // requests. The 'doFirst' delays reading the debug setting on the cluster till execution time.
+    doFirst {
+        systemProperty 'cluster.debug', getDebug()
+        getClusters().forEach { cluster ->
+
+            String allTransportSocketURI = cluster.nodes.stream().flatMap { node ->
+                node.getAllTransportPortURI().stream()
+            }.collect(Collectors.joining(","))
+            String allHttpSocketURI = cluster.nodes.stream().flatMap { node ->
+                node.getAllHttpSocketURI().stream()
+            }.collect(Collectors.joining(","))
+
+            systemProperty "tests.rest.${cluster.name}.http_hosts", "${-> allHttpSocketURI}"
+            systemProperty "tests.rest.${cluster.name}.transport_hosts", "${-> allTransportSocketURI}"
+        }
+
+        systemProperty "https", "false"
+        systemProperty "user", "admin"
+        systemProperty "password", "admin"
+    }
+
+    if (System.getProperty("test.debug") != null) {
+        jvmArgs '-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=*:5005'
+    }
+
+    // NOTE: this IT config discovers only junit5 (jupiter) tests.
+    // https://github.com/opensearch-project/sql/issues/1974
+    filter {
+        includeTestsMatching 'org.opensearch.sql.security.CrossClusterSearchIT'
+    }
+}
+
 // Run PPL ITs and new, legacy and comparison SQL ITs with new SQL engine enabled
 integTest {
     useCluster testClusters.remoteCluster
@@ -305,8 +450,8 @@ integTest {
     // Exclude JDBC related tests
     exclude 'org/opensearch/sql/jdbc/**'
 
-    // Exclude this IT until running IT with security plugin enabled is ready
-    exclude 'org/opensearch/sql/ppl/CrossClusterSearchIT.class'
+    // Exclude this IT, because they executed in another task (:integTestWithSecurity)
+    exclude 'org/opensearch/sql/security/**'
 }
 
 
diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/OpenSearchSQLRestTestCase.java b/integ-test/src/test/java/org/opensearch/sql/legacy/OpenSearchSQLRestTestCase.java
@@ -5,8 +5,6 @@
 
 package org.opensearch.sql.legacy;
 
-import static java.util.Collections.unmodifiableList;
-
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
@@ -49,8 +47,22 @@
 public abstract class OpenSearchSQLRestTestCase extends OpenSearchRestTestCase {
 
   private static final Logger LOG = LogManager.getLogger();
-  public static final String REMOTE_CLUSTER = "remoteCluster";
   public static final String MATCH_ALL_REMOTE_CLUSTER = "*";
+  // Requires to insert cluster name and cluster transport address (host:port)
+  public static final String REMOTE_CLUSTER_SETTING =
+      "{"
+          + "\"persistent\": {"
+          + "  \"cluster\": {"
+          + "    \"remote\": {"
+          + "      \"%s\": {"
+          + "        \"seeds\": ["
+          + "          \"%s\""
+          + "        ]"
+          + "      }"
+          + "    }"
+          + "  }"
+          + "}"
+          + "}";
 
   private static RestClient remoteClient;
 
@@ -106,27 +118,24 @@ protected RestClient buildClient(Settings settings, HttpHost[] hosts) throws IOE
   }
 
   // Modified from initClient in OpenSearchRestTestCase
-  public void initRemoteClient() throws IOException {
-    if (remoteClient == null) {
-      assert remoteAdminClient == null;
-      String cluster = getTestRestCluster(REMOTE_CLUSTER);
-      String[] stringUrls = cluster.split(",");
-      List<HttpHost> hosts = new ArrayList<>(stringUrls.length);
-      for (String stringUrl : stringUrls) {
-        int portSeparator = stringUrl.lastIndexOf(':');
-        if (portSeparator < 0) {
-          throw new IllegalArgumentException("Illegal cluster url [" + stringUrl + "]");
-        }
-        String host = stringUrl.substring(0, portSeparator);
-        int port = Integer.valueOf(stringUrl.substring(portSeparator + 1));
-        hosts.add(buildHttpHost(host, port));
+  public void initRemoteClient(String clusterName) throws IOException {
+    remoteClient = remoteAdminClient = initClient(clusterName);
+  }
+
+  /** Configure http client for the given <b>cluster</b>. */
+  public RestClient initClient(String clusterName) throws IOException {
+    String[] stringUrls = getTestRestCluster(clusterName).split(",");
+    List<HttpHost> hosts = new ArrayList<>(stringUrls.length);
+    for (String stringUrl : stringUrls) {
+      int portSeparator = stringUrl.lastIndexOf(':');
+      if (portSeparator < 0) {
+        throw new IllegalArgumentException("Illegal cluster url [" + stringUrl + "]");
       }
-      final List<HttpHost> clusterHosts = unmodifiableList(hosts);
-      remoteClient = buildClient(restClientSettings(), clusterHosts.toArray(new HttpHost[0]));
-      remoteAdminClient = buildClient(restAdminSettings(), clusterHosts.toArray(new HttpHost[0]));
+      String host = stringUrl.substring(0, portSeparator);
+      int port = Integer.parseInt(stringUrl.substring(portSeparator + 1));
+      hosts.add(buildHttpHost(host, port));
     }
-    assert remoteClient != null;
-    assert remoteAdminClient != null;
+    return buildClient(restClientSettings(), hosts.toArray(new HttpHost[0]));
   }
 
   /** Get a comma delimited list of [host:port] to which to send REST requests. */
@@ -200,6 +209,27 @@ protected static void wipeAllOpenSearchIndices(RestClient client) throws IOExcep
     }
   }
 
+  /**
+   * Configure authentication and pass <b>builder</b> to superclass to configure other stuff.<br>
+   * By default, auth is configure when <b>https</b> is set only.
+   */
+  protected static void configureClient(RestClientBuilder builder, Settings settings)
+      throws IOException {
+    String userName = System.getProperty("user");
+    String password = System.getProperty("password");
+    if (userName != null && password != null) {
+      builder.setHttpClientConfigCallback(
+          httpClientBuilder -> {
+            BasicCredentialsProvider credentialsProvider = new BasicCredentialsProvider();
+            credentialsProvider.setCredentials(
+                new AuthScope(null, -1),
+                new UsernamePasswordCredentials(userName, password.toCharArray()));
+            return httpClientBuilder.setDefaultCredentialsProvider(credentialsProvider);
+          });
+    }
+    OpenSearchRestTestCase.configureClient(builder, settings);
+  }
+
   protected static void configureHttpsClient(
       RestClientBuilder builder, Settings settings, HttpHost httpHost) throws IOException {
     Map<String, String> headers = ThreadContext.buildDefaultHeaders(settings);
@@ -259,16 +289,13 @@ protected static void configureHttpsClient(
    * Initialize rest client to remote cluster, and create a connection to it from the coordinating
    * cluster.
    */
-  public void configureMultiClusters() throws IOException {
-    initRemoteClient();
+  public void configureMultiClusters(String remote) throws IOException {
+    initRemoteClient(remote);
 
     Request connectionRequest = new Request("PUT", "_cluster/settings");
     String connectionSetting =
-        "{\"persistent\": {\"cluster\": {\"remote\": {\""
-            + REMOTE_CLUSTER
-            + "\": {\"seeds\": [\""
-            + getTestTransportCluster(REMOTE_CLUSTER).split(",")[0]
-            + "\"]}}}}}";
+        String.format(
+            REMOTE_CLUSTER_SETTING, remote, getTestTransportCluster(remote).split(",")[0]);
     connectionRequest.setJsonEntity(connectionSetting);
     adminClient().performRequest(connectionRequest);
   }
diff --git a/integ-test/src/test/java/org/opensearch/sql/security/CrossClusterSearchIT.java b/integ-test/src/test/java/org/opensearch/sql/security/CrossClusterSearchIT.java
