Merge pull request #580 from cqse/ts/40621_jacoco_agent_proxy_setting…

…s_are_needed_before_loading_config_from_teamscale

TS-40621 jacoco agent proxy settings are needed before loading config from teamscale

CHANGELOG.md

@@ -5,6 +5,7 @@ We use [semantic versioning](http://semver.org/):
 - PATCH version when you make backwards compatible bug fixes.
 
 # Next version
+- [fix] _agent_: Loading a profiler configuration from Teamscale was not possible if the potentially necessary proxy settings were not set yet.
 
 # 34.1.0
 - [feature] _agent_: New options `proxy-http(s)-host`/`-port`/`-user`/`-password` allow user to specify teamscale-specific proxy settings.

agent/src/main/java/com/teamscale/jacoco/agent/AgentBase.java

@@ -1,8 +1,5 @@
 package com.teamscale.jacoco.agent;
 
-import com.google.common.annotations.VisibleForTesting;
-import com.teamscale.client.ProxySystemProperties;
-import com.teamscale.client.TeamscaleProxySystemProperties;
 import com.teamscale.jacoco.agent.options.AgentOptions;
 import com.teamscale.jacoco.agent.util.LoggingUtils;
 import org.eclipse.jetty.server.Server;
@@ -40,8 +37,6 @@ public abstract class AgentBase {
 	public AgentBase(AgentOptions options) throws IllegalStateException {
 		this.options = options;
 
-		putTeamscaleProxyOptionsIntoSystemProperties(options);
-
 		try {
 			controller = new JacocoRuntimeController(RT.getAgent());
 		} catch (IllegalStateException e) {
@@ -61,15 +56,7 @@ public AgentBase(AgentOptions options) throws IllegalStateException {
 		}
 	}
 
-	/**
-	 * Stores the agent options for proxies in the {@link TeamscaleProxySystemProperties} and overwrites
-	 * the password with the password found in the proxy-password-file if necessary.
-	 */
-	@VisibleForTesting
-	 public static void putTeamscaleProxyOptionsIntoSystemProperties(AgentOptions options) {
-		options.getTeamscaleProxyOptions(ProxySystemProperties.Protocol.HTTP).putTeamscaleProxyOptionsIntoSystemProperties();
-		options.getTeamscaleProxyOptions(ProxySystemProperties.Protocol.HTTPS).putTeamscaleProxyOptionsIntoSystemProperties();
-	}
+
 
 	/**
 	 * Lazily generated string representation of the command line arguments to print to the log.

agent/src/main/java/com/teamscale/jacoco/agent/options/AgentOptionsParser.java

@@ -8,6 +8,7 @@
 import com.google.common.annotations.VisibleForTesting;
 import com.teamscale.client.ProxySystemProperties;
 import com.teamscale.client.StringUtils;
+import com.teamscale.client.TeamscaleProxySystemProperties;
 import com.teamscale.jacoco.agent.commandline.Validator;
 import com.teamscale.jacoco.agent.configuration.AgentOptionReceiveException;
 import com.teamscale.jacoco.agent.configuration.ConfigurationViaTeamscale;
@@ -103,7 +104,12 @@ public static AgentOptions parse(String optionsString, String environmentConfigI
 			}
 		}
 
-		handleConfigFromEnvironment(options);
+		// we have to put the proxy options into system properties before reading the configuration from Teamscale as we
+		// might need them to connect to Teamscale
+		putTeamscaleProxyOptionsIntoSystemProperties(options);
+
+		handleConfigId(options);
+		handleConfigFile(options);
 
 		Validator validator = options.getValidator();
 		if (!validator.isValid()) {
@@ -112,12 +118,30 @@ public static AgentOptions parse(String optionsString, String environmentConfigI
 		return options;
 	}
 
-	private void handleConfigFromEnvironment(
-			AgentOptions options) throws AgentOptionParseException, AgentOptionReceiveException {
+	/**
+	 * Stores the agent options for proxies in the {@link TeamscaleProxySystemProperties} and overwrites
+	 * the password with the password found in the proxy-password-file if necessary.
+	 */
+	@VisibleForTesting
+	public static void putTeamscaleProxyOptionsIntoSystemProperties(AgentOptions options) {
+		options.getTeamscaleProxyOptions(ProxySystemProperties.Protocol.HTTP).putTeamscaleProxyOptionsIntoSystemProperties();
+		options.getTeamscaleProxyOptions(ProxySystemProperties.Protocol.HTTPS).putTeamscaleProxyOptionsIntoSystemProperties();
+	}
+
+	private void handleConfigId(AgentOptions options) throws AgentOptionReceiveException, AgentOptionParseException {
 		if (environmentConfigId != null) {
+			if (options.teamscaleServer.configId != null) {
+				logger.warn("You specified an ID for a profiler configuration in Teamscale both in the agent options and using an environment variable." +
+						" The environment variable will override the ID specified using the agent options." +
+						" Please use one or the other.");
+			}
 			handleOptionPart(options, "config-id=" + environmentConfigId);
 		}
 
+		readConfigFromTeamscale(options);
+	}
+
+	private void handleConfigFile(AgentOptions options) throws AgentOptionParseException, AgentOptionReceiveException {
 		if (environmentConfigFile != null) {
 			handleOptionPart(options, "config-file=" + environmentConfigFile);
 		}
@@ -233,7 +257,7 @@ private boolean handleAgentOptions(AgentOptions options, String key, String valu
 			throws AgentOptionParseException, AgentOptionReceiveException {
 		switch (key) {
 			case "config-id":
-				readConfigFromTeamscale(options, value);
+				storeConfigId(options, value);
 				return true;
 			case CONFIG_FILE_OPTION:
 				readConfigFromFile(options, filePatternResolver.parsePath(key, value).toFile());
@@ -304,14 +328,20 @@ private boolean handleAgentOptions(AgentOptions options, String key, String valu
 		}
 	}
 
-	private void readConfigFromTeamscale(AgentOptions options,
-										 String configId) throws AgentOptionParseException, AgentOptionReceiveException {
+	private void storeConfigId(AgentOptions options, String configId) throws AgentOptionParseException {
 		if (!options.teamscaleServer.isConfiguredForServerConnection()) {
 			throw new AgentOptionParseException(
 					"Has specified config-id '" + configId + "' without teamscale url/user/accessKey! The options need to be defined in teamscale.properties.");
 		}
 		options.teamscaleServer.configId = configId;
-		ConfigurationViaTeamscale configuration = ConfigurationViaTeamscale.retrieve(logger, configId,
+	}
+
+	private void readConfigFromTeamscale(AgentOptions options) throws AgentOptionParseException, AgentOptionReceiveException {
+		if(options.teamscaleServer.configId == null) {
+			return;
+		}
+
+		ConfigurationViaTeamscale configuration = ConfigurationViaTeamscale.retrieve(logger, options.teamscaleServer.configId,
 				options.teamscaleServer.url,
 				options.teamscaleServer.userName,
 				options.teamscaleServer.userAccessToken);

agent/src/test/java/com/teamscale/jacoco/agent/options/AgentOptionsTest.java

@@ -1,24 +1,34 @@
 package com.teamscale.jacoco.agent.options;
 
 import com.teamscale.client.CommitDescriptor;
+import com.teamscale.client.JsonUtils;
+import com.teamscale.client.ProfilerConfiguration;
+import com.teamscale.client.ProfilerRegistration;
 import com.teamscale.client.ProxySystemProperties;
 import com.teamscale.client.TeamscaleProxySystemProperties;
 import com.teamscale.client.TeamscaleServer;
-import com.teamscale.jacoco.agent.AgentBase;
 import com.teamscale.jacoco.agent.upload.artifactory.ArtifactoryConfig;
 import com.teamscale.jacoco.agent.util.TestUtils;
 import com.teamscale.report.util.CommandLineLogger;
 import okhttp3.HttpUrl;
+import okhttp3.mockwebserver.MockResponse;
+import okhttp3.mockwebserver.MockWebServer;
+import okhttp3.mockwebserver.RecordedRequest;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
 
+import java.io.BufferedWriter;
 import java.io.File;
+import java.io.FileWriter;
 import java.io.IOException;
 import java.net.URISyntaxException;
+import java.nio.charset.StandardCharsets;
+import java.util.Base64;
 import java.util.function.Predicate;
 
+import static com.teamscale.client.HttpUtils.PROXY_AUTHORIZATION_HTTP_HEADER;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -345,28 +355,107 @@ public void testTeamscaleProxyOptionsCorrectlySetSystemPropertiesForHttps() thro
 		testTeamscaleProxyOptionsCorrectlySetSystemProperties(ProxySystemProperties.Protocol.HTTPS);
 	}
 
+	/**
+	 * Temporary folder to create the password file for
+	 * {@link AgentOptionsTest#testTeamscaleProxyOptionsAreUsedWhileFetchingConfigFromTeamscale()}.
+	 */
+	@TempDir
+	public File temporaryDirectory;
+
+	/**
+	 * Test that the proxy settings are put into system properties and used for fetching a profiler configuration from
+	 * Teamscale. Also tests that it is possible to specify the proxy password in a file and that this overwrites the
+	 * password specified as agent option.
+	 */
+	@Test
+	public void testTeamscaleProxyOptionsAreUsedWhileFetchingConfigFromTeamscale() throws Exception {
+		String expectedUser = "user";
+		// this is the password passed as agent property, it should be overwritten by the password file
+		String unexpectedPassword = "not-my-password";
+
+		String expectedPassword = "password";
+		File passwordFile = writePasswortToPasswordFile(expectedPassword);
+
+		try (MockWebServer mockProxyServer = new MockWebServer()) {
+			String expectedHost = mockProxyServer.getHostName();
+			int expectedPort = mockProxyServer.getPort();
+
+
+			ProfilerConfiguration expectedProfilerConfiguration = new ProfilerConfiguration();
+			expectedProfilerConfiguration.configurationId = "config-id";
+			expectedProfilerConfiguration.configurationOptions = "mode=testwise\ntia-mode=disk";
+			ProfilerRegistration profilerRegistration = new ProfilerRegistration();
+			profilerRegistration.profilerId = "profiler-id";
+			profilerRegistration.profilerConfiguration = expectedProfilerConfiguration;
+
+			mockProxyServer.enqueue(new MockResponse().setResponseCode(407));
+			mockProxyServer.enqueue(new MockResponse().setResponseCode(200).setBody(JsonUtils.serialize(profilerRegistration)));
+
+			AgentOptions agentOptions= parseProxyOptions("config-id=config,", ProxySystemProperties.Protocol.HTTP, expectedHost, expectedPort, expectedUser, unexpectedPassword, passwordFile);
+
+			assertThat(agentOptions.configurationViaTeamscale.getProfilerConfiguration().configurationId).isEqualTo(expectedProfilerConfiguration.configurationId);
+			assertThat(agentOptions.mode).isEqualTo(EMode.TESTWISE);
+
+			// 2 requests: one without proxy authentication, which failed (407), one with proxy authentication
+			assertThat(mockProxyServer.getRequestCount()).isEqualTo(2);
+
+			mockProxyServer.takeRequest();
+			RecordedRequest requestWithProxyAuth = mockProxyServer.takeRequest(); // this is the interesting request
+
+			// check that the correct password was used
+			String base64EncodedBasicAuth = Base64.getEncoder().encodeToString((expectedUser + ":" + expectedPassword).getBytes(
+					StandardCharsets.UTF_8));
+			assertThat(requestWithProxyAuth.getHeader(PROXY_AUTHORIZATION_HTTP_HEADER)).isEqualTo("Basic " + base64EncodedBasicAuth);
+		}
+
+	}
+
+	private File writePasswortToPasswordFile(String expectedPassword) throws IOException {
+		File passwordFile = new File(temporaryDirectory, "password.txt");
+
+		BufferedWriter bufferedWriter = new BufferedWriter(new FileWriter(passwordFile));
+		bufferedWriter.write(expectedPassword);
+		bufferedWriter.close();
+
+		return passwordFile;
+	}
+
 	private void testTeamscaleProxyOptionsCorrectlySetSystemProperties(ProxySystemProperties.Protocol protocol) throws Exception {
 		String expectedHost = "host";
 		int expectedPort = 9999;
 		String expectedUser = "user";
 		String expectedPassword = "password";
-		String proxyHostOption = String.format("proxy-%s-host=%s", protocol, expectedHost);
-		String proxyPortOption = String.format("proxy-%s-port=%d", protocol, expectedPort);
-		String proxyUserOption = String.format("proxy-%s-user=%s", protocol, expectedUser);
-		String proxyPasswordOption = String.format("proxy-%s-password=%s", protocol, expectedPassword);
-		String optionsString = String.format("%s,%s,%s,%s", proxyHostOption, proxyPortOption, proxyUserOption, proxyPasswordOption);
-		AgentOptions agentOptions = getAgentOptionsParserWithDummyLogger().parse(optionsString);
+		AgentOptions agentOptions = parseProxyOptions("", protocol,
+				expectedHost, expectedPort, expectedUser, expectedPassword, null);
 
 		// clear to be sure the system properties are empty
 		clearTeamscaleProxySystemProperties(protocol);
 
-		AgentBase.putTeamscaleProxyOptionsIntoSystemProperties(agentOptions);
+		AgentOptionsParser.putTeamscaleProxyOptionsIntoSystemProperties(agentOptions);
 
 		assertTeamscaleProxySystemPropertiesAreCorrect(protocol, expectedHost, expectedPort, expectedUser, expectedPassword);
 
 		clearTeamscaleProxySystemProperties(protocol);
 	}
 
+	private static AgentOptions parseProxyOptions(String otherOptionsString, ProxySystemProperties.Protocol protocol,
+			String expectedHost, int expectedPort, String expectedUser,
+			String expectedPassword, File passwordFile) throws Exception {
+		String proxyHostOption = String.format("proxy-%s-host=%s", protocol, expectedHost);
+		String proxyPortOption = String.format("proxy-%s-port=%d", protocol, expectedPort);
+		String proxyUserOption = String.format("proxy-%s-user=%s", protocol, expectedUser);
+		String proxyPasswordOption = String.format("proxy-%s-password=%s", protocol, expectedPassword);
+		String optionsString = String.format("%s%s,%s,%s,%s", otherOptionsString, proxyHostOption, proxyPortOption, proxyUserOption, proxyPasswordOption);
+
+		if(passwordFile != null) {
+			String proxyPasswordFileOption = String.format("proxy-password-file=%s", passwordFile.getAbsoluteFile());
+			optionsString += "," + proxyPasswordFileOption;
+		}
+
+		TeamscaleCredentials credentials = new TeamscaleCredentials(HttpUrl.parse("http://localhost:80"), "unused", "unused");
+		return getAgentOptionsParserWithDummyLoggerAndCredentials(credentials).parse(optionsString);
+	}
+
 	private void assertTeamscaleProxySystemPropertiesAreCorrect(ProxySystemProperties.Protocol protocol, String expectedHost, int expectedPort, String expectedUser, String expectedPassword) throws ProxySystemProperties.IncorrectPortFormatException {
 		TeamscaleProxySystemProperties teamscaleProxySystemProperties = new TeamscaleProxySystemProperties(protocol);
 		assertThat(teamscaleProxySystemProperties.getProxyHost()).isEqualTo(expectedHost);
@@ -400,6 +489,10 @@ private static AgentOptionsParser getAgentOptionsParserWithDummyLogger() {
 		return new AgentOptionsParser(new CommandLineLogger(), null, null, null);
 	}
 
+	private static AgentOptionsParser getAgentOptionsParserWithDummyLoggerAndCredentials(TeamscaleCredentials credentials) {
+		return new AgentOptionsParser(new CommandLineLogger(), null, null, credentials);
+	}
+
 	/**
 	 * Delete created coverage folders
 	 */

agent/src/test/java/com/teamscale/jacoco/agent/testimpact/TestwiseCoverageAgentTest.java

@@ -5,16 +5,13 @@
 import com.teamscale.client.EReportFormat;
 import com.teamscale.client.PrioritizableTest;
 import com.teamscale.client.PrioritizableTestCluster;
-import com.teamscale.client.ProxySystemProperties;
 import com.teamscale.client.TeamscaleClient;
 import com.teamscale.client.TeamscaleServer;
 import com.teamscale.jacoco.agent.options.AgentOptions;
 import com.teamscale.jacoco.agent.options.ETestwiseCoverageMode;
-import com.teamscale.jacoco.agent.options.TeamscaleProxyOptions;
 import com.teamscale.jacoco.agent.util.TestUtils;
 import com.teamscale.report.testwise.jacoco.JaCoCoTestwiseReportGenerator;
 import com.teamscale.report.testwise.model.ETestExecutionResult;
-import com.teamscale.report.util.CommandLineLogger;
 import com.teamscale.tia.client.RunningTest;
 import com.teamscale.tia.client.TestRun;
 import com.teamscale.tia.client.TestRunWithClusteredSuggestions;
@@ -38,7 +35,6 @@
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
-import java.util.Objects;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatCode;
@@ -164,13 +160,7 @@ public void shouldHandleMissingRequestBodyForTestrunStartGracefully() throws Exc
 	private AgentOptions mockOptions(int port) {
 		AgentOptions options = mock(AgentOptions.class);
 		when(options.createTeamscaleClient()).thenReturn(client);
-		when(options.getTeamscaleProxyOptions(any(ProxySystemProperties.Protocol.class))).thenAnswer(invocation -> {
-			if (Objects.requireNonNull(
-					(ProxySystemProperties.Protocol) invocation.getArguments()[0]) == ProxySystemProperties.Protocol.HTTP) {
-				return new TeamscaleProxyOptions(ProxySystemProperties.Protocol.HTTP, new CommandLineLogger());
-			}
-			return new TeamscaleProxyOptions(ProxySystemProperties.Protocol.HTTPS, new CommandLineLogger());
-		});
+
 
 		TeamscaleServer server = new TeamscaleServer();
 		server.commit = new CommitDescriptor("branch", "12345");