From 8f183c82a8be3d869a847e0d6eddcdd4fc82b66a Mon Sep 17 00:00:00 2001 From: qwerty287 <80460567+qwerty287@users.noreply.github.com> Date: Fri, 28 Oct 2022 19:17:30 +0200 Subject: [PATCH] Support changed files for Gitea PRs (#1342) - add tests to fetch changed files - ignore error if gitea version is to low - adjust docs accordingly Co-authored-by: 6543 <6543@obermui.de> Co-authored-by: Lauris BH --- .golangci.yml | 5 ++ docs/docs/20-usage/20-pipeline-syntax.md | 3 +- .../11-forges/10-overview.md | 2 +- go.mod | 2 +- go.sum | 4 +- server/remote/gitea/fixtures/handler.go | 36 +++++++++-- server/remote/gitea/gitea.go | 62 ++++++++++++++++++- server/remote/gitea/gitea_test.go | 25 +++++++- server/remote/gitea/parse_test.go | 11 ++++ server/store/context.go | 4 ++ 10 files changed, 142 insertions(+), 12 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 0c7f6afdc..2a35baaaf 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -37,3 +37,8 @@ issues: linters: - deadcode - unused + # gin force us to use string as context key + - path: server/store/context.go + linters: + - staticcheck + - revive diff --git a/docs/docs/20-usage/20-pipeline-syntax.md b/docs/docs/20-usage/20-pipeline-syntax.md index 45c96c31c..32401ed29 100644 --- a/docs/docs/20-usage/20-pipeline-syntax.md +++ b/docs/docs/20-usage/20-pipeline-syntax.md @@ -412,8 +412,7 @@ when: :::info Path conditions are applied only to **push** and **pull_request** events. -It is currently **only available** for GitHub, GitLab. -Gitea only supports **push** at the moment ([go-gitea/gitea#18228](https://github.com/go-gitea/gitea/pull/18228)). +It is currently **only available** for GitHub, GitLab and Gitea (version 1.18.0 and newer) ::: Execute a step only on a pipeline with certain files being changed: diff --git a/docs/docs/30-administration/11-forges/10-overview.md b/docs/docs/30-administration/11-forges/10-overview.md index 5d5e11122..676584ae6 100644 --- a/docs/docs/30-administration/11-forges/10-overview.md +++ b/docs/docs/30-administration/11-forges/10-overview.md @@ -12,4 +12,4 @@ | [Multi pipeline](../../20-usage/25-multi-pipeline.md) | :white_check_mark: | :white_check_mark: | :white_check_mark: | :x: | :x: | :x: | :x: | | [when.path filter](../../20-usage/20-pipeline-syntax.md#path) | :white_check_mark: | :white_check_mark:¹ | :white_check_mark: | :x: | :x: | :x: | :x: | -¹) [except for pull requests](https://github.com/woodpecker-ci/woodpecker/issues/754) +¹) for Gitea versions 1.17 or lower not for pull requests diff --git a/go.mod b/go.mod index 8ae74c9ca..493b241d6 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/woodpecker-ci/woodpecker go 1.18 require ( - code.gitea.io/sdk/gitea v0.15.1-0.20220831004139-a0127ed0e7fe + code.gitea.io/sdk/gitea v0.15.1-0.20221016183512-2d9ee57af1e0 codeberg.org/6543/go-yaml2json v0.3.0 github.com/antonmedv/expr v1.9.0 github.com/bmatcuk/doublestar/v4 v4.2.0 diff --git a/go.sum b/go.sum index f99342ee8..cd2a5a9e4 100644 --- a/go.sum +++ b/go.sum @@ -32,8 +32,8 @@ cloud.google.com/go/storage v1.5.0/go.mod h1:tpKbwo567HUNpVclU5sGELwQWBDZ8gh0Zeo cloud.google.com/go/storage v1.6.0/go.mod h1:N7U0C8pVQ/+NIKOBQyamJIeKQKkZ+mxpohlUTyfDhBk= cloud.google.com/go/storage v1.8.0/go.mod h1:Wv1Oy7z6Yz3DshWRJFhqM/UCfaWIRTdp0RXyy7KQOVs= cloud.google.com/go/storage v1.10.0/go.mod h1:FLPqc6j+Ki4BU591ie1oL6qBQGu2Bl/tZ9ullr3+Kg0= -code.gitea.io/sdk/gitea v0.15.1-0.20220831004139-a0127ed0e7fe h1:PeLyxnUZE85QuJtBZ4P8qCQcgWG5Ked67mlNgr0WkCQ= -code.gitea.io/sdk/gitea v0.15.1-0.20220831004139-a0127ed0e7fe/go.mod h1:aRmrQC3CAHdJAU1LQt0C9zqzqI8tUB/5oQtNE746aYE= +code.gitea.io/sdk/gitea v0.15.1-0.20221016183512-2d9ee57af1e0 h1:AKpsCoOtVrWWBtANM9319pwCB5ihx1Sdvr1HSbAwr54= +code.gitea.io/sdk/gitea v0.15.1-0.20221016183512-2d9ee57af1e0/go.mod h1:ndkDk99BnfiUCCYEUhpNzi0lpmApXlwRFqClBlOlEBg= codeberg.org/6543/go-yaml2json v0.3.0 h1:BlvjmY0Gous8P+rr8aBdgPYnIfUAqFepF8q7Tp0R5t8= codeberg.org/6543/go-yaml2json v0.3.0/go.mod h1:mz61q14LWF4ZABrgMEDMmk3t9dPi6zgR1uBh2VKV2RQ= dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU= diff --git a/server/remote/gitea/fixtures/handler.go b/server/remote/gitea/fixtures/handler.go index 606444856..41742fcfa 100644 --- a/server/remote/gitea/fixtures/handler.go +++ b/server/remote/gitea/fixtures/handler.go @@ -28,11 +28,12 @@ func Handler() http.Handler { e := gin.New() e.GET("/api/v1/repos/:owner/:name", getRepo) e.GET("/api/v1/repositories/:id", getRepoByID) - e.GET("/api/v1/repos/:owner/:name/raw/:commit/:file", getRepoFile) + e.GET("/api/v1/repos/:owner/:name/raw/:file", getRepoFile) e.POST("/api/v1/repos/:owner/:name/hooks", createRepoHook) e.GET("/api/v1/repos/:owner/:name/hooks", listRepoHooks) e.DELETE("/api/v1/repos/:owner/:name/hooks/:id", deleteRepoHook) e.POST("/api/v1/repos/:owner/:name/statuses/:commit", createRepoCommitStatus) + e.GET("/api/v1/repos/:owner/:name/pulls/:index/files", getPRFiles) e.GET("/api/v1/user/repos", getUserRepos) e.GET("/api/v1/version", getVersion) @@ -69,10 +70,13 @@ func createRepoCommitStatus(c *gin.Context) { } func getRepoFile(c *gin.Context) { - if c.Param("file") == "file_not_found" { + file := c.Param("file") + ref := c.Query("ref") + + if file == "file_not_found" { c.String(404, "") } - if c.Param("commit") == "v1.0.0" || c.Param("commit") == "9ecad50" { + if ref == "v1.0.0" || ref == "9ecad50" { c.String(200, repoFilePayload) } c.String(404, "") @@ -116,7 +120,16 @@ func getUserRepos(c *gin.Context) { } func getVersion(c *gin.Context) { - c.JSON(200, map[string]interface{}{"version": "1.12"}) + c.JSON(200, map[string]interface{}{"version": "1.18.0"}) +} + +func getPRFiles(c *gin.Context) { + page := c.Query("page") + if page == "1" { + c.String(200, prFilesPayload) + } else { + c.String(200, "[]") + } } const listRepoHookPayloads = ` @@ -175,3 +188,18 @@ const userRepoPayload = ` } ] ` + +const prFilesPayload = ` +[ + { + "filename": "README.md", + "status": "changed", + "additions": 2, + "deletions": 0, + "changes": 2, + "html_url": "http://localhost/username/repo/src/commit/e79e4b0e8d9dd6f72b70e776c3317db7c19ca0fd/README.md", + "contents_url": "http://localhost:3000/api/v1/repos/username/repo/contents/README.md?ref=e79e4b0e8d9dd6f72b70e776c3317db7c19ca0fd", + "raw_url": "http://localhost/username/repo/raw/commit/e79e4b0e8d9dd6f72b70e776c3317db7c19ca0fd/README.md" + } +] +` diff --git a/server/remote/gitea/gitea.go b/server/remote/gitea/gitea.go index 87ae5d78b..192dc92a8 100644 --- a/server/remote/gitea/gitea.go +++ b/server/remote/gitea/gitea.go @@ -39,6 +39,7 @@ import ( "github.com/woodpecker-ci/woodpecker/server/model" "github.com/woodpecker-ci/woodpecker/server/remote" "github.com/woodpecker-ci/woodpecker/server/remote/common" + "github.com/woodpecker-ci/woodpecker/server/store" ) const ( @@ -475,7 +476,23 @@ func (c *Gitea) BranchHead(ctx context.Context, u *model.User, r *model.Repo, br // Hook parses the incoming Gitea hook and returns the Repository and Pipeline // details. If the hook is unsupported nil values are returned. func (c *Gitea) Hook(ctx context.Context, r *http.Request) (*model.Repo, *model.Pipeline, error) { - return parseHook(r) + repo, pipeline, err := parseHook(r) + if err != nil { + return nil, nil, err + } + + if pipeline.Event == model.EventPull && len(pipeline.ChangedFiles) == 0 { + index, err := strconv.ParseInt(strings.Split(pipeline.Ref, "/")[2], 10, 64) + if err != nil { + return nil, nil, err + } + pipeline.ChangedFiles, err = c.getChangedFilesForPR(ctx, repo, index) + if err != nil { + log.Error().Err(err).Msgf("could not get changed files for PR %s#%d", repo.FullName, index) + } + } + + return repo, pipeline, nil } // OrgMembership returns if user is member of organization and if user @@ -540,3 +557,46 @@ func getStatus(status model.StatusValue) gitea.StatusState { return gitea.StatusFailure } } + +func (c *Gitea) getChangedFilesForPR(ctx context.Context, repo *model.Repo, index int64) ([]string, error) { + _store, ok := store.TryFromContext(ctx) + if !ok { + log.Error().Msg("could not get store from context") + return []string{}, nil + } + + repo, err := _store.GetRepoNameFallback(repo.RemoteID, repo.FullName) + if err != nil { + return nil, err + } + + user, err := _store.GetUser(repo.UserID) + if err != nil { + return nil, err + } + + client, err := c.newClientToken(ctx, user.Token) + if err != nil { + return nil, err + } + + if client.CheckServerVersionConstraint("1.18.0") != nil { + // version too low + log.Debug().Msg("Gitea version does not support getting changed files for PRs") + return []string{}, nil + } + + return common.Paginate(func(page int) ([]string, error) { + giteaFiles, _, err := client.ListPullRequestFiles(repo.Owner, repo.Name, index, + gitea.ListPullRequestFilesOptions{ListOptions: gitea.ListOptions{Page: page}}) + if err != nil { + return nil, err + } + + var files []string + for _, file := range giteaFiles { + files = append(files, file.Filename) + } + return files, nil + }) +} diff --git a/server/remote/gitea/gitea_test.go b/server/remote/gitea/gitea_test.go index 7df13d6d7..3e3ef82dd 100644 --- a/server/remote/gitea/gitea_test.go +++ b/server/remote/gitea/gitea_test.go @@ -16,15 +16,21 @@ package gitea import ( + "bytes" "context" + "net/http" "net/http/httptest" "testing" "github.com/franela/goblin" "github.com/gin-gonic/gin" + "github.com/stretchr/testify/mock" + "github.com/woodpecker-ci/woodpecker/shared/utils" "github.com/woodpecker-ci/woodpecker/server/model" "github.com/woodpecker-ci/woodpecker/server/remote/gitea/fixtures" + "github.com/woodpecker-ci/woodpecker/server/store" + mocks_store "github.com/woodpecker-ci/woodpecker/server/store/mocks" ) func Test_gitea(t *testing.T) { @@ -36,7 +42,9 @@ func Test_gitea(t *testing.T) { SkipVerify: true, }) - ctx := context.Background() + mockStore := mocks_store.NewStore(t) + ctx := store.InjectToContext(context.Background(), mockStore) + g := goblin.Goblin(t) g.Describe("Gitea", func() { g.After(func() { @@ -154,6 +162,21 @@ func Test_gitea(t *testing.T) { g.It("Should return push details") g.It("Should handle a parsing error") }) + + g.It("Given a PR hook", func() { + buf := bytes.NewBufferString(fixtures.HookPullRequest) + req, _ := http.NewRequest("POST", "/hook", buf) + req.Header = http.Header{} + req.Header.Set(hookEvent, hookPullRequest) + mockStore.On("GetRepoNameFallback", mock.Anything, mock.Anything).Return(fakeRepo, nil) + mockStore.On("GetUser", mock.Anything).Return(fakeUser, nil) + r, b, err := c.Hook(ctx, req) + g.Assert(r).IsNotNil() + g.Assert(b).IsNotNil() + g.Assert(err).IsNil() + g.Assert(b.Event).Equal(model.EventPull) + g.Assert(utils.EqualStringSlice(b.ChangedFiles, []string{"README.md"})).IsTrue() + }) }) } diff --git a/server/remote/gitea/parse_test.go b/server/remote/gitea/parse_test.go index fc8403f44..5a8fb2d3e 100644 --- a/server/remote/gitea/parse_test.go +++ b/server/remote/gitea/parse_test.go @@ -40,6 +40,17 @@ func Test_parser(t *testing.T) { g.Assert(b).IsNil() g.Assert(err).IsNil() }) + g.It("given a PR hook", func() { + buf := bytes.NewBufferString(fixtures.HookPullRequest) + req, _ := http.NewRequest("POST", "/hook", buf) + req.Header = http.Header{} + req.Header.Set(hookEvent, hookPullRequest) + r, b, err := parseHook(req) + g.Assert(r).IsNotNil() + g.Assert(b).IsNotNil() + g.Assert(err).IsNil() + g.Assert(b.Event).Equal(model.EventPull) + }) g.Describe("given a push hook", func() { g.It("should extract repository and pipeline details", func() { buf := bytes.NewBufferString(fixtures.HookPush) diff --git a/server/store/context.go b/server/store/context.go index 1ac117de1..2e63c6e15 100644 --- a/server/store/context.go +++ b/server/store/context.go @@ -41,3 +41,7 @@ func TryFromContext(c context.Context) (Store, bool) { func ToContext(c Setter, store Store) { c.Set(key, store) } + +func InjectToContext(ctx context.Context, store Store) context.Context { + return context.WithValue(ctx, key, store) +}