diff --git a/api/repo/update.go b/api/repo/update.go index 947ddba4d..2d6b166e5 100644 --- a/api/repo/update.go +++ b/api/repo/update.go @@ -285,7 +285,7 @@ func UpdateRepo(c *gin.Context) { }).Infof("platform admin %s updating repo webhook events for repo %s", admn, r.GetFullName()) } // update webhook with new events - err = scm.FromContext(c).Update(u, r, lastHook.GetWebhookID()) + _, err = scm.FromContext(c).Update(u, r, lastHook.GetWebhookID()) if err != nil { retErr := fmt.Errorf("unable to update repo webhook for %s: %w", r.GetFullName(), err) diff --git a/api/scm/sync.go b/api/scm/sync.go index a02981acd..db645f623 100644 --- a/api/scm/sync.go +++ b/api/scm/sync.go @@ -111,8 +111,9 @@ func SyncRepo(c *gin.Context) { return } - // if we have webhook validation, update the repo hook in the SCM - if c.Value("webhookvalidation").(bool) { + // if we have webhook validation and the repo is active in the database, + // update the repo hook in the SCM + if c.Value("webhookvalidation").(bool) && r.GetActive() { // grab last hook from repo to fetch the webhook ID lastHook, err := database.FromContext(c).LastHookForRepo(ctx, r) if err != nil { @@ -124,13 +125,36 @@ func SyncRepo(c *gin.Context) { } // update webhook - err = scm.FromContext(c).Update(u, r, lastHook.GetWebhookID()) + webhookExists, err := scm.FromContext(c).Update(u, r, lastHook.GetWebhookID()) if err != nil { - retErr := fmt.Errorf("unable to update repo webhook for %s: %w", r.GetFullName(), err) - util.HandleError(c, http.StatusInternalServerError, retErr) + // if webhook has been manually deleted from GitHub, + // set to inactive in database + if !webhookExists { - return + r.SetActive(false) + + _, err := database.FromContext(c).UpdateRepo(ctx, r) + if err != nil { + retErr := fmt.Errorf("unable to update repo for org %s: %w", o, err) + + util.HandleError(c, http.StatusInternalServerError, retErr) + + return + } + + c.JSON(http.StatusOK, fmt.Sprintf("webhook not found, repo %s deactivated", r.GetFullName())) + + return + + } else { + + retErr := fmt.Errorf("unable to update repo webhook for %s: %w", r.GetFullName(), err) + + util.HandleError(c, http.StatusInternalServerError, retErr) + + return + } } } diff --git a/api/scm/sync_org.go b/api/scm/sync_org.go index fb809c74e..6ae0b28df 100644 --- a/api/scm/sync_org.go +++ b/api/scm/sync_org.go @@ -124,8 +124,9 @@ func SyncReposForOrg(c *gin.Context) { } } - // if we have webhook validation, update the repo hook in the SCM - if c.Value("webhookvalidation").(bool) { + // if we have webhook validation and the repo is active in the database, + // update the repo hook in the SCM + if c.Value("webhookvalidation").(bool) && repo.GetActive() { // grab last hook from repo to fetch the webhook ID lastHook, err := database.FromContext(c).LastHookForRepo(ctx, repo) if err != nil { @@ -137,13 +138,34 @@ func SyncReposForOrg(c *gin.Context) { } // update webhook - err = scm.FromContext(c).Update(u, repo, lastHook.GetWebhookID()) + webhookExists, err := scm.FromContext(c).Update(u, repo, lastHook.GetWebhookID()) if err != nil { - retErr := fmt.Errorf("unable to update repo webhook for %s: %w", repo.GetFullName(), err) - util.HandleError(c, http.StatusInternalServerError, retErr) + // if webhook has been manually deleted from GitHub, + // set to inactive in database + if !webhookExists { - return + repo.SetActive(false) + + _, err := database.FromContext(c).UpdateRepo(ctx, repo) + if err != nil { + retErr := fmt.Errorf("unable to update repo for org %s: %w", o, err) + + util.HandleError(c, http.StatusInternalServerError, retErr) + + return + } + + continue + + } else { + + retErr := fmt.Errorf("unable to update repo webhook for %s: %w", repo.GetFullName(), err) + + util.HandleError(c, http.StatusInternalServerError, retErr) + + return + } } } } diff --git a/scm/github/repo.go b/scm/github/repo.go index c545c5169..a3d578878 100644 --- a/scm/github/repo.go +++ b/scm/github/repo.go @@ -217,7 +217,7 @@ func (c *client) Enable(u *library.User, r *library.Repo, h *library.Hook) (*lib } // Update edits a repo webhook. -func (c *client) Update(u *library.User, r *library.Repo, hookID int64) error { +func (c *client) Update(u *library.User, r *library.Repo, hookID int64) (bool, error) { c.Logger.WithFields(logrus.Fields{ "org": r.GetOrg(), "repo": r.GetName(), @@ -258,13 +258,11 @@ func (c *client) Update(u *library.User, r *library.Repo, hookID int64) error { } // send API call to update the webhook - _, _, err := client.Repositories.EditHook(ctx, r.GetOrg(), r.GetName(), hookID, hook) + _, resp, err := client.Repositories.EditHook(ctx, r.GetOrg(), r.GetName(), hookID, hook) - if err != nil { - return err - } - - return nil + // track if webhook exists in GitHub; a missing webhook + // indicates the webhook has been manually deleted from GitHub + return resp.StatusCode != http.StatusNotFound, err } // Status sends the commit status for the given SHA from the GitHub repo. diff --git a/scm/github/repo_test.go b/scm/github/repo_test.go index 858d60f18..a23326337 100644 --- a/scm/github/repo_test.go +++ b/scm/github/repo_test.go @@ -673,7 +673,7 @@ func TestGithub_Update(t *testing.T) { client, _ := NewTest(s.URL) // run test - err := client.Update(u, r, hookID) + _, err := client.Update(u, r, hookID) if resp.Code != http.StatusOK { t.Errorf("Update returned %v, want %v", resp.Code, http.StatusOK) @@ -684,6 +684,80 @@ func TestGithub_Update(t *testing.T) { } } +func TestGithub_Update_webhookExists_True(t *testing.T) { + // setup context + gin.SetMode(gin.TestMode) + + resp := httptest.NewRecorder() + _, engine := gin.CreateTestContext(resp) + + // setup mock server + engine.PATCH("/api/v3/repos/:org/:repo/hooks/:hook_id", func(c *gin.Context) { + c.Header("Content-Type", "application/json") + c.Status(http.StatusOK) + }) + + s := httptest.NewServer(engine) + defer s.Close() + + // setup types + u := new(library.User) + u.SetName("foo") + u.SetToken("bar") + + r := new(library.Repo) + + client, _ := NewTest(s.URL) + + // run test + webhookExists, err := client.Update(u, r, 0) + + if !webhookExists { + t.Errorf("Update returned %v, want %v", webhookExists, true) + } + + if err != nil { + t.Errorf("Update returned err: %v", err) + } +} + +func TestGithub_Update_webhookExists_False(t *testing.T) { + // setup context + gin.SetMode(gin.TestMode) + + resp := httptest.NewRecorder() + _, engine := gin.CreateTestContext(resp) + + // setup mock server + engine.PATCH("/api/v3/repos/:org/:repo/hooks/:hook_id", func(c *gin.Context) { + c.Header("Content-Type", "application/json") + c.Status(http.StatusNotFound) + }) + + s := httptest.NewServer(engine) + defer s.Close() + + // setup types + u := new(library.User) + u.SetName("foo") + u.SetToken("bar") + + r := new(library.Repo) + + client, _ := NewTest(s.URL) + + // run test + webhookExists, err := client.Update(u, r, 0) + + if webhookExists { + t.Errorf("Update returned %v, want %v", webhookExists, false) + } + + if err == nil { + t.Error("Update should return error") + } +} + func TestGithub_Status_Deployment(t *testing.T) { // setup context gin.SetMode(gin.TestMode) diff --git a/scm/service.go b/scm/service.go index 4de42362f..ae9488724 100644 --- a/scm/service.go +++ b/scm/service.go @@ -101,7 +101,7 @@ type Service interface { Enable(*library.User, *library.Repo, *library.Hook) (*library.Hook, string, error) // Update defines a function that updates // a webhook for a specified repo. - Update(*library.User, *library.Repo, int64) error + Update(*library.User, *library.Repo, int64) (bool, error) // Status defines a function that sends the // commit status for the given SHA from a repo. Status(*library.User, *library.Build, string, string) error