Replace wiremock with okhttp MockWebServer

[codefromthecrypt]

Currently, the only HTTP client that can be used is okhttp, but we use wiremock instead of MockWebServer, the canonical test library for OkHttp.

Here are some reasons to switch to okhttp MockWebServer, and I offer to rewrite everything as I already began this locally:

• There are currently 3 outstanding issues on flaky tests, caused by wiremock and not possible with MockWebServer as it internally uses a semaphore to block before verifying requests
• wiremock ends up using its own assertions instead of assertj, which makes a cognitive glitch writing tests
• wiremock's dependency tree has too many libraries and has caused interference recently
• as a part of the above, CVEs unrelated to this project will show up due to indirect wiremock deps
• there are multiple comments about wiremock not supporting something, which do not apply to mockwebserver, such as multiple query params
• It is common place to repeat the same thing in wiremock, as it is typical to declare the path both setting up the response and verifying the request
• wiremock is slower to execute by one or two orders of magnitude and this accumulates at runtime

Here are some reasons not to switch:

• folks are familiar with wiremock, as well with the problems I described
• by constantly updating wiremock, you can avoid interference problems and CVE exposure
• because the build is already slow, the extra time spent in wiremock will not make a huge difference except running specific tests
• how MockWebServer solves racy conditions is via a blocking takeRequest. If someone adds an extra one of these, it will hang a test until they figured out the bug they created.
• (like testcontainers), MockWebServer 4 depends internally on junit 4, which means there will be an indirect dep still even when tests are migrated to jupiter. This does not interfere with jupiter at runtime in any way.

Here's an example of the deps diff, note that okhttp can no longer interfere with jackson, and it has no opinion on jakarta vs javax on servlets.

33a34,35
> [INFO] |  |  +- com.fasterxml.jackson.core:jackson-annotations:jar:2.16.1:compile
> [INFO] |  |  \- com.fasterxml.jackson.core:jackson-core:jar:2.16.1:compile
52a55
> [INFO] |     |     \- org.ow2.asm:asm:jar:8.0.1:compile
90,130c93,95
< [INFO] +- org.wiremock:wiremock:jar:3.4.2:test
< [INFO] |  +- org.eclipse.jetty:jetty-server:jar:11.0.20:test
< [INFO] |  |  +- org.eclipse.jetty:jetty-http:jar:11.0.20:test
< [INFO] |  |  +- org.eclipse.jetty:jetty-io:jar:11.0.20:test
< [INFO] |  |  \- org.eclipse.jetty.toolchain:jetty-jakarta-servlet-api:jar:5.0.2:test
< [INFO] |  +- org.eclipse.jetty:jetty-servlet:jar:11.0.20:test
< [INFO] |  |  \- org.eclipse.jetty:jetty-security:jar:11.0.20:test
< [INFO] |  +- org.eclipse.jetty:jetty-servlets:jar:11.0.20:test
< [INFO] |  |  \- org.eclipse.jetty:jetty-util:jar:11.0.20:test
< [INFO] |  +- org.eclipse.jetty:jetty-webapp:jar:11.0.20:test
< [INFO] |  |  \- org.eclipse.jetty:jetty-xml:jar:11.0.20:test
< [INFO] |  +- org.eclipse.jetty:jetty-proxy:jar:11.0.20:test
< [INFO] |  |  \- org.eclipse.jetty:jetty-client:jar:11.0.20:test
< [INFO] |  +- org.eclipse.jetty.http2:http2-server:jar:11.0.20:test
< [INFO] |  |  \- org.eclipse.jetty.http2:http2-common:jar:11.0.20:test
< [INFO] |  |     \- org.eclipse.jetty.http2:http2-hpack:jar:11.0.20:test
< [INFO] |  +- org.eclipse.jetty:jetty-alpn-server:jar:11.0.20:test
< [INFO] |  +- org.eclipse.jetty:jetty-alpn-java-server:jar:11.0.20:test
< [INFO] |  +- org.eclipse.jetty:jetty-alpn-java-client:jar:11.0.20:test
< [INFO] |  +- org.eclipse.jetty:jetty-alpn-client:jar:11.0.20:test
< [INFO] |  +- com.fasterxml.jackson.core:jackson-core:jar:2.16.1:compile
< [INFO] |  +- com.fasterxml.jackson.core:jackson-annotations:jar:2.16.1:compile
< [INFO] |  +- com.fasterxml.jackson.datatype:jackson-datatype-jsr310:jar:2.16.1:test
< [INFO] |  +- org.apache.httpcomponents.client5:httpclient5:jar:5.3.1:test
< [INFO] |  |  +- org.apache.httpcomponents.core5:httpcore5:jar:5.2.4:test
< [INFO] |  |  \- org.apache.httpcomponents.core5:httpcore5-h2:jar:5.2.4:test
< [INFO] |  +- org.xmlunit:xmlunit-core:jar:2.9.1:test
< [INFO] |  |  \- jakarta.xml.bind:jakarta.xml.bind-api:jar:2.3.3:test
< [INFO] |  |     \- jakarta.activation:jakarta.activation-api:jar:1.2.2:test
< [INFO] |  +- org.xmlunit:xmlunit-legacy:jar:2.9.1:test
< [INFO] |  +- org.xmlunit:xmlunit-placeholders:jar:2.9.1:test
< [INFO] |  +- net.javacrumbs.json-unit:json-unit-core:jar:2.38.0:test
< [INFO] |  +- com.jayway.jsonpath:json-path:jar:2.9.0:test
< [INFO] |  +- org.ow2.asm:asm:jar:9.6:compile
< [INFO] |  +- net.sf.jopt-simple:jopt-simple:jar:5.0.4:test
< [INFO] |  +- com.github.jknack:handlebars:jar:4.3.1:test
< [INFO] |  +- com.github.jknack:handlebars-helpers:jar:4.3.1:test
< [INFO] |  +- commons-fileupload:commons-fileupload:jar:1.5:test
< [INFO] |  \- com.networknt:json-schema-validator:jar:1.3.3:test
< [INFO] |     +- com.ethlo.time:itu:jar:1.8.0:test
< [INFO] |     \- com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:jar:2.15.3:test
---
> [INFO] +- com.squareup.okhttp3:mockwebserver:jar:3.4.2:test
> [INFO] |  +- com.squareup.okhttp3:okhttp-ws:jar:3.4.2:test
> [INFO] |  \- org.bouncycastle:bcprov-jdk15on:jar:1.50:test

