diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/arch/NativeRuntimeInitRegistrationArchTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/arch/NativeRuntimeInitRegistrationArchTest.java new file mode 100644 index 00000000..7dc08e80 --- /dev/null +++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/arch/NativeRuntimeInitRegistrationArchTest.java @@ -0,0 +1,487 @@ +/* + * Copyright © 2026 CUI-OpenSource-Software (info@cuioss.de) + * + * 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 de.cuioss.sheriff.gateway.arch; + +import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.fields; +import static org.junit.jupiter.api.Assertions.assertAll; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.security.SecureRandom; +import java.util.Arrays; +import java.util.Properties; +import java.util.Set; +import java.util.TreeSet; +import java.util.stream.Collectors; + +import com.tngtech.archunit.base.DescribedPredicate; +import com.tngtech.archunit.core.domain.JavaClass; +import com.tngtech.archunit.core.domain.JavaClasses; +import com.tngtech.archunit.core.domain.JavaField; +import com.tngtech.archunit.core.domain.JavaModifier; +import com.tngtech.archunit.core.importer.ClassFileImporter; +import com.tngtech.archunit.core.importer.ImportOption; +import com.tngtech.archunit.lang.ArchCondition; +import com.tngtech.archunit.lang.ArchRule; +import com.tngtech.archunit.lang.ConditionEvents; +import com.tngtech.archunit.lang.SimpleConditionEvent; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; + +/** + * Standing guard that every gateway class holding a {@code static final SecureRandom} is registered + * for GraalVM runtime initialization. + *

+ * GraalVM initializes static state at build time by default. A + * {@code static final SecureRandom} therefore gets its class initializer run inside the image build, + * which either aborts the build ("instance of Random/SplittableRandom in the image heap") or — worse, + * where the instance is reachable through a shape the analysis tolerates — bakes a + * seeded generator into the image heap, so every process started from that image draws the + * same "random" bytes. Both outcomes are fixed by the same declaration: an + * {@code --initialize-at-run-time=} entry in {@code quarkus.native.additional-build-args}. + *

+ * That declaration lives in {@code application.properties} and the field lives in Java, so nothing + * links the two. Adding a {@code static final SecureRandom} without the matching entry is a silent + * omission that only surfaces during a native build (minutes away) or, in the seeded-heap case, not + * at all. This gate closes that gap in the JVM test cycle. + *

+ * Four legs, all load-bearing. + *

    + *
  1. Positive rule — every selected field's owner must be covered by the registration + * list. Phrased as {@code fields().should(condition)} emitting {@code violated} events, never + * {@code noFields().should(…)}: under the {@code no…} form ArchUnit inverts event polarity, so + * a condition emitting {@code violated} would report nothing and the rule would pass vacuously.
  2. + *
  3. Non-vacuity guard — a direct count proving the selection resolves to at least one + * class today, that the {@code quarkus.native.additional-build-args} key is present, and that + * the parsed registration list is non-empty. Without it, a renamed package or a retired + * configuration key would reduce the rule to a green no-op.
  4. + *
  5. Negative control — the same condition run against + * {@link de.cuioss.sheriff.gateway.arch.specimen.StaticSecureRandomSpecimen}, proving the + * condition actually fails on an unregistered class. The specimen resides under the + * gated package prefix, so the positive rule carves its package out explicitly rather than + * leaning on {@code DO_NOT_INCLUDE_TESTS} as the single guard — see {@link #SPECIMEN_PACKAGE}.
  6. + *
  7. Matched positive controls — {@code SealedSessionCookieCodec} (a per-instance + * {@code private final SecureRandom}, constructed at runtime) and {@code CookieKeyMaterial} (a + * local {@code new SecureRandom()} argument, not a field) must not be + * selected. These are what prove the selection discriminates rather than matching everything + * that mentions {@code SecureRandom}. Both first assert their near-miss is real and + * only then assert exclusion: an exclusion assertion on its own would pass just as happily if + * the class were renamed, moved or deleted, reporting green while proving nothing.
  8. + *
+ *

+ * Coverage is evaluated class → list, never list → class. The gate asks "is this + * class covered by some entry?" and never "does every entry still name a live class". The list + * legitimately carries entries this gate cannot see — most visibly the package-prefix entry + * {@code de.cuioss.sheriff.token.client.flow}, which covers a package in another artifact — and the + * detected set is deliberately not frozen to today's fully-qualified names: freezing it + * would force an edit to this test for every legitimate new registration, which is exactly the + * maintenance tax that makes a gate get deleted. + *

+ * The properties file is read from {@code target/classes}, not the classpath. + * {@code getResourceAsStream("/application.properties")} would resolve against the whole test + * classpath, where a dependency's own {@code application.properties} can win the lookup and the gate + * would then assert against a foreign file. The compiled output path is unambiguous, and + * {@link Files#isRegularFile} guards it so a missing file fails loudly instead of parsing to an empty + * list and passing vacuously. + *

+ * This is a plain JUnit 5 test (no ArchUnit {@code @AnalyzeClasses} runner) so it runs in both + * {@code test} and {@code verify -Ppre-commit}, wiring the guard into the quality gate — the same + * arrangement {@link FrameworkAgnosticArchTest} and {@link NoStoredOptionalArchTest} use. + * + * @author API Sheriff Team + * @since 1.0 + */ +class NativeRuntimeInitRegistrationArchTest { + + /** + * The one package selector this gate is scoped by: the whole gateway tree, not just BFF. + *

+ * Crypto-adjacent static state is not a BFF-only concern. A {@code static final SecureRandom} added + * under {@code auth}, {@code tls}, {@code edge}, {@code forward}, {@code routing} or any other + * gateway package carries the identical build-time-initialization hazard, and a bff-scoped + * selection would simply not see it — the gate would report green while GraalVM baked a seeded + * generator into the image heap. Every holder happens to live under {@code bff} today, so the wider + * radius costs nothing now and closes the gap ahead of the first non-BFF holder. + *

+ * It is deliberately one constant: the import in {@link #PRODUCTION_CLASSES} and — through + * {@link #GATED_CLASS} — both the rule and the guard that proves the rule non-vacuous derive from + * this single value, so one edit moves them together. See {@link #GATED_CLASS} for why that + * selector is one shared object rather than two literals. + */ + private static final String GATED_PACKAGE = "de.cuioss.sheriff.gateway"; + + /** + * The negative control's specimen package, carved out of {@link #GATED_PACKAGE} explicitly. + *

+ * The specimen lives in {@code src/test} under the gated prefix, and + * {@link ImportOption.Predefined#DO_NOT_INCLUDE_TESTS} already keeps it out of + * {@link #PRODUCTION_CLASSES}. The explicit carve-out in {@link #GATED_CLASS} exists so that + * {@code ImportOption} is not the single load-bearing guard: were the production import + * ever widened to include tests, the specimen's deliberate violation would otherwise start failing + * the positive rule rather than only its own negative control. + */ + private static final String SPECIMEN_PACKAGE = "de.cuioss.sheriff.gateway.arch.specimen"; + + private static final String SEALED_SESSION_COOKIE_CODEC = + "de.cuioss.sheriff.gateway.bff.cookie.SealedSessionCookieCodec"; + private static final String COOKIE_KEY_MATERIAL = + "de.cuioss.sheriff.gateway.bff.cookie.CookieKeyMaterial"; + + /** The configuration key carrying the GraalVM build arguments, including the registrations. */ + private static final String NATIVE_BUILD_ARGS_KEY = "quarkus.native.additional-build-args"; + + /** The single build-argument prefix this gate reads out of the comma-separated list. */ + private static final String RUNTIME_INIT_PREFIX = "--initialize-at-run-time="; + + /** + * The compiled properties file, resolved relative to the module base directory Surefire runs in. + * Read from {@code target/classes} rather than the classpath — see the class Javadoc. + */ + private static final Path COMPILED_APPLICATION_PROPERTIES = + Path.of("target", "classes", "application.properties"); + + private static final JavaClasses PRODUCTION_CLASSES = new ClassFileImporter() + .withImportOption(ImportOption.Predefined.DO_NOT_INCLUDE_TESTS) + .importPackages(GATED_PACKAGE); + + /** + * The specimen package is imported separately and with tests included: the control lives + * in {@code src/test}, so the production import above deliberately cannot see it. + */ + private static final JavaClasses SPECIMEN_CLASSES = new ClassFileImporter() + .importPackages(SPECIMEN_PACKAGE); + + /** + * The class-level half of the selection: a gateway class that is not the arch-test specimen. + *

+ * Shared verbatim — the same object, not two expressions of one intent — by the ArchUnit + * rule in {@link #everyStaticSecureRandomHolderIsRegisteredForRuntimeInit()} and by + * {@link #selectedOwnerNames()}. That is what makes the non-vacuity guard's count a statement about + * exactly the set the rule checks, and what stops a future scope edit from moving one without the + * other. + */ + private static final DescribedPredicate GATED_CLASS = + new DescribedPredicate<>("classes under " + GATED_PACKAGE + " outside " + SPECIMEN_PACKAGE) { + @Override + public boolean test(JavaClass javaClass) { + String packageName = javaClass.getPackageName(); + return isWithin(packageName, GATED_PACKAGE) && !isWithin(packageName, SPECIMEN_PACKAGE); + } + }; + + /** + * The selection: a field whose raw type is exactly {@link SecureRandom} and whose modifiers carry + * both {@code STATIC} and {@code FINAL}. + *

+ * Both modifiers are required and neither is redundant. {@code STATIC} is the whole point — only + * static state is touched by build-time class initialization, which is why a per-instance field + * such as {@code SealedSessionCookieCodec}'s is correctly ignored. {@code FINAL} narrows the + * selection to the immutable-holder shape the registration list is actually about; a mutable + * static generator is a different (and separately objectionable) design that this gate does not + * claim to cover. + */ + private static final DescribedPredicate STATIC_FINAL_SECURE_RANDOM = + new DescribedPredicate<>("static final java.security.SecureRandom fields") { + @Override + public boolean test(JavaField field) { + return field.getRawType().isEquivalentTo(SecureRandom.class) + && field.getModifiers().contains(JavaModifier.STATIC) + && field.getModifiers().contains(JavaModifier.FINAL); + } + }; + + @Test + @DisplayName("Every gateway class holding a static final SecureRandom is registered for runtime initialization") + void everyStaticSecureRandomHolderIsRegisteredForRuntimeInit() { + ArchRule rule = fields() + .that().areDeclaredInClassesThat(GATED_CLASS) + .and(STATIC_FINAL_SECURE_RANDOM) + .should(beDeclaredInAClassRegisteredForRuntimeInit(runtimeInitRegistrations())) + .because("GraalVM initializes static state at build time, so an unregistered " + + "static final SecureRandom either fails the native build or bakes a seeded " + + "generator into the image heap — add an " + RUNTIME_INIT_PREFIX + + " entry to " + NATIVE_BUILD_ARGS_KEY + " in application.properties"); + + rule.check(PRODUCTION_CLASSES); + } + + /** + * Guards the guard, on all three axes the rule above silently depends on: the field selection, the + * configuration key, and the parsed registration list. + *

+ * Any one of them resolving to nothing would leave the rule green while checking nothing — a + * renamed gateway package matches zero fields, a retired configuration key parses to an empty list, + * and a reworked build-argument syntax yields zero registrations. The negative control below + * cannot catch that: it exercises its own hardcoded specimen package, so it proves the condition + * works while saying nothing about whether the real inputs still resolve. + *

+ * Deliberately a direct count rather than an {@link ArchRule} with an always-true condition: any + * such condition risks failing for its own reason instead of for emptiness, which would make this + * guard red for a reason unrelated to the gap it exists to detect. + */ + @Test + @DisplayName("Runtime-init gate is non-vacuous: field selection, config key, and parsed list all resolve") + void gateIsNonVacuous() { + Properties compiled = loadCompiledApplicationProperties(); + String declaredValue = compiled.getProperty(NATIVE_BUILD_ARGS_KEY); + Set selectedOwners = selectedOwnerNames(); + Set registrations = declaredValue == null + ? Set.of() + : parseRuntimeInitRegistrations(declaredValue); + + assertAll("runtime-init gate non-vacuity", + () -> assertNotNull(declaredValue, + "Key '" + NATIVE_BUILD_ARGS_KEY + "' is absent from " + + COMPILED_APPLICATION_PROPERTIES + " — the rule above is parsing " + + "nothing and silently protecting nothing. Restore the key, or retire " + + "this gate deliberately if native builds no longer read it."), + () -> assertFalse(selectedOwners.isEmpty(), + "No static final SecureRandom field was found among " + GATED_CLASS.getDescription() + + " — the rule above matched zero fields and passed vacuously. Fix " + + "GATED_PACKAGE if the package was renamed, or retire this gate " + + "deliberately if the last such field was removed."), + () -> assertFalse(registrations.isEmpty(), + "Key '" + NATIVE_BUILD_ARGS_KEY + "' carries no '" + RUNTIME_INIT_PREFIX + + "' entry — every selected class would report as unregistered, or the " + + "build-argument syntax changed under this parser.")); + } + + @Nested + @DisplayName("Matched controls") + class MatchedControls { + + /** + * Negative control: proves the condition actually fails on a class that holds a + * {@code static final SecureRandom} without a matching registration. + *

+ * {@code allowEmptyShould(true)} is deliberate and must not be removed. If the + * specimen package ever empties out, that setting makes {@code check} pass, which makes + * {@code assertThrows} fail loudly and tells us the control has stopped controlling anything. + * Removing it would invert that: an empty package would make {@code check} throw on emptiness, + * {@code assertThrows} would be satisfied by the wrong exception, and the test would go green + * while proving nothing. + */ + @Test + @DisplayName("Gate detects a deliberately unregistered static final SecureRandom (negative control)") + void gateFailsOnUnregisteredSpecimen() { + ArchRule ruleAgainstSpecimens = fields() + .that().areDeclaredInClassesThat().resideInAPackage(SPECIMEN_PACKAGE) + .and(STATIC_FINAL_SECURE_RANDOM) + .should(beDeclaredInAClassRegisteredForRuntimeInit(runtimeInitRegistrations())) + .allowEmptyShould(true); + + assertThrows(AssertionError.class, + () -> ruleAgainstSpecimens.check(SPECIMEN_CLASSES), + "The gate must fail on StaticSecureRandomSpecimen's unregistered static final " + + "SecureRandom — if it does not, the specimen lost the field, lost a " + + "modifier, or was somehow covered by a registration entry"); + } + + /** + * Matched positive control: {@code SealedSessionCookieCodec} holds a {@link SecureRandom} + * field, but a per-instance one, constructed when the codec is constructed at runtime. + * Nothing about it reaches the image heap, so it must not be selected. + *

+ * The control asserts the near-miss is real — the class does declare a {@code SecureRandom} + * field — before asserting it is excluded. Without that first half the control would pass just + * as happily if the field were deleted, proving nothing about the selection. + */ + @Test + @DisplayName("Selection excludes a per-instance SecureRandom field (positive control)") + void selectionExcludesPerInstanceSecureRandomField() { + assertAll("SealedSessionCookieCodec is a real, excluded near-miss", + () -> assertTrue(declaresSecureRandomField(SEALED_SESSION_COOKIE_CODEC), + SEALED_SESSION_COOKIE_CODEC + " no longer declares a SecureRandom field — " + + "this control has stopped controlling anything; point it at another " + + "per-instance holder or retire it deliberately"), + () -> assertFalse(selectedOwnerNames().contains(SEALED_SESSION_COOKIE_CODEC), + SEALED_SESSION_COOKIE_CODEC + " was selected, but its SecureRandom is a " + + "per-instance field constructed at runtime — the selection has " + + "stopped requiring the STATIC modifier and now over-matches")); + } + + /** + * Matched positive control: {@code CookieKeyMaterial} constructs a {@link SecureRandom} as a + * local argument to {@code KeyGenerator.init} and stores none, so it must not be selected. + *

+ * This is the second discrimination axis — the first control proves the selection reads + * modifiers, this one proves it reads fields rather than every mention of the type. + *

+ * Like its sibling above, it asserts the near-miss is real before asserting exclusion, + * and for the same reason: {@link #selectedOwnerNames()} can never contain a class that no + * longer exists, so an exclusion-only control would report green just as happily if + * {@code CookieKeyMaterial} were renamed, moved out of the imported tree or deleted. The + * near-miss has two halves and both are asserted: the class must still reference + * {@link SecureRandom} (otherwise it is no longer near anything) and must still declare no + * {@code SecureRandom} field (otherwise it is no longer a miss). + */ + @Test + @DisplayName("Selection excludes a local SecureRandom construction (positive control)") + void selectionExcludesLocalSecureRandomConstruction() { + assertAll("CookieKeyMaterial is a real, excluded near-miss", + () -> assertTrue(referencesSecureRandom(COOKIE_KEY_MATERIAL), + COOKIE_KEY_MATERIAL + " is not an imported production class referencing " + + "SecureRandom — it was renamed, moved out of the imported tree, " + + "deleted, or stopped using SecureRandom altogether. This control " + + "has stopped controlling anything; point it at another local-" + + "construction site or retire it deliberately"), + () -> assertFalse(declaresSecureRandomField(COOKIE_KEY_MATERIAL), + COOKIE_KEY_MATERIAL + " now declares a SecureRandom field, so it is no " + + "longer the local-construction near-miss this control needs. " + + "Point the control at another local-construction site — and check " + + "whether the new field needs a runtime-init registration"), + () -> assertFalse(selectedOwnerNames().contains(COOKIE_KEY_MATERIAL), + COOKIE_KEY_MATERIAL + " was selected, but it constructs SecureRandom as a " + + "local argument and stores none — the selection has stopped " + + "reading declared fields and now matches any reference to the type")); + } + } + + /** + * The condition: the field's owning class must be covered by the registration list. + *

+ * Phrased to emit {@link SimpleConditionEvent#violated} from a positive + * {@code fields().should(…)} rule — see the class Javadoc for why the {@code noFields()} form + * would pass vacuously here. + * + * @param registrations the class names and package prefixes parsed out of the build arguments + * @return the condition + */ + private static ArchCondition beDeclaredInAClassRegisteredForRuntimeInit( + Set registrations) { + return new ArchCondition<>("be declared in a class registered for GraalVM runtime initialization") { + @Override + public void check(JavaField field, ConditionEvents events) { + String owner = field.getOwner().getFullName(); + if (!isRegistered(owner, registrations)) { + events.add(SimpleConditionEvent.violated(field, + owner + " declares the static final SecureRandom field '" + field.getName() + + "' but no " + RUNTIME_INIT_PREFIX + " entry in " + + NATIVE_BUILD_ARGS_KEY + " covers it — GraalVM would initialize the " + + "class at build time and bake a seeded generator into the image heap")); + } + } + }; + } + + /** + * Answers coverage class → list: a class is registered when an entry names it exactly, or names a + * package that contains it. GraalVM reads a bare package name as a prefix over that package and + * its classes, which is how the one package-prefix entry in the list covers a whole flow package. + */ + private static boolean isRegistered(String className, Set registrations) { + return registrations.stream().anyMatch(entry -> isWithin(className, entry)); + } + + /** + * Reads and parses the registration list, failing the calling test loudly when the key is absent + * rather than handing back an empty set that would make every check pass vacuously. + */ + private static Set runtimeInitRegistrations() { + String declaredValue = loadCompiledApplicationProperties().getProperty(NATIVE_BUILD_ARGS_KEY); + assertNotNull(declaredValue, + "Key '" + NATIVE_BUILD_ARGS_KEY + "' is absent from " + COMPILED_APPLICATION_PROPERTIES); + return parseRuntimeInitRegistrations(declaredValue); + } + + /** + * Splits the comma-separated build-argument list and keeps the {@code --initialize-at-run-time=} + * suffixes. Every other argument (URL protocols, optimisation level, …) is ignored: this gate + * reads one flag out of a shared list and makes no claim about the rest. + */ + private static Set parseRuntimeInitRegistrations(String declaredValue) { + return Arrays.stream(declaredValue.split(",")) + .map(String::trim) + .filter(argument -> argument.startsWith(RUNTIME_INIT_PREFIX)) + .map(argument -> argument.substring(RUNTIME_INIT_PREFIX.length()).trim()) + .filter(entry -> !entry.isEmpty()) + .collect(Collectors.toCollection(TreeSet::new)); + } + + private static Properties loadCompiledApplicationProperties() { + assertTrue(Files.isRegularFile(COMPILED_APPLICATION_PROPERTIES), + "Expected the compiled properties at " + COMPILED_APPLICATION_PROPERTIES.toAbsolutePath() + + " — this gate reads the compiled output rather than the classpath so a " + + "dependency's own application.properties cannot win the lookup. Run the " + + "module's compile phase before this test."); + + Properties compiled = new Properties(); + assertDoesNotThrow(() -> { + try (InputStream declared = Files.newInputStream(COMPILED_APPLICATION_PROPERTIES)) { + compiled.load(declared); + } + }, "Failed to read " + COMPILED_APPLICATION_PROPERTIES); + return compiled; + } + + /** + * The fully-qualified names of the classes the selection currently matches, sorted so a failure + * message reads the same on every run. + */ + private static Set selectedOwnerNames() { + return PRODUCTION_CLASSES.stream() + .filter(GATED_CLASS) + .flatMap(javaClass -> javaClass.getFields().stream()) + .filter(STATIC_FINAL_SECURE_RANDOM::test) + .map(field -> field.getOwner().getFullName()) + .collect(Collectors.toCollection(TreeSet::new)); + } + + private static boolean declaresSecureRandomField(String className) { + return PRODUCTION_CLASSES.stream() + .filter(javaClass -> javaClass.getFullName().equals(className)) + .flatMap(javaClass -> javaClass.getFields().stream()) + .anyMatch(field -> field.getRawType().isEquivalentTo(SecureRandom.class)); + } + + /** + * Whether the named class is present in {@link #PRODUCTION_CLASSES} and depends on + * {@link SecureRandom} in any way — a constructor call, a method call, a parameter or a field type. + *

+ * This is the "is it still a near-miss?" half of the local-construction control. It is deliberately + * broader than field declaration: the whole point of that control is a class that touches + * {@code SecureRandom} without holding one, so the anchor has to see the touch. It is also + * deliberately narrower than mere existence — a class that stopped mentioning + * {@code SecureRandom} entirely is no longer near the selection and controls nothing. + */ + private static boolean referencesSecureRandom(String className) { + return PRODUCTION_CLASSES.stream() + .filter(javaClass -> javaClass.getFullName().equals(className)) + .anyMatch(javaClass -> javaClass.getDirectDependenciesFromSelf().stream() + .anyMatch(dependency -> dependency.getTargetClass().isEquivalentTo(SecureRandom.class))); + } + + /** + * Self-or-descendant containment over a dotted namespace: {@code candidate} is + * {@code prefix}, or sits beneath it. Serves both halves of this gate — the package selection in + * {@link #GATED_CLASS} and the registration lookup in {@link #isRegistered} — so the two cannot + * drift into disagreeing about what "inside" means. + *

+ * The dot is appended deliberately: a bare {@code startsWith} would let {@code …gateway} swallow + * an unrelated sibling package such as {@code …gatewayadmin}, and a registration entry for + * {@code …bff.cookie} cover an unrelated {@code …bff.cookiejar}. + */ + private static boolean isWithin(String candidate, String prefix) { + return candidate.equals(prefix) || candidate.startsWith(prefix + "."); + } +} diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/arch/specimen/StaticSecureRandomSpecimen.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/arch/specimen/StaticSecureRandomSpecimen.java new file mode 100644 index 00000000..13e148d3 --- /dev/null +++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/arch/specimen/StaticSecureRandomSpecimen.java @@ -0,0 +1,69 @@ +/* + * Copyright © 2026 CUI-OpenSource-Software (info@cuioss.de) + * + * 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 de.cuioss.sheriff.gateway.arch.specimen; + +import java.security.SecureRandom; + +/** + * Standing negative control for the runtime-init registration gate in + * {@code NativeRuntimeInitRegistrationArchTest}: a class that deliberately holds a + * {@code static final SecureRandom} while being absent from the + * {@code --initialize-at-run-time=} list in {@code application.properties}, so the gate has a known + * violation to detect. + *

+ * Its matched positive counterparts are not specimens but real production classes — + * {@code SealedSessionCookieCodec} (a per-instance {@code SecureRandom} field) and + * {@code CookieKeyMaterial} (a local {@code new SecureRandom()} argument). Together they prove the + * gate discriminates on the {@code static}/{@code final} modifiers and on field declaration, + * rather than merely always-failing on any mention of {@link SecureRandom}. + *

+ * This class must never be added to the registration list, and the field below must + * keep both the {@code static} and {@code final} modifiers. Either change silently disarms the + * control: the gate would stop reporting a violation here and the {@code assertThrows} guarding it + * would fail, which is the loud failure this constraint exists to keep loud. + *

+ * Living in {@code src/test} it is never compiled into the native image, so the deliberate violation + * carries no build-time or security consequence of its own. + * + * @author API Sheriff Team + * @since 1.0 + */ +public final class StaticSecureRandomSpecimen { + + /** + * The deliberate violation: static, final, and unregistered. + *

+ * Private with a package-private reader below rather than exposed directly — the gate reads + * declared fields from bytecode and is indifferent to visibility, so the narrower shape costs the + * control nothing while keeping the specimen a well-formed class. + */ + private static final SecureRandom UNREGISTERED_RANDOM = new SecureRandom(); + + private StaticSecureRandomSpecimen() { + // Specimen: it exists to be read from bytecode, never to be instantiated. + } + + /** + * Draws a byte from the specimen's generator, so the field is genuinely used. + * + * @return an arbitrary byte + */ + static byte nextByte() { + byte[] drawn = new byte[1]; + UNREGISTERED_RANDOM.nextBytes(drawn); + return drawn[0]; + } +} diff --git a/demo-client/scripts/start-dev-environment.sh b/demo-client/scripts/start-dev-environment.sh index b7d497bb..4b5bb5cf 100755 --- a/demo-client/scripts/start-dev-environment.sh +++ b/demo-client/scripts/start-dev-environment.sh @@ -160,16 +160,23 @@ echo "🐳 Rebuilding the api-sheriff image from the native executable..." export DOCKER_BUILDKIT=1 $COMPOSE_CMD build api-sheriff -# Quarkus file logging writes to the bind-mounted /logs. The container runs as uid 1001 while this -# host directory is created by the (differently-numbered) build user, so without a world-writable -# dedicated subdirectory the file sink fails with "FileNotFoundException: /logs/quarkus.log -# (Permission denied)". Grant world write on that subdirectory ONLY — least privilege, ephemeral -# test output — exactly as integration-tests/scripts/start-integration-container.sh does. The -# container keeps its no-new-privileges / cap_drop / read_only posture. +# Quarkus file logging writes to the bind-mounted /logs. The container runs as the distroless +# 'nonroot' user (uid 65532) while this host directory is created by the (differently-numbered) +# build user, so without a world-writable dedicated subdirectory the file sink fails with +# "FileNotFoundException: /logs/quarkus.log (Permission denied)". Grant world write on that +# subdirectory ONLY — least privilege, ephemeral test output — exactly as +# integration-tests/scripts/start-integration-container.sh does. The container keeps its +# no-new-privileges / cap_drop / read_only posture. +# +# Mode 1777, not 0777: the sticky bit keeps that world write from also being a world DELETE. +# Without it any other local account on this host can remove or replace quarkus.log, which is +# the log a developer reads to diagnose a failed run. The sticky bit restricts unlink and +# rename to the file's owner and the directory's owner, and costs nothing here either: the +# container still creates and rotates its own files, and 'mvn clean' runs as the owning build user. LOG_TARGET_ROOT="${LOG_TARGET_DIR:-${IT_DIR}/target}" export LOG_TARGET_DIR="${LOG_TARGET_ROOT}/quarkus-logs" mkdir -p "${LOG_TARGET_DIR}" -chmod 0777 "${LOG_TARGET_DIR}" +chmod 1777 "${LOG_TARGET_DIR}" echo "📁 Quarkus logs will be written to: ${LOG_TARGET_DIR}/quarkus.log" # Keycloak FIRST, and READY, before either gateway starts. The native app eagerly loads the realm's @@ -213,6 +220,15 @@ done echo "🐳 Starting ONLY ${DEMO_GATEWAY_SERVICES[*]} (no other stack service is touched)..." $COMPOSE_CMD up -d --no-deps "${DEMO_GATEWAY_SERVICES[@]}" +# Wait for READINESS, not liveness, and keep the retry budget as ONE number rather than three +# literals that can drift apart — the same pairing integration-tests/scripts/start-integration-container.sh +# uses, and for the same reason: /q/health/live answers as soon as the process is up, which is +# strictly earlier than the point at which the SPA can be driven against it. The switch costs no +# additional wait — GatewayReadinessCheck's `jwks` datum is a boot-time constructibility fact +# (ADR-0027), so readiness flips at the same moment liveness does. The measured live-to-ready delta +# behind that claim is in doc/development/integration-test-topology.adoc, "The Readiness Contract". +GATEWAY_READY_ATTEMPTS=30 + echo "⏳ Waiting for the demo gateway instances to be ready..." while read -r GATEWAY_SERVICE GATEWAY_MGMT_SCHEME GATEWAY_MGMT_PORT _; do [[ -z "$GATEWAY_SERVICE" ]] && continue @@ -222,20 +238,20 @@ while read -r GATEWAY_SERVICE GATEWAY_MGMT_SCHEME GATEWAY_MGMT_PORT _; do if [[ "$GATEWAY_MGMT_SCHEME" == "https" ]]; then # -k is load-bearing on an HTTPS management interface: it serves a self-signed localhost # bundle, and without it curl fails certificate validation and this wait degrades into a - # silent 30-attempt timeout against a perfectly healthy container. + # silent full-budget timeout against a perfectly healthy container. GATEWAY_PROBE_OPTS+=(-k) GATEWAY_DIAG_OPTS+=(-k) fi GATEWAY_MGMT_URL="${GATEWAY_MGMT_SCHEME}://localhost:${GATEWAY_MGMT_PORT}" echo "⏳ Waiting for ${GATEWAY_SERVICE} (management ${GATEWAY_MGMT_SCHEME} on ${GATEWAY_MGMT_PORT})..." - for i in {1..30}; do - if curl "${GATEWAY_PROBE_OPTS[@]}" "${GATEWAY_MGMT_URL}/q/health/live" > /dev/null 2>&1; then + for ((i = 1; i <= GATEWAY_READY_ATTEMPTS; i++)); do + if curl "${GATEWAY_PROBE_OPTS[@]}" "${GATEWAY_MGMT_URL}/q/health/ready" > /dev/null 2>&1; then echo "✅ ${GATEWAY_SERVICE} is ready!" break fi - if [ "$i" -eq 30 ]; then - echo "❌ ${GATEWAY_SERVICE} failed to start within 30 attempts" + if [ "$i" -eq "$GATEWAY_READY_ATTEMPTS" ]; then + echo "❌ ${GATEWAY_SERVICE} failed to become ready within ${GATEWAY_READY_ATTEMPTS} attempts" # Capture the container log + health payload so a startup failure is diagnosable from # the CI artifacts rather than only from a lost console. DIAG_DIR="${MODULE_DIR}/target/test-results" @@ -247,7 +263,7 @@ while read -r GATEWAY_SERVICE GATEWAY_MGMT_SCHEME GATEWAY_MGMT_PORT _; do echo "" exit 1 fi - echo "⏳ Waiting for ${GATEWAY_SERVICE}... (attempt $i/30)" + echo "⏳ Waiting for ${GATEWAY_SERVICE}... (attempt $i/${GATEWAY_READY_ATTEMPTS})" sleep 1 done done <<< "$GATEWAY_TARGETS" diff --git a/doc/adr/0030-Comment-only_invariants_become_positively-phrased_fitness_functions_shipped_with_a_four-leg_control_set.adoc b/doc/adr/0030-Comment-only_invariants_become_positively-phrased_fitness_functions_shipped_with_a_four-leg_control_set.adoc new file mode 100644 index 00000000..c133d448 --- /dev/null +++ b/doc/adr/0030-Comment-only_invariants_become_positively-phrased_fitness_functions_shipped_with_a_four-leg_control_set.adoc @@ -0,0 +1,160 @@ += ADR-0030: Comment-only invariants become positively-phrased fitness functions shipped with a four-leg control set +:toc: left +:toclevels: 2 +:sectnums: + +// adr-metadata +// summary: A repository invariant asserted only by an explanatory comment is converted into a machine-checked fitness function, phrased positively so the condition's violation events are not inverted away, and shipped with four mandatory control legs so the gate cannot replace a failed comment with a green one. +// tags: fitness-function, archunit, vacuity, test-controls, invariants, native-image +// affects: api-sheriff +// supersedes: +// end-adr-metadata + +== Status + +Proposed + +== Context + +Some repository invariants cannot be expressed in the type system and are instead +recorded as prose beside the value they constrain -- a comment above a +configuration list, a note in a Javadoc block. Prose has a structural weakness as +an enforcement mechanism: it is read by whoever happens to read it, while the +invariant must hold for every future author. A comment that names the exact +failure mode it exists to prevent can still fail to prevent it. + +The obvious remedy is to convert the invariant into an executable assertion. That +move introduces a second, quieter failure mode: an assertion can be *vacuous* -- +green not because the invariant holds but because the rule matches nothing, +inverts its own verdict, or asserts a property that cannot fail. A vacuous gate is +strictly worse than the comment it replaces, because it converts an unenforced +invariant into an apparently-enforced one and removes the reader's motive to check. + +Vacuity has several distinct sources, and closing one leaves the others open: + +* **Polarity inversion.** Under ArchUnit's `no…` phrasing the framework inverts + the polarity of a condition's events: a `satisfied()` event is treated as a + violation and a `violated()` event as a pass. A hand-written `ArchCondition` + whose `check()` reports offenders via `SimpleConditionEvent.violated(...)` -- + the natural way to write one -- therefore reports nothing, every offender is + silently reclassified as compliance, and the rule stays green over an + arbitrarily dirty codebase. +* **Empty selection.** A rule whose predicate matches zero classes finds zero + violations and passes. Nothing distinguishes "no offenders exist" from "the + selector is wrong". +* **Always-passing assertion.** A rule that asserts only the absence of something + passes when the subject is renamed, moved, or deleted -- the property it + believed it was checking is gone, and so is the check. +* **Wrong artifact.** A gate that reads shipped configuration through the + classpath can resolve a test-profile copy instead of the packaged one, and + assert green against a file that is not what ships. + +The structural choice point is therefore not "assert or comment" but "what must an +assertion ship with before it may be trusted as enforcement". + +== Decision + +An invariant that a comment cannot enforce is converted into a fitness function, +and the fitness function is **phrased positively** -- `classes().should(condition)` +or `fields().should(condition)`, with the condition emitting +`SimpleConditionEvent.violated(...)` for offenders. The `no…` form is not used for +a coverage assertion, because it inverts the very events the condition reports. + +Every such fitness function ships **four control legs**, each closing a distinct +vacuity source: + +. **Non-vacuity guard** -- a direct assertion that the selection predicate resolves + to at least one subject today, that any parsed input is non-empty, and that the + configuration key the rule reads is present. This is what distinguishes "no + offenders" from "no selection". +. **Negative control** -- a deliberately non-compliant fixture, in a dedicated + specimen package, that the rule must reject. Proven by asserting the rule throws + against it. This is what proves the rule is *capable of failing*. +. **Matched positive controls** -- real classes from the production tree that are + near-misses for the predicate and must NOT be selected. A positive control + asserts the near-miss property still holds *before* asserting exclusion; + otherwise it passes when the class is renamed or deleted, which is the + always-passing failure one level down. +. **Specimen carve-out** -- the specimen package is excluded from the production + selection *explicitly*, not merely by a test-source import filter, so no single + incidental mechanism is the only thing keeping the deliberate violation out of + the real rule. + +Two further constraints are part of the contract: + +* A gate that reads shipped configuration resolves it **by path** against the + packaged output directory, guarded by a regular-file precondition. Classpath + resolution is not used, because test resources may precede packaged resources on + the test classpath and the gate would assert against the wrong artifact. +* The rule asserts a **property**, never equality with today's membership. + Freezing the selected set to its current members reintroduces the maintenance + burden the gate exists to remove: every legitimate addition becomes a test edit. + +A selection radius is justified against the rule's **scan** radius, not against +today's known members. When a rule imports a wide tree but selects a narrow subtree, +the difference is scanned-then-discarded surface where the invariant is unenforced. + +== Consequences + +=== Positive + +* An invariant that was previously enforced by attention is enforced by the build, + and fails in the fast local gate rather than in a downstream packaging step. +* The control legs make the gate's own health observable: a change that breaks the + selector or neuters the condition fails the guard or the negative control rather + than silently passing. +* Because the rule asserts a property rather than a membership list, legitimate + additions to the constrained set need no test edit. + +=== Negative + +* A fitness function with four control legs is materially larger than the comment + it replaces, and includes a fixture whose only purpose is to be rejected. +* The specimen package is production-shaped code that exists solely as a control, + and a reader encountering it without the surrounding contract may mistake it for + a defect. + +=== Risks + +* A control leg can itself rot. A matched positive control whose near-miss property + silently disappears degrades to an always-passing assertion -- which is why the + contract requires asserting the property before asserting exclusion. +* The specimen carve-out must be maintained alongside the selection predicate. A + widened predicate that does not widen the carve-out pulls the deliberate + violation into the production rule. + +== Alternatives Considered + +**Leave the invariant as an explanatory comment.** Costs nothing and reads well at +the point of definition. It fails because enforcement by reading does not scale to +authors who never read that point: the comment can name the exact failure mode and +still not prevent it. + +**Widen the underlying configuration to a package prefix instead of asserting +per-class.** Removes the invariant entirely by making the constrained set +structural. Rejected because a prefix imposes the constrained behaviour on the +whole package rather than the members that need it -- a materially larger blast +radius, not a like-for-like swap. + +**Generate the configuration from the source set at build time.** Removes the +possibility of drift by construction, which is stronger than any assertion. +Rejected on cost: it introduces new build machinery whose maintenance and failure +modes need separate ownership, where the assertion needs none. + +**Assert the selected set equals its current membership.** Trivially detects any +change. Rejected because it detects *legitimate* changes just as loudly, converting +every valid addition into a test edit -- the maintenance burden the gate was +introduced to remove. + +**Phrase the rule with `no…` and report offenders as violations.** Reads more +naturally as English. Rejected because the framework inverts the condition's event +polarity under that form, so the rule reports nothing and stays green over an +arbitrarily dirty codebase. + +The unifying failure across the rejected options is that each either moves the +enforcement burden somewhere it is not carried, or produces a gate whose green is +not evidence. + +== References + +* link:0025-The_whole_server-TLS_surface_is_neutral_in_gatewayyaml_and_bound_by_exactly_two_seams.adoc[ADR-0025] -- an example of an invariant carried structurally rather than by convention diff --git a/doc/adr/0031-Host-side_readiness_gates_derive_the_probe_URL_from_the_resolved_Compose_model_and_assert_readiness.adoc b/doc/adr/0031-Host-side_readiness_gates_derive_the_probe_URL_from_the_resolved_Compose_model_and_assert_readiness.adoc new file mode 100644 index 00000000..779e5410 --- /dev/null +++ b/doc/adr/0031-Host-side_readiness_gates_derive_the_probe_URL_from_the_resolved_Compose_model_and_assert_readiness.adoc @@ -0,0 +1,148 @@ += ADR-0031: Host-side readiness gates derive the probe URL from the resolved Compose model and assert readiness +:toc: left +:toclevels: 2 +:sectnums: + +// adr-metadata +// summary: A host-side bring-up gate restates neither scheme nor published port -- it derives both from the resolved Compose model via the service's management-scheme label -- and it waits on the readiness probe rather than the liveness probe, with a retry budget traceable to a worst-case ready time measured under contention. +// tags: readiness, bring-up, integration-tests, compose-model, probe-derivation, retry-budget +// affects: integration-tests, demo-client +// supersedes: +// end-adr-metadata + +== Status + +Proposed + +== Context + +A host-side script that brings a container stack up must decide when the stack may +be driven. That decision has two independent failure modes, and both are quiet. + +**Waiting on the wrong signal.** A liveness probe answers as soon as the process is +up. Readiness answers when the instance can serve. The window between them is +exactly the window in which a test suite's first request reaches an instance whose +bindings are not yet resolved. A suite gated on liveness therefore has a +nondeterministic failure surface that grows with the instance's boot cost, and the +resulting failures present as flakes rather than as a gate defect. + +**Restating the model in the gate.** A probe URL embeds a scheme and a published +port. Both are already declared in the Compose model. Restating them in the script +creates two authorities for one fact: republishing a service on a different host +port, adding an instance, or changing an instance's management scheme silently +desynchronises the gate. The failure is asymmetric -- the gate keeps passing while +probing the wrong endpoint, or fails against a healthy container. + +A third consideration constrains the remedy. Switching a gate from liveness to +readiness changes *when* it clears, so its retry budget must be re-derived rather +than carried over. A budget adopted from a neighbouring wait, or picked as a round +number, is not evidence; and a budget derived from an uncontended local run +measures the wrong machine. + +== Decision + +A host-side readiness gate is built on three rules. + +**Gate on readiness, not liveness.** The wait probes the readiness endpoint. The +probe must fail closed: the HTTP client is configured to treat a non-2xx response +as failure, so a service answering "not ready" while starting does not clear the +gate at port-accept time. + +**Derive the whole probe target from the resolved Compose model.** The script +resolves the service set, each service's published management port, and the scheme +its management interface speaks, from the model -- the scheme via an explicit +per-service label rather than by inferring it from a name or a port number. No +host and no *published* host port is restated in the script. The container-side +management port remains a fixed literal, used as the selector that picks the +management binding out of each service's port list; that is a stable +platform-level contract rather than a per-instance fact, so it is not a +derivation target. Adding, removing, or renumbering an instance requires no +script edit. + +Deriving the scheme structurally is what makes a *deliberate* exception free: an +instance whose management interface serves plain HTTP by design is probed over +plain HTTP because its label says so, not because the probe carries a branch on +its service name. The exception costs one label, not one special case. + +**Derive the retry budget from measurement, and record the measurement.** The +budget is traceable to an observed worst-case time-to-ready, measured with all +instances brought up concurrently on a machine under load, plus a stated headroom +factor. The measured figures and the factor are recorded in developer documentation +alongside the gate, so a later reader can re-evaluate the budget rather than guess +at it. Within a script, the retry budget is a single constant read by every site +that needs it -- the loop bound, the last-attempt comparison, the progress +message -- so the number cannot drift between spellings. The constant is +per-script: two bring-up scripts that gate independently each declare their own, +and that duplication is stated as a consequence below rather than claimed away. + +One nuance belongs to the rationale rather than the rule. When a readiness check +reports a *boot-time constructibility* fact read from a cached instance, the +network cost sits in boot, ahead of readiness, not inside the probe. The reason to +re-derive the budget is therefore that readiness lands later than liveness, not +that the probe became network-dependent. Stating the rationale correctly matters: +the wrong rationale over-predicts probe cost and invites an inflated budget. + +== Consequences + +=== Positive + +* The suite cannot start against a half-initialised instance, removing a class of + failure that presents as flakiness rather than as a gate defect. +* Topology changes -- new instances, republished ports, a changed management scheme + -- propagate to the gate with no script edit, so the model stays the single + authority. +* A deliberate per-instance exception is expressed as data on the service rather + than as a branch in the probe, so one loop covers every instance. +* The budget is auditable: its origin is a recorded measurement, not a convention. + +=== Negative + +* The gate now depends on the model being resolvable at bring-up time, and on each + service carrying its scheme label. A service that omits the label is not probed + correctly, so the discovery step must fail loudly rather than degrade. +* Re-deriving the budget requires an instrumented run under contention, which is + more work than adopting a neighbouring value. + +=== Risks + +* The recorded measurement ages. It bounds the live-to-ready delta on the machine + measured, not absolute container start on slower hosts, so the budget must carry + headroom rather than track the observed figures. +* Where two bring-up scripts derive from the same model independently, both the + derivation logic and the retry-budget constant are duplicated. Duplicated + derivation is materially safer than a duplicated literal -- both stay correct + when the model changes -- but the budget constant is a genuine duplicate: the + two can be re-tuned apart, and only a shared library would make them one. + +== Alternatives Considered + +**Keep gating on liveness and accept the window.** Costs nothing and keeps the wait +fast. Rejected because the window is exactly where the suite's first request lands; +the resulting failures are attributed to the tests rather than to the gate, which +is the worst property a failure can have. + +**Keep the probe URL hardcoded and correct it when it drifts.** Simple to read at +the call site. Rejected because the drift is silent in the dangerous direction: the +gate keeps passing against a stale endpoint, and the model change that caused it is +elsewhere. + +**Branch the probe on the service name for the plain-HTTP instance.** Localises the +exception to one conditional. Rejected because it moves a property of the service +into the prober, so every future exception costs another branch and the prober +accumulates knowledge the model already holds. + +**Adopt a neighbouring wait's retry budget.** Removes the need to measure. Rejected +because a budget without a derivation is indistinguishable from a guess, and cannot +be re-evaluated when the boot profile changes. + +**Size the budget from an uncontended local run.** Cheap and green. Rejected +because it measures the wrong machine: the budget must survive a loaded CI runner, +and a fast local observation provides no evidence about that. + +The rejected options share one shape: each keeps a fact in the gate that the model +already owns, or keeps a number whose origin cannot be reconstructed. + +== References + +* link:0025-The_whole_server-TLS_surface_is_neutral_in_gatewayyaml_and_bound_by_exactly_two_seams.adoc[ADR-0025] -- the management-interface TLS posture the scheme label expresses per service +* link:../development/integration-test-topology.adoc[Integration-Test Topology] -- carries the readiness contract, the measured budget, and the per-instance port map diff --git a/doc/development/integration-test-topology.adoc b/doc/development/integration-test-topology.adoc index fc45712c..7f84093a 100644 --- a/doc/development/integration-test-topology.adoc +++ b/doc/development/integration-test-topology.adoc @@ -11,7 +11,7 @@ reverse-engineering `integration-tests/docker-compose.yml`. [IMPORTANT] ==== This is the *integration-test* topology. It is not a production deployment and not a recommended -one. Toxiproxy, go-httpbin, grpc-echo, the passthrough backend and the four variant gateway +one. Toxiproxy, go-httpbin, grpc-echo, the passthrough backend and the five variant gateway instances exist to make specific tests possible. Production and Kubernetes topologies are defined by PLAN-27 and are deliberately not drawn here or anywhere else yet. ==== @@ -57,10 +57,10 @@ link:tls-edge.adoc[TLS Edge -- Front Listener and the Accept-Time SNI Split]. === The collapsed variant group -Four further gateway instances run the *same* `api-sheriff:distroless` image and differ only by an -overlaid `gateway.yaml` and their published ports. Drawing all five in full would bury the -affordances the diagram exists to show, so they are collapsed into one annotated group that still -names every instance and every port pair: +Five further gateway instances run the *same* `api-sheriff:distroless` image and differ only by an +overlaid `gateway.yaml` -- or, for the last one, by a single environment variable -- and their +published ports. Drawing all six in full would bury the affordances the diagram exists to show, so +they are collapsed into one annotated group that still names every instance and every port pair: [cols="2,1,1,3"] |=== @@ -89,6 +89,13 @@ names every instance and every port pair: | 19004 | The admission budget is process-wide; this instance shrinks the caps to single digits so permit exhaustion is reachable in a handful of connections. + +| `api-sheriff-plain-mgmt` +| 10448 +| 19005 +| The one instance whose *management* interface serves plain HTTP, proving the supported downgrade + path. It overlays no `gateway.yaml` at all -- the opt-out is a single environment variable -- so + the management scheme is the only variable under test. See <>. |=== === Mounted material @@ -99,6 +106,119 @@ and `assets/`. The variant instances additionally overlay a single file -- `sheriff-config-/gateway.yaml` on top of the shared `sheriff-config/` mount -- which is the whole mechanism by which they differ. +[#readiness-contract] +== The Readiness Contract + +Bringing the stack up is not the same as being able to drive it. This section records what the +bring-up script waits for, what that wait actually proves, and where its retry budget came from. + +=== The gate asserts readiness, not liveness + +`integration-tests/scripts/start-integration-container.sh` waits on `/q/health/ready` for every +discovered gateway instance, in the single Compose-model-derived wait loop. It is one loop for all +six instances: the service set, each instance's published management port, and the scheme its +management interface speaks are read from the resolved Compose model, so adding, removing or +renumbering an instance needs no edit to the script. + +`/q/health/live` is deliberately *not* the gate. Liveness answers as soon as the process is up, +which is strictly earlier than the point at which the suite may drive the instance -- an instance +that is live but not ready would serve the suite's first request against a validator that is not +yet bound. + +The Keycloak wait is derived the same way, from the `keycloak` service's own +`de.cuioss.sheriff.management-scheme` label and its published management port, so no host and no +port number is restated in the script. + +=== What `GatewayReadinessCheck` attests -- and what it does not + +The probe reports two facts: the configuration document is bound (which proves the boot-time +load-and-validate pipeline succeeded, since an invalid configuration aborts startup), and -- when a +`token_validation` block is configured -- the `@GatewayValidator`-qualified `TokenValidator` +resolves. + +Be precise about the second one, because it reads stronger than it is. It is a *boot-time +constructibility* fact, not a live JWKS signal. The validator is forced into existence at startup +and the probe afterwards reads the cached instance, so it performs no JWKS fetch per probe. + +The consequence is worth stating plainly, because it is the thing most likely to be assumed wrong: + +[IMPORTANT] +==== +An IdP whose JWKS endpoint becomes unreachable *after* boot, a stalled key rotation, and an expired +signing key do *not* take readiness `DOWN`. They surface as `401`s on the request path and as fetch +errors in the container's stdout. Diagnose that class of failure from the container log, never from +a green readiness probe. +==== + +That narrowness is a known and accepted consequence of the exclusion decision recorded in +link:../adr/0027-The_token-validation_extensions_unqualified_health_probes_are_excluded_not_accommodated.adoc[ADR-0027]: +the token-validation extension's two unqualified health probes were removed from the bean set +rather than configured into a green state, because they observe an issuer namespace this gateway +never populates. The live per-issuer loader signal they would have carried is an open gap in the +gateway's own probe, not something the exclusion removed. + +=== Where the retry budget came from + +The gateway wait budget is `GATEWAY_READY_ATTEMPTS`, declared once in the script and read by the +loop bound, the last-attempt comparison and the progress echo -- one number rather than three +literals that can drift apart. + +Its value was measured, not chosen. A sub-second prober (100 ms polling, started *before* +`docker compose up -d` so that it observes the live-to-ready transition rather than inferring it +from a loop that only begins once both are already true) ran against all six gateway instances +brought up concurrently with Keycloak, under CPU contention -- 24 busy workers on 16 cores: + +[cols="2,1,1,1"] +|=== +| Instance | First live | First ready | Delta + +| `api-sheriff` | 8.80s | 8.80s | 0.00s +| `api-sheriff-cookie` | 9.14s | 9.14s | 0.00s +| `api-sheriff-cookie-2` | 8.82s | 8.82s | 0.00s +| `api-sheriff-mtls` | 8.16s | 8.16s | 0.00s +| `api-sheriff-plain-mgmt` | 8.94s | 8.94s | 0.00s +| `api-sheriff-ws-admission`| 8.95s | 8.95s | 0.00s +|=== + +The live-to-ready delta is *0.00s on every instance* -- below the prober's own 100 ms resolution. +That is not luck; it is what the previous section describes. Because the `jwks` datum is settled at +boot and cached, readiness flips at the same moment liveness does. Switching the gate from liveness +to readiness therefore costs no additional wait, and the budget needed no increase to absorb it. + +*The budget is 30 attempts at 1 s, retained on that evidence.* The headroom is stated as a factor +against the worst observed figure: the worst time-to-ready was 9.14s measured from the +`compose up -d` call, and the wait loop's own clock starts strictly later than that -- after compose +returns and after the go-httpbin wait -- so 30 attempts carry better than 3x headroom against that +stricter clock and roughly 30x against the loop's own. The factor is deliberately generous rather +than tight, because the budget is consumed only on failure: a large budget costs a healthy run +nothing and costs a broken run only the time until it gives up. It is *not* to be shrunk toward the +observed figures -- CI runners are slower than the machine measured here, and the measurement bounds +the live-to-ready delta, not the absolute container start. + +=== The demo bring-up gates the same way + +Everything above describes `integration-tests/scripts/start-integration-container.sh`. The parallel +developer bring-up, `demo-client/scripts/start-dev-environment.sh`, gates its gateway wait on +`/q/health/ready` against the same single-number retry budget, so the two scripts agree on the +readiness contract and the measured evidence above justifies both. + +=== `-plain-mgmt` serves plain HTTP by design + +`api-sheriff-plain-mgmt` is the one instance whose management interface is plain HTTP. That is the +supported downgrade path under +link:../adr/0025-The_whole_server-TLS_surface_is_neutral_in_gatewayyaml_and_bound_by_exactly_two_seams.adoc[ADR-0025], +selected by pointing the instance at a TLS bucket that carries no key material. *It is not a defect +to fix.* + +It is handled structurally rather than as a special case: each service declares the scheme its +management interface speaks in a `de.cuioss.sheriff.management-scheme` label, and the wait loop adds +`-k` only when that label says `https`. The scheme difference is therefore data, not a branch on a +service name -- which is what lets one loop cover all six instances. + +The corollary is a standing assertion: this instance must be probed over `http://` with *no* `-k`. +If it ever needs `-k`, the plain-HTTP opt-out has silently stopped working, and that is the bug -- +not the probe. + == Keeping the Diagram Honest The diagram is hand-authored SVG, so it does not regenerate itself and CI does not check it. Two @@ -110,3 +230,20 @@ consequences: is commonly got wrong -- is in the link:diagram-type-deployment.md[deployment diagram-type standard]. Rendered PNGs are verification artifacts and are never committed. + +=== Diagram check for the readiness change + +The readiness change recorded above was checked against the diagram, and the check is stated here +rather than left implicit. That change altered no service and no port -- it changed which health +path the wait loop probes, hoisted the retry budget into a named constant, and derived the Keycloak +probe URL from the Compose model. None of those is a fact the diagram depicts, so the diagram is not +made stale by it and is deliberately not redrawn. + +The check did, however, surface a *pre-existing* gap that predates this change: the collapsed +variant group in the SVG is labelled `variant instances (4)` and names only `api-sheriff-mtls`, +`api-sheriff-cookie`, `api-sheriff-cookie-2` and `api-sheriff-ws-admission`. It omits +`api-sheriff-plain-mgmt` -- the same omission the table above previously carried and now fixes. The +diagram's `` accessibility text repeats the omission. Closing it means adding the fifth member +plus its `10448 · 19005` port pair, correcting the count label and the ``, and re-rendering +against both themes; that is diagram work rather than documentation work and is deliberately left to +a follow-up rather than folded in here. diff --git a/integration-tests/scripts/start-integration-container.sh b/integration-tests/scripts/start-integration-container.sh index 249dc529..1011a0d4 100755 --- a/integration-tests/scripts/start-integration-container.sh +++ b/integration-tests/scripts/start-integration-container.sh @@ -73,19 +73,120 @@ fi # Set LOG_TARGET_DIR to a dedicated log subdirectory for Quarkus file logging. -# The api-sheriff native/distroless container runs as uid 1001, but this host -# directory is created by the (differently-numbered) Maven user, so the bind-mounted +# The api-sheriff container runs as the distroless 'nonroot' user (uid 65532), but this +# host directory is created by the (differently-numbered) Maven user, so the bind-mounted # /logs is not writable by the container and the file log sink fails with # "FileNotFoundException: /logs/quarkus.log (Permission denied)". Grant world write on -# a dedicated 'quarkus-logs' subdirectory only — least privilege — so uid 1001 can write +# a dedicated 'quarkus-logs' subdirectory only — least privilege — so the container can write # quarkus.log there without making the entire build target tree world-writable (ephemeral # test output — the container keeps its no-new-privileges / cap_drop / read_only posture). +# +# Mode 1777, not 0777: the sticky bit is what keeps that world write from also being a +# world DELETE. Without it any local account on a shared CI runner or developer host can +# remove or replace quarkus.log — the gateway's own log for the run, and the first thing +# read to diagnose a failure — so that evidence is locally tamperable. The sticky bit +# restricts unlink and rename to the file's owner and the directory's owner, costing the +# container nothing: it still creates and rotates the files it owns, and 'mvn clean' runs +# as the build user that OWNS this directory. LOG_TARGET_ROOT="${LOG_TARGET_DIR:-${PROJECT_DIR}/target}" export LOG_TARGET_DIR="${LOG_TARGET_ROOT}/quarkus-logs" mkdir -p "${LOG_TARGET_DIR}" -chmod 0777 "${LOG_TARGET_DIR}" +chmod 1777 "${LOG_TARGET_DIR}" echo "📁 Quarkus logs will be written to: ${LOG_TARGET_DIR}/quarkus.log" +# Discover every host-side probe target from the resolved Compose model, BEFORE anything is started. +# +# The service set, each service's published management port, and the scheme its management interface +# speaks are all DERIVED from that model — none of them is restated here. An earlier version +# hand-maintained a "service:port" list plus a separate block for the plain-HTTP instance, under a +# comment instructing the reader to keep the list in lockstep with docker-compose.yml. A hardcoded +# list that must mirror a set defined elsewhere is a defect unless it is derived from that source, so +# it is derived: adding, removing or renumbering an api-sheriff* service needs no edit here, and +# neither does moving Keycloak's published management port. +# +# The scheme comes from each service's de.cuioss.sheriff.management-scheme label rather than from its +# name, and that is what collapses the plain-management special case into a single readiness loop. +# That instance is probed over http:// with NO -k: if it ever needs -k, the plain-management opt-out +# has silently stopped working and THAT is the bug, not the probe. +# +# Keycloak carries the same label for the same reason, so its wait derives its whole probe URL here +# too rather than restating a scheme and a port the model already owns. +# +# The block runs BEFORE the first `compose up` on purpose: a model this script cannot read is a +# failure worth having in two seconds rather than after Keycloak has booted. +echo "⏳ Discovering probe targets from the Compose model..." +if ! DISCOVERED_TARGETS="$($COMPOSE_CMD config --format json | python3 -c ' +import json +import sys + +SCHEME_LABEL = "de.cuioss.sheriff.management-scheme" +MANAGEMENT_CONTAINER_PORT = "9000" +IDP_SERVICE = "keycloak" +GATEWAY_PREFIX = "api-sheriff" + +try: + model = json.load(sys.stdin) +except ValueError as exc: + sys.exit("could not parse the resolved Compose model as JSON (%s). This script needs a Compose " + "version supporting `config --format json`." % exc) + +all_services = model.get("services") or {} +selected = {name: spec for name, spec in all_services.items() if name.startswith(GATEWAY_PREFIX)} +if not selected: + sys.exit("no api-sheriff* services found in the resolved Compose model — refusing to run with a " + "readiness gate that would probe nothing") +if IDP_SERVICE not in all_services: + sys.exit("no %s service found in the resolved Compose model — refusing to run with an IdP wait " + "that would probe nothing" % IDP_SERVICE) +selected[IDP_SERVICE] = all_services[IDP_SERVICE] + +rows = [] +problems = [] +for name in sorted(selected): + spec = selected[name] + scheme = (spec.get("labels") or {}).get(SCHEME_LABEL) + published = [port.get("published") for port in (spec.get("ports") or []) + if str(port.get("target")) == MANAGEMENT_CONTAINER_PORT and port.get("published")] + usable = True + if scheme not in ("http", "https"): + problems.append("%s: missing or invalid %s label (got %r)" % (name, SCHEME_LABEL, scheme)) + usable = False + if len(published) != 1: + problems.append("%s: expected exactly one host port published against container port %s, " + "found %r" % (name, MANAGEMENT_CONTAINER_PORT, published)) + usable = False + if usable: + rows.append("%s %s %s" % (name, scheme, published[0])) + +if problems: + sys.exit("probe-target discovery failed:\n " + "\n ".join(problems)) + +sys.stdout.write("\n".join(rows) + "\n") +')"; then + echo "❌ Could not derive the probe targets from docker-compose.yml (see the error above)" + exit 1 +fi + +# Split the derived rows by role, mirroring demo-client/scripts/start-dev-environment.sh's +# IDP_TARGET / GATEWAY_TARGETS split. The IdP row drives the Keycloak wait and the Keycloak banner +# entry; the api-sheriff* rows drive the gateway readiness loop and the Application URLs banner. +KEYCLOAK_TARGET="$(printf '%s\n' "$DISCOVERED_TARGETS" | grep "^keycloak ")" +READINESS_TARGETS="$(printf '%s\n' "$DISCOVERED_TARGETS" | grep -v "^keycloak ")" +read -r _ KC_MGMT_SCHEME KC_MGMT_PORT <<< "$KEYCLOAK_TARGET" +KEYCLOAK_HEALTH_URL="${KC_MGMT_SCHEME}://localhost:${KC_MGMT_PORT}/health/ready" + +# -f matters here exactly as it does on the gateway probe below: /health/ready answers 503 while +# Keycloak is still starting, and without -f curl exits 0 on that 503 — so the wait would clear as +# soon as the port ACCEPTED rather than when Keycloak was actually ready, which is the very race the +# comment below says this gate exists to remove. +KEYCLOAK_PROBE_OPTS=(-sf --connect-timeout 2 --max-time 5) +if [[ "$KC_MGMT_SCHEME" == "https" ]]; then + # -k is load-bearing on an HTTPS management interface: it serves a self-signed localhost bundle, + # and without it curl fails certificate validation and this wait degrades into a silent + # 120-attempt timeout against a perfectly healthy container. + KEYCLOAK_PROBE_OPTS+=(-k) +fi + # Bring up Keycloak FIRST and wait until it is READY before starting the gateway. The api-sheriff # native app eagerly loads the Keycloak issuers' JWKS at boot; if it starts before Keycloak can # answer, that initial load fails (ConnectException) and — with a long background-refresh interval — @@ -98,14 +199,14 @@ echo "🐳 Starting Keycloak first (the Quarkus $MODE gateway starts only after (cd "${PROJECT_DIR}" && $COMPOSE_CMD up -d keycloak) # Wait for Keycloak to be ready first -echo "⏳ Waiting for Keycloak to be ready..." +echo "⏳ Waiting for Keycloak to be ready (management ${KC_MGMT_SCHEME} on ${KC_MGMT_PORT})..." for i in {1..120}; do - if curl -k -s --connect-timeout 2 --max-time 5 https://localhost:1090/health/ready > /dev/null 2>&1; then + if curl "${KEYCLOAK_PROBE_OPTS[@]}" "${KEYCLOAK_HEALTH_URL}" > /dev/null 2>&1; then echo "✅ Keycloak is ready!" break fi if [ "$i" -eq 120 ]; then - echo "❌ Keycloak failed to become ready within 120 attempts" + echo "❌ Keycloak did not answer ${KEYCLOAK_HEALTH_URL} within 120 attempts" echo "Check logs with: ${COMPOSE_BASE} logs keycloak" exit 1 fi @@ -151,72 +252,40 @@ if [[ "${BENCHMARK_MODE:-false}" == "true" ]]; then done fi -# Wait for every gateway instance to become ready. +# Wait for every gateway instance to become READY — not merely live. # -# The instance set, each instance's published management port, and the scheme its management -# interface speaks are all DERIVED from the resolved Compose model — none of them is restated here. -# An earlier version hand-maintained a "service:port" list plus a separate block for the plain-HTTP -# instance, under a comment instructing the reader to keep the list in lockstep with -# docker-compose.yml. A hardcoded list that must mirror a set defined elsewhere is a defect unless it -# is derived from that source, so it is derived: adding, removing or renumbering an api-sheriff* -# service needs no edit in this file. -# -# The scheme comes from each service's de.cuioss.sheriff.management-scheme label rather than from its -# name, and that is what collapses the plain-management special case into this one loop. That -# instance is probed over http:// with NO -k: if it ever needs -k, the plain-management opt-out has -# silently stopped working and THAT is the bug, not the probe. +# The probe is /q/health/ready, which on this gateway means GatewayReadinessCheck reported UP: +# the configuration document is bound and, when a token_validation block is configured, the +# @GatewayValidator-qualified TokenValidator resolved. /q/health/live answers as soon as the +# process is up, which is strictly earlier than the point at which the suite can drive it — an +# instance that is live but not ready serves the first IT request against an unbound validator. # # Every instance must be waited on, not just the primary one: the suites drive the TLS ports # directly — MtlsHandshakeIT, the Bff*Cookie*IT suites (BffCookieStatelessnessIT drives BOTH cookie # instances in one test), WebSocketProxyIT's relay-exhaustion regression against the low-admission # instance — so an unwaited instance is a race that surfaces as a connection refusal in the IT phase # rather than as a start-up failure here. -echo "⏳ Discovering gateway readiness targets from the Compose model..." -if ! READINESS_TARGETS="$($COMPOSE_CMD config --format json | python3 -c ' -import json -import sys - -SCHEME_LABEL = "de.cuioss.sheriff.management-scheme" -MANAGEMENT_CONTAINER_PORT = "9000" - -try: - model = json.load(sys.stdin) -except ValueError as exc: - sys.exit("could not parse the resolved Compose model as JSON (%s). This script needs a Compose " - "version supporting `config --format json`." % exc) - -services = {name: spec for name, spec in (model.get("services") or {}).items() - if name.startswith("api-sheriff")} -if not services: - sys.exit("no api-sheriff* services found in the resolved Compose model — refusing to run with a " - "readiness gate that would probe nothing") - -rows = [] -problems = [] -for name in sorted(services): - spec = services[name] - scheme = (spec.get("labels") or {}).get(SCHEME_LABEL) - published = [port.get("published") for port in (spec.get("ports") or []) - if str(port.get("target")) == MANAGEMENT_CONTAINER_PORT and port.get("published")] - usable = True - if scheme not in ("http", "https"): - problems.append("%s: missing or invalid %s label (got %r)" % (name, SCHEME_LABEL, scheme)) - usable = False - if len(published) != 1: - problems.append("%s: expected exactly one host port published against container port %s, " - "found %r" % (name, MANAGEMENT_CONTAINER_PORT, published)) - usable = False - if usable: - rows.append("%s %s %s" % (name, scheme, published[0])) - -if problems: - sys.exit("gateway readiness discovery failed:\n " + "\n ".join(problems)) - -sys.stdout.write("\n".join(rows) + "\n") -')"; then - echo "❌ Could not derive the gateway readiness targets from docker-compose.yml (see the error above)" - exit 1 -fi +# +# READINESS_TARGETS is the api-sheriff*-only subset of the rows discovered before the bring-up. +# +# The retry budget is ONE number, declared once and read by all three of the loop bound, the +# last-attempt comparison and the progress echo. It used to be three literal 30s that could drift +# apart; a re-size now touches a single line. +# +# The value is measured, not chosen by feel. A sub-second prober run against all six instances under +# CPU contention put the live-to-ready delta at 0.00s on every one of them — which is not luck but +# what GatewayReadinessCheck means: its `jwks` datum is a BOOT-TIME constructibility fact (ADR-0027), +# forced into existence by TokenValidatorProducer.onStartup and cached thereafter, so readiness flips +# at the same moment liveness does and this gate costs no additional wait over the liveness probe it +# replaced. +# +# The per-instance figures and the headroom argument have a single home — +# doc/development/integration-test-topology.adoc, "Where the retry budget came from". Read the +# numbers there rather than restating them here, where they would drift. +# +# 30 attempts is retained on that evidence, and is consumed only on failure, so the headroom is free. +# Do not shrink it toward the observed times: CI runners are slower than the machine measured there. +GATEWAY_READY_ATTEMPTS=30 echo "⏳ Waiting for the discovered gateway instances to be ready..." START_TIME=$(date +%s) @@ -230,20 +299,20 @@ while read -r GATEWAY_SERVICE GATEWAY_MGMT_SCHEME GATEWAY_MGMT_PORT; do if [[ "$GATEWAY_MGMT_SCHEME" == "https" ]]; then # -k is load-bearing on the HTTPS instances: their management interface serves a self-signed # localhost bundle, and without it curl fails certificate validation and this wait degrades - # into a silent 30-attempt timeout against a perfectly healthy container. + # into a silent full-budget timeout against a perfectly healthy container. GATEWAY_PROBE_OPTS+=(-k) GATEWAY_DIAG_OPTS+=(-k) fi GATEWAY_MGMT_URL="${GATEWAY_MGMT_SCHEME}://localhost:${GATEWAY_MGMT_PORT}" echo "⏳ Waiting for ${GATEWAY_SERVICE} (management ${GATEWAY_MGMT_SCHEME} on ${GATEWAY_MGMT_PORT})..." - for i in {1..30}; do - if curl "${GATEWAY_PROBE_OPTS[@]}" "${GATEWAY_MGMT_URL}/q/health/live" > /dev/null 2>&1; then + for ((i = 1; i <= GATEWAY_READY_ATTEMPTS; i++)); do + if curl "${GATEWAY_PROBE_OPTS[@]}" "${GATEWAY_MGMT_URL}/q/health/ready" > /dev/null 2>&1; then echo "✅ ${GATEWAY_SERVICE} gateway instance is ready!" break fi - if [ "$i" -eq 30 ]; then - echo "❌ ${GATEWAY_SERVICE} gateway instance failed to start within 30 attempts" + if [ "$i" -eq "$GATEWAY_READY_ATTEMPTS" ]; then + echo "❌ ${GATEWAY_SERVICE} gateway instance failed to start within ${GATEWAY_READY_ATTEMPTS} attempts" # Capture the container log + health payload so a startup failure is diagnosable from CI # artifacts (uploaded via the failsafe-reports folder). DIAG_DIR="target/failsafe-reports" @@ -255,7 +324,7 @@ while read -r GATEWAY_SERVICE GATEWAY_MGMT_SCHEME GATEWAY_MGMT_PORT; do echo "" exit 1 fi - echo "⏳ Waiting for ${GATEWAY_SERVICE}... (attempt $i/30)" + echo "⏳ Waiting for ${GATEWAY_SERVICE}... (attempt $i/${GATEWAY_READY_ATTEMPTS})" sleep 1 done done <<< "$READINESS_TARGETS" @@ -292,7 +361,9 @@ done <<< "$READINESS_TARGETS" echo " 🔑 Keycloak: https://localhost:1443/auth" echo "" echo "🧪 Quick test commands (an https:// management port serves a self-signed cert — -k is required there):" -echo " curl -k https://localhost:1090/health/ready" +# Printed from the SAME derived Keycloak row the wait above probed, so this cannot drift from what +# docker-compose.yml publishes. +echo " curl ${KEYCLOAK_PROBE_OPTS[*]} ${KEYCLOAK_HEALTH_URL}" echo "" echo "🛑 To stop: ./scripts/stop-integration-container.sh" echo "📋 To view logs: ${COMPOSE_BASE} logs -f" diff --git a/integration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/ManagementPlainHttpActivationWiringTest.java b/integration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/ManagementPlainHttpActivationWiringTest.java index 94c319ea..9646d71e 100644 --- a/integration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/ManagementPlainHttpActivationWiringTest.java +++ b/integration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/ManagementPlainHttpActivationWiringTest.java @@ -100,8 +100,9 @@ void optOutInstanceCarriesNoManagementCertificate() throws Exception { @DisplayName("the opt-out instance publishes management port 19005") void optOutInstancePublishesTheDedicatedManagementPort() throws Exception { assertTrue(publishedPorts(OPT_OUT_SERVICE).stream().anyMatch(p -> p.startsWith("19005:")), - "the opt-out instance must publish 19005 — it is the port the dedicated http:// " - + "readiness gate and ManagementPlainHttpOptOutIT both address"); + "the opt-out instance must publish 19005 — it is the port the Compose-derived " + + "readiness gate probes over http:// and the port ManagementPlainHttpOptOutIT " + + "addresses"); } @Test