Merge pull request #655 from cqse/ts/41920_config_options_ordering

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

@@ -2,7 +2,10 @@
 
 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 +16,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 +131,10 @@ private static Pair<AgentOptions, List<Exception>> getAndApplyAgentOptions(Strin
 
 		TeamscaleCredentials credentials = TeamscalePropertiesUtils.parseCredentials();
 		if (credentials == null) {
-			delayedLogger.warn("Did not find a teamscale.properties file!");
+			// As many users still don't use the installer based setup, this log message will be shown in almost every log.
+			// We use a debug log, as this message can be confusing for customers that think a teamscale.properties file is synonymous with a config file.
+			delayedLogger.debug(
+					"No explicit teamscale.properties file given. Looking for Teamscale credentials in a config file or via a command line argument. This is expected unless the installer based setup was used.");
 		}
 
 		Pair<AgentOptions, List<Exception>> 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);
@@ -296,7 +298,7 @@ private boolean handleAgentOptions(AgentOptions options, String key, String valu
 			throws AgentOptionParseException, AgentOptionReceiveException {
 		switch (key) {
 			case "config-id":
-				storeConfigId(options, value);
+				options.teamscaleServer.configId = value;
 				return true;
 			case CONFIG_FILE_OPTION:
 				readConfigFromFile(options, parsePath(filePatternResolver, key, value).toFile());
@@ -384,20 +386,17 @@ 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.");
-		}
-		options.teamscaleServer.configId = configId;
-	}
-
 	private void readConfigFromTeamscale(
 			AgentOptions options) throws AgentOptionParseException, AgentOptionReceiveException {
 		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.");
+		}
+		// Set ssl validation option in case it needs to be off before trying to reach Teamscale.
+		HttpUtils.setShouldValidateSsl(options.shouldValidateSsl());
 		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(