diff --git a/go/delivery/gate.go b/go/delivery/gate.go index fe9cf56..eff609e 100644 --- a/go/delivery/gate.go +++ b/go/delivery/gate.go @@ -183,12 +183,20 @@ func ParseOwedKey(key string) (repository string, number int, head, act string, } // Merge is the value of a merges key: the ask approved and, for each pull request, the head the approval expects -// and how far its merge has come. +// and how far its merge has come. Approved is the Approve's heads of every member of the group, ApprovedKey of +// the member to its full head sha: the forge's merge tool wants them for every sibling whichever member it is +// handed (ADR 0450 ยง3). A merge written before Approved existed has none and is not checked against it. type Merge struct { - V int `json:"v"` - Group string `json:"group"` - Ask string `json:"ask"` - Pulls []MergePull `json:"pulls"` + V int `json:"v"` + Group string `json:"group"` + Ask string `json:"ask"` + Approved map[string]string `json:"approved,omitempty"` + Pulls []MergePull `json:"pulls"` +} + +// ApprovedKey is the key of a member in Merge.Approved: `@`. +func ApprovedKey(repository string, number int) string { + return repository + "@" + strconv.Itoa(number) } // MergePull is one pull request of a merge: its expected head, its state, the merge commit once merged, the @@ -210,8 +218,22 @@ func (m Merge) check() error { if len(m.Pulls) == 0 { return fmt.Errorf("delivery: a merge names no pull request") } + for key, sha := range m.Approved { + if sha == "" { + return fmt.Errorf("delivery: the approved head of %s is empty", key) + } + } for _, p := range m.Pulls { who := fmt.Sprintf("%s#%d", p.Repository, p.Number) + if len(m.Approved) > 0 { + sha, ok := m.Approved[ApprovedKey(p.Repository, p.Number)] + if !ok { + return fmt.Errorf("delivery: the pull request %s is handed to the merge but is not among the approved heads", who) + } + if sha != p.Head.Head { + return fmt.Errorf("delivery: the pull request %s is handed at head %q but the Approve was for %q", who, p.Head.Head, sha) + } + } switch p.State { case MergeMerging: case MergeMerged: diff --git a/go/delivery/gate_test.go b/go/delivery/gate_test.go index 437d27b..e1d1e3a 100644 --- a/go/delivery/gate_test.go +++ b/go/delivery/gate_test.go @@ -3,6 +3,7 @@ package delivery import ( "encoding/json" "errors" + "strconv" "strings" "testing" "time" @@ -379,3 +380,71 @@ func TestKeysEscapeAndRoundTrip(t *testing.T) { // 1323/AC1 t.Errorf("decode merge: %v %v", back.Group, err) } } + +func approvedMerge() Merge { + m := sampleMerge() + m.Approved = map[string]string{ + ApprovedKey("novox/mesh-sdk", 36): "aaaa", + ApprovedKey("novox/mesh-control", 7): "bbbb", + ApprovedKey("novox/other", 1): "dddd", + } + return m +} + +func TestMergeCarriesApprovedHeads(t *testing.T) { // 1321/AC1 + m := approvedMerge() + m.Pulls = m.Pulls[:1] // two members approved besides the one handed so far + m.Pulls[0].State = MergeMerging + m.Pulls[0].Commit = "" + data, err := EncodeMerge(m) + if err != nil { + t.Fatal(err) + } + back, err := DecodeMerge(data) + if err != nil { + t.Fatal(err) + } + m.V = Version + if !SameMerge(back, m) || len(back.Approved) != 3 || back.Approved["novox/mesh-control@7"] != "bbbb" { + t.Errorf("decoded %+v, want %+v", back, m) + } +} + +func TestMergeRefusesAPullNotApproved(t *testing.T) { // 1321/AC2 + m := approvedMerge() + delete(m.Approved, ApprovedKey("novox/other", 1)) + if _, err := EncodeMerge(m); err == nil || !strings.Contains(err.Error(), "novox/other#1") { + t.Errorf("a handed pull missing from the approved heads: %v", err) + } + m = approvedMerge() + m.Approved[ApprovedKey("novox/other", 1)] = "eeee" + if _, err := EncodeMerge(m); err == nil { + t.Error("a handed pull at another head than the Approve's accepted") + } + m = approvedMerge() + m.Approved[ApprovedKey("novox/other", 1)] = "" + if _, err := EncodeMerge(m); err == nil { + t.Error("an empty approved head accepted") + } + data, _ := json.Marshal(approvedMerge()) + var raw map[string]any + _ = json.Unmarshal(data, &raw) + raw["v"] = Version + raw["approved"] = map[string]string{"novox/mesh-sdk@36": "aaaa"} + bad, _ := json.Marshal(raw) + if _, err := DecodeMerge(bad); err == nil { + t.Error("decoding a merge whose pulls are not all approved accepted") + } +} + +func TestMergeWithoutApprovedHeadsDecodes(t *testing.T) { // 1321/AC3 + old := `{"v":` + strconv.Itoa(Version) + `,"group":"feat/x","ask":"merge.feat/x.0123456789ab","pulls":[{"repository":"novox/mesh-sdk","number":36,"head":"aaaa","state":"merging"}]}` + m, err := DecodeMerge([]byte(old)) + if err != nil || len(m.Approved) != 0 || len(m.Pulls) != 1 { + t.Fatalf("old merge = %+v, %v", m, err) + } + data, err := EncodeMerge(sampleMerge()) + if err != nil || strings.Contains(string(data), "approved") { + t.Errorf("a merge without approved heads wrote them: %s %v", data, err) + } +}