Rationalize agent dependencies

Reduce the amount of library code pushed to the agent by re-organising code. Reduces size, attack surface area and classpath complexity
to make it easier to understand what is going on.

agent-common/src/main/java/com/thoughtworks/go/agent/launcher/ServerBinaryDownloader.java

@@ -23,7 +23,6 @@
 import com.thoughtworks.go.util.PerfTimer;
 import com.thoughtworks.go.util.SslVerificationMode;
 import org.apache.http.HttpResponse;
-import org.apache.http.HttpStatus;
 import org.apache.http.client.ClientProtocolException;
 import org.apache.http.client.config.RequestConfig;
 import org.apache.http.client.methods.CloseableHttpResponse;
@@ -35,6 +34,7 @@
 import org.slf4j.LoggerFactory;
 
 import java.io.*;
+import java.net.HttpURLConnection;
 import java.security.GeneralSecurityException;
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
@@ -142,7 +142,7 @@ private void handleInvalidResponse(HttpResponse response, String url) throws IOE
         try (PrintWriter out = new PrintWriter(sw)) {
             out.print("Problem accessing GoCD Server at ");
             out.println(url);
-            if (response.getStatusLine().getStatusCode() != HttpStatus.SC_OK) {
+            if (response.getStatusLine().getStatusCode() != HttpURLConnection.HTTP_OK) {
                 LOG.info("Response code: {}", response.getStatusLine().getStatusCode());
                 out.println("Possible causes:");
                 out.println("1. Your GoCD Server is down, not accessible or starting up.");

agent/build.gradle

@@ -53,20 +53,26 @@ dependencies {
   packagingOnly project.deps.bouncyCastlePkix
 
   implementation project(path: ':agent-common', configuration: 'runtimeElements')
-  implementation project(path: ':common', configuration: 'runtimeElements')
+  implementation(project(path: ':common', configuration: 'runtimeElements')) {
+    exclude(group: 'org.quartz-scheduler') // Comes from config-api, but we don't need this
+    exclude(group: 'com.bazaarvoice.jolt') // Comes from go-plugin-access, but only for config repo migrations
+  }
   implementation project.deps.jakartaAnnotation
   implementation project.deps.commonsConfiguration
   implementation project.deps.nanohttpd
   implementation project.deps.oshi
   compileOnly project.deps.jetBrainsAnnotations
 
-  testImplementation project(path: ':common', configuration: 'testOutput')
-  testImplementation project(path: ':config:config-api', configuration: 'testOutput')
   testImplementation project(path: ':test:test-utils', configuration: 'runtimeElements')
 
   testImplementation project.deps.junit5Api
   testRuntimeOnly project.deps.junit5Engine
+  testImplementation project.deps.junit5Params
   testImplementation project.deps.springTestJunit5
+  testImplementation project.deps.assertJ
+  testImplementation project.deps.systemStubs
+  testImplementation project.deps.mockitoCore
+  testImplementation project.deps.mockitoJunit5
 
   extractedAtTopLevel project(path: ':agent-process-launcher', configuration: 'runtimeElements')
   extractedAtTopLevel project(path: ':jar-class-loader', configuration: 'runtimeElements')
@@ -146,14 +152,9 @@ task verifyJar(type: VerifyJarTask) {
       "httpcore-4.4.16.jar",
       "httpmime-${project.versions.apacheHttpComponents}.jar",
       "istack-commons-runtime-4.1.2.jar",
-      "jackson-annotations-${project.versions.jacksonBom}.jar",
-      "jackson-core-${project.versions.jacksonBom}.jar",
-      "jackson-databind-${project.versions.jacksonBom}.jar",
       "jakarta.activation-api-2.1.2.jar",
       "jakarta.annotation-api-${project.versions.jakartaAnnotation}.jar",
-      "jakarta.servlet-api-${project.versions.servletApi}.jar",
       "jakarta.xml.bind-api-4.0.1.jar",
-      "javax.inject-1.jar",
       "jaxb-core-${project.versions.jaxb}.jar",
       "jaxb-runtime-${project.versions.jaxb}.jar",
       "jcl-over-slf4j-${project.versions.slf4jBom}.jar",
@@ -162,16 +163,13 @@ task verifyJar(type: VerifyJarTask) {
       "jna-jpms-5.13.0.jar",
       "jna-platform-jpms-5.13.0.jar",
       "joda-time-${project.versions.jodaTime}.jar",
-      "jolt-core-${project.versions.jolt}.jar",
-      "json-utils-${project.versions.jolt}.jar",
       "logback-classic-${project.versions.logback}.jar",
       "logback-core-${project.versions.logback}.jar",
       "nanohttpd-${project.versions.nanohttpd}.jar",
       "objenesis-${project.versions.objenesis}.jar",
       "org.apache.felix.framework-${project.versions.felix}.jar",
       "oshi-core-java11-${project.versions.oshi}.jar",
       "plugin-metadata-store-${project.version}.jar",
-      "quartz-${project.versions.quartz}.jar",
       "semantic-version-${project.versions.semanticVersion}.jar",
       "slf4j-api-${project.versions.slf4jBom}.jar",
       "spring-aop-${project.versions.spring}.jar",
@@ -181,7 +179,6 @@ task verifyJar(type: VerifyJarTask) {
       "spring-expression-${project.versions.spring}.jar",
       "spring-tx-${project.versions.spring}.jar",
       "spring-web-${project.versions.spring}.jar",
-      "spring-webmvc-${project.versions.spring}.jar",
       "txw2-${project.versions.jaxb}.jar",
       "util-${project.version}.jar"
     ]

agent/src/main/java/com/thoughtworks/go/agent/service/AgentUpgradeService.java

@@ -30,6 +30,7 @@
 import org.springframework.stereotype.Service;
 
 import java.io.IOException;
+import java.net.HttpURLConnection;
 import java.util.Map;
 
 import static com.thoughtworks.go.agent.common.util.HeaderUtil.parseExtraProperties;
@@ -86,7 +87,7 @@ private boolean upgradesEnabled() {
     private void checkForUpgradeAndExtraProperties(String agentMd5, String launcherMd5, String agentPluginsMd5, String tfsImplMd5) throws IOException {
         HttpGet method = getAgentLatestStatusGetMethod();
         try (final CloseableHttpResponse response = httpClient.execute(method)) {
-            if (response.getStatusLine().getStatusCode() != 200) {
+            if (response.getStatusLine().getStatusCode() != HttpURLConnection.HTTP_OK) {
                 LOGGER.error("[Agent Upgrade] Got status {} {} from GoCD", response.getStatusLine().getStatusCode(), response.getStatusLine());
                 return;
             }

agent/src/test-functional/java/com/thoughtworks/go/agent/service/RemoteRegistrationRequesterTest.java

@@ -32,14 +32,14 @@
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.mockito.ArgumentMatcher;
-import org.springframework.http.HttpStatus;
 
 import java.io.IOException;
+import java.net.HttpURLConnection;
 import java.util.List;
 import java.util.Properties;
 
-import static org.hamcrest.Matchers.*;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.*;
 import static org.mockito.ArgumentMatchers.isA;
 import static org.mockito.Mockito.*;
 
@@ -64,7 +64,7 @@ void shouldPassAllParametersToPostForRegistrationOfNonElasticAgent() throws IOEx
         GoAgentServerHttpClient httpClient = mock(GoAgentServerHttpClient.class);
         final CloseableHttpResponse response = mock(CloseableHttpResponse.class);
         final ProtocolVersion protocolVersion = new ProtocolVersion("https", 1, 2);
-        when(response.getStatusLine()).thenReturn(new BasicStatusLine(protocolVersion, HttpStatus.OK.value(), null));
+        when(response.getStatusLine()).thenReturn(new BasicStatusLine(protocolVersion, HttpURLConnection.HTTP_OK, null));
         when(response.getEntity()).thenReturn(new StringEntity(""));
         when(httpClient.execute(isA(HttpRequestBase.class))).thenReturn(response);
         final DefaultAgentRegistry defaultAgentRegistry = new DefaultAgentRegistry();
@@ -84,7 +84,7 @@ void shouldPassAllParametersToPostForRegistrationOfElasticAgent() throws IOExcep
         GoAgentServerHttpClient httpClient = mock(GoAgentServerHttpClient.class);
         final CloseableHttpResponse response = mock(CloseableHttpResponse.class);
         final ProtocolVersion protocolVersion = new ProtocolVersion("https", 1, 2);
-        when(response.getStatusLine()).thenReturn(new BasicStatusLine(protocolVersion, HttpStatus.OK.value(), null));
+        when(response.getStatusLine()).thenReturn(new BasicStatusLine(protocolVersion, HttpURLConnection.HTTP_OK, null));
         when(response.getEntity()).thenReturn(new StringEntity(""));
         when(httpClient.execute(isA(HttpRequestBase.class))).thenReturn(response);
 

agent/src/test-functional/java/com/thoughtworks/go/agent/service/SslInfrastructureServiceTest.java

@@ -16,9 +16,9 @@
 package com.thoughtworks.go.agent.service;
 
 import com.thoughtworks.go.agent.AgentAutoRegistrationPropertiesImpl;
+import com.thoughtworks.go.agent.URLService;
 import com.thoughtworks.go.agent.common.ssl.GoAgentServerHttpClient;
 import com.thoughtworks.go.config.AgentRegistry;
-import com.thoughtworks.go.agent.URLService;
 import org.apache.http.NameValuePair;
 import org.apache.http.ProtocolVersion;
 import org.apache.http.client.methods.CloseableHttpResponse;
@@ -33,9 +33,9 @@
 import org.mockito.ArgumentCaptor;
 import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
-import org.springframework.http.HttpStatus;
 
 import java.io.File;
+import java.net.HttpURLConnection;
 import java.nio.charset.StandardCharsets;
 import java.util.List;
 
@@ -107,8 +107,8 @@ void shouldDeleteTokenFromDiskWhenServerRejectsTheRegistrationRequestWithForbidd
         final ProtocolVersion protocolVersion = new ProtocolVersion("https", 1, 2);
         when(agentRegistry.uuid()).thenReturn("some-uuid");
         when(agentRegistry.tokenPresent()).thenReturn(true);
-        when(httpResponse.getStatusLine()).thenReturn(new BasicStatusLine(protocolVersion, HttpStatus.OK.value(), null));
-        when(httpResponseForbidden.getStatusLine()).thenReturn(new BasicStatusLine(protocolVersion, HttpStatus.FORBIDDEN.value(), null));
+        when(httpResponse.getStatusLine()).thenReturn(new BasicStatusLine(protocolVersion, HttpURLConnection.HTTP_OK, null));
+        when(httpResponseForbidden.getStatusLine()).thenReturn(new BasicStatusLine(protocolVersion, HttpURLConnection.HTTP_FORBIDDEN, null));
         when(httpClient.execute(any(HttpRequestBase.class))).thenReturn(httpResponseForbidden).thenReturn(httpResponse);
         sslInfrastructureService.createSslInfrastructure();
 

api/api-cctray/src/main/java/com/thoughtworks/go/api/cctray/CctrayController.java

@@ -23,14 +23,14 @@
 import com.thoughtworks.go.spark.SparkController;
 import com.thoughtworks.go.spark.spring.SparkSpringController;
 import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.http.HttpStatus;
 import org.springframework.stereotype.Component;
 import spark.HaltException;
 import spark.Request;
 import spark.Response;
 
 import java.io.IOException;
 import java.io.OutputStreamWriter;
+import java.net.HttpURLConnection;
 
 import static spark.Spark.*;
 
@@ -105,7 +105,7 @@ public void checkNonAnonymousUser(Request req, Response res) {
     }
 
     private HaltException renderForbiddenResponse() {
-        return halt(HttpStatus.FORBIDDEN.value(), ACCESS_DENIED_XML_RESPONSE);
+        return halt(HttpURLConnection.HTTP_FORBIDDEN, ACCESS_DENIED_XML_RESPONSE);
     }
 
 }

api/api-pipeline-selection-v1/src/main/java/com/thoughtworks/go/apiv1/pipelineselection/PipelineSelectionController.java

@@ -40,11 +40,11 @@
 import org.apache.commons.codec.digest.DigestUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.http.HttpStatus;
 import org.springframework.stereotype.Component;
 import spark.Request;
 import spark.Response;
 
+import java.net.HttpURLConnection;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Objects;
@@ -58,7 +58,6 @@ public class PipelineSelectionController extends ApiController implements SparkS
     private static final int ONE_YEAR = 3600 * 24 * 365;
     private static final String COOKIE_NAME = "selected_pipelines";
 
-    private static final int OK = HttpStatus.OK.value();
     private static final String DATA_IS_OUT_OF_DATE = "Update failed because the view is out-of-date. Try refreshing the page.";
 
     private final ApiAuthenticationHelper apiAuthenticationHelper;
@@ -148,7 +147,7 @@ public String update(Request request, Response response) {
             response.cookie("/go", COOKIE_NAME, String.valueOf(recordId), ONE_YEAR, systemEnvironment.isSessionCookieSecure(), true);
         }
 
-        response.status(OK);
+        response.status(HttpURLConnection.HTTP_OK);
         return format("{ \"contentHash\": \"%s\" }", pipelineSelectionsService.load(fromCookie, userId).etag());
     }
 

common/build.gradle

@@ -17,7 +17,6 @@
 description = 'GoCD Common Module'
 
 dependencies {
-  api project(path: ':config:config-api', configuration: 'runtimeElements')
   api project(path: ':commandline', configuration: 'runtimeElements')
   api project(path: ':base', configuration: 'runtimeElements')
   api project(path: ':util', configuration: 'runtimeElements')
@@ -26,7 +25,6 @@ dependencies {
   api project.deps.apacheHttpMime
   api project.deps.commonsCollections4
   api project.deps.commonsText
-  api project.deps.springTx
   implementation project.deps.jodaTime
   annotationProcessor project.deps.lombok
   compileOnly project.deps.lombok

common/src/main/java/com/thoughtworks/go/agent/HttpService.java

@@ -33,8 +33,8 @@
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Component;
 
-import javax.servlet.http.HttpServletResponse;
 import java.io.*;
+import java.net.HttpURLConnection;
 import java.util.Properties;
 
 @Component
@@ -91,7 +91,7 @@ public int download(String url, FetchHandler handler) throws IOException {
                 timer.stop();
                 int statusCode = response.getStatusLine().getStatusCode();
 
-                if (statusCode == HttpServletResponse.SC_OK) {
+                if (statusCode == HttpURLConnection.HTTP_OK) {
                     if (response.getEntity() != null) {
                         try (InputStream is = response.getEntity().getContent()) {
                             handler.handle(is);

common/src/main/java/com/thoughtworks/go/domain/ChecksumFileHandler.java

@@ -21,10 +21,10 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import javax.servlet.http.HttpServletResponse;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
+import java.net.HttpURLConnection;
 
 import static org.apache.commons.io.FileUtils.deleteQuietly;
 
@@ -49,20 +49,20 @@ public void handle(InputStream stream) throws IOException {
 
     @Override
     public boolean handleResult(int returncode, GoPublisher goPublisher) {
-        if (returncode == HttpServletResponse.SC_NOT_FOUND) {
+        if (returncode == HttpURLConnection.HTTP_NOT_FOUND) {
             deleteQuietly(checksumFile);
             goPublisher.taggedConsumeLineWithPrefix(GoPublisher.ERR, "[WARN] The md5checksum property file was not found on the server. Hence, Go can not verify the integrity of the artifacts.");
             return true;
         }
-        if (returncode == HttpServletResponse.SC_NOT_MODIFIED) {
+        if (returncode == HttpURLConnection.HTTP_NOT_MODIFIED) {
             LOG.info("[Agent Fetch Artifact] Not downloading checksum file as it has not changed");
             return true;
         }
-        if (returncode == HttpServletResponse.SC_OK) {
+        if (returncode == HttpURLConnection.HTTP_OK) {
             LOG.info("[Agent Fetch Artifact] Saved checksum property file [{}]", checksumFile);
             return true;
         }
-        return returncode < HttpServletResponse.SC_BAD_REQUEST;
+        return returncode < HttpURLConnection.HTTP_BAD_REQUEST;
     }
 
     @Override

common/src/main/java/com/thoughtworks/go/domain/ChecksumValidationPublisher.java

@@ -17,15 +17,15 @@
 
 import com.thoughtworks.go.work.GoPublisher;
 
-import javax.servlet.http.HttpServletResponse;
 import java.io.File;
 import java.io.Serializable;
+import java.net.HttpURLConnection;
 import java.util.HashSet;
 import java.util.Set;
 
 public class ChecksumValidationPublisher implements com.thoughtworks.go.agent.ChecksumValidationPublisher, Serializable {
-    private Set<String> md5NotFoundPaths = new HashSet<>();
-    private Set<String> md5MismatchPaths = new HashSet<>();
+    private final Set<String> md5NotFoundPaths = new HashSet<>();
+    private final Set<String> md5MismatchPaths = new HashSet<>();
     private boolean md5ChecksumFileWasNotFound;
 
     @Override
@@ -58,12 +58,12 @@ public void publish(int httpCode, File artifact, GoPublisher goPublisher) {
             goPublisher.taggedConsumeLineWithPrefix(GoPublisher.ERR, String.format("[WARN] The md5checksum value of the artifact [%s] was not found on the server. Hence, Go could not verify the integrity of its contents.", md5NotFoundPath));
         }
 
-        if (httpCode == HttpServletResponse.SC_NOT_MODIFIED) {
+        if (httpCode == HttpURLConnection.HTTP_NOT_MODIFIED) {
             goPublisher.taggedConsumeLineWithPrefix(GoPublisher.OUT, "Artifact is not modified, skipped fetching it");
         }
 
-        if (httpCode == HttpServletResponse.SC_OK) {
-            if (md5NotFoundPaths.size() > 0 || md5ChecksumFileWasNotFound) {
+        if (httpCode == HttpURLConnection.HTTP_OK) {
+            if (!md5NotFoundPaths.isEmpty() || md5ChecksumFileWasNotFound) {
                 goPublisher.taggedConsumeLineWithPrefix(GoPublisher.ERR, String.format("Saved artifact to [%s] without verifying the integrity of its contents.", artifact));
             } else {
                 goPublisher.taggedConsumeLineWithPrefix(GoPublisher.OUT, String.format("Saved artifact to [%s] after verifying the integrity of its contents.", artifact));

common/src/main/java/com/thoughtworks/go/domain/DirHandler.java

@@ -22,10 +22,10 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import javax.servlet.http.HttpServletResponse;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
+import java.net.HttpURLConnection;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipInputStream;
 
@@ -72,7 +72,7 @@ private String getSrcFilePath(ZipEntry entry) {
     @Override
     public boolean handleResult(int httpCode, GoPublisher goPublisher) {
         checksumValidationPublisher.publish(httpCode, destOnAgent, goPublisher);
-        return httpCode < HttpServletResponse.SC_BAD_REQUEST;
+        return httpCode < HttpURLConnection.HTTP_BAD_REQUEST;
     }
 
     @Override

common/src/main/java/com/thoughtworks/go/domain/DownloadAction.java

@@ -15,13 +15,13 @@
  */
 package com.thoughtworks.go.domain;
 
-import com.thoughtworks.go.util.Clock;
 import com.thoughtworks.go.agent.HttpService;
+import com.thoughtworks.go.util.Clock;
 import com.thoughtworks.go.work.GoPublisher;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import javax.servlet.http.HttpServletResponse;
+import java.net.HttpURLConnection;
 
 public class DownloadAction {
 
@@ -71,7 +71,7 @@ private void publishDownloadError(String url, String cause, long backout) {
 
     private int download(HttpService httpService, String url, FetchHandler handler) throws Exception {
         int returnCode = httpService.download(url, handler);
-        while (returnCode == HttpServletResponse.SC_ACCEPTED) {
+        while (returnCode == HttpURLConnection.HTTP_ACCEPTED) {
             clock.sleepForMillis(DOWNLOAD_SLEEP_MILLIS);
             returnCode = httpService.download(url, handler);
         }