From 5c70bcedaed6d16c862efbb89b78fc6ad69db1b5 Mon Sep 17 00:00:00 2001 From: StephanHCB Date: Wed, 12 Jun 2024 16:10:08 +0200 Subject: [PATCH] fix(#292): add tests --- api/openapi-v3-spec.json | 31 ++++ .../web/controller/webhookctl/webhookctl.go | 69 ++++---- test/acceptance/util_setup_test.go | 6 + test/acceptance/webhook_acc_test.go | 147 ++++++++++++++++++ test/mock/bitbucketmock/bitbucketmock.go | 14 +- .../acceptance-expected/webhook-invalid.json | 5 + test/resources/valid-config.yaml | 2 +- 7 files changed, 241 insertions(+), 33 deletions(-) create mode 100644 test/acceptance/webhook_acc_test.go create mode 100644 test/resources/acceptance-expected/webhook-invalid.json diff --git a/api/openapi-v3-spec.json b/api/openapi-v3-spec.json index 1b94989..1c72aff 100644 --- a/api/openapi-v3-spec.json +++ b/api/openapi-v3-spec.json @@ -1874,6 +1874,37 @@ } } } + }, + "/webhook/bitbucket": { + "post": { + "tags": [ + "webhook" + ], + "operationId": "gitNotifyBitBucket", + "description": "webhook notification endpoint to be configured in bitbucket server to notify the service that it should check for new commits, or that a PR needs to be validated", + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "type": "object", + "additionalProperties": true + } + } + } + }, + "responses": { + "204": { + "description": "No content" + }, + "400": { + "description": "Payload parse failure - need to send a valid BitBucket webhook payload" + }, + "500": { + "description": "Internal Server Error" + } + } + } } }, "components": { diff --git a/internal/web/controller/webhookctl/webhookctl.go b/internal/web/controller/webhookctl/webhookctl.go index d2daf16..6527f3d 100644 --- a/internal/web/controller/webhookctl/webhookctl.go +++ b/internal/web/controller/webhookctl/webhookctl.go @@ -20,6 +20,8 @@ type Impl struct { Timestamp librepo.Timestamp Updater service.Updater PRValidator service.PRValidator + + EnableAsync bool } func New( @@ -33,6 +35,7 @@ func New( Timestamp: timestamp, Updater: updater, PRValidator: prValidator, + EnableAsync: true, } } @@ -82,40 +85,48 @@ func (c *Impl) WebhookBitBucket(w http.ResponseWriter, r *http.Request) { return } - routineCtx, routineCtxCancel := contexthelper.AsyncCopyRequestContext(ctx, "webhookBitbucket", "backgroundJob") - go func() { - defer routineCtxCancel() + if c.EnableAsync { + routineCtx, routineCtxCancel := contexthelper.AsyncCopyRequestContext(ctx, "webhookBitbucket", "backgroundJob") + go func() { + defer routineCtxCancel() - switch eventPayload.(type) { - case bitbucketserver.PullRequestOpenedPayload: - payload, ok := eventPayload.(bitbucketserver.PullRequestOpenedPayload) - c.validatePullRequest(routineCtx, "opened", ok, payload.PullRequest) - case bitbucketserver.PullRequestModifiedPayload: - payload, ok := eventPayload.(bitbucketserver.PullRequestModifiedPayload) - c.validatePullRequest(routineCtx, "modified", ok, payload.PullRequest) - case bitbucketserver.PullRequestFromReferenceUpdatedPayload: - payload, ok := eventPayload.(bitbucketserver.PullRequestFromReferenceUpdatedPayload) - c.validatePullRequest(routineCtx, "from_reference", ok, payload.PullRequest) - case bitbucketserver.RepositoryReferenceChangedPayload: - payload, ok := eventPayload.(bitbucketserver.RepositoryReferenceChangedPayload) - if !ok || len(payload.Changes) < 1 || payload.Changes[0].ReferenceID == "" { - aulogging.Logger.Ctx(routineCtx).Error().Printf("bad request while processing bitbucket webhook - got reference changed with invalid info - ignoring webhook") - return - } - aulogging.Logger.Ctx(routineCtx).Info().Printf("got repository reference changed, refreshing caches") - - err = c.Updater.PerformFullUpdateWithNotifications(routineCtx) - if err != nil { - aulogging.Logger.Ctx(routineCtx).Error().WithErr(err).Printf("webhook error") - } - default: - // ignore unknown events - } - }() + c.WebhookBitBucketProcessSync(routineCtx, eventPayload) + }() + } else { + c.WebhookBitBucketProcessSync(ctx, eventPayload) + } util.SuccessNoBody(ctx, w, r, http.StatusNoContent) } +func (c *Impl) WebhookBitBucketProcessSync(ctx context.Context, eventPayload any) { + switch eventPayload.(type) { + case bitbucketserver.PullRequestOpenedPayload: + payload, ok := eventPayload.(bitbucketserver.PullRequestOpenedPayload) + c.validatePullRequest(ctx, "opened", ok, payload.PullRequest) + case bitbucketserver.PullRequestModifiedPayload: + payload, ok := eventPayload.(bitbucketserver.PullRequestModifiedPayload) + c.validatePullRequest(ctx, "modified", ok, payload.PullRequest) + case bitbucketserver.PullRequestFromReferenceUpdatedPayload: + payload, ok := eventPayload.(bitbucketserver.PullRequestFromReferenceUpdatedPayload) + c.validatePullRequest(ctx, "from_reference", ok, payload.PullRequest) + case bitbucketserver.RepositoryReferenceChangedPayload: + payload, ok := eventPayload.(bitbucketserver.RepositoryReferenceChangedPayload) + if !ok || len(payload.Changes) < 1 || payload.Changes[0].ReferenceID == "" { + aulogging.Logger.Ctx(ctx).Error().Printf("bad request while processing bitbucket webhook - got reference changed with invalid info - ignoring webhook") + return + } + aulogging.Logger.Ctx(ctx).Info().Printf("got repository reference changed, refreshing caches") + + err := c.Updater.PerformFullUpdateWithNotifications(ctx) + if err != nil { + aulogging.Logger.Ctx(ctx).Error().WithErr(err).Printf("webhook error") + } + default: + // ignore unknown events + } +} + func (c *Impl) validatePullRequest(ctx context.Context, operation string, parsedOk bool, pullRequestPayload bitbucketserver.PullRequest) { description := fmt.Sprintf("id: %d, toRef: %s, fromRef: %s", pullRequestPayload.ID, pullRequestPayload.ToRef.ID, pullRequestPayload.FromRef.ID) if !parsedOk || pullRequestPayload.ID == 0 || pullRequestPayload.ToRef.ID == "" || pullRequestPayload.FromRef.ID == "" { diff --git a/test/acceptance/util_setup_test.go b/test/acceptance/util_setup_test.go index 81927c4..03c8562 100644 --- a/test/acceptance/util_setup_test.go +++ b/test/acceptance/util_setup_test.go @@ -6,6 +6,7 @@ import ( "github.com/Interhyp/metadata-service/internal/repository/notifier" "github.com/Interhyp/metadata-service/internal/service/trigger" "github.com/Interhyp/metadata-service/internal/web/app" + "github.com/Interhyp/metadata-service/internal/web/controller/webhookctl" "github.com/Interhyp/metadata-service/internal/web/server" "github.com/Interhyp/metadata-service/test/mock/bitbucketmock" "github.com/Interhyp/metadata-service/test/mock/idpmock" @@ -72,6 +73,8 @@ func (a *ApplicationWithMocksImpl) Create() error { return err } + a.WebhookCtl.(*webhookctl.Impl).EnableAsync = false + return nil } @@ -170,4 +173,7 @@ func tstReset() { for _, client := range notifierImpl.Clients { client.(*notifiermock.NotifierClientMock).Reset() } + bbImpl.Recording = nil + bbImpl.PRHead = "" + bbImpl.ChangedFilesResponse = nil } diff --git a/test/acceptance/webhook_acc_test.go b/test/acceptance/webhook_acc_test.go new file mode 100644 index 0000000..c7f6ce5 --- /dev/null +++ b/test/acceptance/webhook_acc_test.go @@ -0,0 +1,147 @@ +package acceptance + +import ( + "bytes" + "encoding/json" + "github.com/Interhyp/metadata-service/internal/acorn/repository" + "github.com/StephanHCB/go-backend-service-common/docs" + bitbucketserver "github.com/go-playground/webhooks/v6/bitbucket-server" + "github.com/stretchr/testify/require" + "net/http" + "testing" + "time" +) + +func TestPOSTWebhookBitbucket_Success(t *testing.T) { + tstReset() + + docs.Given("Given a pull request in BitBucket with valid file edits") + bbImpl.PRHead = "e2d2000000000000000000000000000000000000" + bbImpl.ChangedFilesResponse = []repository.File{ + { + Path: "owners/fun-owner/owner.info.yaml", + Contents: `contact: someone@example.com +productOwner: someone +displayName: Test Owner`, + }, + { + Path: "owners/fun-owner/services/golang-forever.yaml", + Contents: `description: golang is the best +repositories: + - golang-forever/implementation +alertTarget: someone@example.com +developmentOnly: false +operationType: WORKLOAD +lifecycle: experimental`, + }, + } + + docs.When("When BitBucket sends a webhook with valid payload") + body := bitbucketserver.PullRequestOpenedPayload{ + Date: bitbucketserver.Date(time.Now()), + EventKey: bitbucketserver.PullRequestOpenedEvent, + Actor: bitbucketserver.User{ + // don't care + }, + PullRequest: bitbucketserver.PullRequest{ + ID: 42, + Title: "some pr title", + Description: "some pr description", + FromRef: bitbucketserver.RepositoryReference{ + ID: "e2d2000000000000000000000000000000000000", // pr head + }, + ToRef: bitbucketserver.RepositoryReference{ + ID: "e100000000000000000000000000000000000000", // mainline + }, + Locked: false, + Author: bitbucketserver.PullRequestParticipant{}, + }, + } + bodyBytes, err := json.Marshal(&body) + require.Nil(t, err) + request, err := http.NewRequest(http.MethodPost, ts.URL+"/webhook/bitbucket", bytes.NewReader(bodyBytes)) + require.Nil(t, err) + request.Header.Set("X-Event-Key", string(bitbucketserver.PullRequestOpenedEvent)) + rawResponse, err := http.DefaultClient.Do(request) + require.Nil(t, err) + response, err := tstWebResponseFromResponse(rawResponse) + require.Nil(t, err) + + docs.Then("Then the request is successful") + tstAssertNoBody(t, response, err, http.StatusNoContent) + + docs.Then("And the expected interactions with the BitBucket API have occurred") + require.EqualValues(t, []string{ + "GetChangedFilesOnPullRequest(42)", + "CreatePullRequestComment(42, all changed files are valid|)", + "AddCommitBuildStatus(e2d2000000000000000000000000000000000000, metadata-service, true)", + }, bbImpl.Recording) +} + +func TestPOSTWebhookBitbucket_InvalidPR(t *testing.T) { + tstReset() + + docs.Given("Given a pull request in BitBucket with invalid file edits") + bbImpl.PRHead = "e2d2000000000000000000000000000000000000" + bbImpl.ChangedFilesResponse = []repository.File{ + { + Path: "owners/fun-owner/repositories/golang-forever.implementation.yaml", + Contents: `unknown: field`, + }, + } + + docs.When("When BitBucket sends a webhook with valid payload") + body := bitbucketserver.PullRequestOpenedPayload{ + Date: bitbucketserver.Date(time.Now()), + EventKey: bitbucketserver.PullRequestOpenedEvent, + Actor: bitbucketserver.User{ + // don't care + }, + PullRequest: bitbucketserver.PullRequest{ + ID: 42, + Title: "some pr title", + Description: "some pr description", + FromRef: bitbucketserver.RepositoryReference{ + ID: "e2d2000000000000000000000000000000000000", // pr head + }, + ToRef: bitbucketserver.RepositoryReference{ + ID: "e100000000000000000000000000000000000000", // mainline + }, + Locked: false, + Author: bitbucketserver.PullRequestParticipant{}, + }, + } + bodyBytes, err := json.Marshal(&body) + require.Nil(t, err) + request, err := http.NewRequest(http.MethodPost, ts.URL+"/webhook/bitbucket", bytes.NewReader(bodyBytes)) + require.Nil(t, err) + request.Header.Set("X-Event-Key", string(bitbucketserver.PullRequestOpenedEvent)) + rawResponse, err := http.DefaultClient.Do(request) + require.Nil(t, err) + response, err := tstWebResponseFromResponse(rawResponse) + require.Nil(t, err) + + docs.Then("Then the request is successful") + tstAssertNoBody(t, response, err, http.StatusNoContent) + + docs.Then("And the expected interactions with the BitBucket API have occurred") + require.EqualValues(t, []string{ + "GetChangedFilesOnPullRequest(42)", + "CreatePullRequestComment(42, # yaml validation failure||There were validation errors in changed files. Please fix yaml syntax and/or remove unknown fields:|| - failed to parse `owners/fun-owner/repositories/golang-forever.implementation.yaml`:| yaml: unmarshal errors:| line 1: field unknown not found in type openapi.RepositoryDto|)", + "AddCommitBuildStatus(e2d2000000000000000000000000000000000000, metadata-service, false)", + }, bbImpl.Recording) +} + +func TestPOSTWebhookBitbucket_InvalidPayload(t *testing.T) { + tstReset() + + docs.Given("Given an unauthenticated user") + token := tstUnauthenticated() + + docs.When("When they send a webhook with invalid payload") + body := bitbucketserver.PullRequestOpenedPayload{} // hopefully invalid + response, err := tstPerformPost("/webhook/bitbucket", token, &body) + + docs.Then("Then the request fails and the error response is as expected") + tstAssert(t, response, err, http.StatusBadRequest, "webhook-invalid.json") +} diff --git a/test/mock/bitbucketmock/bitbucketmock.go b/test/mock/bitbucketmock/bitbucketmock.go index 98e1d89..db5d722 100644 --- a/test/mock/bitbucketmock/bitbucketmock.go +++ b/test/mock/bitbucketmock/bitbucketmock.go @@ -2,13 +2,18 @@ package bitbucketmock import ( "context" + "fmt" "github.com/Interhyp/metadata-service/internal/acorn/repository" "github.com/pkg/errors" + "strings" ) const FILTER_FAILED_USERNAME = "filterfailedusername" type BitbucketMock struct { + ChangedFilesResponse []repository.File + PRHead string + Recording []string } func New() repository.Bitbucket { @@ -51,13 +56,16 @@ func (b *BitbucketMock) FilterExistingUsernames(ctx context.Context, usernames [ } func (b *BitbucketMock) GetChangedFilesOnPullRequest(ctx context.Context, pullRequestId int) ([]repository.File, string, error) { - return []repository.File{}, "", nil + b.Recording = append(b.Recording, fmt.Sprintf("GetChangedFilesOnPullRequest(%d)", pullRequestId)) + return b.ChangedFilesResponse, b.PRHead, nil } -func (r *BitbucketMock) AddCommitBuildStatus(ctx context.Context, commitHash string, url string, key string, success bool) error { +func (b *BitbucketMock) AddCommitBuildStatus(ctx context.Context, commitHash string, url string, key string, success bool) error { + b.Recording = append(b.Recording, fmt.Sprintf("AddCommitBuildStatus(%s, %s, %t)", commitHash, key, success)) return nil } -func (r *BitbucketMock) CreatePullRequestComment(ctx context.Context, pullRequestId int, comment string) error { +func (b *BitbucketMock) CreatePullRequestComment(ctx context.Context, pullRequestId int, comment string) error { + b.Recording = append(b.Recording, fmt.Sprintf("CreatePullRequestComment(%d, %s)", pullRequestId, strings.ReplaceAll(comment, "\n", "|"))) return nil } diff --git a/test/resources/acceptance-expected/webhook-invalid.json b/test/resources/acceptance-expected/webhook-invalid.json new file mode 100644 index 0000000..da02430 --- /dev/null +++ b/test/resources/acceptance-expected/webhook-invalid.json @@ -0,0 +1,5 @@ +{ + "details": "parse payload error", + "message": "webhook.payload.invalid", + "timestamp": "2022-11-06T18:14:10Z" +} \ No newline at end of file diff --git a/test/resources/valid-config.yaml b/test/resources/valid-config.yaml index 6e77ce6..05b21ca 100644 --- a/test/resources/valid-config.yaml +++ b/test/resources/valid-config.yaml @@ -9,7 +9,7 @@ PULL_REQUEST_BUILD_URL: https://metadata-service.example.com AUTH_OIDC_TOKEN_AUDIENCE: some-audience AUTH_GROUP_WRITE: admin -SSH_METADATA_REPO_URL: git://metadata +SSH_METADATA_REPO_URL: git://er/metadata.git METADATA_REPO_URL: http://metadata UPDATE_JOB_INTERVAL_MINUTES: 5