diff --git a/fleetd/src/test/java/dev/ltms/fleet/config/FleetConfigTest.java b/fleetd/src/test/java/dev/ltms/fleet/config/FleetConfigTest.java index 2ba37ec..bf07edf 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/config/FleetConfigTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/config/FleetConfigTest.java @@ -6,11 +6,18 @@ import dev.ltms.fleet.peer.MemberRole; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import java.lang.reflect.ParameterizedType; +import java.lang.reflect.RecordComponent; +import java.lang.reflect.Type; import java.nio.file.Files; import java.nio.file.Path; +import java.util.ArrayList; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.TreeMap; +import java.util.regex.Matcher; import java.util.regex.Pattern; import static org.junit.jupiter.api.Assertions.*; @@ -1366,9 +1373,18 @@ class FleetConfigTest { } /** - * Every optional knob the example documents must bind under the exact spelling used there. - * Keep this list in step with {@code fleetd.example.yaml}: a rename that updates the record - * but not the example (or vice versa) fails here instead of silently no-op'ing in production. + * A spot check that the knobs listed below bind under the exact spelling the example uses — + * it asserts real VALUES arrive in the record, which no name-matching guard can do. + * + *

This is NOT a coverage guard, and must not be read as one (fleetd #113). The list + * inside it is hand-written, so it only ever covers what someone remembered to add. Coverage + * — "is every key the code reads documented, and does every documented key bind?" — comes + * from {@link #everyNestedConfigKeyIsDocumentedInTheExample} and + * {@link #everyLiveKeyInTheExampleBindsToARecordComponent}, both of which derive their key + * set from the record tree and therefore cannot drift. + * + *

Adding a knob here is optional. Leaving one out is not a coverage gap, because the two + * derived guards above already fail on an undocumented or unbindable key. */ @Test void everyOptionalKnobDocumentedInTheExampleBinds(@TempDir Path dir) throws Exception { @@ -1525,6 +1541,230 @@ class FleetConfigTest { return p.matcher(yaml).find(); } + /** + * The nested half of {@link #everyKnownTopLevelKeyIsDocumentedInTheExample} (fleetd #113). + * + *

That guard walks {@link FleetConfig#KNOWN_TOP_LEVEL_KEYS} and anchors its regex at + * column 0, so it sees ONLY top-level keys. Every nested key — {@code profiles..model}, + * {@code health.paneProbeIntervalSeconds} and a hundred others — is outside its scope, and + * neither its name nor its output says so. A green run then reads as "the example documents + * the schema" when most of the schema was never looked at. + * + *

This walks the record tree rather than a name list, so a key added to any nested record + * is covered the moment it compiles, with no edit here. That is the point: a hand-maintained + * second copy of a list always drifts from the thing it mirrors. + * + *

Scope, stated on purpose (fleetd #113 criterion 3 — every check reports what it + * did and did not look at): + *

+ */ + @Test + void everyNestedConfigKeyIsDocumentedInTheExample() throws Exception { + Path example = Path.of("fleetd.example.yaml"); + assertTrue(Files.exists(example), "fleetd.example.yaml must ship next to the pom"); + String text = Files.readString(example); + + Map pathByName = configKeyPaths(); + + // The denominator. An under-counting walk passes every subset check vacuously, which is + // the exact shape fleetd #113 collects — so the walk has to prove it descended at all. + // The floor is DERIVED, not a literal: the nested walk must find substantially more keys + // than the top-level set the old guard used, or it has not gone below the first level. + int topLevel = FleetConfig.KNOWN_TOP_LEVEL_KEYS.size(); + assertTrue(pathByName.size() > topLevel * 2, + "the record walk found " + pathByName.size() + " config key(s) against " + + topLevel + " top-level key(s) — it has stopped descending into the " + + "nested records, so this guard would pass vacuously. Fix the walk " + + "before trusting a green run."); + + List undocumented = pathByName.entrySet().stream() + .filter(e -> !keyDocumentedAnywhere(text, e.getKey())) + .map(Map.Entry::getValue) + .sorted() + .toList(); + + assertTrue(undocumented.isEmpty(), () -> "checked " + pathByName.size() + + " config key(s) that FleetConfig can bind; " + undocumented.size() + + " appear nowhere in fleetd.example.yaml: " + undocumented + + " — document each one there, commented out if optional. fleetd.yaml is " + + "gitignored, so the example is the only committed description of the schema."); + } + + /** + * The other direction: a LIVE key in the example that {@link FleetConfig} cannot bind. That is + * a key an operator would copy into {@code fleetd.yaml} expecting it to do something, where it + * would be silently ignored. + * + *

Scope, stated on purpose: only live (uncommented) keys are checked. Most of the + * example is commented-out prose, and that prose contains lines like {@code # mode: token} + * that are indistinguishable from keys by text alone. Parsing them would produce false + * failures, so they are deliberately out of scope — and saying so here is the point, rather + * than letting a reader assume the whole file was validated. + */ + @Test + void everyLiveKeyInTheExampleBindsToARecordComponent() throws Exception { + Path example = Path.of("fleetd.example.yaml"); + String text = Files.readString(example); + + List> paths = liveKeyPaths(text); + assertTrue(paths.size() >= 20, + "only " + paths.size() + " live key path(s) were parsed out of the example — the " + + "parser is not seeing the file, so this guard would pass vacuously."); + + List unbindable = paths.stream() + .filter(path -> !pathBinds(path)) + .map(path -> String.join(".", path)) + .distinct() + .sorted() + .toList(); + + assertTrue(unbindable.isEmpty(), () -> "checked " + paths.size() + + " live key path(s) in fleetd.example.yaml; " + unbindable.size() + + " bind to nothing in FleetConfig: " + unbindable + + " — an operator copying one of these into fleetd.yaml gets silence, not an error."); + } + + /** + * Every configuration key {@link FleetConfig} can bind, at every depth, as + * {@code name -> a dotted path to one place it appears}. Derived from the record components, + * so it cannot drift from the code. + */ + private static Map configKeyPaths() { + Map out = new TreeMap<>(); + collectConfigKeys(FleetConfig.class, "", new HashSet<>(), out); + return out; + } + + private static void collectConfigKeys(Class type, String prefix, Set seen, + Map out) { + if (!type.isRecord() || !seen.add(type.getName())) { + return; + } + for (RecordComponent rc : type.getRecordComponents()) { + String path = prefix.isEmpty() ? rc.getName() : prefix + "." + rc.getName(); + out.putIfAbsent(rc.getName(), path); + Class nested = rc.getType(); + if (nested.isRecord()) { + collectConfigKeys(nested, path, seen, out); + } else if (Map.class.isAssignableFrom(nested) || List.class.isAssignableFrom(nested)) { + Class element = elementRecord(rc); + if (element != null) { + String childPrefix = Map.class.isAssignableFrom(nested) + ? path + "." : path + "[]"; + collectConfigKeys(element, childPrefix, seen, out); + } + } + } + } + + /** The record type inside a {@code Map} or {@code List} component, else null. */ + private static Class elementRecord(RecordComponent rc) { + if (rc.getGenericType() instanceof ParameterizedType pt) { + Type[] args = pt.getActualTypeArguments(); + if (args.length > 0 && args[args.length - 1] instanceof Class c && c.isRecord()) { + return c; + } + } + return null; + } + + /** + * True when {@code key} is documented in the example, in either of the two conventions that + * file actually uses: + *

    + *
  1. as a YAML key at any indentation, live or commented out ({@code key:}); or
    2. + *
  2. in a prose block that describes a section's sub-keys, one per line, as + * {@code # key → what it does}.
    3. + *
+ * + *

The second form is not decoration. {@code broker.uri} is documented ONLY that way, on + * purpose: writing it out as a copy-pasteable {@code uri: amqp://user:pass@host} invites an + * operator to paste a password into a file, which is the very thing {@code uriEnv} exists to + * avoid. A guard that demanded the key form would push the file toward doing that. So this + * encodes the convention the example really uses rather than imposing a new one. + */ + private static boolean keyDocumentedAnywhere(String yaml, String key) { + String quoted = Pattern.quote(key); + Pattern asYamlKey = Pattern.compile("(?m)^\\s*(?:#\\s*)?" + quoted + ":"); + Pattern asProseEntry = Pattern.compile("(?m)^\\s*#\\s*" + quoted + "\\s+\u2192"); + return asYamlKey.matcher(yaml).find() || asProseEntry.matcher(yaml).find(); + } + + /** Every live (uncommented) key in {@code yaml}, as a path from the document root. */ + private static List> liveKeyPaths(String yaml) { + Pattern keyLine = Pattern.compile("^(\\s*)([A-Za-z][A-Za-z0-9_]*):(\\s.*)?$"); + List stack = new ArrayList<>(); + List indents = new ArrayList<>(); + List> paths = new ArrayList<>(); + for (String line : yaml.split("\n", -1)) { + if (line.isBlank() || line.stripLeading().startsWith("#")) { + continue; + } + Matcher m = keyLine.matcher(line); + if (!m.matches()) { + continue; + } + int indent = m.group(1).length(); + while (!indents.isEmpty() && indents.get(indents.size() - 1) >= indent) { + indents.remove(indents.size() - 1); + stack.remove(stack.size() - 1); + } + indents.add(indent); + stack.add(m.group(2)); + paths.add(List.copyOf(stack)); + } + return paths; + } + + /** True when a dotted YAML path resolves to something {@link FleetConfig} can bind. */ + private static boolean pathBinds(List path) { + Class type = FleetConfig.class; + boolean nextSegmentIsAFreeFormName = false; + for (int i = 0; i < path.size(); i++) { + if (nextSegmentIsAFreeFormName) { + nextSegmentIsAFreeFormName = false; + continue; + } + RecordComponent rc = componentNamed(type, path.get(i)); + if (rc == null) { + return false; + } + Class t = rc.getType(); + if (t.isRecord()) { + type = t; + } else if (Map.class.isAssignableFrom(t)) { + Class element = elementRecord(rc); + if (element == null) { + return true; // Map: its entries are data, not schema + } + type = element; + nextSegmentIsAFreeFormName = true; + } else { + // A scalar or a list of scalars: nothing may legitimately nest under it. + return i == path.size() - 1; + } + } + return true; + } + + private static RecordComponent componentNamed(Class type, String name) { + if (type == null || !type.isRecord()) { + return null; + } + for (RecordComponent rc : type.getRecordComponents()) { + if (rc.getName().equals(name)) { + return rc; + } + } + return null; + } + @Test void placementDefaultsToFixedForExistingConfigs(@TempDir Path dir) throws Exception { Path f = dir.resolve("no-placement.yaml");