diff --git a/cmd/mesh-controller/acts.go b/cmd/mesh-controller/acts.go index 3989f0f8..9a8cd694 100644 --- a/cmd/mesh-controller/acts.go +++ b/cmd/mesh-controller/acts.go @@ -408,12 +408,21 @@ func issueOnAssign(ctx context.Context, open *stores, node, module string) strin return "" } user := broker.Principal{Kind: broker.KindModule, Node: node, Module: module}.Username() - if _, reads := m.OwnSecrets["broker"]; !reads { + _, reads := m.OwnSecrets["broker"] + if !reads && !runsTheProvisionerClient(m) { + // A module whose bus accounts are its processes' alone has none of its own: only the processes that + // are not issued yet are issued (issue 1226). + return processAccountsOnAssign(ctx, inv, m, node) + } + if !reads { // A module running the provisioner client alone is minted the client's account (issue 917). user = broker.Principal{Kind: broker.KindProvisioner, Node: node, Module: module}.Username() } - if _, minted, err := inv.BusUserHash(ctx, user); err != nil || minted { + if _, minted, err := inv.BusUserHash(ctx, user); err != nil { return "" + } else if minted { + // Its own account stands and is kept; a process it gained is not issued yet (issue 1226). + return processAccountsOnAssign(ctx, inv, m, node) } busAddress, err := broker.BusAddress() if err == nil { @@ -426,6 +435,16 @@ func issueOnAssign(ctx context.Context, open *stores, node, module string) strin return fmt.Sprintf("its bus credential is issued and sealed to %s, and arrives with the push", node) } +// processAccountsOnAssign issues the process accounts of an assigned module not issued yet, and says what it did. +func processAccountsOnAssign(ctx context.Context, inv *inventory.Inventory, m catalogue.Manifest, node string) string { + line, err := issueMissingProcessAccounts(ctx, inv, m, node) + if err != nil { + return fmt.Sprintf("its process accounts are not issued (%v): `module issue %s --node %s` first — "+ + "`push %s` refuses to send %s until it is", err, m.Module, node, node, m.Module) + } + return line +} + // refusedMovingTheController refuses assigning or unassigning the controller's module through a verb (review of // novox/hq ADR 0272): the node it is assigned to is the control-node, and the control-node's operator account is // the controller's terminal, so whoever moves the module chooses the terminal. At the terminal alone, as every diff --git a/cmd/mesh-controller/build.go b/cmd/mesh-controller/build.go index accc7e29..b7077f1b 100644 --- a/cmd/mesh-controller/build.go +++ b/cmd/mesh-controller/build.go @@ -768,6 +768,9 @@ func takeIn(ctx context.Context, inv *inventory.Inventory, result link.BuildResu } return manifest, kept, err } + // A bus process the new definition gains gets its account now, on each machine assigned the module, so the + // next push composes a real account (issue 1226). + issueGainedProcessAccounts(ctx, inv, manifest) // Which repository it is registered from, by the forge's own id (novox/hq ADR 0266). if err := inv.SetSourceIdentity(ctx, manifest.Module, repoID); err != nil { return manifest, kept, err diff --git a/cmd/mesh-controller/compose_perform.go b/cmd/mesh-controller/compose_perform.go index c71c4bfe..6e05f2bd 100644 --- a/cmd/mesh-controller/compose_perform.go +++ b/cmd/mesh-controller/compose_perform.go @@ -233,6 +233,12 @@ func prepareSend(ctx context.Context, open *stores, node string) ([]string, erro if !slices.Contains(in.BusMinted, "provisioner."+node) { continue } + default: + // A bus process's account nobody issued still gets the stand-in, until the composition leaves + // the module off (issue 1753): the warning names the verb (issue 1226). + if line := processAccountWarning(m, node, name, in.BusMinted); line != "" { + sayWarning(ctx, line) + } } if _, err := inv.SecretForModule(ctx, node, m.Module, name); err == nil { did = true diff --git a/cmd/mesh-controller/process_accounts.go b/cmd/mesh-controller/process_accounts.go new file mode 100644 index 00000000..43bbc84d --- /dev/null +++ b/cmd/mesh-controller/process_accounts.go @@ -0,0 +1,130 @@ +package main + +import ( + "context" + "fmt" + "io" + "os" + "strings" + + "git.novox.be/novox/mesh-sdk/go/catalogue" + + "github.com/novox/mesh-controller/internal/broker" + "github.com/novox/mesh-controller/internal/inventory" +) + +// processUser is the bus user of one process of a module on a node (novox/hq ADR 0436 §1). +func processUser(node, module, process string) string { + return broker.Principal{Kind: broker.KindProcess, Node: node, Module: module, Process: process}.Username() +} + +// processAccountWarning is the warning line for the own secret broker- of a process that declares bus +// when nobody issued its account (novox/hq ADR 0436 §2, issue 1226): "" for any other secret and for an issued +// account. The send goes on with the stand-in until the composition leaves the module off (issue 1753), so the +// line is the only loud thing about it. +func processAccountWarning(m catalogue.Manifest, node, secret string, minted []string) string { + id, isProcess := strings.CutPrefix(secret, "broker-") + if !isProcess { + return "" + } + if _, declared := m.ProcessBuses()[id]; !declared { + return "" + } + user := processUser(node, m.Module, id) + for _, u := range minted { + if u == user { + return "" + } + } + return fmt.Sprintf("%s/%s process %s: its bus account is not issued — mesh-controller.issue %s %s", + node, m.Module, id, node, m.Module) +} + +// sayWarning writes a line where the compose's other warnings go: the act's answer when it carries a writer, +// else the controller's journal. +func sayWarning(ctx context.Context, line string) { + var w io.Writer = os.Stderr + if lw, ok := ctx.Value(leftOutKey{}).(io.Writer); ok { + w = lw + } + fmt.Fprintln(w, line) +} + +// processAccountsMissing is each process of m that declares bus and whose account is not minted yet on node. +func processAccountsMissing(m catalogue.Manifest, node string, minted []string) []broker.ProcessPrincipal { + have := map[string]bool{} + for _, u := range minted { + have[u] = true + } + var out []broker.ProcessPrincipal + for _, pr := range inventory.ProcessesOf(m) { + if !have[processUser(node, m.Module, pr.Process)] { + out = append(out, pr) + } + } + return out +} + +// issueMissingProcessAccounts issues the accounts of m's bus processes on node that are not minted, and no +// other: an account already issued is rotated on purpose, never by a later delivery or assignment (issue 1226). +// It says what it issued, one line, and nothing when nothing was missing. +func issueMissingProcessAccounts(ctx context.Context, inv *inventory.Inventory, m catalogue.Manifest, node string) (string, error) { + if len(m.ProcessBuses()) == 0 || mayIssue(m) != nil { + return "", nil + } + minted, err := inv.BusMinted(ctx) + if err != nil { + return "", err + } + missing := processAccountsMissing(m, node, minted) + if len(missing) == 0 { + return "", nil + } + known, err := broker.FromEnvironment() + if err != nil { + return "", fmt.Errorf("cannot deliver a credential without knowing where the bus is: %w", err) + } + reachable, err := brokerReachableAt(ctx, inv, known, node) + if err != nil { + return "", err + } + busAddress, err := broker.BusAddress() + if err != nil { + busAddress = "" + } + var ids []string + for _, pr := range missing { + if err := issueProcess(ctx, inv, m, pr, node, busAddress, known, reachable); err != nil { + return "", err + } + ids = append(ids, pr.Process) + } + return fmt.Sprintf("its process account(s) %s issued and sealed to %s, and arrive with the push", + strings.Join(ids, ", "), node), nil +} + +// issueGainedProcessAccounts runs after a module's new definition is registered: on each machine assigned it, the +// accounts of its bus processes not yet minted are issued, so a process a delivery adds is not pushed with a +// random stand-in (issue 1226). Never fatal: the build worked and the module is registered, so a failure is said, +// and the push refuses the module until `mesh-controller.issue` is run. +func issueGainedProcessAccounts(ctx context.Context, inv *inventory.Inventory, m catalogue.Manifest) { + if len(m.ProcessBuses()) == 0 { + return + } + nodes, err := inv.Running(ctx, m.Module) + if err != nil { + fmt.Printf("%s: its machines are not read to issue its process accounts: %v\n", m.Module, err) + return + } + for _, node := range nodes { + line, err := issueMissingProcessAccounts(ctx, inv, m, node) + if err != nil { + fmt.Printf("%s on %s: its process accounts are not issued (%v): `mesh-controller.issue %s %s` first\n", + m.Module, node, err, node, m.Module) + continue + } + if line != "" { + fmt.Printf("%s on %s: %s\n", m.Module, node, line) + } + } +} diff --git a/cmd/mesh-controller/process_accounts_integration_test.go b/cmd/mesh-controller/process_accounts_integration_test.go new file mode 100644 index 00000000..b1a34582 --- /dev/null +++ b/cmd/mesh-controller/process_accounts_integration_test.go @@ -0,0 +1,135 @@ +//go:build integration + +package main + +import ( + "bytes" + "encoding/json" + "path/filepath" + "strings" + "testing" + + "git.novox.be/novox/mesh-sdk/go/catalogue" + "git.novox.be/novox/mesh-sdk/go/compose" + + "github.com/novox/mesh-controller/internal/broker" + "github.com/novox/mesh-controller/internal/inventory" + "github.com/novox/mesh-controller/internal/link" + "github.com/novox/mesh-controller/internal/testbus" +) + +func gainedAndPlain(t *testing.T) (plain, gained catalogue.Manifest) { + t.Helper() + plain, err := catalogue.ParseManifest([]byte(`{"module":"mesh-mail","version":"1", + "own-secrets":{"broker-send":{"path":"/var/lib/mesh-mail/broker-send","owner":"mesh-mail-send"}}, + "resources":[{"id":"account","type":"user","name":"mesh-mail-send"}]}`)) + if err != nil { + t.Fatal(err) + } + return plain, mailSenderWithProcess() +} + +// 1226/AC2: given an assigned module whose new definition adds a bus process, when the delivery records it +// (takeIn, the function that records a build result), the process's account is minted; and 1226/AC3: the same +// build heard again replaces nothing. +func TestAGainedProcessIsIssuedWhenTheDeliveryRecordsIt(t *testing.T) { + open := aMesh(t) + ctx := t.Context() + inv := open.inventory + + dir := t.TempDir() + if err := busCertificate([]string{"--into", dir}); err != nil { + t.Fatal(err) + } + t.Setenv(broker.AddressVar, strings.TrimPrefix(testbus.URL(t), "nats://")) + t.Setenv(broker.CertificateVar, filepath.Join(dir, "tls.crt")) + + plain, gained := gainedAndPlain(t) + register(t, open, plain) + if _, err := assign(ctx, open, "laptop", "mesh-mail"); err != nil { + t.Fatal(err) + } + if _, minted, _ := inv.BusUserHash(ctx, "laptop.mesh-mail.send"); minted { + t.Fatal("the account was minted before the module gained the process") + } + + manifest, _ := json.Marshal(gained) + build := func(id, commit string) { + t.Helper() + if _, _, err := takeIn(ctx, inv, asTheOperator(t, inv, link.BuildResult{ + ID: id, Repository: "http://forge.internal:20000/novox/mesh-mail.git", Path: "modules/mesh-mail", + Ref: "main", On: "anchor", Commit: commit, Manifest: manifest, + Source: &link.SourceOnSeat{Seat: "git", Repository: "novox/mesh-mail"}, + })); err != nil { + t.Fatal(err) + } + } + build("b-1", "aaaaaaa1111") + hash, held, err := inv.BusUserHash(ctx, "laptop.mesh-mail.send") + if err != nil || !held { + t.Fatalf("1226/AC2: recording the gained definition did not mint the process's account: held %v, %v", held, err) + } + // And the next compose carries the account, with no warning (1226/AC2). + var said bytes.Buffer + if _, err := prepareSend(leftOutSaidBy(ctx, &said), open, "laptop"); err != nil { + t.Fatalf("1226/AC2: the next compose failed: %v", err) + } + if strings.Contains(said.String(), "bus account is not issued") { + t.Fatalf("1226/AC2: the next compose warns of an account just minted: %q", said.String()) + } + // The row the compose reads is the account issueProcess accepted (origin accepted), never a value the mesh + // made for it (a stand-in). + in, err := inputFor(ctx, open) + if err != nil { + t.Fatal(err) + } + var row *compose.OwnSecretRow + for i, r := range in.OwnSecrets { + if r.Node == "laptop" && r.Module == "mesh-mail" && r.Name == "broker-send" { + row = &in.OwnSecrets[i] + } + } + if row == nil { + t.Fatal("1226/AC2: the next compose carries no row for the process's account") + } + if row.Origin != inventory.OriginAccepted { + t.Fatalf("1226/AC2: the process's account is a stand-in the mesh made (origin %q), not the issued account", row.Origin) + } + build("b-2", "bbbbbbb2222") + again, _, err := inv.BusUserHash(ctx, "laptop.mesh-mail.send") + if err != nil || again != hash { + t.Fatalf("1226/AC3: recording again replaced the issued account (%v)", err) + } +} + +// 1226/AC1 and AC4 against the store: given two modules on a machine, one with an unissued process account, when +// the machine is composed, the rest of it is composed and sent, and only the warning in the push's answer names +// the first, with `mesh-controller.issue `. +func TestAnUnissuedProcessAccountIsWarnedOfAndTheRestIsComposed(t *testing.T) { + open := aMesh(t) + ctx := t.Context() + _, gained := gainedAndPlain(t) + register(t, open, gained) + other := catalogue.Manifest{Module: "other", Version: "1", + OwnSecrets: catalogue.OwnSecrets{"token": {Path: "/var/lib/other/token"}}} + register(t, open, other) + for _, m := range []string{"mesh-mail", "other"} { + if _, err := assign(ctx, open, "laptop", m); err != nil { + t.Fatal(err) + } + } + var said bytes.Buffer + if _, err := prepareSend(leftOutSaidBy(ctx, &said), open, "laptop"); err != nil { + t.Fatalf("1226/AC4: an unissued process account held the machine: %v", err) + } + want := "laptop/mesh-mail process send: its bus account is not issued — mesh-controller.issue laptop mesh-mail" + if !strings.Contains(said.String(), want) { + t.Fatalf("1226/AC1: the push says %q, want %q", said.String(), want) + } + if strings.Contains(said.String(), "other") { + t.Fatalf("1226/AC4: the other module is warned of: %q", said.String()) + } + if _, held, err := open.inventory.ModuleSecretIfIssued(ctx, "laptop", "other", "token"); err != nil || !held { + t.Fatalf("1226/AC4: the other module's secret was not composed: held %v, %v", held, err) + } +} diff --git a/cmd/mesh-controller/process_accounts_test.go b/cmd/mesh-controller/process_accounts_test.go new file mode 100644 index 00000000..d9e3f2af --- /dev/null +++ b/cmd/mesh-controller/process_accounts_test.go @@ -0,0 +1,36 @@ +package main + +import "testing" + +// 1226/AC1: given a module whose process declares bus and has no issued account, when a machine is composed, the +// push's warning names issue, and the machine's other modules are composed (AC4: nothing is refused). +func TestAProcessAccountNobodyIssuedIsWarnedOfNamingIssue(t *testing.T) { + m := mailSenderWithProcess() + got := processAccountWarning(m, "laptop", "broker-send", nil) + want := "laptop/mesh-mail process send: its bus account is not issued — mesh-controller.issue laptop mesh-mail" + if got != want { + t.Fatalf("1226/AC1: the warning is %q, want %q", got, want) + } + // Other names are not this rule's. + if processAccountWarning(m, "laptop", "broker", nil) != "" || processAccountWarning(m, "laptop", "broker-other", nil) != "" || + processAccountWarning(m, "laptop", "token", nil) != "" { + t.Fatal("1226/AC1: a name that is no declared process's account was warned of") + } + if got := processAccountWarning(m, "laptop", "broker-send", []string{"laptop.mesh-mail.send"}); got != "" { + t.Fatalf("1226/AC3: an issued account was warned of: %q", got) + } +} + +// 1226/AC3: an account already issued is not issued again by a later delivery or assignment. +func TestAnIssuedProcessAccountIsNotIssuedAgain(t *testing.T) { + m := mailSenderWithProcess() + if got := processAccountsMissing(m, "laptop", nil); len(got) != 1 || got[0].Process != "send" { + t.Fatalf("1226/AC3: an account not issued is not missing: %v", got) + } + if got := processAccountsMissing(m, "laptop", []string{"laptop.mesh-mail.send"}); len(got) != 0 { + t.Fatalf("1226/AC3: an issued account would be issued again: %v", got) + } + if got := processAccountsMissing(m, "ace", []string{"laptop.mesh-mail.send"}); len(got) != 1 { + t.Fatalf("1226/AC3: another machine's account stands in for this one's: %v", got) + } +}