From 112afe957914dfadca9f4838b82eecf54f06e964 Mon Sep 17 00:00:00 2001 From: Luke Kysow <1034429+lkysow@users.noreply.github.com> Date: Thu, 2 Jan 2020 09:22:13 -0600 Subject: [PATCH 1/2] Allow absolute project dirs Previously, we wouldn't modify the config project: /a/b/c and would use the absolute directory. Now we prepend ./ and then run Clean() to ensure that every project dir is relative. --- server/events/yaml/raw/project.go | 8 ++-- server/events/yaml/raw/project_test.go | 60 +++++++++++++++++++++++++- 2 files changed, 63 insertions(+), 5 deletions(-) diff --git a/server/events/yaml/raw/project.go b/server/events/yaml/raw/project.go index d30ee0147..e19fe6e5c 100644 --- a/server/events/yaml/raw/project.go +++ b/server/events/yaml/raw/project.go @@ -67,10 +67,10 @@ func (p Project) Validate() error { func (p Project) ToValid() valid.Project { var v valid.Project - cleanedDir := filepath.Clean(*p.Dir) - if cleanedDir == "/" { - cleanedDir = "." - } + // Prepend ./ and then run .Clean() so we're guaranteed to have a relative + // directory. This is necessary because we use this dir without sanitation + // in DefaultProjectFinder. + cleanedDir := filepath.Clean("./" + *p.Dir) v.Dir = cleanedDir if p.Workspace == nil || *p.Workspace == "" { diff --git a/server/events/yaml/raw/project_test.go b/server/events/yaml/raw/project_test.go index 8da75c01f..626f09b54 100644 --- a/server/events/yaml/raw/project_test.go +++ b/server/events/yaml/raw/project_test.go @@ -281,8 +281,9 @@ func TestProject_ToValid(t *testing.T) { }, }, }, + // Directories. { - description: "dir with /", + description: "dir set to /", input: raw.Project{ Dir: String("/"), }, @@ -295,6 +296,20 @@ func TestProject_ToValid(t *testing.T) { }, }, }, + { + description: "dir starting with /", + input: raw.Project{ + Dir: String("/a/b/c"), + }, + exp: valid.Project{ + Dir: "a/b/c", + Workspace: "default", + Autoplan: valid.Autoplan{ + WhenModified: []string{"**/*.tf*"}, + Enabled: true, + }, + }, + }, { description: "dir with trailing slash", input: raw.Project{ @@ -324,6 +339,49 @@ func TestProject_ToValid(t *testing.T) { }, }, }, + { + description: "dir set to ./", + input: raw.Project{ + Dir: String("./"), + }, + exp: valid.Project{ + Dir: ".", + Workspace: "default", + Autoplan: valid.Autoplan{ + WhenModified: []string{"**/*.tf*"}, + Enabled: true, + }, + }, + }, + { + description: "dir set to ././", + input: raw.Project{ + Dir: String("././"), + }, + exp: valid.Project{ + Dir: ".", + Workspace: "default", + Autoplan: valid.Autoplan{ + WhenModified: []string{"**/*.tf*"}, + Enabled: true, + }, + }, + }, + { + description: "dir set to .", + input: raw.Project{ + Dir: String("."), + }, + exp: valid.Project{ + Dir: ".", + Workspace: "default", + Autoplan: valid.Autoplan{ + WhenModified: []string{"**/*.tf*"}, + Enabled: true, + }, + }, + }, + { description: "workspace set to empty string", input: raw.Project{ From 4e8d19be0a9a042093c4c3ad9ceee6419355cfa0 Mon Sep 17 00:00:00 2001 From: Luke Kysow <1034429+lkysow@users.noreply.github.com> Date: Thu, 2 Jan 2020 09:45:38 -0600 Subject: [PATCH 2/2] Convert AZDO paths to relative Previously the AZDO paths returned by GetModifiedFiles were absolute. Now they're relative to the repo root to match the rest of the VCS's. --- server/events/vcs/azuredevops_client.go | 13 +++++++++---- server/events/vcs/azuredevops_client_test.go | 2 +- server/events/vcs/bitbucketcloud/client.go | 4 ++-- server/events/vcs/bitbucketserver/client.go | 4 ++-- server/events/vcs/client.go | 2 ++ server/events/vcs/github_client.go | 4 ++-- server/events/vcs/gitlab_client.go | 4 ++-- 7 files changed, 20 insertions(+), 13 deletions(-) diff --git a/server/events/vcs/azuredevops_client.go b/server/events/vcs/azuredevops_client.go index 40ab1fc8a..c5b6b724c 100644 --- a/server/events/vcs/azuredevops_client.go +++ b/server/events/vcs/azuredevops_client.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "net/url" + "path/filepath" "strings" "time" @@ -49,8 +50,8 @@ func NewAzureDevopsClient(hostname string, token string) (*AzureDevopsClient, er return client, nil } -// GetModifiedFiles returns the names of files that were modified in the merge request. -// The names include the path to the file from the repo root, ex. parent/child/file.txt. +// GetModifiedFiles returns the names of files that were modified in the merge request +// relative to the repo root, e.g. parent/child/file.txt. func (g *AzureDevopsClient) GetModifiedFiles(repo models.Repo, pull models.PullRequest) ([]string, error) { var files []string @@ -66,13 +67,17 @@ func (g *AzureDevopsClient) GetModifiedFiles(repo models.Repo, pull models.PullR for _, change := range r.Changes { item := change.GetItem() - files = append(files, item.GetPath()) + // Convert the path to a relative path from the repo's root. + relativePath := filepath.Clean("./" + item.GetPath()) + files = append(files, relativePath) // If the file was renamed, we'll want to run plan in the directory // it was moved from as well. changeType := azuredevops.Rename.String() if change.ChangeType == &changeType { - files = append(files, change.GetSourceServerItem()) + // Convert the path to a relative path from the repo's root. + relativePath = filepath.Clean("./" + change.GetSourceServerItem()) + files = append(files, relativePath) } } diff --git a/server/events/vcs/azuredevops_client_test.go b/server/events/vcs/azuredevops_client_test.go index 1c91b0d65..8b6edb926 100644 --- a/server/events/vcs/azuredevops_client_test.go +++ b/server/events/vcs/azuredevops_client_test.go @@ -203,7 +203,7 @@ func TestAzureDevopsClient_GetModifiedFiles(t *testing.T) { "changeType": "add" } ]}` - resp := fmt.Sprintf(itemRespTemplate, "file1.txt", "file1.txt", "file2.txt", "file2.txt") + resp := fmt.Sprintf(itemRespTemplate, "/file1.txt", "/file1.txt", "/file2.txt", "/file2.txt") testServer := httptest.NewTLSServer( http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch r.RequestURI { diff --git a/server/events/vcs/bitbucketcloud/client.go b/server/events/vcs/bitbucketcloud/client.go index 3bb48b2ce..d55b7c598 100644 --- a/server/events/vcs/bitbucketcloud/client.go +++ b/server/events/vcs/bitbucketcloud/client.go @@ -38,8 +38,8 @@ func NewClient(httpClient *http.Client, username string, password string, atlant } } -// GetModifiedFiles returns the names of files that were modified in the merge request. -// The names include the path to the file from the repo root, ex. parent/child/file.txt. +// GetModifiedFiles returns the names of files that were modified in the merge request +// relative to the repo root, e.g. parent/child/file.txt. func (b *Client) GetModifiedFiles(repo models.Repo, pull models.PullRequest) ([]string, error) { var files []string diff --git a/server/events/vcs/bitbucketserver/client.go b/server/events/vcs/bitbucketserver/client.go index 27796198e..bff3b6258 100644 --- a/server/events/vcs/bitbucketserver/client.go +++ b/server/events/vcs/bitbucketserver/client.go @@ -58,8 +58,8 @@ func NewClient(httpClient *http.Client, username string, password string, baseUR }, nil } -// GetModifiedFiles returns the names of files that were modified in the merge request. -// The names include the path to the file from the repo root, ex. parent/child/file.txt. +// GetModifiedFiles returns the names of files that were modified in the merge request +// relative to the repo root, e.g. parent/child/file.txt. func (b *Client) GetModifiedFiles(repo models.Repo, pull models.PullRequest) ([]string, error) { var files []string diff --git a/server/events/vcs/client.go b/server/events/vcs/client.go index 2b72fc214..153af1265 100644 --- a/server/events/vcs/client.go +++ b/server/events/vcs/client.go @@ -21,6 +21,8 @@ import ( // Client is used to make API calls to a VCS host like GitHub or GitLab. type Client interface { + // GetModifiedFiles returns the names of files that were modified in the merge request + // relative to the repo root, e.g. parent/child/file.txt. GetModifiedFiles(repo models.Repo, pull models.PullRequest) ([]string, error) CreateComment(repo models.Repo, pullNum int, comment string) error PullIsApproved(repo models.Repo, pull models.PullRequest) (bool, error) diff --git a/server/events/vcs/github_client.go b/server/events/vcs/github_client.go index 2f66699b2..11220b195 100644 --- a/server/events/vcs/github_client.go +++ b/server/events/vcs/github_client.go @@ -61,8 +61,8 @@ func NewGithubClient(hostname string, user string, pass string) (*GithubClient, }, nil } -// GetModifiedFiles returns the names of files that were modified in the pull request. -// The names include the path to the file from the repo root, ex. parent/child/file.txt. +// GetModifiedFiles returns the names of files that were modified in the pull request +// relative to the repo root, e.g. parent/child/file.txt. func (g *GithubClient) GetModifiedFiles(repo models.Repo, pull models.PullRequest) ([]string, error) { var files []string nextPage := 0 diff --git a/server/events/vcs/gitlab_client.go b/server/events/vcs/gitlab_client.go index c96550adb..002822805 100644 --- a/server/events/vcs/gitlab_client.go +++ b/server/events/vcs/gitlab_client.go @@ -92,8 +92,8 @@ func NewGitlabClient(hostname string, token string, logger *logging.SimpleLogger return client, nil } -// GetModifiedFiles returns the names of files that were modified in the merge request. -// The names include the path to the file from the repo root, ex. parent/child/file.txt. +// GetModifiedFiles returns the names of files that were modified in the merge request +// relative to the repo root, e.g. parent/child/file.txt. func (g *GitlabClient) GetModifiedFiles(repo models.Repo, pull models.PullRequest) ([]string, error) { const maxPerPage = 100 var files []string