From cb74efcbec155738cd39a7908a1f44e2cdf5bcae Mon Sep 17 00:00:00 2001 From: David May <1301201+wass3r@users.noreply.github.com> Date: Wed, 13 May 2020 00:01:08 +0000 Subject: [PATCH] fix(hooks): account for multiple hooks in repo disable (#151) * fix: account for multiple hooks * also activate repo after repair --- api/repo.go | 15 +++++++ source/github/repo.go | 21 +++++---- source/github/repo_test.go | 46 +++++++++++++++++++ source/github/testdata/hooks_multi.json | 59 +++++++++++++++++++++++++ 4 files changed, 133 insertions(+), 8 deletions(-) create mode 100644 source/github/testdata/hooks_multi.json diff --git a/api/repo.go b/api/repo.go index 2a99d84df..db81b7bda 100644 --- a/api/repo.go +++ b/api/repo.go @@ -454,6 +454,21 @@ func RepairRepo(c *gin.Context) { return } + // if the repo was previously inactive, mark it as active + if !r.GetActive() { + r.SetActive(true) + + // send API call to update the repo + err = database.FromContext(c).UpdateRepo(r) + if err != nil { + retErr := fmt.Errorf("unable to set repo %s to active: %w", r.GetFullName(), err) + + util.HandleError(c, http.StatusInternalServerError, retErr) + + return + } + } + c.JSON(http.StatusOK, fmt.Sprintf("Repo %s repaired", r.GetFullName())) } diff --git a/source/github/repo.go b/source/github/repo.go index 510df248d..647527c9f 100644 --- a/source/github/repo.go +++ b/source/github/repo.go @@ -80,13 +80,15 @@ func (c *client) Disable(u *library.User, org, name string) error { return err } - // since 0 might be a real value (though unlikely?) - var id *int64 + // accounting for situations in which multiple hooks have been + // associated with this vela instance, which causes some + // disable, repair, enable operations to act in undesirable ways + var ids []int64 // iterate through each element in the hooks for _, hook := range hooks { // skip if the hook has no ID - if hook.ID == nil { + if hook.GetID() == 0 { continue } @@ -95,17 +97,20 @@ func (c *client) Disable(u *library.User, org, name string) error { // capture hook ID if the hook url matches if hookURL == fmt.Sprintf("%s/webhook", c.LocalHost) { - id = hook.ID + ids = append(ids, hook.GetID()) } } - // skip if we got no hook ID - if id == nil { + // skip if we have no hook IDs + if len(ids) == 0 { return nil } - // send API call to delete the webhook - _, err = client.Repositories.DeleteHook(ctx, org, name, *id) + // go through all found hook IDs and delete them + for _, id := range ids { + // send API call to delete the webhook + _, err = client.Repositories.DeleteHook(ctx, org, name, id) + } return err } diff --git a/source/github/repo_test.go b/source/github/repo_test.go index b3beb7920..dc87355bb 100644 --- a/source/github/repo_test.go +++ b/source/github/repo_test.go @@ -349,6 +349,52 @@ func TestGithub_Disable_HooksButNotFound(t *testing.T) { } } +func TestGithub_Disable_MultipleHooks(t *testing.T) { + // setup context + gin.SetMode(gin.TestMode) + + resp := httptest.NewRecorder() + _, engine := gin.CreateTestContext(resp) + count := 0 + wantCount := 2 + + // setup mock server + engine.GET("/api/v3/repos/:org/:repo/hooks", func(c *gin.Context) { + c.Header("Content-Type", "application/json") + c.Status(http.StatusOK) + c.File("testdata/hooks_multi.json") + }) + engine.DELETE("/api/v3/repos/:org/:repo/hooks/:hook_id", func(c *gin.Context) { + count++ + c.Status(http.StatusNoContent) + }) + + s := httptest.NewServer(engine) + defer s.Close() + + // setup types + u := new(library.User) + u.SetName("foo") + u.SetToken("bar") + + client, _ := NewTest(s.URL, "https://foo.bar.com") + + // run test + err := client.Disable(u, "foo", "bar") + + if count != wantCount { + t.Errorf("Count returned %d, want %d", count, wantCount) + } + + if resp.Code != http.StatusOK { + t.Errorf("Disable returned %v, want %v", resp.Code, http.StatusOK) + } + + if err != nil { + t.Errorf("Disable returned err: %v", err) + } +} + func TestGithub_Enable(t *testing.T) { // setup context gin.SetMode(gin.TestMode) diff --git a/source/github/testdata/hooks_multi.json b/source/github/testdata/hooks_multi.json new file mode 100644 index 000000000..704941dd9 --- /dev/null +++ b/source/github/testdata/hooks_multi.json @@ -0,0 +1,59 @@ +[ + { + "id": 1, + "url": "https://api.github.com/repos/foo/bar/hooks/1", + "test_url": "https://api.github.com/repos/foo/bar/hooks/1/test", + "ping_url": "https://api.github.com/repos/foo/bar/hooks/1/pings", + "name": "web", + "events": [ + "push", + "pull_request", + "deployment" + ], + "active": true, + "config": { + "url": "https://foo.bar.com/webhook", + "content_type": "form" + }, + "updated_at": "2011-09-06T20:39:23Z", + "created_at": "2011-09-06T17:26:27Z" + }, + { + "id": 2, + "url": "https://api.github.com/repos/foo/bar/hooks/2", + "test_url": "https://api.github.com/repos/foo/bar/hooks/2/test", + "ping_url": "https://api.github.com/repos/foo/bar/hooks/2/pings", + "name": "web", + "events": [ + "push", + "pull_request", + "deployment" + ], + "active": true, + "config": { + "url": "https://foo.bar.com/webhook", + "content_type": "form" + }, + "updated_at": "2011-09-06T20:39:23Z", + "created_at": "2011-09-06T17:26:27Z" + }, + { + "id": 3, + "url": "https://api.github.com/repos/foo/bar/hooks/3", + "test_url": "https://api.github.com/repos/foo/bar/hooks/3/test", + "ping_url": "https://api.github.com/repos/foo/bar/hooks/3/pings", + "name": "web", + "events": [ + "push", + "pull_request", + "deployment" + ], + "active": true, + "config": { + "url": "https://not.foo.bar.com/webhook", + "content_type": "form" + }, + "updated_at": "2011-09-06T20:39:23Z", + "created_at": "2011-09-06T17:26:27Z" + } +] \ No newline at end of file