From a0ca05dc148f132c8b88d20f150bffe8b6065a1a Mon Sep 17 00:00:00 2001 From: Matt Ulmer <25484774+Ulminator@users.noreply.github.com> Date: Tue, 24 Oct 2023 22:10:53 +0000 Subject: [PATCH] fix: Enable --skip-clone-no-changes for fork PRs (#3891) --- server/events/project_command_builder.go | 12 +++++++++++- server/events/project_command_builder_test.go | 2 +- 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 | 9 +++++---- server/events/vcs/gitlab_client.go | 9 +++++---- server/events/vcs/mocks/mock_client.go | 8 ++++---- server/events/vcs/not_configured_vcs_client.go | 2 +- server/events/vcs/proxy.go | 4 ++-- 11 files changed, 33 insertions(+), 21 deletions(-) diff --git a/server/events/project_command_builder.go b/server/events/project_command_builder.go index 2347c9072a..47696bd255 100644 --- a/server/events/project_command_builder.go +++ b/server/events/project_command_builder.go @@ -338,7 +338,17 @@ func (p *DefaultProjectCommandBuilder) buildAllCommandsByCfg(ctx *command.Contex if p.SkipCloneNoChanges && p.VCSClient.SupportsSingleFileDownload(ctx.Pull.BaseRepo) { repoCfgFile := p.GlobalCfg.RepoConfigFile(ctx.Pull.BaseRepo.ID()) - hasRepoCfg, repoCfgData, err := p.VCSClient.GetFileContent(ctx.Pull, repoCfgFile) + + var hasRepoCfg bool + var repoCfgData []byte + var err error + if ctx.HeadRepo.Owner != ctx.Pull.BaseRepo.Owner { + hasRepoCfg, repoCfgData, err = p.VCSClient.GetFileContent(ctx.HeadRepo, ctx.Pull.HeadBranch, repoCfgFile) + + } else { + hasRepoCfg, repoCfgData, err = p.VCSClient.GetFileContent(ctx.Pull.BaseRepo, ctx.Pull.HeadBranch, repoCfgFile) + } + if err != nil { return nil, errors.Wrapf(err, "downloading %s", repoCfgFile) } diff --git a/server/events/project_command_builder_test.go b/server/events/project_command_builder_test.go index 6a3e73798d..abbe434b76 100644 --- a/server/events/project_command_builder_test.go +++ b/server/events/project_command_builder_test.go @@ -1533,7 +1533,7 @@ parallel_plan: true`, vcsClient := vcsmocks.NewMockClient() When(vcsClient.GetModifiedFiles(Any[models.Repo](), Any[models.PullRequest]())).ThenReturn(c.ModifiedFiles, nil) When(vcsClient.SupportsSingleFileDownload(Any[models.Repo]())).ThenReturn(true) - When(vcsClient.GetFileContent(Any[models.PullRequest](), Any[string]())).ThenReturn(true, []byte(c.AtlantisYAML), nil) + When(vcsClient.GetFileContent(Any[models.Repo](), Any[string](), Any[string]())).ThenReturn(true, []byte(c.AtlantisYAML), nil) workingDir := mocks.NewMockWorkingDir() logger := logging.NewNoopLogger(t) diff --git a/server/events/vcs/azuredevops_client.go b/server/events/vcs/azuredevops_client.go index 5ff3f3ff38..dfe2bc3d11 100644 --- a/server/events/vcs/azuredevops_client.go +++ b/server/events/vcs/azuredevops_client.go @@ -398,7 +398,7 @@ func (g *AzureDevopsClient) SupportsSingleFileDownload(repo models.Repo) bool { return false } -func (g *AzureDevopsClient) GetFileContent(pull models.PullRequest, fileName string) (bool, []byte, error) { +func (g *AzureDevopsClient) GetFileContent(repo models.Repo, branch string, fileName string) (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 fa1751db19..76ff838af2 100644 --- a/server/events/vcs/bitbucketcloud/client.go +++ b/server/events/vcs/bitbucketcloud/client.go @@ -274,7 +274,7 @@ func (b *Client) SupportsSingleFileDownload(models.Repo) bool { // GetFileContent a repository file content from VCS (which support fetch a single file from repository) // The first return value indicates whether the repo contains a file or not // if BaseRepo had a file, its content will placed on the second return value -func (b *Client) GetFileContent(pull models.PullRequest, fileName string) (bool, []byte, error) { +func (b *Client) GetFileContent(repo models.Repo, branch string, fileName string) (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 943fd3b6c8..486191b905 100644 --- a/server/events/vcs/bitbucketserver/client.go +++ b/server/events/vcs/bitbucketserver/client.go @@ -358,7 +358,7 @@ func (b *Client) SupportsSingleFileDownload(repo models.Repo) bool { // GetFileContent a repository file content from VCS (which support fetch a single file from repository) // The first return value indicates whether the repo contains a file or not // if BaseRepo had a file, its content will placed on the second return value -func (b *Client) GetFileContent(pull models.PullRequest, fileName string) (bool, []byte, error) { +func (b *Client) GetFileContent(repo models.Repo, branch string, fileName string) (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 dd2e489f6f..79db335dc4 100644 --- a/server/events/vcs/client.go +++ b/server/events/vcs/client.go @@ -46,7 +46,7 @@ type Client interface { // GetFileContent a repository file content from VCS (which support fetch a single file from repository) // The first return value indicates whether the repo contains a file or not // if BaseRepo had a file, its content will placed on the second return value - GetFileContent(pull models.PullRequest, fileName string) (bool, []byte, error) + GetFileContent(repo models.Repo, branch string, fileName string) (bool, []byte, error) SupportsSingleFileDownload(repo models.Repo) bool GetCloneURL(VCSHostType models.VCSHostType, repo string) (string, error) diff --git a/server/events/vcs/github_client.go b/server/events/vcs/github_client.go index ab075fba03..cf3f457575 100644 --- a/server/events/vcs/github_client.go +++ b/server/events/vcs/github_client.go @@ -701,10 +701,11 @@ func (g *GithubClient) ExchangeCode(code string) (*GithubAppTemporarySecrets, er // GetFileContent a repository file content from VCS (which support fetch a single file from repository) // The first return value indicates whether the repo contains a file or not // if BaseRepo had a file, its content will placed on the second return value -func (g *GithubClient) GetFileContent(pull models.PullRequest, fileName string) (bool, []byte, error) { - opt := github.RepositoryContentGetOptions{Ref: pull.HeadBranch} - fileContent, _, resp, err := g.client.Repositories.GetContents(g.ctx, pull.BaseRepo.Owner, pull.BaseRepo.Name, fileName, &opt) - g.logger.Debug("GET /repos/%v/%v/contents/%s returned: %v", pull.BaseRepo.Owner, pull.BaseRepo.Name, fileName, resp.StatusCode) +func (g *GithubClient) GetFileContent(repo models.Repo, branch string, fileName string) (bool, []byte, error) { + opt := github.RepositoryContentGetOptions{Ref: branch} + + fileContent, _, resp, err := g.client.Repositories.GetContents(g.ctx, repo.Owner, repo.Name, fileName, &opt) + g.logger.Debug("GET /repos/%v/%v/contents/%s returned: %v", repo.Owner, repo.Name, fileName, resp.StatusCode) if resp.StatusCode == http.StatusNotFound { return false, []byte{}, nil diff --git a/server/events/vcs/gitlab_client.go b/server/events/vcs/gitlab_client.go index d5d89ab529..be5d4b785a 100644 --- a/server/events/vcs/gitlab_client.go +++ b/server/events/vcs/gitlab_client.go @@ -504,11 +504,12 @@ func (g *GitlabClient) GetTeamNamesForUser(repo models.Repo, user models.User) ( // GetFileContent a repository file content from VCS (which support fetch a single file from repository) // The first return value indicates whether the repo contains a file or not // if BaseRepo had a file, its content will placed on the second return value -func (g *GitlabClient) GetFileContent(pull models.PullRequest, fileName string) (bool, []byte, error) { - opt := gitlab.GetRawFileOptions{Ref: gitlab.String(pull.HeadBranch)} +func (g *GitlabClient) GetFileContent(repo models.Repo, branch string, fileName string) (bool, []byte, error) { - bytes, resp, err := g.Client.RepositoryFiles.GetRawFile(pull.BaseRepo.FullName, fileName, &opt) - g.logger.Debug("GET /projects/%s/repository/files/%s/raw returned: %d", pull.BaseRepo.FullName, fileName, resp.StatusCode) + opt := gitlab.GetRawFileOptions{Ref: gitlab.String(branch)} + + bytes, resp, err := g.Client.RepositoryFiles.GetRawFile(repo.FullName, fileName, &opt) + g.logger.Debug("GET /projects/%s/repository/files/%s/raw returned: %d", repo.FullName, fileName, resp.StatusCode) if resp.StatusCode == http.StatusNotFound { return false, []byte{}, nil } diff --git a/server/events/vcs/mocks/mock_client.go b/server/events/vcs/mocks/mock_client.go index 4150e5ffeb..97e796d0bc 100644 --- a/server/events/vcs/mocks/mock_client.go +++ b/server/events/vcs/mocks/mock_client.go @@ -74,11 +74,11 @@ func (mock *MockClient) GetCloneURL(VCSHostType models.VCSHostType, repo string) return ret0, ret1 } -func (mock *MockClient) GetFileContent(pull models.PullRequest, fileName string) (bool, []byte, error) { +func (mock *MockClient) GetFileContent(repo models.Repo, branch string, fileName string) (bool, []byte, error) { if mock == nil { panic("mock must not be nil. Use myMock := NewMockClient().") } - params := []pegomock.Param{pull, fileName} + params := []pegomock.Param{repo, branch, fileName} result := pegomock.GetGenericMockFrom(mock).Invoke("GetFileContent", params, []reflect.Type{reflect.TypeOf((*bool)(nil)).Elem(), reflect.TypeOf((*[]byte)(nil)).Elem(), reflect.TypeOf((*error)(nil)).Elem()}) var ret0 bool var ret1 []byte @@ -431,8 +431,8 @@ func (c *MockClient_GetCloneURL_OngoingVerification) GetAllCapturedArguments() ( return } -func (verifier *VerifierMockClient) GetFileContent(pull models.PullRequest, fileName string) *MockClient_GetFileContent_OngoingVerification { - params := []pegomock.Param{pull, fileName} +func (verifier *VerifierMockClient) GetFileContent(repo models.Repo, branch string, fileName string) *MockClient_GetFileContent_OngoingVerification { + params := []pegomock.Param{repo, branch, fileName} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "GetFileContent", params, verifier.timeout) return &MockClient_GetFileContent_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } diff --git a/server/events/vcs/not_configured_vcs_client.go b/server/events/vcs/not_configured_vcs_client.go index 6ce6f2da56..a6e34fe37b 100644 --- a/server/events/vcs/not_configured_vcs_client.go +++ b/server/events/vcs/not_configured_vcs_client.go @@ -67,7 +67,7 @@ func (a *NotConfiguredVCSClient) SupportsSingleFileDownload(repo models.Repo) bo return false } -func (a *NotConfiguredVCSClient) GetFileContent(pull models.PullRequest, fileName string) (bool, []byte, error) { +func (a *NotConfiguredVCSClient) GetFileContent(repo models.Repo, branch string, fileName string) (bool, []byte, error) { return true, []byte{}, a.err() } func (a *NotConfiguredVCSClient) GetCloneURL(VCSHostType models.VCSHostType, repo string) (string, error) { diff --git a/server/events/vcs/proxy.go b/server/events/vcs/proxy.go index 25637bcd0f..0c964111d0 100644 --- a/server/events/vcs/proxy.go +++ b/server/events/vcs/proxy.go @@ -96,8 +96,8 @@ func (d *ClientProxy) GetTeamNamesForUser(repo models.Repo, user models.User) ([ return d.clients[repo.VCSHost.Type].GetTeamNamesForUser(repo, user) } -func (d *ClientProxy) GetFileContent(pull models.PullRequest, fileName string) (bool, []byte, error) { - return d.clients[pull.BaseRepo.VCSHost.Type].GetFileContent(pull, fileName) +func (d *ClientProxy) GetFileContent(repo models.Repo, branch string, fileName string) (bool, []byte, error) { + return d.clients[repo.VCSHost.Type].GetFileContent(repo, branch, fileName) } func (d *ClientProxy) SupportsSingleFileDownload(repo models.Repo) bool {