fix(hooks): do not create hook twice (#4312)

engine/vcs/bitbucket/client_hook.go

@@ -4,45 +4,43 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"net/http"
+
 	"github.com/ovh/cds/sdk"
 )
 
-const bitbucketHookKey string = "de.aeffle.stash.plugin.stash-http-get-post-receive-hook%3Ahttp-get-post-receive-hook"
-
-func (b *bitbucketClient) getHooksConfig(ctx context.Context, repo string) (HooksConfig, error) {
-	oldHookConfig := HooksConfig{}
-
+func (b *bitbucketClient) getHooks(ctx context.Context, repo string) ([]WebHook, error) {
 	project, slug, err := getRepo(repo)
 	if err != nil {
-		return oldHookConfig, sdk.WithStack(err)
+		return nil, sdk.WithStack(err)
 	}
 
-	getPath := fmt.Sprintf("/projects/%s/repos/%s/settings/hooks/%s/settings", project, slug, bitbucketHookKey)
-	if err := b.do(ctx, "GET", "core", getPath, nil, nil, &oldHookConfig, nil); err != nil {
-		return oldHookConfig, sdk.WrapError(err, "Unable to get hook config")
+	var resp GetWebHooksResponse
+	getPath := fmt.Sprintf("/projects/%s/repos/%s/webhooks", project, slug)
+	if err := b.do(ctx, "GET", "core", getPath, nil, nil, &resp, nil); err != nil {
+		return nil, sdk.WrapError(err, "Unable to get hook config")
 	}
 
-	return oldHookConfig, nil
+	return resp.Values, nil
 }
 
 func (b *bitbucketClient) GetHook(ctx context.Context, repo, url string) (sdk.VCSHook, error) {
-	hcfg, err := b.getHooksConfig(ctx, repo)
+	whooks, err := b.getHooks(ctx, repo)
 	if err != nil {
 		return sdk.VCSHook{}, err
 	}
 
-	for i, h := range hcfg.Details {
+	for _, h := range whooks {
 		if h.URL == url {
 			return sdk.VCSHook{
-				ContentType: h.PostContentType,
-				Disable:     false,
-				Events:      []string{"push"},
-				ID:          fmt.Sprintf("%d", i),
+				Disable:     h.Active,
+				Events:      h.Events,
+				ID:          fmt.Sprintf("%d", h.ID),
 				InsecureSSL: false,
-				Method:      h.Method,
-				Name:        fmt.Sprintf("Location %d", i),
+				Method:      http.MethodPost,
+				Name:        h.Name,
 				URL:         h.URL,
-				Body:        h.PostData,
+				Body:        "",
 			}, nil
 		}
 	}
@@ -56,6 +54,18 @@ func (b *bitbucketClient) CreateHook(ctx context.Context, repo string, hook *sdk
 		return err
 	}
 
+	// Get hooks
+	hooks, err := b.getHooks(ctx, repo)
+	if err != nil {
+		return err
+	}
+
+	for _, h := range hooks {
+		if h.URL == hook.URL {
+			return nil
+		}
+	}
+
 	url := fmt.Sprintf("/projects/%s/repos/%s/webhooks", project, slug)
 	request := WebHook{
 		URL:           hook.URL,
@@ -76,71 +86,12 @@ func (b *bitbucketClient) CreateHook(ctx context.Context, repo string, hook *sdk
 	return nil
 }
 
-func (b *bitbucketClient) UpdateHook(ctx context.Context, repo, url string, hook sdk.VCSHook) error {
-	project, slug, err := getRepo(repo)
-	if err != nil {
-		return sdk.WithStack(err)
-	}
-
-	hcfg, err := b.getHooksConfig(ctx, repo)
-	if err != nil {
-		return sdk.WithStack(err)
-	}
-
-	for i := range hcfg.Details {
-		h := &hcfg.Details[i]
-		if h.URL == url {
-			h.Method = hook.Method
-			h.PostContentType = hook.ContentType
-			h.PostData = hook.Body
-			break
-		}
-	}
-
-	values, err := json.Marshal(&hcfg)
-	if err != nil {
-		return sdk.WrapError(err, "Unable to unmarshal hooks config")
-	}
-
-	updatePath := fmt.Sprintf("/projects/%s/repos/%s/settings/hooks/%s/settings", project, slug, bitbucketHookKey)
-	if err := b.do(ctx, "PUT", "core", updatePath, nil, values, &hook, nil); err != nil {
-		return sdk.WrapError(err, "Unable to update hook config")
-	}
-
-	return nil
-}
-
 func (b *bitbucketClient) DeleteHook(ctx context.Context, repo string, hook sdk.VCSHook) error {
 	project, slug, err := getRepo(repo)
 	if err != nil {
 		return sdk.WithStack(err)
 	}
 
-	if !hook.Workflow {
-		hcfg, err := b.getHooksConfig(ctx, repo)
-		if err != nil {
-			return sdk.WithStack(err)
-		}
-
-		for i, h := range hcfg.Details {
-			if hook.URL == h.URL {
-				hcfg.Details = append(hcfg.Details[:i], hcfg.Details[i+1:]...)
-				break
-			}
-		}
-
-		values, err := json.Marshal(&hcfg)
-		if err != nil {
-			return sdk.WrapError(err, "Unable to unmarshal hooks config")
-		}
-
-		updatePath := fmt.Sprintf("/projects/%s/repos/%s/settings/hooks/%s/settings", project, slug, bitbucketHookKey)
-		if err := b.do(ctx, "PUT", "core", updatePath, nil, values, &hook, nil); err != nil {
-			return sdk.WrapError(err, "Unable to update hook config")
-		}
-		return nil
-	}
-
 	url := fmt.Sprintf("/projects/%s/repos/%s/webhooks/%s", project, slug, hook.ID)
 	if err := b.do(ctx, "DELETE", "core", url, nil, nil, nil, nil); err != nil {
 		if !sdk.ErrorIs(err, sdk.ErrNotFound) {

engine/vcs/bitbucket/client_hook_test.go

@@ -34,26 +34,3 @@ func TestDeleteHook(t *testing.T) {
 	err = client.DeleteHook(context.Background(), "CDS/cds-event-function", h)
 	test.NoError(t, err)
 }
-
-func TestUpdateHook(t *testing.T) {
-	client := getAuthorizedClient(t)
-
-	h := sdk.VCSHook{
-		Method: "POST",
-		URL:    "http://localhost:8080",
-	}
-
-	err := client.CreateHook(context.Background(), "CDS/cds-event-function", &h)
-	test.NoError(t, err)
-
-	h = sdk.VCSHook{
-		Method: "GET",
-		URL:    "http://localhost:8080",
-	}
-
-	err = client.UpdateHook(context.Background(), "CDS/cds-event-function", h.URL, h)
-	test.NoError(t, err)
-
-	err = client.DeleteHook(context.Background(), "CDS/cds-event-function", h)
-	test.NoError(t, err)
-}

engine/vcs/bitbucket/types.go

@@ -24,6 +24,11 @@ type WebHook struct {
 	URL           string            `json:"url"`
 }
 
+// GetWebHooksResponse represent the response send by bitbucket when listing webhooks
+type GetWebHooksResponse struct {
+	Values []WebHook `json:"values"`
+}
+
 type Branch struct {
 	ID         string `json:"id"`
 	DisplayID  string `json:"displayId"`

engine/vcs/gerrit/client_hook.go

@@ -10,9 +10,6 @@ import (
 func (c *gerritClient) GetHook(ctx context.Context, repo, id string) (sdk.VCSHook, error) {
 	return sdk.VCSHook{}, fmt.Errorf("Not implemented")
 }
-func (c *gerritClient) UpdateHook(ctx context.Context, repo, id string, hook sdk.VCSHook) error {
-	return nil
-}
 
 //CreateHook enables the defaut HTTP POST Hook in Gitlab
 func (c *gerritClient) CreateHook(ctx context.Context, repo string, hook *sdk.VCSHook) error {

engine/vcs/github/client_hook.go

@@ -125,9 +125,7 @@ func (g *githubClient) GetHook(ctx context.Context, fullname, webhookURL string)
 
 	return sdk.VCSHook{}, sdk.WithStack(sdk.ErrNotFound)
 }
-func (g *githubClient) UpdateHook(ctx context.Context, repo, id string, hook sdk.VCSHook) error {
-	return fmt.Errorf("Not yet implemented")
-}
+
 func (g *githubClient) DeleteHook(ctx context.Context, repo string, hook sdk.VCSHook) error {
 	return g.delete("/repos/" + repo + "/hooks/" + hook.ID)
 }

engine/vcs/gitlab/client_hook.go

@@ -16,9 +16,6 @@ import (
 func (c *gitlabClient) GetHook(ctx context.Context, repo, id string) (sdk.VCSHook, error) {
 	return sdk.VCSHook{}, fmt.Errorf("Not yet implemented")
 }
-func (c *gitlabClient) UpdateHook(ctx context.Context, repo, id string, hook sdk.VCSHook) error {
-	return fmt.Errorf("Not yet implemented")
-}
 
 //CreateHook enables the defaut HTTP POST Hook in Gitlab
 func (c *gitlabClient) CreateHook(ctx context.Context, repo string, hook *sdk.VCSHook) error {

sdk/vcs.go

@@ -60,7 +60,6 @@ type VCSAuthorizedClient interface {
 	//Hooks
 	CreateHook(ctx context.Context, repo string, hook *VCSHook) error
 	GetHook(ctx context.Context, repo, url string) (VCSHook, error)
-	UpdateHook(ctx context.Context, repo, url string, hook VCSHook) error
 	DeleteHook(ctx context.Context, repo string, hook VCSHook) error
 
 	//Events