diff --git a/Dockerfile b/Dockerfile index 1e5280d..dce2d0b 100644 --- a/Dockerfile +++ b/Dockerfile @@ -24,7 +24,6 @@ COPY gradle/ gradle/ COPY gradlew settings.gradle.kts build.gradle.kts gradle.properties ./ COPY api/ api/ -COPY grpc/ grpc/ COPY velocity/ velocity/ # `:velocity:build` produces the shaded plugin JAR (the api module, the NATS diff --git a/grpc/build.gradle.kts b/grpc/build.gradle.kts deleted file mode 100644 index 37fd32a..0000000 --- a/grpc/build.gradle.kts +++ /dev/null @@ -1,48 +0,0 @@ -import com.google.protobuf.gradle.id - -plugins { - `java-library` - id("com.google.protobuf") version "0.10.0" -} - -repositories { - mavenCentral() - maven { - url = uri("https://maven.pkg.github.com/groundsgg/*") - credentials { - username = - providers.gradleProperty("github.user").orNull ?: System.getenv("GITHUB_ACTOR") - password = - providers.gradleProperty("github.token").orNull ?: System.getenv("GITHUB_TOKEN") - } - } -} - -// Mirrors :api — the root convention puts Kotlin on 25, and the generated Java -// stubs would then compile against a different target than the Kotlin stub-gen -// task the same convention adds. -tasks.withType { options.release.set(21) } - -tasks.withType { - compilerOptions { jvmTarget.set(org.jetbrains.kotlin.gradle.dsl.JvmTarget.JVM_21) } -} - -dependencies { - api("io.grpc:grpc-protobuf:1.78.0") - api("io.grpc:grpc-stub:1.78.0") - api("com.google.protobuf:protobuf-java:4.29.0") - // javax.annotation.Generated, which protoc-gen-grpc-java emits and the JDK - // no longer ships. compileOnly: nothing reads the annotation at runtime. - compileOnly("org.apache.tomcat:annotations-api:6.0.53") - - // Proto-only jar: it ships config_service.proto and config_admin.proto and no - // compiled classes, so the stubs are generated here from the pinned contract - // rather than inherited from an artifact that could drift from it. - protobuf("gg.grounds:library-grpc-contracts-config:0.2.0") -} - -protobuf { - protoc { artifact = "com.google.protobuf:protoc:4.29.0" } - plugins { id("grpc") { artifact = "io.grpc:protoc-gen-grpc-java:1.78.0" } } - generateProtoTasks { all().forEach { it.plugins { id("grpc") } } } -} diff --git a/settings.gradle.kts b/settings.gradle.kts index 372da41..8d927d8 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -16,4 +16,4 @@ pluginManagement { rootProject.name = "plugin-proxy" -include("api", "grpc", "velocity") +include("api", "velocity") diff --git a/velocity/build.gradle.kts b/velocity/build.gradle.kts index 0d9ddc0..56e3842 100644 --- a/velocity/build.gradle.kts +++ b/velocity/build.gradle.kts @@ -14,7 +14,9 @@ repositories { dependencies { implementation(project(":api")) - implementation(project(":grpc")) + // service-config answers REST; the MOTD store parses its JSON with gson, the + // way plugin-match's and plugin-social's clients do. + implementation("com.google.code.gson:gson:2.11.0") // The tab list header and footer are player-facing text, so they come from a bundle and are // drawn in the design tokens, like every other line the network shows a player. implementation("gg.grounds:library-i18n:0.2.0") @@ -27,7 +29,6 @@ dependencies { implementation("io.micrometer:micrometer-registry-prometheus:1.16.6") // The transport for the service-config channel. Shaded by gRPC itself, so it does not fight // with the Netty the proxy runs on. - implementation("io.grpc:grpc-netty-shaded:1.78.0") testImplementation("org.junit.jupiter:junit-jupiter-api:6.1.2") // Adventure reaches the plugin through Velocity at runtime, which is compileOnly here and so diff --git a/velocity/src/main/kotlin/gg/grounds/proxy/velocity/command/MotdCommand.kt b/velocity/src/main/kotlin/gg/grounds/proxy/velocity/command/MotdCommand.kt index 97e9daf..5b6b7f8 100644 --- a/velocity/src/main/kotlin/gg/grounds/proxy/velocity/command/MotdCommand.kt +++ b/velocity/src/main/kotlin/gg/grounds/proxy/velocity/command/MotdCommand.kt @@ -275,13 +275,11 @@ class MotdCommand( private fun error(message: String): Component = Component.text(message, NamedTextColor.RED) /** - * gRPC failures arrive as a status line that reads like a stack trace. The description is the - * part service-config wrote for a human — a caller that is not allowed to write says so there. + * The store already unwrapped what service-config wrote for a human — a caller that is not + * allowed to write says so in the problem's detail. Anything else falls back to the exception, + * which is at least a sentence rather than a status line. */ - private fun describe(ex: Exception): String = - (ex as? io.grpc.StatusRuntimeException)?.status?.let { status -> - status.description ?: status.code.name - } ?: (ex.message ?: ex::class.java.simpleName) + private fun describe(ex: Exception): String = ex.message ?: ex::class.java.simpleName private fun actorOf(source: CommandSource): String = (source as? Player)?.username ?: "console" diff --git a/velocity/src/main/kotlin/gg/grounds/proxy/velocity/motd/MotdConfigStore.kt b/velocity/src/main/kotlin/gg/grounds/proxy/velocity/motd/MotdConfigStore.kt index 8f67d41..3692ff7 100644 --- a/velocity/src/main/kotlin/gg/grounds/proxy/velocity/motd/MotdConfigStore.kt +++ b/velocity/src/main/kotlin/gg/grounds/proxy/velocity/motd/MotdConfigStore.kt @@ -1,45 +1,32 @@ package gg.grounds.proxy.velocity.motd -import gg.grounds.grpc.config.ConfigAdminServiceGrpc -import gg.grounds.grpc.config.ConfigServiceGrpc -import gg.grounds.grpc.config.DeleteDocumentRequest -import gg.grounds.grpc.config.GetDocumentRequest -import gg.grounds.grpc.config.PutDocumentRequest -import io.grpc.CallOptions -import io.grpc.Channel -import io.grpc.ClientCall -import io.grpc.ClientInterceptor -import io.grpc.ForwardingClientCall -import io.grpc.LoadBalancerRegistry -import io.grpc.ManagedChannel -import io.grpc.ManagedChannelBuilder -import io.grpc.Metadata -import io.grpc.MethodDescriptor -import io.grpc.NameResolverRegistry -import io.grpc.Status -import io.grpc.StatusRuntimeException -import io.grpc.internal.DnsNameResolverProvider -import io.grpc.internal.PickFirstLoadBalancerProvider +import com.google.gson.Gson +import com.google.gson.JsonObject +import java.net.URI +import java.net.http.HttpClient +import java.net.http.HttpRequest +import java.net.http.HttpResponse import java.nio.file.Files import java.nio.file.Path -import java.util.concurrent.TimeUnit +import java.time.Duration /** * The MOTD's home in service-config: one document, `motd/active`, under a fixed app and the * deployment's environment. * - * Reads go through `ConfigService`, which any authenticated caller may use; writes go through - * `ConfigAdminService`, which service-config restricts to admin service accounts and to writers - * explicitly allowed for this app. A proxy that is not on that list can still show the MOTD — it - * just cannot change it, and `/motd set` says so rather than failing silently. + * Reads go to the consumer API, which any authenticated caller may use; writes go to the admin API, + * which service-config restricts to admin service accounts and to writers explicitly allowed for + * this app. A proxy that is not on that list can still show the MOTD — it just cannot change it, + * and `/motd set` says so rather than failing silently. * - * Deliberately not built on the shared `plugin-config` client: that one reads only, and attaches no - * credential, so it cannot do either half of this against a service-config with auth enabled. + * Deliberately not built on the shared `plugin-config` client: that one reads only, so it cannot do + * the writing half at all. */ class MotdConfigStore( private val app: String, private val env: String, - private val channel: ManagedChannel, + private val baseUri: URI, + private val http: HttpClient, ) : MotdStore, AutoCloseable { /** @@ -47,73 +34,82 @@ class MotdConfigStore( * an error — the caller then leaves Velocity's own MOTD alone. */ override fun read(): MotdDocument? { - val response = - try { - ConfigServiceGrpc.newBlockingStub(channel) - .withDeadlineAfter(DEADLINE_SECONDS, TimeUnit.SECONDS) - .getDocument( - GetDocumentRequest.newBuilder() - .setApp(app) - .setEnv(env) - .setNamespace(NAMESPACE) - .setConfigKey(CONFIG_KEY) - .build() - ) - } catch (ex: StatusRuntimeException) { - if (ex.status.code == Status.Code.NOT_FOUND) return null - throw ex - } - return MotdDocument.fromJson(response.document.contentJson) + val response = send(request(consumerPath()).GET()) + if (response.statusCode() == 404) return null + requireSuccess(response, "read the MOTD") + val document = + GSON.fromJson(response.body(), JsonObject::class.java)?.get("contentJson")?.asString + ?: return null + return MotdDocument.fromJson(document) } /** Stores [document] as the network's MOTD, replacing whatever was there. */ override fun write(document: MotdDocument, updatedBy: String) { - ConfigAdminServiceGrpc.newBlockingStub(channel) - .withDeadlineAfter(DEADLINE_SECONDS, TimeUnit.SECONDS) - .putDocument( - PutDocumentRequest.newBuilder() - .setApp(app) - .setEnv(env) - .setNamespace(NAMESPACE) - .setConfigKey(CONFIG_KEY) - .setContentJson(document.toJson()) - .setUpdatedBy(updatedBy) - .build() - ) - // No expected_version: two operators racing on /motd is a coin flip either way, and a + // No expectedVersion: two operators racing on /motd is a coin flip either way, and a // rejected write that says "someone else changed it, try again" is worse in chat than the // second one simply winning. The dashboard, which can show the conflict, is where // optimistic concurrency earns its keep. + val body = GSON.toJson(mapOf("contentJson" to document.toJson(), "updatedBy" to updatedBy)) + val response = + send( + request(adminPath()) + .header("Content-Type", "application/json") + .PUT(HttpRequest.BodyPublishers.ofString(body)) + ) + requireSuccess(response, "set the MOTD") } /** Removes the stored MOTD. Returns true when there was one to remove. */ - override fun clear(deletedBy: String): Boolean = - ConfigAdminServiceGrpc.newBlockingStub(channel) - .withDeadlineAfter(DEADLINE_SECONDS, TimeUnit.SECONDS) - .deleteDocument( - DeleteDocumentRequest.newBuilder() - .setApp(app) - .setEnv(env) - .setNamespace(NAMESPACE) - .setConfigKey(CONFIG_KEY) - .setDeletedBy(deletedBy) - .build() - ) - .deleted + override fun clear(deletedBy: String): Boolean { + val response = send(request(adminPath()).DELETE()) + requireSuccess(response, "clear the MOTD") + return GSON.fromJson(response.body(), JsonObject::class.java)?.get("deleted")?.asBoolean + ?: false + } override fun close() { - channel.shutdown() - if (!channel.awaitTermination(SHUTDOWN_SECONDS, TimeUnit.SECONDS)) { - channel.shutdownNow() - } + http.close() + } + + private fun consumerPath() = + "/v1/config/apps/$app/envs/$env/namespaces/$NAMESPACE/documents/$CONFIG_KEY" + + private fun adminPath() = + "/v1/config/admin/apps/$app/envs/$env/namespaces/$NAMESPACE/documents/$CONFIG_KEY" + + private fun request(path: String): HttpRequest.Builder { + val builder = + HttpRequest.newBuilder(baseUri.resolve(path)) + .timeout(DEADLINE) + .header("Accept", "application/json") + readToken()?.let { builder.header("Authorization", "Bearer $it") } + return builder + } + + private fun send(builder: HttpRequest.Builder): HttpResponse = + http.send(builder.build(), HttpResponse.BodyHandlers.ofString()) + + /** + * Non-2xx becomes an exception carrying what service-config wrote for a human — a caller that + * is not allowed to write says so in the problem's `detail`, and `/motd` shows that line rather + * than a status code. + */ + private fun requireSuccess(response: HttpResponse, action: String) { + if (response.statusCode() in 200..299) return + val detail = + runCatching { + GSON.fromJson(response.body(), JsonObject::class.java)?.get("detail")?.asString + } + .getOrNull() + throw MotdStoreException(detail ?: "Could not $action (HTTP ${response.statusCode()})") } companion object { const val NAMESPACE = "motd" const val CONFIG_KEY = "active" - private const val DEADLINE_SECONDS = 5L - private const val SHUTDOWN_SECONDS = 3L + private val GSON = Gson() + private val DEADLINE: Duration = Duration.ofSeconds(5) /** * Where the projected ServiceAccount token is mounted. The kubelet rotates it well before @@ -123,21 +119,17 @@ class MotdConfigStore( private const val DEFAULT_TOKEN_PATH = "/var/run/secrets/grounds/token" fun open(app: String, env: String, target: String): MotdConfigStore { - // Velocity loads each plugin in its own classloader, and gRPC's service-loader - // discovery finds nothing there. Registering both providers by hand is what makes a - // `dns:///` target resolvable from inside a shaded plugin jar; without it the channel - // comes up and every call fails with UNAVAILABLE. - NameResolverRegistry.getDefaultRegistry().register(DnsNameResolverProvider()) - LoadBalancerRegistry.getDefaultRegistry().register(PickFirstLoadBalancerProvider()) - - val channel = - ManagedChannelBuilder.forTarget(target) - .usePlaintext() - .intercept(BearerTokenInterceptor(::readToken)) - .build() - return MotdConfigStore(app, env, channel) + // The chart injects the address with no scheme; java.net.http throws parsing that + // directly, so default to http. + val baseUri = URI.create(if (target.contains("://")) target else "http://$target") + return MotdConfigStore(app, env, baseUri, HttpClient.newHttpClient()) } + /** + * A missing token is sent unauthenticated on purpose: locally there is no projected volume + * and service-config runs with auth off, and in the cluster the server rejecting the call + * is a clearer failure than the client refusing to make it. + */ private fun readToken(): String? { val path = Path.of(System.getenv("GROUNDS_TOKEN_FILE") ?: DEFAULT_TOKEN_PATH) return try { @@ -147,34 +139,7 @@ class MotdConfigStore( } } } - - /** - * Attaches the projected ServiceAccount token, which service-config verifies against the - * cluster's JWKS and expects to carry the `grounds-services` audience. - * - * A missing token is passed through unauthenticated on purpose: locally there is no projected - * volume and service-config runs with auth off, and in the cluster the server rejecting the - * call is a clearer failure than the client refusing to make it. - */ - internal class BearerTokenInterceptor(private val token: () -> String?) : ClientInterceptor { - override fun interceptCall( - method: MethodDescriptor, - callOptions: CallOptions, - next: Channel, - ): ClientCall = - object : - ForwardingClientCall.SimpleForwardingClientCall( - next.newCall(method, callOptions) - ) { - override fun start(responseListener: Listener, headers: Metadata) { - token()?.let { headers.put(AUTHORIZATION, "Bearer $it") } - super.start(responseListener, headers) - } - } - - private companion object { - val AUTHORIZATION: Metadata.Key = - Metadata.Key.of("authorization", Metadata.ASCII_STRING_MARSHALLER) - } - } } + +/** A refusal or an outage from service-config, carrying the line it wrote for a human. */ +class MotdStoreException(message: String) : RuntimeException(message) diff --git a/velocity/src/test/kotlin/gg/grounds/proxy/velocity/motd/MotdConfigStoreTest.kt b/velocity/src/test/kotlin/gg/grounds/proxy/velocity/motd/MotdConfigStoreTest.kt new file mode 100644 index 0000000..bd48163 --- /dev/null +++ b/velocity/src/test/kotlin/gg/grounds/proxy/velocity/motd/MotdConfigStoreTest.kt @@ -0,0 +1,126 @@ +package gg.grounds.proxy.velocity.motd + +import com.sun.net.httpserver.HttpExchange +import com.sun.net.httpserver.HttpServer +import java.net.InetSocketAddress +import java.util.concurrent.CopyOnWriteArrayList +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows + +/** + * The store against a stand-in service-config. + * + * The two things worth pinning: reads and writes go to *different* halves of the API, because + * service-config grants them separately — a proxy that may show the MOTD need not be allowed to + * change it. And a refusal has to arrive in chat as the sentence service-config wrote, since that + * is the only place a missing writer grant is explained. + */ +class MotdConfigStoreTest { + + @Test + fun `no MOTD set is null, not an error`() { + // The normal state of a fresh network. The caller leaves Velocity's own MOTD alone. + withServer({ _ -> 404 to """{"code":"not_found","detail":"No such document."}""" }) { store + -> + assertNull(store.read()) + } + } + + @Test + fun `a stored MOTD is unwrapped from the document`() { + withServer({ _ -> + 200 to + """{"namespace":"motd","configKey":"active","contentJson":"{\"text\":\"hello\"}","version":3}""" + }) { store -> + assertEquals("hello", store.read()?.text) + } + } + + @Test + fun `reads use the consumer API and writes the admin API`() { + val seen = CopyOnWriteArrayList() + withServer({ exchange -> + seen.add("${exchange.requestMethod} ${exchange.requestURI.path}") + when (exchange.requestMethod) { + "GET" -> + 200 to + """{"contentJson":"{\"text\":\"hello\"}","namespace":"motd","configKey":"active","version":1}""" + "DELETE" -> 200 to """{"deleted":true,"version":2}""" + else -> 200 to """{"version":2}""" + } + }) { store -> + store.read() + store.write(MotdDocument(text = "hi"), updatedBy = "hendrik") + store.clear(deletedBy = "hendrik") + } + + assertEquals( + listOf( + "GET /v1/config/apps/velocity/envs/stage/namespaces/motd/documents/active", + "PUT /v1/config/admin/apps/velocity/envs/stage/namespaces/motd/documents/active", + "DELETE /v1/config/admin/apps/velocity/envs/stage/namespaces/motd/documents/active", + ), + seen, + ) + } + + @Test + fun `a refusal surfaces the sentence service-config wrote`() { + withServer({ _ -> + 403 to + """{"title":"Forbidden","status":403,"code":"forbidden","detail":"replace document on app 'velocity' requires admin or a configured writer"}""" + }) { store -> + val error = + assertThrows { + store.write(MotdDocument(text = "hi"), updatedBy = "hendrik") + } + assertTrue(error.message!!.contains("configured writer")) + } + } + + @Test + fun `a failure with no readable body still says what was attempted`() { + withServer({ _ -> 500 to "" }) { store -> + val error = assertThrows { store.read() } + assertTrue(error.message!!.contains("read the MOTD")) + } + } + + @Test + fun `clearing reports whether there was anything to clear`() { + withServer({ _ -> 200 to """{"deleted":false,"version":4}""" }) { store -> + assertFalse(store.clear(deletedBy = "hendrik")) + } + } + + private fun withServer( + handler: (HttpExchange) -> Pair, + block: (MotdConfigStore) -> Unit, + ) { + val server = HttpServer.create(InetSocketAddress("127.0.0.1", 0), 0) + server.createContext("/") { exchange -> + exchange.requestBody.readBytes() + val (status, body) = handler(exchange) + val bytes = body.toByteArray() + if (bytes.isEmpty()) { + exchange.sendResponseHeaders(status, -1) + } else { + exchange.sendResponseHeaders(status, bytes.size.toLong()) + exchange.responseBody.use { it.write(bytes) } + } + exchange.close() + } + server.start() + val store = MotdConfigStore.open("velocity", "stage", "127.0.0.1:${server.address.port}") + try { + block(store) + } finally { + store.close() + server.stop(0) + } + } +}