diff --git a/api/build.gradle.kts b/api/build.gradle.kts index 63eb571..a31e52f 100644 --- a/api/build.gradle.kts +++ b/api/build.gradle.kts @@ -160,6 +160,81 @@ tasks { } } +// Boots the shadowed jar and fails if the application context does not start. +// +// This exists because of an outage that every other check in this repository was blind to. A +// @Scheduled(fixedDelay = "60s") annotation -- a spelling Micronaut's converter rejects -- made the +// context fail during start(). It was published, rolled out, and the single api replica went into +// CrashLoopBackOff, taking the whole API down. The entire test suite passed throughout. +// +// It passed for a reason worth knowing: the failure only happens in the *shadowed* jar. On the +// plain test classpath the very same annotation converts fine, so no @MicronautTest -- not even one +// that starts an embedded server -- can see it. Only running the artifact that ships can. +// +// Deliberately not a Test task: there is no test to write. The assertion is "the jar we are about +// to publish starts", and the only way to make it is to start it. +val startupSmokeTest by tasks.registering { + group = "verification" + description = "Boots the shadowed api jar and fails if the Micronaut context does not start." + dependsOn(tasks.shadowJar) + + val jar = tasks.shadowJar.flatMap { it.archiveFile } + val javaLauncher = javaToolchains.launcherFor(java.toolchain) + inputs.file(jar) + outputs.upToDateWhen { false } + + doLast { + val process = ProcessBuilder( + javaLauncher.get().executablePath.asFile.absolutePath, + "-jar", + jar.get().asFile.absolutePath, + ).apply { + redirectErrorStream(true) + // Everything it would reach is pointed somewhere unreachable on purpose. The question + // is only whether startup gets far enough to announce itself. + environment()["APUS_JWT_ISSUER"] = "https://issuer.invalid/v2.0" + environment()["APUS_JWT_JWKS_URI"] = "http://127.0.0.1:1/unused-jwks-endpoint" + environment()["KUBECONFIG"] = "/dev/null" + environment()["MICRONAUT_SERVER_PORT"] = "0" + }.start() + + val output = StringBuilder() + var started = false + try { + process.inputStream.bufferedReader().use { reader -> + val deadline = System.currentTimeMillis() + 90_000 + while (System.currentTimeMillis() < deadline) { + val line = reader.readLine() ?: break + output.appendLine(line) + // Micronaut prints this only once the context is fully up. + if (line.contains("Startup completed") || line.contains("Server Running")) { + started = true + break + } + // Fail fast and loudly rather than waiting out the timeout. + if (line.contains("Error starting Micronaut server")) { + break + } + } + } + } finally { + process.destroyForcibly() + process.waitFor() + } + + if (!started) { + throw GradleException( + "the shadowed api jar did not start -- this is what reaches the cluster:\n\n$output", + ) + } + logger.lifecycle("startupSmokeTest: the shadowed api jar starts") + } +} + +tasks.named("check") { + dependsOn(startupSmokeTest) +} + // TenantIsolationIntegrationTest starts a k3s container (via Testcontainers), applies the // `:operator` module's generated CRDs to it, and proves cross-tenant isolation over a real, // JWT-authenticated HTTP call against a real API server -- minutes of work and Docker, exactly diff --git a/api/src/main/java/net/onelitefeather/apus/api/directory/TenantGroupIndexLoader.java b/api/src/main/java/net/onelitefeather/apus/api/directory/TenantGroupIndexLoader.java index 9d59705..6f00a60 100644 --- a/api/src/main/java/net/onelitefeather/apus/api/directory/TenantGroupIndexLoader.java +++ b/api/src/main/java/net/onelitefeather/apus/api/directory/TenantGroupIndexLoader.java @@ -17,8 +17,13 @@ */ package net.onelitefeather.apus.api.directory; -import io.micronaut.scheduling.annotation.Scheduled; +import io.micronaut.context.event.ApplicationEventListener; +import io.micronaut.runtime.event.ApplicationStartupEvent; +import io.micronaut.scheduling.TaskExecutors; +import io.micronaut.scheduling.TaskScheduler; +import jakarta.inject.Named; import jakarta.inject.Singleton; +import java.time.Duration; import net.onelitefeather.apus.api.rest.tenant.TenantRepository; import net.onelitefeather.apus.api.support.PrincipalResolver; import org.slf4j.Logger; @@ -30,14 +35,23 @@ * {@link DirectoryGuard}, which decides which groups Apus may act on. * *

