diff --git a/build.sbt b/build.sbt index 807c6178..22c7824b 100644 --- a/build.sbt +++ b/build.sbt @@ -20,7 +20,7 @@ ThisBuild / organization := "app.softnetwork" name := "softclient4es" -ThisBuild / version := "0.20.1" +ThisBuild / version := "0.20.2-SNAPSHOT" ThisBuild / scalaVersion := scala213 diff --git a/core/src/main/resources/softnetwork-elastic.conf b/core/src/main/resources/softnetwork-elastic.conf index acbcc45e..17fc8b69 100644 --- a/core/src/main/resources/softnetwork-elastic.conf +++ b/core/src/main/resources/softnetwork-elastic.conf @@ -11,7 +11,8 @@ elastic { port = 9200 port = ${?ELASTIC_PORT} - method = "noauth" # Options: "noauth", "basic", "api-key", "bearer-token" + # No default: absent unless ELASTIC_AUTH_METHOD is set, so ElasticCredentials + # auto-detects the method from the supplied credentials (basic / api-key / bearer). method = ${?ELASTIC_AUTH_METHOD} username = "" diff --git a/core/src/main/scala/app/softnetwork/elastic/client/Cli.scala b/core/src/main/scala/app/softnetwork/elastic/client/Cli.scala index 173bf175..4dc144bd 100644 --- a/core/src/main/scala/app/softnetwork/elastic/client/Cli.scala +++ b/core/src/main/scala/app/softnetwork/elastic/client/Cli.scala @@ -28,7 +28,7 @@ object Cli extends App { implicit val ec: ExecutionContext = system.dispatcher // Parse command line arguments - val config = parseArgs(args) + val config = CliConfig.parseArgs(args) try { val gateway = ElasticClientFactory.createWithMonitoring(config.elasticConfig) @@ -62,169 +62,4 @@ object Cli extends App { // Cleanup system.terminate() } - - // ==================== Argument Parsing ==================== - - private def parseArgs(args: Array[String]): CliConfig = { - var scheme = "http" - var host = "localhost" - var port = 9200 - var username: Option[String] = None - var password: Option[String] = None - var apiKey: Option[String] = None - var bearerToken: Option[String] = None - var executeFile: Option[String] = None - var executeCommand: Option[String] = None - var promptPassword = false - - var i = 0 - while (i < args.length) { - args(i) match { - case "-s" | "--scheme" => - scheme = args(i + 1) - i += 2 - - case "-h" | "--host" => - host = args(i + 1) - i += 2 - - case "-p" | "--port" => - port = args(i + 1).toInt - i += 2 - - case "-u" | "--username" => - username = Some(args(i + 1)) - i += 2 - - case "-P" | "--password" => - password = Some(args(i + 1)) - i += 2 - - case "-W" => - promptPassword = true - i += 1 - - case "-k" | "--api-key" => - apiKey = Some(args(i + 1)) - i += 2 - - case "-b" | "--bearer-token" => - bearerToken = Some(args(i + 1)) - i += 2 - - case "-f" | "--file" => - executeFile = Some(args(i + 1)) - i += 2 - - case "-c" | "--command" => - executeCommand = Some(args(i + 1)) - i += 2 - - case "--help" => - printUsage() - System.exit(0) - - case unknown => - System.err.println(s"Unknown argument: $unknown") - printUsage() - System.exit(1) - } - } - - if (promptPassword) { - val console = System.console() - if (console == null) { - System.err.println("Error: -W requires an interactive terminal") - System.exit(1) - } - System.err.print("Enter password: ") - System.err.flush() - password = Some(new String(console.readPassword())) - } - - CliConfig( - scheme, - host, - port, - username, - password, - apiKey, - bearerToken, - executeFile, - executeCommand - ) - } - - private def printUsage(): Unit = { - println( - """ - |Elasticsearch SQL CLI - | - |Usage: - | softclient4es [OPTIONS] - | - |Options: - | -s, --scheme Connection scheme (http or https, default: http) - | -h, --host Elasticsearch host (default: localhost) - | -p, --port Elasticsearch port (default: 9200) - | -u, --username Username for authentication - | -P, --password Password for authentication - | -W Prompt for password interactively (input not echoed) - | -k, --api-key API key for authentication - | -b, --bearer-token Bearer token for authentication - | -f, --file Execute SQL from file and exit - | -c, --command Execute SQL command and exit - | --help Show this help message - | - |Examples: - | # Start interactive REPL - | softclient4es - | - | # Connect to remote host - | softclient4es -h prod-es.example.com -p 9200 - | - | # Execute SQL file - | softclient4es -f queries.sql - | - | # Execute single command - | softclient4es -c "SELECT * FROM users LIMIT 10" - | - |Interactive Commands: - | help (\h) Display help information - | quit (\q) Exit the REPL - | exit (\q) Exit the REPL - | history Display command history - | clear Clear the screen - | timing Toggle timing display ON/OFF - | format Set or show output format - | timeout Set or show query timeout - | - | - |Table Commands: - | tables (\t) List all tables - | \st Show table details - | \ct
Show table ddl - | \dt
Describe table schema - | - |Pipeline Commands: - | pipelines (\p) List all pipelines - | \sp Show pipeline details - | \cp Show pipeline ddl - | \dp Describe pipeline schema - | - |Watcher Commands: - | watchers (\w) List all watchers - | \sw Show watcher status - | - |Policy Commands: - | policies (\pol) List all enrich policies - | \sl Show enrich policy details - | - |Stream Commands: - | consume (\c) Consume streaming results from last query - | stream (\s) Show stream status - | cancel (\x) Cancel active stream - |""".stripMargin - ) - } } diff --git a/core/src/main/scala/app/softnetwork/elastic/client/CliConfig.scala b/core/src/main/scala/app/softnetwork/elastic/client/CliConfig.scala index ccae7e56..598e5694 100644 --- a/core/src/main/scala/app/softnetwork/elastic/client/CliConfig.scala +++ b/core/src/main/scala/app/softnetwork/elastic/client/CliConfig.scala @@ -17,37 +17,314 @@ package app.softnetwork.elastic.client import app.softnetwork.elastic.client.repl.ReplConfig -import com.typesafe.config.{Config, ConfigFactory} +import com.typesafe.config.{Config, ConfigFactory, ConfigValueFactory} +import com.typesafe.scalalogging.LazyLogging +/** CLI configuration. + * + * Connection settings are `Option`s: `Some` ONLY when the user actually passed the corresponding + * flag. Unset flags are OMITTED from the primary config layer so the standard Typesafe fallback + * chain can supply them: + * + * CLI flag > ELASTIC_* env var > -Dconfig.file (conf/application.conf) > built-in + * softnetwork-elastic.conf (hard defaults http / localhost / 9200) + */ case class CliConfig( - scheme: String, - host: String, - port: Int, - username: Option[String], - password: Option[String], - apiKey: Option[String], - bearerToken: Option[String], - executeFile: Option[String], - executeCommand: Option[String], + scheme: Option[String] = None, + host: Option[String] = None, + port: Option[Int] = None, + username: Option[String] = None, + password: Option[String] = None, + apiKey: Option[String] = None, + bearerToken: Option[String] = None, + executeFile: Option[String] = None, + executeCommand: Option[String] = None, replConfig: ReplConfig = ReplConfig.default ) { - private lazy val elasticConfigAsString: String = - s""" - |elastic { - | credentials { - | scheme = "$scheme" - | host = "$host" - | port = $port - | username = ${username.map(u => s""""$u"""").getOrElse("null")} - | password = ${password.map(p => s""""$p"""").getOrElse("null")} - | api-key = ${apiKey.map(k => s""""$k"""").getOrElse("null")} - | bearer-token = ${bearerToken.map(t => s""""$t"""").getOrElse("null")} - | } - |} - |""".stripMargin - - lazy val elasticConfig: Config = ConfigFactory - .parseString(elasticConfigAsString) - .withFallback(ConfigFactory.load("softnetwork-elastic.conf")) + import CliConfig._ + + lazy val elasticConfig: Config = buildElasticConfig(sys.env.get) + + /** Test seam. `env` abstracts `sys.env.get` (JVM env vars are immutable in-process); `external` + * abstracts the -Dconfig.file layer (unit tests inject a parsed Config instead of mutating the + * JVM-global `config.file` system property - see CliConfigSpec). + * + * The `external` default MUST stay the no-arg `ConfigFactory.load()`: it is the only load + * variant that honours -Dconfig.file, which the shipped launcher passes. Replacing it with + * `load("softnetwork-elastic.conf")` silently reintroduces issue #162(b) - the unit matrix + * cannot catch that (it injects `external`); only the published-style manual smoke exercises + * this default end-to-end. + */ + private[client] def buildElasticConfig( + env: String => Option[String], + external: Config = ConfigFactory.load() + ): Config = { + val cliLayer: Config = layerOf( + Seq( + scheme.map(SchemePath -> _), + host.map(HostPath -> _), + port.map(p => PortPath -> (Int.box(p): AnyRef)), + username.map(UsernamePath -> _), + password.map(PasswordPath -> _), + apiKey.map(ApiKeyPath -> _), + bearerToken.map(BearerTokenPath -> _) + ).flatten + ) + // Built-in defaults tail: http / localhost / 9200 + metrics/discovery/watcher blocks. + val builtinLayer: Config = ConfigFactory.load("softnetwork-elastic.conf") + sanitize( + cliLayer + .withFallback(envLayer(env)) + .withFallback(external) + .withFallback(builtinLayer) + ) + } +} + +object CliConfig extends LazyLogging { + + private[client] val SchemePath = "elastic.credentials.scheme" + private[client] val HostPath = "elastic.credentials.host" + private[client] val PortPath = "elastic.credentials.port" + private[client] val UsernamePath = "elastic.credentials.username" + private[client] val PasswordPath = "elastic.credentials.password" + private[client] val ApiKeyPath = "elastic.credentials.api-key" + private[client] val BearerTokenPath = "elastic.credentials.bearer-token" + + private def layerOf(entries: Seq[(String, AnyRef)]): Config = + entries.foldLeft(ConfigFactory.empty()) { case (config, (path, value)) => + config.withValue(path, ConfigValueFactory.fromAnyRef(value)) + } + + /** Explicit env layer. An env var set to the EMPTY (or whitespace-only) string is treated as + * UNSET - HOCON `${?VAR}` substitution would consider it present and override every fallback. + * + * Value handling: the PRESENCE check trims, but credential VALUES pass through verbatim - a + * password/token may legitimately contain leading or trailing whitespace, and mutating it breaks + * authentication with correct credentials. Connection coordinates (scheme/host/port) ARE + * trimmed: whitespace there is always accidental and would corrupt the URL / crash the port + * reader. + * + * Alias precedence (first match wins): + * - host: ELASTIC_IP beats ELASTIC_HOST (parity with softnetwork-elastic.conf, where the later + * `host = ${?ELASTIC_IP}` line wins) + * - auth settings: installer-documented ELASTIC_ beats library-internal + * ELASTIC_CREDENTIALS_ + */ + private[client] def envLayer(env: String => Option[String]): Config = { + def firstNonBlank(names: String*): Option[String] = + names.flatMap(name => env(name).filter(_.trim.nonEmpty)).headOption + def firstCoordinate(names: String*): Option[String] = + firstNonBlank(names: _*).map(_.trim) + layerOf( + Seq( + firstCoordinate("ELASTIC_SCHEME").map(SchemePath -> _), + firstCoordinate("ELASTIC_IP", "ELASTIC_HOST").map(HostPath -> _), + firstCoordinate("ELASTIC_PORT").map(PortPath -> _), + firstNonBlank("ELASTIC_USERNAME", "ELASTIC_CREDENTIALS_USERNAME") + .map(UsernamePath -> _), + firstNonBlank("ELASTIC_PASSWORD", "ELASTIC_CREDENTIALS_PASSWORD") + .map(PasswordPath -> _), + firstNonBlank("ELASTIC_API_KEY", "ELASTIC_CREDENTIALS_API_KEY") + .map(ApiKeyPath -> _), + firstNonBlank("ELASTIC_BEARER_TOKEN", "ELASTIC_CREDENTIALS_BEARER_TOKEN") + .map(BearerTokenPath -> _) + ).flatten + ) + } + + /** Last-resort guard: a connection-critical setting that resolved EMPTY (e.g. an empty env var + * leaking through a `${?VAR}` substitution inside a conf file, bypassing the launcher filter) + * falls back to the hard default instead of producing `http://:9200` or a ConfigReader crash on + * `port = ""`. `hasPath` is false for HOCON `null`, so `key = null` in a user file is also + * repaired. + */ + private[client] def sanitize(resolved: Config): Config = + Seq[(String, AnyRef)]( + SchemePath -> "http", + HostPath -> "localhost", + PortPath -> Int.box(9200) + ).foldLeft(resolved) { case (config, (path, default)) => + val missingOrEmpty = + !config.hasPath(path) || config.getString(path).trim.isEmpty + if (missingOrEmpty) { + logger.warn( + s"Connection setting '$path' resolved to an empty value - falling back to default '$default'" + ) + config.withValue(path, ConfigValueFactory.fromAnyRef(default)) + } else { + config + } + } + + // ==================== Argument Parsing (moved from Cli — Cli extends App and is + // therefore untestable without triggering its delayedInit main body) ==================== + + def parseArgs(args: Array[String]): CliConfig = { + var scheme: Option[String] = None + var host: Option[String] = None + var port: Option[Int] = None + var username: Option[String] = None + var password: Option[String] = None + var apiKey: Option[String] = None + var bearerToken: Option[String] = None + var executeFile: Option[String] = None + var executeCommand: Option[String] = None + var promptPassword = false + + var i = 0 + while (i < args.length) { + args(i) match { + case "-s" | "--scheme" => + scheme = Some(args(i + 1)) + i += 2 + + case "-h" | "--host" => + host = Some(args(i + 1)) + i += 2 + + case "-p" | "--port" => + port = Some(args(i + 1).toInt) + i += 2 + + case "-u" | "--username" => + username = Some(args(i + 1)) + i += 2 + + case "-P" | "--password" => + password = Some(args(i + 1)) + i += 2 + + case "-W" => + promptPassword = true + i += 1 + + case "-k" | "--api-key" => + apiKey = Some(args(i + 1)) + i += 2 + + case "-b" | "--bearer-token" => + bearerToken = Some(args(i + 1)) + i += 2 + + case "-f" | "--file" => + executeFile = Some(args(i + 1)) + i += 2 + + case "-c" | "--command" => + executeCommand = Some(args(i + 1)) + i += 2 + + case "--help" => + printUsage() + System.exit(0) + + case unknown => + System.err.println(s"Unknown argument: $unknown") + printUsage() + System.exit(1) + } + } + + if (promptPassword) { + val console = System.console() + if (console == null) { + System.err.println("Error: -W requires an interactive terminal") + System.exit(1) + } + System.err.print("Enter password: ") + System.err.flush() + password = Some(new String(console.readPassword())) + } + + CliConfig( + scheme, + host, + port, + username, + password, + apiKey, + bearerToken, + executeFile, + executeCommand + ) + } + + private[client] def printUsage(): Unit = { + println( + """ + |Elasticsearch SQL CLI + | + |Usage: + | softclient4es [OPTIONS] + | + |Options: + | -s, --scheme Connection scheme (http or https, default: http) + | -h, --host Elasticsearch host (default: localhost) + | -p, --port Elasticsearch port (default: 9200) + | -u, --username Username for authentication + | -P, --password Password for authentication + | -W Prompt for password interactively (input not echoed) + | -k, --api-key API key for authentication + | -b, --bearer-token Bearer token for authentication + | -f, --file Execute SQL from file and exit + | -c, --command Execute SQL command and exit + | --help Show this help message + | + |Configuration precedence: + | CLI flag > ELASTIC_* environment variable > conf/application.conf (-Dconfig.file) + | > built-in defaults (http://localhost:9200) + | + |Examples: + | # Start interactive REPL + | softclient4es + | + | # Connect to remote host + | softclient4es -h prod-es.example.com -p 9200 + | + | # Execute SQL file + | softclient4es -f queries.sql + | + | # Execute single command + | softclient4es -c "SELECT * FROM users LIMIT 10" + | + |Interactive Commands: + | help (\h) Display help information + | quit (\q) Exit the REPL + | exit (\q) Exit the REPL + | history Display command history + | clear Clear the screen + | timing Toggle timing display ON/OFF + | format Set or show output format + | timeout Set or show query timeout + | + | + |Table Commands: + | tables (\t) List all tables + | \st
Show table details + | \ct
Show table ddl + | \dt
Describe table schema + | + |Pipeline Commands: + | pipelines (\p) List all pipelines + | \sp Show pipeline details + | \cp Show pipeline ddl + | \dp Describe pipeline schema + | + |Watcher Commands: + | watchers (\w) List all watchers + | \sw Show watcher status + | + |Policy Commands: + | policies (\pol) List all enrich policies + | \sl Show enrich policy details + | + |Stream Commands: + | consume (\c) Consume streaming results from last query + | stream (\s) Show stream status + | cancel (\x) Cancel active stream + |""".stripMargin + ) + } } diff --git a/core/src/test/scala/app/softnetwork/elastic/client/CliConfigSpec.scala b/core/src/test/scala/app/softnetwork/elastic/client/CliConfigSpec.scala new file mode 100644 index 00000000..ee3ab239 --- /dev/null +++ b/core/src/test/scala/app/softnetwork/elastic/client/CliConfigSpec.scala @@ -0,0 +1,318 @@ +/* + * Copyright 2025 SOFTNETWORK + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package app.softnetwork.elastic.client + +import com.typesafe.config.{Config, ConfigFactory} +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +/** Precedence matrix for the REPL/CLI connection settings (issue #162): + * + * CLI flag > ELASTIC_* env var > -Dconfig.file > built-in defaults + * + * Env vars are stubbed through the `env` seam parameter (JVM env is immutable in-process). The + * config-file layer is stubbed through the `external` seam parameter - NEVER via the `config.file` + * system property: that sysprop plus `ConfigFactory.invalidateCaches()` are JVM-global, `Test / + * parallelExecution := false` only covers the ROOT project, and other suites (licensing) call + * no-arg `ConfigFactory.load()` concurrently. The production default (`external = + * ConfigFactory.load()`) is exercised end-to-end by the manual smoke on a published-style install. + * + * PRECONDITION: the test JVM must not itself run with ELASTIC_* connection env vars exported - the + * built-in softnetwork-elastic.conf resolves them for real. Guarded by `assume` below (skips, not + * fails, on a polluted environment). + */ +class CliConfigSpec extends AnyFlatSpec with Matchers { + + private val noEnv: String => Option[String] = _ => None + + private def envOf(pairs: (String, String)*): String => Option[String] = + pairs.toMap.get + + /** The `-Dconfig.file` layer, injected as a parsed Config (literals only - no substitutions + * needed for the matrix). + */ + private def fileOf(content: String): Config = ConfigFactory.parseString(content) + + private def credentials(config: Config, key: String): String = + config.getString(s"elastic.credentials.$key") + + private def assumeCleanEnvironment(): Unit = + Seq( + "ELASTIC_SCHEME", + "ELASTIC_HOST", + "ELASTIC_IP", + "ELASTIC_PORT", + "ELASTIC_AUTH_METHOD", + "ELASTIC_USERNAME", + "ELASTIC_PASSWORD", + "ELASTIC_API_KEY", + "ELASTIC_BEARER_TOKEN", + "ELASTIC_CREDENTIALS_USERNAME", + "ELASTIC_CREDENTIALS_PASSWORD", + "ELASTIC_CREDENTIALS_API_KEY", + "ELASTIC_CREDENTIALS_BEARER_TOKEN" + ).foreach(name => assume(sys.env.get(name).forall(_.trim.isEmpty))) + + behavior of "CliConfig connection precedence" + + it should "fall back to http/localhost/9200 when nothing is set anywhere (AC 4)" in { + assumeCleanEnvironment() + // production path: default external layer (real no-arg ConfigFactory.load()) + val config = CliConfig().buildElasticConfig(noEnv) + credentials(config, "scheme") shouldBe "http" + credentials(config, "host") shouldBe "localhost" + config.getInt("elastic.credentials.port") shouldBe 9200 + credentials(config, "username") shouldBe "" + credentials(config, "password") shouldBe "" + credentials(config, "api-key") shouldBe "" + credentials(config, "bearer-token") shouldBe "" + } + + it should "let an env var configure the connection when no flag is passed (AC 1)" in { + assumeCleanEnvironment() + val config = CliConfig().buildElasticConfig(envOf("ELASTIC_PORT" -> "19200")) + config.getInt("elastic.credentials.port") shouldBe 19200 + credentials(config, "host") shouldBe "localhost" // untouched settings keep defaults + } + + it should "let the config-file layer configure the connection (AC 2)" in { + assumeCleanEnvironment() + val config = CliConfig().buildElasticConfig( + noEnv, + external = fileOf("elastic.credentials.port = 19200") + ) + config.getInt("elastic.credentials.port") shouldBe 19200 + credentials(config, "host") shouldBe "localhost" + } + + it should "rank flag over env var over config file over default for port (AC 3)" in { + assumeCleanEnvironment() + val file = fileOf("elastic.credentials.port = 29200") + // file beats default + CliConfig() + .buildElasticConfig(noEnv, external = file) + .getInt("elastic.credentials.port") shouldBe 29200 + // env beats file + CliConfig() + .buildElasticConfig(envOf("ELASTIC_PORT" -> "19200"), external = file) + .getInt("elastic.credentials.port") shouldBe 19200 + // flag beats env and file + CliConfig(port = Some(1234)) + .buildElasticConfig(envOf("ELASTIC_PORT" -> "19200"), external = file) + .getInt("elastic.credentials.port") shouldBe 1234 + } + + it should "apply the same precedence to every connection setting (AC 5)" in { + assumeCleanEnvironment() + case class Case( + key: String, + withFlag: CliConfig, + envName: String, + flagValue: String + ) + val cases = Seq( + Case("scheme", CliConfig(scheme = Some("https")), "ELASTIC_SCHEME", "https"), + Case("host", CliConfig(host = Some("flag-host")), "ELASTIC_HOST", "flag-host"), + Case( + "username", + CliConfig(username = Some("flag-user")), + "ELASTIC_USERNAME", + "flag-user" + ), + Case( + "password", + CliConfig(password = Some("flag-pass")), + "ELASTIC_PASSWORD", + "flag-pass" + ), + Case("api-key", CliConfig(apiKey = Some("flag-key")), "ELASTIC_API_KEY", "flag-key"), + Case( + "bearer-token", + CliConfig(bearerToken = Some("flag-token")), + "ELASTIC_BEARER_TOKEN", + "flag-token" + ) + ) + cases.foreach { c => + val file = fileOf(s"""elastic.credentials.${c.key} = "from-file"""") + // file beats default + credentials( + CliConfig().buildElasticConfig(noEnv, external = file), + c.key + ) shouldBe "from-file" + // env beats file + credentials( + CliConfig().buildElasticConfig(envOf(c.envName -> "from-env"), external = file), + c.key + ) shouldBe "from-env" + // flag beats env and file + credentials( + c.withFlag.buildElasticConfig(envOf(c.envName -> "from-env"), external = file), + c.key + ) shouldBe c.flagValue + } + } + + it should "treat an env var set to the empty string as unset (AC 8)" in { + assumeCleanEnvironment() + // empty env must NOT mask the file value + credentials( + CliConfig().buildElasticConfig( + envOf("ELASTIC_HOST" -> ""), + external = fileOf("""elastic.credentials.host = "file-host"""") + ), + "host" + ) shouldBe "file-host" + // empty env with no file: default survives, no crash on port + val config = CliConfig().buildElasticConfig( + envOf("ELASTIC_HOST" -> "", "ELASTIC_PORT" -> "", "ELASTIC_SCHEME" -> " ") + ) + credentials(config, "host") shouldBe "localhost" + credentials(config, "scheme") shouldBe "http" + config.getInt("elastic.credentials.port") shouldBe 9200 + } + + it should "keep credential values verbatim while trimming connection coordinates (AC 12)" in { + assumeCleanEnvironment() + val config = CliConfig().buildElasticConfig( + envOf( + "ELASTIC_HOST" -> " remote-host ", + "ELASTIC_USERNAME" -> "user", + "ELASTIC_PASSWORD" -> " spacey pass " + ) + ) + credentials(config, "host") shouldBe "remote-host" // coordinates trimmed + credentials(config, "password") shouldBe " spacey pass " // credentials verbatim + } + + it should "repair an empty value leaking from a config file (sanitize guard)" in { + assumeCleanEnvironment() + val config = CliConfig().buildElasticConfig( + noEnv, + external = fileOf(""" + elastic.credentials.host = "" + elastic.credentials.scheme = "" + elastic.credentials.port = "" + """) + ) + credentials(config, "host") shouldBe "localhost" + credentials(config, "scheme") shouldBe "http" + config.getInt("elastic.credentials.port") shouldBe 9200 + } + + it should "honour env alias precedence (ELASTIC_IP > ELASTIC_HOST, ELASTIC_USERNAME > ELASTIC_CREDENTIALS_USERNAME)" in { + assumeCleanEnvironment() + credentials( + CliConfig().buildElasticConfig( + envOf("ELASTIC_IP" -> "10.0.0.1", "ELASTIC_HOST" -> "other-host") + ), + "host" + ) shouldBe "10.0.0.1" + credentials( + CliConfig().buildElasticConfig(envOf("ELASTIC_CREDENTIALS_USERNAME" -> "legacy")), + "username" + ) shouldBe "legacy" + credentials( + CliConfig().buildElasticConfig( + envOf( + "ELASTIC_USERNAME" -> "modern", + "ELASTIC_CREDENTIALS_USERNAME" -> "legacy" + ) + ), + "username" + ) shouldBe "modern" + } + + behavior of "CliConfig auth method auto-detection (AC 11)" + + it should "auto-detect the auth method from supplied credentials once method has no builtin default" in { + assumeCleanEnvironment() + ElasticConfig( + CliConfig(username = Some("elastic"), password = Some("changeme")) + .buildElasticConfig(noEnv) + ).credentials.authMethod shouldBe Some(BasicAuth) + ElasticConfig( + CliConfig().buildElasticConfig( + envOf("ELASTIC_USERNAME" -> "elastic", "ELASTIC_PASSWORD" -> "changeme") + ) + ).credentials.authMethod shouldBe Some(BasicAuth) + ElasticConfig( + CliConfig(apiKey = Some("key")).buildElasticConfig(noEnv) + ).credentials.authMethod shouldBe Some(ApiKeyAuth) + ElasticConfig( + CliConfig(bearerToken = Some("token")).buildElasticConfig(noEnv) + ).credentials.authMethod shouldBe Some(BearerTokenAuth) + // no credentials anywhere: no auth, exactly as before the fix + ElasticConfig( + CliConfig().buildElasticConfig(noEnv) + ).credentials.authMethod shouldBe None + // an explicit method still wins over auto-detection + ElasticConfig( + CliConfig(username = Some("elastic"), password = Some("changeme")) + .buildElasticConfig( + noEnv, + external = fileOf("""elastic.credentials.method = "noauth"""") + ) + ).credentials.authMethod shouldBe Some(NoAuth) + } + + behavior of "CliConfig.parseArgs" + + it should "leave every connection setting unset when no flag is passed (AC 10)" in { + val parsed = CliConfig.parseArgs(Array.empty) + parsed.scheme shouldBe None + parsed.host shouldBe None + parsed.port shouldBe None + parsed.username shouldBe None + parsed.password shouldBe None + parsed.apiKey shouldBe None + parsed.bearerToken shouldBe None + parsed.executeFile shouldBe None + parsed.executeCommand shouldBe None + } + + it should "capture exactly the flags that were passed (AC 3, 10)" in { + val parsed = CliConfig.parseArgs( + Array("-s", "https", "-h", "example.com", "-p", "19200", "-c", "SHOW TABLES") + ) + parsed.scheme shouldBe Some("https") + parsed.host shouldBe Some("example.com") + parsed.port shouldBe Some(19200) + parsed.executeCommand shouldBe Some("SHOW TABLES") + parsed.username shouldBe None + + val auth = CliConfig.parseArgs( + Array( + "--username", + "user", + "--password", + "pass", + "--api-key", + "key", + "--bearer-token", + "token", + "--file", + "queries.sql" + ) + ) + auth.username shouldBe Some("user") + auth.password shouldBe Some("pass") + auth.apiKey shouldBe Some("key") + auth.bearerToken shouldBe Some("token") + auth.executeFile shouldBe Some("queries.sql") + } +} diff --git a/documentation/client/repl.md b/documentation/client/repl.md index c90e2383..cea068df 100644 --- a/documentation/client/repl.md +++ b/documentation/client/repl.md @@ -290,6 +290,21 @@ $env:PATH += ";$env:USERPROFILE\softclient4es\bin" ## Connection +### Configuration Precedence + +Connection settings are resolved with the following precedence (highest first): + +```text +CLI flag > ELASTIC_* environment variable > conf/application.conf (-Dconfig.file) > built-in defaults (http://localhost:9200) +``` + +Notes: + +- An environment variable set to the **empty string** (or whitespace only) is treated as **unset** — it never masks a value from the configuration file or the defaults. +- Credential values (`username`, `password`, `api-key`, `bearer-token`) are passed through **verbatim**; `scheme`, `host` and `port` values are trimmed. +- `ELASTIC_IP` takes precedence over `ELASTIC_HOST`; the installer-documented `ELASTIC_USERNAME`/`ELASTIC_PASSWORD`/`ELASTIC_API_KEY`/`ELASTIC_BEARER_TOKEN` names take precedence over the library-internal `ELASTIC_CREDENTIALS_*` forms (both keep working). +- When no explicit authentication `method` is configured, the client **auto-detects** it from the supplied credentials: API key > bearer token > basic auth > none. + ### Configuration File The REPL reads default connection settings from `conf/application.conf`: diff --git a/es6/jest/src/main/scala/app/softnetwork/elastic/client/jest/JestClientCompanion.scala b/es6/jest/src/main/scala/app/softnetwork/elastic/client/jest/JestClientCompanion.scala index 5d635140..afbaf667 100644 --- a/es6/jest/src/main/scala/app/softnetwork/elastic/client/jest/JestClientCompanion.scala +++ b/es6/jest/src/main/scala/app/softnetwork/elastic/client/jest/JestClientCompanion.scala @@ -123,11 +123,16 @@ trait JestClientCompanion extends ElasticClientCompanion[JestClient] with Loggin logger.info("🔐 Configuring Bearer Token Auth") builder - case Some(NoAuth) => + // None = no method configured and nothing to auto-detect from the credentials + // (since #162 the builtin conf no longer forces method = "noauth", so an + // unauthenticated setup legitimately resolves to None) — same as Some(NoAuth). + case Some(NoAuth) | None => logger.warn("No authentication configured") builder case _ => + // A method was explicitly selected but its credentials are missing/empty + // (e.g. method = "basic" without username) — fail loudly. throw new IllegalStateException( s"Invalid authentication configuration: ${elasticConfig.credentials.authMethod}" ) diff --git a/es6/jest/src/test/scala/app/softnetwork/elastic/client/JestClientCompanionSpec.scala b/es6/jest/src/test/scala/app/softnetwork/elastic/client/JestClientCompanionSpec.scala index 99c8d3bd..d53a4f06 100644 --- a/es6/jest/src/test/scala/app/softnetwork/elastic/client/JestClientCompanionSpec.scala +++ b/es6/jest/src/test/scala/app/softnetwork/elastic/client/JestClientCompanionSpec.scala @@ -85,6 +85,29 @@ class JestClientCompanionSpec val companion = TestCompanion() companion.testConnection() shouldBe true } + + // Regression for #162 follow-up: the builtin conf no longer defaults + // method = "noauth", so a config with no credentials and no method resolves + // authMethod to None — the client must treat that as no-auth, not throw + // "Invalid authentication configuration: None". + "create client when no credentials and no method are configured (authMethod None)" in { + val base = ElasticConfig(elasticConfig) + val noAuthConfig = base.copy( + credentials = base.credentials.copy( + method = None, + username = "", + password = "", + apiKey = None, + bearerToken = None + ) + ) + noAuthConfig.credentials.authMethod shouldBe None + + val companion = TestCompanion(noAuthConfig) + val client = companion.apply() + client should not be null + companion.testConnection() shouldBe true + } } case class TestCompanion(config: ElasticConfig) extends JestClientCompanion { diff --git a/es6/jest/src/test/scala/app/softnetwork/elastic/client/JestClientNoAuthSpec.scala b/es6/jest/src/test/scala/app/softnetwork/elastic/client/JestClientNoAuthSpec.scala new file mode 100644 index 00000000..c8916f07 --- /dev/null +++ b/es6/jest/src/test/scala/app/softnetwork/elastic/client/JestClientNoAuthSpec.scala @@ -0,0 +1,66 @@ +/* + * Copyright 2025 SOFTNETWORK + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package app.softnetwork.elastic.client + +import app.softnetwork.elastic.client.jest.JestClientCompanion +import com.typesafe.config.ConfigFactory +import org.scalatest.matchers.should.Matchers +import org.scalatest.wordspec.AnyWordSpec + +/** Pure-JVM regression for the #162 follow-up: the builtin softnetwork-elastic.conf no longer + * defaults `method = "noauth"`, so a configuration with no credentials and no explicit method + * resolves `authMethod` to `None`. `JestClientCompanion.buildHttpConfig` used to throw + * `IllegalStateException("Invalid authentication configuration: None")` at client CREATION time + * (172 CI failures) — `None` must be treated as no-auth, exactly like `Some(NoAuth)`. + * + * Client construction never dials the cluster, so this spec needs no embedded/Docker Elasticsearch + * — it runs everywhere (the embedded-ES suite cannot start on Apple Silicon: ES 6.8's jvm.options + * passes the x86-only `-XX:UseAVX=2`). + */ +class JestClientNoAuthSpec extends AnyWordSpec with Matchers { + + "JestClientCompanion" should { + + "create a client when no credentials and no method are configured (authMethod None)" in { + val config = ElasticConfig(ConfigFactory.load("softnetwork-elastic.conf")) + // hermetic: immune to ELASTIC_* env vars resolved by the builtin conf + .copy(credentials = ElasticCredentials()) + config.credentials.authMethod shouldBe None + + val companion = new JestClientCompanion { + override def elasticConfig: ElasticConfig = config + } + try { + val client = companion.apply() + client should not be null + } finally { + companion.close() + } + } + + "still reject an explicitly selected method with missing credentials" in { + val config = ElasticConfig(ConfigFactory.load("softnetwork-elastic.conf")) + .copy(credentials = ElasticCredentials(method = Some("basic"))) + config.credentials.authMethod shouldBe Some(BasicAuth) + + val companion = new JestClientCompanion { + override def elasticConfig: ElasticConfig = config + } + an[IllegalStateException] should be thrownBy companion.apply() + } + } +} diff --git a/install.sh b/install.sh index 2ba51005..caf76eb5 100755 --- a/install.sh +++ b/install.sh @@ -686,6 +686,7 @@ create_config() { cat > "$TARGET_DIR/conf/application.conf" << 'EOF' # SoftClient4ES Configuration # Override these settings or use command-line options +# Precedence: CLI flag > ELASTIC_* env var > this file > built-in defaults elastic { credentials { @@ -783,6 +784,23 @@ check_java() { check_java +# HOCON \${?VAR} substitution treats an env var set to "" as PRESENT and lets it +# override file/default values (empty host => http://:9200). Treat empty as unset. +# The ELASTIC_WATCHER_* family is included because the builtin conf carries the +# same \${?VAR} substitution lines for the watcher block. +for var in ELASTIC_SCHEME ELASTIC_HOST ELASTIC_IP ELASTIC_PORT \\ + ELASTIC_USERNAME ELASTIC_PASSWORD ELASTIC_API_KEY ELASTIC_BEARER_TOKEN \\ + ELASTIC_AUTH_METHOD \\ + ELASTIC_CREDENTIALS_USERNAME ELASTIC_CREDENTIALS_PASSWORD \\ + ELASTIC_CREDENTIALS_API_KEY ELASTIC_CREDENTIALS_BEARER_TOKEN \\ + ELASTIC_WATCHER_SCHEME ELASTIC_WATCHER_HOST ELASTIC_WATCHER_PORT \\ + ELASTIC_WATCHER_AUTH_METHOD ELASTIC_WATCHER_USERNAME ELASTIC_WATCHER_PASSWORD \\ + ELASTIC_WATCHER_API_KEY ELASTIC_WATCHER_BEARER_TOKEN; do + if [[ -z "\${!var:-}" ]]; then + unset "\$var" + fi +done + # Default JVM options JAVA_OPTS="\${JAVA_OPTS:--Xmx512m}"