let HookParse func explicit ignore events (#1942)

for now it's not clear defined, what to do on an unsupported event.
e.g. gitea webhook panel shows 500 error and no message.

now we have a successful webhook and a message to show an info
This commit is contained in:
6543 2023-07-14 02:03:54 +02:00 committed by GitHub
parent 004d72a853
commit d9991e67e5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 57 additions and 19 deletions

View file

@ -19,6 +19,7 @@
package api package api
import ( import (
"errors"
"fmt" "fmt"
"net/http" "net/http"
"regexp" "regexp"
@ -27,6 +28,7 @@ import (
"github.com/rs/zerolog/log" "github.com/rs/zerolog/log"
"github.com/woodpecker-ci/woodpecker/server" "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/model"
"github.com/woodpecker-ci/woodpecker/server/pipeline" "github.com/woodpecker-ci/woodpecker/server/pipeline"
"github.com/woodpecker-ci/woodpecker/server/store" "github.com/woodpecker-ci/woodpecker/server/store"
@ -108,11 +110,19 @@ func PostHook(c *gin.Context) {
tmpRepo, tmpBuild, err := forge.Hook(c, c.Request) tmpRepo, tmpBuild, err := forge.Hook(c, c.Request)
if err != nil { 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" msg := "failure to parse hook"
log.Debug().Err(err).Msg(msg) log.Debug().Err(err).Msg(msg)
c.String(http.StatusBadRequest, msg) c.String(http.StatusBadRequest, msg)
return return
} }
if tmpBuild == nil { if tmpBuild == nil {
msg := "ignoring hook: hook parsing resulted in empty pipeline" msg := "ignoring hook: hook parsing resulted in empty pipeline"
log.Debug().Msg(msg) log.Debug().Msg(msg)

View file

@ -20,6 +20,7 @@ import (
"net/http" "net/http"
"github.com/woodpecker-ci/woodpecker/server/forge/bitbucket/internal" "github.com/woodpecker-ci/woodpecker/server/forge/bitbucket/internal"
"github.com/woodpecker-ci/woodpecker/server/forge/types"
"github.com/woodpecker-ci/woodpecker/server/model" "github.com/woodpecker-ci/woodpecker/server/model"
) )
@ -34,15 +35,20 @@ const (
// parseHook parses a Bitbucket hook from an http.Request request and returns // 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. // Repo and Pipeline detail. If a hook type is unsupported nil values are returned.
func parseHook(r *http.Request) (*model.Repo, *model.Pipeline, error) { 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: case hookPush:
return parsePushHook(payload) return parsePushHook(payload)
case hookPullCreated, hookPullUpdated: case hookPullCreated, hookPullUpdated:
return parsePullHook(payload) 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. // parsePushHook parses a push hook and returns the Repo and Pipeline details.

View file

@ -20,8 +20,10 @@ import (
"testing" "testing"
"github.com/franela/goblin" "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/bitbucket/fixtures"
"github.com/woodpecker-ci/woodpecker/server/forge/types"
"github.com/woodpecker-ci/woodpecker/server/model" "github.com/woodpecker-ci/woodpecker/server/model"
) )
@ -37,7 +39,7 @@ func Test_parser(t *testing.T) {
r, b, err := parseHook(req) r, b, err := parseHook(req)
g.Assert(r).IsNil() g.Assert(r).IsNil()
g.Assert(b).IsNil() g.Assert(b).IsNil()
g.Assert(err).IsNil() assert.ErrorIs(t, err, &types.ErrIgnoreEvent{})
}) })
g.Describe("Given a pull request hook payload", func() { g.Describe("Given a pull request hook payload", func() {

View file

@ -85,7 +85,7 @@ type Forge interface {
// Hook parses the post-commit hook from the Request body and returns the // Hook parses the post-commit hook from the Request body and returns the
// required data in a standard format. // 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 // OrgMembership returns if user is member of organization and if user
// is admin/owner in that organization. // is admin/owner in that organization.

View file

@ -489,11 +489,6 @@ func (c *Gitea) Hook(ctx context.Context, r *http.Request) (*model.Repo, *model.
return nil, nil, err return nil, nil, err
} }
if repo == nil || pipeline == nil {
// ignore hook
return nil, nil, nil
}
if pipeline.Event == model.EventPull && len(pipeline.ChangedFiles) == 0 { if pipeline.Event == model.EventPull && len(pipeline.ChangedFiles) == 0 {
index, err := strconv.ParseInt(strings.Split(pipeline.Ref, "/")[2], 10, 64) index, err := strconv.ParseInt(strings.Split(pipeline.Ref, "/")[2], 10, 64)
if err != nil { if err != nil {

View file

@ -22,6 +22,7 @@ import (
"github.com/rs/zerolog/log" "github.com/rs/zerolog/log"
"github.com/woodpecker-ci/woodpecker/server/forge/types"
"github.com/woodpecker-ci/woodpecker/server/model" "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) return parsePullRequestHook(r.Body)
} }
log.Debug().Msgf("unsuported hook type: '%s'", hookType) 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. // parsePushHook parses a push hook and returns the Repo and Pipeline details.

View file

@ -21,8 +21,10 @@ import (
"testing" "testing"
"github.com/franela/goblin" "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/gitea/fixtures"
"github.com/woodpecker-ci/woodpecker/server/forge/types"
"github.com/woodpecker-ci/woodpecker/server/model" "github.com/woodpecker-ci/woodpecker/server/model"
"github.com/woodpecker-ci/woodpecker/shared/utils" "github.com/woodpecker-ci/woodpecker/shared/utils"
) )
@ -38,7 +40,7 @@ func Test_parser(t *testing.T) {
r, b, err := parseHook(req) r, b, err := parseHook(req)
g.Assert(r).IsNil() g.Assert(r).IsNil()
g.Assert(b).IsNil() g.Assert(b).IsNil()
g.Assert(err).IsNil() assert.ErrorIs(t, err, &types.ErrIgnoreEvent{})
}) })
g.It("given a PR hook", func() { g.It("given a PR hook", func() {
buf := bytes.NewBufferString(fixtures.HookPullRequest) buf := bytes.NewBufferString(fixtures.HookPullRequest)

View file

@ -24,6 +24,7 @@ import (
"github.com/google/go-github/v39/github" "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/server/model"
"github.com/woodpecker-ci/woodpecker/shared/utils" "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 return nil, repo, pipeline, err
case *github.PullRequestEvent: case *github.PullRequestEvent:
return parsePullHook(hook, merge) 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. // parsePushHook parses a push hook and returns the Repo and Pipeline details.

View file

@ -22,8 +22,10 @@ import (
"testing" "testing"
"github.com/franela/goblin" "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/github/fixtures"
"github.com/woodpecker-ci/woodpecker/server/forge/types"
"github.com/woodpecker-ci/woodpecker/server/model" "github.com/woodpecker-ci/woodpecker/server/model"
) )
@ -50,8 +52,8 @@ func Test_parser(t *testing.T) {
p, r, b, err := parseHook(req, false) p, r, b, err := parseHook(req, false)
g.Assert(r).IsNil() g.Assert(r).IsNil()
g.Assert(b).IsNil() g.Assert(b).IsNil()
g.Assert(err).IsNil()
g.Assert(p).IsNil() g.Assert(p).IsNil()
assert.ErrorIs(t, err, &types.ErrIgnoreEvent{})
}) })
g.Describe("given a push hook", func() { g.Describe("given a push hook", func() {

View file

@ -33,6 +33,7 @@ import (
"github.com/woodpecker-ci/woodpecker/server" "github.com/woodpecker-ci/woodpecker/server"
"github.com/woodpecker-ci/woodpecker/server/forge" "github.com/woodpecker-ci/woodpecker/server/forge"
"github.com/woodpecker-ci/woodpecker/server/forge/common" "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" forge_types "github.com/woodpecker-ci/woodpecker/server/forge/types"
"github.com/woodpecker-ci/woodpecker/server/model" "github.com/woodpecker-ci/woodpecker/server/model"
"github.com/woodpecker-ci/woodpecker/server/store" "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 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 { if err != nil {
return nil, nil, err return nil, nil, err
} }
@ -618,7 +620,7 @@ func (g *GitLab) Hook(ctx context.Context, req *http.Request) (*model.Repo, *mod
case *gitlab.TagEvent: case *gitlab.TagEvent:
return convertTagHook(event) return convertTagHook(event)
default: default:
return nil, nil, nil return nil, nil, &types.ErrIgnoreEvent{Event: string(eventType)}
} }
} }

View file

@ -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 package mocks

View file

@ -15,7 +15,10 @@
package types package types
import "errors" import (
"errors"
"fmt"
)
// AuthError represents forge authentication error. // AuthError represents forge authentication error.
type AuthError struct { type AuthError struct {
@ -40,3 +43,16 @@ func (ae *AuthError) Error() string {
var _ error = new(AuthError) var _ error = new(AuthError)
var ErrNotImplemented = errors.New("not implemented") 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
}

View file

@ -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 package mocks