Those two must be the same set. If they ever diverged, one of them would be wider than the - * other -- either recognising members of a group Apus refuses to manage, or managing a group - * whose members it does not recognise. Handing both from one place is what makes divergence - * impossible rather than merely unlikely. + * other -- either recognising members of a group Apus refuses to manage, or managing a group whose + * members it does not recognise. Handing both from one place is what makes divergence impossible + * rather than merely unlikely. * *

Polled rather than watched. A tenant's group id changes about as often as a tenant is * created, the list is small, and a poll cannot get stuck half-subscribed the way a watch can -- - * the failure mode of a stalled watch here would be members silently failing to resolve, with - * nothing obviously broken to look at. + * a stalled watch here would show up as members silently failing to resolve, with nothing + * obviously broken to look at. + * + *

Scheduled through {@link TaskScheduler} with a typed {@link Duration}, not {@code + * @Scheduled}. The annotation takes its interval as a string, and this class originally used + * {@code @Scheduled(fixedDelay = "60s")} -- a spelling Micronaut's converter rejects. Because the + * scheduled-method processor runs during context startup, that did not fail the one bean; it + * failed the whole application, and the api pod went into CrashLoopBackOff in the cluster with + * {@code SchedulerConfigurationException: Invalid fixed delay definition: 60s}. A {@code Duration} + * constant cannot be spelled wrong, and if it could, the compiler would say so instead of the + * kubelet. * *