Here's an example diff of a test, note a couple tech debt removed.

diff --git a/util/src/test/java/io/kubernetes/client/ExecTest.java b/util/src/test/java/io/kubernetes/client/ExecTest.java
index c05c5c088..c9cbc5de4 100644
--- a/util/src/test/java/io/kubernetes/client/ExecTest.java
+++ b/util/src/test/java/io/kubernetes/client/ExecTest.java
@@ -12,24 +12,14 @@ limitations under the License.
 */
 package io.kubernetes.client;
 
-import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
-import static com.github.tomakehurst.wiremock.client.WireMock.equalTo;
-import static com.github.tomakehurst.wiremock.client.WireMock.get;
-import static com.github.tomakehurst.wiremock.client.WireMock.getRequestedFor;
-import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo;
-import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.options;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.junit.Assert.fail;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 
-import com.github.tomakehurst.wiremock.core.Admin;
-import com.github.tomakehurst.wiremock.extension.Parameters;
-import com.github.tomakehurst.wiremock.extension.PostServeAction;
-import com.github.tomakehurst.wiremock.junit.WireMockRule;
-import com.github.tomakehurst.wiremock.stubbing.ServeEvent;
 import io.kubernetes.client.Exec.ExecProcess;
 import io.kubernetes.client.openapi.ApiClient;
 import io.kubernetes.client.openapi.ApiException;
@@ -44,28 +34,16 @@ import java.io.InputStream;
 import java.io.OutputStream;
 import java.nio.charset.StandardCharsets;
 import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.Semaphore;
 import java.util.function.Consumer;
