From 2d3f44d03b4c5b597cbaac802e2e48b6e46dd220 Mon Sep 17 00:00:00 2001 From: forgejo-backport-action Date: Thu, 7 Aug 2025 14:29:03 +0200 Subject: [PATCH] [v12.0/forgejo] fix: add .forgejo/CODEOWNERS support (#8746) (#8790) **Backport:** https://codeberg.org/forgejo/forgejo/pulls/8773 Currently, the docs mention that a CODEOWNERS file can be located in .forgejo for code owner PR review assignment, but this does not work. Add support for this location. This fixes #8746. ## Checklist The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org). ### Tests - I added test coverage for Go changes... - [ ] in their respective `*_test.go` for unit tests. - [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server. - I added test coverage for JavaScript changes... - [ ] in `web_src/js/*.test.js` if it can be unit tested. - [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)). ### Documentation - [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [x] I did not document these changes and I do not expect someone else to do it. ### Release notes - [ ] I do not want this change to show in the release notes. - [x] I want the title to show in the release notes with a link to this pull request. - [ ] I want the content of the `release-notes/.md` to be be used for the release notes instead of the title. ## Release notes - Bug fixes - [PR](https://codeberg.org/forgejo/forgejo/pulls/8790): fix: add .forgejo/CODEOWNERS support (#8746) Co-authored-by: John Moon Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8790 Co-authored-by: forgejo-backport-action Co-committed-by: forgejo-backport-action --- routers/web/repo/view.go | 2 +- services/issue/pull.go | 2 +- tests/integration/codeowner_test.go | 297 +++++++++++++++------------- 3 files changed, 159 insertions(+), 142 deletions(-) diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index bb3e1388a8..91a8661097 100644 --- a/routers/web/repo/view.go +++ b/routers/web/repo/view.go @@ -438,7 +438,7 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry) { if workFlowErr != nil { ctx.Data["FileError"] = ctx.Locale.Tr("actions.runs.invalid_workflow_helper", workFlowErr.Error()) } - } else if slices.Contains([]string{"CODEOWNERS", "docs/CODEOWNERS", ".gitea/CODEOWNERS"}, ctx.Repo.TreePath) { + } else if slices.Contains([]string{"CODEOWNERS", "docs/CODEOWNERS", ".gitea/CODEOWNERS", ".forgejo/CODEOWNERS"}, ctx.Repo.TreePath) { if rc, size, err := blob.NewTruncatedReader(setting.UI.MaxDisplayFileSize); err == nil { _, warnings := issue_model.GetCodeOwnersFromReader(ctx, rc, size > setting.UI.MaxDisplayFileSize) if len(warnings) > 0 { diff --git a/services/issue/pull.go b/services/issue/pull.go index 2eef1fbfa8..6245344ccb 100644 --- a/services/issue/pull.go +++ b/services/issue/pull.go @@ -71,7 +71,7 @@ func PullRequestCodeOwnersReview(ctx context.Context, issue *issues_model.Issue, } var rules []*issues_model.CodeOwnerRule - for _, file := range []string{"CODEOWNERS", "docs/CODEOWNERS", ".gitea/CODEOWNERS"} { + for _, file := range []string{"CODEOWNERS", "docs/CODEOWNERS", ".gitea/CODEOWNERS", ".forgejo/CODEOWNERS"} { if blob, err := commit.GetBlobByPath(file); err == nil { rc, size, err := blob.NewTruncatedReader(setting.UI.MaxDisplayFileSize) if err == nil { diff --git a/tests/integration/codeowner_test.go b/tests/integration/codeowner_test.go index e2eeb843d8..b85a5f213d 100644 --- a/tests/integration/codeowner_test.go +++ b/tests/integration/codeowner_test.go @@ -26,175 +26,192 @@ import ( "github.com/stretchr/testify/require" ) -func TestCodeOwner(t *testing.T) { - onGiteaRun(t, func(t *testing.T, u *url.URL) { - user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) +func CodeOwnerTestCommon(t *testing.T, u *url.URL, codeownerTest CodeownerTest) { + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - // Create the repo. - repo, _, f := tests.CreateDeclarativeRepo(t, user2, "", - []unit_model.Type{unit_model.TypePullRequests}, nil, - []*files_service.ChangeRepoFile{ - { - Operation: "create", - TreePath: "CODEOWNERS", - ContentReader: strings.NewReader("README.md @user5\ntest-file @user4"), - }, + // Create the repo. + repo, _, f := tests.CreateDeclarativeRepo(t, user2, codeownerTest.Name, + []unit_model.Type{unit_model.TypePullRequests}, nil, + []*files_service.ChangeRepoFile{ + { + Operation: "create", + TreePath: codeownerTest.Path, + ContentReader: strings.NewReader("README.md @user5\ntest-file @user4"), }, - ) - defer f() + }, + ) + defer f() - dstPath := t.TempDir() - r := fmt.Sprintf("%suser2/%s.git", u.String(), repo.Name) - cloneURL, _ := url.Parse(r) - cloneURL.User = url.UserPassword("user2", userPassword) - require.NoError(t, git.CloneWithArgs(t.Context(), nil, cloneURL.String(), dstPath, git.CloneRepoOptions{})) + dstPath := t.TempDir() + r := fmt.Sprintf("%suser2/%s.git", u.String(), repo.Name) + cloneURL, _ := url.Parse(r) + cloneURL.User = url.UserPassword("user2", userPassword) + require.NoError(t, git.CloneWithArgs(t.Context(), nil, cloneURL.String(), dstPath, git.CloneRepoOptions{})) - t.Run("Normal", func(t *testing.T) { - defer tests.PrintCurrentTest(t)() + t.Run("Normal", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() - err := os.WriteFile(path.Join(dstPath, "README.md"), []byte("## test content"), 0o666) - require.NoError(t, err) + err := os.WriteFile(path.Join(dstPath, "README.md"), []byte("## test content"), 0o666) + require.NoError(t, err) - err = git.AddChanges(dstPath, true) - require.NoError(t, err) + err = git.AddChanges(dstPath, true) + require.NoError(t, err) - err = git.CommitChanges(dstPath, git.CommitChangesOptions{ - Committer: &git.Signature{ - Email: "user2@example.com", - Name: "user2", - When: time.Now(), - }, - Author: &git.Signature{ - Email: "user2@example.com", - Name: "user2", - When: time.Now(), - }, - Message: "Add README.", - }) - require.NoError(t, err) - - err = git.NewCommand(git.DefaultContext, "push", "origin", "HEAD:refs/for/main", "-o", "topic=codeowner-normal").Run(&git.RunOpts{Dir: dstPath}) - require.NoError(t, err) - - pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: repo.ID, HeadBranch: "user2/codeowner-normal"}) - unittest.AssertExistsIf(t, true, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 5}) + err = git.CommitChanges(dstPath, git.CommitChangesOptions{ + Committer: &git.Signature{ + Email: "user2@example.com", + Name: "user2", + When: time.Now(), + }, + Author: &git.Signature{ + Email: "user2@example.com", + Name: "user2", + When: time.Now(), + }, + Message: "Add README.", }) + require.NoError(t, err) - t.Run("Forked repository", func(t *testing.T) { - defer tests.PrintCurrentTest(t)() + err = git.NewCommand(git.DefaultContext, "push", "origin", "HEAD:refs/for/main", "-o", "topic=codeowner-normal").Run(&git.RunOpts{Dir: dstPath}) + require.NoError(t, err) - session := loginUser(t, "user1") - testRepoFork(t, session, user2.Name, repo.Name, "user1", "repo1") + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: repo.ID, HeadBranch: "user2/codeowner-normal"}) + unittest.AssertExistsIf(t, true, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 5}) + }) - repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user1", Name: "repo1"}) + t.Run("Forked repository", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() - r := fmt.Sprintf("%suser1/repo1.git", u.String()) - remoteURL, _ := url.Parse(r) - remoteURL.User = url.UserPassword("user2", userPassword) - doGitAddRemote(dstPath, "forked", remoteURL)(t) + session := loginUser(t, "user1") + testRepoFork(t, session, user2.Name, repo.Name, "user1", codeownerTest.Name) - err := git.NewCommand(git.DefaultContext, "push", "forked", "HEAD:refs/for/main", "-o", "topic=codeowner-forked").Run(&git.RunOpts{Dir: dstPath}) - require.NoError(t, err) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user1", Name: codeownerTest.Name}) - pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: repo.ID, HeadBranch: "user2/codeowner-forked"}) - unittest.AssertExistsIf(t, false, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 5}) + r := fmt.Sprintf("%suser1/%s.git", u.String(), codeownerTest.Name) + remoteURL, _ := url.Parse(r) + remoteURL.User = url.UserPassword("user2", userPassword) + doGitAddRemote(dstPath, "forked", remoteURL)(t) + + err := git.NewCommand(git.DefaultContext, "push", "forked", "HEAD:refs/for/main", "-o", "topic=codeowner-forked").Run(&git.RunOpts{Dir: dstPath}) + require.NoError(t, err) + + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: repo.ID, HeadBranch: "user2/codeowner-forked"}) + unittest.AssertExistsIf(t, false, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 5}) + }) + + t.Run("Out of date", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + // Push the changes made from the previous subtest. + require.NoError(t, git.NewCommand(git.DefaultContext, "push", "origin").Run(&git.RunOpts{Dir: dstPath})) + + // Reset the tree to the previous commit. + require.NoError(t, git.NewCommand(git.DefaultContext, "reset", "--hard", "HEAD~1").Run(&git.RunOpts{Dir: dstPath})) + + err := os.WriteFile(path.Join(dstPath, "test-file"), []byte("## test content"), 0o666) + require.NoError(t, err) + + err = git.AddChanges(dstPath, true) + require.NoError(t, err) + + err = git.CommitChanges(dstPath, git.CommitChangesOptions{ + Committer: &git.Signature{ + Email: "user2@example.com", + Name: "user2", + When: time.Now(), + }, + Author: &git.Signature{ + Email: "user2@example.com", + Name: "user2", + When: time.Now(), + }, + Message: "Add test-file.", }) + require.NoError(t, err) - t.Run("Out of date", func(t *testing.T) { - defer tests.PrintCurrentTest(t)() + err = git.NewCommand(git.DefaultContext, "push", "origin", "HEAD:refs/for/main", "-o", "topic=codeowner-out-of-date").Run(&git.RunOpts{Dir: dstPath}) + require.NoError(t, err) - // Push the changes made from the previous subtest. - require.NoError(t, git.NewCommand(git.DefaultContext, "push", "origin").Run(&git.RunOpts{Dir: dstPath})) + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: repo.ID, HeadBranch: "user2/codeowner-out-of-date"}) + unittest.AssertExistsIf(t, true, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 4}) + unittest.AssertExistsIf(t, false, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 5}) + }) + t.Run("From a forked repository", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() - // Reset the tree to the previous commit. - require.NoError(t, git.NewCommand(git.DefaultContext, "reset", "--hard", "HEAD~1").Run(&git.RunOpts{Dir: dstPath})) + session := loginUser(t, "user1") - err := os.WriteFile(path.Join(dstPath, "test-file"), []byte("## test content"), 0o666) - require.NoError(t, err) + r := fmt.Sprintf("%suser1/%s.git", u.String(), codeownerTest.Name) + remoteURL, _ := url.Parse(r) + remoteURL.User = url.UserPassword("user1", userPassword) + doGitAddRemote(dstPath, "forked-2", remoteURL)(t) - err = git.AddChanges(dstPath, true) - require.NoError(t, err) + err := git.NewCommand(git.DefaultContext, "push", "forked-2", "HEAD:branch").Run(&git.RunOpts{Dir: dstPath}) + require.NoError(t, err) - err = git.CommitChanges(dstPath, git.CommitChangesOptions{ - Committer: &git.Signature{ - Email: "user2@example.com", - Name: "user2", - When: time.Now(), - }, - Author: &git.Signature{ - Email: "user2@example.com", - Name: "user2", - When: time.Now(), - }, - Message: "Add test-file.", - }) - require.NoError(t, err) - - err = git.NewCommand(git.DefaultContext, "push", "origin", "HEAD:refs/for/main", "-o", "topic=codeowner-out-of-date").Run(&git.RunOpts{Dir: dstPath}) - require.NoError(t, err) - - pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: repo.ID, HeadBranch: "user2/codeowner-out-of-date"}) - unittest.AssertExistsIf(t, true, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 4}) - unittest.AssertExistsIf(t, false, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 5}) + req := NewRequestWithValues(t, "POST", repo.FullName()+"/compare/main...user1/"+codeownerTest.Name+":branch", map[string]string{ + "_csrf": GetCSRF(t, session, repo.FullName()+"/compare/main...user1/"+codeownerTest.Name+":branch"), + "title": "pull request", }) - t.Run("From a forked repository", func(t *testing.T) { - defer tests.PrintCurrentTest(t)() + session.MakeRequest(t, req, http.StatusOK) - session := loginUser(t, "user1") + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: repo.ID, HeadBranch: "branch"}) + unittest.AssertExistsIf(t, true, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 4}) + }) - r := fmt.Sprintf("%suser1/repo1.git", u.String()) - remoteURL, _ := url.Parse(r) - remoteURL.User = url.UserPassword("user1", userPassword) - doGitAddRemote(dstPath, "forked-2", remoteURL)(t) + t.Run("Codeowner user with no permission", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() - err := git.NewCommand(git.DefaultContext, "push", "forked-2", "HEAD:branch").Run(&git.RunOpts{Dir: dstPath}) - require.NoError(t, err) + // Make repository private, only user2 (owner of repository) has now access to this repository. + repo.IsPrivate = true + _, err := db.GetEngine(db.DefaultContext).Cols("is_private").Update(repo) + require.NoError(t, err) - req := NewRequestWithValues(t, "POST", repo.FullName()+"/compare/main...user1/repo1:branch", map[string]string{ - "_csrf": GetCSRF(t, session, repo.FullName()+"/compare/main...user1/repo1:branch"), - "title": "pull request", - }) - session.MakeRequest(t, req, http.StatusOK) + err = os.WriteFile(path.Join(dstPath, "README.md"), []byte("## very sensitive info"), 0o666) + require.NoError(t, err) - pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: repo.ID, HeadBranch: "branch"}) - unittest.AssertExistsIf(t, true, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 4}) + err = git.AddChanges(dstPath, true) + require.NoError(t, err) + + err = git.CommitChanges(dstPath, git.CommitChangesOptions{ + Committer: &git.Signature{ + Email: "user2@example.com", + Name: "user2", + When: time.Now(), + }, + Author: &git.Signature{ + Email: "user2@example.com", + Name: "user2", + When: time.Now(), + }, + Message: "Add secrets to the README.", }) + require.NoError(t, err) - t.Run("Codeowner user with no permission", func(t *testing.T) { - defer tests.PrintCurrentTest(t)() + err = git.NewCommand(git.DefaultContext, "push", "origin", "HEAD:refs/for/main", "-o", "topic=codeowner-private").Run(&git.RunOpts{Dir: dstPath}) + require.NoError(t, err) - // Make repository private, only user2 (owner of repository) has now access to this repository. - repo.IsPrivate = true - _, err := db.GetEngine(db.DefaultContext).Cols("is_private").Update(repo) - require.NoError(t, err) - - err = os.WriteFile(path.Join(dstPath, "README.md"), []byte("## very sensitive info"), 0o666) - require.NoError(t, err) - - err = git.AddChanges(dstPath, true) - require.NoError(t, err) - - err = git.CommitChanges(dstPath, git.CommitChangesOptions{ - Committer: &git.Signature{ - Email: "user2@example.com", - Name: "user2", - When: time.Now(), - }, - Author: &git.Signature{ - Email: "user2@example.com", - Name: "user2", - When: time.Now(), - }, - Message: "Add secrets to the README.", - }) - require.NoError(t, err) - - err = git.NewCommand(git.DefaultContext, "push", "origin", "HEAD:refs/for/main", "-o", "topic=codeowner-private").Run(&git.RunOpts{Dir: dstPath}) - require.NoError(t, err) - - // In CODEOWNERS file the codeowner for README.md is user5, but does not have access to this private repository. - pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: repo.ID, HeadBranch: "user2/codeowner-private"}) - unittest.AssertExistsIf(t, false, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 5}) - }) + // In CODEOWNERS file the codeowner for README.md is user5, but does not have access to this private repository. + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: repo.ID, HeadBranch: "user2/codeowner-private"}) + unittest.AssertExistsIf(t, false, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 5}) + }) +} + +type CodeownerTest struct { + Name string + Path string +} + +func TestCodeOwner(t *testing.T) { + onGiteaRun(t, func(t *testing.T, u *url.URL) { + tests := []CodeownerTest{ + {Name: "root", Path: "CODEOWNERS"}, + {Name: "docs", Path: "docs/CODEOWNERS"}, + {Name: "gitea", Path: ".gitea/CODEOWNERS"}, + {Name: "forgejo", Path: ".forgejo/CODEOWNERS"}, + } + for _, test := range tests { + CodeOwnerTestCommon(t, u, test) + } }) }