Skip to content

Commit

Permalink
Fetch Pull Request as part of Comment Event Conversion (#241)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Aayyush
Aayyush authored Apr 18, 2022
1 parent 16c0034 commit 98965e8
Show file tree
Hide file tree
Showing 19 changed files with 709 additions and 432 deletions.
128 changes: 96 additions & 32 deletions server/controllers/events/events_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,22 @@ const bitbucketCloudRequestIDHeader = "X-Request-UUID"
const bitbucketServerRequestIDHeader = "X-Request-ID"
const bitbucketServerSignatureHeader = "X-Hub-Signature"

//go:generate pegomock generate -m --use-experimental-model-gen --package mocks -o mocks/mock_azuredevops_pull_getter.go AzureDevopsPullGetter

// AzureDevopsPullGetter makes API calls to get pull requests.
type AzureDevopsPullGetter interface {
// GetPullRequest gets the pull request with id pullNum for the repo.
GetPullRequest(repo models.Repo, pullNum int) (*azuredevops.GitPullRequest, error)
}

//go:generate pegomock generate -m --use-experimental-model-gen --package mocks -o mocks/mock_gitlab_merge_request_getter.go GitlabMergeRequestGetter

// GitlabMergeRequestGetter makes API calls to get merge requests.
type GitlabMergeRequestGetter interface {
// GetMergeRequest gets the pull request with the id pullNum for the repo.
GetMergeRequest(repoFullName string, pullNum int) (*gitlab.MergeRequest, error)
}

type commentEventHandler interface {
Handle(ctx context.Context, request *httputils.BufferedRequest, event event_types.Comment) error
}
Expand Down Expand Up @@ -98,6 +114,9 @@ func NewVCSEventsController(
azureDevopsWebhookBasicPassword []byte,
repoConverter github_converter.RepoConverter,
pullConverter github_converter.PullConverter,
githubPullGetter github_converter.PullGetter,
azureDevopsPullGetter AzureDevopsPullGetter,
gitlabMergeRequestGetter GitlabMergeRequestGetter,
) *VCSEventsController {

prHandler := handlers.NewPullRequestEvent(
Expand Down Expand Up @@ -125,6 +144,7 @@ func NewVCSEventsController(
allowDraftPRs,
repoConverter,
pullConverter,
githubPullGetter,
)
},
}
Expand All @@ -151,6 +171,8 @@ func NewVCSEventsController(
AzureDevopsWebhookBasicUser: azureDevopsWebhookBasicUser,
AzureDevopsWebhookBasicPassword: azureDevopsWebhookBasicPassword,
AzureDevopsRequestValidator: &DefaultAzureDevopsRequestValidator{},
AzureDevopsPullGetter: azureDevopsPullGetter,
GitlabMergeRequestGetter: gitlabMergeRequestGetter,
}
}

Expand Down Expand Up @@ -265,6 +287,9 @@ type VCSEventsController struct {
// Azure DevOps Team Project. If empty, no request validation is done.
AzureDevopsWebhookBasicPassword []byte
AzureDevopsRequestValidator AzureDevopsRequestValidator

GitlabMergeRequestGetter GitlabMergeRequestGetter
AzureDevopsPullGetter AzureDevopsPullGetter
}

// Post handles POST webhook requests.
Expand Down Expand Up @@ -413,14 +438,14 @@ func (e *VCSEventsController) HandleBitbucketCloudCommentEvent(w http.ResponseWr
return
}
err = e.CommentEventHandler.Handle(context.TODO(), cloneableRequest, event_types.Comment{
BaseRepo: baseRepo,
MaybeHeadRepo: &headRepo,
MaybePull: &pull,
User: user,
PullNum: pull.Num,
Comment: comment,
VCSHost: models.BitbucketCloud,
Timestamp: eventTimestamp,
BaseRepo: baseRepo,
HeadRepo: headRepo,
Pull: pull,
User: user,
PullNum: pull.Num,
Comment: comment,
VCSHost: models.BitbucketCloud,
Timestamp: eventTimestamp,
})

