Refactor --skip-clone-no-changes work.

server/events/matchers/models_pullrequest.go

@@ -3,6 +3,7 @@ package matchers
 
 import (
 	"reflect"
+
 	"github.com/petergtz/pegomock"
 	models "github.com/runatlantis/atlantis/server/events/models"
 )

server/events/matchers/models_repo.go

@@ -3,6 +3,7 @@ package matchers
 
 import (
 	"reflect"
+
 	"github.com/petergtz/pegomock"
 	models "github.com/runatlantis/atlantis/server/events/models"
 )

server/events/matchers/ptr_to_logging_simplelogger.go

@@ -3,6 +3,7 @@ package matchers
 
 import (
 	"reflect"
+
 	"github.com/petergtz/pegomock"
 	logging "github.com/runatlantis/atlantis/server/logging"
 )

server/events/mock_workingdir_test.go

@@ -4,11 +4,12 @@
 package events
 
 import (
+	"reflect"
+	"time"
+
 	pegomock "github.com/petergtz/pegomock"
 	models "github.com/runatlantis/atlantis/server/events/models"
 	logging "github.com/runatlantis/atlantis/server/logging"
-	"reflect"
-	"time"
 )
 
 type MockWorkingDir struct {

server/events/project_command_builder.go

@@ -104,20 +104,19 @@ func (p *DefaultProjectCommandBuilder) BuildApplyCommands(ctx *CommandContext, c
 func (p *DefaultProjectCommandBuilder) buildPlanAllCommands(ctx *CommandContext, commentFlags []string, verbose bool) ([]models.ProjectCommandContext, error) {
 	// We'll need the list of modified files.
 	modifiedFiles, err := p.VCSClient.GetModifiedFiles(ctx.BaseRepo, ctx.Pull)
-
 	if err != nil {
 		return nil, err
 	}
 	ctx.Log.Debug("%d files were modified in this pull request", len(modifiedFiles))
 
-	if p.SkipCloneNoChanges && p.VCSClient.IsSupportDownloadSingleFile(ctx.BaseRepo) {
+	if p.SkipCloneNoChanges && p.VCSClient.SupportsSingleFileDownload(ctx.BaseRepo) {
 		hasRepoCfg, repoCfgData, err := p.VCSClient.DownloadRepoConfigFile(ctx.Pull)
 		if err != nil {
 			return nil, errors.Wrapf(err, "downloading %s", yaml.AtlantisYAMLFilename)
 		}
 
 		if hasRepoCfg {
-			repoCfg, err := p.ParserValidator.ParseRepoCfg(repoCfgData, "", p.GlobalCfg, ctx.BaseRepo.ID())
+			repoCfg, err := p.ParserValidator.ParseRepoCfgData(repoCfgData, p.GlobalCfg, ctx.BaseRepo.ID())
 			if err != nil {
 				return nil, errors.Wrapf(err, "parsing %s", yaml.AtlantisYAMLFilename)
 			}
@@ -131,6 +130,9 @@ func (p *DefaultProjectCommandBuilder) buildPlanAllCommands(ctx *CommandContext,
 				ctx.Log.Info("skipping repo clone since no project was modified")
 				return []models.ProjectCommandContext{}, nil
 			}
+			// NOTE: We discard this work here and end up doing it again after
+			// cloning to ensure all the return values are set properly with
+			// the actual clone directory.
 		}
 	}
 
@@ -159,7 +161,7 @@ func (p *DefaultProjectCommandBuilder) buildPlanAllCommands(ctx *CommandContext,
 	if hasRepoCfg {
 		// If there's a repo cfg then we'll use it to figure out which projects
 		// should be planed.
-		repoCfg, err := p.ParserValidator.ParseRepoCfg([]byte{}, repoDir, p.GlobalCfg, ctx.BaseRepo.ID())
+		repoCfg, err := p.ParserValidator.ParseRepoCfg(repoDir, p.GlobalCfg, ctx.BaseRepo.ID())
 		if err != nil {
 			return nil, errors.Wrapf(err, "parsing %s", yaml.AtlantisYAMLFilename)
 		}
@@ -343,7 +345,7 @@ func (p *DefaultProjectCommandBuilder) getCfg(ctx *CommandContext, projectName s
 	}
 
 	var repoConfig valid.RepoCfg
-	repoConfig, err = p.ParserValidator.ParseRepoCfg([]byte{}, repoDir, p.GlobalCfg, ctx.BaseRepo.ID())
+	repoConfig, err = p.ParserValidator.ParseRepoCfg(repoDir, p.GlobalCfg, ctx.BaseRepo.ID())
 	if err != nil {
 		return
 	}

server/events/project_command_builder_test.go

@@ -894,3 +894,43 @@ projects:
 		})
 	}
 }
+
+// Test that we don't clone the repo if there were no changes based on the atlantis.yaml file.
+func TestDefaultProjectCommandBuilder_SkipCloneNoChanges(t *testing.T) {
+	atlantisYAML := `
+version: 3
+projects:
+- dir: dir1`
+
+	RegisterMockTestingT(t)
+	vcsClient := vcsmocks.NewMockClient()
+	When(vcsClient.GetModifiedFiles(matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest())).ThenReturn([]string{"main.tf"}, nil)
+	When(vcsClient.SupportsSingleFileDownload(matchers.AnyModelsRepo())).ThenReturn(true)
+	When(vcsClient.DownloadRepoConfigFile(matchers.AnyModelsPullRequest())).ThenReturn(true, []byte(atlantisYAML), nil)
+	workingDir := mocks.NewMockWorkingDir()
+
+	builder := &events.DefaultProjectCommandBuilder{
+		WorkingDirLocker:   events.NewDefaultWorkingDirLocker(),
+		WorkingDir:         workingDir,
+		ParserValidator:    &yaml.ParserValidator{},
+		VCSClient:          vcsClient,
+		ProjectFinder:      &events.DefaultProjectFinder{},
+		CommentBuilder:     &events.CommentParser{},
+		GlobalCfg:          valid.NewGlobalCfg(true, false, false),
+		SkipCloneNoChanges: true,
+	}
+
+	var actCtxs []models.ProjectCommandContext
+	var err error
+	actCtxs, err = builder.BuildAutoplanCommands(&events.CommandContext{
+		BaseRepo:      models.Repo{},
+		HeadRepo:      models.Repo{},
+		Pull:          models.PullRequest{},
+		User:          models.User{},
+		Log:           nil,
+		PullMergeable: true,
+	})
+	Ok(t, err)
+	Equals(t, 0, len(actCtxs))
+	workingDir.VerifyWasCalled(Never()).Clone(matchers.AnyPtrToLoggingSimpleLogger(), matchers.AnyModelsRepo(), matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest(), AnyString())
+}

server/events/project_finder.go

@@ -120,8 +120,14 @@ func (p *DefaultProjectFinder) DetermineProjectsViaConfig(log *logging.SimpleLog
 			}
 			if match {
 				log.Debug("file %q matched pattern", file)
-				// Skipping checking existing if remote atlantis.yaml was used
-				if len(absRepoDir) != 0 {
+				// If we're checking using an atlantis.yaml file we downloaded
+				// directly from the repo (when doing a no-clone check) then
+				// absRepoDir will be empty. Since we didn't clone the repo
+				// yet we can't do this check. If there was a file modified
+				// in a deleted directory then when we finally do clone the repo
+				// we'll call this function again and then we'll detect the
+				// directory was deleted.
+				if absRepoDir != "" {
 					_, err := os.Stat(filepath.Join(absRepoDir, project.Dir))
 					if err == nil {
 						projects = append(projects, project)

server/events/vcs/azuredevops_client.go

@@ -365,7 +365,7 @@ func SplitAzureDevopsRepoFullName(repoFullName string) (owner string, project st
 	return repoFullName[:lastSlashIdx], "", repoFullName[lastSlashIdx+1:]
 }
 
-func (g *AzureDevopsClient) IsSupportDownloadSingleFile(repo models.Repo) bool {
+func (g *AzureDevopsClient) SupportsSingleFileDownload(repo models.Repo) bool {
 	return false
 }
 

server/events/vcs/bitbucketcloud/client.go

@@ -244,7 +244,7 @@ func (b *Client) makeRequest(method string, path string, reqBody io.Reader) ([]b
 	return respBody, nil
 }
 
-func (b *Client) IsSupportDownloadSingleFile(models.Repo) bool {
+func (b *Client) SupportsSingleFileDownload(models.Repo) bool {
 	return false
 }
 

server/events/vcs/bitbucketserver/client.go

@@ -312,7 +312,7 @@ func (b *Client) makeRequest(method string, path string, reqBody io.Reader) ([]b
 	return respBody, nil
 }
 
-func (b *Client) IsSupportDownloadSingleFile(repo models.Repo) bool {
+func (b *Client) SupportsSingleFileDownload(repo models.Repo) bool {
 	return false
 }
 

server/events/vcs/client.go

@@ -43,5 +43,5 @@ type Client interface {
 	// The first return value indicate that repo contain atlantis.yaml or not
 	// if BaseRepo had one repo config file, its content will placed on the second return value
 	DownloadRepoConfigFile(pull models.PullRequest) (bool, []byte, error)
-	IsSupportDownloadSingleFile(repo models.Repo) bool
+	SupportsSingleFileDownload(repo models.Repo) bool
 }

server/events/vcs/github_client.go

@@ -409,6 +409,6 @@ func (g *GithubClient) DownloadRepoConfigFile(pull models.PullRequest) (bool, []
 	return true, decodedData, nil
 }
 
-func (g *GithubClient) IsSupportDownloadSingleFile(repo models.Repo) bool {
+func (g *GithubClient) SupportsSingleFileDownload(repo models.Repo) bool {
 	return true
 }

server/events/vcs/gitlab_client.go

@@ -15,12 +15,13 @@ package vcs
 
 import (
 	"fmt"
-	"github.com/runatlantis/atlantis/server/events/yaml"
 	"net"
 	"net/http"
 	"net/url"
 	"strings"
 
+	"github.com/runatlantis/atlantis/server/events/yaml"
+
 	"github.com/runatlantis/atlantis/server/events/vcs/common"
 
 	version "github.com/hashicorp/go-version"
@@ -290,6 +291,6 @@ func (g *GitlabClient) DownloadRepoConfigFile(pull models.PullRequest) (bool, []
 	return true, bytes, nil
 }
 
-func (g *GitlabClient) IsSupportDownloadSingleFile(repo models.Repo) bool {
+func (g *GitlabClient) SupportsSingleFileDownload(repo models.Repo) bool {
 	return true
 }

server/events/vcs/mocks/matchers/slice_of_byte.go

@@ -0,0 +1,20 @@
+// Code generated by pegomock. DO NOT EDIT.
+package matchers
+
+import (
+	"reflect"
+	"github.com/petergtz/pegomock"
+	
+)
+
+func AnySliceOfByte() []byte {
+	pegomock.RegisterMatcher(pegomock.NewAnyMatcher(reflect.TypeOf((*([]byte))(nil)).Elem()))
+	var nullValue []byte
+	return nullValue
+}
+
+func EqSliceOfByte(value []byte) []byte {
+	pegomock.RegisterMatcher(&pegomock.EqMatcher{Value: value})
+	var nullValue []byte
+	return nullValue
+}

server/events/vcs/mocks/mock_client.go

@@ -161,12 +161,42 @@ func (mock *MockClient) MarkdownPullLink(pull models.PullRequest) (string, error
 	return ret0, ret1
 }
 
-func (mock *MockClient) IsSupportDownloadSingleFile(repo models.Repo) bool {
-  return false
+func (mock *MockClient) DownloadRepoConfigFile(pull models.PullRequest) (bool, []byte, error) {
+	if mock == nil {
+		panic("mock must not be nil. Use myMock := NewMockClient().")
+	}
+	params := []pegomock.Param{pull}
+	result := pegomock.GetGenericMockFrom(mock).Invoke("DownloadRepoConfigFile", params, []reflect.Type{reflect.TypeOf((*bool)(nil)).Elem(), reflect.TypeOf((*[]byte)(nil)).Elem(), reflect.TypeOf((*error)(nil)).Elem()})
+	var ret0 bool
+	var ret1 []byte
+	var ret2 error
+	if len(result) != 0 {
+		if result[0] != nil {
+			ret0 = result[0].(bool)
+		}
+		if result[1] != nil {
+			ret1 = result[1].([]byte)
+		}
+		if result[2] != nil {
+			ret2 = result[2].(error)
+		}
+	}
+	return ret0, ret1, ret2
 }
 
-func (mock *MockClient) DownloadRepoConfigFile(pull models.PullRequest) (bool, []byte, error) {
-  return false, []byte{}, nil
+func (mock *MockClient) SupportsSingleFileDownload(repo models.Repo) bool {
+	if mock == nil {
+		panic("mock must not be nil. Use myMock := NewMockClient().")
+	}
+	params := []pegomock.Param{repo}
+	result := pegomock.GetGenericMockFrom(mock).Invoke("SupportsSingleFileDownload", params, []reflect.Type{reflect.TypeOf((*bool)(nil)).Elem()})
+	var ret0 bool
+	if len(result) != 0 {
+		if result[0] != nil {
+			ret0 = result[0].(bool)
+		}
+	}
+	return ret0
 }
 
 func (mock *MockClient) VerifyWasCalledOnce() *VerifierMockClient {
@@ -469,3 +499,57 @@ func (c *MockClient_MarkdownPullLink_OngoingVerification) GetAllCapturedArgument
 	}
 	return
 }
+
+func (verifier *VerifierMockClient) DownloadRepoConfigFile(pull models.PullRequest) *MockClient_DownloadRepoConfigFile_OngoingVerification {
+	params := []pegomock.Param{pull}
+	methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "DownloadRepoConfigFile", params, verifier.timeout)
+	return &MockClient_DownloadRepoConfigFile_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations}
+}
+
+type MockClient_DownloadRepoConfigFile_OngoingVerification struct {
+	mock              *MockClient
+	methodInvocations []pegomock.MethodInvocation
+}
+
+func (c *MockClient_DownloadRepoConfigFile_OngoingVerification) GetCapturedArguments() models.PullRequest {
+	pull := c.GetAllCapturedArguments()
+	return pull[len(pull)-1]
+}
+
+func (c *MockClient_DownloadRepoConfigFile_OngoingVerification) GetAllCapturedArguments() (_param0 []models.PullRequest) {
+	params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations)
+	if len(params) > 0 {
+		_param0 = make([]models.PullRequest, len(c.methodInvocations))
+		for u, param := range params[0] {
+			_param0[u] = param.(models.PullRequest)
+		}
+	}
+	return
+}
+
+func (verifier *VerifierMockClient) SupportsSingleFileDownload(repo models.Repo) *MockClient_SupportsSingleFileDownload_OngoingVerification {
+	params := []pegomock.Param{repo}
+	methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "SupportsSingleFileDownload", params, verifier.timeout)
+	return &MockClient_SupportsSingleFileDownload_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations}
+}
+
+type MockClient_SupportsSingleFileDownload_OngoingVerification struct {
+	mock              *MockClient
+	methodInvocations []pegomock.MethodInvocation
+}
+
+func (c *MockClient_SupportsSingleFileDownload_OngoingVerification) GetCapturedArguments() models.Repo {
+	repo := c.GetAllCapturedArguments()
+	return repo[len(repo)-1]
+}
+
+func (c *MockClient_SupportsSingleFileDownload_OngoingVerification) GetAllCapturedArguments() (_param0 []models.Repo) {
+	params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations)
+	if len(params) > 0 {
+		_param0 = make([]models.Repo, len(c.methodInvocations))
+		for u, param := range params[0] {
+			_param0[u] = param.(models.Repo)
+		}
+	}
+	return
+}

server/events/vcs/not_configured_vcs_client.go

@@ -54,7 +54,7 @@ func (a *NotConfiguredVCSClient) err() error {
 	return fmt.Errorf("atlantis was not configured to support repos from %s", a.Host.String())
 }
 
-func (a *NotConfiguredVCSClient) IsSupportDownloadSingleFile(repo models.Repo) bool {
+func (a *NotConfiguredVCSClient) SupportsSingleFileDownload(repo models.Repo) bool {
 	return false
 }
 

server/events/vcs/proxy.go

@@ -88,6 +88,6 @@ func (d *ClientProxy) DownloadRepoConfigFile(pull models.PullRequest) (bool, []b
 	return d.clients[pull.BaseRepo.VCSHost.Type].DownloadRepoConfigFile(pull)
 }
 
-func (d *ClientProxy) IsSupportDownloadSingleFile(repo models.Repo) bool {
-	return d.clients[repo.VCSHost.Type].IsSupportDownloadSingleFile(repo)
+func (d *ClientProxy) SupportsSingleFileDownload(repo models.Repo) bool {
+	return d.clients[repo.VCSHost.Type].SupportsSingleFileDownload(repo)
 }

server/events/yaml/parser_validator.go

@@ -44,28 +44,24 @@ func (p *ParserValidator) HasRepoCfg(absRepoDir string) (bool, error) {
 // ParseRepoCfg returns the parsed and validated atlantis.yaml config for the
 // repo at absRepoDir.
 // If there was no config file, it will return an os.IsNotExist(error).
-func (p *ParserValidator) ParseRepoCfg(repoCfgData []byte, absRepoDir string, globalCfg valid.GlobalCfg, repoID string) (valid.RepoCfg, error) {
-	var configData []byte
-	var err error
+func (p *ParserValidator) ParseRepoCfg(absRepoDir string, globalCfg valid.GlobalCfg, repoID string) (valid.RepoCfg, error) {
+	configFile := p.repoCfgPath(absRepoDir, AtlantisYAMLFilename)
+	configData, err := ioutil.ReadFile(configFile) // nolint: gosec
 
-	if len(repoCfgData) > 0 {
-		configData = repoCfgData
-	} else {
-		configFile := p.repoCfgPath(absRepoDir, AtlantisYAMLFilename)
-		configData, err = ioutil.ReadFile(configFile) // nolint: gosec
-
-		if err != nil {
-			if !os.IsNotExist(err) {
-				return valid.RepoCfg{}, errors.Wrapf(err, "unable to read %s file", AtlantisYAMLFilename)
-			}
-			// Don't wrap os.IsNotExist errors because we want our callers to be
-			// able to detect if it's a NotExist err.
-			return valid.RepoCfg{}, err
+	if err != nil {
+		if !os.IsNotExist(err) {
+			return valid.RepoCfg{}, errors.Wrapf(err, "unable to read %s file", AtlantisYAMLFilename)
 		}
+		// Don't wrap os.IsNotExist errors because we want our callers to be
+		// able to detect if it's a NotExist err.
+		return valid.RepoCfg{}, err
 	}
+	return p.ParseRepoCfgData(configData, globalCfg, repoID)
+}
 
+func (p *ParserValidator) ParseRepoCfgData(repoCfgData []byte, globalCfg valid.GlobalCfg, repoID string) (valid.RepoCfg, error) {
 	var rawConfig raw.RepoCfg
-	if err := yaml.UnmarshalStrict(configData, &rawConfig); err != nil {
+	if err := yaml.UnmarshalStrict(repoCfgData, &rawConfig); err != nil {
 		return valid.RepoCfg{}, err
 	}
 
@@ -90,7 +86,7 @@ func (p *ParserValidator) ParseRepoCfg(repoCfgData []byte, absRepoDir string, gl
 		}
 	}
 
-	err = globalCfg.ValidateRepoCfg(validConfig, repoID)
+	err := globalCfg.ValidateRepoCfg(validConfig, repoID)
 	return validConfig, err
 }
 

server/events/yaml/parser_validator_test.go

@@ -46,7 +46,7 @@ func TestHasRepoCfg_InvalidFileExtension(t *testing.T) {
 
 func TestParseRepoCfg_DirDoesNotExist(t *testing.T) {
 	r := yaml.ParserValidator{}
-	_, err := r.ParseRepoCfg([]byte{}, "/not/exist", globalCfg, "")
+	_, err := r.ParseRepoCfg("/not/exist", globalCfg, "")
 	Assert(t, os.IsNotExist(err), "exp not exist err")
 }
 
@@ -54,7 +54,7 @@ func TestParseRepoCfg_FileDoesNotExist(t *testing.T) {
 	tmpDir, cleanup := TempDir(t)
 	defer cleanup()
 	r := yaml.ParserValidator{}
-	_, err := r.ParseRepoCfg([]byte{}, tmpDir, globalCfg, "")
+	_, err := r.ParseRepoCfg(tmpDir, globalCfg, "")
 	Assert(t, os.IsNotExist(err), "exp not exist err")
 }
 
@@ -65,7 +65,7 @@ func TestParseRepoCfg_BadPermissions(t *testing.T) {
 	Ok(t, err)
 
 	r := yaml.ParserValidator{}
-	_, err = r.ParseRepoCfg([]byte{}, tmpDir, globalCfg, "")
+	_, err = r.ParseRepoCfg(tmpDir, globalCfg, "")
 	ErrContains(t, "unable to read atlantis.yaml file: ", err)
 }
 
@@ -99,7 +99,7 @@ func TestParseCfgs_InvalidYAML(t *testing.T) {
 			err := ioutil.WriteFile(confPath, []byte(c.input), 0600)
 			Ok(t, err)
 			r := yaml.ParserValidator{}
-			_, err = r.ParseRepoCfg([]byte{}, tmpDir, globalCfg, "")
+			_, err = r.ParseRepoCfg(tmpDir, globalCfg, "")
 			ErrContains(t, c.expErr, err)
 			_, err = r.ParseGlobalCfg(confPath, valid.NewGlobalCfg(false, false, false))
 			ErrContains(t, c.expErr, err)
@@ -845,7 +845,7 @@ workflows:
 			Ok(t, err)
 
 			r := yaml.ParserValidator{}
-			act, err := r.ParseRepoCfg([]byte{}, tmpDir, globalCfg, "")
+			act, err := r.ParseRepoCfg(tmpDir, globalCfg, "")
 			if c.expErr != "" {
 				ErrEquals(t, c.expErr, err)
 				return
@@ -873,7 +873,7 @@ workflows:
 	Ok(t, err)
 
 	r := yaml.ParserValidator{}
-	_, err = r.ParseRepoCfg([]byte{}, tmpDir, valid.NewGlobalCfg(false, false, false), "repo_id")
+	_, err = r.ParseRepoCfg(tmpDir, valid.NewGlobalCfg(false, false, false), "repo_id")
 	ErrEquals(t, "repo config not allowed to set 'workflow' key: server-side config needs 'allowed_overrides: [workflow]'", err)
 }
 
@@ -1337,7 +1337,7 @@ func TestParseRepoCfg_V2ShellParsing(t *testing.T) {
 			Ok(t, ioutil.WriteFile(v3Path, []byte("version: 3\n"+cfg), 0600))
 
 			p := &yaml.ParserValidator{}
-			v2Cfg, err := p.ParseRepoCfg([]byte{}, v2Dir, valid.NewGlobalCfg(true, false, false), "")
+			v2Cfg, err := p.ParseRepoCfg(v2Dir, valid.NewGlobalCfg(true, false, false), "")
 			if c.expV2Err != "" {
 				ErrEquals(t, c.expV2Err, err)
 			} else {
@@ -1346,7 +1346,7 @@ func TestParseRepoCfg_V2ShellParsing(t *testing.T) {
 				Equals(t, c.expV2, v2Cfg.Workflows["custom"].Apply.Steps[0].RunCommand)
 			}
 
-			v3Cfg, err := p.ParseRepoCfg([]byte{}, v3Dir, valid.NewGlobalCfg(true, false, false), "")
+			v3Cfg, err := p.ParseRepoCfg(v3Dir, valid.NewGlobalCfg(true, false, false), "")
 			Ok(t, err)
 			Equals(t, c.in, v3Cfg.Workflows["custom"].Plan.Steps[0].RunCommand)
 			Equals(t, c.in, v3Cfg.Workflows["custom"].Apply.Steps[0].RunCommand)