diff --git a/cmd/mesh-controller/build_source_integration_test.go b/cmd/mesh-controller/build_source_integration_test.go index 19ab0247..eadf9ade 100644 --- a/cmd/mesh-controller/build_source_integration_test.go +++ b/cmd/mesh-controller/build_source_integration_test.go @@ -317,7 +317,7 @@ func TestAModuleMovesOnTheOperatorsApproveOfTheMergeThatMovedIt(t *testing.T) { keptDeliveryMerge = func(_ context.Context, owner, repo string, number int, head string) (deliveryMerge, bool, error) { return a.merge, !c.unkept && owner == "novox" && repo == "mesh-host" && number == 7 && head == a.merge.HeadSHA, nil } - routerAskOf = func(_ context.Context, _ catalogueReader, id string) (*asks.Ask, *asks.Warrant, error) { + routerAskOf = func(_ context.Context, _ catalogueReader, _, id string) (*asks.Ask, *asks.Warrant, error) { if id != a.merge.AskID { t.Errorf("%s: the router was asked for %q", c.what, id) } diff --git a/cmd/mesh-controller/first_build_ask_integration_test.go b/cmd/mesh-controller/first_build_ask_integration_test.go index b8ee5ce1..8f0a7ac8 100644 --- a/cmd/mesh-controller/first_build_ask_integration_test.go +++ b/cmd/mesh-controller/first_build_ask_integration_test.go @@ -51,7 +51,7 @@ func TestTheApprovedFirstBuildPassesTheGuardOfAVerbAndIsQueued(t *testing.T) { keptDeliveryMerge = func(context.Context, string, string, int, string) (deliveryMerge, bool, error) { return a.merge, true, nil } - routerAskOf = func(context.Context, catalogueReader, string) (*asks.Ask, *asks.Warrant, error) { + routerAskOf = func(context.Context, catalogueReader, string, string) (*asks.Ask, *asks.Warrant, error) { return &a.ask, &a.warrant, nil } t.Cleanup(func() { keptDeliveryMerge, routerAskOf = wasKept, wasAsk }) diff --git a/cmd/mesh-controller/first_builds_test.go b/cmd/mesh-controller/first_builds_test.go index 7a6b2d14..b0c4d009 100644 --- a/cmd/mesh-controller/first_builds_test.go +++ b/cmd/mesh-controller/first_builds_test.go @@ -58,7 +58,7 @@ func TestAFirstMergeIntoANewRepositoryAsksItsModulesBuildsOnlyOnTheApprove(t *te keptDeliveryMerge = func(context.Context, string, string, int, string) (deliveryMerge, bool, error) { return a.merge, !c.unkept, nil } - routerAskOf = func(context.Context, catalogueReader, string) (*asks.Ask, *asks.Warrant, error) { + routerAskOf = func(context.Context, catalogueReader, string, string) (*asks.Ask, *asks.Warrant, error) { return &a.ask, &a.warrant, nil } dirs, _, err := firstBuildsOf(t.Context(), nil, m, entries, nil) diff --git a/cmd/mesh-controller/moved.go b/cmd/mesh-controller/moved.go index 361582b6..3e9d95da 100644 --- a/cmd/mesh-controller/moved.go +++ b/cmd/mesh-controller/moved.go @@ -13,6 +13,7 @@ import ( "github.com/nats-io/nats.go/jetstream" "git.novox.be/novox/mesh-sdk/go/asks" + sdkdelivery "git.novox.be/novox/mesh-sdk/go/delivery" "github.com/novox/mesh-controller/internal/broker" "github.com/novox/mesh-controller/internal/catalogue" @@ -46,6 +47,28 @@ type deliveryMerge struct { AskID string `json:"ask_id"` Option string `json:"option"` Heads map[string]string `json:"heads"` + // Asker is the module that asked the ask, whose warrant is read from the router under that name (novox/hq ADR + // 0437 §6): empty is today's module, mesh-delivery. + Asker string `json:"asker,omitempty"` +} + +// askerOf is the asker whose warrant the merge is read under: the one the announcement names, mesh-delivery when it +// names none. +func (d deliveryMerge) askerOf() string { + if d.Asker == "" { + return theDelivery + } + return d.Asker +} + +// askerAccepted is whether the controller takes a warrant from this asker for a merge: the teller, and mesh-delivery +// until the gate takes the seat over (novox/hq ADR 0437 §6). Any other registers no move. +func askerAccepted(asker string) error { + if asker == theDelivery || asker == sdkdelivery.TellerModule { + return nil + } + return fmt.Errorf("the merge names the asker %q, and the controller takes a warrant only from %s and %s", + asker, sdkdelivery.TellerModule, theDelivery) } // parseDeliveryMerge reads an announcement, refusing one that names no pull request, head or ask. @@ -81,6 +104,10 @@ func mergeAct(ask, option string, heads map[string]string) asks.Act { // proved by the channel it came through, of this very ask as it was published (digest), bound to the heads the // announcement names, and those heads hold the head merged. Answers why it holds, or why not. func movedOnApprove(d deliveryMerge, ask *asks.Ask, w *asks.Warrant) (string, error) { + asker := d.askerOf() + if err := askerAccepted(asker); err != nil { + return "", err + } switch { case ask == nil || w == nil: return "", errors.New("the router holds no closed record of the ask the merge was made on") @@ -89,7 +116,7 @@ func movedOnApprove(d deliveryMerge, ask *asks.Ask, w *asks.Warrant) (string, er case d.Option != optionApprove: return "", fmt.Errorf("the merge was made on the option %q, not an Approve", d.Option) } - option, err := w.For(theDelivery, *ask) + option, err := w.For(asker, *ask) if err != nil { return "", err } @@ -188,9 +215,9 @@ var keptDeliveryMerge = func(ctx context.Context, owner, repo string, number int return d, err == nil, err } -// routerAskOf is the router's record of one of mesh-delivery's asks: the ask as published and its warrant. A +// routerAskOf is the router's record of one of the delivery's asks, under its asker: the ask as published and its warrant. A // variable so a test can hand it one. -var routerAskOf = func(ctx context.Context, inv catalogueReader, id string) (*asks.Ask, *asks.Warrant, error) { +var routerAskOf = func(ctx context.Context, inv catalogueReader, asker, id string) (*asks.Ask, *asks.Warrant, error) { serving := servingBus.Load() if serving == nil { return nil, nil, errors.New("only the serving controller reads the router's record") @@ -202,7 +229,7 @@ var routerAskOf = func(ctx context.Context, inv catalogueReader, id string) (*as if bucket == "" { return nil, nil, errors.New("no module declares the operator channel's records") } - _, ask, w, err := readRouterEntry(ctx, serving.Conn(), bucket, theDelivery, id) + _, ask, w, err := readRouterEntry(ctx, serving.Conn(), bucket, asker, id) return ask, w, err } @@ -283,7 +310,11 @@ func approvedAnnounced(ctx context.Context, inv catalogueReader, owner, name str if !kept { return "", fmt.Errorf("%w: %s/%s#%d at %.8s", errNotAnnounced, owner, name, k.Number, k.HeadSHA) } - ask, w, err := routerAskOf(ctx, inv, d.AskID) + // The warrant is read under the asker the announcement names, and under none the controller does not take. + if err := askerAccepted(d.askerOf()); err != nil { + return "", fmt.Errorf("%w: %w", errWordNotAnApprove, err) + } + ask, w, err := routerAskOf(ctx, inv, d.askerOf(), d.AskID) if err != nil { return "", err } diff --git a/cmd/mesh-controller/moved_asker_test.go b/cmd/mesh-controller/moved_asker_test.go new file mode 100644 index 00000000..9c6c3e37 --- /dev/null +++ b/cmd/mesh-controller/moved_asker_test.go @@ -0,0 +1,139 @@ +package main + +import ( + "context" + "strings" + "testing" + + "git.novox.be/novox/mesh-sdk/go/asks" + sdkdelivery "git.novox.be/novox/mesh-sdk/go/delivery" +) + +// The controller reads a merge's warrant under the asker the announcement names, and accepts only the teller and +// today's mesh-delivery (novox/hq ADR 0437 §6, issue 1213). The teller's name is the SDK's. + +// askedBy is an approved merge whose announcement names an asker ("" names none) and whose warrant is that asker's. +func askedBy(t *testing.T, named, warrantOf string) anApprovedMerge { + t.Helper() + a := approvedMerge(t) + a.merge.Asker = named + a.warrant.Asker = warrantOf + return a +} + +// 1213/AC1: an announcement naming the teller, and the teller's warrant of the operator's verified Approve of the +// merge's heads: the move is registered (the warrant is read from the router under the teller). +func TestAMergeNamingTheTellerIsMovedOnTheTellersWarrant(t *testing.T) { + a := askedBy(t, sdkdelivery.TellerModule, sdkdelivery.TellerModule) + if _, err := movedOnApprove(a.merge, &a.ask, &a.warrant); err != nil { + t.Fatalf("the teller's warrant of the operator's Approve was refused: %v", err) + } + was := routerAskOf + t.Cleanup(func() { routerAskOf = was }) + wasKept := keptDeliveryMerge + t.Cleanup(func() { keptDeliveryMerge = wasKept }) + keptDeliveryMerge = func(context.Context, string, string, int, string) (deliveryMerge, bool, error) { + return a.merge, true, nil + } + var readUnder string + routerAskOf = func(_ context.Context, _ catalogueReader, asker, _ string) (*asks.Ask, *asks.Warrant, error) { + readUnder = asker + return &a.ask, &a.warrant, nil + } + if _, err := approvedAnnounced(t.Context(), nil, "novox", "mesh-controller", announcedPull(7, a.merge.HeadSHA)); err != nil { + t.Fatalf("the merge was not moved on the teller's warrant: %v", err) + } + if readUnder != sdkdelivery.TellerModule { + t.Errorf("the warrant was read under %q", readUnder) + } +} + +// 1213/AC2: an announcement naming mesh-delivery, or none (today's module): read under mesh-delivery, registered. +func TestAMergeNamingNoAskerIsMovedOnMeshDeliverysWarrant(t *testing.T) { + for _, named := range []string{"", theDelivery} { + a := askedBy(t, named, theDelivery) + if _, err := movedOnApprove(a.merge, &a.ask, &a.warrant); err != nil { + t.Errorf("asker %q: refused: %v", named, err) + } + } + was := routerAskOf + wasKept := keptDeliveryMerge + t.Cleanup(func() { routerAskOf, keptDeliveryMerge = was, wasKept }) + a := askedBy(t, "", theDelivery) + keptDeliveryMerge = func(context.Context, string, string, int, string) (deliveryMerge, bool, error) { + return a.merge, true, nil + } + var readUnder string + routerAskOf = func(_ context.Context, _ catalogueReader, asker, _ string) (*asks.Ask, *asks.Warrant, error) { + readUnder = asker + return &a.ask, &a.warrant, nil + } + if _, err := approvedAnnounced(t.Context(), nil, "novox", "mesh-controller", announcedPull(7, a.merge.HeadSHA)); err != nil { + t.Fatal(err) + } + if readUnder != theDelivery { + t.Errorf("with no asker named the warrant was read under %q, not %q", readUnder, theDelivery) + } +} + +// 1213/AC3: any other asker registers no move, and the reason names the asker refused; the router is not asked. +func TestAMergeNamingAnyOtherAskerMovesNothingAndNamesIt(t *testing.T) { + was := routerAskOf + wasKept := keptDeliveryMerge + t.Cleanup(func() { routerAskOf, keptDeliveryMerge = was, wasKept }) + for _, other := range []string{sdkdelivery.GateModule, "mesh-controller", "unknown-module"} { + a := askedBy(t, other, other) // even a warrant of that very asker + _, err := movedOnApprove(a.merge, &a.ask, &a.warrant) + if err == nil || !strings.Contains(err.Error(), other) { + t.Errorf("asker %q: %v", other, err) + } + keptDeliveryMerge = func(context.Context, string, string, int, string) (deliveryMerge, bool, error) { + return a.merge, true, nil + } + routerAskOf = func(context.Context, catalogueReader, string, string) (*asks.Ask, *asks.Warrant, error) { + t.Errorf("the router was read under the refused asker %q", other) + return &a.ask, &a.warrant, nil + } + if _, err := approvedAnnounced(t.Context(), nil, "novox", "mesh-controller", announcedPull(7, a.merge.HeadSHA)); err == nil || + !strings.Contains(err.Error(), other) { + t.Errorf("asker %q, approved as announced: %v", other, err) + } + } +} + +// 1213/AC4: a warrant whose own asker differs from the asker read, or whose heads differ from the head merged, moves nothing. +func TestAWarrantOfAnotherAskerOrOtherHeadsMovesNothing(t *testing.T) { + a := askedBy(t, sdkdelivery.TellerModule, theDelivery) // read under the teller, but the warrant says mesh-delivery + if why, err := movedOnApprove(a.merge, &a.ask, &a.warrant); err == nil { + t.Errorf("a warrant of another asker was accepted: %s", why) + } + a = askedBy(t, "", sdkdelivery.TellerModule) // today's module named, the teller's warrant + if why, err := movedOnApprove(a.merge, &a.ask, &a.warrant); err == nil { + t.Errorf("the teller's warrant under mesh-delivery was accepted: %s", why) + } + a = askedBy(t, sdkdelivery.TellerModule, sdkdelivery.TellerModule) + a.merge.HeadSHA = "d4d4d4d4d4d4d4d4d4d4d4d4d4d4d4d4d4d4d4d4" + if why, err := movedOnApprove(a.merge, &a.ask, &a.warrant); err == nil { + t.Errorf("a head the Approve did not cover was accepted: %s", why) + } +} + +// 1213/AC5: the announcement's asker is read and kept with the rest of it, under the JSON name asker. +func TestTheAnnouncementsAskerIsReadAndKept(t *testing.T) { + body := `{"owner":"novox","repo":"r","number":1,"head_sha":"abc","ask_id":"a","option":"approve","asker":"mesh-delivery-teller"}` + d, err := parseDeliveryMerge([]byte(body)) + if err != nil || d.Asker != sdkdelivery.TellerModule { + t.Fatalf("read %+v, %v", d, err) + } + none, err := parseDeliveryMerge([]byte(strings.Replace(body, `,"asker":"mesh-delivery-teller"`, "", 1))) + if err != nil || none.Asker != "" { + t.Fatalf("an announcement naming none read as %+v, %v", none, err) + } +} + +// announcedPull is the forge's announcement of a merge of a pull request at a head. +func announcedPull(number int, head string) keptMerge { + var k keptMerge + k.Number, k.HeadSHA = number, head + return k +} diff --git a/cmd/mesh-controller/new_module_approved_test.go b/cmd/mesh-controller/new_module_approved_test.go index 3c442a8d..953aebc7 100644 --- a/cmd/mesh-controller/new_module_approved_test.go +++ b/cmd/mesh-controller/new_module_approved_test.go @@ -93,7 +93,7 @@ func TestANewModuleIsRegisteredOnTheOperatorsApproveOfItsMergeAndRefusedOtherwis keptDeliveryMerge = func(context.Context, string, string, int, string) (deliveryMerge, bool, error) { return a.merge, !c.unkept, nil } - routerAskOf = func(context.Context, catalogueReader, string) (*asks.Ask, *asks.Warrant, error) { + routerAskOf = func(context.Context, catalogueReader, string, string) (*asks.Ask, *asks.Warrant, error) { return &a.ask, &a.warrant, nil } if c.forge != nil { @@ -149,7 +149,7 @@ func TestASecondNewModuleFromARepositoryIsJudgedAlongsideAndAsksNoWarrant(t *tes t.Error("1146/AC5: the announced merges were asked for a module judged alongside") return deliveryMerge{}, false, nil } - routerAskOf = func(context.Context, catalogueReader, string) (*asks.Ask, *asks.Warrant, error) { + routerAskOf = func(context.Context, catalogueReader, string, string) (*asks.Ask, *asks.Warrant, error) { t.Error("1146/AC5: the router was asked for a module judged alongside") return nil, nil, nil }