From 54e3dc2ef0d5e37f3bf3fe3f17c31c48e4441495 Mon Sep 17 00:00:00 2001 From: Quan Hoang Date: Mon, 18 May 2020 18:32:43 +0700 Subject: [PATCH 1/2] Skip cloning PR repository in case of no projects configured in repo config file were changed --- cmd/server.go | 5 + cmd/server_test.go | 1 + server/events/project_command_builder.go | 62 +++++++--- .../project_command_builder_internal_test.go | 17 +-- server/events/project_command_builder_test.go | 109 ++++++++++-------- server/events/project_finder.go | 13 ++- server/events/vcs/azuredevops_client.go | 8 ++ server/events/vcs/bitbucketcloud/client.go | 11 ++ server/events/vcs/bitbucketserver/client.go | 11 ++ server/events/vcs/client.go | 6 + server/events/vcs/github_client.go | 36 +++++- server/events/vcs/gitlab_client.go | 24 ++++ server/events/vcs/mocks/mock_client.go | 8 ++ .../events/vcs/not_configured_vcs_client.go | 8 ++ server/events/vcs/proxy.go | 8 ++ server/events/yaml/parser_validator.go | 27 +++-- server/events/yaml/parser_validator_test.go | 16 +-- server/events_controller_e2e_test.go | 17 +-- server/server.go | 17 +-- server/user_config.go | 1 + 20 files changed, 286 insertions(+), 119 deletions(-) diff --git a/cmd/server.go b/cmd/server.go index 1aecb70045..76e96283c1 100644 --- a/cmd/server.go +++ b/cmd/server.go @@ -81,6 +81,7 @@ const ( SilenceAllowlistErrorsFlag = "silence-allowlist-errors" // SilenceWhitelistErrorsFlag is deprecated for SilenceAllowlistErrorsFlag. SilenceWhitelistErrorsFlag = "silence-whitelist-errors" + SkipCloneNoChanges = "skip-clone-no-changes" SlackTokenFlag = "slack-token" SSLCertFileFlag = "ssl-cert-file" SSLKeyFileFlag = "ssl-key-file" @@ -319,6 +320,10 @@ var boolFlags = map[string]boolFlag{ " This writes secrets to disk and should only be enabled in a secure environment.", defaultValue: false, }, + SkipCloneNoChanges: { + description: "Skips cloning the PR repo if there are no projects were changed in the PR.", + defaultValue: false, + }, } var intFlags = map[string]intFlag{ PortFlag: { diff --git a/cmd/server_test.go b/cmd/server_test.go index 9569c5ac2a..c9ac658f9d 100644 --- a/cmd/server_test.go +++ b/cmd/server_test.go @@ -86,6 +86,7 @@ var testFlags = map[string]interface{}{ SilenceForkPRErrorsFlag: true, SilenceAllowlistErrorsFlag: true, SilenceVCSStatusNoPlans: true, + SkipCloneNoChanges: true, SlackTokenFlag: "slack-token", SSLCertFileFlag: "cert-file", SSLKeyFileFlag: "key-file", diff --git a/server/events/project_command_builder.go b/server/events/project_command_builder.go index 4980067aa6..ec8d2cad27 100644 --- a/server/events/project_command_builder.go +++ b/server/events/project_command_builder.go @@ -53,14 +53,15 @@ type ProjectCommandBuilder interface { // This class combines the data from the comment and any atlantis.yaml file or // Atlantis server config and then generates a set of contexts. type DefaultProjectCommandBuilder struct { - ParserValidator *yaml.ParserValidator - ProjectFinder ProjectFinder - VCSClient vcs.Client - WorkingDir WorkingDir - WorkingDirLocker WorkingDirLocker - GlobalCfg valid.GlobalCfg - PendingPlanFinder *DefaultPendingPlanFinder - CommentBuilder CommentBuilder + ParserValidator *yaml.ParserValidator + ProjectFinder ProjectFinder + VCSClient vcs.Client + WorkingDir WorkingDir + WorkingDirLocker WorkingDirLocker + GlobalCfg valid.GlobalCfg + PendingPlanFinder *DefaultPendingPlanFinder + CommentBuilder CommentBuilder + SkipCloneNoChanges bool } // See ProjectCommandBuilder.BuildAutoplanCommands. @@ -101,8 +102,41 @@ func (p *DefaultProjectCommandBuilder) BuildApplyCommands(ctx *CommandContext, c // buildPlanAllCommands builds plan contexts for all projects we determine were // modified in this ctx. 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) { + 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()) + if err != nil { + return nil, errors.Wrapf(err, "parsing %s", yaml.AtlantisYAMLFilename) + } + ctx.Log.Info("successfully parsed remote %s file", yaml.AtlantisYAMLFilename) + matchingProjects, err := p.ProjectFinder.DetermineProjectsViaConfig(ctx.Log, modifiedFiles, repoCfg, "") + if err != nil { + return nil, err + } + ctx.Log.Info("%d projects are changed on MR %q based on their when_modified config", len(matchingProjects), ctx.Pull.Num) + if len(matchingProjects) == 0 { + ctx.Log.Info("skipping repo clone since no project was modified") + return []models.ProjectCommandContext{}, nil + } + } + } + // Need to lock the workspace we're about to clone to. workspace := DefaultWorkspace + unlockFn, err := p.WorkingDirLocker.TryLock(ctx.BaseRepo.FullName, ctx.Pull.Num, workspace) if err != nil { ctx.Log.Warn("workspace was locked") @@ -111,18 +145,10 @@ func (p *DefaultProjectCommandBuilder) buildPlanAllCommands(ctx *CommandContext, ctx.Log.Debug("got workspace lock") defer unlockFn() - // 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)) - repoDir, _, err := p.WorkingDir.Clone(ctx.Log, ctx.BaseRepo, ctx.HeadRepo, ctx.Pull, workspace) if err != nil { return nil, err } - // Parse config file if it exists. hasRepoCfg, err := p.ParserValidator.HasRepoCfg(repoDir) if err != nil { @@ -133,7 +159,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(repoDir, p.GlobalCfg, ctx.BaseRepo.ID()) + repoCfg, err := p.ParserValidator.ParseRepoCfg([]byte{}, repoDir, p.GlobalCfg, ctx.BaseRepo.ID()) if err != nil { return nil, errors.Wrapf(err, "parsing %s", yaml.AtlantisYAMLFilename) } @@ -317,7 +343,7 @@ func (p *DefaultProjectCommandBuilder) getCfg(ctx *CommandContext, projectName s } var repoConfig valid.RepoCfg - repoConfig, err = p.ParserValidator.ParseRepoCfg(repoDir, p.GlobalCfg, ctx.BaseRepo.ID()) + repoConfig, err = p.ParserValidator.ParseRepoCfg([]byte{}, repoDir, p.GlobalCfg, ctx.BaseRepo.ID()) if err != nil { return } diff --git a/server/events/project_command_builder_internal_test.go b/server/events/project_command_builder_internal_test.go index fc04e9a6c0..9adfb64606 100644 --- a/server/events/project_command_builder_internal_test.go +++ b/server/events/project_command_builder_internal_test.go @@ -563,14 +563,15 @@ projects: } builder := &DefaultProjectCommandBuilder{ - WorkingDirLocker: NewDefaultWorkingDirLocker(), - WorkingDir: workingDir, - ParserValidator: parser, - VCSClient: vcsClient, - ProjectFinder: &DefaultProjectFinder{}, - PendingPlanFinder: &DefaultPendingPlanFinder{}, - CommentBuilder: &CommentParser{}, - GlobalCfg: globalCfg, + WorkingDirLocker: NewDefaultWorkingDirLocker(), + WorkingDir: workingDir, + ParserValidator: parser, + VCSClient: vcsClient, + ProjectFinder: &DefaultProjectFinder{}, + PendingPlanFinder: &DefaultPendingPlanFinder{}, + CommentBuilder: &CommentParser{}, + GlobalCfg: globalCfg, + SkipCloneNoChanges: false, } // We run a test for each type of command. diff --git a/server/events/project_command_builder_test.go b/server/events/project_command_builder_test.go index dad6276306..c4848b04e6 100644 --- a/server/events/project_command_builder_test.go +++ b/server/events/project_command_builder_test.go @@ -135,14 +135,15 @@ projects: } builder := &events.DefaultProjectCommandBuilder{ - WorkingDirLocker: events.NewDefaultWorkingDirLocker(), - WorkingDir: workingDir, - ParserValidator: &yaml.ParserValidator{}, - VCSClient: vcsClient, - ProjectFinder: &events.DefaultProjectFinder{}, - PendingPlanFinder: &events.DefaultPendingPlanFinder{}, - CommentBuilder: &events.CommentParser{}, - GlobalCfg: valid.NewGlobalCfg(false, false, false), + WorkingDirLocker: events.NewDefaultWorkingDirLocker(), + WorkingDir: workingDir, + ParserValidator: &yaml.ParserValidator{}, + VCSClient: vcsClient, + ProjectFinder: &events.DefaultProjectFinder{}, + PendingPlanFinder: &events.DefaultPendingPlanFinder{}, + CommentBuilder: &events.CommentParser{}, + GlobalCfg: valid.NewGlobalCfg(false, false, false), + SkipCloneNoChanges: false, } ctxs, err := builder.BuildAutoplanCommands(&events.CommandContext{ @@ -358,13 +359,14 @@ projects: } 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), + WorkingDirLocker: events.NewDefaultWorkingDirLocker(), + WorkingDir: workingDir, + ParserValidator: &yaml.ParserValidator{}, + VCSClient: vcsClient, + ProjectFinder: &events.DefaultProjectFinder{}, + CommentBuilder: &events.CommentParser{}, + GlobalCfg: valid.NewGlobalCfg(true, false, false), + SkipCloneNoChanges: false, } var actCtxs []models.ProjectCommandContext @@ -491,13 +493,14 @@ projects: } 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), + WorkingDirLocker: events.NewDefaultWorkingDirLocker(), + WorkingDir: workingDir, + ParserValidator: &yaml.ParserValidator{}, + VCSClient: vcsClient, + ProjectFinder: &events.DefaultProjectFinder{}, + CommentBuilder: &events.CommentParser{}, + GlobalCfg: valid.NewGlobalCfg(true, false, false), + SkipCloneNoChanges: false, } ctxs, err := builder.BuildPlanCommands( @@ -562,14 +565,15 @@ func TestDefaultProjectCommandBuilder_BuildMultiApply(t *testing.T) { ThenReturn(tmpDir, nil) builder := &events.DefaultProjectCommandBuilder{ - WorkingDirLocker: events.NewDefaultWorkingDirLocker(), - WorkingDir: workingDir, - ParserValidator: &yaml.ParserValidator{}, - VCSClient: nil, - ProjectFinder: &events.DefaultProjectFinder{}, - PendingPlanFinder: &events.DefaultPendingPlanFinder{}, - CommentBuilder: &events.CommentParser{}, - GlobalCfg: valid.NewGlobalCfg(false, false, false), + WorkingDirLocker: events.NewDefaultWorkingDirLocker(), + WorkingDir: workingDir, + ParserValidator: &yaml.ParserValidator{}, + VCSClient: nil, + ProjectFinder: &events.DefaultProjectFinder{}, + PendingPlanFinder: &events.DefaultPendingPlanFinder{}, + CommentBuilder: &events.CommentParser{}, + GlobalCfg: valid.NewGlobalCfg(false, false, false), + SkipCloneNoChanges: false, } ctxs, err := builder.BuildApplyCommands( @@ -630,13 +634,14 @@ projects: AnyString())).ThenReturn(repoDir, nil) builder := &events.DefaultProjectCommandBuilder{ - WorkingDirLocker: events.NewDefaultWorkingDirLocker(), - WorkingDir: workingDir, - ParserValidator: &yaml.ParserValidator{}, - VCSClient: nil, - ProjectFinder: &events.DefaultProjectFinder{}, - CommentBuilder: &events.CommentParser{}, - GlobalCfg: valid.NewGlobalCfg(true, false, false), + WorkingDirLocker: events.NewDefaultWorkingDirLocker(), + WorkingDir: workingDir, + ParserValidator: &yaml.ParserValidator{}, + VCSClient: nil, + ProjectFinder: &events.DefaultProjectFinder{}, + CommentBuilder: &events.CommentParser{}, + GlobalCfg: valid.NewGlobalCfg(true, false, false), + SkipCloneNoChanges: false, } ctx := &events.CommandContext{ @@ -692,13 +697,14 @@ func TestDefaultProjectCommandBuilder_EscapeArgs(t *testing.T) { When(vcsClient.GetModifiedFiles(matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest())).ThenReturn([]string{"main.tf"}, nil) 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), + WorkingDirLocker: events.NewDefaultWorkingDirLocker(), + WorkingDir: workingDir, + ParserValidator: &yaml.ParserValidator{}, + VCSClient: vcsClient, + ProjectFinder: &events.DefaultProjectFinder{}, + CommentBuilder: &events.CommentParser{}, + GlobalCfg: valid.NewGlobalCfg(true, false, false), + SkipCloneNoChanges: false, } var actCtxs []models.ProjectCommandContext @@ -856,13 +862,14 @@ projects: AnyString())).ThenReturn(tmpDir, nil) builder := &events.DefaultProjectCommandBuilder{ - WorkingDirLocker: events.NewDefaultWorkingDirLocker(), - WorkingDir: workingDir, - VCSClient: vcsClient, - ParserValidator: &yaml.ParserValidator{}, - ProjectFinder: &events.DefaultProjectFinder{}, - CommentBuilder: &events.CommentParser{}, - GlobalCfg: valid.NewGlobalCfg(true, false, false), + WorkingDirLocker: events.NewDefaultWorkingDirLocker(), + WorkingDir: workingDir, + VCSClient: vcsClient, + ParserValidator: &yaml.ParserValidator{}, + ProjectFinder: &events.DefaultProjectFinder{}, + CommentBuilder: &events.CommentParser{}, + GlobalCfg: valid.NewGlobalCfg(true, false, false), + SkipCloneNoChanges: false, } actCtxs, err := builder.BuildPlanCommands( diff --git a/server/events/project_finder.go b/server/events/project_finder.go index 6c68a1fd61..b48250da85 100644 --- a/server/events/project_finder.go +++ b/server/events/project_finder.go @@ -120,11 +120,16 @@ func (p *DefaultProjectFinder) DetermineProjectsViaConfig(log *logging.SimpleLog } if match { log.Debug("file %q matched pattern", file) - _, err := os.Stat(filepath.Join(absRepoDir, project.Dir)) - if err == nil { - projects = append(projects, project) + // Skipping checking existing if remote atlantis.yaml was used + if len(absRepoDir) != 0 { + _, err := os.Stat(filepath.Join(absRepoDir, project.Dir)) + if err == nil { + projects = append(projects, project) + } else { + log.Debug("project at dir %q not included because dir does not exist", project.Dir) + } } else { - log.Debug("project at dir %q not included because dir does not exist", project.Dir) + projects = append(projects, project) } break } diff --git a/server/events/vcs/azuredevops_client.go b/server/events/vcs/azuredevops_client.go index 26a69a5e79..f99719f3e9 100644 --- a/server/events/vcs/azuredevops_client.go +++ b/server/events/vcs/azuredevops_client.go @@ -364,3 +364,11 @@ func SplitAzureDevopsRepoFullName(repoFullName string) (owner string, project st } return repoFullName[:lastSlashIdx], "", repoFullName[lastSlashIdx+1:] } + +func (g *AzureDevopsClient) IsSupportDownloadSingleFile(repo models.Repo) bool { + return false +} + +func (g *AzureDevopsClient) DownloadRepoConfigFile(pull models.PullRequest) (bool, []byte, error) { + return false, []byte{}, fmt.Errorf("Not Implemented") +} diff --git a/server/events/vcs/bitbucketcloud/client.go b/server/events/vcs/bitbucketcloud/client.go index 88d6312941..4e67c3840a 100644 --- a/server/events/vcs/bitbucketcloud/client.go +++ b/server/events/vcs/bitbucketcloud/client.go @@ -243,3 +243,14 @@ func (b *Client) makeRequest(method string, path string, reqBody io.Reader) ([]b } return respBody, nil } + +func (b *Client) IsSupportDownloadSingleFile(models.Repo) bool { + return false +} + +// DownloadRepoConfigFile return `atlantis.yaml` content from VCS (which support fetch a single file from repository) +// 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 +func (b *Client) DownloadRepoConfigFile(pull models.PullRequest) (bool, []byte, error) { + return false, []byte{}, fmt.Errorf("Not Implemented") +} diff --git a/server/events/vcs/bitbucketserver/client.go b/server/events/vcs/bitbucketserver/client.go index 0c99627375..f983c36140 100644 --- a/server/events/vcs/bitbucketserver/client.go +++ b/server/events/vcs/bitbucketserver/client.go @@ -311,3 +311,14 @@ func (b *Client) makeRequest(method string, path string, reqBody io.Reader) ([]b } return respBody, nil } + +func (b *Client) IsSupportDownloadSingleFile(repo models.Repo) bool { + return false +} + +// DownloadRepoConfigFile return `atlantis.yaml` content from VCS (which support fetch a single file from repository) +// 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 +func (b *Client) DownloadRepoConfigFile(pull models.PullRequest) (bool, []byte, error) { + return false, []byte{}, fmt.Errorf("not implemented") +} diff --git a/server/events/vcs/client.go b/server/events/vcs/client.go index 5afb69f7b4..0ecda34a60 100644 --- a/server/events/vcs/client.go +++ b/server/events/vcs/client.go @@ -38,4 +38,10 @@ type Client interface { UpdateStatus(repo models.Repo, pull models.PullRequest, state models.CommitStatus, src string, description string, url string) error MergePull(pull models.PullRequest) error MarkdownPullLink(pull models.PullRequest) (string, error) + + // DownloadRepoConfigFile return `atlantis.yaml` content from VCS (which support fetch a single file from repository) + // 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 } diff --git a/server/events/vcs/github_client.go b/server/events/vcs/github_client.go index 0b5ed38c5d..22f54ff77f 100644 --- a/server/events/vcs/github_client.go +++ b/server/events/vcs/github_client.go @@ -15,17 +15,19 @@ package vcs import ( "context" + "encoding/base64" "fmt" + "net/http" "strings" "time" - "github.com/runatlantis/atlantis/server/events/models" - "github.com/runatlantis/atlantis/server/events/vcs/common" - "github.com/runatlantis/atlantis/server/logging" - "github.com/Laisky/graphql" "github.com/google/go-github/v31/github" "github.com/pkg/errors" + "github.com/runatlantis/atlantis/server/events/models" + "github.com/runatlantis/atlantis/server/events/vcs/common" + "github.com/runatlantis/atlantis/server/events/yaml" + "github.com/runatlantis/atlantis/server/logging" "github.com/shurcooL/githubv4" ) @@ -384,3 +386,29 @@ func (g *GithubClient) ExchangeCode(code string) (*GithubAppTemporarySecrets, er return data, err } + +// DownloadRepoConfigFile return `atlantis.yaml` content from VCS (which support fetch a single file from repository) +// 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 +func (g *GithubClient) DownloadRepoConfigFile(pull models.PullRequest) (bool, []byte, error) { + opt := github.RepositoryContentGetOptions{Ref: pull.HeadBranch} + fileContent, _, resp, err := g.client.Repositories.GetContents(g.ctx, pull.BaseRepo.Owner, pull.BaseRepo.Name, yaml.AtlantisYAMLFilename, &opt) + + if resp.StatusCode == http.StatusNotFound { + return false, []byte{}, nil + } + if err != nil { + return true, []byte{}, err + } + + decodedData, err := base64.StdEncoding.DecodeString(*fileContent.Content) + if err != nil { + return true, []byte{}, err + } + + return true, decodedData, nil +} + +func (g *GithubClient) IsSupportDownloadSingleFile(repo models.Repo) bool { + return true +} diff --git a/server/events/vcs/gitlab_client.go b/server/events/vcs/gitlab_client.go index 2b6b8e561e..f5207a1d2b 100644 --- a/server/events/vcs/gitlab_client.go +++ b/server/events/vcs/gitlab_client.go @@ -15,7 +15,9 @@ package vcs import ( "fmt" + "github.com/runatlantis/atlantis/server/events/yaml" "net" + "net/http" "net/url" "strings" @@ -269,3 +271,25 @@ func MustConstraint(constraint string) version.Constraints { } return c } + +// DownloadRepoConfigFile return `atlantis.yaml` content from VCS (which support fetch a single file from repository) +// 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 +func (g *GitlabClient) DownloadRepoConfigFile(pull models.PullRequest) (bool, []byte, error) { + opt := gitlab.GetRawFileOptions{Ref: gitlab.String(pull.HeadBranch)} + + bytes, resp, err := g.Client.RepositoryFiles.GetRawFile(pull.BaseRepo.FullName, yaml.AtlantisYAMLFilename, &opt) + if resp.StatusCode == http.StatusNotFound { + return false, []byte{}, nil + } + + if err != nil { + return true, []byte{}, err + } + + return true, bytes, nil +} + +func (g *GitlabClient) IsSupportDownloadSingleFile(repo models.Repo) bool { + return true +} diff --git a/server/events/vcs/mocks/mock_client.go b/server/events/vcs/mocks/mock_client.go index 4a0e9a669b..3ab189002d 100644 --- a/server/events/vcs/mocks/mock_client.go +++ b/server/events/vcs/mocks/mock_client.go @@ -161,6 +161,14 @@ 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) { + return false, []byte{}, nil +} + func (mock *MockClient) VerifyWasCalledOnce() *VerifierMockClient { return &VerifierMockClient{ mock: mock, diff --git a/server/events/vcs/not_configured_vcs_client.go b/server/events/vcs/not_configured_vcs_client.go index c8fe20ed9d..98cd9cc85d 100644 --- a/server/events/vcs/not_configured_vcs_client.go +++ b/server/events/vcs/not_configured_vcs_client.go @@ -53,3 +53,11 @@ func (a *NotConfiguredVCSClient) MarkdownPullLink(pull models.PullRequest) (stri 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 { + return false +} + +func (a *NotConfiguredVCSClient) DownloadRepoConfigFile(pull models.PullRequest) (bool, []byte, error) { + return true, []byte{}, a.err() +} diff --git a/server/events/vcs/proxy.go b/server/events/vcs/proxy.go index 6511bf2d99..c22bfb64c4 100644 --- a/server/events/vcs/proxy.go +++ b/server/events/vcs/proxy.go @@ -83,3 +83,11 @@ func (d *ClientProxy) MergePull(pull models.PullRequest) error { func (d *ClientProxy) MarkdownPullLink(pull models.PullRequest) (string, error) { return d.clients[pull.BaseRepo.VCSHost.Type].MarkdownPullLink(pull) } + +func (d *ClientProxy) DownloadRepoConfigFile(pull models.PullRequest) (bool, []byte, error) { + 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) +} diff --git a/server/events/yaml/parser_validator.go b/server/events/yaml/parser_validator.go index 0e2bd84a24..3bbbfda8cd 100644 --- a/server/events/yaml/parser_validator.go +++ b/server/events/yaml/parser_validator.go @@ -44,17 +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(absRepoDir string, globalCfg valid.GlobalCfg, repoID string) (valid.RepoCfg, error) { - 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) +func (p *ParserValidator) ParseRepoCfg(repoCfgData []byte, absRepoDir string, globalCfg valid.GlobalCfg, repoID string) (valid.RepoCfg, error) { + var configData []byte + var err error + + 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 } - // 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 } var rawConfig raw.RepoCfg diff --git a/server/events/yaml/parser_validator_test.go b/server/events/yaml/parser_validator_test.go index fd687317b8..214ea22790 100644 --- a/server/events/yaml/parser_validator_test.go +++ b/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("/not/exist", globalCfg, "") + _, err := r.ParseRepoCfg([]byte{}, "/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(tmpDir, globalCfg, "") + _, err := r.ParseRepoCfg([]byte{}, 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(tmpDir, globalCfg, "") + _, err = r.ParseRepoCfg([]byte{}, 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(tmpDir, globalCfg, "") + _, err = r.ParseRepoCfg([]byte{}, 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(tmpDir, globalCfg, "") + act, err := r.ParseRepoCfg([]byte{}, 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(tmpDir, valid.NewGlobalCfg(false, false, false), "repo_id") + _, err = r.ParseRepoCfg([]byte{}, 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(v2Dir, valid.NewGlobalCfg(true, false, false), "") + v2Cfg, err := p.ParseRepoCfg([]byte{}, 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(v3Dir, valid.NewGlobalCfg(true, false, false), "") + v3Cfg, err := p.ParseRepoCfg([]byte{}, 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) diff --git a/server/events_controller_e2e_test.go b/server/events_controller_e2e_test.go index ee4929a747..d3f51d9735 100644 --- a/server/events_controller_e2e_test.go +++ b/server/events_controller_e2e_test.go @@ -476,14 +476,15 @@ func setupE2E(t *testing.T, repoDir string) (server.EventsController, *vcsmocks. AllowForkPRs: allowForkPRs, AllowForkPRsFlag: "allow-fork-prs", ProjectCommandBuilder: &events.DefaultProjectCommandBuilder{ - ParserValidator: parser, - ProjectFinder: &events.DefaultProjectFinder{}, - VCSClient: e2eVCSClient, - WorkingDir: workingDir, - WorkingDirLocker: locker, - PendingPlanFinder: &events.DefaultPendingPlanFinder{}, - CommentBuilder: commentParser, - GlobalCfg: globalCfg, + ParserValidator: parser, + ProjectFinder: &events.DefaultProjectFinder{}, + VCSClient: e2eVCSClient, + WorkingDir: workingDir, + WorkingDirLocker: locker, + PendingPlanFinder: &events.DefaultPendingPlanFinder{}, + CommentBuilder: commentParser, + GlobalCfg: globalCfg, + SkipCloneNoChanges: false, }, DB: boltdb, PendingPlanFinder: &events.DefaultPendingPlanFinder{}, diff --git a/server/server.go b/server/server.go index 9a33e703ac..18983cceb4 100644 --- a/server/server.go +++ b/server/server.go @@ -370,14 +370,15 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { SilenceVCSStatusNoPlans: userConfig.SilenceVCSStatusNoPlans, DisableApplyAll: userConfig.DisableApplyAll, ProjectCommandBuilder: &events.DefaultProjectCommandBuilder{ - ParserValidator: validator, - ProjectFinder: &events.DefaultProjectFinder{}, - VCSClient: vcsClient, - WorkingDir: workingDir, - WorkingDirLocker: workingDirLocker, - GlobalCfg: globalCfg, - PendingPlanFinder: pendingPlanFinder, - CommentBuilder: commentParser, + ParserValidator: validator, + ProjectFinder: &events.DefaultProjectFinder{}, + VCSClient: vcsClient, + WorkingDir: workingDir, + WorkingDirLocker: workingDirLocker, + GlobalCfg: globalCfg, + PendingPlanFinder: pendingPlanFinder, + CommentBuilder: commentParser, + SkipCloneNoChanges: userConfig.SkipCloneNoChanges, }, ProjectCommandRunner: &events.DefaultProjectCommandRunner{ Locker: projectLocker, diff --git a/server/user_config.go b/server/user_config.go index 04acc6d6f0..c7e2d4b4f5 100644 --- a/server/user_config.go +++ b/server/user_config.go @@ -58,6 +58,7 @@ type UserConfig struct { SilenceAllowlistErrors bool `mapstructure:"silence-allowlist-errors"` // SilenceWhitelistErrors is deprecated in favour of SilenceAllowlistErrors SilenceWhitelistErrors bool `mapstructure:"silence-whitelist-errors"` + SkipCloneNoChanges bool `mapstructure:"skip-clone-no-changes"` SlackToken string `mapstructure:"slack-token"` SSLCertFile string `mapstructure:"ssl-cert-file"` SSLKeyFile string `mapstructure:"ssl-key-file"` From cbe0b18c0e8fc6cc8f7b40e808cb0ba06dbfc6cd Mon Sep 17 00:00:00 2001 From: Luke Kysow <1034429+lkysow@users.noreply.github.com> Date: Tue, 18 Aug 2020 15:45:30 -0700 Subject: [PATCH 2/2] Refactor --skip-clone-no-changes work. --- server/events/matchers/models_pullrequest.go | 1 + server/events/matchers/models_repo.go | 1 + .../matchers/ptr_to_logging_simplelogger.go | 1 + server/events/mock_workingdir_test.go | 5 +- server/events/project_command_builder.go | 12 ++- server/events/project_command_builder_test.go | 40 ++++++++ server/events/project_finder.go | 10 +- server/events/vcs/azuredevops_client.go | 2 +- server/events/vcs/bitbucketcloud/client.go | 2 +- server/events/vcs/bitbucketserver/client.go | 2 +- server/events/vcs/client.go | 2 +- server/events/vcs/github_client.go | 2 +- server/events/vcs/gitlab_client.go | 5 +- .../vcs/mocks/matchers/slice_of_byte.go | 20 ++++ server/events/vcs/mocks/mock_client.go | 92 ++++++++++++++++++- .../events/vcs/not_configured_vcs_client.go | 2 +- server/events/vcs/proxy.go | 4 +- server/events/yaml/parser_validator.go | 34 +++---- server/events/yaml/parser_validator_test.go | 16 ++-- 19 files changed, 203 insertions(+), 50 deletions(-) create mode 100644 server/events/vcs/mocks/matchers/slice_of_byte.go diff --git a/server/events/matchers/models_pullrequest.go b/server/events/matchers/models_pullrequest.go index dd1fb0d4ee..37e4780130 100644 --- a/server/events/matchers/models_pullrequest.go +++ b/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" ) diff --git a/server/events/matchers/models_repo.go b/server/events/matchers/models_repo.go index 418f13cfcf..e985fd3a90 100644 --- a/server/events/matchers/models_repo.go +++ b/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" ) diff --git a/server/events/matchers/ptr_to_logging_simplelogger.go b/server/events/matchers/ptr_to_logging_simplelogger.go index 04c72791bc..095fa65a72 100644 --- a/server/events/matchers/ptr_to_logging_simplelogger.go +++ b/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" ) diff --git a/server/events/mock_workingdir_test.go b/server/events/mock_workingdir_test.go index 8313ca5050..db1288f599 100644 --- a/server/events/mock_workingdir_test.go +++ b/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 { diff --git a/server/events/project_command_builder.go b/server/events/project_command_builder.go index ec8d2cad27..007034824d 100644 --- a/server/events/project_command_builder.go +++ b/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 } diff --git a/server/events/project_command_builder_test.go b/server/events/project_command_builder_test.go index c4848b04e6..50bbd51ca5 100644 --- a/server/events/project_command_builder_test.go +++ b/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()) +} diff --git a/server/events/project_finder.go b/server/events/project_finder.go index b48250da85..2c2dc92394 100644 --- a/server/events/project_finder.go +++ b/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) diff --git a/server/events/vcs/azuredevops_client.go b/server/events/vcs/azuredevops_client.go index f99719f3e9..15a26526a3 100644 --- a/server/events/vcs/azuredevops_client.go +++ b/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 } diff --git a/server/events/vcs/bitbucketcloud/client.go b/server/events/vcs/bitbucketcloud/client.go index 4e67c3840a..ff60faf6c6 100644 --- a/server/events/vcs/bitbucketcloud/client.go +++ b/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 } diff --git a/server/events/vcs/bitbucketserver/client.go b/server/events/vcs/bitbucketserver/client.go index f983c36140..adb6eba0d8 100644 --- a/server/events/vcs/bitbucketserver/client.go +++ b/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 } diff --git a/server/events/vcs/client.go b/server/events/vcs/client.go index 0ecda34a60..dad6fa1901 100644 --- a/server/events/vcs/client.go +++ b/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 } diff --git a/server/events/vcs/github_client.go b/server/events/vcs/github_client.go index 22f54ff77f..3181d06956 100644 --- a/server/events/vcs/github_client.go +++ b/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 } diff --git a/server/events/vcs/gitlab_client.go b/server/events/vcs/gitlab_client.go index f5207a1d2b..b991eaba30 100644 --- a/server/events/vcs/gitlab_client.go +++ b/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 } diff --git a/server/events/vcs/mocks/matchers/slice_of_byte.go b/server/events/vcs/mocks/matchers/slice_of_byte.go new file mode 100644 index 0000000000..9a9894fa07 --- /dev/null +++ b/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 +} diff --git a/server/events/vcs/mocks/mock_client.go b/server/events/vcs/mocks/mock_client.go index 3ab189002d..0b20b49b63 100644 --- a/server/events/vcs/mocks/mock_client.go +++ b/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 +} diff --git a/server/events/vcs/not_configured_vcs_client.go b/server/events/vcs/not_configured_vcs_client.go index 98cd9cc85d..7ce68a3a03 100644 --- a/server/events/vcs/not_configured_vcs_client.go +++ b/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 } diff --git a/server/events/vcs/proxy.go b/server/events/vcs/proxy.go index c22bfb64c4..a583d70cc0 100644 --- a/server/events/vcs/proxy.go +++ b/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) } diff --git a/server/events/yaml/parser_validator.go b/server/events/yaml/parser_validator.go index 3bbbfda8cd..d21b221ccd 100644 --- a/server/events/yaml/parser_validator.go +++ b/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 - - 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 +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 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 } diff --git a/server/events/yaml/parser_validator_test.go b/server/events/yaml/parser_validator_test.go index 214ea22790..fd687317b8 100644 --- a/server/events/yaml/parser_validator_test.go +++ b/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)