Skip to content

Commit

Permalink
fix logic on delete webhooks, ensure we dont use bitbucket client whe…
Browse files Browse the repository at this point in the history
…re the context was canceled for deleting webhooks and ensure the custom client uses the ctx as well.

Signed-off-by: Ryan Currah <[email protected]>
  • Loading branch information
ryancurrah committed Jan 27, 2024
1 parent 7c10b21 commit fa58b2d
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 11 deletions.
29 changes: 19 additions & 10 deletions eventsources/sources/bitbucketserver/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func (router *Router) HandleRoute(writer http.ResponseWriter, request *http.Requ
return
}

// When SkipRefsOnOpenPullRequest is enabled and webhook event type is repo:refs_changed,
// When SkipBranchRefsChangedOnOpenPR is enabled and webhook event type is repo:refs_changed,
// check if a Pull Request is opened for the commit, if one is opened the event will be skipped.
if bitbucketserverEventSource.SkipBranchRefsChangedOnOpenPR && slices.Contains(bitbucketserverEventSource.Events, "repo:refs_changed") {
refsChanged := refsChangedWebhookEvent{}
Expand Down Expand Up @@ -164,7 +164,12 @@ func (router *Router) PostInactivate() error {
route := router.route
logger := route.Logger

if !bitbucketserverEventSource.DeleteHookOnFinish && len(router.hookIDs) == 0 {
if !bitbucketserverEventSource.DeleteHookOnFinish {
logger.Info("not configured to delete webhooks, skipping")
return nil
}

if len(router.hookIDs) == 0 {
logger.Info("no need to delete webhooks, skipping")
return nil
}
Expand All @@ -174,7 +179,7 @@ func (router *Router) PostInactivate() error {
bitbucketRepositories := bitbucketserverEventSource.GetBitbucketServerRepositories()

if len(bitbucketserverEventSource.Projects) > 0 {
bitbucketProjectRepositories, err := router.getProjectRepositories(bitbucketserverEventSource.Projects)
bitbucketProjectRepositories, err := getProjectRepositories(router.deleteClient, bitbucketserverEventSource.Projects)
if err != nil {
return err
}
Expand All @@ -188,7 +193,7 @@ func (router *Router) PostInactivate() error {
return fmt.Errorf("can not find hook ID for project-key: %s, repository-slug: %s", repo.ProjectKey, repo.RepositorySlug)
}

_, err := router.client.DefaultApi.DeleteWebhook(repo.ProjectKey, repo.RepositorySlug, int32(id))
_, err := router.deleteClient.DefaultApi.DeleteWebhook(repo.ProjectKey, repo.RepositorySlug, int32(id))
if err != nil {
return fmt.Errorf("failed to delete bitbucketserver webhook. err: %w", err)
}
Expand Down Expand Up @@ -235,16 +240,19 @@ func (el *EventListener) StartListening(ctx context.Context, dispatch func([]byt
defer cancel()

bitbucketClient := newBitbucketServerClient(ctx, bitbucketConfig, bitbucketToken)
bitbucketDeleteClient := newBitbucketServerClient(context.Background(), bitbucketConfig, bitbucketToken)

route := webhook.NewRoute(bitbucketserverEventSource.Webhook, logger, el.GetEventSourceName(), el.GetEventName(), el.Metrics)
router := &Router{
route: route,
client: bitbucketClient,
customClient: &customBitbucketClient{
client: bitbucketConfig.HTTPClient,
ctx: ctx,
token: bitbucketToken,
url: bitbucketURL,
},
deleteClient: bitbucketDeleteClient,
bitbucketserverEventSource: bitbucketserverEventSource,
hookIDs: make(map[string]int),
}
Expand All @@ -268,7 +276,7 @@ func (el *EventListener) StartListening(ctx context.Context, dispatch func([]byt
bitbucketRepositories := bitbucketserverEventSource.GetBitbucketServerRepositories()

if len(bitbucketserverEventSource.Projects) > 0 {
bitbucketProjectRepositories, err := router.getProjectRepositories(bitbucketserverEventSource.Projects)
bitbucketProjectRepositories, err := getProjectRepositories(router.client, bitbucketserverEventSource.Projects)
if err != nil {
logger.Errorw("failed to apply Bitbucket webhook", zap.Error(err))
}
Expand All @@ -287,8 +295,8 @@ func (el *EventListener) StartListening(ctx context.Context, dispatch func([]byt
}
}

// When running multiple replicas of the eventsource, they will all try to create the webhook.
// Randomly sleep some time to mitigate the issue.
// When running multiple replicas of this event source, they will try to create webhooks at the same time.
// Randomly delay running the initial apply webhooks func to mitigate the issue.
randomNum, _ := rand.Int(rand.Reader, big.NewInt(int64(2000)))
time.Sleep(time.Duration(randomNum.Int64()) * time.Millisecond)
applyWebhooks()
Expand Down Expand Up @@ -517,12 +525,12 @@ type bitbucketServerReposPager struct {
}

// getProjectRepositories returns all the Bitbucket Server repositories in the provided projects
func (router *Router) getProjectRepositories(projects []string) ([]v1alpha1.BitbucketServerRepository, error) {
func getProjectRepositories(client *bitbucketv1.APIClient, projects []string) ([]v1alpha1.BitbucketServerRepository, error) {
var bitbucketRepos []bitbucketv1.Repository
for _, project := range projects {
paginationOptions := map[string]interface{}{"start": 0, "limit": 500}
for {
response, err := router.client.DefaultApi.GetRepositoriesWithOptions(project, paginationOptions)
response, err := client.DefaultApi.GetRepositoriesWithOptions(project, paginationOptions)
if err != nil {
return nil, fmt.Errorf("unable to list repositories for project %s: %w", project, err)
}
Expand Down Expand Up @@ -600,6 +608,7 @@ type refsChangedWebhookEvent struct {
// Specifically getting Pull Requests associated to a commit is not supported by gfleury/go-bitbucket-v1.
type customBitbucketClient struct {
client *http.Client
ctx context.Context
token string
url *url.URL
}
Expand All @@ -622,7 +631,7 @@ func (c *customBitbucketClient) authHeader(req *http.Request) {
}

func (c *customBitbucketClient) get(u string) ([]byte, error) {
req, err := http.NewRequest(http.MethodGet, u, nil)
req, err := http.NewRequestWithContext(c.ctx, http.MethodGet, u, nil)
if err != nil {
return nil, err
}
Expand Down
2 changes: 2 additions & 0 deletions eventsources/sources/bitbucketserver/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ type Router struct {
client *bitbucketv1.APIClient
// customClient is a custom bitbucket server client which implements a method the gfleury/go-bitbucket-v1 client is missing
customClient *customBitbucketClient
// deleteClient is used to delete webhooks. This client does not contain the cancelable context of the default client
deleteClient *bitbucketv1.APIClient
// hookIDs is a map of webhook IDs
// (projectKey + "," + repoSlug) -> hook ID
// Bitbucket Server API docs:
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ require (
go.uber.org/ratelimit v0.2.0
go.uber.org/zap v1.26.0
golang.org/x/crypto v0.18.0
golang.org/x/exp v0.0.0-20230905200255-921286631fa9
google.golang.org/api v0.159.0
google.golang.org/grpc v1.61.0
gopkg.in/jcmturner/gokrb5.v5 v5.3.0
Expand Down Expand Up @@ -160,7 +161,6 @@ require (
go.opentelemetry.io/otel v1.22.0 // indirect
go.opentelemetry.io/otel/metric v1.22.0 // indirect
go.opentelemetry.io/otel/trace v1.22.0 // indirect
golang.org/x/exp v0.0.0-20230905200255-921286631fa9 // indirect
gomodules.xyz/envconfig v1.3.1-0.20190308184047-426f31af0d45 // indirect
gomodules.xyz/notify v0.1.1 // indirect
google.golang.org/genproto/googleapis/api v0.0.0-20240122161410-6c6643bf1457 // indirect
Expand Down

0 comments on commit fa58b2d

Please sign in to comment.