if err != nil {
Expand All @@ -446,14 +471,14 @@ func (e *VCSEventsController) HandleBitbucketServerCommentEvent(w http.ResponseW
return
}
err = e.CommentEventHandler.Handle(context.TODO(), cloneableRequest, event_types.Comment{
BaseRepo: baseRepo,
MaybeHeadRepo: &headRepo,
MaybePull: &pull,
User: user,
PullNum: pull.Num,
Comment: comment,
VCSHost: models.BitbucketServer,
Timestamp: eventTimestamp,
BaseRepo: baseRepo,
HeadRepo: headRepo,
Pull: pull,
User: user,
PullNum: pull.Num,
Comment: comment,
VCSHost: models.BitbucketServer,
Timestamp: eventTimestamp,
})

if err != nil {
Expand Down Expand Up @@ -558,6 +583,12 @@ func (e *VCSEventsController) HandleGitlabCommentEvent(w http.ResponseWriter, ev
e.respond(w, logging.Error, http.StatusBadRequest, "Error parsing webhook: %s", err)
return
}

pull, err := e.getGitlabData(baseRepo, event.MergeRequest.ID)
if err != nil {
e.respond(w, logging.Error, http.StatusBadRequest, "Error getting merge request: %s", err)
return
}
eventTimestamp := time.Now()
lvl := logging.Debug
cloneableRequest, err := httputils.NewBufferedRequest(request)
Expand All @@ -566,14 +597,14 @@ func (e *VCSEventsController) HandleGitlabCommentEvent(w http.ResponseWriter, ev
return
}
err = e.CommentEventHandler.Handle(context.TODO(), cloneableRequest, event_types.Comment{
BaseRepo: baseRepo,
MaybeHeadRepo: &headRepo,
MaybePull: nil,
User: user,
PullNum: event.MergeRequest.IID,
Comment: event.ObjectAttributes.Note,
VCSHost: models.Gitlab,
Timestamp: eventTimestamp,
BaseRepo: baseRepo,
HeadRepo: headRepo,
Pull: pull,
User: user,
PullNum: event.MergeRequest.IID,
Comment: event.ObjectAttributes.Note,
VCSHost: models.Gitlab,
Timestamp: eventTimestamp,
})

if err != nil {
Expand Down Expand Up @@ -645,6 +676,12 @@ func (e *VCSEventsController) HandleAzureDevopsPullRequestCommentedEvent(w http.
e.respond(w, logging.Error, http.StatusBadRequest, "Error parsing pull request repository field: %s; %s", err, azuredevopsReqID)
return
}
pull, headRepo, err := e.getAzureDevopsData(baseRepo, resource.PullRequest.GetPullRequestID())
if err != nil {
e.respond(w, logging.Error, http.StatusBadRequest, "Error getting pull request %s", err)
return
}

eventTimestamp := time.Now()
lvl := logging.Debug
cloneableRequest, err := httputils.NewBufferedRequest(request)
Expand All @@ -653,14 +690,14 @@ func (e *VCSEventsController) HandleAzureDevopsPullRequestCommentedEvent(w http.
return
}
err = e.CommentEventHandler.Handle(context.TODO(), cloneableRequest, event_types.Comment{
BaseRepo: baseRepo,
MaybeHeadRepo: nil,
MaybePull: nil,
User: user,
PullNum: resource.PullRequest.GetPullRequestID(),
Comment: string(strippedComment),
VCSHost: models.AzureDevops,
Timestamp: eventTimestamp,
BaseRepo: baseRepo,
HeadRepo: headRepo,
Pull: pull,
User: user,
PullNum: resource.PullRequest.GetPullRequestID(),
Comment: string(strippedComment),
VCSHost: models.AzureDevops,
Timestamp: eventTimestamp,
})

if err != nil {
Expand Down Expand Up @@ -735,3 +772,30 @@ func (e *VCSEventsController) respond(w http.ResponseWriter, lvl logging.LogLeve
w.WriteHeader(code)
fmt.Fprintln(w, response)
}

func (c *VCSEventsController) getGitlabData(baseRepo models.Repo, pullNum int) (models.PullRequest, error) {
if c.GitlabMergeRequestGetter == nil {
return models.PullRequest{}, errors.New("Atlantis not configured to support GitLab")
}
mr, err := c.GitlabMergeRequestGetter.GetMergeRequest(baseRepo.FullName, pullNum)
if err != nil {
return models.PullRequest{}, errors.Wrap(err, "making merge request API call to GitLab")
}
pull := c.Parser.ParseGitlabMergeRequest(mr, baseRepo)
return pull, nil
}

func (c *VCSEventsController) getAzureDevopsData(baseRepo models.Repo, pullNum int) (models.PullRequest, models.Repo, error) {
if c.AzureDevopsPullGetter == nil {
return models.PullRequest{}, models.Repo{}, errors.New("atlantis not configured to support Azure DevOps")
}
adPull, err := c.AzureDevopsPullGetter.GetPullRequest(baseRepo, pullNum)
if err != nil {
return models.PullRequest{}, models.Repo{}, errors.Wrap(err, "making pull request API call to Azure DevOps")
}
pull, _, headRepo, err := c.Parser.ParseAzureDevopsPull(adPull)
if err != nil {
return pull, headRepo, errors.Wrap(err, "extracting required fields from comment data")
}
return pull, headRepo, nil
}
3 changes: 1 addition & 2 deletions server/controllers/events/events_controller_e2e_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -912,9 +912,7 @@ func setupE2E(t *testing.T, repoFixtureDir string, userConfig *server.UserConfig
}
staleCommandChecker := &testStaleCommandChecker{}
commandRunner := &events.DefaultCommandRunner{
EventParser: eventParser,
VCSClient: vcsClient,
GithubPullGetter: ghClient,
GlobalCfg: globalCfg,
StatsScope: statsScope,
CommentCommandRunnerByCmd: commentCommandRunnerByCmd,
Expand Down Expand Up @@ -986,6 +984,7 @@ func setupE2E(t *testing.T, repoFixtureDir string, userConfig *server.UserConfig
false,
repoConverter,
pullConverter,
ghClient,
),
},
}
Expand Down
50 changes: 38 additions & 12 deletions server/controllers/events/events_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func AnyStatus() []*github.RepoStatus {

func TestPost_NotGitlab(t *testing.T) {
t.Log("when the request is not for gitlab or github a 400 is returned")
e, _, _, _, _, _ := setup(t)
e, _, _, _, _, _, _ := setup(t)
w := httptest.NewRecorder()
req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil))
e.Post(w, req)
Expand All @@ -67,7 +67,7 @@ func TestPost_NotGitlab(t *testing.T) {

func TestPost_UnsupportedVCSGitlab(t *testing.T) {
t.Log("when the request is for an unsupported vcs a 400 is returned")
e, _, _, _, _, _ := setup(t)
e, _, _, _, _, _, _ := setup(t)
e.SupportedVCSHosts = nil
req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil))
req.Header.Set(gitlabHeader, "value")
Expand All @@ -78,7 +78,7 @@ func TestPost_UnsupportedVCSGitlab(t *testing.T) {

func TestPost_InvalidGitlabSecret(t *testing.T) {
t.Log("when the gitlab payload can't be validated a 400 is returned")
e, gl, _, _, _, _ := setup(t)
e, gl, _, _, _, _, _ := setup(t)
w := httptest.NewRecorder()
req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil))
req.Header.Set(gitlabHeader, "value")
Expand All @@ -89,7 +89,7 @@ func TestPost_InvalidGitlabSecret(t *testing.T) {

func TestPost_UnsupportedGitlabEvent(t *testing.T) {
t.Log("when the event type is an unsupported gitlab event we ignore it")
e, gl, _, _, _, _ := setup(t)
e, gl, _, _, _, _, _ := setup(t)
w := httptest.NewRecorder()
req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil))
req.Header.Set(gitlabHeader, "value")
Expand All @@ -101,7 +101,7 @@ func TestPost_UnsupportedGitlabEvent(t *testing.T) {
// Test that if the comment comes from a commit rather than a merge request,
// we give an error and ignore it.
func TestPost_GitlabCommentOnCommit(t *testing.T) {
e, gl, _, _, _, _ := setup(t)
e, gl, _, _, _, _, _ := setup(t)
req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil))
w := httptest.NewRecorder()
req.Header.Set(gitlabHeader, "value")
Expand All @@ -112,7 +112,7 @@ func TestPost_GitlabCommentOnCommit(t *testing.T) {

func TestPost_GitlabCommentSuccess(t *testing.T) {
t.Log("when the event is a gitlab comment with a valid command we call the command handler")
e, gl, _, _, _, _ := setup(t)
e, gl, _, _, _, _, _ := setup(t)
req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil))
req.Header.Set(gitlabHeader, "value")
When(gl.ParseAndValidate(req, secret)).ThenReturn(gitlab.MergeCommentEvent{}, nil)
Expand All @@ -123,7 +123,7 @@ func TestPost_GitlabCommentSuccess(t *testing.T) {

func TestPost_GitlabMergeRequestInvalid(t *testing.T) {
t.Log("when the event is a gitlab merge request with invalid data we return a 400")
e, gl, p, _, _, _ := setup(t)
e, gl, p, _, _, _, _ := setup(t)
req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil))
req.Header.Set(gitlabHeader, "value")
When(gl.ParseAndValidate(req, secret)).ThenReturn(gitlab.MergeEvent{}, nil)
Expand All @@ -138,7 +138,7 @@ func TestPost_GitlabMergeRequestInvalid(t *testing.T) {
func TestPost_GitlabMergeRequestClosedErrCleaningPull(t *testing.T) {
t.Skip("relies too much on mocks, should use real event parser")
t.Log("when the event is a closed gitlab merge request and an error occurs calling CleanUpPull we return a 500")
e, gl, p, c, _, _ := setup(t)
e, gl, p, c, _, _, _ := setup(t)
req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil))
req.Header.Set(gitlabHeader, "value")
var event gitlab.MergeEvent
Expand All @@ -156,7 +156,7 @@ func TestPost_GitlabMergeRequestClosedErrCleaningPull(t *testing.T) {
func TestPost_GitlabMergeRequestSuccess(t *testing.T) {
t.Skip("relies too much on mocks, should use real event parser")
t.Log("when the event is a gitlab merge request and the cleanup works we return a 200")
e, gl, p, _, _, _ := setup(t)
e, gl, p, _, _, _, _ := setup(t)
req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil))
req.Header.Set(gitlabHeader, "value")
When(gl.ParseAndValidate(req, secret)).ThenReturn(gitlab.MergeEvent{}, nil)
Expand All @@ -168,6 +168,27 @@ func TestPost_GitlabMergeRequestSuccess(t *testing.T) {
ResponseContains(t, w, http.StatusOK, "Pull request cleaned successfully")
}

func TestPost_GitlabMergeRequestError(t *testing.T) {
t.Log("if getting the gitlab merge request fails we return 400 with error")

// setup
e, gl, p, _, _, _, gl_getter := setup(t)
req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil))
req.Header.Set(gitlabHeader, "value")
repo := models.Repo{}
pullRequest := models.PullRequest{}
When(gl.ParseAndValidate(req, secret)).ThenReturn(gitlab.MergeCommentEvent{}, nil)
When(p.ParseGitlabMergeRequestEvent(gitlab.MergeEvent{})).ThenReturn(pullRequest, models.OpenedPullEvent, repo, repo, models.User{}, nil)
When(gl_getter.GetMergeRequest(repo.FullName, pullRequest.Num)).ThenReturn(nil, errors.New("error"))
w := httptest.NewRecorder()

// act
e.Post(w, req)

// assert
ResponseContains(t, w, http.StatusBadRequest, "error")
}

// Test Bitbucket server pull closed events.
func TestPost_BBServerPullClosed(t *testing.T) {
cases := []struct {
Expand Down Expand Up @@ -247,7 +268,7 @@ func TestPost_PullOpenedOrUpdated(t *testing.T) {

for _, c := range cases {
t.Run(c.Description, func(t *testing.T) {
e, gl, p, _, _, _ := setup(t)
e, gl, p, _, _, _, _ := setup(t)
req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil))
var pullRequest models.PullRequest
var repo models.Repo
Expand All @@ -270,8 +291,10 @@ func TestPost_PullOpenedOrUpdated(t *testing.T) {
}
}

func setup(t *testing.T) (events_controllers.VCSEventsController, *mocks.MockGitlabRequestParserValidator, *emocks.MockEventParsing, *emocks.MockPullCleaner, *vcsmocks.MockClient, *emocks.MockCommentParsing) {
func setup(t *testing.T) (events_controllers.VCSEventsController, *mocks.MockGitlabRequestParserValidator, *emocks.MockEventParsing, *emocks.MockPullCleaner, *vcsmocks.MockClient, *emocks.MockCommentParsing, *mocks.MockGitlabMergeRequestGetter) {
RegisterMockTestingT(t)
gitLabMock := mocks.NewMockGitlabMergeRequestGetter()
azureDevopsMock := mocks.NewMockAzureDevopsPullGetter()
gl := mocks.NewMockGitlabRequestParserValidator()
p := emocks.NewMockEventParsing()
cp := emocks.NewMockCommentParsing()
Expand All @@ -294,8 +317,11 @@ func setup(t *testing.T) (events_controllers.VCSEventsController, *mocks.MockGit
GitlabRequestParserValidator: gl,
RepoAllowlistChecker: repoAllowlistChecker,
VCSClient: vcsmock,

GitlabMergeRequestGetter: gitLabMock,
AzureDevopsPullGetter: azureDevopsMock,
}
return e, gl, p, c, vcsmock, cp
return e, gl, p, c, vcsmock, cp, gitLabMock
}

// This struct shouldn't be using these anyways since it should be broken down into separate packages (ie. see github)
Expand Down
4 changes: 2 additions & 2 deletions server/controllers/events/handlers/comment.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ func (h *CommandHandler) Handle(ctx context.Context, _ *http.BufferedRequest, ev
h.CommandRunner.RunCommentCommand(
ctx,
event.BaseRepo,
event.MaybeHeadRepo,
event.MaybePull,
event.HeadRepo,
event.Pull,
event.User,
event.PullNum,
command,
Expand Down
Loading

0 comments on commit 98965e8

Please sign in to comment.