From aea0e3a104ab6e3bbb61142f90d83437dd0af130 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Tue, 30 Aug 2016 11:40:08 +0100 Subject: [PATCH] Split up github service into 2 services One for commands/expansions, the other for webhooks. Call the type 'github-webhook'. --- src/github.com/matrix-org/go-neb/api.go | 5 + .../go-neb/services/github/github.go | 276 +------------- .../go-neb/services/github/github_webhook.go | 339 ++++++++++++++++++ 3 files changed, 350 insertions(+), 270 deletions(-) create mode 100644 src/github.com/matrix-org/go-neb/services/github/github_webhook.go diff --git a/src/github.com/matrix-org/go-neb/api.go b/src/github.com/matrix-org/go-neb/api.go index 3796f2e..57685f0 100644 --- a/src/github.com/matrix-org/go-neb/api.go +++ b/src/github.com/matrix-org/go-neb/api.go @@ -240,6 +240,11 @@ func (s *configureServiceHandler) OnIncomingRequest(req *http.Request) (interfac if httpErr != nil { return nil, httpErr } + log.WithFields(log.Fields{ + "service_id": service.ServiceID(), + "service_type": service.ServiceType(), + "service_user_id": service.ServiceUserID(), + }).Print("Incoming configure service request") // Have mutexes around each service to queue up multiple requests for the same service ID mut := s.getMutexForServiceID(service.ServiceID()) diff --git a/src/github.com/matrix-org/go-neb/services/github/github.go b/src/github.com/matrix-org/go-neb/services/github/github.go index d0c200c..c68d6aa 100644 --- a/src/github.com/matrix-org/go-neb/services/github/github.go +++ b/src/github.com/matrix-org/go-neb/services/github/github.go @@ -10,12 +10,9 @@ import ( "github.com/matrix-org/go-neb/plugin" "github.com/matrix-org/go-neb/realms/github" "github.com/matrix-org/go-neb/services/github/client" - "github.com/matrix-org/go-neb/services/github/webhook" "github.com/matrix-org/go-neb/types" - "github.com/matrix-org/go-neb/util" "net/http" "regexp" - "sort" "strconv" "strings" ) @@ -25,28 +22,15 @@ import ( var ownerRepoIssueRegex = regexp.MustCompile(`(([A-z0-9-_]+)/([A-z0-9-_]+))?#([0-9]+)`) type githubService struct { - id string - serviceUserID string - webhookEndpointURL string - ClientUserID string // optional; required for webhooks - RealmID string - SecretToken string - HandleCommands bool - HandleExpansions bool - Rooms map[string]struct { // room_id => {} - Repos map[string]struct { // owner/repo => { events: ["push","issue","pull_request"] } - Events []string - } - } + id string + serviceUserID string + RealmID string } func (s *githubService) ServiceUserID() string { return s.serviceUserID } func (s *githubService) ServiceID() string { return s.id } func (s *githubService) ServiceType() string { return "github" } func (s *githubService) cmdGithubCreate(roomID, userID string, args []string) (interface{}, error) { - if !s.HandleCommands { - return nil, nil - } cli := s.githubClientFor(userID, false) if cli == nil { r, err := database.GetServiceDB().LoadAuthRealm(s.RealmID) @@ -145,9 +129,6 @@ func (s *githubService) Plugin(cli *matrix.Client, roomID string) plugin.Plugin plugin.Expansion{ Regexp: ownerRepoIssueRegex, Expand: func(roomID, userID string, matchingGroups []string) interface{} { - if !s.HandleExpansions { - return nil - } // There's an optional group in the regex so matchingGroups can look like: // [foo/bar#55 foo/bar foo bar 55] // [#55 55] @@ -192,58 +173,7 @@ func (s *githubService) Plugin(cli *matrix.Client, roomID string) plugin.Plugin } } func (s *githubService) OnReceiveWebhook(w http.ResponseWriter, req *http.Request, cli *matrix.Client) { - evType, repo, msg, err := webhook.OnReceiveRequest(req, s.SecretToken) - if err != nil { - w.WriteHeader(err.Code) - return - } - logger := log.WithFields(log.Fields{ - "event": evType, - "repo": *repo.FullName, - }) - repoExistsInConfig := false - - for roomID, roomConfig := range s.Rooms { - for ownerRepo, repoConfig := range roomConfig.Repos { - if !strings.EqualFold(*repo.FullName, ownerRepo) { - continue - } - repoExistsInConfig = true // even if we don't notify for it. - notifyRoom := false - for _, notifyType := range repoConfig.Events { - if evType == notifyType { - notifyRoom = true - break - } - } - if notifyRoom { - logger.WithFields(log.Fields{ - "msg": msg, - "room_id": roomID, - }).Print("Sending notification to room") - if _, e := cli.SendMessageEvent(roomID, "m.room.message", msg); e != nil { - logger.WithError(e).WithField("room_id", roomID).Print( - "Failed to send notification to room.") - } - } - } - } - - if !repoExistsInConfig { - segs := strings.Split(*repo.FullName, "/") - if len(segs) != 2 { - logger.Error("Received event with malformed owner/repo.") - w.WriteHeader(400) - return - } - if err := s.deleteHook(segs[0], segs[1]); err != nil { - logger.WithError(err).Print("Failed to delete webhook") - } else { - logger.Info("Deleted webhook") - } - } - - w.WriteHeader(200) + w.WriteHeader(400) } // Register will create webhooks for the repos specified in Rooms @@ -270,53 +200,7 @@ func (s *githubService) Register(oldService types.Service, client *matrix.Client return fmt.Errorf("Realm is of type '%s', not 'github'", realm.Type()) } - if s.ClientUserID != "" { - // In order to register the GH service as a client, you must have authed with GH. - cli := s.githubClientFor(s.ClientUserID, false) - if cli == nil { - return fmt.Errorf( - "User %s does not have a Github auth session with realm %s.", s.ClientUserID, realm.ID()) - } - // Make sure they have specified some webhooks (it makes no sense otherwise) - reposForWebhooks := s.repoList() - if len(reposForWebhooks) == 0 { - return fmt.Errorf("No repos for webhooks specified") - } - - // Fetch the old service list and work out the difference between the two. - var oldRepos []string - if oldService != nil { - old, ok := oldService.(*githubService) - if !ok { - log.WithFields(log.Fields{ - "service_id": oldService.ServiceID(), - "service_type": oldService.ServiceType(), - }).Print("Cannot cast old github service to GithubService") - // non-fatal though, we'll just make the hooks - } else { - oldRepos = old.repoList() - } - } - - // Add the repos in the new service but not the old service - newRepos, _ := util.Difference(reposForWebhooks, oldRepos) - for _, r := range newRepos { - logger := log.WithField("repo", r) - err := s.createHook(cli, r) - if err != nil { - logger.WithError(err).Error("Failed to create webhook") - return err - } - logger.Info("Created webhook") - } - - if err := s.joinWebhookRooms(client); err != nil { - return err - } - } - log.Infof("%+v", s) - return nil } @@ -350,153 +234,6 @@ func (s *githubService) defaultRepo(roomID string) string { return defaultRepo } -func (s *githubService) joinWebhookRooms(client *matrix.Client) error { - for roomID := range s.Rooms { - if _, err := client.JoinRoom(roomID, "", ""); err != nil { - // TODO: Leave the rooms we successfully joined? - return err - } - } - return nil -} - -func (s *githubService) repoList() []string { - var repos []string - if s.Rooms == nil { - return repos - } - for _, roomConfig := range s.Rooms { - for ownerRepo := range roomConfig.Repos { - if strings.Count(ownerRepo, "/") != 1 { - log.WithField("repo", ownerRepo).Error("Bad owner/repo key in config") - continue - } - exists := false - for _, r := range repos { - if r == ownerRepo { - exists = true - break - } - } - if !exists { - repos = append(repos, ownerRepo) - } - } - } - return repos -} - -func (s *githubService) createHook(cli *github.Client, ownerRepo string) error { - o := strings.Split(ownerRepo, "/") - owner := o[0] - repo := o[1] - // make a hook for all GH events since we'll filter it when we receive webhook requests - name := "web" // https://developer.github.com/v3/repos/hooks/#create-a-hook - cfg := map[string]interface{}{ - "content_type": "json", - "url": s.webhookEndpointURL, - } - if s.SecretToken != "" { - cfg["secret"] = s.SecretToken - } - events := []string{"push", "pull_request", "issues", "issue_comment", "pull_request_review_comment"} - _, res, err := cli.Repositories.CreateHook(owner, repo, &github.Hook{ - Name: &name, - Config: cfg, - Events: events, - }) - - if res.StatusCode == 422 { - errResponse, ok := err.(*github.ErrorResponse) - if !ok { - return err - } - for _, ghErr := range errResponse.Errors { - if strings.Contains(ghErr.Message, "already exists") { - log.WithField("repo", ownerRepo).Print("422 : Hook already exists") - return nil - } - } - return err - } - - return err -} - -func (s *githubService) deleteHook(owner, repo string) error { - logger := log.WithFields(log.Fields{ - "endpoint": s.webhookEndpointURL, - "repo": owner + "/" + repo, - }) - logger.Info("Removing hook") - - cli := s.githubClientFor(s.ClientUserID, false) - if cli == nil { - logger.WithField("user_id", s.ClientUserID).Print("Cannot delete webhook: no authenticated client exists for user ID.") - return fmt.Errorf("no authenticated client exists for user ID") - } - - // Get a list of webhooks for this owner/repo and find the one which has the - // same endpoint URL which is what github uses to determine equivalence. - hooks, _, err := cli.Repositories.ListHooks(owner, repo, nil) - if err != nil { - return err - } - var hook *github.Hook - for _, h := range hooks { - if h.Config["url"] == nil { - logger.Print("Ignoring nil config.url") - continue - } - hookURL, ok := h.Config["url"].(string) - if !ok { - logger.Print("Ignoring non-string config.url") - continue - } - if hookURL == s.webhookEndpointURL { - hook = h - break - } - } - if hook == nil { - return fmt.Errorf("Failed to find hook with endpoint: %s", s.webhookEndpointURL) - } - - _, err = cli.Repositories.DeleteHook(owner, repo, *hook.ID) - return err -} - -func sameRepos(a *githubService, b *githubService) bool { - getRepos := func(s *githubService) []string { - r := make(map[string]bool) - for _, roomConfig := range s.Rooms { - for ownerRepo := range roomConfig.Repos { - r[ownerRepo] = true - } - } - var rs []string - for k := range r { - rs = append(rs, k) - } - return rs - } - aRepos := getRepos(a) - bRepos := getRepos(b) - - if len(aRepos) != len(bRepos) { - return false - } - - sort.Strings(aRepos) - sort.Strings(bRepos) - for i := 0; i < len(aRepos); i++ { - if aRepos[i] != bRepos[i] { - return false - } - } - return true -} - func (s *githubService) githubClientFor(userID string, allowUnauth bool) *github.Client { token, err := getTokenForUser(s.RealmID, userID) if err != nil { @@ -542,9 +279,8 @@ func getTokenForUser(realmID, userID string) (string, error) { func init() { types.RegisterService(func(serviceID, serviceUserID, webhookEndpointURL string) types.Service { return &githubService{ - id: serviceID, - serviceUserID: serviceUserID, - webhookEndpointURL: webhookEndpointURL, + id: serviceID, + serviceUserID: serviceUserID, } }) } diff --git a/src/github.com/matrix-org/go-neb/services/github/github_webhook.go b/src/github.com/matrix-org/go-neb/services/github/github_webhook.go new file mode 100644 index 0000000..3a49d0e --- /dev/null +++ b/src/github.com/matrix-org/go-neb/services/github/github_webhook.go @@ -0,0 +1,339 @@ +package services + +import ( + "fmt" + log "github.com/Sirupsen/logrus" + "github.com/google/go-github/github" + "github.com/matrix-org/go-neb/database" + "github.com/matrix-org/go-neb/matrix" + "github.com/matrix-org/go-neb/plugin" + "github.com/matrix-org/go-neb/services/github/client" + "github.com/matrix-org/go-neb/services/github/webhook" + "github.com/matrix-org/go-neb/types" + "github.com/matrix-org/go-neb/util" + "net/http" + "sort" + "strings" +) + +type githubWebhookService struct { + id string + serviceUserID string + webhookEndpointURL string + ClientUserID string // optional; required for webhooks + RealmID string + SecretToken string + Rooms map[string]struct { // room_id => {} + Repos map[string]struct { // owner/repo => { events: ["push","issue","pull_request"] } + Events []string + } + } +} + +func (s *githubWebhookService) ServiceUserID() string { return s.serviceUserID } +func (s *githubWebhookService) ServiceID() string { return s.id } +func (s *githubWebhookService) ServiceType() string { return "github-webhook" } +func (s *githubWebhookService) Plugin(cli *matrix.Client, roomID string) plugin.Plugin { + return plugin.Plugin{} +} +func (s *githubWebhookService) OnReceiveWebhook(w http.ResponseWriter, req *http.Request, cli *matrix.Client) { + evType, repo, msg, err := webhook.OnReceiveRequest(req, s.SecretToken) + if err != nil { + w.WriteHeader(err.Code) + return + } + logger := log.WithFields(log.Fields{ + "event": evType, + "repo": *repo.FullName, + }) + repoExistsInConfig := false + + for roomID, roomConfig := range s.Rooms { + for ownerRepo, repoConfig := range roomConfig.Repos { + if !strings.EqualFold(*repo.FullName, ownerRepo) { + continue + } + repoExistsInConfig = true // even if we don't notify for it. + notifyRoom := false + for _, notifyType := range repoConfig.Events { + if evType == notifyType { + notifyRoom = true + break + } + } + if notifyRoom { + logger.WithFields(log.Fields{ + "msg": msg, + "room_id": roomID, + }).Print("Sending notification to room") + if _, e := cli.SendMessageEvent(roomID, "m.room.message", msg); e != nil { + logger.WithError(e).WithField("room_id", roomID).Print( + "Failed to send notification to room.") + } + } + } + } + + if !repoExistsInConfig { + segs := strings.Split(*repo.FullName, "/") + if len(segs) != 2 { + logger.Error("Received event with malformed owner/repo.") + w.WriteHeader(400) + return + } + if err := s.deleteHook(segs[0], segs[1]); err != nil { + logger.WithError(err).Print("Failed to delete webhook") + } else { + logger.Info("Deleted webhook") + } + } + + w.WriteHeader(200) +} + +// Register will create webhooks for the repos specified in Rooms +// +// The hooks made are a delta between the old service and the current configuration. If all webhooks are made, +// Register() succeeds. If any webhook fails to be created, Register() fails. A delta is used to allow clients to incrementally +// build up the service config without recreating the hooks every time a change is made. +// +// Hooks are deleted when this service receives a webhook event from Github for a repo which has no user configurations. +// +// Hooks can get out of sync if a user manually deletes a hook in the Github UI. In this case, toggling the repo configuration will +// force NEB to recreate the hook. +func (s *githubWebhookService) Register(oldService types.Service, client *matrix.Client) error { + if s.RealmID == "" || s.ClientUserID == "" { + return fmt.Errorf("RealmID and ClientUserID is required") + } + // check realm exists + realm, err := database.GetServiceDB().LoadAuthRealm(s.RealmID) + if err != nil { + return err + } + // make sure the realm is of the type we expect + if realm.Type() != "github" { + return fmt.Errorf("Realm is of type '%s', not 'github'", realm.Type()) + } + + // In order to register the GH service as a client, you must have authed with GH. + cli := s.githubClientFor(s.ClientUserID, false) + if cli == nil { + return fmt.Errorf( + "User %s does not have a Github auth session with realm %s.", s.ClientUserID, realm.ID()) + } + // Make sure they have specified some webhooks (it makes no sense otherwise) + reposForWebhooks := s.repoList() + if len(reposForWebhooks) == 0 { + return fmt.Errorf("No repos for webhooks specified") + } + + // Fetch the old service list and work out the difference between the two. + var oldRepos []string + if oldService != nil { + old, ok := oldService.(*githubWebhookService) + if !ok { + log.WithFields(log.Fields{ + "service_id": oldService.ServiceID(), + "service_type": oldService.ServiceType(), + }).Print("Cannot cast old github service to GithubWebhookService") + // non-fatal though, we'll just make the hooks + } else { + oldRepos = old.repoList() + } + } + + // Add the repos in the new service but not the old service + newRepos, _ := util.Difference(reposForWebhooks, oldRepos) + for _, r := range newRepos { + logger := log.WithField("repo", r) + err := s.createHook(cli, r) + if err != nil { + logger.WithError(err).Error("Failed to create webhook") + return err + } + logger.Info("Created webhook") + } + + if err := s.joinWebhookRooms(client); err != nil { + return err + } + + log.Infof("%+v", s) + + return nil +} + +func (s *githubWebhookService) joinWebhookRooms(client *matrix.Client) error { + for roomID := range s.Rooms { + if _, err := client.JoinRoom(roomID, "", ""); err != nil { + // TODO: Leave the rooms we successfully joined? + return err + } + } + return nil +} + +func (s *githubWebhookService) repoList() []string { + var repos []string + if s.Rooms == nil { + return repos + } + for _, roomConfig := range s.Rooms { + for ownerRepo := range roomConfig.Repos { + if strings.Count(ownerRepo, "/") != 1 { + log.WithField("repo", ownerRepo).Error("Bad owner/repo key in config") + continue + } + exists := false + for _, r := range repos { + if r == ownerRepo { + exists = true + break + } + } + if !exists { + repos = append(repos, ownerRepo) + } + } + } + return repos +} + +func (s *githubWebhookService) createHook(cli *github.Client, ownerRepo string) error { + o := strings.Split(ownerRepo, "/") + owner := o[0] + repo := o[1] + // make a hook for all GH events since we'll filter it when we receive webhook requests + name := "web" // https://developer.github.com/v3/repos/hooks/#create-a-hook + cfg := map[string]interface{}{ + "content_type": "json", + "url": s.webhookEndpointURL, + } + if s.SecretToken != "" { + cfg["secret"] = s.SecretToken + } + events := []string{"push", "pull_request", "issues", "issue_comment", "pull_request_review_comment"} + _, res, err := cli.Repositories.CreateHook(owner, repo, &github.Hook{ + Name: &name, + Config: cfg, + Events: events, + }) + + if res.StatusCode == 422 { + errResponse, ok := err.(*github.ErrorResponse) + if !ok { + return err + } + for _, ghErr := range errResponse.Errors { + if strings.Contains(ghErr.Message, "already exists") { + log.WithField("repo", ownerRepo).Print("422 : Hook already exists") + return nil + } + } + return err + } + + return err +} + +func (s *githubWebhookService) deleteHook(owner, repo string) error { + logger := log.WithFields(log.Fields{ + "endpoint": s.webhookEndpointURL, + "repo": owner + "/" + repo, + }) + logger.Info("Removing hook") + + cli := s.githubClientFor(s.ClientUserID, false) + if cli == nil { + logger.WithField("user_id", s.ClientUserID).Print("Cannot delete webhook: no authenticated client exists for user ID.") + return fmt.Errorf("no authenticated client exists for user ID") + } + + // Get a list of webhooks for this owner/repo and find the one which has the + // same endpoint URL which is what github uses to determine equivalence. + hooks, _, err := cli.Repositories.ListHooks(owner, repo, nil) + if err != nil { + return err + } + var hook *github.Hook + for _, h := range hooks { + if h.Config["url"] == nil { + logger.Print("Ignoring nil config.url") + continue + } + hookURL, ok := h.Config["url"].(string) + if !ok { + logger.Print("Ignoring non-string config.url") + continue + } + if hookURL == s.webhookEndpointURL { + hook = h + break + } + } + if hook == nil { + return fmt.Errorf("Failed to find hook with endpoint: %s", s.webhookEndpointURL) + } + + _, err = cli.Repositories.DeleteHook(owner, repo, *hook.ID) + return err +} + +func sameRepos(a *githubWebhookService, b *githubWebhookService) bool { + getRepos := func(s *githubWebhookService) []string { + r := make(map[string]bool) + for _, roomConfig := range s.Rooms { + for ownerRepo := range roomConfig.Repos { + r[ownerRepo] = true + } + } + var rs []string + for k := range r { + rs = append(rs, k) + } + return rs + } + aRepos := getRepos(a) + bRepos := getRepos(b) + + if len(aRepos) != len(bRepos) { + return false + } + + sort.Strings(aRepos) + sort.Strings(bRepos) + for i := 0; i < len(aRepos); i++ { + if aRepos[i] != bRepos[i] { + return false + } + } + return true +} + +func (s *githubWebhookService) githubClientFor(userID string, allowUnauth bool) *github.Client { + token, err := getTokenForUser(s.RealmID, userID) + if err != nil { + log.WithFields(log.Fields{ + log.ErrorKey: err, + "user_id": userID, + "realm_id": s.RealmID, + }).Print("Failed to get token for user") + } + if token != "" { + return client.New(token) + } else if allowUnauth { + return client.New("") + } else { + return nil + } +} + +func init() { + types.RegisterService(func(serviceID, serviceUserID, webhookEndpointURL string) types.Service { + return &githubWebhookService{ + id: serviceID, + serviceUserID: serviceUserID, + webhookEndpointURL: webhookEndpointURL, + } + }) +}