New parsing of comments. Accept all flags

Author: lkysow

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)}

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) {

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
+}

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
+}
+

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")

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 {

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
 		}
 	}