Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug(repo sync): ignore inactive repos #953

Merged
merged 14 commits into from
Sep 7, 2023
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
7 changes: 4 additions & 3 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,7 +125,7 @@ func SyncRepo(c *gin.Context) {
}

// update webhook
err = scm.FromContext(c).Update(u, r, lastHook.GetWebhookID())
_, err = scm.FromContext(c).Update(u, r, lastHook.GetWebhookID())
KellyMerrick marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
retErr := fmt.Errorf("unable to update repo webhook for %s: %w", r.GetFullName(), err)

Expand Down
23 changes: 20 additions & 3 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,12 +138,28 @@ func SyncReposForOrg(c *gin.Context) {
}

// update webhook
err = scm.FromContext(c).Update(u, repo, lastHook.GetWebhookID())
isWebhookDel, 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)
ecrupper marked this conversation as resolved.
Show resolved Hide resolved

// if webhook has been manually deleted from GitHub,
// set to inactive in database
if isWebhookDel {

repo.SetActive(false)

_, err := database.FromContext(c).UpdateRepo(ctx, repo)
ecrupper marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
retErr := fmt.Errorf("unable to update repo for org %s: %w", o, err)

util.HandleError(c, http.StatusInternalServerError, retErr)

return
}
}

return
}
}
Expand Down
15 changes: 11 additions & 4 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,20 @@ 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)

// track if webhook cannot be captured from GitHub (no longer found),
// indicating the webhook has been manually deleted from GitHub
isWebhookDel := false
if resp.StatusCode == http.StatusNotFound {
isWebhookDel = true
}

if err != nil {
return err
return isWebhookDel, err
}

return nil
return isWebhookDel, nil
ecrupper marked this conversation as resolved.
Show resolved Hide resolved
}

// 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_isWebhookDel_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.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
isWebhookDel, err := client.Update(u, r, 0)

if isWebhookDel {
t.Errorf("Update returned %v, want %v", isWebhookDel, false)
}

if err != nil {
t.Errorf("Update returned err: %v", err)
}
}

func TestGithub_Update_isWebhookDel_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.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
isWebhookDel, err := client.Update(u, r, 0)

if !isWebhookDel {
t.Errorf("Update returned %v, want %v", isWebhookDel, true)
}

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