[8.x] [TEST] Migrated REST tests extending JsonLogsIntegTestCase (#115188) (#116025)

* [TEST] Migrated REST tests extending JsonLogsIntegTestCase (#115188)

REST tests extending JsonLogsIntegTestCase migrated to the new REST
testing framework, using 'elasticsearch.internal-java-rest-test' Gradle
plugin
Explicit handling of test single-cluster config by checking if cluster
is explicitly configured as single-node to avoid an incorrect configuration
which may cause split brain.

Replaced `ElasticsearchJavaPlugin` with `ElasticsearchJavaBasePlugin` in
 `RestTestBasePlugin` for better granularity.
Additionally, updated the `DistributionDownloadPlugin` to set configurations as
non-consumable and the `InternalDistributionArchiveSetupPlugin` to mark
certain configurations as consumable. This ensures that configurations
are correctly utilized during build setup and execution phases.

(cherry picked from commit 130cc74)

* Fixed DefaultSettingsProvider to use Java 17 (#116029)

It is needed to be able to backport the change to the 8.x.
Having same code for both versions can help us to avoid some merge
conflicts in the future.

Author: jozala

build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/InternalDistributionArchiveSetupPlugin.java

@@ -76,12 +76,14 @@ private void registerAndConfigureDistributionArchivesExtension(Project project)
                 sub.getArtifacts().add(DEFAULT_CONFIGURATION_NAME, distributionArchive.getArchiveTask());
                 var extractedConfiguration = sub.getConfigurations().create(EXTRACTED_CONFIGURATION_NAME);
                 extractedConfiguration.setCanBeResolved(false);
+                extractedConfiguration.setCanBeConsumed(true);
                 extractedConfiguration.getAttributes()
                     .attribute(ArtifactTypeDefinition.ARTIFACT_TYPE_ATTRIBUTE, ArtifactTypeDefinition.DIRECTORY_TYPE);
                 sub.getArtifacts().add(EXTRACTED_CONFIGURATION_NAME, distributionArchive.getExpandedDistTask());
                 // The "composite" configuration is specifically used for resolving transformed artifacts in an included build
                 var compositeConfiguration = sub.getConfigurations().create(COMPOSITE_CONFIGURATION_NAME);
                 compositeConfiguration.setCanBeResolved(false);
+                compositeConfiguration.setCanBeConsumed(true);
                 compositeConfiguration.getAttributes()
                     .attribute(ArtifactTypeDefinition.ARTIFACT_TYPE_ATTRIBUTE, ArtifactTypeDefinition.DIRECTORY_TYPE);
                 compositeConfiguration.getAttributes().attribute(Attribute.of("composite", Boolean.class), true);

build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/RestrictedBuildApiService.java

@@ -53,8 +53,6 @@ private static ListMultimap<Class<?>, String> createLegacyRestTestBasePluginUsag
         map.put(LegacyRestTestBasePlugin.class, ":plugins:repository-hdfs");
         map.put(LegacyRestTestBasePlugin.class, ":plugins:store-smb");
         map.put(LegacyRestTestBasePlugin.class, ":qa:ccs-rolling-upgrade-remote-cluster");
-        map.put(LegacyRestTestBasePlugin.class, ":qa:ccs-unavailable-clusters");
-        map.put(LegacyRestTestBasePlugin.class, ":qa:logging-config");
         map.put(LegacyRestTestBasePlugin.class, ":qa:mixed-cluster");
         map.put(LegacyRestTestBasePlugin.class, ":qa:multi-cluster-search");
         map.put(LegacyRestTestBasePlugin.class, ":qa:remote-clusters");
@@ -65,15 +63,13 @@ private static ListMultimap<Class<?>, String> createLegacyRestTestBasePluginUsag
         map.put(LegacyRestTestBasePlugin.class, ":qa:smoke-test-ingest-with-all-dependencies");
         map.put(LegacyRestTestBasePlugin.class, ":qa:smoke-test-plugins");
         map.put(LegacyRestTestBasePlugin.class, ":qa:system-indices");
-        map.put(LegacyRestTestBasePlugin.class, ":qa:unconfigured-node-name");
         map.put(LegacyRestTestBasePlugin.class, ":qa:verify-version-constants");
         map.put(LegacyRestTestBasePlugin.class, ":test:external-modules:test-apm-integration");
         map.put(LegacyRestTestBasePlugin.class, ":test:external-modules:test-delayed-aggs");
         map.put(LegacyRestTestBasePlugin.class, ":test:external-modules:test-die-with-dignity");
         map.put(LegacyRestTestBasePlugin.class, ":test:external-modules:test-error-query");
         map.put(LegacyRestTestBasePlugin.class, ":test:external-modules:test-latency-simulating-directory");
         map.put(LegacyRestTestBasePlugin.class, ":test:yaml-rest-runner");
-        map.put(LegacyRestTestBasePlugin.class, ":distribution:archives:integ-test-zip");
         map.put(LegacyRestTestBasePlugin.class, ":x-pack:plugin:core");
         map.put(LegacyRestTestBasePlugin.class, ":x-pack:plugin:ent-search");
         map.put(LegacyRestTestBasePlugin.class, ":x-pack:plugin:fleet");

build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/test/rest/RestTestBasePlugin.java

@@ -18,7 +18,7 @@
 import org.elasticsearch.gradle.Version;
 import org.elasticsearch.gradle.VersionProperties;
 import org.elasticsearch.gradle.distribution.ElasticsearchDistributionTypes;
-import org.elasticsearch.gradle.internal.ElasticsearchJavaPlugin;
+import org.elasticsearch.gradle.internal.ElasticsearchJavaBasePlugin;
 import org.elasticsearch.gradle.internal.InternalDistributionDownloadPlugin;
 import org.elasticsearch.gradle.internal.info.BuildParams;
 import org.elasticsearch.gradle.internal.test.ErrorReportingTestListener;
@@ -89,7 +89,7 @@ public RestTestBasePlugin(ProviderFactory providerFactory) {
 
     @Override
     public void apply(Project project) {
-        project.getPluginManager().apply(ElasticsearchJavaPlugin.class);
+        project.getPluginManager().apply(ElasticsearchJavaBasePlugin.class);
         project.getPluginManager().apply(InternalDistributionDownloadPlugin.class);
 
         // Register integ-test and default distributions

build-tools/src/main/java/org/elasticsearch/gradle/DistributionDownloadPlugin.java

@@ -89,7 +89,9 @@ public void apply(Project project) {
     private void setupDistributionContainer(Project project) {
         distributionsContainer = project.container(ElasticsearchDistribution.class, name -> {
             var fileConfiguration = project.getConfigurations().create(DISTRO_CONFIG_PREFIX + name);
+            fileConfiguration.setCanBeConsumed(false);
             var extractedConfiguration = project.getConfigurations().create(DISTRO_EXTRACTED_CONFIG_PREFIX + name);
+            extractedConfiguration.setCanBeConsumed(false);
             extractedConfiguration.getAttributes()
                 .attribute(ArtifactTypeDefinition.ARTIFACT_TYPE_ATTRIBUTE, ArtifactTypeDefinition.DIRECTORY_TYPE);
             var distribution = new ElasticsearchDistribution(

distribution/archives/integ-test-zip/build.gradle

@@ -9,7 +9,7 @@
 
 import org.apache.tools.ant.filters.ReplaceTokens
 
-apply plugin: 'elasticsearch.legacy-java-rest-test'
+apply plugin: 'elasticsearch.internal-java-rest-test'
 
 // The integ-test-distribution is published to maven
 apply plugin: 'elasticsearch.publish'
@@ -35,19 +35,3 @@ publishing {
     }
   }
 }
-
-tasks.named("javaRestTest").configure {
-  dependsOn "assemble"
-  /*
-   * There are two unique things going on here:
-   * 1. These tests can be run against an external cluster with
-   *    -Dtests.rest.cluster=whatever and -Dtest.cluster=whatever
-   * 2. *One* of these tests is incompatible with that and should be skipped
-   *    when running against an external cluster.
-   */
-  if (project.providers.systemProperty("tests.rest.cluster").isPresent()) {
-    nonInputProperties.systemProperty 'tests.logfile', testClusters.named('javaRestTest').map(c -> c.singleNode().serverLog)
-  } else {
-    systemProperty 'tests.logfile', '--external--'
-  }
-}

distribution/archives/integ-test-zip/src/javaRestTest/java/org/elasticsearch/test/rest/CreatedLocationHeaderIT.java

@@ -11,6 +11,8 @@
 
 import org.elasticsearch.client.Request;
 import org.elasticsearch.client.Response;
+import org.elasticsearch.test.cluster.ElasticsearchCluster;
+import org.junit.ClassRule;
 
 import java.io.IOException;
 
@@ -23,6 +25,14 @@
  */
 public class CreatedLocationHeaderIT extends ESRestTestCase {
 
+    @ClassRule
+    public static ElasticsearchCluster cluster = ElasticsearchCluster.local().build();
+
+    @Override
+    protected String getTestRestCluster() {
+        return cluster.getHttpAddresses();
+    }
+
     public void testCreate() throws IOException {
         locationTestCase("PUT", "test/_doc/1");
     }

distribution/archives/integ-test-zip/src/javaRestTest/java/org/elasticsearch/test/rest/JsonLogsFormatAndParseIT.java

@@ -10,36 +10,36 @@
 package org.elasticsearch.test.rest;
 
 import org.elasticsearch.common.logging.JsonLogsIntegTestCase;
+import org.elasticsearch.test.cluster.ElasticsearchCluster;
+import org.elasticsearch.test.cluster.LogType;
 import org.hamcrest.Matcher;
+import org.junit.ClassRule;
 
-import java.io.BufferedReader;
-import java.io.IOException;
-import java.nio.charset.StandardCharsets;
-import java.nio.file.Files;
-import java.nio.file.Path;
-import java.security.AccessController;
-import java.security.PrivilegedAction;
+import java.io.InputStream;
 
 import static org.hamcrest.Matchers.is;
 
 public class JsonLogsFormatAndParseIT extends JsonLogsIntegTestCase {
+
+    private static final String NODE_NAME = "test-node-0";
+
+    @ClassRule
+    public static ElasticsearchCluster cluster = ElasticsearchCluster.local()
+        .withNode(localNodeSpecBuilder -> localNodeSpecBuilder.name(NODE_NAME))
+        .build();
+
+    @Override
+    protected String getTestRestCluster() {
+        return cluster.getHttpAddresses();
+    }
+
     @Override
     protected Matcher<String> nodeNameMatcher() {
-        return is("integTest-0");
+        return is(NODE_NAME);
     }
 
     @Override
-    protected BufferedReader openReader(Path logFile) {
-        assumeFalse(
-            "Skipping test because it is being run against an external cluster.",
-            logFile.getFileName().toString().equals("--external--")
-        );
-        return AccessController.doPrivileged((PrivilegedAction<BufferedReader>) () -> {
-            try {
-                return Files.newBufferedReader(logFile, StandardCharsets.UTF_8);
-            } catch (IOException e) {
-                throw new RuntimeException(e);
-            }
-        });
+    protected InputStream openLogsStream() {
+        return cluster.getNodeLog(0, LogType.SERVER_JSON);
     }
 }

distribution/archives/integ-test-zip/src/javaRestTest/java/org/elasticsearch/test/rest/NodeRestUsageIT.java

@@ -15,6 +15,8 @@
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.search.aggregations.AggregationBuilders;
 import org.elasticsearch.search.builder.SearchSourceBuilder;
+import org.elasticsearch.test.cluster.ElasticsearchCluster;
+import org.junit.ClassRule;
 
 import java.io.IOException;
 import java.util.Collections;
@@ -28,6 +30,14 @@
 
 public class NodeRestUsageIT extends ESRestTestCase {
 
+    @ClassRule
+    public static ElasticsearchCluster cluster = ElasticsearchCluster.local().build();
+
+    @Override
+    protected String getTestRestCluster() {
+        return cluster.getHttpAddresses();
+    }
+
     @SuppressWarnings("unchecked")
     public void testWithRestUsage() throws IOException {
         // First get the current usage figures

distribution/archives/integ-test-zip/src/javaRestTest/java/org/elasticsearch/test/rest/RequestsWithoutContentIT.java

@@ -11,13 +11,23 @@
 
 import org.elasticsearch.client.Request;
 import org.elasticsearch.client.ResponseException;
+import org.elasticsearch.test.cluster.ElasticsearchCluster;
+import org.junit.ClassRule;
 
 import java.io.IOException;
 
 import static org.hamcrest.CoreMatchers.containsString;
 
 public class RequestsWithoutContentIT extends ESRestTestCase {
 
+    @ClassRule
+    public static ElasticsearchCluster cluster = ElasticsearchCluster.local().build();
+
+    @Override
+    protected String getTestRestCluster() {
+        return cluster.getHttpAddresses();
+    }
+
     public void testIndexMissingBody() throws IOException {
         ResponseException responseException = expectThrows(
             ResponseException.class,

distribution/archives/integ-test-zip/src/javaRestTest/java/org/elasticsearch/test/rest/WaitForRefreshAndCloseIT.java

@@ -17,8 +17,10 @@
 import org.elasticsearch.client.Response;
 import org.elasticsearch.client.ResponseException;
 import org.elasticsearch.client.ResponseListener;
+import org.elasticsearch.test.cluster.ElasticsearchCluster;
 import org.junit.After;
 import org.junit.Before;
+import org.junit.ClassRule;
 
 import java.io.IOException;
 import java.util.Map;
@@ -32,6 +34,15 @@
  * Tests that wait for refresh is fired if the index is closed.
  */
 public class WaitForRefreshAndCloseIT extends ESRestTestCase {
+
+    @ClassRule
+    public static ElasticsearchCluster cluster = ElasticsearchCluster.local().build();
+
+    @Override
+    protected String getTestRestCluster() {
+        return cluster.getHttpAddresses();
+    }
+
     @Before
     public void setupIndex() throws IOException {
         Request request = new Request("PUT", "/test");

qa/logging-config/build.gradle

@@ -7,34 +7,14 @@
  * License v3.0 only", or the "Server Side Public License, v 1".
  */
 apply plugin: 'elasticsearch.build'
-apply plugin: 'elasticsearch.legacy-java-rest-test'
+apply plugin: 'elasticsearch.internal-java-rest-test'
 
 dependencies {
   testImplementation project(":libs:x-content")
   testImplementation project(":test:framework")
 
 }
 
-testClusters.all {
-  setting 'xpack.security.enabled', 'false'
-}
-
-testClusters.matching { it.name == "javaRestTest" }.configureEach {
-  /**
-   * Provide a custom log4j configuration where layout is an old style pattern and confirm that Elasticsearch
-   * can successfully startup.
-   */
-  extraConfigFile 'log4j2.properties', file('es-v7-log4j2.properties')
-}
-
-tasks.named("javaRestTest").configure {
-  nonInputProperties.systemProperty 'tests.logfile',
-    "${-> testClusters.javaRestTest.singleNode().getServerLog().absolutePath.replaceAll("_server.json", ".log")}"
-
-  nonInputProperties.systemProperty 'tests.jsonLogfile',
-    "${-> testClusters.javaRestTest.singleNode().getServerLog()}"
-}
-
 tasks.named("test").configure {
   systemProperty 'tests.security.manager', 'false'
 }

qa/logging-config/src/javaRestTest/java/org/elasticsearch/common/logging/CustomLoggingConfigIT.java

@@ -8,18 +8,19 @@
  */
 package org.elasticsearch.common.logging;
 
-import org.elasticsearch.core.SuppressForbidden;
+import org.elasticsearch.test.cluster.ElasticsearchCluster;
+import org.elasticsearch.test.cluster.LogType;
+import org.elasticsearch.test.cluster.util.resource.Resource;
 import org.elasticsearch.test.rest.ESRestTestCase;
 import org.hamcrest.Matchers;
+import org.junit.ClassRule;
 
+import java.io.BufferedReader;
 import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
 import java.io.UncheckedIOException;
 import java.nio.charset.StandardCharsets;
-import java.nio.file.Files;
-import java.nio.file.Path;
-import java.nio.file.Paths;
-import java.security.AccessController;
-import java.security.PrivilegedAction;
 import java.util.List;
 
 import static org.hamcrest.Matchers.matchesRegex;
@@ -34,53 +35,44 @@ public class CustomLoggingConfigIT extends ESRestTestCase {
     // [2020-03-20T14:51:59,989][INFO ][o.e.g.GatewayService ] [integTest-0] recovered [0] indices into cluster_state
     private static final String NODE_STARTED = ".*recovered.*cluster_state.*";
 
+    @ClassRule
+    public static ElasticsearchCluster cluster = ElasticsearchCluster.local()
+        .setting("xpack.security.enabled", "false")
+        .configFile("log4j2.properties", Resource.fromClasspath("es-v7-log4j2.properties"))
+        .build();
+
+    @Override
+    protected String getTestRestCluster() {
+        return cluster.getHttpAddresses();
+    }
+
     public void testSuccessfulStartupWithCustomConfig() throws Exception {
         assertBusy(() -> {
-            List<String> lines = readAllLines(getPlaintextLogFile());
+            List<String> lines = getPlaintextLog();
             assertThat(lines, Matchers.hasItem(matchesRegex(NODE_STARTED)));
         });
     }
 
     public void testParseAllV7JsonLines() throws Exception {
         assertBusy(() -> {
-            List<String> lines = readAllLines(getJSONLogFile());
+            List<String> lines = getJSONLog();
             assertThat(lines, Matchers.hasItem(matchesRegex(NODE_STARTED)));
         });
     }
 
-    private List<String> readAllLines(Path logFile) {
-        return AccessController.doPrivileged((PrivilegedAction<List<String>>) () -> {
-            try {
-                return Files.readAllLines(logFile, StandardCharsets.UTF_8);
-            } catch (IOException e) {
-                throw new UncheckedIOException(e);
-            }
-        });
-    }
-
-    @SuppressForbidden(reason = "PathUtils doesn't have permission to read this file")
-    private Path getJSONLogFile() {
-        String logFileString = System.getProperty("tests.logfile");
-        if (logFileString == null) {
-            fail(
-                "tests.logfile must be set to run this test. It is automatically "
-                    + "set by gradle. If you must set it yourself then it should be the absolute path to the "
-                    + "log file."
-            );
+    private List<String> getJSONLog() {
+        try (InputStream nodeLog = cluster.getNodeLog(0, LogType.SERVER_JSON)) {
+            return new BufferedReader(new InputStreamReader(nodeLog, StandardCharsets.UTF_8)).lines().toList();
+        } catch (IOException e) {
+            throw new UncheckedIOException(e);
         }
-        return Paths.get(logFileString);
     }
 
-    @SuppressForbidden(reason = "PathUtils doesn't have permission to read this file")
-    private Path getPlaintextLogFile() {
-        String logFileString = System.getProperty("tests.logfile");
-        if (logFileString == null) {
-            fail(
-                "tests.logfile must be set to run this test. It is automatically "
-                    + "set by gradle. If you must set it yourself then it should be the absolute path to the "
-                    + "log file."
-            );
+    private List<String> getPlaintextLog() {
+        try (InputStream nodeLog = cluster.getNodeLog(0, LogType.SERVER)) {
+            return new BufferedReader(new InputStreamReader(nodeLog, StandardCharsets.UTF_8)).lines().toList();
+        } catch (IOException e) {
+            throw new UncheckedIOException(e);
         }
-        return Paths.get(logFileString);
     }
 }