Rename DetermineCommand to Parse

2026-08-05 07:48:28 +00:00 · 2018-02-28 10:56:36 -08:00
parent 2af2f10cbc
commit a24d940f5d
5 changed files with 62 additions and 49 deletions
--- a/server/events/comment_parser.go
+++ b/server/events/comment_parser.go
@@ -12,10 +12,14 @@ import (

 //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 {
-	DetermineCommand(comment string, vcsHost vcs.Host) CommentParseResult
+	// 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
@@ -35,7 +39,7 @@ type CommentParseResult struct {
 	Ignore bool
 }

-// DetermineCommand parses the comment as an Atlantis command.
+// Parse parses the comment as an Atlantis command.
 //
 // Valid commands contain:
 // - The initial "executable" name, 'run' or 'atlantis' or '@GithubUser'
@@ -52,7 +56,7 @@ type CommentParseResult struct {
 // - atlantis plan --verbose -- -key=value -key2 value2
 //
 // nolint: gocyclo
-func (e *CommentParser) DetermineCommand(comment string, vcsHost vcs.Host) CommentParseResult {
+func (e *CommentParser) Parse(comment string, vcsHost vcs.Host) CommentParseResult {
 	if multiLineRegex.MatchString(comment) {
 		return CommentParseResult{Ignore: true}
 	}
@@ -137,6 +141,7 @@ func (e *CommentParser) DetermineCommand(comment string, vcsHost vcs.Host) Comme
 	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.
@@ -152,21 +157,11 @@ func (e *CommentParser) DetermineCommand(comment string, vcsHost vcs.Host) Comme
 		}
 	}

-	// 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 CommentParseResult{CommentResponse: fmt.Sprintf("Error: Using a relative path %q with -d/--dir is not allowed", dir)}
-		}
-
-		dir = validatedDir
+	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, "..") {
@@ -178,6 +173,24 @@ func (e *CommentParser) DetermineCommand(comment string, vcsHost vcs.Host) Comme
 	}
 }

