From 7e3a718f1b7d9100fbac2ee8317fd35042b63b39 Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Mon, 21 Aug 2023 10:25:12 -0700 Subject: [PATCH] Run IT tests with security plugin (#335) (#1986) * Run IT tests with security plugin (#335) * Add extra IT flow. Signed-off-by: Yury-Fridlyand * Remove unneeded files. Signed-off-by: Yury-Fridlyand * Typo fix. Signed-off-by: Yury-Fridlyand * Fix GHA matrix syntax. Signed-off-by: Yury-Fridlyand * Fix GHA matrix syntax. Signed-off-by: Yury-Fridlyand * Code clean up. Signed-off-by: Yury-Fridlyand * Optimize downloading. Signed-off-by: Yury-Fridlyand * Apply suggestions from code review Signed-off-by: Yury-Fridlyand Co-authored-by: Andrew Carbonetto * Update integ-test/build.gradle Signed-off-by: Yury-Fridlyand Co-authored-by: Andrew Carbonetto * Typo fix. Signed-off-by: Yury-Fridlyand * Rework implementation. Signed-off-by: Yury-Fridlyand * Address PR review. Signed-off-by: Yury-Fridlyand * Address PR feedback + some fixes. Signed-off-by: Yury-Fridlyand --------- Signed-off-by: Yury-Fridlyand Co-authored-by: Andrew Carbonetto * Minor fix. Signed-off-by: Yury-Fridlyand * Address PR feedback. Signed-off-by: Yury-Fridlyand * Typo fix. Signed-off-by: Yury-Fridlyand --------- Signed-off-by: Yury-Fridlyand Co-authored-by: Andrew Carbonetto --- .../workflows/integ-tests-with-security.yml | 43 +++++ integ-test/build.gradle | 163 +++++++++++++++++- .../sql/legacy/OpenSearchSQLRestTestCase.java | 85 +++++---- .../CrossClusterSearchIT.java | 52 ++++-- 4 files changed, 293 insertions(+), 50 deletions(-) create mode 100644 .github/workflows/integ-tests-with-security.yml rename integ-test/src/test/java/org/opensearch/sql/{ppl => security}/CrossClusterSearchIT.java (79%) diff --git a/.github/workflows/integ-tests-with-security.yml b/.github/workflows/integ-tests-with-security.yml new file mode 100644 index 0000000000..0d54b8cfef --- /dev/null +++ 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 index 6ee9cb425e..7cb0983670 100644 --- 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) (() -> 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 index 385c9bc6ba..d73e3468d4 100644 --- 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 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 cluster. */ + public RestClient initClient(String clusterName) throws IOException { + String[] stringUrls = getTestRestCluster(clusterName).split(","); + List 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 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 builder to superclass to configure other stuff.
+ * By default, auth is configure when https 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 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/ppl/CrossClusterSearchIT.java b/integ-test/src/test/java/org/opensearch/sql/security/CrossClusterSearchIT.java similarity index 79% rename from integ-test/src/test/java/org/opensearch/sql/ppl/CrossClusterSearchIT.java rename to integ-test/src/test/java/org/opensearch/sql/security/CrossClusterSearchIT.java index 19e3debdf0..086f32cba7 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/CrossClusterSearchIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/security/CrossClusterSearchIT.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package org.opensearch.sql.ppl; +package org.opensearch.sql.security; import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_ACCOUNT; import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_BANK; @@ -14,15 +14,30 @@ import static org.opensearch.sql.util.MatcherUtils.verifyDataRows; import java.io.IOException; +import lombok.SneakyThrows; import org.json.JSONObject; -import org.junit.Rule; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.junit.rules.ExpectedException; import org.opensearch.client.ResponseException; +import org.opensearch.sql.ppl.PPLIntegTestCase; +/** Cross Cluster Search tests to be executed with security plugin. */ public class CrossClusterSearchIT extends PPLIntegTestCase { - @Rule public ExpectedException exceptionRule = ExpectedException.none(); + static { + // find a remote cluster + String[] clusterNames = System.getProperty("cluster.names").split(","); + var remote = "remoteCluster"; + for (var cluster : clusterNames) { + if (cluster.startsWith("remote")) { + remote = cluster; + break; + } + } + REMOTE_CLUSTER = remote; + } + + public static final String REMOTE_CLUSTER; private static final String TEST_INDEX_BANK_REMOTE = REMOTE_CLUSTER + ":" + TEST_INDEX_BANK; private static final String TEST_INDEX_DOG_REMOTE = REMOTE_CLUSTER + ":" + TEST_INDEX_DOG; @@ -30,14 +45,25 @@ public class CrossClusterSearchIT extends PPLIntegTestCase { MATCH_ALL_REMOTE_CLUSTER + ":" + TEST_INDEX_DOG; private static final String TEST_INDEX_ACCOUNT_REMOTE = REMOTE_CLUSTER + ":" + TEST_INDEX_ACCOUNT; + private static boolean initialized = false; + + @SneakyThrows + @BeforeEach + public void initialize() { + if (!initialized) { + setUpIndices(); + initialized = true; + } + } + @Override - public void init() throws IOException { - configureMultiClusters(); + protected void init() throws Exception { + configureMultiClusters(REMOTE_CLUSTER); loadIndex(Index.BANK); loadIndex(Index.BANK, remoteClient()); loadIndex(Index.DOG); loadIndex(Index.DOG, remoteClient()); - loadIndex(Index.ACCOUNT, remoteClient()); + loadIndex(Index.ACCOUNT); } @Test @@ -55,11 +81,13 @@ public void testMatchAllCrossClusterSearchAllFields() throws IOException { @Test public void testCrossClusterSearchWithoutLocalFieldMappingShouldFail() throws IOException { - exceptionRule.expect(ResponseException.class); - exceptionRule.expectMessage("400 Bad Request"); - exceptionRule.expectMessage("IndexNotFoundException"); - - executeQuery(String.format("search source=%s", TEST_INDEX_ACCOUNT_REMOTE)); + var exception = + assertThrows( + ResponseException.class, + () -> executeQuery(String.format("search source=%s", TEST_INDEX_ACCOUNT_REMOTE))); + assertTrue( + exception.getMessage().contains("IndexNotFoundException") + && exception.getMessage().contains("400 Bad Request")); } @Test