mesh-controller.try fault B: derive the test's expected url from what issueProcess is given, not a literal (issue 1181)
mesh/merge-gate head 4104d49ec8b9 merged into trunk 1490c3b0756a — pass: builds build-agent, mesh-controller, route-proxy → ace, g14, novox, shanks; no bus…
mesh/repo-check head 4104d49ec8b9 merged into trunk 1490c3b0756a — pass: its merge-check.sh passed
mesh/delivery delivered

The test set MESH_BROKER_ADDRESS to a literal and asserted it, but
issueOnTheNewBus works out the machine's reachable address from the
catalogue's network records, not from this process's environment — the
real try found a different, correct address. Calls issueProcess
directly now, with a known broker and reachable address of its own
choosing, exactly as issued_to_vault_test.go calls issueWith and
issueProvisionerClient directly; the asserted url is that same input.
No production code changed — issueOnTheNewBus reads the right address.
This commit is contained in:
jochen
2026-10-19 03:35:57 +02:00
parent 1991fe931b
commit 4104d49ec8
@@ -4,48 +4,24 @@ package main
import ( import (
"context" "context"
"crypto/ecdsa"
"crypto/elliptic"
"crypto/rand"
"crypto/x509"
"crypto/x509/pkix"
"encoding/pem"
"math/big"
"os"
"path/filepath"
"strings" "strings"
"testing" "testing"
"time"
"github.com/novox/mesh-controller/internal/broker" "github.com/novox/mesh-controller/internal/broker"
) )
// aTestBrokerCertificate puts a real self-signed certificate on disk, so broker.FromEnvironment can read a // review round 3, fault B (1181): the test asserted a literal url, while issueOnTheNewBus works out the
// fingerprint from it without a live bus — the same shape internal/broker's own writeCertificate makes. // machine's reachable address from the catalogue's network records (brokerReachableAt), not from this
func aTestBrokerCertificate(t *testing.T) string { // process's environment — the real try found "nats://anchor.internal:5671", not the literal guessed here.
t.Helper() // Fixed by calling issueProcess directly, with the known broker and reachable address given to it, exactly
key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) // as issued_to_vault_test.go's issue/issueWith and issueProvisionerClient are called directly; the url
if err != nil { // asserted below is one of those two inputs, never a separate guess.
t.Fatal(err) //
}
template := x509.Certificate{SerialNumber: big.NewInt(1), Subject: pkix.Name{CommonName: "mesh-broker"},
NotBefore: time.Now().Add(-time.Hour), NotAfter: time.Now().Add(24 * time.Hour)}
der, err := x509.CreateCertificate(rand.Reader, &template, &template, &key.PublicKey, key)
if err != nil {
t.Fatal(err)
}
path := filepath.Join(t.TempDir(), "tls.crt")
if err := os.WriteFile(path, pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}), 0o644); err != nil {
t.Fatal(err)
}
return path
}
// review round 1, findings 3 and 4 (1181/AC1, AC2): module issue mints and delivers a process's own // review round 1, findings 3 and 4 (1181/AC1, AC2): module issue mints and delivers a process's own
// account against the store — no bus to dial, busAddress "" — for mesh-mail's real manifest shape (review // account against the store — no bus to dial, busAddress "" — for mesh-mail's real manifest shape (review
// finding 1: no own secret `broker` at all, its tools served by the tool runner), and issuing it again is // round 1 finding 1: no own secret `broker` at all, its tools served by the tool runner), and issuing it
// the path every bus account takes (`rollout mint`'s new KindProcess case, and a re-run of `module issue`): // again is the path every bus account takes (`rollout mint`'s KindProcess case, and a re-run of `module
// it rotates the password, and the old hash no longer matches. // issue`): it rotates the password, and the old hash no longer matches.
func TestModuleIssueMintsAndDeliversAProcesssAccount(t *testing.T) { func TestModuleIssueMintsAndDeliversAProcesssAccount(t *testing.T) {
open := aMesh(t) open := aMesh(t)
ctx := t.Context() ctx := t.Context()
@@ -60,12 +36,20 @@ func TestModuleIssueMintsAndDeliversAProcesssAccount(t *testing.T) {
vaultAskerForIssue = func(context.Context) vaultAsker { return vault.ask } vaultAskerForIssue = func(context.Context) vaultAsker { return vault.ask }
t.Cleanup(func() { vaultAskerForIssue = func(context.Context) vaultAsker { return newStandInVault(nil).ask } }) t.Cleanup(func() { vaultAskerForIssue = func(context.Context) vaultAsker { return newStandInVault(nil).ask } })
t.Setenv(broker.CertificateVar, aTestBrokerCertificate(t)) pr, found := processPrincipalFor(m, "send")
t.Setenv(broker.AddressVar, "nats://novox.example:4222") if !found {
t.Fatal("mesh-mail declares no process send")
if err := issueOnTheNewBus(ctx, inv, m, "laptop", ""); err != nil {
t.Fatal(err)
} }
known := broker.Broker{Fingerprint: "ab"}
const reachable = "10.0.0.1:4222"
issue := func() {
t.Helper()
if err := issueProcess(ctx, inv, m, pr, "laptop", "", known, reachable); err != nil {
t.Fatal(err)
}
}
issue()
user := broker.Principal{Kind: broker.KindProcess, Node: "laptop", Module: "mesh-mail", Process: "send"}.Username() user := broker.Principal{Kind: broker.KindProcess, Node: "laptop", Module: "mesh-mail", Process: "send"}.Username()
if user != "laptop.mesh-mail.send" { if user != "laptop.mesh-mail.send" {
t.Fatalf("the process's username: %q", user) t.Fatalf("the process's username: %q", user)
@@ -75,10 +59,11 @@ func TestModuleIssueMintsAndDeliversAProcesssAccount(t *testing.T) {
t.Fatalf("the process's account was not minted: held %v, %v", held, err) t.Fatalf("the process's account was not minted: held %v, %v", held, err)
} }
// 1181/AC1: accepted with the provisioner client's JSON, and handed to the vault whole. // 1181/AC1: accepted with the provisioner client's JSON, and handed to the vault whole — the url and
// the fingerprint being exactly what issueProcess was given above, not a guess at what it computes.
taken := vault.taken["accept"] taken := vault.taken["accept"]
for _, want := range []string{`"url":"nats://novox.example:4222"`, `"node":"laptop"`, `"module":"mesh-mail"`, for _, want := range []string{`"url":"nats://` + reachable + `"`, `"fingerprint":"ab"`, `"node":"laptop"`,
`"user":"laptop.mesh-mail.send"`, `"password"`} { `"module":"mesh-mail"`, `"user":"laptop.mesh-mail.send"`, `"password"`} {
if !strings.Contains(taken, want) { if !strings.Contains(taken, want) {
t.Errorf("the vault was handed %q, without %s", taken, want) t.Errorf("the vault was handed %q, without %s", taken, want)
} }
@@ -86,14 +71,12 @@ func TestModuleIssueMintsAndDeliversAProcesssAccount(t *testing.T) {
if !strings.Contains(vault.said(), `"name":"broker-send"`) { if !strings.Contains(vault.said(), `"name":"broker-send"`) {
t.Errorf("the vault was not told the secret's name broker-send: %s", vault.said()) t.Errorf("the vault was not told the secret's name broker-send: %s", vault.said())
} }
// The file's own owner (mesh-mail-send, review finding 7-adjacent) is 1170's mechanism, proven generically // The file's own owner (mesh-mail-send, review round 1 finding 7's area) is 1170's mechanism, proven
// there and for this fixture in TestAProcessesOwnSecretIsOwnedByItsOwnAccount. // generically there and for this fixture in TestAProcessesOwnSecretIsOwnedByItsOwnAccount.
// 1181/AC2: issued again — the path `rollout mint`'s new KindProcess case and a re-run of `module issue` // 1181/AC2: issued again — the path `rollout mint`'s KindProcess case and a re-run of `module issue`
// both take — rotates the password; the old hash no longer matches what is recorded. // both take — rotates the password; the old hash no longer matches what is recorded.
if err := issueOnTheNewBus(ctx, inv, m, "laptop", ""); err != nil { issue()
t.Fatal(err)
}
hash2, held, err := inv.BusUserHash(ctx, user) hash2, held, err := inv.BusUserHash(ctx, user)
if err != nil || !held { if err != nil || !held {
t.Fatalf("the process's account is gone after a re-issue: held %v, %v", held, err) t.Fatalf("the process's account is gone after a re-issue: held %v, %v", held, err)