diff --git a/server/events/command_handler.go b/server/events/command_handler.go index b34f4b8e85..8658cc9a1d 100644 --- a/server/events/command_handler.go +++ b/server/events/command_handler.go @@ -42,7 +42,6 @@ type GitlabMergeRequestGetter interface { type CommandHandler struct { PlanExecutor Executor ApplyExecutor Executor - HelpExecutor Executor LockURLGenerator LockURLGenerator VCSClient vcs.ClientProxy GithubPullGetter GithubPullGetter @@ -125,7 +124,7 @@ func (c *CommandHandler) run(ctx *CommandContext) { if ctx.Pull.State != models.Open { ctx.Log.Info("command was run on closed pull request") - c.VCSClient.CreateComment(ctx.BaseRepo, ctx.Pull, "Atlantis commands can't be run on closed pull requests", ctx.VCSHost) // nolint: errcheck + c.VCSClient.CreateComment(ctx.BaseRepo, ctx.Pull.Num, "Atlantis commands can't be run on closed pull requests", ctx.VCSHost) // nolint: errcheck return } @@ -148,8 +147,6 @@ func (c *CommandHandler) run(ctx *CommandContext) { cr = c.PlanExecutor.Execute(ctx) case Apply: cr = c.ApplyExecutor.Execute(ctx) - case Help: - cr = c.HelpExecutor.Execute(ctx) default: ctx.Log.Err("failed to determine desired command, neither plan nor apply") } @@ -167,14 +164,14 @@ func (c *CommandHandler) updatePull(ctx *CommandContext, res CommandResponse) { // Update the pull request's status icon and comment back. c.CommitStatusUpdater.UpdateProjectResult(ctx, res) // nolint: errcheck comment := c.MarkdownRenderer.Render(res, ctx.Command.Name, ctx.Log.History.String(), ctx.Command.Verbose) - c.VCSClient.CreateComment(ctx.BaseRepo, ctx.Pull, comment, ctx.VCSHost) // nolint: errcheck + c.VCSClient.CreateComment(ctx.BaseRepo, ctx.Pull.Num, comment, ctx.VCSHost) // nolint: errcheck } // logPanics logs and creates a comment on the pull request for panics. func (c *CommandHandler) logPanics(ctx *CommandContext) { if err := recover(); err != nil { stack := recovery.Stack(3) - c.VCSClient.CreateComment(ctx.BaseRepo, ctx.Pull, // nolint: errcheck + c.VCSClient.CreateComment(ctx.BaseRepo, ctx.Pull.Num, // nolint: errcheck fmt.Sprintf("**Error: goroutine panic. This is a bug.**\n```\n%s\n%s```", err, stack), ctx.VCSHost) ctx.Log.Err("PANIC: %s\n%s", err, stack) } diff --git a/server/events/command_handler_test.go b/server/events/command_handler_test.go index 7bdd416d11..a583c28021 100644 --- a/server/events/command_handler_test.go +++ b/server/events/command_handler_test.go @@ -21,7 +21,6 @@ import ( ) var applier *mocks.MockExecutor -var helper *mocks.MockExecutor var planner *mocks.MockExecutor var eventParsing *mocks.MockEventParsing var vcsClient *vcsmocks.MockClientProxy @@ -35,7 +34,6 @@ var logBytes *bytes.Buffer func setup(t *testing.T) { RegisterMockTestingT(t) applier = mocks.NewMockExecutor() - helper = mocks.NewMockExecutor() planner = mocks.NewMockExecutor() eventParsing = mocks.NewMockEventParsing() ghStatus = mocks.NewMockCommitStatusUpdater() @@ -49,7 +47,6 @@ func setup(t *testing.T) { ch = events.CommandHandler{ PlanExecutor: planner, ApplyExecutor: applier, - HelpExecutor: helper, VCSClient: vcsClient, CommitStatusUpdater: ghStatus, EventParser: eventParsing, @@ -66,7 +63,7 @@ func TestExecuteCommand_LogPanics(t *testing.T) { setup(t) When(ghStatus.Update(fixtures.Repo, fixtures.Pull, vcs.Pending, nil, vcs.Github)).ThenPanic("panic") ch.ExecuteCommand(fixtures.Repo, fixtures.Repo, fixtures.User, 1, nil, vcs.Github) - _, _, comment, _ := vcsClient.VerifyWasCalledOnce().CreateComment(matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest(), AnyString(), matchers.AnyVcsHost()).GetCapturedArguments() + _, _, comment, _ := vcsClient.VerifyWasCalledOnce().CreateComment(matchers.AnyModelsRepo(), AnyInt(), AnyString(), matchers.AnyVcsHost()).GetCapturedArguments() Assert(t, strings.Contains(comment, "Error: goroutine panic"), "comment should be about a goroutine panic") } @@ -125,7 +122,7 @@ func TestExecuteCommand_ClosedPull(t *testing.T) { When(eventParsing.ParseGithubPull(pull)).ThenReturn(modelPull, fixtures.Repo, nil) ch.ExecuteCommand(fixtures.Repo, fixtures.Repo, fixtures.User, fixtures.Pull.Num, nil, vcs.Github) - vcsClient.VerifyWasCalledOnce().CreateComment(fixtures.Repo, modelPull, "Atlantis commands can't be run on closed pull requests", vcs.Github) + vcsClient.VerifyWasCalledOnce().CreateComment(fixtures.Repo, modelPull.Num, "Atlantis commands can't be run on closed pull requests", vcs.Github) } func TestExecuteCommand_WorkspaceLocked(t *testing.T) { @@ -150,17 +147,17 @@ func TestExecuteCommand_WorkspaceLocked(t *testing.T) { ghStatus.VerifyWasCalledOnce().Update(fixtures.Repo, fixtures.Pull, vcs.Pending, &cmd, vcs.Github) _, response := ghStatus.VerifyWasCalledOnce().UpdateProjectResult(matchers.AnyPtrToEventsCommandContext(), matchers.AnyEventsCommandResponse()).GetCapturedArguments() Equals(t, msg, response.Failure) - vcsClient.VerifyWasCalledOnce().CreateComment(fixtures.Repo, fixtures.Pull, + vcsClient.VerifyWasCalledOnce().CreateComment(fixtures.Repo, fixtures.Pull.Num, "**Plan Failed**: "+msg+"\n\n", vcs.Github) } func TestExecuteCommand_FullRun(t *testing.T) { - t.Log("when running a plan, apply or help should comment") + t.Log("when running a plan, apply should comment") pull := &github.PullRequest{ State: github.String("closed"), } cmdResponse := events.CommandResponse{} - for _, c := range []events.CommandName{events.Help, events.Plan, events.Apply} { + for _, c := range []events.CommandName{events.Plan, events.Apply} { setup(t) cmd := events.Command{ Name: c, @@ -170,8 +167,6 @@ func TestExecuteCommand_FullRun(t *testing.T) { When(eventParsing.ParseGithubPull(pull)).ThenReturn(fixtures.Pull, fixtures.Repo, nil) When(workspaceLocker.TryLock(fixtures.Repo.FullName, cmd.Workspace, fixtures.Pull.Num)).ThenReturn(true) switch c { - case events.Help: - When(helper.Execute(matchers.AnyPtrToEventsCommandContext())).ThenReturn(cmdResponse) case events.Plan: When(planner.Execute(matchers.AnyPtrToEventsCommandContext())).ThenReturn(cmdResponse) case events.Apply: @@ -183,7 +178,7 @@ func TestExecuteCommand_FullRun(t *testing.T) { ghStatus.VerifyWasCalledOnce().Update(fixtures.Repo, fixtures.Pull, vcs.Pending, &cmd, vcs.Github) _, response := ghStatus.VerifyWasCalledOnce().UpdateProjectResult(matchers.AnyPtrToEventsCommandContext(), matchers.AnyEventsCommandResponse()).GetCapturedArguments() Equals(t, cmdResponse, response) - vcsClient.VerifyWasCalledOnce().CreateComment(matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest(), AnyString(), matchers.AnyVcsHost()) + vcsClient.VerifyWasCalledOnce().CreateComment(matchers.AnyModelsRepo(), AnyInt(), AnyString(), matchers.AnyVcsHost()) workspaceLocker.VerifyWasCalledOnce().Unlock(fixtures.Repo.FullName, cmd.Workspace, fixtures.Pull.Num) } } diff --git a/server/events/command_name.go b/server/events/command_name.go index 63d75a6ceb..529811f6c3 100644 --- a/server/events/command_name.go +++ b/server/events/command_name.go @@ -6,7 +6,6 @@ type CommandName int const ( Apply CommandName = iota Plan - Help // Adding more? Don't forget to update String() below ) @@ -17,8 +16,6 @@ func (c CommandName) String() string { return "apply" case Plan: return "plan" - case Help: - return "help" } return "" } diff --git a/server/events/comment_parser.go b/server/events/comment_parser.go new file mode 100644 index 0000000000..31991e3569 --- /dev/null +++ b/server/events/comment_parser.go @@ -0,0 +1,221 @@ +package events + +import ( + "fmt" + "io/ioutil" + "path/filepath" + "strings" + + "github.com/runatlantis/atlantis/server/events/vcs" + "github.com/spf13/pflag" +) + +//go:generate pegomock generate -m --use-experimental-model-gen --package mocks -o mocks/mock_comment_parsing.go CommentParsing + +// CommentParsing handles parsing pull request comments. +type CommentParsing interface { + // Parse attempts to parse a pull request comment to see if it's an Atlantis + // commmand. + Parse(comment string, vcsHost vcs.Host) CommentParseResult +} + +// CommentParser implements CommentParsing +type CommentParser struct { + GithubUser string + GithubToken string + GitlabUser string + GitlabToken string +} + +// CommentParseResult describes the result of parsing a comment as a command. +type CommentParseResult struct { + // Command is the successfully parsed command. Will be nil if + // CommentResponse or Ignore is set. + Command *Command + // CommentResponse is set when we should respond immediately to the command + // for example for atlantis help. + CommentResponse string + // Ignore is set to true when we should just ignore this comment. + Ignore bool +} + +// Parse parses the comment as an Atlantis command. +// +// Valid commands contain: +// - The initial "executable" name, 'run' or 'atlantis' or '@GithubUser' +// where GithubUser is the API user Atlantis is running as. +// - Then a command, either 'plan', 'apply', or 'help'. +// - Then optional flags, then an optional separator '--' followed by optional +// extra flags to be appended to the terraform plan/apply command. +// +// Examples: +// - atlantis help +// - run plan +// - @GithubUser plan -w staging +// - atlantis plan -w staging -d dir --verbose +// - atlantis plan --verbose -- -key=value -key2 value2 +// +// nolint: gocyclo +func (e *CommentParser) Parse(comment string, vcsHost vcs.Host) CommentParseResult { + if multiLineRegex.MatchString(comment) { + return CommentParseResult{Ignore: true} + } + + // strings.Fields strips out newlines but that's okay since we've removed + // multiline strings above. + args := strings.Fields(comment) + if len(args) < 1 { + return CommentParseResult{Ignore: true} + } + + // Helpfully warn the user if they're using "terraform" instead of "atlantis" + if args[0] == "terraform" { + return CommentParseResult{CommentResponse: DidYouMeanAtlantisComment} + } + + // Atlantis can be invoked using the name of the VCS host user we're + // running under. Need to be able to match against that user. + vcsUser := e.GithubUser + if vcsHost == vcs.Gitlab { + vcsUser = e.GitlabUser + } + executableNames := []string{"run", "atlantis", "@" + vcsUser} + + // If the comment doesn't start with the name of our 'executable' then + // ignore it. + if !e.stringInSlice(args[0], executableNames) { + return CommentParseResult{Ignore: true} + } + + // If they've just typed the name of the executable then give them the help + // output. + if len(args) == 1 { + return CommentParseResult{CommentResponse: HelpComment} + } + command := args[1] + + // Help output. + if e.stringInSlice(command, []string{"help", "-h", "--help"}) { + return CommentParseResult{CommentResponse: HelpComment} + } + + // Need to have a plan or apply at this point. + if !e.stringInSlice(command, []string{"plan", "apply"}) { + return CommentParseResult{CommentResponse: fmt.Sprintf("```\nError: unknown command %q.\nRun 'atlantis --help' for usage.\n```", command)} + } + + var workspace string + var dir string + var verbose bool + var extraArgs []string + var flagSet *pflag.FlagSet + var name CommandName + + // Set up the flag parsing depending on the command. + const defaultWorkspace = "default" + switch command { + case "plan": + name = Plan + flagSet = pflag.NewFlagSet("plan", pflag.ContinueOnError) + flagSet.SetOutput(ioutil.Discard) + flagSet.StringVarP(&workspace, "workspace", "w", defaultWorkspace, "Switch to this Terraform workspace before planning.") + flagSet.StringVarP(&dir, "dir", "d", "", "Which directory to run plan in relative to root of repo. Use '.' for root. If not specified, will attempt to run plan for all Terraform projects we think were modified in this changeset.") + flagSet.BoolVarP(&verbose, "verbose", "", false, "Append Atlantis log to comment.") + case "apply": + name = Apply + flagSet = pflag.NewFlagSet("apply", pflag.ContinueOnError) + flagSet.SetOutput(ioutil.Discard) + flagSet.StringVarP(&workspace, "workspace", "w", defaultWorkspace, "Apply the plan for this Terraform workspace.") + flagSet.StringVarP(&dir, "dir", "d", "", "Apply the plan for this directory, relative to root of repo. Use '.' for root. If not specified, will run apply against all plans created for this workspace.") + flagSet.BoolVarP(&verbose, "verbose", "", false, "Append Atlantis log to comment.") + default: + return CommentParseResult{CommentResponse: fmt.Sprintf("Error: unknown command %q – this is a bug", command)} + } + + // Now parse the flags. + // It's safe to use [2:] because we know there's at least 2 elements in args. + err := flagSet.Parse(args[2:]) + if err == pflag.ErrHelp { + return CommentParseResult{CommentResponse: fmt.Sprintf("```\nUsage of %s:\n%s\n```", command, flagSet.FlagUsagesWrapped(usagesCols))} + } + if err != nil { + return CommentParseResult{CommentResponse: fmt.Sprintf("```\nError: %s.\nUsage of %s:\n%s\n```", err.Error(), command, flagSet.FlagUsagesWrapped(usagesCols))} + } + + // We only use the extra args after the --. For example given a comment: + // "atlantis plan -bad-option -- -target=hi" + // we only append "-target=hi" to the eventual command. + // todo: keep track of the args we're discarding and include that with + // comment as a warning. + if flagSet.ArgsLenAtDash() != -1 { + extraArgsUnsafe := flagSet.Args()[flagSet.ArgsLenAtDash():] + // Quote all extra args so there isn't a security issue when we append + // them to the terraform commands, ex. "; cat /etc/passwd" + for _, arg := range extraArgsUnsafe { + quotesEscaped := strings.Replace(arg, `"`, `\"`, -1) + extraArgs = append(extraArgs, fmt.Sprintf(`"%s"`, quotesEscaped)) + } + } + + dir, err = e.validateDir(dir) + if err != nil { + return CommentParseResult{CommentResponse: err.Error()} + } + + // Because we use the workspace name as a file, need to make sure it's + // not doing something weird like being a relative dir. + if strings.Contains(workspace, "..") { + return CommentParseResult{CommentResponse: "Error: Value for -w/--workspace can't contain '..'"} + } + + return CommentParseResult{ + Command: &Command{Name: name, Verbose: verbose, Workspace: workspace, Dir: dir, Flags: extraArgs}, + } +} + +func (e *CommentParser) validateDir(dir string) (string, error) { + if dir == "" { + return dir, nil + } + validatedDir := filepath.Clean(dir) + // Join with . so the path is relative. This helps us if they use '/', + // and is safe to do if their path is relative since it's a no-op. + validatedDir = filepath.Join(".", validatedDir) + // Need to clean again to resolve relative validatedDirs. + validatedDir = filepath.Clean(validatedDir) + // Detect relative dirs since they're not allowed. + if strings.HasPrefix(validatedDir, "..") { + return "", fmt.Errorf("Error: Using a relative path %q with -d/--dir is not allowed", dir) + } + + return validatedDir, nil +} + +func (e *CommentParser) stringInSlice(a string, list []string) bool { + for _, b := range list { + if b == a { + return true + } + } + return false +} + +var HelpComment = "```cmake\n" + + `atlantis +Terraform automation and collaboration for your team + +Usage: + atlantis [options] + +Commands: + plan Runs 'terraform plan' for the changes in this pull request. + apply Runs 'terraform apply' on the plans generated by 'atlantis plan'. + help View help. + +Flags: + -h, --help help for atlantis + +Use "atlantis [command] --help" for more information about a command. +` + +var DidYouMeanAtlantisComment = "Did you mean to use `atlantis` instead of `terraform`?" diff --git a/server/events/comment_parser_test.go b/server/events/comment_parser_test.go new file mode 100644 index 0000000000..8fbd69e2ef --- /dev/null +++ b/server/events/comment_parser_test.go @@ -0,0 +1,364 @@ +package events_test + +import ( + "fmt" + "strings" + "testing" + + "github.com/runatlantis/atlantis/server/events" + "github.com/runatlantis/atlantis/server/events/vcs" + . "github.com/runatlantis/atlantis/testing" +) + +var commentParser = events.CommentParser{ + GithubUser: "github-user", + GithubToken: "github-token", + GitlabUser: "gitlab-user", + GitlabToken: "gitlab-token", +} + +func TestParse_Ignored(t *testing.T) { + t.Log("given a comment that should be ignored we should set " + + "CommentParseResult.Ignore to true") + ignoreComments := []string{ + "", + "a", + "abc", + "atlantis plan\nbut with newlines", + "terraform plan\nbut with newlines", + } + for _, c := range ignoreComments { + r := commentParser.Parse(c, vcs.Github) + Assert(t, r.Ignore, "expected Ignore to be true for comment %q", c) + } +} + +func TestParse_HelpResponse(t *testing.T) { + t.Log("given a comment that should result in help output we " + + "should set CommentParseResult.CommentResult") + helpComments := []string{ + "run", + "atlantis", + "@github-user", + "atlantis help", + "atlantis --help", + "atlantis -h", + "atlantis help something else", + "atlantis help plan", + } + for _, c := range helpComments { + r := commentParser.Parse(c, vcs.Github) + Equals(t, events.HelpComment, r.CommentResponse) + } +} + +func TestParse_DidYouMeanAtlantis(t *testing.T) { + t.Log("given a comment that should result in a 'did you mean atlantis'" + + "response, should set CommentParseResult.CommentResult") + comments := []string{ + "terraform", + "terraform help", + "terraform --help", + "terraform -h", + "terraform plan", + "terraform apply", + "terraform plan -w workspace -d . -- test", + } + for _, c := range comments { + r := commentParser.Parse(c, vcs.Github) + Assert(t, r.CommentResponse == events.DidYouMeanAtlantisComment, + "For comment %q expected CommentResponse==%q but got %q", c, events.DidYouMeanAtlantisComment, r.CommentResponse) + } +} + +func TestParse_InvalidCommand(t *testing.T) { + t.Log("given a comment with an invalid atlantis command, should return " + + "a warning.") + comments := []string{ + "atlantis paln", + "atlantis Plan", + "atlantis appely apply", + } + for _, c := range comments { + r := commentParser.Parse(c, vcs.Github) + exp := fmt.Sprintf("```\nError: unknown command %q.\nRun 'atlantis --help' for usage.\n```", strings.Fields(c)[1]) + Assert(t, r.CommentResponse == exp, + "For comment %q expected CommentResponse==%q but got %q", c, exp, r.CommentResponse) + } +} + +func TestParse_SubcommandUsage(t *testing.T) { + t.Log("given a comment asking for the usage of a subcommand should " + + "return help") + comments := []string{ + "atlantis plan -h", + "atlantis plan --help", + "atlantis apply -h", + "atlantis apply --help", + } + for _, c := range comments { + r := commentParser.Parse(c, vcs.Github) + exp := "Usage of " + strings.Fields(c)[1] + Assert(t, strings.Contains(r.CommentResponse, exp), + "For comment %q expected CommentResponse %q to contain %q", c, r.CommentResponse, exp) + Assert(t, !strings.Contains(r.CommentResponse, "Error:"), + "For comment %q expected CommentResponse %q to not contain %q", c, r.CommentResponse, "Error: ") + } +} + +func TestParse_InvalidFlags(t *testing.T) { + t.Log("given a comment with a valid atlantis command but invalid" + + " flags, should return a warning and the proper usage") + cases := []struct { + comment string + exp string + }{ + { + "atlantis plan -e", + "Error: unknown shorthand flag: 'e' in -e", + }, + { + "atlantis plan --abc", + "Error: unknown flag: --abc", + }, + { + "atlantis apply -e", + "Error: unknown shorthand flag: 'e' in -e", + }, + { + "atlantis apply --abc", + "Error: unknown flag: --abc", + }, + } + for _, c := range cases { + r := commentParser.Parse(c.comment, vcs.Github) + Assert(t, strings.Contains(r.CommentResponse, c.exp), + "For comment %q expected CommentResponse %q to contain %q", c.comment, r.CommentResponse, c.exp) + Assert(t, strings.Contains(r.CommentResponse, "Usage of "), + "For comment %q expected CommentResponse %q to contain %q", c.comment, r.CommentResponse, "Usage of ") + } +} + +func TestParse_RelativeDirPath(t *testing.T) { + t.Log("if -d is used with a relative path, should return an error") + comments := []string{ + "atlantis plan -d ..", + "atlantis apply -d ..", + // These won't return an error because we prepend with . when parsing. + //"atlantis plan -d /..", + //"atlantis apply -d /..", + "atlantis plan -d ./..", + "atlantis apply -d ./..", + "atlantis plan -d a/b/../../..", + "atlantis apply -d a/../..", + } + for _, c := range comments { + r := commentParser.Parse(c, vcs.Github) + exp := "Error: Using a relative path" + Assert(t, strings.Contains(r.CommentResponse, exp), + "For comment %q expected CommentResponse %q to contain %q", c, r.CommentResponse, exp) + } +} + +func TestParse_InvalidWorkspace(t *testing.T) { + t.Log("if -w is used with '..', should return an error") + comments := []string{ + "atlantis plan -w ..", + "atlantis apply -w ..", + "atlantis plan -w ..abc", + "atlantis apply -w abc..", + "atlantis plan -w abc..abc", + "atlantis apply -w ../../../etc/passwd", + } + for _, c := range comments { + r := commentParser.Parse(c, vcs.Github) + exp := "Error: Value for -w/--workspace can't contain '..'" + Assert(t, r.CommentResponse == exp, + "For comment %q expected CommentResponse %q to be %q", c, r.CommentResponse, exp) + } +} + +func TestParse_Parsing(t *testing.T) { + cases := []struct { + flags string + expWorkspace string + expDir string + expVerbose bool + expExtraArgs string + }{ + // Test defaults. + { + "", + "default", + "", + false, + "", + }, + // Test each flag individually. + { + "-w workspace", + "workspace", + "", + false, + "", + }, + { + "-d dir", + "default", + "dir", + false, + "", + }, + { + "--verbose", + "default", + "", + true, + "", + }, + // Test all of them with different permutations. + { + "-w workspace -d dir --verbose", + "workspace", + "dir", + true, + "", + }, + { + "-d dir -w workspace --verbose", + "workspace", + "dir", + true, + "", + }, + { + "--verbose -w workspace -d dir", + "workspace", + "dir", + true, + "", + }, + // Test that flags after -- are ignored + { + "-w workspace -d dir -- --verbose", + "workspace", + "dir", + false, + "\"--verbose\"", + }, + { + "-w workspace -- -d dir --verbose", + "workspace", + "", + false, + "\"-d\" \"dir\" \"--verbose\"", + }, + // Test missing arguments. + { + "-w -d dir --verbose", + "-d", + "", + true, + "", + }, + // Test the extra args parsing. + { + "--", + "default", + "", + false, + "", + }, + { + "abc --", + "default", + "", + false, + "", + }, + // Test trying to escape quoting + { + "abc -- \";echo \"hi", + "default", + "", + false, + `"\";echo" "\"hi"`, + }, + { + "-w workspace -d dir --verbose -- arg one -two --three &&", + "workspace", + "dir", + true, + "\"arg\" \"one\" \"-two\" \"--three\" \"&&\"", + }, + // Test whitespace. + { + "\t-w\tworkspace\t-d\tdir\t--verbose\t--\targ\tone\t-two\t--three\t&&", + "workspace", + "dir", + true, + "\"arg\" \"one\" \"-two\" \"--three\" \"&&\"", + }, + { + " -w workspace -d dir --verbose -- arg one -two --three &&", + "workspace", + "dir", + true, + "\"arg\" \"one\" \"-two\" \"--three\" \"&&\"", + }, + // Test that the dir string is normalized. + { + "-d /", + "default", + ".", + false, + "", + }, + { + "-d /adir", + "default", + "adir", + false, + "", + }, + { + "-d .", + "default", + ".", + false, + "", + }, + { + "-d ./", + "default", + ".", + false, + "", + }, + { + "-d ./adir", + "default", + "adir", + false, + "", + }, + } + for _, test := range cases { + for _, cmdName := range []string{"plan", "apply"} { + comment := fmt.Sprintf("atlantis %s %s", cmdName, test.flags) + r := commentParser.Parse(comment, vcs.Github) + Assert(t, r.CommentResponse == "", "CommentResponse should have been empty but was %q for comment %q", r.CommentResponse, comment) + Assert(t, test.expDir == r.Command.Dir, "exp dir to equal %q but was %q for comment %q", test.expDir, r.Command.Dir, comment) + Assert(t, test.expWorkspace == r.Command.Workspace, "exp workspace to equal %q but was %q for comment %q", test.expWorkspace, r.Command.Workspace, comment) + Assert(t, test.expVerbose == r.Command.Verbose, "exp verbose to equal %v but was %v for comment %q", test.expVerbose, r.Command.Verbose, comment) + actExtraArgs := strings.Join(r.Command.Flags, " ") + Assert(t, test.expExtraArgs == actExtraArgs, "exp extra args to equal %v but got %v for comment %q", test.expExtraArgs, actExtraArgs, comment) + if cmdName == "plan" { + Assert(t, r.Command.Name == events.Plan, "did not parse comment %q as plan command", comment) + } + if cmdName == "apply" { + Assert(t, r.Command.Name == events.Apply, "did not parse comment %q as apply command", comment) + } + } + } +} diff --git a/server/events/event_parser.go b/server/events/event_parser.go index 3225e527b7..fd3db3ae77 100644 --- a/server/events/event_parser.go +++ b/server/events/event_parser.go @@ -3,17 +3,20 @@ package events import ( "errors" "fmt" - "path/filepath" + "regexp" "strings" "github.com/google/go-github/github" "github.com/lkysow/go-gitlab" "github.com/runatlantis/atlantis/server/events/models" - "github.com/runatlantis/atlantis/server/events/vcs" - "github.com/spf13/pflag" ) const gitlabPullOpened = "opened" +const usagesCols = 90 + +// multiLineRegex is used to ignore multi-line comments since those aren't valid +// Atlantis commands. +var multiLineRegex = regexp.MustCompile(`.*\r?\n.+`) //go:generate pegomock generate -m --use-experimental-model-gen --package mocks -o mocks/mock_event_parsing.go EventParsing @@ -29,7 +32,6 @@ type Command struct { } type EventParsing interface { - DetermineCommand(comment string, vcsHost vcs.Host) (*Command, error) ParseGithubIssueCommentEvent(comment *github.IssueCommentEvent) (baseRepo models.Repo, user models.User, pullNum int, err error) ParseGithubPull(pull *github.PullRequest) (models.PullRequest, models.Repo, error) ParseGithubRepo(ghRepo *github.Repository) (models.Repo, error) @@ -45,106 +47,6 @@ type EventParser struct { GitlabToken string } -// DetermineCommand parses the comment as an atlantis command. If it succeeds, -// it returns the command. Otherwise it returns error. -// nolint: gocyclo -func (e *EventParser) DetermineCommand(comment string, vcsHost vcs.Host) (*Command, error) { - // valid commands contain: - // the initial "executable" name, 'run' or 'atlantis' or '@GithubUser' where GithubUser is the api user atlantis is running as - // then a command, either 'plan', 'apply', or 'help' - // then an optional workspace argument, an optional --verbose flag and any other flags - // - // examples: - // atlantis help - // run plan - // @GithubUser plan staging - // atlantis plan staging --verbose - // atlantis plan staging --verbose -key=value -key2 value2 - err := errors.New("not an Atlantis command") - args := strings.Fields(comment) - if len(args) < 2 { - return nil, err - } - - vcsUser := e.GithubUser - if vcsHost == vcs.Gitlab { - vcsUser = e.GitlabUser - } - if !e.stringInSlice(args[0], []string{"run", "atlantis", "@" + vcsUser}) { - return nil, err - } - if !e.stringInSlice(args[1], []string{"plan", "apply", "help", "-help", "--help"}) { - return nil, err - } - - command := args[1] - if command == "help" || command == "-help" || command == "--help" { - return &Command{Name: Help}, nil - } - - var workspace string - var dir string - var verbose bool - var extraArgs []string - var flagSet *pflag.FlagSet - var name CommandName - - // Set up the flag parsing depending on the command. - const defaultWorkspace = "default" - if command == "plan" { - name = Plan - flagSet = pflag.NewFlagSet("plan", pflag.ContinueOnError) - flagSet.StringVarP(&workspace, "workspace", "w", defaultWorkspace, fmt.Sprintf("Switch to this Terraform workspace before planning. Defaults to '%s'", defaultWorkspace)) - flagSet.StringVarP(&dir, "dir", "d", "", "Which directory to run plan in relative to root of repo. Use '.' for root. If not specified, will attempt to run plan for all Terraform projects we think were modified in this changeset.") - flagSet.BoolVarP(&verbose, "verbose", "", false, "Append Atlantis log to comment.") - } else if command == "apply" { - name = Apply - flagSet = pflag.NewFlagSet("apply", pflag.ContinueOnError) - flagSet.StringVarP(&workspace, "workspace", "w", defaultWorkspace, fmt.Sprintf("Apply the plan for this Terraform workspace. Defaults to '%s'", defaultWorkspace)) - flagSet.StringVarP(&dir, "dir", "d", "", "Apply the plan for this directory, relative to root of repo. Use '.' for root. If not specified, will run apply against all plans created for this workspace.") - flagSet.BoolVarP(&verbose, "verbose", "", false, "Append Atlantis log to comment.") - } else { - return nil, fmt.Errorf("unknown command %q – this is a bug", command) - } - - // Now parse the flags. - if err := flagSet.Parse(args[2:]); err != nil { - return nil, err - } - // We only use the extra args after the --. For example given a comment: - // "atlantis plan -bad-option -- -target=hi" - // we only append "-target=hi" to the eventual command. - // todo: keep track of the args we're discarding and include that with - // comment as a warning. - if flagSet.ArgsLenAtDash() != -1 { - extraArgs = flagSet.Args()[flagSet.ArgsLenAtDash():] - } - - // If dir is specified, must ensure it's a valid path. - if dir != "" { - validatedDir := filepath.Clean(dir) - // Join with . so the path is relative. This helps us if they use '/', - // and is safe to do if their path is relative since it's a no-op. - validatedDir = filepath.Join(".", validatedDir) - // Need to clean again to resolve relative validatedDirs. - validatedDir = filepath.Clean(validatedDir) - // Detect relative dirs since they're not allowed. - if strings.HasPrefix(validatedDir, "..") { - return nil, fmt.Errorf("relative path %q not allowed", dir) - } - - dir = validatedDir - } - // Because we use the workspace name as a file, need to make sure it's - // not doing something weird like being a relative dir. - if strings.Contains(workspace, "..") { - return nil, errors.New("workspace can't contain '..'") - } - - c := &Command{Name: name, Verbose: verbose, Workspace: workspace, Dir: dir, Flags: extraArgs} - return c, nil -} - func (e *EventParser) ParseGithubIssueCommentEvent(comment *github.IssueCommentEvent) (baseRepo models.Repo, user models.User, pullNum int, err error) { baseRepo, err = e.ParseGithubRepo(comment.Repo) if err != nil { @@ -335,12 +237,3 @@ func (e *EventParser) ParseGitlabMergeRequest(mr *gitlab.MergeRequest) models.Pu State: pullState, } } - -func (e *EventParser) stringInSlice(a string, list []string) bool { - for _, b := range list { - if b == a { - return true - } - } - return false -} diff --git a/server/events/event_parser_test.go b/server/events/event_parser_test.go index c8c5bc8aea..a13b9f3e24 100644 --- a/server/events/event_parser_test.go +++ b/server/events/event_parser_test.go @@ -3,8 +3,6 @@ package events_test import ( "encoding/json" "errors" - "fmt" - "strings" "testing" "github.com/google/go-github/github" @@ -12,7 +10,6 @@ import ( "github.com/mohae/deepcopy" "github.com/runatlantis/atlantis/server/events" "github.com/runatlantis/atlantis/server/events/models" - "github.com/runatlantis/atlantis/server/events/vcs" . "github.com/runatlantis/atlantis/server/events/vcs/fixtures" . "github.com/runatlantis/atlantis/testing" ) @@ -24,250 +21,6 @@ var parser = events.EventParser{ GitlabToken: "gitlab-token", } -func TestDetermineCommandInvalid(t *testing.T) { - t.Log("given an invalid comment, should return an error") - comments := []string{ - // just the executable, no command - "run", - "atlantis", - "@github-user", - // invalid command - "run slkjd", - "atlantis slkjd", - "@github-user slkjd", - "atlantis plans", - // relative dirs - "atlantis plan -d ..", - "atlantis plan -d ../", - "atlantis plan -d a/../../", - // using .. in workspace - "atlantis plan -w a..", - "atlantis plan -w ../", - "atlantis plan -w ..", - "atlantis plan -w a/../b", - // misc - "related comment mentioning atlantis", - } - for _, c := range comments { - _, e := parser.DetermineCommand(c, vcs.Github) - Assert(t, e != nil, "expected error for comment: "+c) - } -} - -func TestDetermineCommand_ExecutableNames(t *testing.T) { - t.Log("should be allowed to use different executable names in the comments") - parsed, err := parser.DetermineCommand("atlantis plan", vcs.Github) - Ok(t, err) - Equals(t, events.Plan, parsed.Name) - - parsed, err = parser.DetermineCommand("run plan", vcs.Github) - Ok(t, err) - Equals(t, events.Plan, parsed.Name) - - parsed, err = parser.DetermineCommand("@github-user plan", vcs.Github) - Ok(t, err) - Equals(t, events.Plan, parsed.Name) - - parsed, err = parser.DetermineCommand("@gitlab-user plan", vcs.Gitlab) - Ok(t, err) - Equals(t, events.Plan, parsed.Name) -} - -func TestDetermineCommand_Help(t *testing.T) { - t.Log("given a help comment, should match") - helpArgs := []string{ - "help", - "-help", - "--help", - "help -verbose", - "help --hi", - "help somethingelse", - } - for _, arg := range helpArgs { - comment := fmt.Sprintf("atlantis %s", arg) - command, err := parser.DetermineCommand(comment, vcs.Github) - Assert(t, err == nil, "did not parse comment %q as help command, got err: %s", comment, err) - Assert(t, command.Name == events.Help, "did not parse comment %q as help command", comment) - } -} - -func TestDetermineCommand_Parsing(t *testing.T) { - cases := []struct { - flags string - expWorkspace string - expDir string - expVerbose bool - expExtraArgs string - }{ - // Test defaults. - { - "", - "default", - "", - false, - "", - }, - // Test each flag individually. - { - "-w workspace", - "workspace", - "", - false, - "", - }, - { - "-d dir", - "default", - "dir", - false, - "", - }, - { - "--verbose", - "default", - "", - true, - "", - }, - // Test all of them with different permutations. - { - "-w workspace -d dir --verbose", - "workspace", - "dir", - true, - "", - }, - { - "-d dir -w workspace --verbose", - "workspace", - "dir", - true, - "", - }, - { - "--verbose -w workspace -d dir", - "workspace", - "dir", - true, - "", - }, - // Test that flags after -- are ignored - { - "-w workspace -d dir -- --verbose", - "workspace", - "dir", - false, - "--verbose", - }, - { - "-w workspace -- -d dir --verbose", - "workspace", - "", - false, - "-d dir --verbose", - }, - // Test missing arguments. - { - "-w -d dir --verbose", - "-d", - "", - true, - "", - }, - // Test the extra args parsing. - { - "--", - "default", - "", - false, - "", - }, - { - "abc --", - "default", - "", - false, - "", - }, - { - "-w workspace -d dir --verbose -- arg one -two --three &&", - "workspace", - "dir", - true, - "arg one -two --three &&", - }, - // Test whitespace. - { - "\t-w\tworkspace\t-d\tdir\t--verbose\t--\targ\tone\t-two\t--three\t&&", - "workspace", - "dir", - true, - "arg one -two --three &&", - }, - { - " -w workspace -d dir --verbose -- arg one -two --three &&", - "workspace", - "dir", - true, - "arg one -two --three &&", - }, - // Test that the dir string is normalized. - { - "-d /", - "default", - ".", - false, - "", - }, - { - "-d /adir", - "default", - "adir", - false, - "", - }, - { - "-d .", - "default", - ".", - false, - "", - }, - { - "-d ./", - "default", - ".", - false, - "", - }, - { - "-d ./adir", - "default", - "adir", - false, - "", - }, - } - for _, test := range cases { - for _, cmdName := range []string{"plan", "apply"} { - comment := fmt.Sprintf("atlantis %s %s", cmdName, test.flags) - t.Logf("testing comment: %s", comment) - cmd, err := parser.DetermineCommand(comment, vcs.Github) - Assert(t, err == nil, "unexpected err parsing %q: %s", comment, err) - Equals(t, test.expDir, cmd.Dir) - Equals(t, test.expWorkspace, cmd.Workspace) - Equals(t, test.expVerbose, cmd.Verbose) - Equals(t, test.expExtraArgs, strings.Join(cmd.Flags, " ")) - if cmdName == "plan" { - Assert(t, cmd.Name == events.Plan, "did not parse comment %q as plan command", comment) - } - if cmdName == "apply" { - Assert(t, cmd.Name == events.Apply, "did not parse comment %q as apply command", comment) - } - } - } -} - func TestParseGithubRepo(t *testing.T) { testRepo := Repo testRepo.FullName = nil diff --git a/server/events/help_executor.go b/server/events/help_executor.go deleted file mode 100644 index 74cbd2e4e2..0000000000 --- a/server/events/help_executor.go +++ /dev/null @@ -1,11 +0,0 @@ -package events - -// HelpExecutor executes the help command. -type HelpExecutor struct{} - -// Execute executes the help command. -func (h *HelpExecutor) Execute(ctx *CommandContext) CommandResponse { - // We don't actually need to do anything since the comment renderer - // will see that it is a help command and render the help comment. - return CommandResponse{} -} diff --git a/server/events/help_executor_test.go b/server/events/help_executor_test.go deleted file mode 100644 index a1c6243116..0000000000 --- a/server/events/help_executor_test.go +++ /dev/null @@ -1,15 +0,0 @@ -package events_test - -import ( - "testing" - - "github.com/runatlantis/atlantis/server/events" - . "github.com/runatlantis/atlantis/testing" -) - -func TestExecute(t *testing.T) { - h := events.HelpExecutor{} - ctx := events.CommandContext{} - r := h.Execute(&ctx) - Equals(t, events.CommandResponse{}, r) -} diff --git a/server/events/markdown_renderer.go b/server/events/markdown_renderer.go index 1841068606..704ad58713 100644 --- a/server/events/markdown_renderer.go +++ b/server/events/markdown_renderer.go @@ -38,9 +38,6 @@ type ResultData struct { // Render formats the data into a markdown string. // nolint: interfacer func (g *MarkdownRenderer) Render(res CommandResponse, cmdName CommandName, log string, verbose bool) string { - if cmdName == Help { - return g.renderTemplate(helpTmpl, nil) - } commandStr := strings.Title(cmdName.String()) common := CommonData{commandStr, verbose, log} if res.Error != nil { @@ -97,31 +94,6 @@ func (g *MarkdownRenderer) renderTemplate(tmpl *template.Template, data interfac return buf.String() } -var helpTmpl = template.Must(template.New("").Parse("```cmake\n" + - `atlantis - Terraform collaboration tool that enables you to collaborate on infrastructure -safely and securely. - -Usage: atlantis [workspace] [--verbose] - -Commands: -plan Runs 'terraform plan' on the files changed in the pull request -apply Runs 'terraform apply' using the plans generated by 'atlantis plan' -help Get help - -Examples: - -# Generates a plan for staging workspace -atlantis plan staging - -# Generates a plan for a standalone terraform project -atlantis plan - -# Applies a plan for staging workspace -atlantis apply staging - -# Applies a plan for a standalone terraform project -atlantis apply -`)) var singleProjectTmpl = template.Must(template.New("").Parse("{{ range $result := .Results }}{{$result}}{{end}}\n" + logTmpl)) var multiProjectTmpl = template.Must(template.New("").Parse( "Ran {{.Command}} in {{ len .Results }} directories:\n" + diff --git a/server/events/mocks/matchers/events_commandparseresult.go b/server/events/mocks/matchers/events_commandparseresult.go new file mode 100644 index 0000000000..12e5991a7e --- /dev/null +++ b/server/events/mocks/matchers/events_commandparseresult.go @@ -0,0 +1,20 @@ +package matchers + +import ( + "reflect" + + "github.com/petergtz/pegomock" + events "github.com/runatlantis/atlantis/server/events" +) + +func AnyEventsCommandParseResult() events.CommentParseResult { + pegomock.RegisterMatcher(pegomock.NewAnyMatcher(reflect.TypeOf((*(events.CommentParseResult))(nil)).Elem())) + var nullValue events.CommentParseResult + return nullValue +} + +func EqEventsCommandParseResult(value events.CommentParseResult) events.CommentParseResult { + pegomock.RegisterMatcher(&pegomock.EqMatcher{Value: value}) + var nullValue events.CommentParseResult + return nullValue +} diff --git a/server/events/mocks/matchers/events_commentparseresult.go b/server/events/mocks/matchers/events_commentparseresult.go new file mode 100644 index 0000000000..5409329944 --- /dev/null +++ b/server/events/mocks/matchers/events_commentparseresult.go @@ -0,0 +1,20 @@ +package matchers + +import ( + "reflect" + + "github.com/petergtz/pegomock" + events "github.com/runatlantis/atlantis/server/events" +) + +func AnyEventsCommentParseResult() events.CommentParseResult { + pegomock.RegisterMatcher(pegomock.NewAnyMatcher(reflect.TypeOf((*(events.CommentParseResult))(nil)).Elem())) + var nullValue events.CommentParseResult + return nullValue +} + +func EqEventsCommentParseResult(value events.CommentParseResult) events.CommentParseResult { + pegomock.RegisterMatcher(&pegomock.EqMatcher{Value: value}) + var nullValue events.CommentParseResult + return nullValue +} diff --git a/server/events/mocks/mock_comment_parsing.go b/server/events/mocks/mock_comment_parsing.go new file mode 100644 index 0000000000..54dabbd7de --- /dev/null +++ b/server/events/mocks/mock_comment_parsing.go @@ -0,0 +1,81 @@ +// Automatically generated by pegomock. DO NOT EDIT! +// Source: github.com/runatlantis/atlantis/server/events (interfaces: CommentParsing) + +package mocks + +import ( + "reflect" + + pegomock "github.com/petergtz/pegomock" + events "github.com/runatlantis/atlantis/server/events" + vcs "github.com/runatlantis/atlantis/server/events/vcs" +) + +type MockCommentParsing struct { + fail func(message string, callerSkip ...int) +} + +func NewMockCommentParsing() *MockCommentParsing { + return &MockCommentParsing{fail: pegomock.GlobalFailHandler} +} + +func (mock *MockCommentParsing) Parse(comment string, vcsHost vcs.Host) events.CommentParseResult { + params := []pegomock.Param{comment, vcsHost} + result := pegomock.GetGenericMockFrom(mock).Invoke("Parse", params, []reflect.Type{reflect.TypeOf((*events.CommentParseResult)(nil)).Elem()}) + var ret0 events.CommentParseResult + if len(result) != 0 { + if result[0] != nil { + ret0 = result[0].(events.CommentParseResult) + } + } + return ret0 +} + +func (mock *MockCommentParsing) VerifyWasCalledOnce() *VerifierCommentParsing { + return &VerifierCommentParsing{mock, pegomock.Times(1), nil} +} + +func (mock *MockCommentParsing) VerifyWasCalled(invocationCountMatcher pegomock.Matcher) *VerifierCommentParsing { + return &VerifierCommentParsing{mock, invocationCountMatcher, nil} +} + +func (mock *MockCommentParsing) VerifyWasCalledInOrder(invocationCountMatcher pegomock.Matcher, inOrderContext *pegomock.InOrderContext) *VerifierCommentParsing { + return &VerifierCommentParsing{mock, invocationCountMatcher, inOrderContext} +} + +type VerifierCommentParsing struct { + mock *MockCommentParsing + invocationCountMatcher pegomock.Matcher + inOrderContext *pegomock.InOrderContext +} + +func (verifier *VerifierCommentParsing) Parse(comment string, vcsHost vcs.Host) *CommentParsing_Parse_OngoingVerification { + params := []pegomock.Param{comment, vcsHost} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "Parse", params) + return &CommentParsing_Parse_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} +} + +type CommentParsing_Parse_OngoingVerification struct { + mock *MockCommentParsing + methodInvocations []pegomock.MethodInvocation +} + +func (c *CommentParsing_Parse_OngoingVerification) GetCapturedArguments() (string, vcs.Host) { + comment, vcsHost := c.GetAllCapturedArguments() + return comment[len(comment)-1], vcsHost[len(vcsHost)-1] +} + +func (c *CommentParsing_Parse_OngoingVerification) GetAllCapturedArguments() (_param0 []string, _param1 []vcs.Host) { + params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) + if len(params) > 0 { + _param0 = make([]string, len(params[0])) + for u, param := range params[0] { + _param0[u] = param.(string) + } + _param1 = make([]vcs.Host, len(params[1])) + for u, param := range params[1] { + _param1[u] = param.(vcs.Host) + } + } + return +} diff --git a/server/events/mocks/mock_event_parsing.go b/server/events/mocks/mock_event_parsing.go index 64f6b11ab7..ad401861df 100644 --- a/server/events/mocks/mock_event_parsing.go +++ b/server/events/mocks/mock_event_parsing.go @@ -9,9 +9,7 @@ import ( github "github.com/google/go-github/github" go_gitlab "github.com/lkysow/go-gitlab" pegomock "github.com/petergtz/pegomock" - events "github.com/runatlantis/atlantis/server/events" models "github.com/runatlantis/atlantis/server/events/models" - vcs "github.com/runatlantis/atlantis/server/events/vcs" ) type MockEventParsing struct { @@ -22,22 +20,6 @@ func NewMockEventParsing() *MockEventParsing { return &MockEventParsing{fail: pegomock.GlobalFailHandler} } -func (mock *MockEventParsing) DetermineCommand(comment string, vcsHost vcs.Host) (*events.Command, error) { - params := []pegomock.Param{comment, vcsHost} - result := pegomock.GetGenericMockFrom(mock).Invoke("DetermineCommand", params, []reflect.Type{reflect.TypeOf((**events.Command)(nil)).Elem(), reflect.TypeOf((*error)(nil)).Elem()}) - var ret0 *events.Command - var ret1 error - if len(result) != 0 { - if result[0] != nil { - ret0 = result[0].(*events.Command) - } - if result[1] != nil { - ret1 = result[1].(error) - } - } - return ret0, ret1 -} - func (mock *MockEventParsing) ParseGithubIssueCommentEvent(comment *github.IssueCommentEvent) (models.Repo, models.User, int, error) { params := []pegomock.Param{comment} result := pegomock.GetGenericMockFrom(mock).Invoke("ParseGithubIssueCommentEvent", params, []reflect.Type{reflect.TypeOf((*models.Repo)(nil)).Elem(), reflect.TypeOf((*models.User)(nil)).Elem(), reflect.TypeOf((*int)(nil)).Elem(), reflect.TypeOf((*error)(nil)).Elem()}) @@ -164,37 +146,6 @@ type VerifierEventParsing struct { inOrderContext *pegomock.InOrderContext } -func (verifier *VerifierEventParsing) DetermineCommand(comment string, vcsHost vcs.Host) *EventParsing_DetermineCommand_OngoingVerification { - params := []pegomock.Param{comment, vcsHost} - methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "DetermineCommand", params) - return &EventParsing_DetermineCommand_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} -} - -type EventParsing_DetermineCommand_OngoingVerification struct { - mock *MockEventParsing - methodInvocations []pegomock.MethodInvocation -} - -func (c *EventParsing_DetermineCommand_OngoingVerification) GetCapturedArguments() (string, vcs.Host) { - comment, vcsHost := c.GetAllCapturedArguments() - return comment[len(comment)-1], vcsHost[len(vcsHost)-1] -} - -func (c *EventParsing_DetermineCommand_OngoingVerification) GetAllCapturedArguments() (_param0 []string, _param1 []vcs.Host) { - params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) - if len(params) > 0 { - _param0 = make([]string, len(params[0])) - for u, param := range params[0] { - _param0[u] = param.(string) - } - _param1 = make([]vcs.Host, len(params[1])) - for u, param := range params[1] { - _param1[u] = param.(vcs.Host) - } - } - return -} - func (verifier *VerifierEventParsing) ParseGithubIssueCommentEvent(comment *github.IssueCommentEvent) *EventParsing_ParseGithubIssueCommentEvent_OngoingVerification { params := []pegomock.Param{comment} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "ParseGithubIssueCommentEvent", params) diff --git a/server/events/plan_executor_test.go b/server/events/plan_executor_test.go index a26440eed6..168114367f 100644 --- a/server/events/plan_executor_test.go +++ b/server/events/plan_executor_test.go @@ -97,6 +97,54 @@ func TestExecute_DirectoryAndWorkspaceSet(t *testing.T) { Equals(t, "lockurl-key", result.PlanSuccess.LockURL) } +func TestExecute_AddedArgs(t *testing.T) { + t.Log("Test that we include extra-args added to the comment in the plan command") + p, runner, _ := setupPlanExecutorTest(t) + ctx := deepcopy.Copy(planCtx).(events.CommandContext) + ctx.Log = logging.NewNoopLogger() + ctx.Command.Flags = []string{"\"-target=resource\"", "\"-var\"", "\"a=b\"", "\";\"", "\"echo\"", "\"hi\""} + + When(p.VCSClient.GetModifiedFiles(matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest(), matchers.AnyVcsHost())).ThenReturn([]string{"file.tf"}, nil) + When(p.Workspace.Clone(ctx.Log, ctx.BaseRepo, ctx.HeadRepo, ctx.Pull, "workspace")). + ThenReturn("/tmp/clone-repo", nil) + When(p.ProjectPreExecute.Execute(&ctx, "/tmp/clone-repo", models.Project{RepoFullName: "", Path: "."})). + ThenReturn(events.PreExecuteResult{ + LockResponse: locking.TryLockResponse{ + LockKey: "key", + }, + }) + r := p.Execute(&ctx) + + runner.VerifyWasCalledOnce().RunCommandWithVersion( + ctx.Log, + "/tmp/clone-repo", + []string{ + "plan", + "-refresh", + "-no-color", + "-out", + "/tmp/clone-repo/workspace.tfplan", + "-var", + "atlantis_user=anubhavmishra", + // NOTE: extra args should be quoted to prevent an attacker from + // appending malicious commands. + "\"-target=resource\"", + "\"-var\"", + "\"a=b\"", + "\";\"", + "\"echo\"", + "\"hi\"", + }, + nil, + "workspace", + ) + Assert(t, len(r.ProjectResults) == 1, "exp one project result") + result := r.ProjectResults[0] + Assert(t, result.PlanSuccess != nil, "exp plan success to not be nil") + Equals(t, "", result.PlanSuccess.TerraformOutput) + Equals(t, "lockurl-key", result.PlanSuccess.LockURL) +} + func TestExecute_Success(t *testing.T) { t.Log("If there are no errors, the plan should be returned") p, runner, _ := setupPlanExecutorTest(t) diff --git a/server/events/pull_closed_executor.go b/server/events/pull_closed_executor.go index 658e087073..7f3cd3d12d 100644 --- a/server/events/pull_closed_executor.go +++ b/server/events/pull_closed_executor.go @@ -64,7 +64,7 @@ func (p *PullClosedExecutor) CleanUpPull(repo models.Repo, pull models.PullReque if err = pullClosedTemplate.Execute(&buf, templateData); err != nil { return errors.Wrap(err, "rendering template for comment") } - return p.VCSClient.CreateComment(repo, pull, buf.String(), host) + return p.VCSClient.CreateComment(repo, pull.Num, buf.String(), host) } // buildTemplateData formats the lock data into a slice that can easily be diff --git a/server/events/pull_closed_executor_test.go b/server/events/pull_closed_executor_test.go index d89be41687..2e21c2eb1e 100644 --- a/server/events/pull_closed_executor_test.go +++ b/server/events/pull_closed_executor_test.go @@ -58,7 +58,7 @@ func TestCleanUpPullNoLocks(t *testing.T) { When(l.UnlockByPull(fixtures.Repo.FullName, fixtures.Pull.Num)).ThenReturn(nil, nil) err := pce.CleanUpPull(fixtures.Repo, fixtures.Pull, vcs.Github) Ok(t, err) - cp.VerifyWasCalled(Never()).CreateComment(matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest(), AnyString(), matchers.AnyVcsHost()) + cp.VerifyWasCalled(Never()).CreateComment(matchers.AnyModelsRepo(), AnyInt(), AnyString(), matchers.AnyVcsHost()) } func TestCleanUpPullComments(t *testing.T) { @@ -139,7 +139,7 @@ func TestCleanUpPullComments(t *testing.T) { When(l.UnlockByPull(fixtures.Repo.FullName, fixtures.Pull.Num)).ThenReturn(c.Locks, nil) err := pce.CleanUpPull(fixtures.Repo, fixtures.Pull, vcs.Github) Ok(t, err) - _, _, comment, _ := cp.VerifyWasCalledOnce().CreateComment(matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest(), AnyString(), matchers.AnyVcsHost()).GetCapturedArguments() + _, _, comment, _ := cp.VerifyWasCalledOnce().CreateComment(matchers.AnyModelsRepo(), AnyInt(), AnyString(), matchers.AnyVcsHost()).GetCapturedArguments() expected := "Locks and plans deleted for the projects and workspaces modified in this pull request:\n\n" + c.Exp Equals(t, expected, comment) diff --git a/server/events/vcs/client.go b/server/events/vcs/client.go index 2e2db4de1d..d4e30eb7da 100644 --- a/server/events/vcs/client.go +++ b/server/events/vcs/client.go @@ -9,7 +9,7 @@ import ( // Client is used to make API calls to a VCS host like GitHub or GitLab. type Client interface { GetModifiedFiles(repo models.Repo, pull models.PullRequest) ([]string, error) - CreateComment(repo models.Repo, pull models.PullRequest, comment string) error + CreateComment(repo models.Repo, pullNum int, comment string) error PullIsApproved(repo models.Repo, pull models.PullRequest) (bool, error) UpdateStatus(repo models.Repo, pull models.PullRequest, state CommitStatus, description string) error } diff --git a/server/events/vcs/github_client.go b/server/events/vcs/github_client.go index fbf4127789..6b344a091d 100644 --- a/server/events/vcs/github_client.go +++ b/server/events/vcs/github_client.go @@ -70,8 +70,8 @@ func (g *GithubClient) GetModifiedFiles(repo models.Repo, pull models.PullReques } // CreateComment creates a comment on the pull request. -func (g *GithubClient) CreateComment(repo models.Repo, pull models.PullRequest, comment string) error { - _, _, err := g.client.Issues.CreateComment(g.ctx, repo.Owner, repo.Name, pull.Num, &github.IssueComment{Body: &comment}) +func (g *GithubClient) CreateComment(repo models.Repo, pullNum int, comment string) error { + _, _, err := g.client.Issues.CreateComment(g.ctx, repo.Owner, repo.Name, pullNum, &github.IssueComment{Body: &comment}) return err } diff --git a/server/events/vcs/gitlab_client.go b/server/events/vcs/gitlab_client.go index 50ad36becb..7e036d469b 100644 --- a/server/events/vcs/gitlab_client.go +++ b/server/events/vcs/gitlab_client.go @@ -48,8 +48,8 @@ func (g *GitlabClient) GetModifiedFiles(repo models.Repo, pull models.PullReques } // CreateComment creates a comment on the merge request. -func (g *GitlabClient) CreateComment(repo models.Repo, pull models.PullRequest, comment string) error { - _, _, err := g.Client.Notes.CreateMergeRequestNote(repo.FullName, pull.Num, &gitlab.CreateMergeRequestNoteOptions{Body: gitlab.String(comment)}) +func (g *GitlabClient) CreateComment(repo models.Repo, pullNum int, comment string) error { + _, _, err := g.Client.Notes.CreateMergeRequestNote(repo.FullName, pullNum, &gitlab.CreateMergeRequestNoteOptions{Body: gitlab.String(comment)}) return err } diff --git a/server/events/vcs/mocks/mock_proxy.go b/server/events/vcs/mocks/mock_proxy.go index efb20dff92..287757193e 100644 --- a/server/events/vcs/mocks/mock_proxy.go +++ b/server/events/vcs/mocks/mock_proxy.go @@ -35,8 +35,8 @@ func (mock *MockClientProxy) GetModifiedFiles(repo models.Repo, pull models.Pull return ret0, ret1 } -func (mock *MockClientProxy) CreateComment(repo models.Repo, pull models.PullRequest, comment string, host vcs.Host) error { - params := []pegomock.Param{repo, pull, comment, host} +func (mock *MockClientProxy) CreateComment(repo models.Repo, pullNum int, comment string, host vcs.Host) error { + params := []pegomock.Param{repo, pullNum, comment, host} result := pegomock.GetGenericMockFrom(mock).Invoke("CreateComment", params, []reflect.Type{reflect.TypeOf((*error)(nil)).Elem()}) var ret0 error if len(result) != 0 { @@ -128,8 +128,8 @@ func (c *ClientProxy_GetModifiedFiles_OngoingVerification) GetAllCapturedArgumen return } -func (verifier *VerifierClientProxy) CreateComment(repo models.Repo, pull models.PullRequest, comment string, host vcs.Host) *ClientProxy_CreateComment_OngoingVerification { - params := []pegomock.Param{repo, pull, comment, host} +func (verifier *VerifierClientProxy) CreateComment(repo models.Repo, pullNum int, comment string, host vcs.Host) *ClientProxy_CreateComment_OngoingVerification { + params := []pegomock.Param{repo, pullNum, comment, host} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "CreateComment", params) return &ClientProxy_CreateComment_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } @@ -139,21 +139,21 @@ type ClientProxy_CreateComment_OngoingVerification struct { methodInvocations []pegomock.MethodInvocation } -func (c *ClientProxy_CreateComment_OngoingVerification) GetCapturedArguments() (models.Repo, models.PullRequest, string, vcs.Host) { - repo, pull, comment, host := c.GetAllCapturedArguments() - return repo[len(repo)-1], pull[len(pull)-1], comment[len(comment)-1], host[len(host)-1] +func (c *ClientProxy_CreateComment_OngoingVerification) GetCapturedArguments() (models.Repo, int, string, vcs.Host) { + repo, pullNum, comment, host := c.GetAllCapturedArguments() + return repo[len(repo)-1], pullNum[len(pullNum)-1], comment[len(comment)-1], host[len(host)-1] } -func (c *ClientProxy_CreateComment_OngoingVerification) GetAllCapturedArguments() (_param0 []models.Repo, _param1 []models.PullRequest, _param2 []string, _param3 []vcs.Host) { +func (c *ClientProxy_CreateComment_OngoingVerification) GetAllCapturedArguments() (_param0 []models.Repo, _param1 []int, _param2 []string, _param3 []vcs.Host) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { _param0 = make([]models.Repo, len(params[0])) for u, param := range params[0] { _param0[u] = param.(models.Repo) } - _param1 = make([]models.PullRequest, len(params[1])) + _param1 = make([]int, len(params[1])) for u, param := range params[1] { - _param1[u] = param.(models.PullRequest) + _param1[u] = param.(int) } _param2 = make([]string, len(params[2])) for u, param := range params[2] { diff --git a/server/events/vcs/not_configured_vcs_client.go b/server/events/vcs/not_configured_vcs_client.go index 7e02cba665..dd240f4b68 100644 --- a/server/events/vcs/not_configured_vcs_client.go +++ b/server/events/vcs/not_configured_vcs_client.go @@ -16,7 +16,7 @@ type NotConfiguredVCSClient struct { func (a *NotConfiguredVCSClient) GetModifiedFiles(repo models.Repo, pull models.PullRequest) ([]string, error) { return nil, a.err() } -func (a *NotConfiguredVCSClient) CreateComment(repo models.Repo, pull models.PullRequest, comment string) error { +func (a *NotConfiguredVCSClient) CreateComment(repo models.Repo, pullNum int, comment string) error { return a.err() } func (a *NotConfiguredVCSClient) PullIsApproved(repo models.Repo, pull models.PullRequest) (bool, error) { diff --git a/server/events/vcs/proxy.go b/server/events/vcs/proxy.go index 0ca3e2ec76..ee778b9093 100644 --- a/server/events/vcs/proxy.go +++ b/server/events/vcs/proxy.go @@ -11,7 +11,7 @@ import ( // VCS host is required. type ClientProxy interface { GetModifiedFiles(repo models.Repo, pull models.PullRequest, host Host) ([]string, error) - CreateComment(repo models.Repo, pull models.PullRequest, comment string, host Host) error + CreateComment(repo models.Repo, pullNum int, comment string, host Host) error PullIsApproved(repo models.Repo, pull models.PullRequest, host Host) (bool, error) UpdateStatus(repo models.Repo, pull models.PullRequest, state CommitStatus, description string, host Host) error } @@ -48,12 +48,12 @@ func (d *DefaultClientProxy) GetModifiedFiles(repo models.Repo, pull models.Pull return nil, invalidVCSErr } -func (d *DefaultClientProxy) CreateComment(repo models.Repo, pull models.PullRequest, comment string, host Host) error { +func (d *DefaultClientProxy) CreateComment(repo models.Repo, pullNum int, comment string, host Host) error { switch host { case Github: - return d.GithubClient.CreateComment(repo, pull, comment) + return d.GithubClient.CreateComment(repo, pullNum, comment) case Gitlab: - return d.GitlabClient.CreateComment(repo, pull, comment) + return d.GitlabClient.CreateComment(repo, pullNum, comment) } return invalidVCSErr } diff --git a/server/events_controller.go b/server/events_controller.go index 1bfda73721..145c5f263b 100644 --- a/server/events_controller.go +++ b/server/events_controller.go @@ -22,6 +22,7 @@ type EventsController struct { PullCleaner events.PullCleaner Logger *logging.SimpleLogger Parser events.EventParsing + CommentParser events.CommentParsing // GithubWebHookSecret is the secret added to this webhook via the GitHub // UI that identifies this call as coming from GitHub. If empty, no // request validation is done. @@ -35,6 +36,7 @@ type EventsController struct { // SupportedVCSHosts is which VCS hosts Atlantis was configured upon // startup to support. SupportedVCSHosts []vcs.Host + VCSClient vcs.ClientProxy } // Post handles POST webhook requests. @@ -91,17 +93,11 @@ func (e *EventsController) HandleGithubCommentEvent(w http.ResponseWriter, event return } - command, err := e.Parser.DetermineCommand(event.Comment.GetBody(), vcs.Github) - if err != nil { - e.respond(w, logging.Debug, http.StatusOK, "Ignoring: %s %s", err, githubReqID) - return - } - - // Respond with success and then actually execute the command asynchronously. - // We use a goroutine so that this function returns and the connection is - // closed. - fmt.Fprintln(w, "Processing...") - go e.CommandRunner.ExecuteCommand(baseRepo, models.Repo{}, user, pullNum, command, vcs.Github) + // We pass in an empty models.Repo for headRepo because we need to do additional + // calls to get that information but we need this code path to be generic. + // Later on in CommandHandler we detect that this is a GitHub event and + // make the necessary calls to get the headRepo. + e.handleCommentEvent(w, baseRepo, models.Repo{}, user, pullNum, event.Comment.GetBody(), vcs.Github) } // HandleGithubPullRequestEvent will delete any locks associated with the pull @@ -152,9 +148,29 @@ func (e *EventsController) handleGitlabPost(w http.ResponseWriter, r *http.Reque // commands can come from. It's exported to make testing easier. func (e *EventsController) HandleGitlabCommentEvent(w http.ResponseWriter, event gitlab.MergeCommentEvent) { baseRepo, headRepo, user := e.Parser.ParseGitlabMergeCommentEvent(event) - command, err := e.Parser.DetermineCommand(event.ObjectAttributes.Note, vcs.Gitlab) - if err != nil { - e.respond(w, logging.Debug, http.StatusOK, "Ignoring: %s", err) + e.handleCommentEvent(w, baseRepo, headRepo, user, event.MergeRequest.IID, event.ObjectAttributes.Note, vcs.Gitlab) +} + +func (e *EventsController) handleCommentEvent(w http.ResponseWriter, baseRepo models.Repo, headRepo models.Repo, user models.User, pullNum int, comment string, vcsHost vcs.Host) { + parseResult := e.CommentParser.Parse(comment, vcsHost) + if parseResult.Ignore { + truncated := comment + if len(truncated) > 40 { + truncated = comment[:40] + "..." + } + e.respond(w, logging.Debug, http.StatusOK, "Ignoring non-command comment: %q", truncated) + return + } + + // If the command isn't valid or doesn't require processing, ex. + // "atlantis help" then we just comment back immediately. + // We do this here rather than earlier because we need access to the pull + // variable to comment back on the pull request. + if parseResult.CommentResponse != "" { + if err := e.VCSClient.CreateComment(baseRepo, pullNum, parseResult.CommentResponse, vcsHost); err != nil { + e.Logger.Err("unable to comment on pull request: %s", err) + } + e.respond(w, logging.Info, http.StatusOK, "Commenting back on pull request") return } @@ -162,7 +178,7 @@ func (e *EventsController) HandleGitlabCommentEvent(w http.ResponseWriter, event // We use a goroutine so that this function returns and the connection is // closed. fmt.Fprintln(w, "Processing...") - go e.CommandRunner.ExecuteCommand(baseRepo, headRepo, user, event.MergeRequest.IID, command, vcs.Gitlab) + go e.CommandRunner.ExecuteCommand(baseRepo, headRepo, user, pullNum, parseResult.Command, vcsHost) } // HandleGitlabMergeRequestEvent will delete any locks associated with the pull diff --git a/server/events_controller_test.go b/server/events_controller_test.go index e16a1e7f11..5179ed59d0 100644 --- a/server/events_controller_test.go +++ b/server/events_controller_test.go @@ -16,6 +16,7 @@ import ( "github.com/runatlantis/atlantis/server/events/mocks/matchers" "github.com/runatlantis/atlantis/server/events/models" "github.com/runatlantis/atlantis/server/events/vcs" + vcsmocks "github.com/runatlantis/atlantis/server/events/vcs/mocks" "github.com/runatlantis/atlantis/server/logging" "github.com/runatlantis/atlantis/server/mocks" ) @@ -28,7 +29,7 @@ var eventsReq *http.Request func TestPost_NotGithubOrGitlab(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() e.Post(w, eventsReq) responseContains(t, w, http.StatusBadRequest, "Ignoring request") @@ -36,7 +37,7 @@ func TestPost_NotGithubOrGitlab(t *testing.T) { func TestPost_UnsupportedVCSGithub(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 eventsReq.Header.Set(githubHeader, "value") w := httptest.NewRecorder() @@ -46,7 +47,7 @@ func TestPost_UnsupportedVCSGithub(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 eventsReq.Header.Set(gitlabHeader, "value") w := httptest.NewRecorder() @@ -56,7 +57,7 @@ func TestPost_UnsupportedVCSGitlab(t *testing.T) { func TestPost_InvalidGithubSecret(t *testing.T) { t.Log("when the github payload can't be validated a 400 is returned") - e, v, _, _, _, _ := setup(t) + e, v, _, _, _, _, _, _ := setup(t) w := httptest.NewRecorder() eventsReq.Header.Set(githubHeader, "value") When(v.Validate(eventsReq, secret)).ThenReturn(nil, errors.New("err")) @@ -66,7 +67,7 @@ func TestPost_InvalidGithubSecret(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() eventsReq.Header.Set(gitlabHeader, "value") When(gl.Validate(eventsReq, secret)).ThenReturn(nil, errors.New("err")) @@ -76,7 +77,7 @@ func TestPost_InvalidGitlabSecret(t *testing.T) { func TestPost_UnsupportedGithubEvent(t *testing.T) { t.Log("when the event type is an unsupported github event we ignore it") - e, v, _, _, _, _ := setup(t) + e, v, _, _, _, _, _, _ := setup(t) w := httptest.NewRecorder() eventsReq.Header.Set(githubHeader, "value") When(v.Validate(eventsReq, nil)).ThenReturn([]byte(`{"not an event": ""}`), nil) @@ -86,7 +87,7 @@ func TestPost_UnsupportedGithubEvent(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() eventsReq.Header.Set(gitlabHeader, "value") When(gl.Validate(eventsReq, secret)).ThenReturn([]byte(`{"not an event": ""}`), nil) @@ -96,7 +97,7 @@ func TestPost_UnsupportedGitlabEvent(t *testing.T) { func TestPost_GithubCommentNotCreated(t *testing.T) { t.Log("when the event is a github comment but it's not a created event we ignore it") - e, v, _, _, _, _ := setup(t) + e, v, _, _, _, _, _, _ := setup(t) eventsReq.Header.Set(githubHeader, "issue_comment") // comment action is deleted, not created event := `{"action": "deleted"}` @@ -108,7 +109,7 @@ func TestPost_GithubCommentNotCreated(t *testing.T) { func TestPost_GithubInvalidComment(t *testing.T) { t.Log("when the event is a github comment without all expected data we return a 400") - e, v, _, p, _, _ := setup(t) + e, v, _, p, _, _, _, _ := setup(t) eventsReq.Header.Set(githubHeader, "issue_comment") event := `{"action": "created"}` When(v.Validate(eventsReq, secret)).ThenReturn([]byte(event), nil) @@ -120,31 +121,60 @@ func TestPost_GithubInvalidComment(t *testing.T) { func TestPost_GitlabCommentInvalidCommand(t *testing.T) { t.Log("when the event is a gitlab comment with an invalid command we ignore it") - e, _, gl, p, _, _ := setup(t) + e, _, gl, _, _, _, _, cp := setup(t) eventsReq.Header.Set(gitlabHeader, "value") When(gl.Validate(eventsReq, secret)).ThenReturn(gitlab.MergeCommentEvent{}, nil) - When(p.DetermineCommand("", vcs.Gitlab)).ThenReturn(nil, errors.New("err")) + When(cp.Parse("", vcs.Gitlab)).ThenReturn(events.CommentParseResult{Ignore: true}) w := httptest.NewRecorder() e.Post(w, eventsReq) - responseContains(t, w, http.StatusOK, "Ignoring: err") + responseContains(t, w, http.StatusOK, "Ignoring non-command comment: \"\"") } func TestPost_GithubCommentInvalidCommand(t *testing.T) { t.Log("when the event is a github comment with an invalid command we ignore it") - e, v, _, p, _, _ := setup(t) + e, v, _, p, _, _, _, cp := setup(t) eventsReq.Header.Set(githubHeader, "issue_comment") event := `{"action": "created"}` When(v.Validate(eventsReq, secret)).ThenReturn([]byte(event), nil) When(p.ParseGithubIssueCommentEvent(matchers.AnyPtrToGithubIssueCommentEvent())).ThenReturn(models.Repo{}, models.User{}, 1, nil) - When(p.DetermineCommand("", vcs.Github)).ThenReturn(nil, errors.New("err")) + When(cp.Parse("", vcs.Github)).ThenReturn(events.CommentParseResult{Ignore: true}) w := httptest.NewRecorder() e.Post(w, eventsReq) - responseContains(t, w, http.StatusOK, "Ignoring: err") + responseContains(t, w, http.StatusOK, "Ignoring non-command comment: \"\"") +} + +func TestPost_GitlabCommentResponse(t *testing.T) { + t.Log("when the event is a gitlab comment that warrants a comment response we comment back") + e, _, gl, _, _, _, vcsClient, cp := setup(t) + eventsReq.Header.Set(gitlabHeader, "value") + When(gl.Validate(eventsReq, secret)).ThenReturn(gitlab.MergeCommentEvent{}, nil) + When(cp.Parse("", vcs.Gitlab)).ThenReturn(events.CommentParseResult{CommentResponse: "a comment"}) + w := httptest.NewRecorder() + e.Post(w, eventsReq) + vcsClient.VerifyWasCalledOnce().CreateComment(models.Repo{}, 0, "a comment", vcs.Gitlab) + responseContains(t, w, http.StatusOK, "Commenting back on pull request") +} + +func TestPost_GithubCommentResponse(t *testing.T) { + t.Log("when the event is a github comment that warrants a comment response we comment back") + e, v, _, p, _, _, vcsClient, cp := setup(t) + eventsReq.Header.Set(githubHeader, "issue_comment") + event := `{"action": "created"}` + When(v.Validate(eventsReq, secret)).ThenReturn([]byte(event), nil) + baseRepo := models.Repo{} + user := models.User{} + When(p.ParseGithubIssueCommentEvent(matchers.AnyPtrToGithubIssueCommentEvent())).ThenReturn(baseRepo, user, 1, nil) + When(cp.Parse("", vcs.Github)).ThenReturn(events.CommentParseResult{CommentResponse: "a comment"}) + w := httptest.NewRecorder() + + e.Post(w, eventsReq) + vcsClient.VerifyWasCalledOnce().CreateComment(baseRepo, 1, "a comment", vcs.Github) + responseContains(t, w, http.StatusOK, "Commenting back on pull request") } 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, _, cr, _ := setup(t) + e, _, gl, _, cr, _, _, _ := setup(t) eventsReq.Header.Set(gitlabHeader, "value") When(gl.Validate(eventsReq, secret)).ThenReturn(gitlab.MergeCommentEvent{}, nil) w := httptest.NewRecorder() @@ -158,7 +188,7 @@ func TestPost_GitlabCommentSuccess(t *testing.T) { func TestPost_GithubCommentSuccess(t *testing.T) { t.Log("when the event is a github comment with a valid command we call the command handler") - e, v, _, p, cr, _ := setup(t) + e, v, _, p, cr, _, _, cp := setup(t) eventsReq.Header.Set(githubHeader, "issue_comment") event := `{"action": "created"}` When(v.Validate(eventsReq, secret)).ThenReturn([]byte(event), nil) @@ -166,7 +196,7 @@ func TestPost_GithubCommentSuccess(t *testing.T) { user := models.User{} cmd := events.Command{} When(p.ParseGithubIssueCommentEvent(matchers.AnyPtrToGithubIssueCommentEvent())).ThenReturn(baseRepo, user, 1, nil) - When(p.DetermineCommand("", vcs.Github)).ThenReturn(&cmd, nil) + When(cp.Parse("", vcs.Github)).ThenReturn(events.CommentParseResult{Command: &cmd}) w := httptest.NewRecorder() e.Post(w, eventsReq) responseContains(t, w, http.StatusOK, "Processing...") @@ -178,7 +208,7 @@ func TestPost_GithubCommentSuccess(t *testing.T) { func TestPost_GithubPullRequestNotClosed(t *testing.T) { t.Log("when the event is a github pull reuqest but it's not a closed event we ignore it") - e, v, _, _, _, _ := setup(t) + e, v, _, _, _, _, _, _ := setup(t) eventsReq.Header.Set(githubHeader, "pull_request") event := `{"action": "opened"}` When(v.Validate(eventsReq, secret)).ThenReturn([]byte(event), nil) @@ -189,7 +219,7 @@ func TestPost_GithubPullRequestNotClosed(t *testing.T) { func TestPost_GitlabMergeRequestNotClosed(t *testing.T) { t.Log("when the event is a gitlab merge request but it's not a closed event we ignore it") - e, _, gl, p, _, _ := setup(t) + e, _, gl, p, _, _, _, _ := setup(t) eventsReq.Header.Set(gitlabHeader, "value") event := gitlab.MergeEvent{} When(gl.Validate(eventsReq, secret)).ThenReturn(event, nil) @@ -201,7 +231,7 @@ func TestPost_GitlabMergeRequestNotClosed(t *testing.T) { func TestPost_GithubPullRequestInvalid(t *testing.T) { t.Log("when the event is a github pull request with invalid data we return a 400") - e, v, _, p, _, _ := setup(t) + e, v, _, p, _, _, _, _ := setup(t) eventsReq.Header.Set(githubHeader, "pull_request") event := `{"action": "closed"}` @@ -214,7 +244,7 @@ func TestPost_GithubPullRequestInvalid(t *testing.T) { func TestPost_GithubPullRequestInvalidRepo(t *testing.T) { t.Log("when the event is a github pull request with invalid repo data we return a 400") - e, v, _, p, _, _ := setup(t) + e, v, _, p, _, _, _, _ := setup(t) eventsReq.Header.Set(githubHeader, "pull_request") event := `{"action": "closed"}` @@ -229,7 +259,7 @@ func TestPost_GithubPullRequestInvalidRepo(t *testing.T) { func TestPost_GithubPullRequestErrCleaningPull(t *testing.T) { t.Log("when the event is a pull request and we have an error calling CleanUpPull we return a 503") RegisterMockTestingT(t) - e, v, _, p, _, c := setup(t) + e, v, _, p, _, c, _, _ := setup(t) eventsReq.Header.Set(githubHeader, "pull_request") event := `{"action": "closed"}` @@ -246,7 +276,7 @@ func TestPost_GithubPullRequestErrCleaningPull(t *testing.T) { func TestPost_GitlabMergeRequestErrCleaningPull(t *testing.T) { t.Log("when the event is a gitlab merge request and an error occurs calling CleanUpPull we return a 503") - e, _, gl, p, _, c := setup(t) + e, _, gl, p, _, c, _, _ := setup(t) eventsReq.Header.Set(gitlabHeader, "value") event := gitlab.MergeEvent{} When(gl.Validate(eventsReq, secret)).ThenReturn(event, nil) @@ -261,7 +291,7 @@ func TestPost_GitlabMergeRequestErrCleaningPull(t *testing.T) { func TestPost_GithubPullRequestSuccess(t *testing.T) { t.Log("when the event is a pull request and everything works we return a 200") - e, v, _, p, _, c := setup(t) + e, v, _, p, _, c, _, _ := setup(t) eventsReq.Header.Set(githubHeader, "pull_request") event := `{"action": "closed"}` @@ -278,7 +308,7 @@ func TestPost_GithubPullRequestSuccess(t *testing.T) { func TestPost_GitlabMergeRequestSuccess(t *testing.T) { 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) eventsReq.Header.Set(gitlabHeader, "value") event := gitlab.MergeEvent{} When(gl.Validate(eventsReq, secret)).ThenReturn(event, nil) @@ -290,24 +320,28 @@ func TestPost_GitlabMergeRequestSuccess(t *testing.T) { responseContains(t, w, http.StatusOK, "Merge request cleaned successfully") } -func setup(t *testing.T) (server.EventsController, *mocks.MockGithubRequestValidator, *mocks.MockGitlabRequestParser, *emocks.MockEventParsing, *emocks.MockCommandRunner, *emocks.MockPullCleaner) { +func setup(t *testing.T) (server.EventsController, *mocks.MockGithubRequestValidator, *mocks.MockGitlabRequestParser, *emocks.MockEventParsing, *emocks.MockCommandRunner, *emocks.MockPullCleaner, *vcsmocks.MockClientProxy, *emocks.MockCommentParsing) { RegisterMockTestingT(t) eventsReq, _ = http.NewRequest("GET", "", bytes.NewBuffer(nil)) v := mocks.NewMockGithubRequestValidator() gl := mocks.NewMockGitlabRequestParser() p := emocks.NewMockEventParsing() + cp := emocks.NewMockCommentParsing() cr := emocks.NewMockCommandRunner() c := emocks.NewMockPullCleaner() + vcsmock := vcsmocks.NewMockClientProxy() e := server.EventsController{ Logger: logging.NewNoopLogger(), GithubRequestValidator: v, Parser: p, + CommentParser: cp, CommandRunner: cr, PullCleaner: c, GithubWebHookSecret: secret, SupportedVCSHosts: []vcs.Host{vcs.Github, vcs.Gitlab}, GitlabWebHookSecret: secret, GitlabRequestParser: gl, + VCSClient: vcsmock, } - return e, v, gl, p, cr, c + return e, v, gl, p, cr, c, vcsmock, cp } diff --git a/server/server.go b/server/server.go index 9302438792..0ed76c360a 100644 --- a/server/server.go +++ b/server/server.go @@ -184,7 +184,6 @@ func NewServer(config Config) (*Server, error) { Locker: lockingClient, ProjectFinder: &events.DefaultProjectFinder{}, } - helpExecutor := &events.HelpExecutor{} pullClosedExecutor := &events.PullClosedExecutor{ VCSClient: vcsClient, Locker: lockingClient, @@ -197,10 +196,15 @@ func NewServer(config Config) (*Server, error) { GitlabUser: config.GitlabUser, GitlabToken: config.GitlabToken, } + commentParser := &events.CommentParser{ + GithubUser: config.GithubUser, + GithubToken: config.GithubToken, + GitlabUser: config.GitlabUser, + GitlabToken: config.GitlabToken, + } commandHandler := &events.CommandHandler{ ApplyExecutor: applyExecutor, PlanExecutor: planExecutor, - HelpExecutor: helpExecutor, LockURLGenerator: planExecutor, EventParser: eventParser, VCSClient: vcsClient, @@ -215,12 +219,14 @@ func NewServer(config Config) (*Server, error) { CommandRunner: commandHandler, PullCleaner: pullClosedExecutor, Parser: eventParser, + CommentParser: commentParser, Logger: logger, GithubWebHookSecret: []byte(config.GithubWebHookSecret), GithubRequestValidator: &DefaultGithubRequestValidator{}, GitlabRequestParser: &DefaultGitlabRequestParser{}, GitlabWebHookSecret: []byte(config.GitlabWebHookSecret), SupportedVCSHosts: supportedVCSHosts, + VCSClient: vcsClient, } router := mux.NewRouter() return &Server{