A failed refresh leaves the previous index in place. Not an empty one: the Kubernetes * API being briefly unreachable must not log everybody out of their tenant. The very first load @@ -45,29 +59,51 @@ * honestly be. */ @Singleton -public class TenantGroupIndexLoader { +public class TenantGroupIndexLoader implements ApplicationEventListener { private static final Logger LOGGER = LoggerFactory.getLogger(TenantGroupIndexLoader.class); + /** How often the index is rebuilt. See the class Javadoc for why this is not a string. */ + static final Duration REFRESH_INTERVAL = Duration.ofSeconds(60); + private final TenantRepository tenants; private final PrincipalResolver principals; private final DirectoryGuard guard; + private final TaskScheduler scheduler; - public TenantGroupIndexLoader(TenantRepository tenants, PrincipalResolver principals, DirectoryGuard guard) { + public TenantGroupIndexLoader( + TenantRepository tenants, + PrincipalResolver principals, + DirectoryGuard guard, + @Named(TaskExecutors.SCHEDULED) TaskScheduler scheduler) { this.tenants = tenants; this.principals = principals; this.guard = guard; + this.scheduler = scheduler; + } + + /** + * Loads the index once the context is up, then keeps it fresh. + * + *

Deliberately not in the constructor: a constructor that talks to the Kubernetes API makes + * bean creation depend on a network call, and a failure there is far harder to attribute than + * one after startup has been announced. + */ + @Override + public void onApplicationEvent(ApplicationStartupEvent event) { refresh(); + scheduler.scheduleAtFixedRate(REFRESH_INTERVAL, REFRESH_INTERVAL, this::refresh); } /** Rebuilds the index from the current tenant list and publishes it to both consumers. */ - @Scheduled(fixedDelay = "60s") - public final void refresh() { + void refresh() { try { TenantGroupIndex index = TenantGroupIndex.of(tenants.list()); principals.setGroupIndex(index); guard.setManagedGroups(index.managedGroups()); - LOGGER.debug("tenant group index refreshed: {} managed group(s)", index.managedGroups().size()); + LOGGER.debug( + "tenant group index refreshed: {} managed group(s)", + index.managedGroups().size()); } catch (RuntimeException e) { // Keep serving with what we had. Losing the index would sign everybody out of their // tenant over a transient API-server hiccup. diff --git a/api/src/test/java/net/onelitefeather/apus/api/ApplicationStartupTest.java b/api/src/test/java/net/onelitefeather/apus/api/ApplicationStartupTest.java new file mode 100644 index 0000000..7d76163 --- /dev/null +++ b/api/src/test/java/net/onelitefeather/apus/api/ApplicationStartupTest.java @@ -0,0 +1,66 @@ +/** + * Apus - render and host BlueMap maps on Kubernetes. + * Copyright (C) 2026 OneLiteFeather and contributors + *

+ * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + *

+ * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + *

+ * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ +package net.onelitefeather.apus.api; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import io.micronaut.context.ApplicationContext; +import io.micronaut.runtime.server.EmbeddedServer; +import net.onelitefeather.apus.api.directory.TenantGroupIndexLoader; +import net.onelitefeather.apus.api.security.ImpersonationPolicy; +import org.junit.jupiter.api.Test; + +/** + * Boots a real context with an embedded server and asserts it comes up, then resolves the beans + * this module's newest wiring added. Deliberately shallow: it asserts nothing about behaviour. Its + * job is to fail when the application would not start at all -- an unsatisfiable bean, a factory + * that throws, a configuration property that will not bind. + * + *

What it cannot do, stated because it was written believing otherwise. This test was + * added in response to an outage: {@code TenantGroupIndexLoader} declared + * {@code @Scheduled(fixedDelay = "60s")}, a spelling Micronaut's converter rejects, the context + * failed during {@code start()}, and the api pod went into CrashLoopBackOff and took the whole API + * down. This test was then checked against that exact bug, reintroduced faithfully -- and it + * passed. On the plain test classpath the annotation converts fine; it only fails inside + * the shadowed jar. No {@code @MicronautTest}, not even one that starts a server, can see that + * class of failure. + * + *

What does see it is {@code :api:startupSmokeTest}, which boots the shadowed jar itself and is + * wired into {@code check}. That task was verified in both directions -- green on the fix, red on + * the reintroduced bug. If you are looking for the guard against "the artifact does not start", + * it is there, not here. + */ +class ApplicationStartupTest { + + @Test + void theApplicationStarts() { + try (ApplicationContext context = ApplicationContext.run(EmbeddedServer.class, "apitest") + .getApplicationContext()) { + assertTrue(context.isRunning(), "the application context must be running"); + + // Resolved explicitly rather than trusting that startup implies they exist: a bean + // nothing asks for can be broken in ways a context start never notices. + assertNotNull( + context.getBean(TenantGroupIndexLoader.class), + "the tenant group index loader must be resolvable -- it is what maps a token's" + + " groups claim onto a tenant"); + assertNotNull(context.getBean(ImpersonationPolicy.class)); + } + } +} diff --git a/operator/src/main/java/net/onelitefeather/apus/operator/rook/CephObjectStoreUserSpec.java b/operator/src/main/java/net/onelitefeather/apus/operator/rook/CephObjectStoreUserSpec.java index e126afd..35aac2c 100644 --- a/operator/src/main/java/net/onelitefeather/apus/operator/rook/CephObjectStoreUserSpec.java +++ b/operator/src/main/java/net/onelitefeather/apus/operator/rook/CephObjectStoreUserSpec.java @@ -17,7 +17,16 @@ */ package net.onelitefeather.apus.operator.rook; -/** Desired state of a Rook CephObjectStoreUser. Plain data, no Kubernetes access. */ +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; + +/** + * Desired state of a Rook CephObjectStoreUser. Plain data, no Kubernetes access. + * + *

Tolerates unmodelled fields like the status types do. A spec is read back as well as written: + * this operator reads an existing user before touching it, and a field somebody set through + * {@code kubectl} -- or one a Rook upgrade defaults in -- must not make that read throw. + */ +@JsonIgnoreProperties(ignoreUnknown = true) public class CephObjectStoreUserSpec { private String store; diff --git a/operator/src/main/java/net/onelitefeather/apus/operator/rook/CephObjectStoreUserStatus.java b/operator/src/main/java/net/onelitefeather/apus/operator/rook/CephObjectStoreUserStatus.java index f5b541e..b8594e9 100644 --- a/operator/src/main/java/net/onelitefeather/apus/operator/rook/CephObjectStoreUserStatus.java +++ b/operator/src/main/java/net/onelitefeather/apus/operator/rook/CephObjectStoreUserStatus.java @@ -17,7 +17,24 @@ */ package net.onelitefeather.apus.operator.rook; -/** Observed state of a Rook CephObjectStoreUser. */ +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; + +/** + * Observed state of a Rook CephObjectStoreUser. + * + *

Ignores everything it does not model, and that is load-bearing. Rook owns this CRD and + * extends it whenever it likes. It added {@code status.info} and {@code status.observedGeneration}; + * this class knew only {@code phase}, so reading a real user threw + * {@code UnrecognizedPropertyException: Unrecognized field "info"} -- inside {@code + * TenantReconciler}, which reads the existing user before touching it. Every reconciliation of + * every tenant failed from that moment on, and because a tenant only reconciles when something + * changes, nothing looked broken until a release needed one: the operator exhausted its retries and + * the tenant silently never got the resources it was owed. + * + *

The answer is not to add the missing fields as they appear -- that is a race against another + * project's roadmap. A model of somebody else's resource must not fail on a field it does not read. + */ +@JsonIgnoreProperties(ignoreUnknown = true) public class CephObjectStoreUserStatus { private String phase; diff --git a/operator/src/main/java/net/onelitefeather/apus/operator/rook/ObjectBucketClaimSpec.java b/operator/src/main/java/net/onelitefeather/apus/operator/rook/ObjectBucketClaimSpec.java index e7c4cc2..b236311 100644 --- a/operator/src/main/java/net/onelitefeather/apus/operator/rook/ObjectBucketClaimSpec.java +++ b/operator/src/main/java/net/onelitefeather/apus/operator/rook/ObjectBucketClaimSpec.java @@ -17,10 +17,18 @@ */ package net.onelitefeather.apus.operator.rook; +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import java.util.LinkedHashMap; import java.util.Map; -/** Desired state of a Rook ObjectBucketClaim. Plain data, no Kubernetes access. */ +/** + * Desired state of a Rook ObjectBucketClaim. Plain data, no Kubernetes access. + * + *

Tolerates unmodelled fields for the same reason {@link CephObjectStoreUserSpec} does: Rook + * fills in defaults of its own (e.g. {@code objectBucketName}), and reading a claim back must not + * fail over one. + */ +@JsonIgnoreProperties(ignoreUnknown = true) public class ObjectBucketClaimSpec { private String bucketName; diff --git a/operator/src/main/java/net/onelitefeather/apus/operator/rook/ObjectBucketClaimStatus.java b/operator/src/main/java/net/onelitefeather/apus/operator/rook/ObjectBucketClaimStatus.java index d36ff56..328fa6e 100644 --- a/operator/src/main/java/net/onelitefeather/apus/operator/rook/ObjectBucketClaimStatus.java +++ b/operator/src/main/java/net/onelitefeather/apus/operator/rook/ObjectBucketClaimStatus.java @@ -17,7 +17,16 @@ */ package net.onelitefeather.apus.operator.rook; -/** Observed state of a Rook ObjectBucketClaim. */ +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; + +/** + * Observed state of a Rook ObjectBucketClaim. + * + *

Ignores unmodelled fields for the same reason {@link CephObjectStoreUserStatus} does, and + * before it costs anything to learn: Rook owns this CRD, and reading one of its objects must not + * fail over a field this operator never looks at. + */ +@JsonIgnoreProperties(ignoreUnknown = true) public class ObjectBucketClaimStatus { /** Rook sets this to "Bound" once the bucket exists and credentials are written. */ diff --git a/operator/src/test/java/net/onelitefeather/apus/operator/rook/RookResourceSerialisationTest.java b/operator/src/test/java/net/onelitefeather/apus/operator/rook/RookResourceSerialisationTest.java index 79d5e1d..6562548 100644 --- a/operator/src/test/java/net/onelitefeather/apus/operator/rook/RookResourceSerialisationTest.java +++ b/operator/src/test/java/net/onelitefeather/apus/operator/rook/RookResourceSerialisationTest.java @@ -85,4 +85,52 @@ void deserialisesAClaimStatusFromTheCluster() { assertEquals("Bound", claim.getStatus().getPhase()); assertEquals("apus-friends-survival", claim.getSpec().getBucketName()); } + + /** + * Rook owns these CRDs and may add a field to them whenever it likes -- and it did. + * + *

The JSON below is copied verbatim from a live {@code CephObjectStoreUser} in + * {@code rook-ceph-fr01}. Only {@code phase} was modelled here, so reading that object threw + * {@code UnrecognizedPropertyException: Unrecognized field "info"} -- inside {@code + * TenantReconciler}, which reads the existing user before touching it. Every reconciliation of + * every tenant failed from the moment Rook populated {@code status.info}, and because a tenant + * only reconciles when something changes, nothing appeared broken until a release needed one. + * The operator then exhausted its retries and the tenant silently never got its application + * instance. + * + *

The fix is not to add {@code info}: it is to stop caring what else Rook sends. A model of + * somebody else's resource has no business failing on a field it does not read. + */ + @Test + void aRookStatusWithFieldsWeDoNotModelIsReadAnyway() { + String json = + """ + {"apiVersion":"ceph.rook.io/v1","kind":"CephObjectStoreUser",\ + "metadata":{"name":"apus-onelitefeather-dev","namespace":"rook-ceph-fr01"},\ + "spec":{"store":"feather-s3","displayName":"apus-onelitefeather-dev"},\ + "status":{"info":{"secretName":"rook-ceph-object-user-feather-s3-apus-onelitefeather-dev"},\ + "observedGeneration":1,"phase":"Ready"}}"""; + + CephObjectStoreUser user = Serialization.unmarshal(json, CephObjectStoreUser.class); + + assertEquals("Ready", user.getStatus().getPhase()); + assertEquals("feather-s3", user.getSpec().getStore()); + } + + /** The same tolerance on the other Rook resource, before it costs anything to learn. */ + @Test + void anObjectBucketClaimWithFieldsWeDoNotModelIsReadAnyway() { + String json = + """ + {"apiVersion":"objectbucket.io/v1alpha1","kind":"ObjectBucketClaim",\ + "metadata":{"name":"apus-friends-survival","namespace":"bluemap-friends"},\ + "spec":{"bucketName":"apus-friends-survival","storageClassName":"ceph-bucket-fr01",\ + "objectBucketName":"obc-bluemap-friends-apus-friends-survival"},\ + "status":{"phase":"Bound","conditions":[],"somethingRookAddsLater":true}}"""; + + ObjectBucketClaim claim = Serialization.unmarshal(json, ObjectBucketClaim.class); + + assertEquals("Bound", claim.getStatus().getPhase()); + assertEquals("apus-friends-survival", claim.getSpec().getBucketName()); + } }