Skip to content

Commit

Permalink
fix: add path to WorkingDir methods (runatlantis#2180)
Browse files Browse the repository at this point in the history
* fix: add path to WorkingDir methods

`WorkingDirLocker` and `WorkingDir` are closely related -- the former acquires a lock, which ensures that the latter can safely operate on a given file path.

In runatlantis#2131 we added `path` to the `WorkingDirLocker` lock key, but neglected to add the same to `WorkingDir`.

This commit adds `path` as an argument to certain `WorkingDir` methods, and includes `path` in the directory that we use to clone the repository for a given project.

Since `path` can include certain special characters such as `/`, we encode `path` as base32 when using it as part of a file path. This should ensure no special characters are used in the filesystem, and that the value can be decoded if desired (unlike hashes such as md5).

Additional changes:

- All calls to changed methods have been updated, including unit tests
- Mocks have been regenerated for `WorkingDir` and `WorkingDirLocker`
- `working_dir_test.go`
  - Commands that operate on filesystem paths have been updated to include the base32 encoded `path` value
  - When running `git init`, we include `-b master` as additional arguments, to ensure that `master` is our default branch (expected by the unit tests)

* Try fixing E2E tests

* Fix DefaultPendingPlanFinder

In addition to iterating over `workspaceDirs`, also iterate over `pathDirs`, and use both `workspace` and `path` to build `repoDir`.

* Fix DefaultPendingPlanFinder unit tests

* Fix DefaultProjectCommandBuilder unit tests

Co-authored-by: Kevin Snyder <[email protected]>
Co-authored-by: Kevin Snyder <[email protected]>
  • Loading branch information
3 people authored Apr 8, 2022
1 parent 25eea31 commit 1a83444
Show file tree
Hide file tree
Showing 28 changed files with 458 additions and 345 deletions.
6 changes: 3 additions & 3 deletions server/controllers/events/events_controller_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,7 @@ func TestSimlpleWorkflow_terraformLockFile(t *testing.T) {
ResponseContains(t, w, 200, "Processing...")

// check lock file content
actualLockFileContent, err := os.ReadFile(fmt.Sprintf("%s/repos/runatlantis/atlantis-tests/2/default/.terraform.lock.hcl", atlantisWorkspace.DataDir))
actualLockFileContent, err := os.ReadFile(fmt.Sprintf("%s/repos/runatlantis/atlantis-tests/2/default/FY======/.terraform.lock.hcl", atlantisWorkspace.DataDir))
Ok(t, err)
if c.LockFileTracked {
if string(oldLockFileContent) != string(actualLockFileContent) {
Expand All @@ -588,7 +588,7 @@ func TestSimlpleWorkflow_terraformLockFile(t *testing.T) {
if !c.LockFileTracked {
// replace the lock file generated by the previous init to simulate
// dependcies needing updating in a latter plan
runCmd(t, "", "cp", oldLockFilePath, fmt.Sprintf("%s/repos/runatlantis/atlantis-tests/2/default/.terraform.lock.hcl", atlantisWorkspace.DataDir))
runCmd(t, "", "cp", oldLockFilePath, fmt.Sprintf("%s/repos/runatlantis/atlantis-tests/2/default/FY======/.terraform.lock.hcl", atlantisWorkspace.DataDir))
}

// Now send any other comments.
Expand All @@ -600,7 +600,7 @@ func TestSimlpleWorkflow_terraformLockFile(t *testing.T) {
}

// check lock file content
actualLockFileContent, err = os.ReadFile(fmt.Sprintf("%s/repos/runatlantis/atlantis-tests/2/default/.terraform.lock.hcl", atlantisWorkspace.DataDir))
actualLockFileContent, err = os.ReadFile(fmt.Sprintf("%s/repos/runatlantis/atlantis-tests/2/default/FY======/.terraform.lock.hcl", atlantisWorkspace.DataDir))
Ok(t, err)
if c.LockFileTracked {
if string(oldLockFileContent) != string(actualLockFileContent) {
Expand Down
2 changes: 1 addition & 1 deletion server/controllers/locks_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ func (l *LocksController) DeleteLock(w http.ResponseWriter, r *http.Request) {
} else {
defer unlock()
// nolint: vetshadow
if err := l.WorkingDir.DeleteForWorkspace(lock.Pull.BaseRepo, lock.Pull, lock.Workspace); err != nil {
if err := l.WorkingDir.DeleteForWorkspace(lock.Pull.BaseRepo, lock.Pull, lock.Workspace, lock.Project.Path); err != nil {
l.Logger.Err("unable to delete workspace: %s", err)
}
}
Expand Down
2 changes: 1 addition & 1 deletion server/events/delete_lock_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func (l *DefaultDeleteLockCommand) deleteWorkingDir(lock models.ProjectLock) {
} else {
defer unlock()
// nolint: vetshadow
if err := l.WorkingDir.DeleteForWorkspace(lock.Pull.BaseRepo, lock.Pull, lock.Workspace); err != nil {
if err := l.WorkingDir.DeleteForWorkspace(lock.Pull.BaseRepo, lock.Pull, lock.Workspace, lock.Project.Path); err != nil {
l.Logger.Err("unable to delete workspace: %s", err)
}
}
Expand Down
2 changes: 1 addition & 1 deletion server/events/delete_lock_command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func TestDeleteLock_Success(t *testing.T) {
lock, err := dlc.DeleteLock("id")
Ok(t, err)
Assert(t, lock != nil, "lock was nil")
workingDir.VerifyWasCalledOnce().DeleteForWorkspace(pull.BaseRepo, pull, "workspace")
workingDir.VerifyWasCalledOnce().DeleteForWorkspace(pull.BaseRepo, pull, "workspace", "path")
}

func TestDeleteLocksByPull_LockerErr(t *testing.T) {
Expand Down
4 changes: 2 additions & 2 deletions server/events/github_app_working_dir.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ type GithubAppWorkingDir struct {
}

// Clone writes a fresh token for Github App authentication
func (g *GithubAppWorkingDir) Clone(log logging.SimpleLogging, headRepo models.Repo, p models.PullRequest, workspace string) (string, bool, error) {
func (g *GithubAppWorkingDir) Clone(log logging.SimpleLogging, headRepo models.Repo, p models.PullRequest, workspace string, path string) (string, bool, error) {

log.Info("Refreshing git tokens for Github App")

Expand Down Expand Up @@ -51,5 +51,5 @@ func (g *GithubAppWorkingDir) Clone(log logging.SimpleLogging, headRepo models.R
headRepo.CloneURL = strings.Replace(headRepo.CloneURL, "://:", authURL, 1)
headRepo.SanitizedCloneURL = strings.Replace(baseRepo.SanitizedCloneURL, "://:", "://x-access-token:", 1)

return g.WorkingDir.Clone(log, headRepo, p, workspace)
return g.WorkingDir.Clone(log, headRepo, p, workspace, path)
}
6 changes: 3 additions & 3 deletions server/events/github_app_working_dir_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func TestClone_GithubAppNoneExisting(t *testing.T) {
cloneDir, _, err := gwd.Clone(logger, models.Repo{}, models.PullRequest{
BaseRepo: models.Repo{},
HeadBranch: "branch",
}, "default")
}, "default", ".")
Ok(t, err)

// Use rev-parse to verify at correct commit.
Expand Down Expand Up @@ -88,11 +88,11 @@ func TestClone_GithubAppSetsCorrectUrl(t *testing.T) {
modifiedBaseRepo.SanitizedCloneURL = "https://x-access-token:<redacted>@github.com/runatlantis/atlantis.git"

When(credentials.GetToken()).ThenReturn("token", nil)
When(workingDir.Clone(logger, modifiedBaseRepo, models.PullRequest{BaseRepo: modifiedBaseRepo}, "default")).ThenReturn(
When(workingDir.Clone(logger, modifiedBaseRepo, models.PullRequest{BaseRepo: modifiedBaseRepo}, "default", ".")).ThenReturn(
"", true, nil,
)

_, success, _ := ghAppWorkingDir.Clone(logger, headRepo, models.PullRequest{BaseRepo: baseRepo}, "default")
_, success, _ := ghAppWorkingDir.Clone(logger, headRepo, models.PullRequest{BaseRepo: baseRepo}, "default", ".")

Assert(t, success == true, "clone url mutation error")
}
3 changes: 1 addition & 2 deletions server/events/matchers/logging_simplelogging.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions server/events/matchers/models_pullrequest.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions server/events/matchers/models_repo.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

114 changes: 84 additions & 30 deletions server/events/mock_workingdir_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions server/events/mocks/matchers/logging_simplelogging.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions server/events/mocks/matchers/models_pullrequest.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions server/events/mocks/matchers/models_repo.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 1a83444

Please sign in to comment.