Skip to content

Commit

Permalink
bug(repo sync): ignore inactive repos (#953)
Browse files Browse the repository at this point in the history
* bug(repo sync): ignore inactive repos

* bug(repo sync): deactivate repo in db if webhook manually deleted

* bug(repo sync): update repo to inactive

* pushing failing test for troubleshooting

* bug(repo sync): add tests for isWebhookDel bool

* bug(repo sync): modify bool to webhookExists, add to single repo sync

* bug(repo sync): cleaning up code based on feedback

* bug(repo sync): correct comparison for test to pass

* bug(repo sync): ...b
  • Loading branch information
KellyMerrick committed Sep 7, 2023
1 parent 0464cb5 commit bb79525
Show file tree
Hide file tree
Showing 6 changed files with 140 additions and 22 deletions.
2 changes: 1 addition & 1 deletion api/repo/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
36 changes: 30 additions & 6 deletions api/scm/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}
}
}

Expand Down
34 changes: 28 additions & 6 deletions api/scm/sync_org.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}
}
}
}
Expand Down
12 changes: 5 additions & 7 deletions scm/github/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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.
Expand Down
76 changes: 75 additions & 1 deletion scm/github/repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion scm/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit bb79525

Please sign in to comment.