diff --git a/modules/gitea/README.md b/modules/gitea/README.md index c11629d5..4770fb95 100644 --- a/modules/gitea/README.md +++ b/modules/gitea/README.md @@ -154,7 +154,9 @@ listed repository: naming the description, and a `merge-check.sh` declaring `# mesh-check-toolchain: go` that vets and runs the unit tests, then the integration tests (`-tags integration`), of every Go module in the tree, and says so and passes on a tree with none. The rule is then narrowed to main's rule of ADR 0398 ยง2 (no push, only `mesh-tools` merges), - requiring `mesh/repo-check` and `mesh/merge-gate`, and only then is the repository moved into the organisation; + requiring `mesh/repo-check` and `mesh/merge-gate`, and only then is the repository moved into the organisation. + There main's rule is read back and put right once more (the step `rule-after-move`): the forge drops from a merge + list a user without access to the repository, and under the setup account `mesh-tools` has none; - one found still under the setup account is finished: the steps it lacks are done, main is never written twice; - one in the organisation is only put right: main's rule narrowed, a missing one of the two statuses added and every status already there kept. diff --git a/modules/gitea/cmd/repositories/fake_test.go b/modules/gitea/cmd/repositories/fake_test.go index 43f50b11..f78be551 100644 --- a/modules/gitea/cmd/repositories/fake_test.go +++ b/modules/gitea/cmd/repositories/fake_test.go @@ -118,6 +118,28 @@ func lets(rule map[string]any, login string) bool { return false } +// reaches says whether a login has access to a repository, as the forge judges a merge whitelist: its owner does, +// and in the organisation the tools' team does (it is on every repository there); nobody else here. +func reaches(full, login string) bool { + owner := strings.SplitN(full, "/", 2)[0] + return owner == login || (owner == org && login == tools) +} + +// keepReaching drops from a rule's merge whitelist every user without access to the repository, as gitea's +// updateUserWhitelist does on creating or editing a branch protection. +func keepReaching(full string, rule map[string]any) { + if _, ok := rule["merge_whitelist_usernames"]; !ok { + return + } + kept := []any{} + for _, u := range strs(rule["merge_whitelist_usernames"]) { + if reaches(full, u) { + kept = append(kept, u) + } + } + rule["merge_whitelist_usernames"] = kept +} + func (f *fakeForge) ServeHTTP(w http.ResponseWriter, r *http.Request) { f.mu.Lock() defer f.mu.Unlock() @@ -197,6 +219,7 @@ func (f *fakeForge) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } delete(in, "rule_name") + keepReaching(m[1]+"/"+m[2], in) repo.Rules[name] = in reply(201, in) case r.Method == http.MethodPatch && m[3] != "": @@ -208,6 +231,7 @@ func (f *fakeForge) ServeHTTP(w http.ResponseWriter, r *http.Request) { for k, v := range in { rule[k] = v } + keepReaching(m[1]+"/"+m[2], rule) reply(200, rule) default: reply(405, nil) diff --git a/modules/gitea/cmd/repositories/repositories.go b/modules/gitea/cmd/repositories/repositories.go index f62a5048..475554e7 100644 --- a/modules/gitea/cmd/repositories/repositories.go +++ b/modules/gitea/cmd/repositories/repositories.go @@ -39,6 +39,9 @@ const ( StepNarrow = "narrow" StepMove = "move" StepPutRight = "put-right" + // StepRuleAfterMove reads main's rule back once the repository is in the organisation and puts it right: the + // forge drops from a merge whitelist a user without access, and under the setup account the tools' have none. + StepRuleAfterMove = "rule-after-move" ) // What a pass did with one repository. @@ -250,6 +253,9 @@ func (s *Syncer) one(ctx context.Context, name string, r Repo) Result { map[string]any{"new_owner": s.Org}, nil); err != nil { return refused(name, StepMove, err) } + if _, err := s.putRight(ctx, s.Org, name); err != nil { + return refused(name, StepRuleAfterMove, err) + } return Result{Name: name, Did: DidMade} } diff --git a/modules/gitea/cmd/repositories/repositories_test.go b/modules/gitea/cmd/repositories/repositories_test.go index 6c4cf096..7f5b0399 100644 --- a/modules/gitea/cmd/repositories/repositories_test.go +++ b/modules/gitea/cmd/repositories/repositories_test.go @@ -31,6 +31,7 @@ func TestNewRepositoryIsMadeInOrder(t *testing.T) { "POST /api/v1/repos/mesh-admin/mesh-delivery/contents", "PATCH /api/v1/repos/mesh-admin/mesh-delivery/branch_protections/main", "POST /api/v1/repos/mesh-admin/mesh-delivery/transfer", + "PATCH /api/v1/repos/novox/mesh-delivery/branch_protections/main", } if got := f.writes(); strings.Join(got, "\n") != strings.Join(want, "\n") { t.Fatalf("writes, in order:\n%s\nwant:\n%s", strings.Join(got, "\n"), strings.Join(want, "\n")) @@ -84,6 +85,33 @@ func TestNewRepositoryIsMadeInOrder(t *testing.T) { } } +// 1124/AC2: the forge drops from a merge whitelist a user without access to the repository, and under the setup +// account the tools' account has none; so once the repository is in the organisation its main rule is read back and +// put right: the tools' account the only one that merges, both statuses required. +func TestOrganisationRuleListsTheToolsAfterTheMove(t *testing.T) { + f := newFake() + s, _ := syncer(t, f) + o := run(t, s, Value{"mesh-delivery": {}}) + repo := f.repos["novox/mesh-delivery"] + if repo == nil { + t.Fatal("not moved into the organisation") + } + rule := repo.Rules[mainRule] + if rule["enable_merge_whitelist"] != true || strings.Join(strs(rule["merge_whitelist_usernames"]), ",") != tools { + t.Errorf("after the move main's merge whitelist is %v, want %s alone", rule["merge_whitelist_usernames"], tools) + } + if rule["enable_push"] != false { + t.Errorf("after the move main takes a push: %v", rule) + } + checks := strs(rule["status_check_contexts"]) + if rule["enable_status_check"] != true || !has(checks, "mesh/repo-check") || !has(checks, "mesh/merge-gate") { + t.Errorf("after the move main does not require both statuses: %v", rule) + } + if r := result(o, "mesh-delivery"); r.Did != DidMade { + t.Errorf("outcome %+v, want made", r) + } +} + // 1124/AC2: the setup login's rule lets that login alone push, and it is what the first commit lands through. func TestAdminRuleLetsTheSetupLoginAlonePush(t *testing.T) { f := newFake() @@ -164,8 +192,10 @@ func TestRepositoryUnderTheSetupAccountIsFinished(t *testing.T) { "POST /api/v1/repos/mesh-admin/empty/contents", "PATCH /api/v1/repos/mesh-admin/empty/branch_protections/main", "POST /api/v1/repos/mesh-admin/empty/transfer", + "PATCH /api/v1/repos/novox/empty/branch_protections/main", "PATCH /api/v1/repos/mesh-admin/half/branch_protections/main", "POST /api/v1/repos/mesh-admin/half/transfer", + "PATCH /api/v1/repos/novox/half/branch_protections/main", }, "\n") if got != want { t.Fatalf("writes:\n%s\nwant:\n%s", got, want) @@ -223,6 +253,9 @@ func TestRefusedStepStopsThatRepositoryOnly(t *testing.T) { StepFirstCommit: func(m, p string) bool { return m == "POST" && strings.HasSuffix(p, "/a/contents") }, StepNarrow: func(m, p string) bool { return m == "PATCH" && strings.Contains(p, "/a/branch_protections/") }, StepMove: func(m, p string) bool { return m == "POST" && strings.HasSuffix(p, "/a/transfer") }, + StepRuleAfterMove: func(m, p string) bool { + return m == "PATCH" && strings.HasPrefix(p, "/api/v1/repos/novox/a/branch_protections/") + }, } for step, match := range steps { t.Run(step, func(t *testing.T) {