ts-41920 move config id validation to ignore order of options

CHANGELOG.md

@@ -6,6 +6,8 @@ We use [semantic versioning](http://semver.org/):
 
 # Next version
 - [fix] _agent_: Fix wrong class description of GitMultiProjectPropertiesLocator in logs.
+- [fix] _agent_: Specifying `config-id` before Teamscale credentials led to option parsing errors. 
+
 # 34.2.2
 - [fix] _teamscale-gradle-plugin_: 401 Unauthorized error when trying to upload reports to Teamscale 
 

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

@@ -1,8 +1,10 @@
 package com.teamscale.jacoco.agent;
 
-import com.teamscale.client.HttpUtils;
 import com.teamscale.jacoco.agent.configuration.AgentOptionReceiveException;
+import com.teamscale.jacoco.agent.logging.DebugLogDirectoryPropertyDefiner;
+import com.teamscale.jacoco.agent.logging.LogDirectoryPropertyDefiner;
 import com.teamscale.jacoco.agent.logging.LogToTeamscaleAppender;
+import com.teamscale.jacoco.agent.logging.LoggingUtils;
 import com.teamscale.jacoco.agent.options.AgentOptionParseException;
 import com.teamscale.jacoco.agent.options.AgentOptions;
 import com.teamscale.jacoco.agent.options.AgentOptionsParser;
@@ -13,9 +15,6 @@
 import com.teamscale.jacoco.agent.testimpact.TestwiseCoverageAgent;
 import com.teamscale.jacoco.agent.upload.UploaderException;
 import com.teamscale.jacoco.agent.util.AgentUtils;
-import com.teamscale.jacoco.agent.logging.DebugLogDirectoryPropertyDefiner;
-import com.teamscale.jacoco.agent.logging.LogDirectoryPropertyDefiner;
-import com.teamscale.jacoco.agent.logging.LoggingUtils;
 import com.teamscale.report.util.ILogger;
 import org.conqat.lib.commons.collections.CollectionUtils;
 import org.conqat.lib.commons.collections.Pair;
@@ -131,7 +130,8 @@ private static Pair<AgentOptions, List<Exception>> getAndApplyAgentOptions(Strin
 
 		TeamscaleCredentials credentials = TeamscalePropertiesUtils.parseCredentials();
 		if (credentials == null) {
-			delayedLogger.warn("Did not find a teamscale.properties file!");
+			// With the current config mechanisms, this will almost always be shown.
+			delayedLogger.debug("No explicit teamscale.properties file given.");
 		}
 
 		Pair<AgentOptions, List<Exception>> parseResult;
@@ -160,7 +160,6 @@ private static Pair<AgentOptions, List<Exception>> getAndApplyAgentOptions(Strin
 		initializeLogging(agentOptions, delayedLogger);
 		Logger logger = LoggingUtils.getLogger(Agent.class);
 		delayedLogger.logTo(logger);
-		HttpUtils.setShouldValidateSsl(agentOptions.shouldValidateSsl());
 
 		return parseResult;
 	}

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

@@ -6,6 +6,7 @@
 package com.teamscale.jacoco.agent.options;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.teamscale.client.HttpUtils;
 import com.teamscale.client.ProxySystemProperties;
 import com.teamscale.client.StringUtils;
 import com.teamscale.client.TeamscaleProxySystemProperties;
@@ -24,6 +25,7 @@
 import org.conqat.lib.commons.collections.Pair;
 import org.conqat.lib.commons.filesystem.FileSystemUtils;
 
+import javax.annotation.Nullable;
 import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.IOException;
@@ -68,7 +70,7 @@ public class AgentOptionsParser {
 	 */
 	public static Pair<AgentOptions, List<Exception>> parse(String optionsString, String environmentConfigId,
 			String environmentConfigFile,
-			TeamscaleCredentials credentials,
+			@Nullable TeamscaleCredentials credentials,
 			ILogger logger) throws AgentOptionParseException, AgentOptionReceiveException {
 		AgentOptionsParser parser = new AgentOptionsParser(logger, environmentConfigId, environmentConfigFile,
 				credentials);
@@ -136,6 +138,8 @@ public void throwOnCollectedErrors() throws Exception {
 		// 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);
+		// Same as above, with the ssl validation.
+		HttpUtils.setShouldValidateSsl(options.shouldValidateSsl());
 
 		handleConfigId(options);
 		handleConfigFile(options);
@@ -384,11 +388,7 @@ private boolean handleAgentOptions(AgentOptions options, String key, String valu
 		}
 	}
 
-	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.");
-		}
+	private void storeConfigId(AgentOptions options, String configId) {
 		options.teamscaleServer.configId = configId;
 	}
 
@@ -397,7 +397,10 @@ private void readConfigFromTeamscale(
 		if (options.teamscaleServer.configId == null) {
 			return;
 		}
-
+		if (!options.teamscaleServer.isConfiguredForServerConnection()) {
+			throw new AgentOptionParseException(
+					"Config-id '" + options.teamscaleServer.configId + "' specified without teamscale url/user/accessKey! These options must be provided locally via config-file or command line argument.");
+		}
 		ConfigurationViaTeamscale configuration = ConfigurationViaTeamscale.retrieve(logger,
 				options.teamscaleServer.configId,
 				options.teamscaleServer.url,

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

@@ -194,6 +194,19 @@ public void sapNwdiRequiresAllTeamscaleOptionsExceptProject() {
 				.hasMessageNotContaining("the 'teamscale-' upload options are incomplete");
 	}
 
+	/**
+	 * Test that we can define a config id first, before adding teamscale server credentials. We still expect an
+	 * exception to be thrown, because there is no Teamscale server to reach, but no parse exception.
+	 */
+	@Test
+	public void testConfigIdOptionOrderIrrelevant() {
+		assertThatThrownBy(() -> parseAndThrow(
+				"config-id=myConfig,teamscale-server-url=http://awesome-teamscale.com,teamscale-user=user,teamscale-access-token=mycoolkey")).hasMessageNotContaining(
+						"Failed to parse agent options")
+				.hasMessageContaining(
+						"Failed to retrieve profiler configuration from Teamscale!");
+	}
+
 	@Test
 	public void revisionOrCommitRequireProject() {
 		assertThatThrownBy(