diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 87cbc3d..6cffe51 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,6 +23,11 @@ jobs: run: cargo clippy --all-targets -- -D warnings - name: build run: cargo build --locked --verbose + # Includes tests/openapi_conformance.rs, the SHAPE gate: it loads the committed openapi.json + # and checks every serde type in src/client.rs against its schema (field set, required vs + # nullable, round trip, endpoint wiring). The spec-drift job below only compares a version + # STRING, so this is the check that fails when a struct drifts from the spec in the same + # commit. Keep `cargo test` in this workflow: it is the only thing running that gate. - name: test run: cargo test --locked --verbose @@ -52,8 +57,52 @@ jobs: - name: Drive every command against the real gateway run: scripts/integration.sh /tmp/busbar-dl/busbar target/release/busbar-admin + spec-mirror: + # THE SHAPE OF THE SPEC ITSELF, which nothing else in this repo checks. + # + # Three questions, three different gates, and it is worth being explicit about which is which: + # 1. "Does src/client.rs match the committed openapi.json?" -> tests/openapi_conformance.rs + # (in the build job). Rust vs its OWN copy of the spec. + # 2. "Is the committed spec's VERSION STRING current?" -> the spec-drift job below. + # 3. "Is the committed spec the same DOCUMENT as core's?" -> this job. Nothing answered it. + # + # Question 2 is not a substitute for question 3, and assuming it was is what shipped: a mirror + # can carry core's exact info.version and still be missing properties, because the version + # string is copied along with everything else and says nothing about what was left behind. So + # this job never looks at the version. It walks components.schemas property-by-property and + # paths method-by-method and names every schema and property that differs. + # + # And it FAILS CLOSED. The spec-drift job below prints ::warning:: and exits 0 when the GitHub + # API is unreachable, which means a rate-limited runner reports "no drift" without having + # looked. This job exits non-zero instead. An unknown is not a pass. + # + # REF: latest-release, not a branch. This client is built and integration-tested against the + # latest RELEASED busbar (see the integration job, which downloads exactly that), so the spec + # it commits must be the released engine's spec. Tracking a branch here would make the client + # document endpoints and fields no released busbar serves. + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: '3.11' + + # SELF-TEST FIRST, then the verdict. Fleet convention, and the whole reason this job exists: + # an assertion nobody has watched fail is indistinguishable from `exit 0`. This proves the + # gate goes RED on a missing schema, a missing property, an extra property, a changed + # required set, a changed enum, a changed type, and a core spec it could not fetch or parse. + - name: spec-mirror gate self-test (red before green) + run: python3 scripts/spec_mirror_gate.py --selftest + + - name: committed openapi.json is the same document as busbar core's + env: + GH_TOKEN: ${{ github.token }} + run: python3 scripts/spec_mirror_gate.py --mirror openapi.json --busbar-ref latest-release + spec-drift: # The admin client is hand-rolled against openapi.json (committed at the repo root). + # This job answers "is the committed spec CURRENT?" only. Whether src/client.rs still MATCHES + # that spec is a separate question, answered by tests/openapi_conformance.rs in the build job. # Fail when the latest busbar release ships a NEWER spec version than the one committed, # so drift is visible instead of silent. Degrades to a warning if the GitHub API is # unavailable or rate-limited (keeps the check non-flaky). diff --git a/README.md b/README.md index b91136c..60cc708 100644 --- a/README.md +++ b/README.md @@ -4,9 +4,15 @@ A human-facing CLI for the [busbar](https://github.com/GetBusbar) gateway's **ad (`/api/v1/admin`). It speaks the frozen v1 contract over HTTP/HTTPS with a thin, hand-rolled client (no OpenAPI generator), so it's small and easy to extend. -The contract it targets is committed at [`openapi.json`](openapi.json) (busbar **1.5.2**); +The contract it targets is committed at [`openapi.json`](openapi.json) (busbar **1.5.3**); CI compares that spec's version against the latest busbar release so drift is visible. +Because the client is hand-rolled, a version match is not a shape match: +[`tests/openapi_conformance.rs`](tests/openapi_conformance.rs) loads that same committed +`openapi.json` and checks every request/response type in `src/client.rs` against its schema, field +set, required/nullable-ness, and endpoint wiring. Renaming a Rust field, making a required field +optional, or resyncing a spec that grew a property all fail `cargo test`. + ## Install From source (published later on crates.io / as a GitHub release + Homebrew tap): diff --git a/scripts/__pycache__/spec_mirror_gate.cpython-314.pyc b/scripts/__pycache__/spec_mirror_gate.cpython-314.pyc new file mode 100644 index 0000000..acbc07e Binary files /dev/null and b/scripts/__pycache__/spec_mirror_gate.cpython-314.pyc differ diff --git a/scripts/__pycache__/spec_mirror_selftest.cpython-314.pyc b/scripts/__pycache__/spec_mirror_selftest.cpython-314.pyc new file mode 100644 index 0000000..2319660 Binary files /dev/null and b/scripts/__pycache__/spec_mirror_selftest.cpython-314.pyc differ diff --git a/scripts/spec_mirror_gate.py b/scripts/spec_mirror_gate.py new file mode 100755 index 0000000..486cd4d --- /dev/null +++ b/scripts/spec_mirror_gate.py @@ -0,0 +1,799 @@ +#!/usr/bin/env python3 +"""spec-mirror-gate: compare a MIRRORED openapi.json against busbar core's, STRUCTURALLY. + +busbar core generates crates/busbar/src/admin/v1/json/openapi.json and that document is the +source of truth. Other repos keep tracked copies of it (busbar-admin's ./openapi.json, the +marketing site's website/public/openapi.json). Those copies are the thing this gate guards. + +WHY THIS EXISTS. The pre-existing drift check compared `info.version` STRINGS. That check was +green on a mirror whose HookView was missing `phase`, `fires_at` and `groups`, because the stale +copy carried the same version string as core's current one. A gate that reports green about a +property it never looks at is worse than no gate: it converts an unknown into a false assurance. +So this gate never looks at the version. It walks components.schemas schema-by-schema and +property-by-property, and paths path-by-path and method-by-method, and every finding NAMES the +schema and the property (or the path and the method). "The specs differ" is not a finding. + +FAIL CLOSED. If core's spec cannot be fetched or cannot be parsed, that is exit code 2, a +FAILURE. Unknown is not green. There is no --skip-on-network-error and there must never be one. + +Exit codes: + 0 mirror matches core structurally + 1 structural drift found (each finding is printed, named) + 2 FATAL: core's spec could not be fetched or parsed, or the mirror could not be read + +Usage: + spec_mirror_gate.py --mirror openapi.json --busbar-ref dev + spec_mirror_gate.py --mirror website/public/openapi.json --core-spec /tmp/core.json + spec_mirror_gate.py --selftest +""" + +import argparse +import json +import os +import sys +import urllib.error +import urllib.request + +CORE_REPO = "GetBusbar/busbar" +CORE_SPEC_PATH = "crates/busbar/src/admin/v1/json/openapi.json" +DEFAULT_REF = "dev" + +# The keys type_sig() folds into one signature. A type-changed finding accounts for all of them, +# so the residual sweep must not report them again. +TYPE_KEYS = ( + "type", + "format", + "nullable", + "$ref", + "oneOf", + "anyOf", + "allOf", + "items", + "additionalProperties", +) + +HTTP_METHODS = ( + "get", + "put", + "post", + "delete", + "patch", + "head", + "options", + "trace", +) + + +class Fatal(Exception): + """Core's spec could not be obtained or understood. Never downgrade this to a skip.""" + + +# --------------------------------------------------------------------------- +# fetching core's spec +# --------------------------------------------------------------------------- + + +def _get(url, headers, timeout): + req = urllib.request.Request(url, headers=headers) + with urllib.request.urlopen(req, timeout=timeout) as resp: + return resp.read() + + +LATEST_RELEASE = "latest-release" + + +def resolve_ref(ref, timeout=30, opener=None): + """Turn the sentinel `latest-release` into a real tag, or pass a ref through unchanged. + + A repo that PUBLISHES a spec (the marketing site) or ships a client for the RELEASED engine + (busbar-admin) must track the released document, not whatever is on a branch. Resolving that + tag is itself a fetch, so it gets the same rule as every other fetch here: it cannot fail + quietly. A ref we could not resolve is FATAL. + """ + if ref != LATEST_RELEASE: + return ref + get = opener or _get + token = os.environ.get("GH_TOKEN") or os.environ.get("GITHUB_TOKEN") + headers = {"Accept": "application/vnd.github+json", "User-Agent": "spec-mirror-gate"} + if token: + headers["Authorization"] = "Bearer %s" % token + url = "https://api.github.com/repos/%s/releases/latest" % CORE_REPO + try: + payload = json.loads(get(url, headers, timeout)) + except Exception as exc: # noqa: BLE001 + raise Fatal( + "could not resolve %r: %s failed: %s.\n" + "The pre-existing spec-drift job treated exactly this as a warning and exited 0. " + "That is the fail-open this gate exists to remove." % (ref, url, exc) + ) + tag = payload.get("tag_name") + if not tag: + raise Fatal( + "could not resolve %r: %s returned no tag_name (rate limited?)" % (ref, url) + ) + return tag + + +def fetch_core_spec(ref, timeout=30, opener=None): + """Return core's openapi.json bytes at `ref`, or raise Fatal. + + Tries the GitHub contents API first when a token is present (works for private repos), + then raw.githubusercontent.com. Both failing is FATAL, not a skip. + """ + get = opener or _get + token = os.environ.get("GH_TOKEN") or os.environ.get("GITHUB_TOKEN") + attempts = [] + if token: + attempts.append( + ( + "https://api.github.com/repos/%s/contents/%s?ref=%s" + % (CORE_REPO, CORE_SPEC_PATH, ref), + { + "Authorization": "Bearer %s" % token, + "Accept": "application/vnd.github.raw", + "User-Agent": "spec-mirror-gate", + }, + ) + ) + attempts.append( + ( + "https://raw.githubusercontent.com/%s/%s/%s" + % (CORE_REPO, ref, CORE_SPEC_PATH), + {"User-Agent": "spec-mirror-gate"}, + ) + ) + + errors = [] + for url, headers in attempts: + try: + return get(url, headers, timeout) + except Exception as exc: # noqa: BLE001 - every failure mode is fatal here + errors.append("%s: %s" % (url, exc)) + raise Fatal( + "could not fetch core's %s at ref %r from %s:\n %s" + % (CORE_SPEC_PATH, ref, CORE_REPO, "\n ".join(errors)) + ) + + +def load_spec(raw, origin): + try: + doc = json.loads(raw) + except Exception as exc: # noqa: BLE001 + raise Fatal("%s is not parseable JSON: %s" % (origin, exc)) + if not isinstance(doc, dict): + raise Fatal("%s is not a JSON object" % origin) + if not isinstance(doc.get("components"), dict) or not isinstance( + doc["components"].get("schemas"), dict + ): + raise Fatal("%s has no components.schemas object" % origin) + if not isinstance(doc.get("paths"), dict): + raise Fatal("%s has no paths object" % origin) + return doc + + +# --------------------------------------------------------------------------- +# structural model +# --------------------------------------------------------------------------- + + +def type_sig(node): + """A compact, order-insensitive description of a schema node's TYPE.""" + if node is True: + return "any" + if node is False: + return "never" + if not isinstance(node, dict): + return "literal(%r)" % (node,) + parts = [] + if "$ref" in node: + parts.append("$ref=%s" % node["$ref"]) + t = node.get("type") + if t is not None: + parts.append( + "type=%s" % (",".join(sorted(str(x) for x in t)) if isinstance(t, list) else t) + ) + if "format" in node: + parts.append("format=%s" % node["format"]) + if node.get("nullable") is True: + parts.append("nullable=true") + for kw in ("oneOf", "anyOf", "allOf"): + sub = node.get(kw) + if isinstance(sub, list): + parts.append("%s=[%s]" % (kw, ",".join(sorted(type_sig(s) for s in sub)))) + items = node.get("items") + if isinstance(items, (dict, bool)): + parts.append("items(%s)" % type_sig(items)) + ap = node.get("additionalProperties") + if isinstance(ap, dict): + parts.append("additionalProperties(%s)" % type_sig(ap)) + elif ap is False: + parts.append("additionalProperties=false") + return " ".join(parts) if parts else "any" + + +def enum_of(node): + if isinstance(node, dict) and isinstance(node.get("enum"), list): + return [json.dumps(v, sort_keys=True) for v in node["enum"]] + return None + + +def props_of(node): + if isinstance(node, dict) and isinstance(node.get("properties"), dict): + return node["properties"] + return {} + + +def required_of(node): + if isinstance(node, dict) and isinstance(node.get("required"), list): + return set(str(x) for x in node["required"]) + return set() + + +def esc(token): + """JSON Pointer escaping (RFC 6901).""" + return str(token).replace("~", "~0").replace("/", "~1") + + +def collect_nodes(node, prefix, pointer, out): + """Flatten a schema into {dotted-path: (node, json-pointer)}. + + Two names for the same place: the dotted path (HookView.groups[]) is what a human reads in a + finding, the JSON pointer is what the residual sweep uses to suppress double-reporting. + """ + out[prefix] = (node, pointer) + if not isinstance(node, dict): + return + for name, sub in sorted(props_of(node).items()): + collect_nodes(sub, "%s.%s" % (prefix, name), + "%s/properties/%s" % (pointer, esc(name)), out) + items = node.get("items") + if isinstance(items, dict): + collect_nodes(items, "%s[]" % prefix, "%s/items" % pointer, out) + ap = node.get("additionalProperties") + if isinstance(ap, dict): + collect_nodes(ap, "%s{}" % prefix, "%s/additionalProperties" % pointer, out) + + +# --------------------------------------------------------------------------- +# comparison +# --------------------------------------------------------------------------- + + +class Finding(object): + def __init__(self, kind, subject, detail, covers=None): + self.kind = kind + self.subject = subject + self.detail = detail + # JSON pointer subtree this finding already accounts for, so the residual sweep does not + # report the same divergence a second time in a less readable form. + if covers is None: + self.covers = [] + elif isinstance(covers, str): + self.covers = [covers] + else: + self.covers = list(covers) + + def __str__(self): + return "%-22s %-46s %s" % (self.kind, self.subject, self.detail) + + +def _sorted_diff(a, b): + return sorted(a - b), sorted(b - a) + + +def compare_schemas(core, mirror, findings): + core_s = core["components"]["schemas"] + mirror_s = mirror["components"]["schemas"] + + missing, extra = _sorted_diff(set(core_s), set(mirror_s)) + for name in missing: + findings.append( + Finding( + "schema-missing", + name, + "core defines schema %r; the mirror does not have it at all" % name, + covers="/components/schemas/%s" % esc(name), + ) + ) + for name in extra: + findings.append( + Finding( + "schema-extra", + name, + "the mirror defines schema %r; core does not" % name, + covers="/components/schemas/%s" % esc(name), + ) + ) + + for name in sorted(set(core_s) & set(mirror_s)): + core_nodes = {} + mirror_nodes = {} + root = "/components/schemas/%s" % esc(name) + collect_nodes(core_s[name], name, root, core_nodes) + collect_nodes(mirror_s[name], name, root, mirror_nodes) + + for path in sorted(set(core_nodes) & set(mirror_nodes)): + cn, ptr = core_nodes[path] + mn = mirror_nodes[path][0] + + # properties + cp, mp = set(props_of(cn)), set(props_of(mn)) + gone, added = _sorted_diff(cp, mp) + for p in gone: + findings.append( + Finding( + "property-missing", + "%s.%s" % (path, p), + "schema %s is missing property %r that core has (core type: %s)" + % (path, p, type_sig(props_of(cn)[p])), + covers="%s/properties/%s" % (ptr, esc(p)), + ) + ) + for p in added: + findings.append( + Finding( + "property-extra", + "%s.%s" % (path, p), + "schema %s has property %r that core does not (mirror type: %s)" + % (path, p, type_sig(props_of(mn)[p])), + covers="%s/properties/%s" % (ptr, esc(p)), + ) + ) + + # required + cr, mr = required_of(cn), required_of(mn) + gone, added = _sorted_diff(cr, mr) + for p in gone: + findings.append( + Finding( + "required-missing", + "%s.%s" % (path, p), + "core marks %r required on schema %s; the mirror does not" + % (p, path), + covers="%s/required" % ptr, + ) + ) + for p in added: + findings.append( + Finding( + "required-extra", + "%s.%s" % (path, p), + "the mirror marks %r required on schema %s; core does not" + % (p, path), + covers="%s/required" % ptr, + ) + ) + + # enum + ce, me = enum_of(cn), enum_of(mn) + if ce is None and me is not None: + findings.append( + Finding( + "enum-added", + path, + "the mirror constrains %s to an enum (%s); core does not" + % (path, ", ".join(me)), + covers="%s/enum" % ptr, + ) + ) + elif ce is not None and me is None: + findings.append( + Finding( + "enum-dropped", + path, + "core constrains %s to an enum (%s); the mirror does not" + % (path, ", ".join(ce)), + covers="%s/enum" % ptr, + ) + ) + elif ce is not None and me is not None: + gone, added = _sorted_diff(set(ce), set(me)) + for v in gone: + findings.append( + Finding( + "enum-variant-missing", + path, + "enum %s is missing variant %s that core has" % (path, v), + covers="%s/enum" % ptr, + ) + ) + for v in added: + findings.append( + Finding( + "enum-variant-extra", + path, + "enum %s has variant %s that core does not" % (path, v), + covers="%s/enum" % ptr, + ) + ) + + # type + cts, mts = type_sig(cn), type_sig(mn) + if cts != mts: + findings.append( + Finding( + "type-changed", + path, + "%s is %s in core but %s in the mirror" % (path, cts, mts), + covers=["%s/%s" % (ptr, k) for k in TYPE_KEYS], + ) + ) + + +def _op_params(op): + out = {} + if isinstance(op, dict) and isinstance(op.get("parameters"), list): + for p in op["parameters"]: + if isinstance(p, dict) and "name" in p: + out["%s:%s" % (p.get("in", "?"), p["name"])] = bool(p.get("required")) + return out + + +def _op_body_sig(op): + if not isinstance(op, dict): + return None + body = op.get("requestBody") + if not isinstance(body, dict): + return None + content = body.get("content") + if not isinstance(content, dict): + return "requestBody(required=%s)" % bool(body.get("required")) + media = sorted(content) + sigs = [ + "%s=%s" % (m, type_sig((content[m] or {}).get("schema", {}))) for m in media + ] + return "requestBody(required=%s, %s)" % (bool(body.get("required")), "; ".join(sigs)) + + +def _op_responses(op): + out = {} + if isinstance(op, dict) and isinstance(op.get("responses"), dict): + for code, resp in op["responses"].items(): + content = (resp or {}).get("content") + if isinstance(content, dict): + out[str(code)] = "; ".join( + "%s=%s" % (m, type_sig((content[m] or {}).get("schema", {}))) + for m in sorted(content) + ) + else: + out[str(code)] = "no-content" + return out + + +def compare_paths(core, mirror, findings): + cp, mp = core["paths"], mirror["paths"] + missing, extra = _sorted_diff(set(cp), set(mp)) + for p in missing: + findings.append( + Finding( + "path-missing", + p, + "core serves path %s; the mirror does not document it" % p, + covers="/paths/%s" % esc(p), + ) + ) + for p in extra: + findings.append( + Finding( + "path-extra", + p, + "the mirror documents path %s; core does not serve it" % p, + covers="/paths/%s" % esc(p), + ) + ) + + for p in sorted(set(cp) & set(mp)): + c_ops = {m: cp[p][m] for m in HTTP_METHODS if isinstance(cp[p], dict) and m in cp[p]} + m_ops = {m: mp[p][m] for m in HTTP_METHODS if isinstance(mp[p], dict) and m in mp[p]} + gone, added = _sorted_diff(set(c_ops), set(m_ops)) + for m in gone: + findings.append( + Finding( + "method-missing", + "%s %s" % (m.upper(), p), + "core serves %s %s; the mirror does not document it" % (m.upper(), p), + covers="/paths/%s/%s" % (esc(p), m), + ) + ) + for m in added: + findings.append( + Finding( + "method-extra", + "%s %s" % (m.upper(), p), + "the mirror documents %s %s; core does not serve it" % (m.upper(), p), + covers="/paths/%s/%s" % (esc(p), m), + ) + ) + + for m in sorted(set(c_ops) & set(m_ops)): + subject = "%s %s" % (m.upper(), p) + optr = "/paths/%s/%s" % (esc(p), m) + c_par, m_par = _op_params(c_ops[m]), _op_params(m_ops[m]) + g, a = _sorted_diff(set(c_par), set(m_par)) + for name in g: + findings.append( + Finding( + "param-missing", + "%s %s" % (subject, name), + "core takes parameter %s on %s; the mirror does not" % (name, subject), + covers="%s/parameters" % optr, + ) + ) + for name in a: + findings.append( + Finding( + "param-extra", + "%s %s" % (subject, name), + "the mirror takes parameter %s on %s; core does not" + % (name, subject), + covers="%s/parameters" % optr, + ) + ) + for name in sorted(set(c_par) & set(m_par)): + if c_par[name] != m_par[name]: + findings.append( + Finding( + "param-required-changed", + "%s %s" % (subject, name), + "parameter %s on %s is required=%s in core but required=%s in the mirror" + % (name, subject, c_par[name], m_par[name]), + covers="%s/parameters" % optr, + ) + ) + + cb, mb = _op_body_sig(c_ops[m]), _op_body_sig(m_ops[m]) + if cb != mb: + findings.append( + Finding( + "request-body-changed", + subject, + "%s request body is %s in core but %s in the mirror" + % (subject, cb, mb), + covers="%s/requestBody" % optr, + ) + ) + + c_res, m_res = _op_responses(c_ops[m]), _op_responses(m_ops[m]) + g, a = _sorted_diff(set(c_res), set(m_res)) + for code in g: + findings.append( + Finding( + "response-missing", + "%s %s" % (subject, code), + "core documents response %s on %s; the mirror does not" + % (code, subject), + covers="%s/responses/%s" % (optr, esc(code)), + ) + ) + for code in a: + findings.append( + Finding( + "response-extra", + "%s %s" % (subject, code), + "the mirror documents response %s on %s; core does not" + % (code, subject), + covers="%s/responses/%s" % (optr, esc(code)), + ) + ) + for code in sorted(set(c_res) & set(m_res)): + if c_res[code] != m_res[code]: + findings.append( + Finding( + "response-changed", + "%s %s" % (subject, code), + "response %s on %s is %s in core but %s in the mirror" + % (code, subject, c_res[code], m_res[code]), + covers="%s/responses/%s" % (optr, esc(code)), + ) + ) + + +# --------------------------------------------------------------------------- +# residual sweep +# --------------------------------------------------------------------------- +# +# The structural walk above names the differences an API consumer can BREAK on. It is not, and +# should not be, a byte comparison. But a mirror is a COPY of a generated file, so anything the +# structural walk did not look at is still a difference someone has to know about, and staying +# green on it would reintroduce the exact defect this gate exists to kill: reporting green about +# something it never checked. So after the named walk, sweep the whole document and report every +# remaining divergence by JSON pointer, minus what the walk already accounted for. +# +# In practice this is what catches a stale `info.version` and a `description` that core rewrote, +# neither of which changes a wire shape but both of which mean the copy is not the original. + + +def pointer_diffs(core, mirror, prefix="", out=None): + """Every JSON pointer at which the two documents disagree.""" + if out is None: + out = [] + if isinstance(core, dict) and isinstance(mirror, dict): + for k in sorted(set(core) | set(mirror)): + ptr = "%s/%s" % (prefix, esc(k)) + if k not in mirror: + out.append((ptr, "present in core, absent from the mirror")) + elif k not in core: + out.append((ptr, "present in the mirror, absent from core")) + else: + pointer_diffs(core[k], mirror[k], ptr, out) + elif isinstance(core, list) and isinstance(mirror, list): + if len(core) != len(mirror): + out.append((prefix, "%d entries in core, %d in the mirror" + % (len(core), len(mirror)))) + else: + for i, (a, b) in enumerate(zip(core, mirror)): + pointer_diffs(a, b, "%s/%d" % (prefix, i), out) + elif core != mirror: + out.append((prefix, describe_scalar_diff(core, mirror))) + return out + + +def describe_scalar_diff(core, mirror): + """Say WHERE two scalars differ. Two long descriptions that share a prefix must not both + truncate to the same 120 characters and read as if the tool is reporting nothing.""" + if isinstance(core, str) and isinstance(mirror, str) and ( + len(core) > 100 or len(mirror) > 100 + ): + i = 0 + while i < min(len(core), len(mirror)) and core[i] == mirror[i]: + i += 1 + return ( + "text differs at character %d (core is %d chars, the mirror is %d); " + "from there core has %s and the mirror has %s" + % (i, len(core), len(mirror), + json.dumps(core[i:i + 90]), json.dumps(mirror[i:i + 90])) + ) + return "core has %s, the mirror has %s" % ( + json.dumps(core)[:160], + json.dumps(mirror)[:160], + ) + + +DOC_TEXT_KEYS = ("description", "summary", "title", "example", "examples", "externalDocs") + + +def compare_residual(core, mirror, findings): + covered = [] + for f in findings: + covered.extend(f.covers) + for ptr, detail in pointer_diffs(core, mirror): + if any(ptr == c or ptr.startswith(c + "/") for c in covered): + continue + last = ptr.rsplit("/", 1)[-1] if "/" in ptr else ptr + if last in DOC_TEXT_KEYS: + kind = "doc-text-changed" + elif ptr == "/info/version": + kind = "version-changed" + else: + kind = "unclassified-difference" + findings.append(Finding(kind, ptr, "%s: %s" % (ptr, detail))) + + +def compare(core, mirror): + findings = [] + compare_schemas(core, mirror, findings) + compare_paths(core, mirror, findings) + compare_residual(core, mirror, findings) + return findings + + +# --------------------------------------------------------------------------- +# reporting +# --------------------------------------------------------------------------- + + +def report(findings, mirror_label, core_label, stream=sys.stdout): + stream.write("spec-mirror-gate\n") + stream.write(" mirror: %s\n" % mirror_label) + stream.write(" core: %s\n" % core_label) + stream.write(" comparison: STRUCTURAL, schema-by-schema and property-by-property over\n" + " components.schemas, and path-by-path and method-by-method over\n" + " paths. The verdict never rests on info.version: a stale mirror\n" + " carrying core's own version string is exactly the false green\n" + " this gate replaces.\n\n") + if not findings: + stream.write("GREEN: the mirror is identical to core's spec.\n") + return 0 + by_kind = {} + for f in findings: + by_kind.setdefault(f.kind, []).append(f) + stream.write("RED: %d difference(s) between the mirror and core.\n\n" % len(findings)) + for kind in sorted(by_kind): + stream.write("[%s] %d\n" % (kind, len(by_kind[kind]))) + for f in by_kind[kind]: + stream.write(" - %s\n" % f.detail) + stream.write("\n") + stream.write( + "The mirror is a COPY of a generated document. Fix it by regenerating in core\n" + " (UPDATE_OPENAPI=1 cargo test -p busbar --features openapi-schema --locked \\\n" + " openapi_json_matches_committed_file)\n" + "and copying %s into this repo. Do not hand-edit the mirror,\n" + "and do not relax this gate.\n" % CORE_SPEC_PATH + ) + return 1 + + +# --------------------------------------------------------------------------- +# entry point +# --------------------------------------------------------------------------- + + +def build_parser(): + p = argparse.ArgumentParser( + prog="spec_mirror_gate.py", + description="Compare a mirrored openapi.json against busbar core's, structurally.", + ) + p.add_argument("--mirror", help="path to this repo's mirrored openapi.json") + p.add_argument( + "--busbar-ref", + default=DEFAULT_REF, + help="the busbar core git ref to compare against: a branch, a tag, a sha, or the " + "sentinel %r which resolves to the latest busbar release tag (default: %s, the engine " + "that ships next)" % (LATEST_RELEASE, DEFAULT_REF), + ) + p.add_argument( + "--core-spec", + help="compare against a LOCAL core spec file instead of fetching it (offline use)", + ) + p.add_argument( + "--selftest", + action="store_true", + help="prove this gate goes RED on every finding class it claims to detect", + ) + p.add_argument("--timeout", type=int, default=30, help="fetch timeout in seconds") + return p + + +def main(argv=None): + args = build_parser().parse_args(argv) + + if args.selftest: + import spec_mirror_selftest + + return spec_mirror_selftest.run(sys.stdout) + + if not args.mirror: + sys.stderr.write("FATAL: --mirror is required (or use --selftest)\n") + return 2 + + try: + try: + with open(args.mirror, "rb") as fh: + mirror_raw = fh.read() + except OSError as exc: + raise Fatal("could not read the mirror at %s: %s" % (args.mirror, exc)) + mirror = load_spec(mirror_raw, "the mirror at %s" % args.mirror) + + if args.core_spec: + try: + with open(args.core_spec, "rb") as fh: + core_raw = fh.read() + except OSError as exc: + raise Fatal("could not read --core-spec %s: %s" % (args.core_spec, exc)) + core_label = "%s (local file)" % args.core_spec + else: + ref = resolve_ref(args.busbar_ref, timeout=args.timeout) + core_raw = fetch_core_spec(ref, timeout=args.timeout) + core_label = "%s@%s:%s%s" % ( + CORE_REPO, + ref, + CORE_SPEC_PATH, + " (resolved from %s)" % LATEST_RELEASE + if ref != args.busbar_ref + else "", + ) + core = load_spec(core_raw, "core's spec (%s)" % core_label) + except Fatal as exc: + sys.stderr.write( + "FATAL: %s\n" + "This gate FAILS CLOSED. An unknown is not a pass: it is exactly the state the\n" + "old version-string check treated as green while the shapes had diverged.\n" % exc + ) + return 2 + + return report(compare(core, mirror), args.mirror, core_label) + + +if __name__ == "__main__": + sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) + sys.exit(main()) diff --git a/scripts/spec_mirror_selftest.py b/scripts/spec_mirror_selftest.py new file mode 100644 index 0000000..20e24e4 --- /dev/null +++ b/scripts/spec_mirror_selftest.py @@ -0,0 +1,345 @@ +#!/usr/bin/env python3 +"""Self-test for spec_mirror_gate: prove the gate goes RED on every class it claims to detect. + +Run it via the gate itself: `python3 spec_mirror_gate.py --selftest`. + +The point of this file is the ORDER the workflows use it in: self-test FIRST, then trust the +gate's verdict. A gate nobody has proved can fail is indistinguishable from `exit 0`, and that +is precisely how the version-string drift check stayed green over a mirror whose HookView had +lost `phase`, `fires_at` and `groups`. + +Every case below asserts three things, not one: + 1. the gate returns a NON-ZERO verdict on the broken pair, + 2. at least one finding is of the EXPECTED KIND, and + 3. that finding NAMES the schema/property (or path/method) at fault. +Point 3 is the one that matters. "The specs differ" would satisfy 1 and 2 and still be useless. + +The last case is the fail-closed case: core's spec unreachable must be FATAL (exit 2), never a +skip and never a pass. +""" + +import io +import json +import os +import tempfile + +import spec_mirror_gate as gate + + +# --------------------------------------------------------------------------- +# fixtures +# --------------------------------------------------------------------------- + + +def base_spec(): + """A tiny but structurally realistic spec, shaped like the real admin document.""" + return { + "openapi": "3.0.3", + "info": {"title": "selftest", "version": "9.9.9"}, + "paths": { + "/hooks": { + "get": { + "operationId": "list_hooks", + "parameters": [ + {"name": "limit", "in": "query", "required": False, + "schema": {"type": "integer"}} + ], + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "type": "array", + "items": {"$ref": "#/components/schemas/HookView"}, + } + } + } + } + }, + }, + "post": { + "operationId": "create_hook", + "requestBody": { + "required": True, + "content": { + "application/json": { + "schema": {"$ref": "#/components/schemas/HookView"} + } + }, + }, + "responses": {"201": {"content": {"application/json": { + "schema": {"$ref": "#/components/schemas/HookView"}}}}}, + }, + } + }, + "components": { + "schemas": { + "HookView": { + "type": "object", + "required": ["name", "kind"], + "properties": { + "name": {"type": "string"}, + "kind": {"type": "string"}, + "phase": {"$ref": "#/components/schemas/HookPhase"}, + "fires_at": {"type": "string", "format": "date-time", + "nullable": True}, + "groups": {"type": "array", "items": {"type": "string"}}, + }, + }, + "HookPhase": { + "type": "string", + "enum": ["pre", "post", "error"], + }, + "SecretView": { + "type": "object", + "properties": {"alias": {"type": "string"}}, + }, + } + }, + } + + +def clone(spec): + return json.loads(json.dumps(spec)) + + +# Each case: (label, mutate_mirror, expected_kind, must_name) +def cases(): + def drop_schema(m): + del m["components"]["schemas"]["SecretView"] + + def add_schema(m): + m["components"]["schemas"]["GhostView"] = {"type": "object", "properties": {}} + + def drop_props(m): + # The real defect, in miniature: the mirror's HookView is three properties behind core. + for p in ("phase", "fires_at", "groups"): + del m["components"]["schemas"]["HookView"]["properties"][p] + + def add_prop(m): + m["components"]["schemas"]["HookView"]["properties"]["legacy_stage"] = { + "type": "string" + } + + def change_required(m): + m["components"]["schemas"]["HookView"]["required"] = ["name"] + + def change_enum(m): + m["components"]["schemas"]["HookPhase"]["enum"] = ["pre", "post"] + + def change_type(m): + m["components"]["schemas"]["HookView"]["properties"]["groups"] = {"type": "string"} + + def drop_path(m): + del m["paths"]["/hooks"] + + def drop_method(m): + del m["paths"]["/hooks"]["post"] + + def drop_param(m): + m["paths"]["/hooks"]["get"]["parameters"] = [] + + def change_description(m): + m["components"]["schemas"]["HookView"]["properties"]["phase"]["description"] = ( + "an older sentence core no longer ships" + ) + + def change_version(m): + m["info"]["version"] = "9.9.8" + + def change_unclassified(m): + m["paths"]["/hooks"]["get"]["operationId"] = "listHooks" + + def change_response(m): + m["paths"]["/hooks"]["get"]["responses"]["200"]["content"]["application/json"][ + "schema" + ] = {"type": "object"} + + return [ + ("schema missing from the mirror", drop_schema, "schema-missing", "SecretView"), + ("schema the mirror has and core does not", add_schema, "schema-extra", "GhostView"), + ("properties missing from a schema", drop_props, "property-missing", "HookView"), + ("property the mirror has and core does not", add_prop, "property-extra", + "legacy_stage"), + ("required set changed", change_required, "required-missing", "kind"), + ("enum variant dropped", change_enum, "enum-variant-missing", "HookPhase"), + ("property type changed", change_type, "type-changed", "HookView.groups"), + ("path missing from the mirror", drop_path, "path-missing", "/hooks"), + ("method missing from the mirror", drop_method, "method-missing", "POST /hooks"), + ("parameter missing from an operation", drop_param, "param-missing", "limit"), + ("response schema changed", change_response, "response-changed", "GET /hooks"), + # The residual sweep. These change no wire shape, but a mirror is a COPY, so the gate + # must still name them rather than stay green on a document it did not fully compare. + ("description text drifted", change_description, "doc-text-changed", + "/components/schemas/HookView/properties/phase/description"), + ("info.version drifted", change_version, "version-changed", "/info/version"), + ("difference outside the structural walk", change_unclassified, + "unclassified-difference", "/paths/~1hooks/get/operationId"), + ] + + +# --------------------------------------------------------------------------- +# runner +# --------------------------------------------------------------------------- + + +def run(out): + failures = [] + passed = 0 + + out.write("spec-mirror-gate self-test: prove RED before trusting GREEN\n") + out.write("=" * 72 + "\n\n") + + # 0. The identical pair must be GREEN. A gate that fails on everything is also useless. + core = base_spec() + findings = gate.compare(core, clone(core)) + if findings: + failures.append( + "control: an identical mirror produced %d finding(s), expected none: %s" + % (len(findings), "; ".join(str(f) for f in findings[:5])) + ) + out.write(" FAIL control (identical mirror must be GREEN)\n") + else: + passed += 1 + out.write(" ok control (identical mirror is GREEN)\n") + + # 1..n. Every finding class must go RED and must NAME the thing. + for label, mutate, kind, must_name in cases(): + mirror = clone(core) + mutate(mirror) + findings = gate.compare(core, mirror) + kinds = set(f.kind for f in findings) + named = [f for f in findings if f.kind == kind and must_name in f.detail] + if not findings: + failures.append("%s: gate stayed GREEN on a broken mirror" % label) + out.write(" FAIL %-46s stayed GREEN\n" % label) + elif kind not in kinds: + failures.append( + "%s: expected a %r finding, got kinds %s" % (label, kind, sorted(kinds)) + ) + out.write(" FAIL %-46s wrong kind %s\n" % (label, sorted(kinds))) + elif not named: + failures.append( + "%s: a %r finding was raised but it does not NAME %r" + % (label, kind, must_name) + ) + out.write(" FAIL %-46s does not name %s\n" % (label, must_name)) + else: + passed += 1 + out.write(" ok %-46s RED, names %s\n" % (label, must_name)) + + # Fail-closed 1: core's spec cannot be fetched. + with tempfile.TemporaryDirectory() as tmp: + mirror_path = os.path.join(tmp, "openapi.json") + with open(mirror_path, "w") as fh: + json.dump(core, fh) + + def exploding_opener(url, headers, timeout): + raise IOError("selftest: network is down") + + real = gate.fetch_core_spec + try: + gate.fetch_core_spec = lambda ref, timeout=30, opener=None: real( + ref, timeout=timeout, opener=exploding_opener + ) + rc = gate.main(["--mirror", mirror_path, "--busbar-ref", "dev"]) + finally: + gate.fetch_core_spec = real + if rc == 2: + passed += 1 + out.write(" ok %-46s FATAL (exit 2), not a skip\n" % "core spec unfetchable") + else: + failures.append( + "unfetchable core spec returned %r; it must be FATAL (2), never a skip" % rc + ) + out.write(" FAIL %-46s returned %r, expected 2\n" + % ("core spec unfetchable", rc)) + + # Fail-closed 1b: `latest-release` cannot be resolved to a tag. Resolving the tag is a + # second network call, so it is a second chance to fail open. It must not take it. The + # pre-existing spec-drift job resolved this exact tag and printed a ::warning:: + exit 0 + # when the API was rate-limited, which is how a rate-limited runner reported "no drift". + real_resolve = gate.resolve_ref + try: + gate.resolve_ref = lambda ref, timeout=30, opener=None: real_resolve( + ref, timeout=timeout, opener=exploding_opener + ) + rc = gate.main(["--mirror", mirror_path, "--busbar-ref", gate.LATEST_RELEASE]) + finally: + gate.resolve_ref = real_resolve + if rc == 2: + passed += 1 + out.write(" ok %-46s FATAL (exit 2), not a skip\n" + % "latest-release unresolvable") + else: + failures.append( + "an unresolvable %r returned %r; it must be FATAL (2), never a skip" + % (gate.LATEST_RELEASE, rc) + ) + out.write(" FAIL %-46s returned %r, expected 2\n" + % ("latest-release unresolvable", rc)) + + # Fail-closed 2: core's spec is fetched but is not parseable. + bad = os.path.join(tmp, "bad.json") + with open(bad, "w") as fh: + fh.write("{ this is not json") + rc = gate.main(["--mirror", mirror_path, "--core-spec", bad]) + if rc == 2: + passed += 1 + out.write(" ok %-46s FATAL (exit 2), not a skip\n" % "core spec unparseable") + else: + failures.append("unparseable core spec returned %r; it must be FATAL (2)" % rc) + out.write(" FAIL %-46s returned %r, expected 2\n" + % ("core spec unparseable", rc)) + + # Fail-closed 3: the version string alone must NOT be able to make it green. This is the + # exact false-green that shipped: same info.version, different shapes. + same_version_stale = clone(core) + for p in ("phase", "fires_at", "groups"): + del same_version_stale["components"]["schemas"]["HookView"]["properties"][p] + assert same_version_stale["info"]["version"] == core["info"]["version"] + stale_path = os.path.join(tmp, "stale.json") + with open(stale_path, "w") as fh: + json.dump(same_version_stale, fh) + core_path = os.path.join(tmp, "core.json") + with open(core_path, "w") as fh: + json.dump(core, fh) + buf = io.StringIO() + rc = gate.report( + gate.compare(core, same_version_stale), stale_path, core_path, buf + ) + text = buf.getvalue() + if rc == 1 and all(n in text for n in ("HookView", "phase", "fires_at", "groups")): + passed += 1 + out.write(" ok %-46s RED despite an identical info.version\n" + % "stale mirror, same version string") + else: + failures.append( + "a stale mirror carrying core's own info.version returned %r; it must be RED " + "and must name HookView/phase/fires_at/groups" % rc + ) + out.write(" FAIL %-46s rc=%r\n" % ("stale mirror, same version string", rc)) + + out.write("\n" + "=" * 72 + "\n") + if failures: + out.write("SELF-TEST FAILED: %d of %d checks did not hold.\n" + % (len(failures), passed + len(failures))) + for f in failures: + out.write(" - %s\n" % f) + out.write("\nDo NOT trust this gate's verdict until the self-test is green.\n") + return 1 + out.write("SELF-TEST PASSED: %d checks. Every finding class goes RED on a broken\n" + "mirror and names the schema/property at fault, and an unknown core spec is\n" + "FATAL rather than green. The gate's verdict can be trusted.\n" % passed) + return 0 + + +# Running this file DIRECTLY must not look like a pass. Without this, `python3 +# spec_mirror_selftest.py` defined `run()` and exited 0 having asserted NOTHING, which is the exact +# shape of failure this whole gate exists to remove: a green that proves nothing ran. The canonical +# entry point is `spec_mirror_gate.py --selftest`, which is what CI invokes; this makes the direct +# invocation do the same thing rather than silently succeed. +if __name__ == "__main__": + import sys + + sys.exit(run(sys.stdout)) diff --git a/src/argmap.rs b/src/argmap.rs new file mode 100644 index 0000000..8456240 --- /dev/null +++ b/src/argmap.rs @@ -0,0 +1,38 @@ +// SPDX-License-Identifier: Apache-2.0 + +//! Pure CLI-argument mappings shared by the binary and its tests. +//! +//! These carry contract meaning (the tri-state `allowed_pools`, the first-`=`-wins label split), +//! so they live in the library rather than in `main.rs`: a test that imports the REAL function +//! catches a regression, a test against a copy of it cannot. + +use anyhow::Result; + +/// Resolve the three distinct `allowed_pools` states the server understands: omitted (`None`) = +/// ALL pools; an explicit empty list (`--no-pools`) = NO pools; a non-empty list = exactly those. +/// A shared function so `cmd_keys_create` and its test exercise the SAME mapping. Collapsing +/// `--no-pools` into `None` would mint an all-pools key when no-pools was asked for (fail-open on +/// privilege). +pub fn resolve_allowed_pools(no_pools: bool, pools: &[String]) -> Option> { + if no_pools { + Some(Vec::new()) + } else if pools.is_empty() { + None + } else { + Some(pools.to_vec()) + } +} + +/// Parse repeated `--label KEY=VALUE` arguments into the map the mint body carries. The split is +/// on the FIRST `=` only, so a value that itself contains `=` (a URL query string, base64 padding) +/// survives intact. +pub fn parse_labels(pairs: &[String]) -> Result> { + pairs + .iter() + .map(|p| { + p.split_once('=') + .map(|(k, v)| (k.to_string(), v.to_string())) + .ok_or_else(|| anyhow::anyhow!("--label must be KEY=VALUE, got {p:?}")) + }) + .collect() +} diff --git a/src/client.rs b/src/client.rs index 0e2b537..e8c6f63 100644 --- a/src/client.rs +++ b/src/client.rs @@ -297,15 +297,19 @@ pub struct ConfigApplyView { // ── Contract-mirroring request/response types ──────────────────────────────────────────────── -#[derive(Deserialize)] -struct ErrorEnvelope { - error: ErrorDetail, +/// The gateway's error envelope (`Error` in the spec): `{"error":{"code","message"}}`. Public so +/// the conformance tests can assert the decoded shape against the committed schema; the CLI itself +/// only renders `code`/`message` into a message. +#[derive(Debug, Deserialize, Serialize)] +pub struct ErrorEnvelope { + pub error: ErrorDetail, } -#[derive(Deserialize)] -struct ErrorDetail { - code: String, - message: String, +/// One error detail: a `code` drawn from the spec's closed vocabulary plus a human `message`. +#[derive(Debug, Deserialize, Serialize)] +pub struct ErrorDetail { + pub code: String, + pub message: String, } /// `GET /info` — mirrors `contract::InfoView`. @@ -610,151 +614,3 @@ pub struct HookPage { #[serde(default)] pub next_cursor: Option, } - -#[cfg(test)] -mod spec_shape_tests { - // These lock the 1.5.2 wire shapes this crate was realigned to (commit that repaired the - // 1.4.x drift). The spec-drift CI job only checks the committed openapi.json's VERSION string; - // it does NOT check that these structs still match the spec's schemas. Without these, a - // rebase reintroducing a 1.4.x field, or flipping `allowed_pools` back to a non-Option Vec, - // compiles clean and ships — exactly the regression class the realignment fixed. - use super::*; - - #[test] - fn key_view_allowed_pools_null_is_all_pools() { - // null (or omitted) allowed_pools => None => "all pools". A regression to `Vec` - // would fail to deserialize null, or silently decode it as an empty (== NO pools) list. - let k: KeyView = serde_json::from_str( - r#"{"id":"vk_1","name":"n","allowed_pools":null,"state":"active","enabled":true}"#, - ) - .expect("null allowed_pools must decode"); - assert_eq!( - k.allowed_pools, None, - "null must mean all-pools (None), not []" - ); - } - - #[test] - fn key_view_allowed_pools_empty_is_no_pools() { - let k: KeyView = - serde_json::from_str(r#"{"id":"vk_1","name":"n","allowed_pools":[]}"#).unwrap(); - assert_eq!( - k.allowed_pools, - Some(Vec::new()), - "explicit [] must stay a distinct empty list (NO pools), never collapse to None" - ); - } - - #[test] - fn created_key_view_carries_signed_token_not_secret() { - // 1.5.0 credential is `token` (+ expires_at), NOT the 1.4.x `secret`. A revert to a - // required `secret` field would fail to decode this real-shaped response. - let c: CreatedKeyView = serde_json::from_str( - r#"{"id":"vk_1","name":"n","token":"bbk_abc","expires_at":1785772871,"state":"active"}"#, - ) - .expect("token-shaped CreatedKeyView must decode"); - assert_eq!(c.token, "bbk_abc"); - assert_eq!(c.expires_at, 1785772871_u64); - } - - #[test] - fn create_key_req_omits_none_fields_and_never_sends_legacy_budget() { - // deny_unknown_fields on the server rejects any stray field. This pins the exact minimal - // body and fails if a 1.4.x budget/rpm/tpm field is ever reintroduced onto the struct. - let req = CreateKeyReq { - name: "svc".into(), - allowed_pools: None, - group: None, - parent: None, - expires_in: None, - expires_at: None, - labels: Default::default(), - issue_aws_credential: false, - }; - let v: serde_json::Value = serde_json::to_value(&req).unwrap(); - let obj = v.as_object().unwrap(); - assert_eq!( - obj.keys().collect::>(), - vec!["name"], - "an all-defaults CreateKeyReq must serialize to exactly {{name}} — any extra key is \ - either a leaked None or a reintroduced legacy field the server will 400 on" - ); - for banned in [ - "budget", - "budget_period", - "rpm_limit", - "tpm_limit", - "max_budget_cents", - ] { - assert!( - !obj.contains_key(banned), - "legacy field {banned} must never serialize" - ); - } - } - - #[test] - fn create_key_req_empty_allowed_pools_serializes_as_empty_array() { - // The --no-pools path sets Some(vec![]); it must reach the wire as `[]`, not be dropped. - let req = CreateKeyReq { - name: "svc".into(), - allowed_pools: Some(Vec::new()), - group: None, - parent: None, - expires_in: None, - expires_at: None, - labels: Default::default(), - issue_aws_credential: false, - }; - let v: serde_json::Value = serde_json::to_value(&req).unwrap(); - assert_eq!(v["allowed_pools"], serde_json::json!([])); - } - - #[test] - fn rotated_key_view_token_and_secret_are_both_optional() { - // The doc invariant is "exactly one of token or secret"; the type models both as optional - // so a token-only rotate decodes without a phantom `secret`. - let r: RotatedKeyView = - serde_json::from_str(r#"{"id":"vk_1","name":"n","token":"bbk_new"}"#).unwrap(); - assert_eq!(r.token.as_deref(), Some("bbk_new")); - assert_eq!(r.secret, None); - } - - #[test] - fn inspect_view_carries_manifest_preview_shape() { - // 1.5.2 added POST /plugins/inspect — a stateless preview whose body is the PluginSchemaView - // shape plus version/kind. This pins the fields busbar-admin renders so a future spec resync - // that drops/renames one (e.g. `trust`, `schema_error`, or the new `version`) fails loudly. - let v: InspectView = serde_json::from_str( - r#"{"name":"acme-store","version":"1.2.3","kind":"store","trust":"unverified", - "source":"manifest","restart_required_default":true, - "schema":{"type":"object"},"schema_error":null}"#, - ) - .expect("inspect preview must decode"); - assert_eq!(v.name, "acme-store"); - assert_eq!(v.version.as_deref(), Some("1.2.3")); - assert_eq!(v.kind.as_deref(), Some("store")); - assert_eq!(v.trust, "unverified"); - assert_eq!(v.restart_required_default, Some(true)); - assert!(v.schema.is_some()); - assert_eq!(v.schema_error, None); - } - - #[test] - fn inspect_view_tolerates_null_kind_and_absent_version() { - // An unresolvable candidate reports kind/version/restart_required_default as null; the CLI - // must still decode (never refuse) so it can render the `trust`/`schema_error` verdict. - let v: InspectView = serde_json::from_str( - r#"{"name":"bad","kind":null,"trust":"rejected","source":"manifest","schema":null, - "schema_error":"settings_schema is not valid JSON"}"#, - ) - .expect("a rejected/unresolvable candidate must still decode"); - assert_eq!(v.kind, None); - assert_eq!(v.version, None); - assert_eq!(v.trust, "rejected"); - assert_eq!( - v.schema_error.as_deref(), - Some("settings_schema is not valid JSON") - ); - } -} diff --git a/src/lib.rs b/src/lib.rs new file mode 100644 index 0000000..f83ac9b --- /dev/null +++ b/src/lib.rs @@ -0,0 +1,13 @@ +// SPDX-License-Identifier: Apache-2.0 +#![forbid(unsafe_code)] + +//! The busbar-admin library surface. +//! +//! The binary (`src/main.rs`) is the clap surface + rendering; everything it needs to talk to a +//! gateway, and every pure mapping it applies to CLI arguments, lives here so integration tests +//! under `tests/` exercise the SAME code the binary runs (a `[[bin]]`-only crate cannot be +//! imported by a test, which is why the wire types could previously only be tested from an +//! inline `#[cfg(test)]` module). + +pub mod argmap; +pub mod client; diff --git a/src/main.rs b/src/main.rs index 8ce4045..5e65a69 100644 --- a/src/main.rs +++ b/src/main.rs @@ -4,29 +4,15 @@ //! busbar-admin — a human-facing CLI for the busbar gateway's admin API (`/api/v1/admin`). //! //! Config resolution is CLI flag > env var > a clear error. The thin admin client lives in -//! [`client`]; this module is the clap surface + human/JSON rendering. - -mod client; +//! `busbar_admin::client`; this module is the clap surface + human/JSON rendering. use anyhow::{Context, Result}; use clap::{Args, Parser, Subcommand}; -use client::{Client, CreateKeyReq, InspectPluginReq, InstallPluginReq, KeyView, PluginView, Tls}; - -/// Resolve the three distinct `allowed_pools` states the server understands: omitted (`None`) = -/// ALL pools; an explicit empty list (`--no-pools`) = NO pools; a non-empty list = exactly those. -/// A shared function so `cmd_keys_create` and its test exercise the SAME mapping — collapsing -/// `--no-pools` into `None` would mint an all-pools key when no-pools was asked for (fail-open on -/// privilege). -fn resolve_allowed_pools(no_pools: bool, pools: &[String]) -> Option> { - if no_pools { - Some(Vec::new()) - } else if pools.is_empty() { - None - } else { - Some(pools.to_vec()) - } -} +use busbar_admin::argmap::{parse_labels, resolve_allowed_pools}; +use busbar_admin::client::{ + Client, CreateKeyReq, InspectPluginReq, InstallPluginReq, KeyView, PluginView, Tls, +}; /// busbar-admin — talk to a busbar gateway's admin API. #[derive(Parser)] @@ -338,17 +324,6 @@ fn cmd_keys_list(c: &Client, json: bool) -> Result<()> { Ok(()) } -fn parse_labels(pairs: &[String]) -> Result> { - pairs - .iter() - .map(|p| { - p.split_once('=') - .map(|(k, v)| (k.to_string(), v.to_string())) - .ok_or_else(|| anyhow::anyhow!("--label must be KEY=VALUE, got {p:?}")) - }) - .collect() -} - fn pools_summary(pools: &Option>) -> String { match pools { None => "(all)".into(), @@ -737,51 +712,3 @@ fn human_duration(secs: u64) -> String { parts.push(format!("{s}s")); parts.join(" ") } - -#[cfg(test)] -mod cli_logic_tests { - use super::*; - - #[test] - fn parse_labels_splits_on_first_equals_only() { - // A value containing '=' (a URL query string, a base64 pad) must survive intact — the - // split is on the FIRST '=', not all of them. - let m = parse_labels(&["url=http://x?a=b".into(), "team=platform".into()]).unwrap(); - assert_eq!(m.get("url").map(String::as_str), Some("http://x?a=b")); - assert_eq!(m.get("team").map(String::as_str), Some("platform")); - } - - #[test] - fn parse_labels_rejects_a_pair_with_no_equals() { - assert!(parse_labels(&["novalue".into()]).is_err()); - } - - #[test] - fn parse_labels_allows_empty_value() { - let m = parse_labels(&["k=".into()]).unwrap(); - assert_eq!(m.get("k").map(String::as_str), Some("")); - } - - // Calls the REAL `resolve_allowed_pools` that `cmd_keys_create` uses — not a copy — so a - // regression in the actual fail-open-on-privilege mapping fails this test. (The round-1 - // version tested a duplicated helper and was tautological: it passed even if the real code - // regressed.) - #[test] - fn allowed_pools_tristate_no_pools_is_empty_not_none() { - assert_eq!( - resolve_allowed_pools(true, &[]), - Some(Vec::new()), - "--no-pools => NO pools" - ); - assert_eq!( - resolve_allowed_pools(false, &[]), - None, - "omitted => ALL pools" - ); - assert_eq!( - resolve_allowed_pools(false, &["p1".into()]), - Some(vec!["p1".into()]), - "a list => exactly those pools" - ); - } -} diff --git a/tests/cli_args.rs b/tests/cli_args.rs new file mode 100644 index 0000000..de8f1f1 --- /dev/null +++ b/tests/cli_args.rs @@ -0,0 +1,45 @@ +// SPDX-License-Identifier: Apache-2.0 + +//! Pure CLI-argument mapping tests. These call the REAL functions `cmd_keys_create` uses (via +//! `busbar_admin::argmap`), not a copy, so a regression in the actual mapping fails here. + +use busbar_admin::argmap::{parse_labels, resolve_allowed_pools}; + +#[test] +fn parse_labels_splits_on_first_equals_only() { + // A value containing '=' (a URL query string, base64 padding) must survive intact: the split + // is on the FIRST '=', not all of them. + let m = parse_labels(&["url=http://x?a=b".into(), "team=platform".into()]).unwrap(); + assert_eq!(m.get("url").map(String::as_str), Some("http://x?a=b")); + assert_eq!(m.get("team").map(String::as_str), Some("platform")); +} + +#[test] +fn parse_labels_rejects_a_pair_with_no_equals() { + assert!(parse_labels(&["novalue".into()]).is_err()); +} + +#[test] +fn parse_labels_allows_empty_value() { + let m = parse_labels(&["k=".into()]).unwrap(); + assert_eq!(m.get("k").map(String::as_str), Some("")); +} + +#[test] +fn allowed_pools_tristate_no_pools_is_empty_not_none() { + assert_eq!( + resolve_allowed_pools(true, &[]), + Some(Vec::new()), + "--no-pools => NO pools" + ); + assert_eq!( + resolve_allowed_pools(false, &[]), + None, + "omitted => ALL pools" + ); + assert_eq!( + resolve_allowed_pools(false, &["p1".into()]), + Some(vec!["p1".into()]), + "a list => exactly those pools" + ); +} diff --git a/tests/openapi_conformance.rs b/tests/openapi_conformance.rs new file mode 100644 index 0000000..92b70b6 --- /dev/null +++ b/tests/openapi_conformance.rs @@ -0,0 +1,769 @@ +// SPDX-License-Identifier: Apache-2.0 + +//! Cross-repo SHAPE conformance: every serde type in `busbar_admin::client` checked against the +//! REAL committed `openapi.json` at the repo root. +//! +//! Why this file exists. busbar-admin is a hand-rolled client for another repo's API, so its +//! structs are a mirror of wire shapes that live somewhere else. The `spec-drift` CI job compares +//! `jq -r .info.version openapi.json` against the latest busbar release tag: a VERSION STRING. It +//! says nothing about whether `src/client.rs` still matches the schemas inside that document, and +//! it degrades to a warning when the GitHub API is unreachable. So a struct could drift arbitrarily +//! from the spec sitting in the same commit and every gate stayed green. +//! +//! What is asserted, per mapped type: +//! 1. Rust field set (the WIRE keys, taken from a real serialize) vs the schema's `properties`. +//! A Rust field with no schema property fails. A schema property with no Rust field fails +//! unless it is listed in that case's `unmodelled` allowlist, which is itself checked for +//! staleness. Adding a property to the schema therefore fails this suite until someone +//! either models it or consciously records the omission. +//! 2. Optionality. A property the schema marks required and NOT nullable must reject an explicit +//! `null`, which is only true if the Rust field is not an `Option`. A property the schema +//! marks nullable must ACCEPT `null`, which is only true if it is an `Option`. The two +//! directions together pin every field's optionality to the document. +//! 3. Round-trip. A schema-shaped value is deserialized, re-serialized, and both the wire keys +//! and the per-property values are compared. +//! 4. Endpoint wiring: every path the client calls exists in `paths` with that method, and its +//! success response points at the schema the corresponding client method decodes into. +//! +//! The spec is loaded with `include_str!` from the actual committed file, not a copy, so there is +//! nothing to keep in sync by hand. + +use std::collections::BTreeSet; +use std::sync::OnceLock; + +use serde::de::DeserializeOwned; +use serde::Serialize; +use serde_json::{json, Map, Value}; + +use busbar_admin::client::{ + BuildInfo, ConfigApplyView, CreateKeyReq, CreatedKeyView, ErrorEnvelope, HookPage, + HookTransportView, HookView, InfoView, InspectPluginReq, InspectView, InstallPluginReq, + KeyPage, KeyView, PluginInstallView, PluginPage, PluginReloadView, PluginView, RevokeView, + RotatedKeyView, TopologyInfo, +}; + +/// The ACTUAL committed spec, compiled into the test binary. `include_str!` is relative to this +/// source file, so this is `/openapi.json` and cannot silently become a stale copy. +const OPENAPI_JSON: &str = include_str!("../openapi.json"); + +fn spec() -> &'static Value { + static SPEC: OnceLock = OnceLock::new(); + SPEC.get_or_init(|| { + serde_json::from_str(OPENAPI_JSON).expect("openapi.json must be valid JSON") + }) +} + +fn schema(name: &str) -> &'static Value { + spec() + .get("components") + .and_then(|c| c.get("schemas")) + .and_then(|s| s.get(name)) + .unwrap_or_else(|| { + panic!("openapi.json has no components.schemas.{name}: the spec was resynced and this schema was renamed or removed") + }) +} + +fn properties(sch: &'static Value) -> &'static Map { + sch.get("properties") + .and_then(Value::as_object) + .expect("schema must carry properties") +} + +fn required_of(sch: &Value) -> BTreeSet { + sch.get("required") + .and_then(Value::as_array) + .map(|a| { + a.iter() + .filter_map(|v| v.as_str().map(str::to_string)) + .collect() + }) + .unwrap_or_default() +} + +/// Is this property allowed to carry `null` on the wire? A `$ref` to another object schema is +/// not. A `type` array containing `"null"` is. A property with NO `type` at all (the spec emits +/// that for a free-form JSON value, e.g. `PluginSchemaView.schema`) can be anything, null +/// included. +fn nullable(prop: &Value) -> bool { + if prop.get("$ref").is_some() { + return false; + } + match prop.get("type") { + None => true, + Some(Value::String(_)) => false, + Some(Value::Array(types)) => types.iter().any(|t| t == "null"), + Some(other) => panic!("unexpected `type` encoding in the spec: {other}"), + } +} + +fn keys_of(v: &Value) -> BTreeSet { + v.as_object() + .expect("expected a JSON object") + .keys() + .cloned() + .collect() +} + +/// One response type under test: the schema it mirrors, a fully schema-shaped sample payload, and +/// the schema properties the Rust type deliberately does not carry. +struct Case<'a> { + /// `components.schemas` entry name. + schema: &'a str, + /// A value shaped per that schema, carrying every property the Rust type models. + sample: Value, + /// Schema properties the client knowingly does not model (the contract is additive-only and + /// the CLI renders a subset). Every entry must still be a real property that is still absent + /// from the Rust type, so this list cannot rot into a blanket excuse. + unmodelled: &'a [&'a str], +} + +/// Run every check for a response type the client DESERIALIZES. +fn check_response(case: Case<'_>) { + let sch = schema(case.schema); + let props = properties(sch); + let required = required_of(sch); + let schema_props: BTreeSet = props.keys().cloned().collect(); + + // The Rust field set, taken from a real serialize of a real deserialize: these are the WIRE + // keys, so a `#[serde(rename)]` is honoured and a renamed Rust field shows up here. + let decoded: T = serde_json::from_value(case.sample.clone()).unwrap_or_else(|e| { + panic!( + "{}: a schema-shaped payload must deserialize into the Rust type, got: {e}", + case.schema + ) + }); + let round = serde_json::to_value(&decoded).expect("re-serializing must succeed"); + let rust_keys = keys_of(&round); + + // 1a. A Rust field with no schema property: the client is inventing wire keys. + let invented: Vec<_> = rust_keys.difference(&schema_props).collect(); + assert!( + invented.is_empty(), + "{}: Rust field(s) {invented:?} have no property in the committed openapi.json schema \ + (renamed field, or a field the spec never had)", + case.schema + ); + + // 1b. Allowlist hygiene: a name listed as unmodelled must still be a real schema property, + // and must still be genuinely absent from the Rust type. + for u in case.unmodelled { + assert!( + schema_props.contains(*u), + "{}: `{u}` is on the unmodelled allowlist but is no longer a property of the schema; \ + drop the stale entry", + case.schema + ); + assert!( + !rust_keys.contains(*u), + "{}: `{u}` is on the unmodelled allowlist but the Rust type DOES carry it; drop the \ + entry so the field is checked like every other", + case.schema + ); + } + + // 1c. A schema property with no Rust field. This is the check that fires when busbar adds a + // property: modelling it or recording it in `unmodelled` is then a deliberate act. + let allow: BTreeSet = case.unmodelled.iter().map(|s| (*s).to_string()).collect(); + let unread: Vec<_> = schema_props + .difference(&rust_keys) + .filter(|p| !allow.contains(*p)) + .collect(); + assert!( + unread.is_empty(), + "{}: schema propert(ies) {unread:?} have no Rust field. The client reads this response, \ + so either model them or add them to this case's `unmodelled` list with a reason", + case.schema + ); + + // 2. Optionality, driven off `required` + nullability, proven by behaviour: an `Option` field + // accepts an explicit null and a non-`Option` field rejects it. + for (name, prop) in props { + if allow.contains(name) { + continue; + } + let mut nulled = case.sample.as_object().cloned().expect("object sample"); + nulled.insert(name.clone(), Value::Null); + let attempt = serde_json::from_value::(Value::Object(nulled)); + if nullable(prop) { + assert!( + attempt.is_ok(), + "{}.{name}: the schema types this property as nullable, so the Rust field must be \ + an Option and accept an explicit null, but decoding failed: {:?}", + case.schema, + attempt.err() + ); + } else if required.contains(name) { + assert!( + attempt.is_err(), + "{}.{name}: the schema marks this property REQUIRED and non-nullable, but the \ + Rust type accepted an explicit null, which means the field is an Option (or \ + otherwise nullable). A required non-nullable property must be a plain field", + case.schema + ); + } + } + + // 3. Round-trip: every modelled property survives decode + encode with its value intact. + for (name, value) in case.sample.as_object().expect("object sample") { + if allow.contains(name) || !schema_props.contains(name) { + continue; + } + assert_eq!( + round.get(name), + Some(value), + "{}.{name}: value did not survive the deserialize/serialize round trip", + case.schema + ); + } +} + +/// Run the field-set checks for a request type the client only SERIALIZES. +/// +/// `full` is a fully populated instance (every optional set) so the complete Rust field set shows +/// up on the wire; `minimal` is an all-defaults instance, which must still emit every property the +/// schema marks required. +fn check_request(schema_name: &str, full: &Value, minimal: &Value, unmodelled: &[&str]) { + let sch = schema(schema_name); + let props = properties(sch); + let required = required_of(sch); + let schema_props: BTreeSet = props.keys().cloned().collect(); + let rust_keys = keys_of(full); + + let invented: Vec<_> = rust_keys.difference(&schema_props).collect(); + assert!( + invented.is_empty(), + "{schema_name}: request field(s) {invented:?} are not properties of the committed schema. \ + The server sets `deny_unknown_fields` on its request bodies, so sending one is a 400" + ); + + let allow: BTreeSet = unmodelled.iter().map(|s| (*s).to_string()).collect(); + for u in unmodelled { + assert!( + schema_props.contains(*u), + "{schema_name}: stale `unmodelled` entry `{u}` is no longer a schema property" + ); + assert!( + !rust_keys.contains(*u), + "{schema_name}: `{u}` is listed as unmodelled but the Rust type carries it" + ); + } + let missing: Vec<_> = schema_props + .difference(&rust_keys) + .filter(|p| !allow.contains(*p)) + .collect(); + assert!( + missing.is_empty(), + "{schema_name}: schema propert(ies) {missing:?} cannot be sent by this client. A new \ + request property means a capability the CLI silently cannot use" + ); + + let minimal_keys = keys_of(minimal); + for req in &required { + assert!( + minimal_keys.contains(req), + "{schema_name}: `{req}` is required by the schema but an all-defaults instance does \ + not serialize it (a `skip_serializing_if` on a required field is a guaranteed 400)" + ); + } +} + +// Sample fragments reused across cases. + +fn key_meta_sample() -> Map { + json!({ + "id": "vk_0123456789abcdef", + "name": "svc-checkout", + "allowed_pools": ["fast", "cheap"], + "group": "team:payments", + "labels": {"team": "payments"}, + "state": "active", + "enabled": true, + "created_at": 1785772871_u64 + }) + .as_object() + .cloned() + .unwrap() +} + +/// Properties `PluginView` carries in the spec but not in Rust. Named once here so the case +/// declaration and the sample-reduction helper below cannot disagree. +const PLUGIN_VIEW_UNMODELLED: &[&str] = &[ + // The manifest NAME of a dynamic-library plugin. The CLI renders `file`, which is the handle + // the sibling endpoints key off; `target` adds nothing to any rendered row. + "target", + // The store C-ABI number: an engine-internal compatibility detail with no CLI column. + "interface_version", + // A relative URL the CLI never follows: it prints `has_schema` (which mirrors + // `schema_url.is_some()`) instead of fetching a per-row schema. + "schema_url", + // The list-row copy of the inspect verdict. `busbar-admin plugins inspect` surfaces it from + // PluginSchemaView, where the CLI actually renders it. + "schema_error", +]; + +/// A `PluginView` row carrying ONLY the properties the Rust type models: what a nested item must +/// be for a value-level round-trip comparison to be meaningful. +fn plugin_view_sample_modelled() -> Value { + let mut row = plugin_view_sample().as_object().cloned().unwrap(); + for k in PLUGIN_VIEW_UNMODELLED { + row.remove(*k); + } + Value::Object(row) +} + +fn plugin_view_sample() -> Value { + // Includes the properties the client does NOT model, so the "unknown fields are ignored" + // tolerance the client documents is exercised on a realistic row. + json!({ + "name": "acme-store", + "type": "store", + "loader": "plugin", + "active": true, + "target": "acme-store", + "file": "acme_store.tar.gz", + "version": "1.2.3", + "publisher": "acme", + "trust": "trusted", + "valid": true, + "error": null, + "has_schema": true, + "interface_version": 1, + "schema_url": "/api/v1/admin/plugins/acme_store.tar.gz/schema", + "schema_error": null + }) +} + +#[test] +fn info_view_matches_schema() { + check_response::(Case { + schema: "InfoView", + sample: json!({ + "version": "1.5.3", + "build": {"auth_modules": ["tokens"], "hook_plugins": ["ranking"], "weighted_floor": true}, + "uptime_seconds": 3661_u64, + "started_at": 1785772871_u64, + "topology": {"pools": 2, "models": 9, "providers": 3}, + "config_persistence": true, + "config_version": 7_u64 + }), + unmodelled: &[], + }); +} + +#[test] +fn build_info_matches_schema() { + check_response::(Case { + schema: "BuildInfo", + sample: json!({ + "auth_modules": ["tokens"], + "hook_plugins": ["ranking"], + "weighted_floor": true + }), + unmodelled: &[], + }); +} + +#[test] +fn topology_info_matches_schema() { + check_response::(Case { + schema: "TopologyInfo", + sample: json!({"pools": 2, "models": 9, "providers": 3}), + unmodelled: &[], + }); +} + +#[test] +fn key_view_matches_schema() { + check_response::(Case { + schema: "KeyView", + sample: Value::Object(key_meta_sample()), + unmodelled: &[], + }); +} + +#[test] +fn key_page_matches_key_page_view_schema() { + check_response::(Case { + schema: "KeyPageView", + sample: json!({"items": [Value::Object(key_meta_sample())], "next_cursor": "eyJvIjoyMDB9"}), + unmodelled: &[], + }); +} + +#[test] +fn created_key_view_matches_schema() { + let mut sample = key_meta_sample(); + sample.insert("token".into(), json!("bbk_live_abc")); + sample.insert("expires_at".into(), json!(1785772871_u64)); + sample.insert("group_provisioned".into(), json!(true)); + sample.insert("aws_access_key_id".into(), json!("AKIAEXAMPLE")); + sample.insert("aws_secret_access_key".into(), json!("s3cr3t")); + check_response::(Case { + schema: "CreatedKeyView", + sample: Value::Object(sample), + unmodelled: &[], + }); +} + +#[test] +fn rotated_key_view_matches_schema() { + let mut sample = key_meta_sample(); + sample.insert("token".into(), json!("bbk_live_new")); + sample.insert("expires_at".into(), json!(1785772871_u64)); + sample.insert("secret".into(), Value::Null); + check_response::(Case { + schema: "RotatedKeyView", + sample: Value::Object(sample), + unmodelled: &[], + }); +} + +#[test] +fn revoke_view_matches_schema() { + check_response::(Case { + schema: "RevokeView", + sample: json!({"revoked": "vk_0123456789abcdef"}), + unmodelled: &[], + }); +} + +#[test] +fn plugin_view_matches_schema() { + check_response::(Case { + schema: "PluginView", + sample: plugin_view_sample(), + unmodelled: PLUGIN_VIEW_UNMODELLED, + }); +} + +#[test] +fn plugin_page_matches_page_plugin_view_schema() { + check_response::(Case { + schema: "Page_PluginView", + sample: json!({"items": [plugin_view_sample_modelled()], "next_cursor": Value::Null}), + unmodelled: &[], + }); +} + +#[test] +fn plugin_install_view_matches_schema() { + check_response::(Case { + schema: "PluginInstallView", + sample: json!({ + "file": "acme_store.tar.gz", + "name": "acme-store", + "version": "1.2.3", + "publisher": "acme", + "trust": "trusted", + "note": "takes effect on the next store reload", + "interface_version": 1 + }), + // The validated store C-ABI number; the install line renders file/name/version/trust. + unmodelled: &["interface_version"], + }); +} + +#[test] +fn inspect_view_matches_plugin_schema_view_schema() { + check_response::(Case { + schema: "PluginSchemaView", + sample: json!({ + "name": "acme-store", + "version": "1.2.3", + "kind": "store", + "trust": "unverified", + "source": "manifest", + "restart_required_default": true, + "schema": {"type": "object"}, + "schema_error": Value::Null + }), + unmodelled: &[], + }); +} + +#[test] +fn plugin_reload_view_matches_schema() { + check_response::(Case { + schema: "PluginReloadView", + sample: json!({ + "plugins": [plugin_view_sample_modelled()], + "note": "a store change applies on the next store reload" + }), + unmodelled: &[], + }); +} + +#[test] +fn hook_view_matches_schema() { + check_response::(Case { + schema: "HookView", + sample: json!({ + "name": "pii-gate", + "kind": "gate", + "transport": {"kind": "plugin", "target": "pii"}, + "prompt": "ro", + "user": "no", + "priority": 10, + "at": "request", + "on_error": "reject", + "timeout_ms": 250_u64, + "global": true, + "settings_keys": ["endpoint"] + }), + // Key NAMES only (values redacted server-side). The `hooks list` table has no column for + // them; they are a config-surface concern, not a registry-row one. + unmodelled: &["settings_keys"], + }); +} + +#[test] +fn hook_transport_view_matches_schema() { + check_response::(Case { + schema: "HookTransportView", + sample: json!({"kind": "plugin", "target": "pii"}), + unmodelled: &[], + }); +} + +#[test] +fn hook_page_matches_page_hook_view_schema() { + check_response::(Case { + schema: "Page_HookView", + sample: json!({ + "items": [{ + "name": "pii-gate", + "kind": "gate", + "transport": {"kind": "plugin", "target": "pii"}, + "prompt": "ro", + "user": "no", + "priority": 10, + "at": Value::Null, + "on_error": "reject", + "timeout_ms": 250_u64, + "global": false + }], + "next_cursor": Value::Null + }), + unmodelled: &[], + }); +} + +#[test] +fn config_apply_view_matches_schema() { + check_response::(Case { + schema: "ConfigApplyView", + sample: json!({"applied": true, "config_version": 8_u64, "note": "live, not persisted"}), + unmodelled: &[], + }); +} + +#[test] +fn error_envelope_matches_schema() { + check_response::(Case { + schema: "Error", + sample: json!({"error": {"code": "unauthorized", "message": "admin token rejected"}}), + unmodelled: &[], + }); +} + +#[test] +fn create_key_req_matches_schema() { + let full = serde_json::to_value(CreateKeyReq { + name: "svc-checkout".into(), + allowed_pools: Some(vec!["fast".into()]), + group: Some("team:payments".into()), + parent: Some("team".into()), + expires_in: Some("7d".into()), + expires_at: Some(1785772871), + labels: [("team".to_string(), "payments".to_string())] + .into_iter() + .collect(), + issue_aws_credential: true, + }) + .unwrap(); + let minimal = serde_json::to_value(CreateKeyReq { + name: "svc-checkout".into(), + allowed_pools: None, + group: None, + parent: None, + expires_in: None, + expires_at: None, + labels: Default::default(), + issue_aws_credential: false, + }) + .unwrap(); + + // The schema sets `additionalProperties: false` (the server derives `deny_unknown_fields`), + // so an extra Rust field here is not a cosmetic drift, it is a guaranteed 400 on every mint. + assert_eq!( + schema("CreateKeyReq").get("additionalProperties"), + Some(&Value::Bool(false)), + "CreateKeyReq stopped forbidding unknown fields; the strictness this case relies on moved" + ); + check_request("CreateKeyReq", &full, &minimal, &[]); +} + +#[test] +fn install_plugin_req_matches_schema() { + let body = serde_json::to_value(InstallPluginReq { + file: "acme_store.tar.gz".into(), + tarball_b64: "H4sIAA==".into(), + }) + .unwrap(); + check_request("InstallPluginReq", &body, &body, &[]); +} + +#[test] +fn inspect_plugin_req_matches_schema() { + let body = serde_json::to_value(InspectPluginReq { + file: "acme_store.tar.gz".into(), + tarball_b64: "H4sIAA==".into(), + }) + .unwrap(); + check_request("InspectPluginReq", &body, &body, &[]); +} + +/// The spec's ONLY closed `enum` on any shape this client touches is the error `code` vocabulary. +/// The client models it as a `String` (it renders `code: message` verbatim), so there is no Rust +/// variant set to compare; what IS load-bearing is that the two codes the CLI branches its 401 and +/// 403 hints on still exist, and that every documented code decodes. +#[test] +fn error_code_enum_vocabulary_matches_schema() { + let codes = schema("Error")["properties"]["error"]["properties"]["code"]["enum"] + .as_array() + .expect("Error.error.code must stay a closed enum") + .iter() + .map(|v| v.as_str().expect("enum members are strings").to_string()) + .collect::>(); + + let expected: BTreeSet = [ + "not_found", + "unauthorized", + "method_not_allowed", + "forbidden", + "invalid_request", + "version_conflict", + "conflict", + "rate_limited", + "internal", + ] + .into_iter() + .map(str::to_string) + .collect(); + assert_eq!( + codes, expected, + "the error-code vocabulary changed in the spec; re-check the 401/403 hint branches in \ + Client::check_status before updating this list" + ); + + for code in &codes { + let env: ErrorEnvelope = + serde_json::from_value(json!({"error": {"code": code, "message": "m"}})) + .unwrap_or_else(|e| panic!("error code {code} must decode: {e}")); + assert_eq!(&env.error.code, code); + assert_eq!(env.error.message, "m"); + } +} + +/// Every endpoint the client calls must exist in the spec with that method, and its success +/// response must point at the schema the client's method decodes into. This is the wiring half of +/// conformance: the cases above prove the STRUCTS match their schemas, this proves each struct is +/// still pointed at the right endpoint. +#[test] +fn client_endpoints_exist_with_the_expected_response_schema() { + // (path, method, success schema the client decodes, or None for a 204/untyped body) + let calls: &[(&str, &str, Option<&str>)] = &[ + ("/info", "get", Some("InfoView")), + ("/keys", "get", Some("KeyPageView")), + ("/keys", "post", Some("CreatedKeyView")), + ("/keys/{id}", "get", Some("KeyView")), + ("/keys/{id}", "delete", None), + ("/keys/{id}/revoke", "post", Some("RevokeView")), + ("/keys/{id}/rotate", "post", Some("RotatedKeyView")), + ("/plugins", "get", Some("Page_PluginView")), + ("/plugins", "post", Some("PluginInstallView")), + ("/plugins/inspect", "post", Some("PluginSchemaView")), + ("/plugins/reload", "post", Some("PluginReloadView")), + ("/hooks", "get", Some("Page_HookView")), + // The CLI passes the effective config through as a raw serde_json::Value: there is no Rust + // type mirroring EffectiveConfigView, by design (it is a rich nested config document that + // the CLI only pretty-prints). + ("/config", "get", Some("EffectiveConfigView")), + ("/config/apply", "post", Some("ConfigApplyView")), + ]; + + let paths = spec()["paths"] + .as_object() + .expect("the spec must carry paths"); + for (rel, method, want) in calls { + let full = format!("/api/v1/admin{rel}"); + let item = paths.get(&full).unwrap_or_else(|| { + panic!("the client calls {method} {full} but the committed spec has no such path") + }); + let op = item.get(method).unwrap_or_else(|| { + panic!("the client calls {method} {full} but the spec defines no {method} on that path") + }); + let responses = op["responses"].as_object().expect("responses object"); + let success = responses + .iter() + .find(|(code, _)| code.starts_with('2')) + .map(|(_, v)| v) + .unwrap_or_else(|| panic!("{method} {full} declares no 2xx response")); + + match want { + None => assert!( + success.get("content").is_none(), + "{method} {full}: the client expects an empty body but the spec now returns one" + ), + Some(name) => { + let got = success["content"]["application/json"]["schema"]["$ref"] + .as_str() + .unwrap_or_else(|| { + panic!("{method} {full}: the 2xx response no longer names a schema by $ref") + }); + assert_eq!( + got, + format!("#/components/schemas/{name}"), + "{method} {full}: the success response schema moved; the client decodes it as \ + {name}" + ); + } + } + } +} + +/// `busbar-admin config apply` sends the JSON file verbatim as a `serde_json::Value`: there is no +/// Rust type for this body, so nothing else in this crate notices if its shape changes. Pin the +/// two facts the CLI's help text and error messages promise: `config` is required, and unknown +/// top-level keys are refused (so a typo'd file fails at the gateway, not silently). +#[test] +fn config_apply_request_body_shape_is_what_the_cli_documents() { + let body = &spec()["paths"]["/api/v1/admin/config/apply"]["post"]["requestBody"]["content"] + ["application/json"]["schema"]; + assert_eq!( + body.get("additionalProperties"), + Some(&Value::Bool(false)), + "POST /config/apply stopped refusing unknown top-level keys" + ); + let required = required_of(body); + assert!( + required.contains("config"), + "POST /config/apply no longer requires a `config` key, but the CLI's --help still tells \ + operators to write {{\"config\": ..., \"providers\": ...}}" + ); + let props: BTreeSet = body["properties"] + .as_object() + .expect("properties") + .keys() + .cloned() + .collect(); + let expected: BTreeSet = ["config", "providers"] + .into_iter() + .map(str::to_string) + .collect(); + assert_eq!( + props, expected, + "the config-apply body gained or lost a top-level key; the CLI passes the file through \ + untyped, so this assertion is the only thing that notices" + ); +} diff --git a/tests/wire_shapes.rs b/tests/wire_shapes.rs new file mode 100644 index 0000000..2a1347c --- /dev/null +++ b/tests/wire_shapes.rs @@ -0,0 +1,149 @@ +// SPDX-License-Identifier: Apache-2.0 + +//! Behavioural wire-shape tests for the hand-rolled admin client. +//! +//! These lock the semantics the CLI depends on (the tri-state `allowed_pools`, the signed-token +//! credential, the minimal mint body) as opposed to `tests/openapi_conformance.rs`, which checks +//! the FIELD SETS against the committed `openapi.json`. Both are needed: conformance proves the +//! names and optionality still match the spec, these prove the values still mean what the +//! rendering code assumes. + +use busbar_admin::client::{CreateKeyReq, CreatedKeyView, InspectView, KeyView, RotatedKeyView}; + +#[test] +fn key_view_allowed_pools_null_is_all_pools() { + // null (or omitted) allowed_pools => None => "all pools". A regression to `Vec` + // would fail to deserialize null, or silently decode it as an empty (== NO pools) list. + let k: KeyView = serde_json::from_str( + r#"{"id":"vk_1","name":"n","allowed_pools":null,"state":"active","enabled":true}"#, + ) + .expect("null allowed_pools must decode"); + assert_eq!( + k.allowed_pools, None, + "null must mean all-pools (None), not []" + ); +} + +#[test] +fn key_view_allowed_pools_empty_is_no_pools() { + let k: KeyView = + serde_json::from_str(r#"{"id":"vk_1","name":"n","allowed_pools":[]}"#).unwrap(); + assert_eq!( + k.allowed_pools, + Some(Vec::new()), + "explicit [] must stay a distinct empty list (NO pools), never collapse to None" + ); +} + +#[test] +fn created_key_view_carries_signed_token_not_secret() { + // The 1.5.0 credential is `token` (+ expires_at), NOT the 1.4.x `secret`. A revert to a + // required `secret` field would fail to decode this real-shaped response. + let c: CreatedKeyView = serde_json::from_str( + r#"{"id":"vk_1","name":"n","token":"bbk_abc","expires_at":1785772871,"state":"active"}"#, + ) + .expect("token-shaped CreatedKeyView must decode"); + assert_eq!(c.token, "bbk_abc"); + assert_eq!(c.expires_at, 1785772871_u64); +} + +#[test] +fn create_key_req_omits_none_fields_and_never_sends_legacy_budget() { + // deny_unknown_fields on the server rejects any stray field. This pins the exact minimal + // body and fails if a 1.4.x budget/rpm/tpm field is ever reintroduced onto the struct. + let req = CreateKeyReq { + name: "svc".into(), + allowed_pools: None, + group: None, + parent: None, + expires_in: None, + expires_at: None, + labels: Default::default(), + issue_aws_credential: false, + }; + let v: serde_json::Value = serde_json::to_value(&req).unwrap(); + let obj = v.as_object().unwrap(); + assert_eq!( + obj.keys().collect::>(), + vec!["name"], + "an all-defaults CreateKeyReq must serialize to exactly {{name}}: any extra key is \ + either a leaked None or a reintroduced legacy field the server will 400 on" + ); + for banned in [ + "budget", + "budget_period", + "rpm_limit", + "tpm_limit", + "max_budget_cents", + ] { + assert!( + !obj.contains_key(banned), + "legacy field {banned} must never serialize" + ); + } +} + +#[test] +fn create_key_req_empty_allowed_pools_serializes_as_empty_array() { + // The --no-pools path sets Some(vec![]); it must reach the wire as `[]`, not be dropped. + let req = CreateKeyReq { + name: "svc".into(), + allowed_pools: Some(Vec::new()), + group: None, + parent: None, + expires_in: None, + expires_at: None, + labels: Default::default(), + issue_aws_credential: false, + }; + let v: serde_json::Value = serde_json::to_value(&req).unwrap(); + assert_eq!(v["allowed_pools"], serde_json::json!([])); +} + +#[test] +fn rotated_key_view_token_and_secret_are_both_optional() { + // The doc invariant is "exactly one of token or secret"; the type models both as optional + // so a token-only rotate decodes without a phantom `secret`. + let r: RotatedKeyView = + serde_json::from_str(r#"{"id":"vk_1","name":"n","token":"bbk_new"}"#).unwrap(); + assert_eq!(r.token.as_deref(), Some("bbk_new")); + assert_eq!(r.secret, None); +} + +#[test] +fn inspect_view_carries_manifest_preview_shape() { + // 1.5.2 added POST /plugins/inspect, a stateless preview whose body is the PluginSchemaView + // shape plus version/kind. This pins the fields busbar-admin renders so a future spec resync + // that drops or renames one (say `trust`, `schema_error`, or `version`) fails loudly. + let v: InspectView = serde_json::from_str( + r#"{"name":"acme-store","version":"1.2.3","kind":"store","trust":"unverified", + "source":"manifest","restart_required_default":true, + "schema":{"type":"object"},"schema_error":null}"#, + ) + .expect("inspect preview must decode"); + assert_eq!(v.name, "acme-store"); + assert_eq!(v.version.as_deref(), Some("1.2.3")); + assert_eq!(v.kind.as_deref(), Some("store")); + assert_eq!(v.trust, "unverified"); + assert_eq!(v.restart_required_default, Some(true)); + assert!(v.schema.is_some()); + assert_eq!(v.schema_error, None); +} + +#[test] +fn inspect_view_tolerates_null_kind_and_absent_version() { + // An unresolvable candidate reports kind/version/restart_required_default as null; the CLI + // must still decode (never refuse) so it can render the `trust`/`schema_error` verdict. + let v: InspectView = serde_json::from_str( + r#"{"name":"bad","kind":null,"trust":"rejected","source":"manifest","schema":null, + "schema_error":"settings_schema is not valid JSON"}"#, + ) + .expect("a rejected/unresolvable candidate must still decode"); + assert_eq!(v.kind, None); + assert_eq!(v.version, None); + assert_eq!(v.trust, "rejected"); + assert_eq!( + v.schema_error.as_deref(), + Some("settings_schema is not valid JSON") + ); +}