diff --git a/server/events/command_handler.go b/server/events/command_handler.go index fb70901fe2..52dd2090bd 100644 --- a/server/events/command_handler.go +++ b/server/events/command_handler.go @@ -86,7 +86,7 @@ func (c *CommandHandler) ExecuteCommand(baseRepo models.Repo, headRepo models.Re case models.Github: pull, headRepo, err = c.getGithubData(baseRepo, pullNum) case models.Gitlab: - pull, err = c.getGitlabData(baseRepo.FullName, pullNum) + pull, err = c.getGitlabData(baseRepo, pullNum) default: err = errors.New("Unknown VCS type, this is a bug!") } @@ -120,15 +120,15 @@ func (c *CommandHandler) getGithubData(baseRepo models.Repo, pullNum int) (model return pull, repo, nil } -func (c *CommandHandler) getGitlabData(repoFullName string, pullNum int) (models.PullRequest, error) { +func (c *CommandHandler) 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(repoFullName, pullNum) + 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.EventParser.ParseGitlabMergeRequest(mr) + pull := c.EventParser.ParseGitlabMergeRequest(mr, baseRepo) return pull, nil } diff --git a/server/events/event_parser.go b/server/events/event_parser.go index eb372704f8..d42e4beda0 100644 --- a/server/events/event_parser.go +++ b/server/events/event_parser.go @@ -48,7 +48,7 @@ type EventParsing interface { ParseGithubRepo(ghRepo *github.Repository) (models.Repo, error) ParseGitlabMergeEvent(event gitlab.MergeEvent) (models.PullRequest, models.Repo, error) ParseGitlabMergeCommentEvent(event gitlab.MergeCommentEvent) (baseRepo models.Repo, headRepo models.Repo, user models.User, err error) - ParseGitlabMergeRequest(mr *gitlab.MergeRequest) models.PullRequest + ParseGitlabMergeRequest(mr *gitlab.MergeRequest, baseRepo models.Repo) models.PullRequest } type EventParser struct { @@ -104,7 +104,11 @@ func (e *EventParser) ParseGithubPull(pull *github.PullRequest) (models.PullRequ return pullModel, headRepoModel, errors.New("number is null") } - headRepoModel, err := e.ParseGithubRepo(pull.Head.Repo) + baseRepoModel, err := e.ParseGithubRepo(pull.Base.Repo) + if err != nil { + return pullModel, headRepoModel, err + } + headRepoModel, err = e.ParseGithubRepo(pull.Head.Repo) if err != nil { return pullModel, headRepoModel, err } @@ -121,6 +125,7 @@ func (e *EventParser) ParseGithubPull(pull *github.PullRequest) (models.PullRequ URL: url, Num: num, State: pullState, + BaseRepo: baseRepoModel, }, headRepoModel, nil } @@ -136,6 +141,7 @@ func (e *EventParser) ParseGitlabMergeEvent(event gitlab.MergeEvent) (models.Pul // GitLab also has a "merged" state, but we map that to Closed so we don't // need to check for it. + repo, err := models.NewRepo(models.Gitlab, event.Project.PathWithNamespace, event.Project.GitHTTPURL, e.GitlabUser, e.GitlabToken) pull := models.PullRequest{ URL: event.ObjectAttributes.URL, Author: event.User.Username, @@ -143,9 +149,9 @@ func (e *EventParser) ParseGitlabMergeEvent(event gitlab.MergeEvent) (models.Pul HeadCommit: event.ObjectAttributes.LastCommit.ID, Branch: event.ObjectAttributes.SourceBranch, State: modelState, + BaseRepo: repo, } - repo, err := models.NewRepo(models.Gitlab, event.Project.PathWithNamespace, event.Project.GitHTTPURL, e.GitlabUser, e.GitlabToken) return pull, repo, err } @@ -169,7 +175,11 @@ func (e *EventParser) ParseGitlabMergeCommentEvent(event gitlab.MergeCommentEven return } -func (e *EventParser) ParseGitlabMergeRequest(mr *gitlab.MergeRequest) models.PullRequest { +// ParseGitlabMergeRequest parses the merge requests and returns a pull request +// model. We require passing in baseRepo because although can't get this information +// from the merge request, the only caller of this function already has that +// data. This means we can construct the pull request object correctly. +func (e *EventParser) ParseGitlabMergeRequest(mr *gitlab.MergeRequest, baseRepo models.Repo) models.PullRequest { pullState := models.Closed if mr.State == gitlabPullOpened { pullState = models.Open @@ -184,5 +194,6 @@ func (e *EventParser) ParseGitlabMergeRequest(mr *gitlab.MergeRequest) models.Pu HeadCommit: mr.SHA, Branch: mr.SourceBranch, State: pullState, + BaseRepo: baseRepo, } } diff --git a/server/events/event_parser_test.go b/server/events/event_parser_test.go index 3d4ab57e3f..9928dc69c9 100644 --- a/server/events/event_parser_test.go +++ b/server/events/event_parser_test.go @@ -128,7 +128,7 @@ func TestParseGithubPull(t *testing.T) { _, _, err = parser.ParseGithubPull(&testPull) ErrEquals(t, "number is null", err) - pullRes, repoRes, err := parser.ParseGithubPull(&Pull) + pullRes, _, err := parser.ParseGithubPull(&Pull) Ok(t, err) Equals(t, models.PullRequest{ URL: Pull.GetHTMLURL(), @@ -137,19 +137,18 @@ func TestParseGithubPull(t *testing.T) { HeadCommit: Pull.Head.GetSHA(), Num: Pull.GetNumber(), State: models.Open, - }, pullRes) - - Equals(t, models.Repo{ - Owner: "owner", - FullName: "owner/repo", - CloneURL: "https://github-user:github-token@github.com/owner/repo.git", - SanitizedCloneURL: Repo.GetCloneURL(), - Name: "repo", - VCSHost: models.VCSHost{ - Hostname: "github.com", - Type: models.Github, + BaseRepo: models.Repo{ + Owner: "owner", + FullName: "owner/repo", + CloneURL: "https://github-user:github-token@github.com/owner/repo.git", + SanitizedCloneURL: Repo.GetCloneURL(), + Name: "repo", + VCSHost: models.VCSHost{ + Hostname: "github.com", + Type: models.Github, + }, }, - }, repoRes) + }, pullRes) } func TestParseGitlabMergeEvent(t *testing.T) { @@ -159,16 +158,8 @@ func TestParseGitlabMergeEvent(t *testing.T) { Ok(t, err) pull, repo, err := parser.ParseGitlabMergeEvent(*event) Ok(t, err) - Equals(t, models.PullRequest{ - URL: "http://example.com/diaspora/merge_requests/1", - Author: "root", - Num: 1, - HeadCommit: "da1560886d4f094c3e6c9ef40349f7d38b5d27d7", - Branch: "ms-viewport", - State: models.Open, - }, pull) - Equals(t, models.Repo{ + expRepo := models.Repo{ FullName: "gitlabhq/gitlab-test", Name: "gitlab-test", SanitizedCloneURL: "https://example.com/gitlabhq/gitlab-test.git", @@ -178,7 +169,19 @@ func TestParseGitlabMergeEvent(t *testing.T) { Hostname: "example.com", Type: models.Gitlab, }, - }, repo) + } + + Equals(t, models.PullRequest{ + URL: "http://example.com/diaspora/merge_requests/1", + Author: "root", + Num: 1, + HeadCommit: "da1560886d4f094c3e6c9ef40349f7d38b5d27d7", + Branch: "ms-viewport", + State: models.Open, + BaseRepo: expRepo, + }, pull) + + Equals(t, expRepo, repo) t.Log("If the state is closed, should set field correctly.") event.ObjectAttributes.State = "closed" @@ -192,7 +195,18 @@ func TestParseGitlabMergeRequest(t *testing.T) { var event *gitlab.MergeRequest err := json.Unmarshal([]byte(mergeRequestJSON), &event) Ok(t, err) - pull := parser.ParseGitlabMergeRequest(event) + repo := models.Repo{ + FullName: "gitlabhq/gitlab-test", + Name: "gitlab-test", + SanitizedCloneURL: "https://example.com/gitlabhq/gitlab-test.git", + Owner: "gitlabhq", + CloneURL: "https://gitlab-user:gitlab-token@example.com/gitlabhq/gitlab-test.git", + VCSHost: models.VCSHost{ + Hostname: "example.com", + Type: models.Gitlab, + }, + } + pull := parser.ParseGitlabMergeRequest(event, repo) Equals(t, models.PullRequest{ URL: "https://gitlab.com/lkysow/atlantis-example/merge_requests/8", Author: "lkysow", @@ -200,11 +214,12 @@ func TestParseGitlabMergeRequest(t *testing.T) { HeadCommit: "0b4ac85ea3063ad5f2974d10cd68dd1f937aaac2", Branch: "abc", State: models.Open, + BaseRepo: repo, }, pull) t.Log("If the state is closed, should set field correctly.") event.State = "closed" - pull = parser.ParseGitlabMergeRequest(event) + pull = parser.ParseGitlabMergeRequest(event, repo) Equals(t, models.Closed, pull.State) } diff --git a/server/events/mocks/mock_event_parsing.go b/server/events/mocks/mock_event_parsing.go index 5e2d1746fd..e88b581dce 100644 --- a/server/events/mocks/mock_event_parsing.go +++ b/server/events/mocks/mock_event_parsing.go @@ -124,8 +124,8 @@ func (mock *MockEventParsing) ParseGitlabMergeCommentEvent(event go_gitlab.Merge return ret0, ret1, ret2, ret3 } -func (mock *MockEventParsing) ParseGitlabMergeRequest(mr *go_gitlab.MergeRequest) models.PullRequest { - params := []pegomock.Param{mr} +func (mock *MockEventParsing) ParseGitlabMergeRequest(mr *go_gitlab.MergeRequest, baseRepo models.Repo) models.PullRequest { + params := []pegomock.Param{mr, baseRepo} result := pegomock.GetGenericMockFrom(mock).Invoke("ParseGitlabMergeRequest", params, []reflect.Type{reflect.TypeOf((*models.PullRequest)(nil)).Elem()}) var ret0 models.PullRequest if len(result) != 0 { @@ -289,8 +289,8 @@ func (c *EventParsing_ParseGitlabMergeCommentEvent_OngoingVerification) GetAllCa return } -func (verifier *VerifierEventParsing) ParseGitlabMergeRequest(mr *go_gitlab.MergeRequest) *EventParsing_ParseGitlabMergeRequest_OngoingVerification { - params := []pegomock.Param{mr} +func (verifier *VerifierEventParsing) ParseGitlabMergeRequest(mr *go_gitlab.MergeRequest, baseRepo models.Repo) *EventParsing_ParseGitlabMergeRequest_OngoingVerification { + params := []pegomock.Param{mr, baseRepo} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "ParseGitlabMergeRequest", params) return &EventParsing_ParseGitlabMergeRequest_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } @@ -300,18 +300,22 @@ type EventParsing_ParseGitlabMergeRequest_OngoingVerification struct { methodInvocations []pegomock.MethodInvocation } -func (c *EventParsing_ParseGitlabMergeRequest_OngoingVerification) GetCapturedArguments() *go_gitlab.MergeRequest { - mr := c.GetAllCapturedArguments() - return mr[len(mr)-1] +func (c *EventParsing_ParseGitlabMergeRequest_OngoingVerification) GetCapturedArguments() (*go_gitlab.MergeRequest, models.Repo) { + mr, baseRepo := c.GetAllCapturedArguments() + return mr[len(mr)-1], baseRepo[len(baseRepo)-1] } -func (c *EventParsing_ParseGitlabMergeRequest_OngoingVerification) GetAllCapturedArguments() (_param0 []*go_gitlab.MergeRequest) { +func (c *EventParsing_ParseGitlabMergeRequest_OngoingVerification) GetAllCapturedArguments() (_param0 []*go_gitlab.MergeRequest, _param1 []models.Repo) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { _param0 = make([]*go_gitlab.MergeRequest, len(params[0])) for u, param := range params[0] { _param0[u] = param.(*go_gitlab.MergeRequest) } + _param1 = make([]models.Repo, len(params[1])) + for u, param := range params[1] { + _param1[u] = param.(models.Repo) + } } return } diff --git a/server/events/models/models.go b/server/events/models/models.go index 3775a40a2f..7344fd28e2 100644 --- a/server/events/models/models.go +++ b/server/events/models/models.go @@ -111,6 +111,8 @@ type PullRequest struct { // Gitlab supports an additional "merged" state but Github doesn't so we map // merged to Closed. State PullRequestState + // BaseRepo is the repository that the pull request will be merged into. + BaseRepo Repo } type PullRequestState int diff --git a/server/events/vcs/fixtures/fixtures.go b/server/events/vcs/fixtures/fixtures.go index 168ddc5b46..e3155f1992 100644 --- a/server/events/vcs/fixtures/fixtures.go +++ b/server/events/vcs/fixtures/fixtures.go @@ -22,7 +22,8 @@ var Pull = github.PullRequest{ Repo: &Repo, }, Base: &github.PullRequestBranch{ - SHA: github.String("sha256"), + SHA: github.String("sha256"), + Repo: &Repo, }, HTMLURL: github.String("html-url"), User: &github.User{ diff --git a/server/locks_controller.go b/server/locks_controller.go new file mode 100644 index 0000000000..de290662fc --- /dev/null +++ b/server/locks_controller.go @@ -0,0 +1,123 @@ +package server + +import ( + "fmt" + "net/http" + "net/url" + "strings" + + "github.com/gorilla/mux" + "github.com/runatlantis/atlantis/server/events/locking" + "github.com/runatlantis/atlantis/server/events/models" + "github.com/runatlantis/atlantis/server/events/vcs" + "github.com/runatlantis/atlantis/server/logging" +) + +// LocksController handles all requests relating to Atlantis locks. +type LocksController struct { + AtlantisVersion string + Locker locking.Locker + Logger *logging.SimpleLogger + VCSClient vcs.ClientProxy + LockDetailTemplate TemplateWriter +} + +// GetLockRoute is the GET /locks/{id} route. It renders the lock detail view. +func (l *LocksController) GetLockRoute(w http.ResponseWriter, r *http.Request) { + id, ok := mux.Vars(r)["id"] + if !ok { + l.respond(w, logging.Warn, http.StatusBadRequest, "No lock id in request") + return + } + + l.GetLock(w, id) +} + +// GetLock handles a lock detail page view. GetLockRoute is expected to +// be called before. This function was extracted to make it testable. +func (l *LocksController) GetLock(w http.ResponseWriter, id string) { + idUnencoded, err := url.QueryUnescape(id) + if err != nil { + l.respond(w, logging.Warn, http.StatusBadRequest, "Invalid lock id: %s", err) + return + } + lock, err := l.Locker.GetLock(idUnencoded) + if err != nil { + l.respond(w, logging.Error, http.StatusInternalServerError, "Failed getting lock: %s", err) + return + } + if lock == nil { + l.respond(w, logging.Info, http.StatusNotFound, "No lock found at id %q", idUnencoded) + return + } + + // Extract the repo owner and repo name. + repo := strings.Split(lock.Project.RepoFullName, "/") + viewData := LockDetailData{ + LockKeyEncoded: id, + LockKey: idUnencoded, + RepoOwner: repo[0], + RepoName: repo[1], + PullRequestLink: lock.Pull.URL, + LockedBy: lock.Pull.Author, + Workspace: lock.Workspace, + AtlantisVersion: l.AtlantisVersion, + } + l.LockDetailTemplate.Execute(w, viewData) // nolint: errcheck +} + +// DeleteLockRoute handles deleting the lock at id. +func (l *LocksController) DeleteLockRoute(w http.ResponseWriter, r *http.Request) { + id, ok := mux.Vars(r)["id"] + if !ok || id == "" { + l.respond(w, logging.Warn, http.StatusBadRequest, "No lock id in request") + return + } + l.DeleteLock(w, id) +} + +// DeleteLock deletes the lock and comments back on the pull request that the +// lock has been deleted. +// DeleteLockRoute should be called first. This method is split out to make this route testable. +func (l *LocksController) DeleteLock(w http.ResponseWriter, id string) { + idUnencoded, err := url.PathUnescape(id) + if err != nil { + l.respond(w, logging.Warn, http.StatusBadRequest, "Invalid lock id %q. Failed with error: %s", id, err) + return + } + lock, err := l.Locker.Unlock(idUnencoded) + if err != nil { + l.respond(w, logging.Error, http.StatusInternalServerError, "deleting lock failed with: %s", err) + return + } + if lock == nil { + l.respond(w, logging.Info, http.StatusNotFound, "No lock found at id %q", idUnencoded) + return + } + + // Once the lock has been deleted, comment back on the pull request. + comment := fmt.Sprintf("**Warning**: The plan for path: `%s` workspace: `%s` was **discarded** via the Atlantis UI.\n\n"+ + "To `apply` you must run `plan` again.", lock.Project.Path, lock.Workspace) + // NOTE: Because BaseRepo was added to the PullRequest model later, previous + // installations of Atlantis will have locks in their DB that do not have + // this field on PullRequest. We skip commenting in this case. + if lock.Pull.BaseRepo != (models.Repo{}) { + err = l.VCSClient.CreateComment(lock.Pull.BaseRepo, lock.Pull.Num, comment) + if err != nil { + l.respond(w, logging.Error, http.StatusInternalServerError, "Failed commenting on pull request: %s", err) + return + } + } else { + l.Logger.Debug("skipping commenting on pull request that lock was deleted because BaseRepo field is empty") + } + l.respond(w, logging.Info, http.StatusOK, "Deleted lock id %q", id) +} + +// respond is a helper function to respond and log the response. lvl is the log +// level to log at, code is the HTTP response code. +func (l *LocksController) respond(w http.ResponseWriter, lvl logging.LogLevel, responseCode int, format string, args ...interface{}) { + response := fmt.Sprintf(format, args...) + l.Logger.Log(lvl, response) + w.WriteHeader(responseCode) + fmt.Fprintln(w, response) +} diff --git a/server/locks_controller_test.go b/server/locks_controller_test.go new file mode 100644 index 0000000000..869ad2ab02 --- /dev/null +++ b/server/locks_controller_test.go @@ -0,0 +1,229 @@ +package server_test + +import ( + "bytes" + "errors" + "net/http" + "net/http/httptest" + "reflect" + "testing" + + . "github.com/petergtz/pegomock" + "github.com/runatlantis/atlantis/server" + "github.com/runatlantis/atlantis/server/events/locking/mocks" + "github.com/runatlantis/atlantis/server/events/models" + vcsmocks "github.com/runatlantis/atlantis/server/events/vcs/mocks" + "github.com/runatlantis/atlantis/server/logging" + sMocks "github.com/runatlantis/atlantis/server/mocks" +) + +func AnyRepo() models.Repo { + RegisterMatcher(NewAnyMatcher(reflect.TypeOf(models.Repo{}))) + return models.Repo{} +} + +func TestGetLockRoute_NoLockID(t *testing.T) { + t.Log("If there is no lock ID in the request then we should get a 400") + eventsReq, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) + w := httptest.NewRecorder() + lc := server.LocksController{ + Logger: logging.NewNoopLogger(), + } + lc.GetLockRoute(w, eventsReq) + responseContains(t, w, http.StatusBadRequest, "No lock id in request") +} + +func TestGetLock_InvalidLockID(t *testing.T) { + t.Log("If the lock ID is invalid then we should get a 400") + lc := server.LocksController{ + Logger: logging.NewNoopLogger(), + } + eventsReq, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) + w := httptest.NewRecorder() + lc.GetLock(w, "%A@") + responseContains(t, w, http.StatusBadRequest, "Invalid lock id") +} + +func TestGetLock_LockerErr(t *testing.T) { + t.Log("If there is an error retrieving the lock, a 500 is returned") + RegisterMockTestingT(t) + l := mocks.NewMockLocker() + When(l.GetLock("id")).ThenReturn(nil, errors.New("err")) + lc := server.LocksController{ + Logger: logging.NewNoopLogger(), + Locker: l, + } + eventsReq, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) + w := httptest.NewRecorder() + lc.GetLock(w, "id") + responseContains(t, w, http.StatusInternalServerError, "err") +} + +func TestGetLock_None(t *testing.T) { + t.Log("If there is no lock at that ID we get a 404") + RegisterMockTestingT(t) + l := mocks.NewMockLocker() + When(l.GetLock("id")).ThenReturn(nil, nil) + lc := server.LocksController{ + Logger: logging.NewNoopLogger(), + Locker: l, + } + eventsReq, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) + w := httptest.NewRecorder() + lc.GetLock(w, "id") + responseContains(t, w, http.StatusNotFound, "No lock found at id \"id\"") +} + +func TestGetLock_Success(t *testing.T) { + t.Log("Should be able to render a lock successfully") + RegisterMockTestingT(t) + l := mocks.NewMockLocker() + When(l.GetLock("id")).ThenReturn(&models.ProjectLock{ + Project: models.Project{RepoFullName: "owner/repo", Path: "path"}, + Pull: models.PullRequest{URL: "url", Author: "lkysow"}, + Workspace: "workspace", + }, nil) + tmpl := sMocks.NewMockTemplateWriter() + lc := server.LocksController{ + Logger: logging.NewNoopLogger(), + Locker: l, + LockDetailTemplate: tmpl, + AtlantisVersion: "1300135", + } + eventsReq, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) + w := httptest.NewRecorder() + lc.GetLock(w, "id") + t.Log(w.Code) + tmpl.VerifyWasCalledOnce().Execute(w, server.LockDetailData{ + LockKeyEncoded: "id", + LockKey: "id", + RepoOwner: "owner", + RepoName: "repo", + PullRequestLink: "url", + LockedBy: "lkysow", + Workspace: "workspace", + AtlantisVersion: "1300135", + }) + responseContains(t, w, http.StatusOK, "") +} + +func TestDeleteLockRoute_NoLockID(t *testing.T) { + t.Log("If there is no lock ID in the request then we should get a 400") + eventsReq, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) + w := httptest.NewRecorder() + lc := server.LocksController{Logger: logging.NewNoopLogger()} + lc.DeleteLockRoute(w, eventsReq) + responseContains(t, w, http.StatusBadRequest, "No lock id in request") +} + +func TestDeleteLock_InvalidLockID(t *testing.T) { + t.Log("If the lock ID is invalid then we should get a 400") + lc := server.LocksController{Logger: logging.NewNoopLogger()} + eventsReq, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) + w := httptest.NewRecorder() + lc.DeleteLock(w, "%A@") + responseContains(t, w, http.StatusBadRequest, "Invalid lock id \"%A@\"") +} + +func TestDeleteLock_LockerErr(t *testing.T) { + t.Log("If there is an error retrieving the lock, a 500 is returned") + RegisterMockTestingT(t) + l := mocks.NewMockLocker() + When(l.Unlock("id")).ThenReturn(nil, errors.New("err")) + lc := server.LocksController{ + Locker: l, + Logger: logging.NewNoopLogger(), + } + eventsReq, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) + w := httptest.NewRecorder() + lc.DeleteLock(w, "id") + responseContains(t, w, http.StatusInternalServerError, "err") +} + +func TestDeleteLock_None(t *testing.T) { + t.Log("If there is no lock at that ID we get a 404") + RegisterMockTestingT(t) + l := mocks.NewMockLocker() + When(l.Unlock("id")).ThenReturn(nil, nil) + lc := server.LocksController{ + Locker: l, + Logger: logging.NewNoopLogger(), + } + eventsReq, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) + w := httptest.NewRecorder() + lc.DeleteLock(w, "id") + responseContains(t, w, http.StatusNotFound, "No lock found at id \"id\"") +} + +func TestDeleteLock_OldFormat(t *testing.T) { + t.Log("If the lock doesn't have BaseRepo set it is deleted successfully") + RegisterMockTestingT(t) + + cp := vcsmocks.NewMockClientProxy() + l := mocks.NewMockLocker() + When(l.Unlock("id")).ThenReturn(&models.ProjectLock{}, nil) + lc := server.LocksController{ + Locker: l, + Logger: logging.NewNoopLogger(), + VCSClient: cp, + } + eventsReq, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) + w := httptest.NewRecorder() + lc.DeleteLock(w, "id") + responseContains(t, w, http.StatusOK, "Deleted lock id \"id\"") + cp.VerifyWasCalled(Never()).CreateComment(AnyRepo(), AnyInt(), AnyString()) +} + +func TestDeleteLock_CommentFailed(t *testing.T) { + t.Log("If the commenting fails we return an error") + RegisterMockTestingT(t) + + cp := vcsmocks.NewMockClientProxy() + When(cp.CreateComment(AnyRepo(), AnyInt(), AnyString())).ThenReturn(errors.New("err")) + l := mocks.NewMockLocker() + When(l.Unlock("id")).ThenReturn(&models.ProjectLock{ + Pull: models.PullRequest{ + BaseRepo: models.Repo{FullName: "owner/repo"}, + }, + }, nil) + lc := server.LocksController{ + Locker: l, + Logger: logging.NewNoopLogger(), + VCSClient: cp, + } + eventsReq, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) + w := httptest.NewRecorder() + lc.DeleteLock(w, "id") + responseContains(t, w, http.StatusInternalServerError, "Failed commenting on pull request: err") +} + +func TestDeleteLock_CommentSuccess(t *testing.T) { + t.Log("We should comment back on the pull request if the lock is deleted") + RegisterMockTestingT(t) + + cp := vcsmocks.NewMockClientProxy() + l := mocks.NewMockLocker() + pull := models.PullRequest{ + BaseRepo: models.Repo{FullName: "owner/repo"}, + } + When(l.Unlock("id")).ThenReturn(&models.ProjectLock{ + Pull: pull, + Workspace: "workspace", + Project: models.Project{ + Path: "path", + RepoFullName: "owner/repo", + }, + }, nil) + lc := server.LocksController{ + Locker: l, + Logger: logging.NewNoopLogger(), + VCSClient: cp, + } + eventsReq, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) + w := httptest.NewRecorder() + lc.DeleteLock(w, "id") + responseContains(t, w, http.StatusOK, "Deleted lock id \"id\"") + cp.VerifyWasCalled(Once()).CreateComment(pull.BaseRepo, pull.Num, + "**Warning**: The plan for path: `path` workspace: `workspace` was **discarded** via the Atlantis UI.\n\n"+ + "To `apply` you must run `plan` again.") +} diff --git a/server/server.go b/server/server.go index 5be4122f68..06c263b275 100644 --- a/server/server.go +++ b/server/server.go @@ -58,6 +58,7 @@ type Server struct { Locker locking.Locker AtlantisURL string EventsController *EventsController + LocksController *LocksController IndexTemplate TemplateWriter LockDetailTemplate TemplateWriter SSLCertFile string @@ -243,6 +244,13 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { repoWhitelist := &events.RepoWhitelist{ Whitelist: userConfig.RepoWhitelist, } + locksController := &LocksController{ + AtlantisVersion: config.AtlantisVersion, + Locker: lockingClient, + Logger: logger, + VCSClient: vcsClient, + LockDetailTemplate: lockTemplate, + } eventsController := &EventsController{ CommandRunner: commandHandler, PullCleaner: pullClosedExecutor, @@ -267,6 +275,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { Locker: lockingClient, AtlantisURL: userConfig.AtlantisURL, EventsController: eventsController, + LocksController: locksController, IndexTemplate: indexTemplate, LockDetailTemplate: lockTemplate, SSLKeyFile: userConfig.SSLKeyFile, @@ -281,8 +290,8 @@ func (s *Server) Start() error { }) s.Router.PathPrefix("/static/").Handler(http.FileServer(&assetfs.AssetFS{Asset: static.Asset, AssetDir: static.AssetDir, AssetInfo: static.AssetInfo})) s.Router.HandleFunc("/events", s.postEvents).Methods("POST") - s.Router.HandleFunc("/locks", s.DeleteLockRoute).Methods("DELETE").Queries("id", "{id:.*}") - lockRoute := s.Router.HandleFunc("/lock", s.GetLockRoute).Methods("GET").Queries("id", "{id}").Name(LockRouteName) + s.Router.HandleFunc("/locks", s.LocksController.DeleteLockRoute).Methods("DELETE").Queries("id", "{id:.*}") + lockRoute := s.Router.HandleFunc("/lock", s.LocksController.GetLockRoute).Methods("GET").Queries("id", "{id}").Name(LockRouteName) // function that planExecutor can use to construct detail view url // injecting this here because this is the earliest routes are created s.CommandHandler.SetLockURL(func(lockID string) string { @@ -355,97 +364,8 @@ func (s *Server) Index(w http.ResponseWriter, _ *http.Request) { }) } -// GetLockRoute is the GET /locks/{id} route. It renders the lock detail view. -func (s *Server) GetLockRoute(w http.ResponseWriter, r *http.Request) { - id, ok := mux.Vars(r)["id"] - if !ok { - w.WriteHeader(http.StatusBadRequest) - fmt.Fprint(w, "No lock id in request") - return - } - s.GetLock(w, r, id) -} - -// GetLock handles a lock detail page view. getLockRoute is expected to -// be called before. This function was extracted to make it testable. -func (s *Server) GetLock(w http.ResponseWriter, _ *http.Request, id string) { - idUnencoded, err := url.QueryUnescape(id) - if err != nil { - w.WriteHeader(http.StatusBadRequest) - fmt.Fprint(w, "Invalid lock id") - return - } - - lock, err := s.Locker.GetLock(idUnencoded) - if err != nil { - w.WriteHeader(http.StatusInternalServerError) - fmt.Fprint(w, err.Error()) - return - } - if lock == nil { - w.WriteHeader(http.StatusNotFound) - fmt.Fprint(w, "No lock found at that id") - return - } - - // Extract the repo owner and repo name. - repo := strings.Split(lock.Project.RepoFullName, "/") - - l := LockDetailData{ - LockKeyEncoded: id, - LockKey: idUnencoded, - RepoOwner: repo[0], - RepoName: repo[1], - PullRequestLink: lock.Pull.URL, - LockedBy: lock.Pull.Author, - Workspace: lock.Workspace, - AtlantisVersion: s.AtlantisVersion, - } - - s.LockDetailTemplate.Execute(w, l) // nolint: errcheck -} - -// DeleteLockRoute handles deleting the lock at id. -func (s *Server) DeleteLockRoute(w http.ResponseWriter, r *http.Request) { - id, ok := mux.Vars(r)["id"] - if !ok || id == "" { - s.respond(w, logging.Warn, http.StatusBadRequest, "No lock id in request") - return - } - s.DeleteLock(w, r, id) -} - -// DeleteLock deletes the lock. DeleteLockRoute should be called first. -// This method is split out to make this route testable. -func (s *Server) DeleteLock(w http.ResponseWriter, _ *http.Request, id string) { - idUnencoded, err := url.PathUnescape(id) - if err != nil { - s.respond(w, logging.Warn, http.StatusBadRequest, "Invalid lock id: %s", err) - return - } - lock, err := s.Locker.Unlock(idUnencoded) - if err != nil { - s.respond(w, logging.Error, http.StatusInternalServerError, "Failed to delete lock %s: %s", idUnencoded, err) - return - } - if lock == nil { - s.respond(w, logging.Warn, http.StatusNotFound, "No lock found at that id", idUnencoded) - return - } - s.respond(w, logging.Info, http.StatusOK, "Deleted lock id %s", idUnencoded) -} - // postEvents handles POST requests to our /events endpoint. These should be // VCS webhook requests. func (s *Server) postEvents(w http.ResponseWriter, r *http.Request) { s.EventsController.Post(w, r) } - -// respond is a helper function to respond and log the response. lvl is the log -// level to log at, code is the HTTP response code. -func (s *Server) respond(w http.ResponseWriter, lvl logging.LogLevel, code int, format string, args ...interface{}) { - response := fmt.Sprintf(format, args...) - s.Logger.Log(lvl, response) - w.WriteHeader(code) - fmt.Fprintln(w, response) -} diff --git a/server/server_test.go b/server/server_test.go index 8c6510f978..21134d9064 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -28,7 +28,6 @@ import ( "github.com/runatlantis/atlantis/server" "github.com/runatlantis/atlantis/server/events/locking/mocks" "github.com/runatlantis/atlantis/server/events/models" - "github.com/runatlantis/atlantis/server/logging" sMocks "github.com/runatlantis/atlantis/server/mocks" . "github.com/runatlantis/atlantis/testing" ) @@ -103,144 +102,6 @@ func TestIndex_Success(t *testing.T) { responseContains(t, w, http.StatusOK, "") } -func TestGetLockRoute_NoLockID(t *testing.T) { - t.Log("If there is no lock ID in the request then we should get a 400") - eventsReq, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) - w := httptest.NewRecorder() - s := server.Server{} - s.GetLockRoute(w, eventsReq) - responseContains(t, w, http.StatusBadRequest, "No lock id in request") -} - -func TestGetLock_InvalidLockID(t *testing.T) { - t.Log("If the lock ID is invalid then we should get a 400") - s := server.Server{} - eventsReq, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) - w := httptest.NewRecorder() - s.GetLock(w, eventsReq, "%A@") - responseContains(t, w, http.StatusBadRequest, "Invalid lock id") -} - -func TestGetLock_LockerErr(t *testing.T) { - t.Log("If there is an error retrieving the lock, a 500 is returned") - RegisterMockTestingT(t) - l := mocks.NewMockLocker() - When(l.GetLock("id")).ThenReturn(nil, errors.New("err")) - s := server.Server{ - Locker: l, - } - eventsReq, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) - w := httptest.NewRecorder() - s.GetLock(w, eventsReq, "id") - responseContains(t, w, http.StatusInternalServerError, "err") -} - -func TestGetLock_None(t *testing.T) { - t.Log("If there is no lock at that ID we get a 404") - RegisterMockTestingT(t) - l := mocks.NewMockLocker() - When(l.GetLock("id")).ThenReturn(nil, nil) - s := server.Server{ - Locker: l, - } - eventsReq, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) - w := httptest.NewRecorder() - s.GetLock(w, eventsReq, "id") - responseContains(t, w, http.StatusNotFound, "No lock found at that id") -} - -func TestGetLock_Success(t *testing.T) { - t.Log("Should be able to render a lock successfully") - RegisterMockTestingT(t) - l := mocks.NewMockLocker() - When(l.GetLock("id")).ThenReturn(&models.ProjectLock{ - Project: models.Project{RepoFullName: "owner/repo", Path: "path"}, - Pull: models.PullRequest{URL: "url", Author: "lkysow"}, - Workspace: "workspace", - }, nil) - tmpl := sMocks.NewMockTemplateWriter() - s := server.Server{ - Locker: l, - LockDetailTemplate: tmpl, - } - eventsReq, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) - w := httptest.NewRecorder() - s.GetLock(w, eventsReq, "id") - tmpl.VerifyWasCalledOnce().Execute(w, server.LockDetailData{ - LockKeyEncoded: "id", - LockKey: "id", - RepoOwner: "owner", - RepoName: "repo", - PullRequestLink: "url", - LockedBy: "lkysow", - Workspace: "workspace", - }) - responseContains(t, w, http.StatusOK, "") -} - -func TestDeleteLockRoute_NoLockID(t *testing.T) { - t.Log("If there is no lock ID in the request then we should get a 400") - eventsReq, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) - w := httptest.NewRecorder() - s := server.Server{Logger: logging.NewNoopLogger()} - s.DeleteLockRoute(w, eventsReq) - responseContains(t, w, http.StatusBadRequest, "No lock id in request") -} - -func TestDeleteLock_InvalidLockID(t *testing.T) { - t.Log("If the lock ID is invalid then we should get a 400") - s := server.Server{Logger: logging.NewNoopLogger()} - eventsReq, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) - w := httptest.NewRecorder() - s.DeleteLock(w, eventsReq, "%A@") - responseContains(t, w, http.StatusBadRequest, "Invalid lock id") -} - -func TestDeleteLock_LockerErr(t *testing.T) { - t.Log("If there is an error retrieving the lock, a 500 is returned") - RegisterMockTestingT(t) - l := mocks.NewMockLocker() - When(l.Unlock("id")).ThenReturn(nil, errors.New("err")) - s := server.Server{ - Locker: l, - Logger: logging.NewNoopLogger(), - } - eventsReq, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) - w := httptest.NewRecorder() - s.DeleteLock(w, eventsReq, "id") - responseContains(t, w, http.StatusInternalServerError, "err") -} - -func TestDeleteLock_None(t *testing.T) { - t.Log("If there is no lock at that ID we get a 404") - RegisterMockTestingT(t) - l := mocks.NewMockLocker() - When(l.Unlock("id")).ThenReturn(nil, nil) - s := server.Server{ - Locker: l, - Logger: logging.NewNoopLogger(), - } - eventsReq, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) - w := httptest.NewRecorder() - s.DeleteLock(w, eventsReq, "id") - responseContains(t, w, http.StatusNotFound, "No lock found at that id") -} - -func TestDeleteLock_Success(t *testing.T) { - t.Log("If the lock is deleted successfully we get a 200") - RegisterMockTestingT(t) - l := mocks.NewMockLocker() - When(l.Unlock("id")).ThenReturn(&models.ProjectLock{}, nil) - s := server.Server{ - Locker: l, - Logger: logging.NewNoopLogger(), - } - eventsReq, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) - w := httptest.NewRecorder() - s.DeleteLock(w, eventsReq, "id") - responseContains(t, w, http.StatusOK, "Deleted lock id id") -} - func responseContains(t *testing.T, r *httptest.ResponseRecorder, status int, bodySubstr string) { Equals(t, status, r.Result().StatusCode) body, _ := ioutil.ReadAll(r.Result().Body)