+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 {
--- a/server/events/comment_parser_test.go
+++ b/server/events/comment_parser_test.go
@@ -17,7 +17,7 @@ var commentParser = events.CommentParser{
 	GitlabToken: "gitlab-token",
 }

-func TestDetermineCommand_Ignored(t *testing.T) {
+func TestParse_Ignored(t *testing.T) {
 	t.Log("given a comment that should be ignored we should set " +
 		"CommentParseResult.Ignore to true")
 	ignoreComments := []string{
@@ -28,12 +28,12 @@ func TestDetermineCommand_Ignored(t *testing.T) {
 		"terraform plan\nbut with newlines",
 	}
 	for _, c := range ignoreComments {
-		r := commentParser.DetermineCommand(c, vcs.Github)
+		r := commentParser.Parse(c, vcs.Github)
 		Assert(t, r.Ignore, "expected Ignore to be true for comment %q", c)
 	}
 }

-func TestDetermineCommand_HelpResponse(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{
@@ -47,12 +47,12 @@ func TestDetermineCommand_HelpResponse(t *testing.T) {
 		"atlantis help plan",
 	}
 	for _, c := range helpComments {
-		r := commentParser.DetermineCommand(c, vcs.Github)
+		r := commentParser.Parse(c, vcs.Github)
 		Equals(t, events.HelpComment, r.CommentResponse)
 	}
 }

-func TestDetermineCommand_DidYouMeanAtlantis(t *testing.T) {
+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{
@@ -65,13 +65,13 @@ func TestDetermineCommand_DidYouMeanAtlantis(t *testing.T) {
 		"terraform plan -w workspace -d . -- test",
 	}
 	for _, c := range comments {
-		r := commentParser.DetermineCommand(c, vcs.Github)
+		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 TestDetermineCommand_InvalidCommand(t *testing.T) {
+func TestParse_InvalidCommand(t *testing.T) {
 	t.Log("given a comment with an invalid atlantis command, should return " +
 		"a warning.")
 	comments := []string{
@@ -80,14 +80,14 @@ func TestDetermineCommand_InvalidCommand(t *testing.T) {
 		"atlantis appely apply",
 	}
 	for _, c := range comments {
-		r := commentParser.DetermineCommand(c, vcs.Github)
+		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 TestDetermineCommand_SubcommandUsage(t *testing.T) {
+func TestParse_SubcommandUsage(t *testing.T) {
 	t.Log("given a comment asking for the usage of a subcommand should " +
 		"return help")
 	comments := []string{
@@ -97,7 +97,7 @@ func TestDetermineCommand_SubcommandUsage(t *testing.T) {
 		"atlantis apply --help",
 	}
 	for _, c := range comments {
-		r := commentParser.DetermineCommand(c, vcs.Github)
+		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)
@@ -106,7 +106,7 @@ func TestDetermineCommand_SubcommandUsage(t *testing.T) {
 	}
 }

-func TestDetermineCommand_InvalidFlags(t *testing.T) {
+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 {
@@ -131,7 +131,7 @@ func TestDetermineCommand_InvalidFlags(t *testing.T) {
 		},
 	}
 	for _, c := range cases {
-		r := commentParser.DetermineCommand(c.comment, vcs.Github)
+		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 "),
@@ -139,7 +139,7 @@ func TestDetermineCommand_InvalidFlags(t *testing.T) {
 	}
 }

-func TestDetermineCommand_RelativeDirPath(t *testing.T) {
+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 ..",
@@ -153,14 +153,14 @@ func TestDetermineCommand_RelativeDirPath(t *testing.T) {
 		"atlantis apply -d a/../..",
 	}
 	for _, c := range comments {
-		r := commentParser.DetermineCommand(c, vcs.Github)
+		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 TestDetermineCommand_InvalidWorkspace(t *testing.T) {
+func TestParse_InvalidWorkspace(t *testing.T) {
 	t.Log("if -w is used with '..', should return an error")
 	comments := []string{
 		"atlantis plan -w ..",
@@ -171,14 +171,14 @@ func TestDetermineCommand_InvalidWorkspace(t *testing.T) {
 		"atlantis apply -w ../../../etc/passwd",
 	}
 	for _, c := range comments {
-		r := commentParser.DetermineCommand(c, vcs.Github)
+		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 TestDetermineCommand_Parsing(t *testing.T) {
+func TestParse_Parsing(t *testing.T) {
 	cases := []struct {
 		flags        string
 		expWorkspace string
@@ -346,7 +346,7 @@ func TestDetermineCommand_Parsing(t *testing.T) {
 	for _, test := range cases {
 		for _, cmdName := range []string{"plan", "apply"} {
 			comment := fmt.Sprintf("atlantis %s %s", cmdName, test.flags)
-			r := commentParser.DetermineCommand(comment, vcs.Github)
+			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)
--- a/server/events/mocks/mock_comment_parsing.go
+++ b/server/events/mocks/mock_comment_parsing.go
@@ -19,9 +19,9 @@ func NewMockCommentParsing() *MockCommentParsing {
 	return &MockCommentParsing{fail: pegomock.GlobalFailHandler}
 }

-func (mock *MockCommentParsing) DetermineCommand(comment string, vcsHost vcs.Host) events.CommentParseResult {
+func (mock *MockCommentParsing) Parse(comment string, vcsHost vcs.Host) events.CommentParseResult {
 	params := []pegomock.Param{comment, vcsHost}
-	result := pegomock.GetGenericMockFrom(mock).Invoke("DetermineCommand", params, []reflect.Type{reflect.TypeOf((*events.CommentParseResult)(nil)).Elem()})
+	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 {
@@ -49,23 +49,23 @@ type VerifierCommentParsing struct {
 	inOrderContext         *pegomock.InOrderContext
 }

-func (verifier *VerifierCommentParsing) DetermineCommand(comment string, vcsHost vcs.Host) *CommentParsing_DetermineCommand_OngoingVerification {
+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, "DetermineCommand", params)
-	return &CommentParsing_DetermineCommand_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations}
+	methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "Parse", params)
+	return &CommentParsing_Parse_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations}
 }

-type CommentParsing_DetermineCommand_OngoingVerification struct {
+type CommentParsing_Parse_OngoingVerification struct {
 	mock              *MockCommentParsing
 	methodInvocations []pegomock.MethodInvocation
 }

-func (c *CommentParsing_DetermineCommand_OngoingVerification) GetCapturedArguments() (string, vcs.Host) {
+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_DetermineCommand_OngoingVerification) GetAllCapturedArguments() (_param0 []string, _param1 []vcs.Host) {
+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]))
--- a/server/events_controller.go
+++ b/server/events_controller.go
@@ -152,7 +152,7 @@ func (e *EventsController) HandleGitlabCommentEvent(w http.ResponseWriter, event
 }

 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.DetermineCommand(comment, vcsHost)
+	parseResult := e.CommentParser.Parse(comment, vcsHost)
 	if parseResult.Ignore {
 		truncated := comment
 		if len(truncated) > 40 {
--- a/server/events_controller_test.go
+++ b/server/events_controller_test.go
@@ -124,7 +124,7 @@ func TestPost_GitlabCommentInvalidCommand(t *testing.T) {
 	e, _, gl, _, _, _, _, cp := setup(t)
 	eventsReq.Header.Set(gitlabHeader, "value")
 	When(gl.Validate(eventsReq, secret)).ThenReturn(gitlab.MergeCommentEvent{}, nil)
-	When(cp.DetermineCommand("", vcs.Gitlab)).ThenReturn(events.CommentParseResult{Ignore: true})
+	When(cp.Parse("", vcs.Gitlab)).ThenReturn(events.CommentParseResult{Ignore: true})
 	w := httptest.NewRecorder()
 	e.Post(w, eventsReq)
 	responseContains(t, w, http.StatusOK, "Ignoring non-command comment: \"\"")
@@ -137,7 +137,7 @@ func TestPost_GithubCommentInvalidCommand(t *testing.T) {
 	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(cp.DetermineCommand("", vcs.Github)).ThenReturn(events.CommentParseResult{Ignore: true})
+	When(cp.Parse("", vcs.Github)).ThenReturn(events.CommentParseResult{Ignore: true})
 	w := httptest.NewRecorder()
 	e.Post(w, eventsReq)
 	responseContains(t, w, http.StatusOK, "Ignoring non-command comment: \"\"")
@@ -148,7 +148,7 @@ func TestPost_GitlabCommentResponse(t *testing.T) {
 	e, _, gl, _, _, _, vcsClient, cp := setup(t)
 	eventsReq.Header.Set(gitlabHeader, "value")
 	When(gl.Validate(eventsReq, secret)).ThenReturn(gitlab.MergeCommentEvent{}, nil)
-	When(cp.DetermineCommand("", vcs.Gitlab)).ThenReturn(events.CommentParseResult{CommentResponse: "a comment"})
+	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)
@@ -164,7 +164,7 @@ func TestPost_GithubCommentResponse(t *testing.T) {
 	baseRepo := models.Repo{}
 	user := models.User{}
 	When(p.ParseGithubIssueCommentEvent(matchers.AnyPtrToGithubIssueCommentEvent())).ThenReturn(baseRepo, user, 1, nil)
-	When(cp.DetermineCommand("", vcs.Github)).ThenReturn(events.CommentParseResult{CommentResponse: "a comment"})
+	When(cp.Parse("", vcs.Github)).ThenReturn(events.CommentParseResult{CommentResponse: "a comment"})
 	w := httptest.NewRecorder()

 	e.Post(w, eventsReq)
@@ -196,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(cp.DetermineCommand("", vcs.Github)).ThenReturn(events.CommentParseResult{Command: &cmd})
+	When(cp.Parse("", vcs.Github)).ThenReturn(events.CommentParseResult{Command: &cmd})
 	w := httptest.NewRecorder()
 	e.Post(w, eventsReq)
 	responseContains(t, w, http.StatusOK, "Processing...")