From e5d5d843f8d23cb982f0883a87a40906c6a859a5 Mon Sep 17 00:00:00 2001 From: Lukas Jost Date: Tue, 4 Aug 2026 21:14:06 +0200 Subject: [PATCH] fix(config): consume service contract target --- README.md | 61 ++++++++++--------- .../proxy/velocity/GroundsProxyPlugin.kt | 21 +++++-- .../proxy/velocity/ConfigServiceTargetTest.kt | 36 +++++++++++ 3 files changed, 83 insertions(+), 35 deletions(-) create mode 100644 velocity/src/test/kotlin/gg/grounds/proxy/velocity/ConfigServiceTargetTest.kt diff --git a/README.md b/README.md index c72355f..ba98085 100644 --- a/README.md +++ b/README.md @@ -8,15 +8,15 @@ A Velocity proxy only knows its own players. Ask it for `dahendriik` while they `ProxyService`, published into the [`ProxyServiceRegistry`](api/src/main/kotlin/gg/grounds/proxy/api/ProxyServiceRegistry.kt): -| method | local player | player on another proxy | -|---|---|---| -| `resolvePlayerId(name)` | Velocity's player list | `PlayerSessionQuery` (service-player) | -| `resolvePlayerName(id)` | Velocity's player list | `PlayerSessionQuery` | -| `isOnline(id)` | Velocity's player list | `PlayerSessionQuery` | -| `getPresence(id)` | current server | session's proxy + server | -| `sendToPlayer(id, msg)` | `player.sendMessage` | publish `proxy.system.` | -| `transferPlayer(id, server)` | connection request | publish `proxy.transfer.` | -| `suggestPlayerNames(prefix)` | filtered in memory | prefix search, capped, 2s cache | +| method | local player | player on another proxy | +| ---------------------------- | ---------------------- | ------------------------------------- | +| `resolvePlayerId(name)` | Velocity's player list | `PlayerSessionQuery` (service-player) | +| `resolvePlayerName(id)` | Velocity's player list | `PlayerSessionQuery` | +| `isOnline(id)` | Velocity's player list | `PlayerSessionQuery` | +| `getPresence(id)` | current server | session's proxy + server | +| `sendToPlayer(id, msg)` | `player.sendMessage` | publish `proxy.system.` | +| `transferPlayer(id, server)` | connection request | publish `proxy.transfer.` | +| `suggestPlayerNames(prefix)` | filtered in memory | prefix search, capped, 2s cache | Local first, always — a player on this proxy is already in memory and costs nothing to find. @@ -37,7 +37,7 @@ plugin-chat / plugin-social consumers: only ever call ProxyService Nobody registers `PlayerSessionQuery` → lookups return null and everything degrades to local-only, silently. That is exactly the state this repo was in before plugin-player shipped: the fallback existed and nothing filled it. -**Why service-player and not a registry in the proxies?** Presence already lives there — `TryPlayerLogin` is on the login path, with heartbeats and a TTL. A second store in the proxies would be a second source of truth with its own expiry, and two of those drift. An earlier attempt (plugin-chat's own `chat.players.join/leave` map) also had no memory: core NATS does not replay, so a proxy only ever learned about players who joined *while it was running*. +**Why service-player and not a registry in the proxies?** Presence already lives there — `TryPlayerLogin` is on the login path, with heartbeats and a TTL. A second store in the proxies would be a second source of truth with its own expiry, and two of those drift. An earlier attempt (plugin-chat's own `chat.players.join/leave` map) also had no memory: core NATS does not replay, so a proxy only ever learned about players who joined _while it was running_. ## Consuming it @@ -64,7 +64,7 @@ Shading `plugin-proxy-api` gives your plugin its own copy of the registry class ## Tab-complete does not list the network -`suggestPlayerNames(prefix, limit)` is a prefix search with a cap, and there is deliberately **no** "give me every online player". Velocity fires tab-complete on *every keystroke*: at 10k players online, a roster dump is a ~200 KB response issued thousands of times a second, with a table scan behind each one. So: local matches from memory, the network only once the prefix is ≥ 2 characters, answers cached 2s per prefix, result capped (default 20). +`suggestPlayerNames(prefix, limit)` is a prefix search with a cap, and there is deliberately **no** "give me every online player". Velocity fires tab-complete on _every keystroke_: at 10k players online, a roster dump is a ~200 KB response issued thousands of times a second, with a table scan behind each one. So: local matches from memory, the network only once the prefix is ≥ 2 characters, answers cached 2s per prefix, result capped (default 20). ## The network MOTD @@ -89,14 +89,14 @@ to a custom role, the same way in-game administration is granted. Resolved per ping, so one stored MOTD reads differently depending on which region answered. -| token | value | -|---|---| -| `{{region}}` | `REGION` — the datacentre (`nl-ams1`) | -| `{{localzone}}`, `{{continent}}` | `CONTINENT` — `eu` / `na` | -| `{{players}}` | the network-wide player count this ping reports | -| `{{max}}` | the player cap this ping reports | +| token | value | +| -------------------------------- | ----------------------------------------------- | +| `{{region}}` | `REGION` — the datacentre (`nl-ams1`) | +| `{{localzone}}`, `{{continent}}` | `CONTINENT` — `eu` / `na` | +| `{{players}}` | the network-wide player count this ping reports | +| `{{max}}` | the player cap this ping reports | -A known token with no value renders as nothing; an *unknown* one is left standing, so a typo shows +A known token with no value renders as nothing; an _unknown_ one is left standing, so a typo shows up in the server list instead of disappearing. ### motd.gg import @@ -117,21 +117,22 @@ that fails keeps the previous MOTD rather than emptying every region's server-li Writes go to `ConfigAdminService`, which service-config restricts to admin service accounts and to writers explicitly allowed for the app — see `GROUNDS_CONFIG_WRITERS` there. A proxy that is not -allowed can still *show* the MOTD; `/motd set` then reports the refusal instead of failing silently. +allowed can still _show_ the MOTD; `/motd set` then reports the refusal instead of failing silently. ## Configuration -| env | meaning | -|---|---| -| `NATS_URL` | broker for `proxy.system.*` / `proxy.transfer.*` (default `nats://nats.infra:4222`) | -| `PROXY_ID` | this proxy's identity, recorded in a player's session — must differ per proxy (`velocity`, `velocity-2`) | -| `GROUNDS_TOKEN_FILE` | projected SA-token, presented as the NATS bearer and as the service-config gRPC bearer (default `/var/run/secrets/grounds/token`) | -| `CONFIG_GRPC_TARGET` | service-config, e.g. `service-config:9000`. **Unset disables `/motd` entirely** and Velocity's own MOTD is served | -| `CONFIG_ENV` | which environment's document to use; falls back to `GROUNDS_PERMISSION_ENVIRONMENT` | -| `CONFIG_APP` | which service-config app holds it (default `velocity` — deliberately not the release name, so `velocity` and `velocity-2` share one MOTD) | -| `MOTD_REFRESH_SECONDS` | how often each proxy re-reads it (default `15`) | -| `REGION` | `{{region}}`, and the region `/region` considers "here" | -| `CONTINENT` | `{{localzone}}` / `{{continent}}` | +| env | meaning | +| ---------------------- | ----------------------------------------------------------------------------------------------------------------------------------------- | +| `NATS_URL` | broker for `proxy.system.*` / `proxy.transfer.*` (default `nats://nats.infra:4222`) | +| `PROXY_ID` | this proxy's identity, recorded in a player's session — must differ per proxy (`velocity`, `velocity-2`) | +| `GROUNDS_TOKEN_FILE` | projected SA-token, presented as the NATS bearer and as the service-config gRPC bearer (default `/var/run/secrets/grounds/token`) | +| `CONFIG_SERVICE_URL` | service-config contract target, e.g. `service-config:9000`. **Unset disables `/motd` entirely** and Velocity's own MOTD is served | +| `CONFIG_GRPC_TARGET` | Legacy fallback for deployments that have not migrated to `CONFIG_SERVICE_URL` | +| `CONFIG_ENV` | which environment's document to use; falls back to `GROUNDS_PERMISSION_ENVIRONMENT` | +| `CONFIG_APP` | which service-config app holds it (default `velocity` — deliberately not the release name, so `velocity` and `velocity-2` share one MOTD) | +| `MOTD_REFRESH_SECONDS` | how often each proxy re-reads it (default `15`) | +| `REGION` | `{{region}}`, and the region `/region` considers "here" | +| `CONTINENT` | `{{localzone}}` / `{{continent}}` | The NATS auth-callout scopes each pod to the subjects declared in its bundle `events:` block, so `proxy.system.*` and `proxy.transfer.*` must be listed there — an undeclared subject is denied and the message vanishes. diff --git a/velocity/src/main/kotlin/gg/grounds/proxy/velocity/GroundsProxyPlugin.kt b/velocity/src/main/kotlin/gg/grounds/proxy/velocity/GroundsProxyPlugin.kt index b36488e..e71d10f 100644 --- a/velocity/src/main/kotlin/gg/grounds/proxy/velocity/GroundsProxyPlugin.kt +++ b/velocity/src/main/kotlin/gg/grounds/proxy/velocity/GroundsProxyPlugin.kt @@ -83,6 +83,17 @@ internal fun parsePlayerCount(payload: String): Int? { return digits.toIntOrNull()?.takeIf { it >= 0 } } +/** + * Resolves service-config through the platform service contract while keeping old deployments + * working until every bundle has migrated from the legacy gRPC-specific variable. + */ +internal fun resolveConfigServiceTarget( + environment: (String) -> String? = System::getenv +): String? = + sequenceOf("CONFIG_SERVICE_URL", "CONFIG_GRPC_TARGET") + .mapNotNull { name -> environment(name)?.trim()?.takeIf(String::isNotEmpty) } + .firstOrNull() + @Plugin(id = "plugin-proxy", name = "GroundsProxyPlugin", version = BuildInfo.VERSION) class GroundsProxyPlugin @Inject @@ -193,14 +204,14 @@ constructor(private val proxy: ProxyServer, private val logger: Logger) { /** * Brings up the network-wide MOTD, if this deployment has a service-config to keep it in. * - * Without `CONFIG_GRPC_TARGET` the whole feature stays off and Velocity's own MOTD is served — - * the same thing that happened before there was a `/motd`. That is the right shape for a - * per-engineer proxy or a local run, where there is no config service to talk to. + * Without a config service contract the whole feature stays off and Velocity's own MOTD is + * served — the same thing that happened before there was a `/motd`. That is the right shape for + * a local run, where there is no config service to talk to. */ private fun startMotd() { - val target = env("CONFIG_GRPC_TARGET") + val target = resolveConfigServiceTarget() if (target == null) { - logger.info("MOTD disabled (reason=CONFIG_GRPC_TARGET_unset)") + logger.info("MOTD disabled (reason=config_service_url_unset)") return } val app = env("CONFIG_APP") ?: DEFAULT_CONFIG_APP diff --git a/velocity/src/test/kotlin/gg/grounds/proxy/velocity/ConfigServiceTargetTest.kt b/velocity/src/test/kotlin/gg/grounds/proxy/velocity/ConfigServiceTargetTest.kt new file mode 100644 index 0000000..dec17af --- /dev/null +++ b/velocity/src/test/kotlin/gg/grounds/proxy/velocity/ConfigServiceTargetTest.kt @@ -0,0 +1,36 @@ +package gg.grounds.proxy.velocity + +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Test + +class ConfigServiceTargetTest { + + @Test + fun `uses the declared config service contract`() { + val environment = + mapOf( + "CONFIG_SERVICE_URL" to "service-config.default.svc.cluster.local:9000", + "CONFIG_GRPC_TARGET" to "legacy-config:9000", + ) + + assertEquals( + "service-config.default.svc.cluster.local:9000", + resolveConfigServiceTarget(environment::get), + ) + } + + @Test + fun `falls back to the legacy config target`() { + val environment = mapOf("CONFIG_GRPC_TARGET" to "legacy-config:9000") + + assertEquals("legacy-config:9000", resolveConfigServiceTarget(environment::get)) + } + + @Test + fun `ignores blank config targets`() { + val environment = mapOf("CONFIG_SERVICE_URL" to " ", "CONFIG_GRPC_TARGET" to "") + + assertNull(resolveConfigServiceTarget(environment::get)) + } +}