+import okhttp3.HttpUrl;
+import okhttp3.mockwebserver.MockResponse;
+import okhttp3.mockwebserver.MockWebServer;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 
 /** Tests for the Exec helper class */
 public class ExecTest {
-
-  public static class CountRequestAction extends PostServeAction {
-    @Override
-    public String getName() {
-      return "semaphore";
-    }
-
-    @Override
-    public void doAction(ServeEvent serveEvent, Admin admin, Parameters parameters) {
-      Semaphore count = (Semaphore) parameters.get("semaphore");
-      count.release();
-    }
-  }
-
   private static final String OUTPUT_EXIT0 = "{\"metadata\":{},\"status\":\"Success\"}";
   private static final String OUTPUT_EXIT1 =
       "{\"metadata\":{},\"status\":\"Failure\",\"message\":\"command terminated with non-zero exit code: Error executing in Docker Container: 1\",\"reason\":\"NonZeroExitCode\",\"details\":{\"causes\":[{\"reason\":\"ExitCode\",\"message\":\"1\"}]}}";
@@ -78,22 +56,17 @@ public class ExecTest {
 
   private String namespace;
   private String podName;
-  private String[] cmd;
 
   private ApiClient client;
 
-  @Rule public WireMockRule wireMockRule = new WireMockRule(options().dynamicPort());
+  @Rule public MockWebServer apiServer = new MockWebServer();
 
   @Before
   public void setup() {
-    client = new ClientBuilder().setBasePath("http://localhost:" + wireMockRule.port()).build();
+    client = new ClientBuilder().setBasePath("http://localhost:" + apiServer.getPort()).build();
 
     namespace = "default";
     podName = "apod";
-    // TODO: When WireMock supports multiple query params with the same name expand
-    // this
-    // See: https://github.com/tomakehurst/wiremock/issues/398
-    cmd = new String[] {"cmd"};
   }
 
   public static InputStream makeStream(int streamNum, byte[] data) {
@@ -111,15 +84,14 @@ public class ExecTest {
       final InputStream is, final OutputStream os, CountDownLatch cLatch) {
     Thread t =
         new Thread(
-            new Runnable() {
-              public void run() {
-                try {
-                  Streams.copy(is, os);
-                } catch (IOException ex) {
-                  ex.printStackTrace();
-                } finally {
-                  cLatch.countDown();
-                }
+            () -> {
+              try {
+                Streams.copy(is, os);
+              } catch (Exception e) {
+                e.printStackTrace(); // TODO: junit-jupiter fail(e);
+                fail(e.getMessage());
+              } finally {
+                cLatch.countDown();
               }
             });
     t.start();
@@ -191,19 +163,11 @@ public class ExecTest {
 
     V1Pod pod = new V1Pod().metadata(new V1ObjectMeta().name(podName).namespace(namespace));
 
-    Semaphore getCount = new Semaphore(2);
-    Parameters getParams = new Parameters();
-    getParams.put("semaphore", getCount);
-
-    wireMockRule.stubFor(
-        get(urlPathEqualTo("/api/v1/namespaces/" + namespace + "/pods/" + podName + "/exec"))
-            .withPostServeAction("semaphore", getParams)
-            .willReturn(
-                aResponse()
-                    .withStatus(404)
-                    .withHeader("Content-Type", "application/json")
-                    .withBody("{}")));
+    // Expect two requests
+    apiServer.enqueue(new MockResponse().setResponseCode(404));
+    apiServer.enqueue(new MockResponse().setResponseCode(404));
 
+    String[] cmd = new String[] {"cmd", "a", "b"};
     Process p = exec.exec(pod, cmd, "container", true, false);
     p.waitFor();
 
@@ -214,30 +178,18 @@ public class ExecTest {
         .execute()
         .waitFor();
 
-    // These will be released for each web call above.
-    // There is a race between the above waitFor() and the request actually being recorded
-    // by WireMock. This fixes it.
-    getCount.acquire(2);
-
-    wireMockRule.verify(
-        getRequestedFor(
-                urlPathEqualTo("/api/v1/namespaces/" + namespace + "/pods/" + podName + "/exec"))
-            .withQueryParam("stdin", equalTo("true"))
-            .withQueryParam("stdout", equalTo("true"))
-            .withQueryParam("stderr", equalTo("true"))
-            .withQueryParam("container", equalTo("container"))
-            .withQueryParam("tty", equalTo("false"))
-            .withQueryParam("command", equalTo("cmd")));
-
-    wireMockRule.verify(
-        getRequestedFor(
-                urlPathEqualTo("/api/v1/namespaces/" + namespace + "/pods/" + podName + "/exec"))
-            .withQueryParam("stdin", equalTo("false"))
-            .withQueryParam("stdout", equalTo("true"))
-            .withQueryParam("stderr", equalTo("false"))
-            .withQueryParam("container", equalTo("container"))
-            .withQueryParam("tty", equalTo("false"))
-            .withQueryParam("command", equalTo("cmd")));
+    // We expect two identical requests. Note that .takeRequest() is blocking, so this is safe.
+    for (int i = 0; i < 2; i++) {
+      HttpUrl apiRequest = HttpUrl.parse(apiServer.takeRequest().getPath());
+      assertThat(apiRequest.encodedPath())
+          .isEqualTo("/api/v1/namespaces/" + namespace + "/pods/" + podName + "/exec");
+      assertThat(apiRequest.queryParameter("stdin")).isEqualTo("true");
+      assertThat(apiRequest.queryParameter("stdout")).isEqualTo("true");
+      assertThat(apiRequest.queryParameter("stderr")).isEqualTo("true");
+      assertThat(apiRequest.queryParameter("tty")).isEqualTo("false");
+      assertThat(apiRequest.queryParameterValues("command"))
+          .containsExactly(cmd);
+    }
 
     assertThat(p.exitValue()).isEqualTo(-1975219);
     verify(consumer, times(1)).accept(any(Throwable.class));