Test: Fix forbidden uses in test framework (#32824)

This commit fixes existing uses of forbidden apis in the test framework
and re-enables the forbidden apis check. It was previously completely
disabled and had missed a rename of the forbidden apis signatures files.

closes #32772

Author: rjernst

test/build.gradle

@@ -28,19 +28,10 @@ subprojects {
   apply plugin: 'nebula.maven-base-publish'
   apply plugin: 'nebula.maven-scm'
 
-
-  // the main files are actually test files, so use the appropriate forbidden api sigs
-  forbiddenApisMain {
-    signaturesURLs = [PrecommitTasks.getResource('/forbidden/jdk-signatures.txt'),
-                      PrecommitTasks.getResource('/forbidden/es-signatures.txt'),
-                      PrecommitTasks.getResource('/forbidden/es-test-signatures.txt')]
-  }
-
   // TODO: should we have licenses for our test deps?
   dependencyLicenses.enabled = false
   dependenciesInfo.enabled = false
 
   // TODO: why is the test framework pulled in...
-  forbiddenApisMain.enabled = false
   jarHell.enabled = false
 }

test/fixtures/build.gradle

@@ -0,0 +1,5 @@
+
+subprojects {
+  // fixtures are mostly external and by default we don't want to check forbidden apis
+  forbiddenApisMain.enabled = false
+}

test/framework/build.gradle

@@ -41,8 +41,9 @@ compileTestJava.options.compilerArgs << '-Xlint:-rawtypes'
 
 // the main files are actually test files, so use the appropriate forbidden api sigs
 forbiddenApisMain {
-  signaturesURLs = [PrecommitTasks.getResource('/forbidden/all-signatures.txt'),
-                    PrecommitTasks.getResource('/forbidden/test-signatures.txt')]
+  signaturesURLs = [PrecommitTasks.getResource('/forbidden/jdk-signatures.txt'),
+                    PrecommitTasks.getResource('/forbidden/es-all-signatures.txt'),
+                    PrecommitTasks.getResource('/forbidden/es-test-signatures.txt')]
 }
 
 // TODO: should we have licenses for our test deps?

test/framework/src/main/java/org/elasticsearch/indices/analysis/AnalysisFactoryTestCase.java

@@ -67,7 +67,7 @@ private static String toCamelCase(String s) {
         Matcher m = UNDERSCORE_THEN_ANYTHING.matcher(s);
         StringBuffer sb = new StringBuffer();
         while (m.find()) {
-            m.appendReplacement(sb, m.group(1).toUpperCase());
+            m.appendReplacement(sb, m.group(1).toUpperCase(Locale.ROOT));
         }
         m.appendTail(sb);
         sb.setCharAt(0, Character.toUpperCase(sb.charAt(0)));

test/framework/src/main/java/org/elasticsearch/test/AbstractBuilderTestCase.java

@@ -21,7 +21,6 @@
 
 import com.carrotsearch.randomizedtesting.RandomizedTest;
 import com.carrotsearch.randomizedtesting.SeedUtils;
-
 import org.apache.lucene.index.IndexReader;
 import org.apache.lucene.util.Accountable;
 import org.elasticsearch.Version;
@@ -42,6 +41,7 @@
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.settings.SettingsModule;
 import org.elasticsearch.common.xcontent.NamedXContentRegistry;
+import org.elasticsearch.core.internal.io.IOUtils;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.env.TestEnvironment;
 import org.elasticsearch.index.Index;
@@ -194,8 +194,8 @@ protected static String expectedFieldName(String builderFieldName) {
 
     @AfterClass
     public static void afterClass() throws Exception {
-        org.apache.lucene.util.IOUtils.close(serviceHolder);
-        org.apache.lucene.util.IOUtils.close(serviceHolderWithNoType);
+        IOUtils.close(serviceHolder);
+        IOUtils.close(serviceHolderWithNoType);
         serviceHolder = null;
         serviceHolderWithNoType = null;
     }

test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java

@@ -54,6 +54,7 @@
 import org.elasticsearch.cluster.metadata.IndexMetaData;
 import org.elasticsearch.common.CheckedBiFunction;
 import org.elasticsearch.common.CheckedRunnable;
+import org.elasticsearch.common.SuppressForbidden;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.io.PathUtils;
 import org.elasticsearch.common.io.PathUtilsForTesting;
@@ -198,13 +199,9 @@ public static void resetPortCounter() {
     }
 
     static {
-        System.setProperty("log4j.shutdownHookEnabled", "false");
-        System.setProperty("log4j2.disable.jmx", "true");
-
+        setTestSysProps();
         LogConfigurator.loadLog4jPlugins();
 
-        // Enable Netty leak detection and monitor logger for logged leak errors
-        System.setProperty("io.netty.leakDetection.level", "paranoid");
         String leakLoggerName = "io.netty.util.ResourceLeakDetector";
         Logger leakLogger = LogManager.getLogger(leakLoggerName);
         Appender leakAppender = new AbstractAppender(leakLoggerName, null,
@@ -243,6 +240,14 @@ public void append(LogEvent event) {
         Collections.sort(javaZoneIds);
         JAVA_ZONE_IDS = Collections.unmodifiableList(javaZoneIds);
     }
+    @SuppressForbidden(reason = "force log4j and netty sysprops")
+    private static void setTestSysProps() {
+        System.setProperty("log4j.shutdownHookEnabled", "false");
+        System.setProperty("log4j2.disable.jmx", "true");
+
+        // Enable Netty leak detection and monitor logger for logged leak errors
+        System.setProperty("io.netty.leakDetection.level", "paranoid");
+    }
 
     protected final Logger logger = Loggers.getLogger(getClass());
     protected final DeprecationLogger deprecationLogger = new DeprecationLogger(logger);

test/framework/src/main/java/org/elasticsearch/test/fixture/AbstractHttpFixture.java

@@ -20,6 +20,8 @@
 package org.elasticsearch.test.fixture;
 
 import com.sun.net.httpserver.HttpServer;
+import org.elasticsearch.common.SuppressForbidden;
+import org.elasticsearch.common.io.PathUtils;
 
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
@@ -32,7 +34,6 @@
 import java.net.URI;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.nio.file.StandardCopyOption;
 import java.util.HashMap;
 import java.util.List;
@@ -48,6 +49,7 @@
 /**
  * Base class for test fixtures that requires a {@link HttpServer} to work.
  */
+@SuppressForbidden(reason = "uses httpserver by design")
 public abstract class AbstractHttpFixture {
 
     protected static final Map<String, String> TEXT_PLAIN_CONTENT_TYPE = contentType("text/plain; charset=utf-8");
@@ -62,7 +64,7 @@ public abstract class AbstractHttpFixture {
     private final Path workingDirectory;
 
     protected AbstractHttpFixture(final String workingDir) {
-        this.workingDirectory = Paths.get(Objects.requireNonNull(workingDir));
+        this.workingDirectory = PathUtils.get(Objects.requireNonNull(workingDir));
     }
 
     /**

test/framework/src/main/java/org/elasticsearch/test/junit/listeners/ReproduceInfoPrinter.java

@@ -22,6 +22,7 @@
 import org.apache.logging.log4j.Logger;
 import org.apache.lucene.util.Constants;
 import org.elasticsearch.common.Strings;
+import org.elasticsearch.common.SuppressForbidden;
 import org.elasticsearch.common.logging.Loggers;
 import org.elasticsearch.test.ESIntegTestCase;
 import org.elasticsearch.test.ESTestCase;
@@ -86,7 +87,12 @@ public void testFailure(Failure failure) throws Exception {
             gradleMessageBuilder.appendClientYamlSuiteProperties();
         }
 
-        System.err.println(b.toString());
+        printToErr(b.toString());
+    }
+
+    @SuppressForbidden(reason = "printing repro info")
+    private static void printToErr(String s) {
+        System.err.println(s);
     }
 
     protected static class GradleMessageBuilder extends ReproduceErrorMessageBuilder {

test/framework/src/main/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java

@@ -29,6 +29,7 @@
 import org.elasticsearch.action.ActionListenerResponseHandler;
 import org.elasticsearch.action.support.PlainActionFuture;
 import org.elasticsearch.cluster.node.DiscoveryNode;
+import org.elasticsearch.common.SuppressForbidden;
 import org.elasticsearch.common.io.stream.BytesStreamOutput;
 import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
 import org.elasticsearch.common.io.stream.StreamInput;
@@ -64,6 +65,7 @@
 import java.net.InetSocketAddress;
 import java.net.ServerSocket;
 import java.net.Socket;
+import java.net.UnknownHostException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -1894,7 +1896,7 @@ public void testTimeoutPerConnection() throws IOException {
             // means that once we received an ACK from the client we just drop the packet on the floor (which is what we want) and we run
             // into a connection timeout quickly. Yet other implementations can for instance can terminate the connection within the 3 way
             // handshake which I haven't tested yet.
-            socket.bind(new InetSocketAddress(InetAddress.getLocalHost(), 0), 1);
+            socket.bind(getLocalEphemeral(), 1);
             socket.setReuseAddress(true);
             DiscoveryNode first = new DiscoveryNode("TEST", new TransportAddress(socket.getInetAddress(),
                 socket.getLocalPort()), emptyMap(),
@@ -2008,7 +2010,7 @@ protected String handleRequest(TcpChannel mockChannel, String profileName, Strea
 
     public void testTcpHandshakeTimeout() throws IOException {
         try (ServerSocket socket = new MockServerSocket()) {
-            socket.bind(new InetSocketAddress(InetAddress.getLocalHost(), 0), 1);
+            socket.bind(getLocalEphemeral(), 1);
             socket.setReuseAddress(true);
             DiscoveryNode dummy = new DiscoveryNode("TEST", new TransportAddress(socket.getInetAddress(),
                 socket.getLocalPort()), emptyMap(),
@@ -2029,7 +2031,7 @@ public void testTcpHandshakeTimeout() throws IOException {
 
     public void testTcpHandshakeConnectionReset() throws IOException, InterruptedException {
         try (ServerSocket socket = new MockServerSocket()) {
-            socket.bind(new InetSocketAddress(InetAddress.getLocalHost(), 0), 1);
+            socket.bind(getLocalEphemeral(), 1);
             socket.setReuseAddress(true);
             DiscoveryNode dummy = new DiscoveryNode("TEST", new TransportAddress(socket.getInetAddress(),
                 socket.getLocalPort()), emptyMap(),
@@ -2665,4 +2667,8 @@ public void onConnectionOpened(final Transport.Connection connection) {
 
     protected abstract void closeConnectionChannel(Transport transport, Transport.Connection connection) throws IOException;
 
+    @SuppressForbidden(reason = "need local ephemeral port")
+    private InetSocketAddress getLocalEphemeral() throws UnknownHostException {
+        return new InetSocketAddress(InetAddress.getLocalHost(), 0);
+    }
 }

test/framework/src/main/java/org/elasticsearch/transport/MockTcpTransport.java

@@ -18,6 +18,7 @@
  */
 package org.elasticsearch.transport;
 
+import org.elasticsearch.cli.SuppressForbidden;
 import org.elasticsearch.core.internal.io.IOUtils;
 import org.elasticsearch.Version;
 import org.elasticsearch.action.ActionListener;
@@ -160,6 +161,7 @@ private void readMessage(MockChannel mockChannel, StreamInput input) throws IOEx
     }
 
     @Override
+    @SuppressForbidden(reason = "real socket for mocking remote connections")
     protected MockChannel initiateChannel(InetSocketAddress address, ActionListener<Void> connectListener) throws IOException {
         final MockSocket socket = new MockSocket();
         final MockChannel channel = new MockChannel(socket, address, "none");