Event names are checked now, per manifest and across the catalogue
Issue 127 stood because nothing compared the two halves. Every manifest was well-formed on its own and every derivation correct on its own, and no cross-module subscription in the mesh matched anything — a subscription that matches nothing is not an error, it is silence. Two checks, because the mistake is possible at two scales. Per manifest: an event is a local name, and `module.` is refused with the name to write instead. A module emitting under what reads as another module's name is refused too, pointing at the seat, where a name outlives whoever holds it. Across the catalogue: where a consumed event's emitter is present, it must emit that event. It cannot demand a live emitter for everything — a module lives in its own repository and may be installed long before the one whose events it wants — so the rule is narrower and still catches this. It found two real dangling subscriptions the moment it ran. Wildcards were undecided and two manifests needed them: `*` is one name and `**` is the rest, spelled the mesh's way and derived to `>` here and `#` on the old bus. A manifest naming either would stop being true when the wire changed, which is the whole reason names are local. And the field documentation taught the old form, examples included — which is why the drift was uniform across 37 manifests rather than scattered. Nobody was guessing; everybody followed the comment.
This commit is contained in:
@@ -0,0 +1,161 @@
|
||||
package catalogue
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"regexp"
|
||||
"strings"
|
||||
)
|
||||
|
||||
// What a module may call an event, and what a consumer may ask for.
|
||||
//
|
||||
// A module names an event **locally**: `order.placed`, not a subject and not a routing key
|
||||
// (design 29 §1). A consumer names the emitter and the event: `billing.order.placed`. The mesh
|
||||
// derives the subject from those, so reorganising the subject space leaves every manifest correct.
|
||||
//
|
||||
// **Nothing checked this until every manifest in the catalogue was wrong the same way**
|
||||
// (novox/hq 04-ISSUES/127). All thirty-seven kept the old bus's routing key —
|
||||
// `module.<module>.<verb>` — which the derivation read as "a module called `module`", so every
|
||||
// cross-module subscription in the mesh pointed at a namespace nobody publishes to. Nothing failed:
|
||||
// the services started and none of them reacted. The documentation on these fields taught the old
|
||||
// form too, which is why the drift was uniform rather than scattered.
|
||||
|
||||
// eventName is one name in a local event: lower-case, and no wildcard.
|
||||
var eventName = regexp.MustCompile(`^[a-z0-9][a-z0-9-]*$`)
|
||||
|
||||
// The wildcards a consumer may use, spelled the mesh's way and derived to whatever the transport
|
||||
// spells them as.
|
||||
//
|
||||
// **A manifest holds no transport token**, which is the whole point of naming locally: the bus the
|
||||
// mesh runs on today spells these `*` and `#`, and the one being built spells them `*` and `>`. A
|
||||
// manifest that said either would be a manifest that stopped being true when the wire changed.
|
||||
const (
|
||||
// OneName stands for exactly one name.
|
||||
OneName = "*"
|
||||
// TheRest stands for one or more names, and may only come last.
|
||||
TheRest = "**"
|
||||
)
|
||||
|
||||
// EventProblems is what is wrong with a manifest's events.
|
||||
//
|
||||
// Refused at registration, because the alternative is a module that installs, starts, connects and
|
||||
// reacts to nothing — and every log line says it is fine.
|
||||
func EventProblems(m Manifest) []string {
|
||||
var problems []string
|
||||
|
||||
for _, e := range m.Emits {
|
||||
if was, stale := staleEventForm(e, m.Module); stale {
|
||||
problems = append(problems, fmt.Sprintf(
|
||||
"%s emits %q, which is the old bus's routing key. An event is named locally now, so "+
|
||||
"write %q — the mesh derives the subject (novox/hq design 29 §1)",
|
||||
m.Module, was, strings.TrimPrefix(was, "module."+m.Module+".")))
|
||||
continue
|
||||
}
|
||||
if strings.HasPrefix(e, "module.") {
|
||||
problems = append(problems, fmt.Sprintf(
|
||||
"%s emits %q: `module.` is reserved, because it is how the old bus spelled a "+
|
||||
"routing key and an event named that way derives into a namespace nobody owns",
|
||||
m.Module, e))
|
||||
continue
|
||||
}
|
||||
if err := localName(e); err != nil {
|
||||
problems = append(problems, fmt.Sprintf("%s emits %q: %v", m.Module, e, err))
|
||||
continue
|
||||
}
|
||||
// **Its own name, never another's.** The bus enforces that a namespace belongs to the module
|
||||
// it is named for, so an event named for somebody else cannot be published at all. If the
|
||||
// event is about a role rather than about this module, it belongs on the seat: a name that
|
||||
// is stable across whoever fills it (04-ISSUES/127).
|
||||
if first, _, split := strings.Cut(e, "."); split && isAModuleNameOtherThan(first, m.Module) {
|
||||
problems = append(problems, fmt.Sprintf(
|
||||
"%s emits %q, which reads as another module's event. A module publishes under its "+
|
||||
"own name only. If this is about a role rather than about %s, declare it on that "+
|
||||
"seat, where the name survives the holder changing",
|
||||
m.Module, e, m.Module))
|
||||
}
|
||||
}
|
||||
|
||||
for _, c := range m.Consumes {
|
||||
if strings.HasPrefix(c, "module.") {
|
||||
problems = append(problems, fmt.Sprintf(
|
||||
"%s consumes %q, which is the old bus's pattern. A consumed event names its emitter "+
|
||||
"and the event: write %q", m.Module, c, strings.TrimPrefix(c, "module.")))
|
||||
continue
|
||||
}
|
||||
if c == "#" {
|
||||
problems = append(problems, fmt.Sprintf(
|
||||
"%s consumes %q, which is the old bus's wildcard for everything. Write %q",
|
||||
m.Module, c, TheRest))
|
||||
continue
|
||||
}
|
||||
if err := consumePattern(c); err != nil {
|
||||
problems = append(problems, fmt.Sprintf("%s consumes %q: %v", m.Module, c, err))
|
||||
}
|
||||
}
|
||||
return problems
|
||||
}
|
||||
|
||||
// staleEventForm says an emitted name is this module's own old routing key, and what it was.
|
||||
func staleEventForm(event, module string) (string, bool) {
|
||||
return event, module != "" && strings.HasPrefix(event, "module."+module+".")
|
||||
}
|
||||
|
||||
// isAModuleNameOtherThan says a first token names some module of this mesh that is not this one.
|
||||
//
|
||||
// Only the mesh's own seats and the catalogue could answer this properly, and neither is reachable
|
||||
// from a parser given one manifest. So this catches the case that actually happened — a name that
|
||||
// is a *provision* the mesh defines, which is where "another module's event" comes from in practice
|
||||
// — and the whole-catalogue check catches the rest.
|
||||
func isAModuleNameOtherThan(first, module string) bool {
|
||||
if first == module || first == "" {
|
||||
return false
|
||||
}
|
||||
if _, isASeat := SeatNamed(first); isASeat {
|
||||
return true
|
||||
}
|
||||
if _, isASeat := SeatDelivering(first); isASeat {
|
||||
return true
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// localName checks one event name: dot-separated names, no wildcards, nothing else.
|
||||
func localName(event string) error {
|
||||
if event == "" {
|
||||
return fmt.Errorf("an event needs a name")
|
||||
}
|
||||
for _, part := range strings.Split(event, ".") {
|
||||
if part == OneName || part == TheRest {
|
||||
return fmt.Errorf("an emitted event names one event, so it carries no wildcard")
|
||||
}
|
||||
if !eventName.MatchString(part) {
|
||||
return fmt.Errorf("%q is not a usable name: lower-case letters, digits and dashes", part)
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// consumePattern checks a consumed pattern: the emitter, then the event, with wildcards.
|
||||
func consumePattern(pattern string) error {
|
||||
if pattern == "" {
|
||||
return fmt.Errorf("a consumed event needs an emitter and an event")
|
||||
}
|
||||
parts := strings.Split(pattern, ".")
|
||||
for i, part := range parts {
|
||||
switch {
|
||||
case part == TheRest:
|
||||
if i != len(parts)-1 {
|
||||
return fmt.Errorf("%q stands for the rest of a name, so nothing may follow it", TheRest)
|
||||
}
|
||||
case part == OneName:
|
||||
case !eventName.MatchString(part):
|
||||
return fmt.Errorf("%q is not a usable name: lower-case letters, digits and dashes", part)
|
||||
}
|
||||
}
|
||||
// `**` alone is every event from every module, which the audit logger wants and says plainly.
|
||||
if len(parts) == 1 && parts[0] != TheRest {
|
||||
return fmt.Errorf(
|
||||
"%q names an emitter and no event. Write <emitter>.<event>, or %q for every event",
|
||||
pattern, TheRest)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
@@ -176,15 +176,29 @@ type Manifest struct {
|
||||
// Requires are names that must be provided by something assigned to the same node.
|
||||
Requires []string `json:"requires,omitempty"`
|
||||
|
||||
// Emits are the event types this module publishes onto the broker — dotted topic keys, e.g.
|
||||
// "module.umami.site.created". Declared so the mesh knows the event graph; events are
|
||||
// provisioning's lighter sibling — 1:many and broadcast, no credential (novox/hq ADR 0041).
|
||||
// Emits are the events this module publishes, named **locally**: `order.placed`, not a subject
|
||||
// and not a routing key. The mesh derives where it lands (design 29 §1), so reorganising the
|
||||
// subject space leaves this manifest correct. Events are provisioning's lighter sibling — 1:many
|
||||
// and broadcast, no credential (novox/hq ADR 0041).
|
||||
//
|
||||
// A module publishes under its own name only. If the event is about a *role* rather than about
|
||||
// this module, it belongs on that seat, where the name outlives whoever holds it.
|
||||
//
|
||||
// **This said "dotted topic keys, e.g. module.umami.site.created" until 04-ISSUES/127**, which
|
||||
// is the old bus's routing key, and is why every manifest in the catalogue had the same mistake:
|
||||
// nobody was guessing, everybody followed this comment.
|
||||
Emits []string `json:"emits,omitempty"`
|
||||
|
||||
// Consumes are the event patterns this module subscribes to — topic patterns over module,
|
||||
// mesh and node events alike, e.g. "node.*.joined" or "#" (the audit logger). The runtime
|
||||
// wires the subscription; the module ships the handler. A Consumes for an event nothing on
|
||||
// the mesh Emits is a dangling edge.
|
||||
// Consumes are the events this module reacts to, each naming its emitter and the event:
|
||||
// `billing.order.placed`. `*` stands for one name and `**` for the rest, so `*.download.completed`
|
||||
// is that event from any module and `**` is every event in the mesh.
|
||||
//
|
||||
// Spelled the mesh's way rather than the wire's, for the reason Emits is: the bus the mesh runs
|
||||
// on today spells these `*` and `#`, the one being built spells them `*` and `>`, and a manifest
|
||||
// naming either would stop being true when the wire changed.
|
||||
//
|
||||
// The runtime wires the subscription; the module ships the handler. A Consumes for an event
|
||||
// nothing on the mesh Emits is a dangling edge.
|
||||
Consumes []string `json:"consumes,omitempty"`
|
||||
|
||||
// Claims are singular resources. Two modules claiming one thing within a scope cannot both
|
||||
@@ -999,6 +1013,10 @@ func ParseManifest(raw []byte) (Manifest, error) {
|
||||
problems = append(problems, fmt.Sprintf("%s requires itself", m.Module))
|
||||
}
|
||||
}
|
||||
// What it may call an event, and what it may ask to hear (events.go). Checked here because a
|
||||
// module whose event names are wrong installs, starts, connects and reacts to nothing, with
|
||||
// every log line saying it is fine (novox/hq 04-ISSUES/127).
|
||||
problems = append(problems, EventProblems(m)...)
|
||||
wellFormed := true
|
||||
for _, c := range m.Claims {
|
||||
if !name.MatchString(c.Name) {
|
||||
|
||||
@@ -71,3 +71,51 @@ func TestNoManifestContainsASubject(t *testing.T) {
|
||||
}
|
||||
t.Logf("%d manifests hold no subject", checked)
|
||||
}
|
||||
|
||||
// **Every module's event names are what design 29 says, across the whole catalogue.**
|
||||
//
|
||||
// The rule above holds by construction and turned out to be weaker than it reads: a manifest holds
|
||||
// no subject, and every manifest in the catalogue still held the old bus's routing key, which
|
||||
// derives into a namespace nobody owns (novox/hq 04-ISSUES/127). Nothing failed — the services
|
||||
// started and none of them reacted. This is the check that was missing.
|
||||
func TestEveryManifestsEventNamesAreLocal(t *testing.T) {
|
||||
manifests := theCatalogue(t)
|
||||
|
||||
var problems []string
|
||||
for _, m := range manifests {
|
||||
problems = append(problems, EventProblems(m)...)
|
||||
}
|
||||
if len(problems) > 0 {
|
||||
t.Fatalf("the catalogue holds %d event name(s) the mesh would derive wrongly:\n %s",
|
||||
len(problems), strings.Join(problems, "\n "))
|
||||
}
|
||||
}
|
||||
|
||||
// theCatalogue is every manifest beside this checkout, parsed the way registration parses one.
|
||||
func theCatalogue(t *testing.T) []Manifest {
|
||||
t.Helper()
|
||||
root := filepath.Join("..", "..", "..", "mesh-catalog", "modules")
|
||||
entries, err := os.ReadDir(root)
|
||||
if err != nil {
|
||||
t.Skipf("catalogue sibling not present: %v", err)
|
||||
}
|
||||
var out []Manifest
|
||||
for _, e := range entries {
|
||||
if !e.IsDir() {
|
||||
continue
|
||||
}
|
||||
raw, err := os.ReadFile(filepath.Join(root, e.Name(), "module.json"))
|
||||
if err != nil {
|
||||
continue
|
||||
}
|
||||
var m Manifest
|
||||
if err := json.Unmarshal(raw, &m); err != nil {
|
||||
t.Fatalf("%s: %v", e.Name(), err)
|
||||
}
|
||||
out = append(out, m)
|
||||
}
|
||||
if len(out) == 0 {
|
||||
t.Skip("no manifests found beside this checkout")
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user