From b31ed1172a1cf61d1695721f786eb3e946ca1750 Mon Sep 17 00:00:00 2001 From: Chenyang Ji Date: Wed, 24 Jul 2024 14:57:59 -0700 Subject: [PATCH] [Draft] fix integration test failures (#45) * fix integration test failures Signed-off-by: Chenyang Ji * add security related config in gradle and integ test Signed-off-by: Chenyang Ji --------- Signed-off-by: Chenyang Ji --- build.gradle | 271 ++++++++++++++++-- .../top_queries/TopQueriesRestIT.java | 167 ++++++++++- 2 files changed, 400 insertions(+), 38 deletions(-) diff --git a/build.gradle b/build.gradle index e8512eaa..f9be1577 100644 --- a/build.gradle +++ b/build.gradle @@ -1,10 +1,52 @@ import org.opensearch.gradle.test.RestIntegTestTask +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 + +buildscript { + ext { + opensearch_version = System.getProperty("opensearch.version", "3.0.0-SNAPSHOT") + isSnapshot = "true" == System.getProperty("build.snapshot", "true") + buildVersionQualifier = System.getProperty("build.version_qualifier", "") + version_tokens = opensearch_version.tokenize('-') + opensearch_build = version_tokens[0] + '.0' + if (buildVersionQualifier) { + opensearch_build += "-${buildVersionQualifier}" + } + opensearch_build_snapshot = opensearch_build + '-SNAPSHOT' + if (isSnapshot) { + opensearch_build += "-SNAPSHOT" + } + } + + repositories { + mavenLocal() + maven { url "https://aws.oss.sonatype.org/content/repositories/snapshots" } + mavenCentral() + maven { url "https://plugins.gradle.org/m2/" } + } + + dependencies { + classpath "org.opensearch.gradle:build-tools:${opensearch_version}" + } +} + + +plugins { + id "de.undercouch.download" version "5.3.0" + id 'com.diffplug.spotless' version '6.25.0' +} apply plugin: 'java' apply plugin: 'idea' apply plugin: 'eclipse' apply plugin: 'opensearch.opensearchplugin' apply plugin: 'opensearch.pluginzip' +apply plugin: 'opensearch.rest-test' +apply plugin: 'opensearch.repositories' def pluginName = 'query-insights' def pluginDescription = 'OpenSearch Query Insights plugin' @@ -12,6 +54,10 @@ def projectPath = 'org.opensearch' def pathToPlugin = 'plugin.insights' def pluginClassName = 'QueryInsightsPlugin' +configurations { + zipArchive +} + publishing { publications { pluginZip(MavenPublication) { publication -> @@ -50,25 +96,6 @@ loggerUsageCheck.enabled = false // No need to validate pom, as we do not upload to maven/sonatype validateNebulaPom.enabled = false -buildscript { - ext { - opensearch_version = System.getProperty("opensearch.version", "3.0.0-SNAPSHOT") - isSnapshot = "true" == System.getProperty("build.snapshot", "true") - buildVersionQualifier = System.getProperty("build.version_qualifier", "") - } - - repositories { - mavenLocal() - maven { url "https://aws.oss.sonatype.org/content/repositories/snapshots" } - mavenCentral() - maven { url "https://plugins.gradle.org/m2/" } - } - - dependencies { - classpath "org.opensearch.gradle:build-tools:${opensearch_version}" - } -} - allprojects { group 'org.opensearch' version = opensearch_version.tokenize('-')[0] + '.0' @@ -87,27 +114,153 @@ repositories { maven { url "https://plugins.gradle.org/m2/" } } +ext { + getSecurityPluginDownloadLink = { -> + var repo = "https://aws.oss.sonatype.org/content/repositories/snapshots/org/opensearch/plugin/" + + "opensearch-security/$opensearch_build_snapshot/" + 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" + } + + var projectAbsPath = projectDir.getAbsolutePath() + File downloadedSecurityPlugin = Paths.get(projectAbsPath, 'bin', 'opensearch-security-snapshot.zip').toFile() + + 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')) + } + } + + // add a check to avoid re-downloading multiple times during single test run + if (!downloadedSecurityPlugin.exists()) { + download.run { + src getSecurityPluginDownloadLink() + dest downloadedSecurityPlugin + } + } else { + println "Security Plugin File Already Exists" + } + + // 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.unsupported.inject_user.enabled': '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", "query_insights_full_access"]', + 'plugins.security.system_indices.enabled' : 'true' + ].forEach { name, value -> + cluster.setting name, value + } + + cluster.plugin provider((Callable) (() -> (RegularFile) (() -> downloadedSecurityPlugin))) + } +} + test { include '**/*Tests.class' } -task integTest(type: RestIntegTestTask) { - description = "Run tests against a cluster" - testClassesDirs = sourceSets.test.output.classesDirs - classpath = sourceSets.test.runtimeClasspath -} tasks.named("check").configure { dependsOn(integTest) } integTest { - // The --debug-jvm command-line option makes the cluster debuggable; this makes the tests debuggable - if (System.getProperty("test.debug") != null) { - jvmArgs '-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=*:5005' + // The --debug-jvm command-line option makes the cluster debuggable; this makes the tests debuggable + if (System.getProperty("test.debug") != null) { + jvmArgs '-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=*:5005' + } + systemProperty "cluster.names", + getClusters().stream().map(cluster -> cluster.getName()).collect(Collectors.joining(",")) + + dependsOn project.tasks.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" + } + 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.plugin.insights.rules.resthandler.top_queries.TopQueriesRestIT' + } + if (System.getProperty("security.enabled") == "true") { + useCluster testClusters.integTestWithSecurity + getClusters().forEach { cluster -> + configureSecurityPlugin(cluster) + } + systemProperty "user", "admin" + systemProperty "password", "admin" + } else { + useCluster testClusters.integTest + } } -testClusters.integTest { - testDistribution = "INTEG_TEST" +tasks.named("integTest").configure { + it.dependsOn(project.tasks.named("bundlePlugin")) +} +testClusters.all { + testDistribution = "INTEG_TEST" // This installs our plugin into the testClusters plugin(project.tasks.bundlePlugin.archiveFile) } @@ -116,6 +269,66 @@ run { useCluster testClusters.integTest } +check.dependsOn spotlessCheck +check.dependsOn jacocoTestReport + +task integTestWithSecurity(type: RestIntegTestTask) { + useCluster testClusters.integTestWithSecurity + + systemProperty "cluster.names", + getClusters().stream().map(cluster -> cluster.getName()).collect(Collectors.joining(",")) + + getClusters().forEach { cluster -> + configureSecurityPlugin(cluster) + } + + dependsOn project.tasks.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.plugin.insights.rules.resthandler.top_queries.TopQueriesRestIT' + } +} + // updateVersion: Task to auto update version to the next development iteration task updateVersion { onlyIf { System.getProperty('newVersion') } diff --git a/src/test/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/TopQueriesRestIT.java b/src/test/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/TopQueriesRestIT.java index 33f7f71a..577686c0 100644 --- a/src/test/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/TopQueriesRestIT.java +++ b/src/test/java/org/opensearch/plugin/insights/rules/resthandler/top_queries/TopQueriesRestIT.java @@ -8,27 +8,175 @@ package org.opensearch.plugin.insights.rules.resthandler.top_queries; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.stream.Collectors; +import org.apache.hc.client5.http.auth.AuthScope; +import org.apache.hc.client5.http.auth.UsernamePasswordCredentials; +import org.apache.hc.client5.http.impl.auth.BasicCredentialsProvider; +import org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManager; +import org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManagerBuilder; +import org.apache.hc.client5.http.ssl.ClientTlsStrategyBuilder; +import org.apache.hc.client5.http.ssl.NoopHostnameVerifier; +import org.apache.hc.core5.http.Header; +import org.apache.hc.core5.http.HttpHost; +import org.apache.hc.core5.http.message.BasicHeader; +import org.apache.hc.core5.http.nio.ssl.TlsStrategy; +import org.apache.hc.core5.reactor.ssl.TlsDetails; +import org.apache.hc.core5.ssl.SSLContextBuilder; +import org.apache.hc.core5.util.Timeout; +import org.junit.After; +import org.junit.Assert; import org.opensearch.client.Request; import org.opensearch.client.Response; +import org.opensearch.client.RestClient; +import org.opensearch.client.RestClientBuilder; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.unit.TimeValue; +import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.common.xcontent.LoggingDeprecationHandler; import org.opensearch.common.xcontent.json.JsonXContent; +import org.opensearch.core.xcontent.DeprecationHandler; +import org.opensearch.core.xcontent.MediaType; import org.opensearch.core.xcontent.NamedXContentRegistry; +import org.opensearch.core.xcontent.XContentParser; import org.opensearch.plugin.insights.settings.QueryInsightsSettings; import org.opensearch.test.rest.OpenSearchRestTestCase; -import org.junit.Assert; - -import java.io.IOException; -import java.nio.charset.StandardCharsets; -import java.util.List; -import java.util.Map; -/** - * Rest Action tests for Query Insights - */ +/** Rest Action tests for Query Insights */ public class TopQueriesRestIT extends OpenSearchRestTestCase { + private static String QUERY_INSIGHTS_INDICES_PREFIX = "top_queries"; + + protected boolean isHttps() { + return Optional.ofNullable(System.getProperty("https")).map("true"::equalsIgnoreCase).orElse(false); + } + + @Override + protected String getProtocol() { + return isHttps() ? "https" : "http"; + } + + @Override + protected RestClient buildClient(Settings settings, HttpHost[] hosts) throws IOException { + RestClientBuilder builder = RestClient.builder(hosts); + if (isHttps()) { + configureHttpsClient(builder, settings); + } else { + configureClient(builder, settings); + } + + builder.setStrictDeprecationMode(false); + return builder.build(); + } + + 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) throws IOException { + // Similar to client configuration with OpenSearch: + // https://github.com/opensearch-project/OpenSearch/blob/2.15.1/test/framework/src/main/java/org/opensearch/test/rest/OpenSearchRestTestCase.java#L841-L863 + builder.setHttpClientConfigCallback(httpClientBuilder -> { + String userName = Optional.ofNullable(System.getProperty("user")) + .orElseThrow(() -> new RuntimeException("user name is missing")); + String password = Optional.ofNullable(System.getProperty("password")) + .orElseThrow(() -> new RuntimeException("password is missing")); + BasicCredentialsProvider credentialsProvider = new BasicCredentialsProvider(); + final AuthScope anyScope = new AuthScope(null, -1); + credentialsProvider.setCredentials(anyScope, new UsernamePasswordCredentials(userName, password.toCharArray())); + try { + final TlsStrategy tlsStrategy = ClientTlsStrategyBuilder.create() + .setHostnameVerifier(NoopHostnameVerifier.INSTANCE) + .setSslContext(SSLContextBuilder.create().loadTrustMaterial(null, (chains, authType) -> true).build()) + // See https://issues.apache.org/jira/browse/HTTPCLIENT-2219 + .setTlsDetailsFactory(sslEngine -> new TlsDetails(sslEngine.getSession(), sslEngine.getApplicationProtocol())) + .build(); + final PoolingAsyncClientConnectionManager connectionManager = PoolingAsyncClientConnectionManagerBuilder.create() + .setTlsStrategy(tlsStrategy) + .build(); + return httpClientBuilder.setDefaultCredentialsProvider(credentialsProvider).setConnectionManager(connectionManager); + } catch (Exception e) { + throw new RuntimeException(e); + } + }); + Map headers = ThreadContext.buildDefaultHeaders(settings); + Header[] defaultHeaders = new Header[headers.size()]; + int i = 0; + for (Map.Entry entry : headers.entrySet()) { + defaultHeaders[i++] = new BasicHeader(entry.getKey(), entry.getValue()); + } + builder.setDefaultHeaders(defaultHeaders); + final String socketTimeoutString = settings.get(CLIENT_SOCKET_TIMEOUT); + final TimeValue socketTimeout = TimeValue.parseTimeValue( + socketTimeoutString == null ? "60s" : socketTimeoutString, + CLIENT_SOCKET_TIMEOUT + ); + builder.setRequestConfigCallback( + conf -> conf.setResponseTimeout(Timeout.ofMilliseconds(Math.toIntExact(socketTimeout.getMillis()))) + ); + if (settings.hasValue(CLIENT_PATH_PREFIX)) { + builder.setPathPrefix(settings.get(CLIENT_PATH_PREFIX)); + } + } + + /** + * wipeAllIndices won't work since it cannot delete security index. Use + * wipeAllQueryInsightsIndices instead. + */ + @Override + protected boolean preserveIndicesUponCompletion() { + return true; + } + + @SuppressWarnings("unchecked") + @After + public void wipeAllQueryInsightsIndices() throws Exception { + Response response = adminClient().performRequest(new Request("GET", "/_cat/indices?format=json&expand_wildcards=all")); + MediaType mediaType = MediaType.fromMediaType(response.getEntity().getContentType()); + try ( + XContentParser parser = mediaType.xContent() + .createParser( + NamedXContentRegistry.EMPTY, + DeprecationHandler.THROW_UNSUPPORTED_OPERATION, + response.getEntity().getContent() + ) + ) { + XContentParser.Token token = parser.nextToken(); + List> parserList = null; + if (token == XContentParser.Token.START_ARRAY) { + parserList = parser.listOrderedMap().stream().map(obj -> (Map) obj).collect(Collectors.toList()); + } else { + parserList = Collections.singletonList(parser.mapOrdered()); + } + + for (Map index : parserList) { + final String indexName = (String) index.get("index"); + if (indexName.startsWith(QUERY_INSIGHTS_INDICES_PREFIX)) { + adminClient().performRequest(new Request("DELETE", "/" + indexName)); + } + } + } + } /** * test Query Insights is installed + * * @throws IOException IOException */ @SuppressWarnings("unchecked") @@ -47,6 +195,7 @@ public void testQueryInsightsPluginInstalled() throws IOException { /** * test enabling top queries + * * @throws IOException IOException */ public void testTopQueriesResponses() throws IOException, InterruptedException {