From 0d1f2a41529e7d44e2cdde374ef21c88497d87c3 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 21 Sep 2026 23:58:36 +0200 Subject: [PATCH] Review: module issue's pre-check factored and tested; a pair delivery for a requirement the module keeps no secret for is refused; builder issue's usage says the module comes first --- cmd/mesh-controller/main.go | 3 ++- cmd/mesh-controller/modules.go | 20 ++++++++++++++------ cmd/mesh-controller/modules_test.go | 26 ++++++++++++++++++++++++++ internal/inventory/secrets.go | 3 +++ internal/inventory/secrets_test.go | 19 ++++++++++++++++--- 5 files changed, 61 insertions(+), 10 deletions(-) create mode 100644 cmd/mesh-controller/modules_test.go diff --git a/cmd/mesh-controller/main.go b/cmd/mesh-controller/main.go index 70b6bfb..f03cd59 100644 --- a/cmd/mesh-controller/main.go +++ b/cmd/mesh-controller/main.go @@ -168,7 +168,8 @@ func usage() { build [--ref R] have a build machine build it, and record what came out build --behind build every module the mesh holds older than its source builds [] what has been built lately, and what came of it - builder issue a broker account for a build machine, scoped to build work + builder issue a broker account for a build machine, scoped to build work, + delivered as the builder module's broker secret (module add it first) licence add|list|use|key model access, under the name a person calls it licence manager the node that holds a refreshable licence's refresh token licence refresh mint a new access token and seal it to every holder diff --git a/cmd/mesh-controller/modules.go b/cmd/mesh-controller/modules.go index ffb6770..1d0d72d 100644 --- a/cmd/mesh-controller/modules.go +++ b/cmd/mesh-controller/modules.go @@ -253,12 +253,8 @@ func moduleCommand(ctx context.Context, args []string) error { if !ok { return fmt.Errorf("this mesh knows no module %q; `module add` it first", module) } - // The account is delivered as the module's own secret named broker; a module that declares - // none has nothing to read it with, and an account nothing reads is an orphan on the bus - // (novox/hq 04-ISSUES/078). Said before the account is made, not after. - if _, reads := m.OwnSecrets["broker"]; !reads { - return fmt.Errorf("%s declares no own secret named broker, so there is nowhere to deliver "+ - "an account; a module that speaks on the bus declares \"own-secrets\": {\"broker\": }", module) + if err := mayIssue(m); err != nil { + return err } management, err := broker.ManagementFromEnvironment() @@ -559,3 +555,15 @@ func brokerReachableAt(ctx context.Context, inv *inventory.Inventory, known brok } return net.JoinHostPort(brokerAt, port), nil } + +// mayIssue says whether a module can be given a broker account. The account is delivered as the +// module's own secret named broker; a module that declares none has nothing to read it with, and +// an account nothing reads is an orphan on the bus (novox/hq 04-ISSUES/078). Said before the +// account is made, not after. +func mayIssue(m catalogue.Manifest) error { + if _, reads := m.OwnSecrets["broker"]; !reads { + return fmt.Errorf("%s declares no own secret named broker, so there is nowhere to deliver "+ + "an account; a module that speaks on the bus declares \"own-secrets\": {\"broker\": }", m.Module) + } + return nil +} diff --git a/cmd/mesh-controller/modules_test.go b/cmd/mesh-controller/modules_test.go new file mode 100644 index 0000000..edfc937 --- /dev/null +++ b/cmd/mesh-controller/modules_test.go @@ -0,0 +1,26 @@ +package main + +import ( + "strings" + "testing" + + "github.com/novox/mesh-controller/internal/catalogue" +) + +// `module issue` makes a broker account and delivers it as the module's own secret named broker. +// A module that declares none is refused before the account exists, so the bus never carries an +// account nothing reads (novox/hq 04-ISSUES/078). +func TestAModuleWithNoBrokerSecretCannotBeIssued(t *testing.T) { + err := mayIssue(catalogue.Manifest{Module: "step-ca", OwnSecrets: map[string]string{"password": "/run/password"}}) + if err == nil { + t.Fatal("a module with no broker own secret was issued an account") + } + for _, want := range []string{"step-ca", "broker", "own-secrets"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("the refusal does not say %q: %v", want, err) + } + } + if err := mayIssue(catalogue.Manifest{Module: "redis", OwnSecrets: map[string]string{"broker": "/run/broker"}}); err != nil { + t.Errorf("a module declaring its broker secret was refused: %v", err) + } +} diff --git a/internal/inventory/secrets.go b/internal/inventory/secrets.go index 235e929..ae96644 100644 --- a/internal/inventory/secrets.go +++ b/internal/inventory/secrets.go @@ -159,6 +159,9 @@ func (i *Inventory) AcceptSecretForPair(ctx context.Context, name, consumer, con return fmt.Errorf("%s does not keep %q for %q; it keeps: %s", consumerModule, local, name, orNone(sortedNames(locals))) } + } else if m.Secrets[name] == "" { + return fmt.Errorf("%s requires %q but keeps no secret for it, so a delivered value would sit unread; "+ + "it keeps secrets for: %s", consumerModule, name, orNone(sortedNames(m.Secrets))) } else if local != "" { return fmt.Errorf("%s keeps one secret for %q, not several; drop --local", consumerModule, name) } diff --git a/internal/inventory/secrets_test.go b/internal/inventory/secrets_test.go index ac4942c..6cbcc69 100644 --- a/internal/inventory/secrets_test.go +++ b/internal/inventory/secrets_test.go @@ -55,7 +55,8 @@ func twoNodesWithKeys(t *testing.T) (*Inventory, context.Context) { // Each requires what a test delivers to it: a pair credential is refused for a // requirement the module does not have (novox/hq 04-ISSUES/078). if err := inv.RegisterModule(ctx, catalogue.Manifest{Module: m, Version: "1", - Requires: []string{"secret", "postgres-database"}}, Source{}); err != nil { + Requires: []string{"secret", "postgres-database", "object-store"}, + Secrets: map[string]string{"secret": "/run/secret", "postgres-database": "/run/pg"}}, Source{}); err != nil { t.Fatal(err) } } @@ -748,11 +749,23 @@ func TestADeliveredSecretIsRefusedUnderANameTheModuleDoesNotDeclare(t *testing.T func TestADeliveredPairCredentialIsRefusedForARequirementTheModuleDoesNotHave(t *testing.T) { inv, ctx := twoNodesWithKeys(t) - // gitea requires secret and postgres-database (the fixture); not an object store. - err := inv.AcceptSecretForPair(ctx, "object-store", "consumer", "gitea", "provider", "", "hunter2") + // gitea requires secret, postgres-database and object-store (the fixture); it keeps a secret + // for the first two only, and requires no cache at all. + err := inv.AcceptSecretForPair(ctx, "redis-cache", "consumer", "gitea", "provider", "", "hunter2") if err == nil || !strings.Contains(err.Error(), "postgres-database") { t.Fatalf("a pair credential for a requirement the module does not have was not refused naming what it requires: %v", err) } + err = inv.AcceptSecretForPair(ctx, "object-store", "consumer", "gitea", "provider", "", "hunter2") + if err == nil || !strings.Contains(err.Error(), "keeps no secret") { + t.Fatalf("a pair credential for a requirement the module keeps no secret for was not refused: %v", err) + } + err = inv.AcceptSecretForPair(ctx, "secret", "consumer", "gitea", "provider", "extra", "hunter2") + if err == nil || !strings.Contains(err.Error(), "drop --local") { + t.Fatalf("a local named where the module keeps one secret was not refused: %v", err) + } + if err := inv.AcceptSecretForPair(ctx, "secret", "consumer", "gitea", "provider", "", "hunter2"); err != nil { + t.Fatalf("a delivery for a requirement the module keeps one secret for was refused: %v", err) + } // A module keeping several secrets for one requirement (ADR 0094) takes a delivery only // under one of its locals. m := catalogue.Manifest{Module: "mailu", Version: "1", Requires: []string{"secret"},