diff --git a/connectors/github/builder.go b/connectors/github/builder.go index aaa4944..638dd2f 100644 --- a/connectors/github/builder.go +++ b/connectors/github/builder.go @@ -9,27 +9,13 @@ import ( "io" "io/ioutil" "net/http" - "path/filepath" - - "strings" - - "errors" "github.com/bivas/rivi/config/client" "github.com/bivas/rivi/types" "github.com/bivas/rivi/types/builder" - "github.com/bivas/rivi/util" "github.com/bivas/rivi/util/log" ) -var ( - supportedEventTypes = []string{ - "issue_comment", - "pull_request", - "pull_request_review", - "pull_request_review_comment"} -) - type builderContext struct { secret []byte client *ghClient @@ -37,27 +23,29 @@ type builderContext struct { } type dataBuilder struct { - logger log.Logger + handlers map[string]eventHandler + defaultHandler eventHandler + logger log.Logger } -func (builder *dataBuilder) validate(context *builderContext, payload []byte, request *http.Request) bool { - if len(context.secret) == 0 { +func validate(secret, payload []byte, request *http.Request) bool { + if len(secret) == 0 { return true } - h := hmac.New(sha1.New, context.secret) + h := hmac.New(sha1.New, secret) h.Write(payload) result := fmt.Sprintf("sha1=%s", hex.EncodeToString(h.Sum(nil))) return request.Header.Get("X-Hub-Signature") == result } -func (builder *dataBuilder) readPayload(context *builderContext, r *http.Request) (*payload, []byte, error) { +func ReadPayload(secret []byte, r *http.Request) (*payload, []byte, error) { body := r.Body defer body.Close() raw, err := ioutil.ReadAll(io.LimitReader(body, r.ContentLength)) if err != nil { return nil, raw, err } - if !builder.validate(context, raw, r) { + if !validate(secret, raw, r) { return nil, raw, fmt.Errorf("Payload could not be validated") } var pr payload @@ -67,120 +55,41 @@ func (builder *dataBuilder) readPayload(context *builderContext, r *http.Request return &pr, raw, nil } -func (builder *dataBuilder) readFromJson(context *builderContext, payload *payload) { - pr := payload.PullRequest - if pr.Number > 0 { - context.data.number = pr.Number - } else { - context.data.number = payload.Number - } - context.data.title = pr.Title - context.data.description = pr.Body - context.data.changedFiles = pr.ChangedFiles - context.data.additions = pr.Additions - context.data.deletions = pr.Deletions - context.data.ref = pr.Base.Ref - head := pr.Head - context.data.origin = types.Origin{ - User: strings.ToLower(head.User.Login), - Repo: head.Repo.Name, - Ref: head.Ref, - Head: head.Sha[0:6], - GitURL: head.Repo.GitURL, - } - context.data.state = pr.State -} - -func (builder *dataBuilder) readFromClient(context *builderContext) { - id := context.data.number - context.data.assignees = context.client.GetAssignees(id) - context.data.state = context.client.GetState(id) - context.data.labels = context.client.GetLabels(id) - context.data.comments = context.client.GetComments(id) - fileNames := context.client.GetFileNames(id) - context.data.fileNames = fileNames - stringSet := util.StringSet{Transformer: filepath.Ext} - context.data.fileExt = stringSet.AddAll(fileNames).Values() - context.data.reviewers = context.client.GetReviewers(id) - context.data.locked = context.client.Locked(id) -} - -func (builder *dataBuilder) checkProcessState(context *builderContext) bool { - builder.logger.DebugWith(log.MetaFields{log.F("issue", context.data.GetShortName())}, - "Current state is '%s'", context.data.state) - return context.data.state != "closed" +func (builder *dataBuilder) findEventHandler(githubEvent string) eventHandler { + handler, ok := builder.handlers[githubEvent] + if !ok { + builder.logger.DebugWith(log.MetaFields{ + log.F("eventType", githubEvent), + }, "Using default event handler") + handler = builder.defaultHandler + } + return handler } func (builder *dataBuilder) BuildFromHook(config client.ClientConfig, r *http.Request) (types.HookData, bool, error) { githubEvent := r.Header.Get("X-Github-Event") - if githubEvent == "ping" { - builder.logger.Info("Got GitHub 'ping' event") - return nil, false, nil - } - supportedEvent := false - for _, event := range supportedEventTypes { - if event == githubEvent { - supportedEvent = true - } - } - if !supportedEvent { - builder.logger.Debug("Got GitHub '%s' event", githubEvent) - return nil, false, nil - } - context := &builderContext{secret: []byte(config.GetSecret())} - pl, raw, err := builder.readPayload(context, r) - if err != nil { - return nil, false, err - } - if pl.Number == 0 && pl.PullRequest.Number == 0 { - builder.logger.Warning("Payload appear to have issue id 0") - builder.logger.Debug("Faulty payload %+v", pl) - return nil, false, fmt.Errorf("Payload appear to have issue id 0") - } - repo := pl.Repository.Name - owner := pl.Repository.Owner.Login - installation := pl.Installation.ID - if installation > 0 { - context.client = newAppClient(config, owner, repo, installation) - } else { - context.client = newClient(config, owner, repo) - } - if context.client == nil { - return nil, false, errors.New("Unable to initialize github client") - } - context.data = &data{owner: owner, repo: repo, payload: raw, client: context.client} - builder.readFromJson(context, pl) - return context.data, builder.checkProcessState(context), nil + return builder.findEventHandler(githubEvent).FromRequest(config, r) } -func (builder *dataBuilder) BuildFromPayload(config client.ClientConfig, raw []byte) (types.Data, bool, error) { - var pl payload - if e := json.Unmarshal(raw, &pl); e != nil { - return nil, false, e - } - repo := pl.Repository.Name - owner := pl.Repository.Owner.Login - installation := pl.Installation.ID - context := &builderContext{} - if installation > 0 { - context.client = newAppClient(config, owner, repo, installation) - } else { - context.client = newClient(config, owner, repo) - } - if context.client == nil { - return nil, false, errors.New("Unable to initialize github client") - } - context.data = &data{owner: owner, repo: repo, payload: raw, client: context.client} - builder.readFromJson(context, &pl) - if context.data.GetNumber() == 0 { - builder.logger.Warning("Payload appear to have issue id 0") - builder.logger.Debug("Faulty payload %+v", pl) - return nil, false, fmt.Errorf("Payload appear to have issue id 0") - } - builder.readFromClient(context) - return context.data, builder.checkProcessState(context), nil +func (builder *dataBuilder) BuildFromPayload(config client.ClientConfig, ofType string, raw []byte) (types.Data, bool, error) { + return builder.findEventHandler(ofType).FromPayload(config, raw) } +var DataBuilder dataBuilder + func init() { - builder.RegisterNewDataBuilder("github", &dataBuilder{logger: log.Get("GitHub.DataBuilder")}) + logger := log.Get("GitHub.DataBuilder") + prHandler := &pullRequestEventHandler{ + logger: logger.Get("PullRequestHandler"), + } + DataBuilder = dataBuilder{ + logger: logger, + handlers: map[string]eventHandler{ + "pull_request": prHandler, + "pull_request_review": prHandler, + "pull_request_review_comment": prHandler, + }, + defaultHandler: defaultHandler, + } + builder.RegisterNewDataBuilder("github", &DataBuilder) } diff --git a/connectors/github/builder_test.go b/connectors/github/builder_test.go new file mode 100644 index 0000000..a7c4e23 --- /dev/null +++ b/connectors/github/builder_test.go @@ -0,0 +1,50 @@ +package github + +import ( + "net/http" + "testing" + + "github.com/bivas/rivi/config/client" + "github.com/bivas/rivi/mocks" + "github.com/bivas/rivi/types" + "github.com/stretchr/testify/assert" +) + +type mockEventHandler struct { + FromRequestCalled bool + FromPayloadCalled bool +} + +func (m *mockEventHandler) FromRequest(client.ClientConfig, *http.Request) (types.HookData, bool, error) { + m.FromRequestCalled = true + return nil, false, nil +} + +func (m *mockEventHandler) FromPayload(client.ClientConfig, []byte) (types.Data, bool, error) { + m.FromPayloadCalled = true + return nil, false, nil +} + +func TestRequestDefault(t *testing.T) { + DataBuilder.handlers["mock"] = &mockEventHandler{} + DataBuilder.defaultHandler = &mockEventHandler{} + + request, err := http.NewRequest("GET", "http://example.com", nil) + assert.NoError(t, err, "shouldn't error") + _, cont, _ := DataBuilder.BuildFromHook(&mocks.MockClientConfig{}, request) + assert.False(t, cont, "shouldn't continue") + assert.True(t, DataBuilder.defaultHandler.(*mockEventHandler).FromRequestCalled) +} + +func TestRequest(t *testing.T) { + DataBuilder.handlers["mock"] = &mockEventHandler{} + DataBuilder.defaultHandler = &mockEventHandler{} + + request, err := http.NewRequest("GET", "http://example.com", nil) + assert.NoError(t, err, "shouldn't error") + request.Header.Set("X-Github-Event", "mock") + _, cont, _ := DataBuilder.BuildFromHook(&mocks.MockClientConfig{}, request) + assert.False(t, cont, "shouldn't continue") + assert.False(t, DataBuilder.defaultHandler.(*mockEventHandler).FromRequestCalled) + assert.True(t, DataBuilder.handlers["mock"].(*mockEventHandler).FromRequestCalled) +} diff --git a/connectors/github/client.go b/connectors/github/client.go index 54bf155..3ac4859 100644 --- a/connectors/github/client.go +++ b/connectors/github/client.go @@ -266,7 +266,7 @@ func (c *ghClient) Merge(issue int, mergeMethod string) { } } -func newClient(config client.ClientConfig, owner, repo string) *ghClient { +func NewClient(config client.ClientConfig, owner, repo string) *ghClient { logger := log.Get("Github.Client") if config.GetOAuthToken() == "" { logger.ErrorWith( @@ -291,7 +291,7 @@ func newClient(config client.ClientConfig, owner, repo string) *ghClient { } } -func newAppClient(config client.ClientConfig, owner, repo string, installation int) *ghClient { +func NewAppClient(config client.ClientConfig, owner, repo string, installation int) *ghClient { logger := log.Get("Github.Client") if config.GetApplicationID() == 0 || config.GetPrivateKeyFile() == "" { logger.ErrorWith( diff --git a/connectors/github/data.go b/connectors/github/data.go index 00d2414..5525de1 100644 --- a/connectors/github/data.go +++ b/connectors/github/data.go @@ -27,6 +27,7 @@ type data struct { assignees []string comments []types.Comment payload []byte + eventType string reviewers map[string]string collaborators []string repoLabels []string @@ -116,6 +117,10 @@ func (d *data) GetRawPayload() []byte { return d.payload } +func (d *data) GetRawType() string { + return d.eventType +} + func (d *data) Merge(mergeMethod string) { d.client.Merge(d.number, mergeMethod) } diff --git a/connectors/github/handler.go b/connectors/github/handler.go new file mode 100644 index 0000000..5e92b1b --- /dev/null +++ b/connectors/github/handler.go @@ -0,0 +1,31 @@ +package github + +import ( + "net/http" + + "github.com/bivas/rivi/config/client" + "github.com/bivas/rivi/types" + "github.com/bivas/rivi/util/log" +) + +type eventHandler interface { + FromRequest(client.ClientConfig, *http.Request) (types.HookData, bool, error) + FromPayload(client.ClientConfig, []byte) (types.Data, bool, error) +} + +type defaultEventHandler struct { + logger log.Logger +} + +func (h *defaultEventHandler) FromRequest(config client.ClientConfig, r *http.Request) (types.HookData, bool, error) { + githubEvent := r.Header.Get("X-Github-Event") + h.logger.Info("Got GitHub '%s' event", githubEvent) + return nil, false, nil +} + +func (h *defaultEventHandler) FromPayload(client.ClientConfig, []byte) (types.Data, bool, error) { + h.logger.Warning("Calling 'FromPayload' of default handler") + return nil, false, nil +} + +var defaultHandler = &defaultEventHandler{log.Get("GitHub.DataBuilder.DefaultHandler")} diff --git a/connectors/github/json.go b/connectors/github/json.go index 067a3fc..2e07119 100644 --- a/connectors/github/json.go +++ b/connectors/github/json.go @@ -35,6 +35,7 @@ type repositorySection struct { Owner struct { Login string `json:"login"` } `json:"owner"` + Private bool `json:"private"` } type payload struct { diff --git a/connectors/github/pullrequest_handler.go b/connectors/github/pullrequest_handler.go new file mode 100644 index 0000000..2dbb323 --- /dev/null +++ b/connectors/github/pullrequest_handler.go @@ -0,0 +1,123 @@ +package github + +import ( + "encoding/json" + "errors" + "fmt" + "net/http" + "path/filepath" + "strings" + + "github.com/bivas/rivi/config/client" + "github.com/bivas/rivi/types" + "github.com/bivas/rivi/util" + "github.com/bivas/rivi/util/log" +) + +type pullRequestEventHandler struct { + logger log.Logger +} + +func (h *pullRequestEventHandler) readFromJson(context *builderContext, payload *payload) { + pr := payload.PullRequest + if pr.Number > 0 { + context.data.number = pr.Number + } else { + context.data.number = payload.Number + } + context.data.title = pr.Title + context.data.description = pr.Body + context.data.changedFiles = pr.ChangedFiles + context.data.additions = pr.Additions + context.data.deletions = pr.Deletions + context.data.ref = pr.Base.Ref + head := pr.Head + context.data.origin = types.Origin{ + User: strings.ToLower(head.User.Login), + Repo: head.Repo.Name, + Ref: head.Ref, + Head: head.Sha[0:6], + GitURL: head.Repo.GitURL, + } + context.data.state = pr.State +} + +func (h *pullRequestEventHandler) readFromClient(context *builderContext) { + id := context.data.number + context.data.assignees = context.client.GetAssignees(id) + context.data.state = context.client.GetState(id) + context.data.labels = context.client.GetLabels(id) + context.data.comments = context.client.GetComments(id) + fileNames := context.client.GetFileNames(id) + context.data.fileNames = fileNames + stringSet := util.StringSet{Transformer: filepath.Ext} + context.data.fileExt = stringSet.AddAll(fileNames).Values() + context.data.reviewers = context.client.GetReviewers(id) + context.data.locked = context.client.Locked(id) +} + +func (h *pullRequestEventHandler) checkProcessState(context *builderContext) bool { + h.logger.DebugWith( + log.MetaFields{ + log.F("issue", context.data.GetShortName())}, + "Current state is '%s'", context.data.state) + return context.data.state != "closed" +} + +func (h *pullRequestEventHandler) FromRequest(config client.ClientConfig, r *http.Request) (types.HookData, bool, error) { + context := &builderContext{secret: []byte(config.GetSecret())} + pl, raw, err := ReadPayload(context.secret, r) + if err != nil { + return nil, false, err + } + if pl.Number == 0 && pl.PullRequest.Number == 0 { + h.logger.Warning("Payload appear to have issue id 0") + h.logger.Debug("Faulty payload %+v", pl) + return nil, false, fmt.Errorf("Payload appear to have issue id 0") + } + repo := pl.Repository.Name + owner := pl.Repository.Owner.Login + installation := pl.Installation.ID + if installation > 0 { + context.client = NewAppClient(config, owner, repo, installation) + } else { + context.client = NewClient(config, owner, repo) + } + if context.client == nil { + return nil, false, errors.New("Unable to initialize github client") + } + context.data = &data{ + owner: owner, repo: repo, + payload: raw, eventType: r.Header.Get("X-Github-Event"), + client: context.client} + h.readFromJson(context, pl) + return context.data, h.checkProcessState(context), nil +} + +func (h *pullRequestEventHandler) FromPayload(config client.ClientConfig, raw []byte) (types.Data, bool, error) { + var pl payload + if e := json.Unmarshal(raw, &pl); e != nil { + return nil, false, e + } + repo := pl.Repository.Name + owner := pl.Repository.Owner.Login + installation := pl.Installation.ID + context := &builderContext{} + if installation > 0 { + context.client = NewAppClient(config, owner, repo, installation) + } else { + context.client = NewClient(config, owner, repo) + } + if context.client == nil { + return nil, false, errors.New("Unable to initialize github client") + } + context.data = &data{owner: owner, repo: repo, payload: raw, client: context.client} + h.readFromJson(context, &pl) + if context.data.GetNumber() == 0 { + h.logger.Warning("Payload appear to have issue id 0") + h.logger.Debug("Faulty payload %+v", pl) + return nil, false, fmt.Errorf("Payload appear to have issue id 0") + } + h.readFromClient(context) + return context.data, h.checkProcessState(context), nil +} diff --git a/engine/mock.go b/engine/mock.go index e7158e3..2d69d5a 100644 --- a/engine/mock.go +++ b/engine/mock.go @@ -30,6 +30,11 @@ type mockData struct { ChangesAdd int ChangesRemove int RawPayload []byte + EventType string +} + +func (m *mockData) GetRawType() string { + return m.EventType } func (m *mockData) GetCollaborators() []string { diff --git a/mocks/data.go b/mocks/data.go index bbe41dd..ed082cc 100644 --- a/mocks/data.go +++ b/mocks/data.go @@ -34,6 +34,11 @@ type MockData struct { Collaborators []string AvailableLabels []string RulesFileContent string + EventType string +} + +func (m *MockData) GetRawType() string { + return m.EventType } func (m *MockData) GetCollaborators() []string { diff --git a/runner/runner_test.go b/runner/runner_test.go index 9579979..5f34d25 100644 --- a/runner/runner_test.go +++ b/runner/runner_test.go @@ -18,7 +18,7 @@ type mockDataBuilder struct { Provider string } -func (m *mockDataBuilder) BuildFromPayload(config client.ClientConfig, payload []byte) (types.Data, bool, error) { +func (m *mockDataBuilder) BuildFromPayload(config client.ClientConfig, ofType string, payload []byte) (types.Data, bool, error) { return &mocks.MockData{Labels: m.Labels, Provider: strings.ToLower(m.Provider)}, true, nil } diff --git a/server/server.go b/server/server.go index ed46b91..8a9df1e 100644 --- a/server/server.go +++ b/server/server.go @@ -32,9 +32,10 @@ func (server *BotServer) initDefaults() { } func (server *BotServer) registerMetrics() { - registry := prometheus.NewRegistry() - registry.Register(prometheus.NewGoCollector()) - server.engine.Any("/metrics", gin.WrapH(promhttp.HandlerFor(registry, promhttp.HandlerOpts{}))) + prometheus.DefaultRegisterer.Register(prometheus.NewGoCollector()) + server.engine.Any("/metrics", gin.WrapH( + promhttp.HandlerFor( + prometheus.DefaultGatherer, promhttp.HandlerOpts{}))) } func (server *BotServer) registerDefaultHandler() { diff --git a/types/builder/data.go b/types/builder/data.go index 6ed1505..6f5b1ff 100644 --- a/types/builder/data.go +++ b/types/builder/data.go @@ -13,7 +13,7 @@ var l log.Logger = log.Get("data.builder") type DataBuilder interface { BuildFromHook(config client.ClientConfig, r *http.Request) (types.HookData, bool, error) - BuildFromPayload(config client.ClientConfig, payload []byte) (types.Data, bool, error) + BuildFromPayload(config client.ClientConfig, ofType string, payload []byte) (types.Data, bool, error) } var builders map[string]DataBuilder = make(map[string]DataBuilder) @@ -59,7 +59,7 @@ func BuildComplete(config client.ClientConfig, data types.ReadOnlyData) (types.D l.Error("No existing builder to work with!") return nil, false } - result, process, err := builder.BuildFromPayload(config, data.GetRawPayload()) + result, process, err := builder.BuildFromPayload(config, data.GetRawType(), data.GetRawPayload()) if err != nil { l.ErrorWith(log.MetaFields{log.E(err)}, "Unable to build from payload.") return nil, false diff --git a/types/data.go b/types/data.go index 89f0281..d6866a9 100644 --- a/types/data.go +++ b/types/data.go @@ -9,6 +9,7 @@ type InfoData interface { type RawData interface { GetRawPayload() []byte + GetRawType() string } type ReadOnlyData interface {