patch(sync): address sync bug with update webhook call (#962)

api/repo/update.go

@@ -284,7 +284,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)
 

api/scm/sync.go

@@ -110,8 +110,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(r)
 		if err != nil {
@@ -123,13 +124,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(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
+			}
 		}
 	}
 

api/scm/sync_org.go

@@ -123,8 +123,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(repo)
 			if err != nil {
@@ -136,13 +137,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(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
+				}
 			}
 		}
 	}

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.

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)

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