diff --git a/cmd/mesh-controller/process_issue_integration_test.go b/cmd/mesh-controller/process_issue_integration_test.go index bd17f106..be013ced 100644 --- a/cmd/mesh-controller/process_issue_integration_test.go +++ b/cmd/mesh-controller/process_issue_integration_test.go @@ -4,48 +4,24 @@ package main import ( "context" - "crypto/ecdsa" - "crypto/elliptic" - "crypto/rand" - "crypto/x509" - "crypto/x509/pkix" - "encoding/pem" - "math/big" - "os" - "path/filepath" "strings" "testing" - "time" "github.com/novox/mesh-controller/internal/broker" ) -// aTestBrokerCertificate puts a real self-signed certificate on disk, so broker.FromEnvironment can read a -// fingerprint from it without a live bus — the same shape internal/broker's own writeCertificate makes. -func aTestBrokerCertificate(t *testing.T) string { - t.Helper() - key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) - if err != nil { - 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 3, fault B (1181): the test asserted a literal url, while issueOnTheNewBus works out the +// machine's reachable address from the catalogue's network records (brokerReachableAt), not from this +// process's environment — the real try found "nats://anchor.internal:5671", not the literal guessed here. +// Fixed by calling issueProcess directly, with the known broker and reachable address given to it, exactly +// as issued_to_vault_test.go's issue/issueWith and issueProvisionerClient are called directly; the url +// asserted below is one of those two inputs, never a separate guess. +// // 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 -// finding 1: no own secret `broker` at all, its tools served by the tool runner), and issuing it again is -// the path every bus account takes (`rollout mint`'s new KindProcess case, and a re-run of `module issue`): -// it rotates the password, and the old hash no longer matches. +// round 1 finding 1: no own secret `broker` at all, its tools served by the tool runner), and issuing it +// again is the path every bus account takes (`rollout mint`'s KindProcess case, and a re-run of `module +// issue`): it rotates the password, and the old hash no longer matches. func TestModuleIssueMintsAndDeliversAProcesssAccount(t *testing.T) { open := aMesh(t) ctx := t.Context() @@ -60,12 +36,20 @@ func TestModuleIssueMintsAndDeliversAProcesssAccount(t *testing.T) { vaultAskerForIssue = func(context.Context) vaultAsker { return vault.ask } t.Cleanup(func() { vaultAskerForIssue = func(context.Context) vaultAsker { return newStandInVault(nil).ask } }) - t.Setenv(broker.CertificateVar, aTestBrokerCertificate(t)) - t.Setenv(broker.AddressVar, "nats://novox.example:4222") - - if err := issueOnTheNewBus(ctx, inv, m, "laptop", ""); err != nil { - t.Fatal(err) + pr, found := processPrincipalFor(m, "send") + if !found { + t.Fatal("mesh-mail declares no process send") } + 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() if user != "laptop.mesh-mail.send" { 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) } - // 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"] - for _, want := range []string{`"url":"nats://novox.example:4222"`, `"node":"laptop"`, `"module":"mesh-mail"`, - `"user":"laptop.mesh-mail.send"`, `"password"`} { + for _, want := range []string{`"url":"nats://` + reachable + `"`, `"fingerprint":"ab"`, `"node":"laptop"`, + `"module":"mesh-mail"`, `"user":"laptop.mesh-mail.send"`, `"password"`} { if !strings.Contains(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"`) { 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 - // there and for this fixture in TestAProcessesOwnSecretIsOwnedByItsOwnAccount. + // The file's own owner (mesh-mail-send, review round 1 finding 7's area) is 1170's mechanism, proven + // 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. - if err := issueOnTheNewBus(ctx, inv, m, "laptop", ""); err != nil { - t.Fatal(err) - } + issue() hash2, held, err := inv.BusUserHash(ctx, user) if err != nil || !held { t.Fatalf("the process's account is gone after a re-issue: held %v, %v", held, err)