From 8ee16a0281b0180c1ca9986a67a99d9c3cb98bd8 Mon Sep 17 00:00:00 2001 From: Luke Kysow Date: Wed, 21 Mar 2018 17:12:17 -0700 Subject: [PATCH] By default atlantis plan will run in root dir. This change makes things cleaner because there will only ever be one plan generated in the comment flow. It's okay to make this change because typing the plan command a couple times isn't too bad AND we're going to be implementing autoplanning which should handle most of the times you needed to type plan from before. --- server/events/comment_parser.go | 13 ++++---- server/events/comment_parser_test.go | 25 ++++++--------- server/events/event_parser.go | 33 +++++++++++++++---- server/events/event_parser_test.go | 48 ++++++++++++++++++++++++++++ 4 files changed, 91 insertions(+), 28 deletions(-) diff --git a/server/events/comment_parser.go b/server/events/comment_parser.go index d0e921d79..2c9e1b0c1 100644 --- a/server/events/comment_parser.go +++ b/server/events/comment_parser.go @@ -31,6 +31,8 @@ const ( DirFlagShort = "d" VerboseFlagLong = "verbose" VerboseFlagShort = "" + DefaultWorkspace = "default" + DefaultDir = "." ) //go:generate pegomock generate -m --use-experimental-model-gen --package mocks -o mocks/mock_comment_parsing.go CommentParsing @@ -135,21 +137,20 @@ func (e *CommentParser) Parse(comment string, vcsHost models.VCSHostType) Commen var name CommandName // Set up the flag parsing depending on the command. - const defaultWorkspace = "default" switch command { case Plan.String(): name = Plan flagSet = pflag.NewFlagSet(Plan.String(), pflag.ContinueOnError) flagSet.SetOutput(ioutil.Discard) - flagSet.StringVarP(&workspace, WorkspaceFlagLong, WorkspaceFlagShort, defaultWorkspace, "Switch to this Terraform workspace before planning.") - flagSet.StringVarP(&dir, DirFlagLong, DirFlagShort, "", "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.StringVarP(&workspace, WorkspaceFlagLong, WorkspaceFlagShort, DefaultWorkspace, "Switch to this Terraform workspace before planning.") + flagSet.StringVarP(&dir, DirFlagLong, DirFlagShort, DefaultDir, "Which directory to run plan in relative to root of repo, ex. 'child/dir'.") flagSet.BoolVarP(&verbose, VerboseFlagLong, VerboseFlagShort, false, "Append Atlantis log to comment.") case Apply.String(): name = Apply flagSet = pflag.NewFlagSet(Apply.String(), pflag.ContinueOnError) flagSet.SetOutput(ioutil.Discard) - flagSet.StringVarP(&workspace, WorkspaceFlagLong, WorkspaceFlagShort, defaultWorkspace, "Apply the plan for this Terraform workspace.") - flagSet.StringVarP(&dir, DirFlagLong, DirFlagShort, "", "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.StringVarP(&workspace, WorkspaceFlagLong, WorkspaceFlagShort, DefaultWorkspace, "Apply the plan for this Terraform workspace.") + flagSet.StringVarP(&dir, DirFlagLong, DirFlagShort, DefaultDir, "Apply the plan for this directory, relative to root of repo, ex. 'child/dir'.") flagSet.BoolVarP(&verbose, VerboseFlagLong, VerboseFlagShort, false, "Append Atlantis log to comment.") default: return CommentParseResult{CommentResponse: fmt.Sprintf("Error: unknown command %q – this is a bug", command)} @@ -198,7 +199,7 @@ func (e *CommentParser) Parse(comment string, vcsHost models.VCSHostType) Commen } return CommentParseResult{ - Command: &Command{Name: name, Verbose: verbose, Workspace: workspace, Dir: dir, Flags: extraArgs}, + Command: NewCommand(dir, extraArgs, name, verbose, workspace), } } diff --git a/server/events/comment_parser_test.go b/server/events/comment_parser_test.go index 87b14f358..81fe4da9f 100644 --- a/server/events/comment_parser_test.go +++ b/server/events/comment_parser_test.go @@ -31,8 +31,6 @@ var commentParser = events.CommentParser{ } 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", @@ -47,8 +45,6 @@ func TestParse_Ignored(t *testing.T) { } 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", @@ -271,7 +267,7 @@ func TestParse_Parsing(t *testing.T) { { "", "default", - "", + ".", false, "", }, @@ -279,7 +275,7 @@ func TestParse_Parsing(t *testing.T) { { "-w workspace", "workspace", - "", + ".", false, "", }, @@ -293,7 +289,7 @@ func TestParse_Parsing(t *testing.T) { { "--verbose", "default", - "", + ".", true, "", }, @@ -330,7 +326,7 @@ func TestParse_Parsing(t *testing.T) { { "-w workspace -- -d dir --verbose", "workspace", - "", + ".", false, "\"-d\" \"dir\" \"--verbose\"", }, @@ -338,7 +334,7 @@ func TestParse_Parsing(t *testing.T) { { "--", "default", - "", + ".", false, "", }, @@ -346,7 +342,7 @@ func TestParse_Parsing(t *testing.T) { { "-- \";echo \"hi", "default", - "", + ".", false, `"\";echo" "\"hi"`, }, @@ -430,10 +426,8 @@ func TestParse_Parsing(t *testing.T) { } var PlanUsage = `Usage of plan: - -d, --dir string 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. + -d, --dir string Which directory to run plan in relative to root of repo, + ex. 'child/dir'. (default ".") --verbose Append Atlantis log to comment. -w, --workspace string Switch to this Terraform workspace before planning. (default "default") @@ -441,8 +435,7 @@ var PlanUsage = `Usage of plan: var ApplyUsage = `Usage of apply: -d, --dir string 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. + repo, ex. 'child/dir'. (default ".") --verbose Append Atlantis log to comment. -w, --workspace string Apply the plan for this Terraform workspace. (default "default") diff --git a/server/events/event_parser.go b/server/events/event_parser.go index d42e4beda..cd3e36f5b 100644 --- a/server/events/event_parser.go +++ b/server/events/event_parser.go @@ -14,6 +14,7 @@ package events import ( + "path" "regexp" "github.com/google/go-github/github" @@ -32,14 +33,34 @@ var multiLineRegex = regexp.MustCompile(`.*\r?\n.+`) //go:generate pegomock generate -m --use-experimental-model-gen --package mocks -o mocks/mock_event_parsing.go EventParsing type Command struct { - Name CommandName - Workspace string - Verbose bool - Flags []string // Dir is the path relative to the repo root to run the command in. - // If empty string then it wasn't specified. "." is the root of the repo. - // Dir will never end in "/". + // Will never be an empty string and will never end in "/". Dir string + // Flags are the extra arguments appended to comment, + // ex. atlantis plan -- -target=resource + Flags []string + Name CommandName + Verbose bool + Workspace string +} + +// NewCommand constructs a Command, setting all missing fields to defaults. +func NewCommand(dir string, flags []string, name CommandName, verbose bool, workspace string) *Command { + // If dir was an empty string, this will return '.'. + validDir := path.Clean(dir) + if validDir == "/" { + validDir = "." + } + if workspace == "" { + workspace = DefaultWorkspace + } + return &Command{ + Dir: validDir, + Flags: flags, + Name: name, + Verbose: verbose, + Workspace: workspace, + } } type EventParsing interface { diff --git a/server/events/event_parser_test.go b/server/events/event_parser_test.go index 9928dc69c..dbd4b515e 100644 --- a/server/events/event_parser_test.go +++ b/server/events/event_parser_test.go @@ -257,6 +257,54 @@ func TestParseGitlabMergeCommentEvent(t *testing.T) { }, user) } +func TestNewCommand_CleansDir(t *testing.T) { + cases := []struct { + Dir string + ExpDir string + }{ + { + "", + ".", + }, + { + "/", + ".", + }, + { + "./", + ".", + }, + // We rely on our callers to not pass in relative dirs. + { + "..", + "..", + }, + } + + for _, c := range cases { + t.Run(c.Dir, func(t *testing.T) { + cmd := events.NewCommand(c.Dir, nil, events.Plan, false, "workspace") + Equals(t, c.ExpDir, cmd.Dir) + }) + } +} + +func TestNewCommand_EmptyWorkspace(t *testing.T) { + cmd := events.NewCommand("dir", nil, events.Plan, false, "") + Equals(t, "default", cmd.Workspace) +} + +func TestNewCommand_AllFieldsSet(t *testing.T) { + cmd := events.NewCommand("dir", []string{"a", "b"}, events.Plan, true, "workspace") + Equals(t, events.Command{ + Workspace: "workspace", + Dir: "dir", + Verbose: true, + Flags: []string{"a", "b"}, + Name: events.Plan, + }, *cmd) +} + var mergeEventJSON = `{ "object_kind": "merge_request", "user": {