From da96a178b8e91cf8c80818cf549f079f851b5501 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Manciot?= Date: Thu, 30 Jul 2026 15:09:10 +0200 Subject: [PATCH] =?UTF-8?q?fix(core):=20watcher=20config=20block=20?= =?UTF-8?q?=E2=80=94=20real=20HOCON=20substitutions=20+=20sanitize=20cover?= =?UTF-8?q?age=20(Closed=20Issue=20#172)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The elastic.watcher block's credential keys were literal strings (method = elastic.credentials.method etc.), so the watcher never inherited the main credentials and auth auto-detection picked the garbage string "elastic.credentials.api-key" as ApiKeyAuth. - softnetwork-elastic.conf: all watcher keys now inherit the resolved elastic.credentials value via real substitutions (optional ${?elastic.credentials.method} — that key is absent by design after REPL.2); scheme/host/port inherit too (watcher targets the main cluster by default); ${?ELASTIC_WATCHER_*} overrides preserved. - CliConfig.envLayer: ELASTIC_WATCHER_* family gets the same empty-env-is-unset treatment as the main family (coordinates trimmed, credential values verbatim). - CliConfig.sanitize: empty/missing watcher scheme/host/port repair to the (already repaired) main connection coordinates. - CliConfigSpec: 6 new pure-JVM tests (inheritance incl. custom values through the builtin substitutions, env override, empty-env-is-unset, sanitize repair, garbage-ApiKeyAuth regression). Co-Authored-By: Claude Fable 5 --- .../main/resources/softnetwork-elastic.conf | 24 ++-- .../elastic/client/CliConfig.scala | 53 +++++++- .../elastic/client/CliConfigSpec.scala | 124 +++++++++++++++++- 3 files changed, 185 insertions(+), 16 deletions(-) diff --git a/core/src/main/resources/softnetwork-elastic.conf b/core/src/main/resources/softnetwork-elastic.conf index 17fc8b69..bd236766 100644 --- a/core/src/main/resources/softnetwork-elastic.conf +++ b/core/src/main/resources/softnetwork-elastic.conf @@ -51,29 +51,37 @@ elastic { } } + # Watcher targets the main cluster by default: every key INHERITS the resolved + # elastic.credentials value via a real HOCON substitution (issue #172 - these used + # to be literal strings like "elastic.credentials.api-key", which never inherited + # anything and tripped auth auto-detection into ApiKeyAuth). A per-key + # ELASTIC_WATCHER_* env var overrides the inherited value. watcher { - scheme = "http" + scheme = ${elastic.credentials.scheme} scheme = ${?ELASTIC_WATCHER_SCHEME} - host = "localhost" + host = ${elastic.credentials.host} host = ${?ELASTIC_WATCHER_HOST} - port = 9200 + port = ${elastic.credentials.port} port = ${?ELASTIC_WATCHER_PORT} - method = elastic.credentials.method + # Optional substitution: elastic.credentials.method is ABSENT unless + # ELASTIC_AUTH_METHOD is set (auth auto-detection design) - a required + # substitution would fail resolution. + method = ${?elastic.credentials.method} method = ${?ELASTIC_WATCHER_AUTH_METHOD} - username = elastic.credentials.username + username = ${elastic.credentials.username} username = ${?ELASTIC_WATCHER_USERNAME} - password = elastic.credentials.password + password = ${elastic.credentials.password} password = ${?ELASTIC_WATCHER_PASSWORD} - api-key = elastic.credentials.api-key + api-key = ${elastic.credentials.api-key} api-key = ${?ELASTIC_WATCHER_API_KEY} - bearer-token = elastic.credentials.bearer-token + bearer-token = ${elastic.credentials.bearer-token} bearer-token = ${?ELASTIC_WATCHER_BEARER_TOKEN} } } \ No newline at end of file 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 598e5694..91371482 100644 --- a/core/src/main/scala/app/softnetwork/elastic/client/CliConfig.scala +++ b/core/src/main/scala/app/softnetwork/elastic/client/CliConfig.scala @@ -92,6 +92,16 @@ object CliConfig extends LazyLogging { private[client] val ApiKeyPath = "elastic.credentials.api-key" private[client] val BearerTokenPath = "elastic.credentials.bearer-token" + // Watcher block (issue #172): inherits elastic.credentials.* in the builtin conf, + // overridable per key through the ELASTIC_WATCHER_* env family. + private[client] val WatcherSchemePath = "elastic.watcher.scheme" + private[client] val WatcherHostPath = "elastic.watcher.host" + private[client] val WatcherPortPath = "elastic.watcher.port" + private[client] val WatcherUsernamePath = "elastic.watcher.username" + private[client] val WatcherPasswordPath = "elastic.watcher.password" + private[client] val WatcherApiKeyPath = "elastic.watcher.api-key" + private[client] val WatcherBearerTokenPath = "elastic.watcher.bearer-token" + private def layerOf(entries: Seq[(String, AnyRef)]): Config = entries.foldLeft(ConfigFactory.empty()) { case (config, (path, value)) => config.withValue(path, ConfigValueFactory.fromAnyRef(value)) @@ -129,7 +139,18 @@ object CliConfig extends LazyLogging { firstNonBlank("ELASTIC_API_KEY", "ELASTIC_CREDENTIALS_API_KEY") .map(ApiKeyPath -> _), firstNonBlank("ELASTIC_BEARER_TOKEN", "ELASTIC_CREDENTIALS_BEARER_TOKEN") - .map(BearerTokenPath -> _) + .map(BearerTokenPath -> _), + // ELASTIC_WATCHER_* family (issue #172): same empty-env-is-unset and trimming + // treatment as the main connection family. ELASTIC_WATCHER_AUTH_METHOD is NOT + // in this layer - parity with ELASTIC_AUTH_METHOD, which only enters through + // the builtin conf tail. + firstCoordinate("ELASTIC_WATCHER_SCHEME").map(WatcherSchemePath -> _), + firstCoordinate("ELASTIC_WATCHER_HOST").map(WatcherHostPath -> _), + firstCoordinate("ELASTIC_WATCHER_PORT").map(WatcherPortPath -> _), + firstNonBlank("ELASTIC_WATCHER_USERNAME").map(WatcherUsernamePath -> _), + firstNonBlank("ELASTIC_WATCHER_PASSWORD").map(WatcherPasswordPath -> _), + firstNonBlank("ELASTIC_WATCHER_API_KEY").map(WatcherApiKeyPath -> _), + firstNonBlank("ELASTIC_WATCHER_BEARER_TOKEN").map(WatcherBearerTokenPath -> _) ).flatten ) } @@ -140,12 +161,30 @@ object CliConfig extends LazyLogging { * `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)) => + private[client] def sanitize(resolved: Config): Config = { + val repaired = repairEmpty( + resolved, + Seq[(String, AnyRef)]( + SchemePath -> "http", + HostPath -> "localhost", + PortPath -> Int.box(9200) + ) + ) + // Watcher coordinates (issue #172): the watcher targets the main cluster unless + // explicitly overridden, so an empty/missing watcher coordinate falls back to the + // (already repaired) main connection coordinate rather than a hard default. + repairEmpty( + repaired, + Seq[(String, AnyRef)]( + WatcherSchemePath -> repaired.getAnyRef(SchemePath), + WatcherHostPath -> repaired.getAnyRef(HostPath), + WatcherPortPath -> repaired.getAnyRef(PortPath) + ) + ) + } + + private def repairEmpty(resolved: Config, defaults: Seq[(String, AnyRef)]): Config = + defaults.foldLeft(resolved) { case (config, (path, default)) => val missingOrEmpty = !config.hasPath(path) || config.getString(path).trim.isEmpty if (missingOrEmpty) { diff --git a/core/src/test/scala/app/softnetwork/elastic/client/CliConfigSpec.scala b/core/src/test/scala/app/softnetwork/elastic/client/CliConfigSpec.scala index ee3ab239..00910e50 100644 --- a/core/src/test/scala/app/softnetwork/elastic/client/CliConfigSpec.scala +++ b/core/src/test/scala/app/softnetwork/elastic/client/CliConfigSpec.scala @@ -64,7 +64,15 @@ class CliConfigSpec extends AnyFlatSpec with Matchers { "ELASTIC_CREDENTIALS_USERNAME", "ELASTIC_CREDENTIALS_PASSWORD", "ELASTIC_CREDENTIALS_API_KEY", - "ELASTIC_CREDENTIALS_BEARER_TOKEN" + "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" ).foreach(name => assume(sys.env.get(name).forall(_.trim.isEmpty))) behavior of "CliConfig connection precedence" @@ -270,6 +278,120 @@ class CliConfigSpec extends AnyFlatSpec with Matchers { ).credentials.authMethod shouldBe Some(NoAuth) } + behavior of "CliConfig watcher block inheritance (issue #172)" + + private def watcher(config: Config, key: String): String = + config.getString(s"elastic.watcher.$key") + + it should "inherit the resolved credentials values when nothing watcher-specific is set" in { + assumeCleanEnvironment() + val config = CliConfig().buildElasticConfig(noEnv) + watcher(config, "scheme") shouldBe "http" + watcher(config, "host") shouldBe "localhost" + config.getInt("elastic.watcher.port") shouldBe 9200 + watcher(config, "username") shouldBe "" + watcher(config, "password") shouldBe "" + watcher(config, "api-key") shouldBe "" + watcher(config, "bearer-token") shouldBe "" + // method inherits the (absent) credentials.method - it must stay absent so + // auth auto-detection engages + config.hasPath("elastic.watcher.method") shouldBe false + // regression: no key may carry the old literal dotted-path garbage + Seq("method", "username", "password", "api-key", "bearer-token").foreach { key => + if (config.hasPath(s"elastic.watcher.$key")) { + watcher(config, key) should not startWith "elastic.credentials" + } + } + } + + it should "inherit CUSTOM credentials values through the builtin conf substitutions" in { + // Exercises the builtin resource's ${elastic.credentials.*} substitutions directly: + // overriding a credentials key before resolution (what ELASTIC_CREDENTIALS_* env + // vars do at load time) must propagate into the watcher block. + assumeCleanEnvironment() + val resolved = ConfigFactory + .parseString( + """ + |elastic.credentials.host = "main-host" + |elastic.credentials.username = "bob" + |elastic.credentials.api-key = "secret-key" + """.stripMargin + ) + .withFallback(ConfigFactory.parseResources("softnetwork-elastic.conf")) + .resolve() + resolved.getString("elastic.watcher.host") shouldBe "main-host" + resolved.getString("elastic.watcher.username") shouldBe "bob" + resolved.getString("elastic.watcher.api-key") shouldBe "secret-key" + } + + it should "let ELASTIC_WATCHER_* env vars override inherited and file values" in { + assumeCleanEnvironment() + val file = fileOf("""elastic.watcher.host = "file-watcher-host"""") + // env beats file + watcher( + CliConfig().buildElasticConfig( + envOf("ELASTIC_WATCHER_HOST" -> "watcher-host"), + external = file + ), + "host" + ) shouldBe "watcher-host" + // env beats the inherited credentials value, without touching the main connection + val config = CliConfig().buildElasticConfig( + envOf( + "ELASTIC_WATCHER_HOST" -> " watcher-host ", + "ELASTIC_WATCHER_PASSWORD" -> " spacey pass " + ) + ) + watcher(config, "host") shouldBe "watcher-host" // coordinates trimmed + watcher(config, "password") shouldBe " spacey pass " // credentials verbatim + credentials(config, "host") shouldBe "localhost" + } + + it should "treat an empty ELASTIC_WATCHER_* env var as unset" in { + assumeCleanEnvironment() + // empty env must NOT mask the file value + watcher( + CliConfig().buildElasticConfig( + envOf("ELASTIC_WATCHER_HOST" -> ""), + external = fileOf("""elastic.watcher.host = "file-watcher-host"""") + ), + "host" + ) shouldBe "file-watcher-host" + // empty env with no file: the inherited value survives + val config = CliConfig().buildElasticConfig( + envOf("ELASTIC_WATCHER_HOST" -> "", "ELASTIC_WATCHER_PORT" -> " ") + ) + watcher(config, "host") shouldBe "localhost" + config.getInt("elastic.watcher.port") shouldBe 9200 + } + + it should "repair an empty watcher coordinate to the main connection coordinate (sanitize guard)" in { + assumeCleanEnvironment() + val config = CliConfig().buildElasticConfig( + noEnv, + external = fileOf(""" + elastic.credentials.host = "main-host" + elastic.watcher.host = "" + elastic.watcher.port = "" + """) + ) + watcher(config, "host") shouldBe "main-host" + config.getInt("elastic.watcher.port") shouldBe 9200 + } + + it should "auto-detect NO auth on the watcher block when nothing is set (garbage-ApiKeyAuth regression)" in { + assumeCleanEnvironment() + // Before the fix, watcher.api-key resolved to the literal string + // "elastic.credentials.api-key", which auto-detection picked up as ApiKeyAuth. + val watcherCredentials = + ElasticConfig(CliConfig().buildElasticConfig(noEnv)).watcher + watcherCredentials.authMethod shouldBe None + watcherCredentials.apiKey.getOrElse("") shouldBe "" + watcherCredentials.bearerToken.getOrElse("") shouldBe "" + watcherCredentials.username shouldBe "" + watcherCredentials.password shouldBe "" + } + behavior of "CliConfig.parseArgs" it should "leave every connection setting unset when no flag is passed (AC 10)" in {