From aa99adb27c08ef95f06063ff223c1298008379c8 Mon Sep 17 00:00:00 2001 From: Luke Kysow Date: Fri, 18 Aug 2017 23:14:00 -0700 Subject: [PATCH] New parsing of comments. Accept all flags --- server/apply_executor.go | 2 +- server/command_handler.go | 5 +- server/event_parser.go | 100 +++++++++++++++++++++-------------- server/event_parser_test.go | 67 ++++++++++++++++------- server/plan_executor.go | 3 +- server/server.go | 1 - testing_util/testing_util.go | 4 +- 7 files changed, 115 insertions(+), 67 deletions(-) diff --git a/server/apply_executor.go b/server/apply_executor.go index 73a91de1..9dc4fb30 100644 --- a/server/apply_executor.go +++ b/server/apply_executor.go @@ -145,7 +145,7 @@ func (a *ApplyExecutor) apply(ctx *CommandContext, repoDir string, plan models.P } } - tfApplyCmd := append([]string{"apply", "-no-color", plan.LocalPath}, applyExtraArgs...) + tfApplyCmd := append(append([]string{"apply", "-no-color", plan.LocalPath}, applyExtraArgs...), ctx.Command.Flags...) output, err := a.terraform.RunCommandWithVersion(ctx.Log, absolutePath, tfApplyCmd, terraformVersion) if err != nil { return ProjectResult{Error: fmt.Errorf("%s\n%s", err.Error(), output)} diff --git a/server/command_handler.go b/server/command_handler.go index 0c0c4124..528428bb 100644 --- a/server/command_handler.go +++ b/server/command_handler.go @@ -64,9 +64,10 @@ func (c CommandName) String() string { } type Command struct { - Verbose bool - Environment string Name CommandName + Environment string + Verbose bool + Flags []string } func (c *CommandHandler) ExecuteCommand(ctx *CommandContext) { diff --git a/server/event_parser.go b/server/event_parser.go index e3e99f73..3afa09b3 100644 --- a/server/event_parser.go +++ b/server/event_parser.go @@ -3,7 +3,6 @@ package server import ( "errors" "fmt" - "regexp" "strings" "github.com/google/go-github/github" @@ -15,71 +14,71 @@ type EventParser struct { GithubToken string } +// DetermineCommand parses the comment as an atlantis command. If it succeeds, +// it returns the command. Otherwise it returns error. func (e *EventParser) DetermineCommand(comment *github.IssueCommentEvent) (*Command, error) { - // regex matches: + // 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 environment and an optional --verbose flag + // then an optional environment argument, an optional --verbose flag and any other flags // // examples: // atlantis help // run plan // @GithubUser plan staging // atlantis plan staging --verbose - // - // capture groups: - // 1: the command, i.e. plan/apply/help - // 2: the environment OR the --verbose flag (if they didn't specify and environment) - // 3: the --verbose flag (if they specified an environment) - atlantisCommentRegex := `^(?:run|atlantis|@` + e.GithubUser + `)[[:blank:]]+(plan|apply|help)(?:[[:blank:]]+([a-zA-Z0-9_-]+))?[[:blank:]]*(--verbose)?$` - runPlanMatcher := regexp.MustCompile(atlantisCommentRegex) - + // atlantis plan staging --verbose -key=value -key2 value2 commentBody := comment.Comment.GetBody() if commentBody == "" { return nil, errors.New("comment.body is null") } - - // extract the command and environment. ex. for "atlantis plan staging", the command is "plan", and the environment is "staging" - match := runPlanMatcher.FindStringSubmatch(commentBody) - if len(match) < 4 { - var truncated = commentBody - if len(truncated) > 30 { - truncated = truncated[0:30] + "..." - } - return nil, errors.New("not an Atlantis command") - } - - // depending on the comment, the command/env/verbose may be in different matching groups - // if there is no env (ex. just atlantis plan --verbose) then, verbose would be in the 2nd group - // if there is an env, then verbose would be in the 3rd - command := match[1] - env := match[2] - verboseFlag := match[3] - if verboseFlag == "" && env == "--verbose" { - verboseFlag = env - env = "" + err := errors.New("not an Atlantis command") + args := strings.Fields(commentBody) + if len(args) < 2 { + return nil, err } - // now we're ready to actually look at the values + env := "default" verbose := false - if verboseFlag == "--verbose" { - verbose = true + var flags []string + + if !e.stringInSlice(args[0], []string{"run", "atlantis", "@" + e.GithubUser}) { + return nil, err + } + if !e.stringInSlice(args[1], []string{"plan", "apply", "help"}) { + return nil, err + } + if args[1] == "help" { + return &Command{Name: Help}, nil } - // if env not specified, use terraform's default - if env == "" { - env = "default" + command := args[1] + + if len(args) > 2 { + flags = args[2:] + + // if the third arg doesn't start with '-' then we assume it's an + // environment not a flag + if !strings.HasPrefix(args[2], "-") { + env = args[2] + flags = args[3:] + } + + // check for --verbose specially and then remove any additional + // occurrences + if e.stringInSlice("--verbose", flags) { + verbose = true + flags = e.removeOccurrences("--verbose", flags) + } } - c := &Command{Verbose: verbose, Environment: env} + c := &Command{Verbose: verbose, Environment: env, Flags: flags} switch command { case "plan": c.Name = Plan case "apply": c.Name = Apply - case "help": - c.Name = Help default: - return nil, fmt.Errorf("something went wrong with our regex, the command we parsed %q was not apply or plan", match[1]) + return nil, fmt.Errorf("something went wrong parsing the command, the command we parsed %q was not apply or plan", command) } return c, nil } @@ -189,3 +188,22 @@ func (e *EventParser) ExtractRepoData(ghRepo *github.Repository) (models.Repo, e Name: repoName, }, nil } + +func (e *EventParser) stringInSlice(a string, list []string) bool { + for _, b := range list { + if b == a { + return true + } + } + return false +} + +func (e *EventParser) removeOccurrences(a string, list []string) []string { + var out []string + for _, b := range list { + if b != a { + out = append(out, b) + } + } + return out +} diff --git a/server/event_parser_test.go b/server/event_parser_test.go index dde417df..9aeaa3e2 100644 --- a/server/event_parser_test.go +++ b/server/event_parser_test.go @@ -1,12 +1,12 @@ package server_test import ( - "fmt" "testing" "github.com/google/go-github/github" "github.com/hootsuite/atlantis/server" . "github.com/hootsuite/atlantis/testing_util" + "strings" ) func TestDetermineCommandInvalid(t *testing.T) { @@ -22,8 +22,6 @@ func TestDetermineCommandInvalid(t *testing.T) { "atlantis slkjd", "@user slkjd", "atlantis plans", - // whitespace - " atlantis plan", // misc "related comment mentioning atlantis", } @@ -55,29 +53,50 @@ func TestDetermineCommandPermutations(t *testing.T) { execNames := []string{"run", "atlantis", "@user"} commandNames := []server.CommandName{server.Plan, server.Apply} envs := []string{"", "default", "env", "env-dash", "env_underscore", "camelEnv"} - verboses := []bool{true, false} + flagCases := [][]string{ + []string{}, + []string{"--verbose"}, + []string{"-key=value"}, + []string{"-key", "value"}, + []string{"-key1=value1", "-key2=value2"}, + []string{"-key1=value1", "-key2", "value2"}, + []string{"-key1", "value1", "-key2=value2"}, + []string{"--verbose", "key2=value2"}, + []string{"-key1=value1", "--verbose"}, + } // test all permutations for _, exec := range execNames { for _, name := range commandNames { for _, env := range envs { - for _, v := range verboses { - vFlag := "" - if v == true { - vFlag = "--verbose" - } + for _, flags := range flagCases { + // If github comments end in a newline they get \r\n appended. + // Ensure that we parse commands properly either way. + for _, lineEnding := range []string{"", "\r\n"} { + comment := strings.Join(append([]string{exec, name.String(), env}, flags...), " ") + lineEnding + t.Log("testing comment: " + comment) + c, err := e.DetermineCommand(buildComment(comment)) + Ok(t, err) + Equals(t, name, c.Name) + if env == "" { + Equals(t, "default", c.Environment) + } else { + Equals(t, env, c.Environment) + } + Equals(t, stringInSlice("--verbose", flags), c.Verbose) - comment := fmt.Sprintf("%s %s %s %s", exec, name.String(), env, vFlag) - t.Log("testing comment: " + comment) - c, err := e.DetermineCommand(buildComment(comment)) - Ok(t, err) - Equals(t, name, c.Name) - if env == "" { - Equals(t, "default", c.Environment) - } else { - Equals(t, env, c.Environment) + // ensure --verbose never shows up in flags + for _, f := range c.Flags { + Assert(t, f != "--verbose", "Should not pass on the --verbose flag: %v", flags) + } + + // check all flags are present + for _, f := range flags { + if f != "--verbose" { + Contains(t, f, c.Flags) + } + } } - Equals(t, v, c.Verbose) } } } @@ -91,3 +110,13 @@ func buildComment(c string) *github.IssueCommentEvent { }, } } + +func stringInSlice(a string, list []string) bool { + for _, b := range list { + if b == a { + return true + } + } + return false +} + diff --git a/server/plan_executor.go b/server/plan_executor.go index f2a8a33a..5aaec536 100644 --- a/server/plan_executor.go +++ b/server/plan_executor.go @@ -148,7 +148,8 @@ func (p *PlanExecutor) plan(ctx *CommandContext, repoDir string, project models. // Run terraform plan planFile := filepath.Join(repoDir, project.Path, fmt.Sprintf("%s.tfplan", tfEnv)) - tfPlanCmd := append([]string{"plan", "-refresh", "-no-color", "-out", planFile, "-var", fmt.Sprintf("%s=%s", atlantisUserTFVar, ctx.User.Username)}, planExtraArgs...) + userVar := fmt.Sprintf("%s=%s", atlantisUserTFVar, ctx.User.Username) + tfPlanCmd := append(append([]string{"plan", "-refresh", "-no-color", "-out", planFile, "-var", userVar}, planExtraArgs...), ctx.Command.Flags...) // check if env/{environment}.tfvars exist tfEnvFileName := filepath.Join("env", tfEnv+".tfvars") diff --git a/server/server.go b/server/server.go index 3547307b..4e7a41fb 100644 --- a/server/server.go +++ b/server/server.go @@ -384,7 +384,6 @@ func (s *Server) handleCommentEvent(w http.ResponseWriter, event *gh.IssueCommen return } - // determine if the comment matches a plan or apply command ctx := &CommandContext{} command, err := s.eventParser.DetermineCommand(event) if err != nil { diff --git a/testing_util/testing_util.go b/testing_util/testing_util.go index 660c6d7b..51e34f5a 100644 --- a/testing_util/testing_util.go +++ b/testing_util/testing_util.go @@ -38,9 +38,9 @@ func Equals(tb testing.TB, exp, act interface{}) { } // Contains fails the test if the slice doesn't contain the expected element -func Contains(tb testing.TB, exp interface{}, slice []interface{}) { +func Contains(tb testing.TB, exp interface{}, slice []string) { for _, v := range slice { - if reflect.DeepEqual(v, exp) { + if v == exp { return } }