diff --git a/server/api/hook.go b/server/api/hook.go index 51f8979c1..3fca72969 100644 --- a/server/api/hook.go +++ b/server/api/hook.go @@ -19,6 +19,7 @@ package api import ( + "errors" "fmt" "net/http" "regexp" @@ -27,6 +28,7 @@ import ( "github.com/rs/zerolog/log" "github.com/woodpecker-ci/woodpecker/server" + "github.com/woodpecker-ci/woodpecker/server/forge/types" "github.com/woodpecker-ci/woodpecker/server/model" "github.com/woodpecker-ci/woodpecker/server/pipeline" "github.com/woodpecker-ci/woodpecker/server/store" @@ -108,11 +110,19 @@ func PostHook(c *gin.Context) { tmpRepo, tmpBuild, err := forge.Hook(c, c.Request) if err != nil { + if errors.Is(err, &types.ErrIgnoreEvent{}) { + msg := fmt.Sprintf("forge driver: %s", err) + log.Debug().Err(err).Msg(msg) + c.String(http.StatusOK, msg) + return + } + msg := "failure to parse hook" log.Debug().Err(err).Msg(msg) c.String(http.StatusBadRequest, msg) return } + if tmpBuild == nil { msg := "ignoring hook: hook parsing resulted in empty pipeline" log.Debug().Msg(msg) diff --git a/server/forge/bitbucket/parse.go b/server/forge/bitbucket/parse.go index def263678..20b79aac9 100644 --- a/server/forge/bitbucket/parse.go +++ b/server/forge/bitbucket/parse.go @@ -20,6 +20,7 @@ import ( "net/http" "github.com/woodpecker-ci/woodpecker/server/forge/bitbucket/internal" + "github.com/woodpecker-ci/woodpecker/server/forge/types" "github.com/woodpecker-ci/woodpecker/server/model" ) @@ -34,15 +35,20 @@ const ( // parseHook parses a Bitbucket hook from an http.Request request and returns // Repo and Pipeline detail. If a hook type is unsupported nil values are returned. func parseHook(r *http.Request) (*model.Repo, *model.Pipeline, error) { - payload, _ := io.ReadAll(r.Body) + payload, err := io.ReadAll(r.Body) + if err != nil { + return nil, nil, err + } - switch r.Header.Get(hookEvent) { + hookType := r.Header.Get(hookEvent) + switch hookType { case hookPush: return parsePushHook(payload) case hookPullCreated, hookPullUpdated: return parsePullHook(payload) + default: + return nil, nil, &types.ErrIgnoreEvent{Event: hookType} } - return nil, nil, nil } // parsePushHook parses a push hook and returns the Repo and Pipeline details. diff --git a/server/forge/bitbucket/parse_test.go b/server/forge/bitbucket/parse_test.go index bb3175622..a163c5e2a 100644 --- a/server/forge/bitbucket/parse_test.go +++ b/server/forge/bitbucket/parse_test.go @@ -20,8 +20,10 @@ import ( "testing" "github.com/franela/goblin" + "github.com/stretchr/testify/assert" "github.com/woodpecker-ci/woodpecker/server/forge/bitbucket/fixtures" + "github.com/woodpecker-ci/woodpecker/server/forge/types" "github.com/woodpecker-ci/woodpecker/server/model" ) @@ -37,7 +39,7 @@ func Test_parser(t *testing.T) { r, b, err := parseHook(req) g.Assert(r).IsNil() g.Assert(b).IsNil() - g.Assert(err).IsNil() + assert.ErrorIs(t, err, &types.ErrIgnoreEvent{}) }) g.Describe("Given a pull request hook payload", func() { diff --git a/server/forge/forge.go b/server/forge/forge.go index 6f4fcd1eb..fea060102 100644 --- a/server/forge/forge.go +++ b/server/forge/forge.go @@ -85,7 +85,7 @@ type Forge interface { // Hook parses the post-commit hook from the Request body and returns the // required data in a standard format. - Hook(ctx context.Context, r *http.Request) (*model.Repo, *model.Pipeline, error) + Hook(ctx context.Context, r *http.Request) (repo *model.Repo, pipeline *model.Pipeline, err error) // OrgMembership returns if user is member of organization and if user // is admin/owner in that organization. diff --git a/server/forge/gitea/gitea.go b/server/forge/gitea/gitea.go index 640869500..9f11fd92c 100644 --- a/server/forge/gitea/gitea.go +++ b/server/forge/gitea/gitea.go @@ -489,11 +489,6 @@ func (c *Gitea) Hook(ctx context.Context, r *http.Request) (*model.Repo, *model. return nil, nil, err } - if repo == nil || pipeline == nil { - // ignore hook - return nil, nil, nil - } - if pipeline.Event == model.EventPull && len(pipeline.ChangedFiles) == 0 { index, err := strconv.ParseInt(strings.Split(pipeline.Ref, "/")[2], 10, 64) if err != nil { diff --git a/server/forge/gitea/parse.go b/server/forge/gitea/parse.go index d617a21cf..fe309a131 100644 --- a/server/forge/gitea/parse.go +++ b/server/forge/gitea/parse.go @@ -22,6 +22,7 @@ import ( "github.com/rs/zerolog/log" + "github.com/woodpecker-ci/woodpecker/server/forge/types" "github.com/woodpecker-ci/woodpecker/server/model" ) @@ -53,7 +54,7 @@ func parseHook(r *http.Request) (*model.Repo, *model.Pipeline, error) { return parsePullRequestHook(r.Body) } log.Debug().Msgf("unsuported hook type: '%s'", hookType) - return nil, nil, nil + return nil, nil, &types.ErrIgnoreEvent{Event: hookType} } // parsePushHook parses a push hook and returns the Repo and Pipeline details. diff --git a/server/forge/gitea/parse_test.go b/server/forge/gitea/parse_test.go index d6191fc63..23f0c95aa 100644 --- a/server/forge/gitea/parse_test.go +++ b/server/forge/gitea/parse_test.go @@ -21,8 +21,10 @@ import ( "testing" "github.com/franela/goblin" + "github.com/stretchr/testify/assert" "github.com/woodpecker-ci/woodpecker/server/forge/gitea/fixtures" + "github.com/woodpecker-ci/woodpecker/server/forge/types" "github.com/woodpecker-ci/woodpecker/server/model" "github.com/woodpecker-ci/woodpecker/shared/utils" ) @@ -38,7 +40,7 @@ func Test_parser(t *testing.T) { r, b, err := parseHook(req) g.Assert(r).IsNil() g.Assert(b).IsNil() - g.Assert(err).IsNil() + assert.ErrorIs(t, err, &types.ErrIgnoreEvent{}) }) g.It("given a PR hook", func() { buf := bytes.NewBufferString(fixtures.HookPullRequest) diff --git a/server/forge/github/parse.go b/server/forge/github/parse.go index ebdd95a70..799bbb163 100644 --- a/server/forge/github/parse.go +++ b/server/forge/github/parse.go @@ -24,6 +24,7 @@ import ( "github.com/google/go-github/v39/github" + "github.com/woodpecker-ci/woodpecker/server/forge/types" "github.com/woodpecker-ci/woodpecker/server/model" "github.com/woodpecker-ci/woodpecker/shared/utils" ) @@ -65,8 +66,9 @@ func parseHook(r *http.Request, merge bool) (*github.PullRequest, *model.Repo, * return nil, repo, pipeline, err case *github.PullRequestEvent: return parsePullHook(hook, merge) + default: + return nil, nil, nil, &types.ErrIgnoreEvent{Event: github.Stringify(hook)} } - return nil, nil, nil, nil } // parsePushHook parses a push hook and returns the Repo and Pipeline details. diff --git a/server/forge/github/parse_test.go b/server/forge/github/parse_test.go index 1988dd3ee..6074d19ad 100644 --- a/server/forge/github/parse_test.go +++ b/server/forge/github/parse_test.go @@ -22,8 +22,10 @@ import ( "testing" "github.com/franela/goblin" + "github.com/stretchr/testify/assert" "github.com/woodpecker-ci/woodpecker/server/forge/github/fixtures" + "github.com/woodpecker-ci/woodpecker/server/forge/types" "github.com/woodpecker-ci/woodpecker/server/model" ) @@ -50,8 +52,8 @@ func Test_parser(t *testing.T) { p, r, b, err := parseHook(req, false) g.Assert(r).IsNil() g.Assert(b).IsNil() - g.Assert(err).IsNil() g.Assert(p).IsNil() + assert.ErrorIs(t, err, &types.ErrIgnoreEvent{}) }) g.Describe("given a push hook", func() { diff --git a/server/forge/gitlab/gitlab.go b/server/forge/gitlab/gitlab.go index a7d302e68..310284163 100644 --- a/server/forge/gitlab/gitlab.go +++ b/server/forge/gitlab/gitlab.go @@ -33,6 +33,7 @@ import ( "github.com/woodpecker-ci/woodpecker/server" "github.com/woodpecker-ci/woodpecker/server/forge" "github.com/woodpecker-ci/woodpecker/server/forge/common" + "github.com/woodpecker-ci/woodpecker/server/forge/types" forge_types "github.com/woodpecker-ci/woodpecker/server/forge/types" "github.com/woodpecker-ci/woodpecker/server/model" "github.com/woodpecker-ci/woodpecker/server/store" @@ -596,7 +597,8 @@ func (g *GitLab) Hook(ctx context.Context, req *http.Request) (*model.Repo, *mod return nil, nil, err } - parsed, err := gitlab.ParseWebhook(gitlab.WebhookEventType(req), payload) + eventType := gitlab.WebhookEventType(req) + parsed, err := gitlab.ParseWebhook(eventType, payload) if err != nil { return nil, nil, err } @@ -618,7 +620,7 @@ func (g *GitLab) Hook(ctx context.Context, req *http.Request) (*model.Repo, *mod case *gitlab.TagEvent: return convertTagHook(event) default: - return nil, nil, nil + return nil, nil, &types.ErrIgnoreEvent{Event: string(eventType)} } } diff --git a/server/forge/mocks/forge.go b/server/forge/mocks/forge.go index 4f7c2e3c1..8867fcba2 100644 --- a/server/forge/mocks/forge.go +++ b/server/forge/mocks/forge.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.31.1. DO NOT EDIT. +// Code generated by mockery v2.32.0. DO NOT EDIT. package mocks diff --git a/server/forge/types/errors.go b/server/forge/types/errors.go index 47aae2f62..7fd257cb9 100644 --- a/server/forge/types/errors.go +++ b/server/forge/types/errors.go @@ -15,7 +15,10 @@ package types -import "errors" +import ( + "errors" + "fmt" +) // AuthError represents forge authentication error. type AuthError struct { @@ -40,3 +43,16 @@ func (ae *AuthError) Error() string { var _ error = new(AuthError) var ErrNotImplemented = errors.New("not implemented") + +type ErrIgnoreEvent struct { + Event string +} + +func (err *ErrIgnoreEvent) Error() string { + return fmt.Sprintf("explicit ignored event '%s'", err.Event) +} + +func (*ErrIgnoreEvent) Is(target error) bool { + _, ok := target.(*ErrIgnoreEvent) //nolint:errorlint + return ok +} diff --git a/server/store/mocks/store.go b/server/store/mocks/store.go index 47ce43184..818dd0265 100644 --- a/server/store/mocks/store.go +++ b/server/store/mocks/store.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.31.1. DO NOT EDIT. +// Code generated by mockery v2.32.0. DO NOT EDIT. package mocks