Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Clone only once per PR #2921

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions server/controllers/events/events_controller_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,7 @@ func TestSimpleWorkflow_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/.terraform.lock.hcl", atlantisWorkspace.DataDir))
Ok(t, err)
if c.LockFileTracked {
if string(oldLockFileContent) != string(actualLockFileContent) {
Expand All @@ -705,7 +705,7 @@ func TestSimpleWorkflow_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/.terraform.lock.hcl", atlantisWorkspace.DataDir))
}

// Now send any other comments.
Expand All @@ -717,7 +717,7 @@ func TestSimpleWorkflow_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/.terraform.lock.hcl", atlantisWorkspace.DataDir))
Ok(t, err)
if c.LockFileTracked {
if string(oldLockFileContent) != string(actualLockFileContent) {
Expand Down Expand Up @@ -1105,7 +1105,10 @@ func setupE2E(t *testing.T, repoDir string, opt setupOption) (events_controllers
workingDir,
locker,
globalCfg,
&events.DefaultPendingPlanFinder{},
&events.DefaultPendingPlanFinder{
Backend: backend,
WorkingDir: workingDir,
},
commentParser,
false,
false,
Expand Down Expand Up @@ -1195,7 +1198,10 @@ func setupE2E(t *testing.T, repoDir string, opt setupOption) (events_controllers
false,
false,
e2eVCSClient,
&events.DefaultPendingPlanFinder{},
&events.DefaultPendingPlanFinder{
Backend: backend,
WorkingDir: workingDir,
},
workingDir,
e2eStatusUpdater,
projectCommandBuilder,
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1,7 @@
Ran Apply for 0 projects:
Ran Apply for dir: `.` workspace: `default`

**Apply Error**
```
no plan found at path "." and workspace "default"–did you run plan?

```
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
Ran Apply for 2 projects:

1. dir: `infrastructure/production` workspace: `default`
1. dir: `infrastructure/staging` workspace: `default`
1. dir: `infrastructure/production` workspace: `default`

### 1. dir: `infrastructure/production` workspace: `default`
### 1. dir: `infrastructure/staging` workspace: `default`
```diff
null_resource.production[0]: Creating...
null_resource.production[0]: Creation complete after *s [id=*******************]
null_resource.staging[0]: Creating...
null_resource.staging[0]: Creation complete after *s [id=*******************]

Apply complete! Resources: 1 added, 0 changed, 0 destroyed.
```

---
### 2. dir: `infrastructure/staging` workspace: `default`
### 2. dir: `infrastructure/production` workspace: `default`
```diff
null_resource.staging[0]: Creating...
null_resource.staging[0]: Creation complete after *s [id=*******************]
null_resource.production[0]: Creating...
null_resource.production[0]: Creation complete after *s [id=*******************]

Apply complete! Resources: 1 added, 0 changed, 0 destroyed.
```
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,14 @@ workflows:
default:
plan:
steps:
- run: rm -rf .terraform
- init:
extra_args: [-backend-config=default.backend.tfvars]
extra_args: [-reconfigure, -backend-config=default.backend.tfvars]
- plan:
extra_args: [-var-file=default.tfvars]
staging:
plan:
steps:
- run: rm -rf .terraform
- init:
extra_args: [-backend-config=staging.backend.tfvars]
extra_args: [-reconfigure, -backend-config=staging.backend.tfvars]
- plan:
extra_args: [-var-file, staging.tfvars]
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,15 @@ workflows:
default:
plan:
steps:
- run: rm -rf .terraform
- init:
extra_args: [-backend-config=default.backend.tfvars]
extra_args: [-reconfigure, -backend-config=default.backend.tfvars]
- plan:
extra_args: [-var-file=default.tfvars]
- run: echo workspace=$WORKSPACE
staging:
plan:
steps:
- run: rm -rf .terraform
- init:
extra_args: [-backend-config=staging.backend.tfvars]
extra_args: [-reconfigure, -backend-config=staging.backend.tfvars]
- plan:
extra_args: [-var-file, staging.tfvars]
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ version: 3
parallel_plan: false
parallel_apply: true
projects:
- dir: production
- dir: .
workspace: production
autoplan:
when_modified: ["**/*.tf*"]
- dir: staging
- dir: .
workspace: staging
autoplan:
when_modified: ["**/*.tf*"]
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
Ran Plan for 2 projects:

1. dir: `production` workspace: `production`
1. dir: `staging` workspace: `staging`
1. dir: `.` workspace: `production`
1. dir: `.` workspace: `staging`

### 1. dir: `production` workspace: `production`
### 1. dir: `.` workspace: `production`
<details><summary>Show Output</summary>

```diff
Expand All @@ -25,15 +25,15 @@ Changes to Outputs:
```

* :arrow_forward: To **apply** this plan, comment:
* `atlantis apply -d production -w production`
* `atlantis apply -w production`
* :put_litter_in_its_place: To **delete** this plan click [here](lock-url)
* :repeat: To **plan** this project again, comment:
* `atlantis plan -d production -w production`
* `atlantis plan -w production`
</details>
Plan: 1 to add, 0 to change, 0 to destroy.

---
### 2. dir: `staging` workspace: `staging`
### 2. dir: `.` workspace: `staging`
<details><summary>Show Output</summary>

```diff
Expand All @@ -55,10 +55,10 @@ Changes to Outputs:
```

* :arrow_forward: To **apply** this plan, comment:
* `atlantis apply -d staging -w staging`
* `atlantis apply -w staging`
* :put_litter_in_its_place: To **delete** this plan click [here](lock-url)
* :repeat: To **plan** this project again, comment:
* `atlantis plan -d staging -w staging`
* `atlantis plan -w staging`
</details>
Plan: 1 to add, 0 to change, 0 to destroy.

Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
Ran Plan for 2 projects:

1. dir: `production` workspace: `production`
1. dir: `staging` workspace: `staging`
1. dir: `.` workspace: `production`
1. dir: `.` workspace: `staging`

### 1. dir: `production` workspace: `production`
### 1. dir: `.` workspace: `production`
<details><summary>Show Output</summary>

```diff
Expand All @@ -25,15 +25,15 @@ Changes to Outputs:
```

* :arrow_forward: To **apply** this plan, comment:
* `atlantis apply -d production -w production`
* `atlantis apply -w production`
* :put_litter_in_its_place: To **delete** this plan click [here](lock-url)
* :repeat: To **plan** this project again, comment:
* `atlantis plan -d production -w production`
* `atlantis plan -w production`
</details>
Plan: 1 to add, 0 to change, 0 to destroy.

---
### 2. dir: `staging` workspace: `staging`
### 2. dir: `.` workspace: `staging`
<details><summary>Show Output</summary>

```diff
Expand All @@ -55,10 +55,10 @@ Changes to Outputs:
```

* :arrow_forward: To **apply** this plan, comment:
* `atlantis apply -d staging -w staging`
* `atlantis apply -w staging`
* :put_litter_in_its_place: To **delete** this plan click [here](lock-url)
* :repeat: To **plan** this project again, comment:
* `atlantis plan -d staging -w staging`
* `atlantis plan -w staging`
</details>
Plan: 1 to add, 0 to change, 0 to destroy.

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
Locks and plans deleted for the projects and workspaces modified in this pull request:

- dir: `production` workspace: `production`
- dir: `staging` workspace: `staging`
- dir: `.` workspaces: `production`, `staging`

This file was deleted.

2 changes: 1 addition & 1 deletion server/controllers/locks_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,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.DeleteWorkspaceForPath(lock.Pull.BaseRepo, lock.Pull, lock.Project.Path, lock.Workspace); err != nil {
l.Logger.Err("unable to delete workspace: %s", err)
}
}
Expand Down
1 change: 1 addition & 0 deletions server/core/runtime/run_step_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ func (r *RunStepRunner) Run(ctx command.ProjectContext, command string, path str
"REPO_REL_DIR": ctx.RepoRelDir,
"USER_NAME": ctx.User.Username,
"WORKSPACE": ctx.Workspace,
"TF_DATA_DIR": fmt.Sprintf(".terraform-%s", ctx.Workspace),
}

finalEnvVars := baseEnvVars
Expand Down
18 changes: 0 additions & 18 deletions server/core/runtime/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,9 @@ package runtime
import (
"bytes"
"fmt"
"regexp"
"strings"

version "github.com/hashicorp/go-version"
"github.com/pkg/errors"
runtimemodels "github.com/runatlantis/atlantis/server/core/runtime/models"
"github.com/runatlantis/atlantis/server/events/command"
"github.com/runatlantis/atlantis/server/events/models"
Expand Down Expand Up @@ -104,19 +102,3 @@ func IsRemotePlan(planContents []byte) bool {
remoteOpsHeaderBytes := []byte(remoteOpsHeader)
return bytes.Equal(planContents[:len(remoteOpsHeaderBytes)], remoteOpsHeaderBytes)
}

// ProjectNameFromPlanfile returns the project name that a planfile with name
// filename is for. If filename is for a project without a name then it will
// return an empty string. workspace is the workspace this project is in.
func ProjectNameFromPlanfile(workspace string, filename string) (string, error) {
r, err := regexp.Compile(fmt.Sprintf(`(.*?)-%s\.tfplan`, workspace))
if err != nil {
return "", errors.Wrap(err, "compiling project name regex, this is a bug")
}
projMatch := r.FindAllStringSubmatch(filename, 1)
if projMatch == nil {
return "", nil
}
rawProjName := projMatch[0][1]
return strings.Replace(rawProjName, planfileSlashReplace, "/", -1), nil
}
37 changes: 0 additions & 37 deletions server/core/runtime/runtime_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,40 +56,3 @@ func TestGetPlanFilename(t *testing.T) {
})
}
}

func TestProjectNameFromPlanfile(t *testing.T) {
cases := []struct {
workspace string
filename string
exp string
}{
{
"workspace",
"workspace.tfplan",
"",
},
{
"workspace",
"project-workspace.tfplan",
"project",
},
{
"workspace",
"project-workspace-workspace.tfplan",
"project-workspace",
},
{
"workspace",
"project::with::slashes::-workspace.tfplan",
"project/with/slashes/",
},
}

for i, c := range cases {
t.Run(fmt.Sprintf("case %d", i), func(t *testing.T) {
act, err := runtime.ProjectNameFromPlanfile(c.workspace, c.filename)
Ok(t, err)
Equals(t, c.exp, act)
})
}
}
1 change: 1 addition & 0 deletions server/core/terraform/terraform_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,7 @@ func (c *DefaultClient) prepCmd(log logging.SimpleLogging, v *version.Version, w
fmt.Sprintf("WORKSPACE=%s", workspace),
fmt.Sprintf("ATLANTIS_TERRAFORM_VERSION=%s", v.String()),
fmt.Sprintf("DIR=%s", path),
fmt.Sprintf("TF_DATA_DIR=.%s", workspace),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before overwriting this env var, can we first check to see if the user supplied it in a custom workflow, and then if it's not set, then we can set it ourselves ?

We should have a test for this too to ensure we get the user provided TF_DATA_DIR if one is set

example:

workflows:
  default:
    plan:
      steps:
      - env:
          name: TF_DATA_DIR
          command: 'echo ".terraform-$WORKSPACE-mydir"'

    apply:
      steps:
      - env:
          name: TF_DATA_DIR
          command: 'echo ".terraform-$WORKSPACE-mydir"'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this scenario the user-provided environments override the TF_DATA_DIR both on RunCommandAsync and RunCommandWithVersion since prepCmd comes before than these on the call stack.

}
if c.usePluginCache {
envVars = append(envVars, fmt.Sprintf("TF_PLUGIN_CACHE_DIR=%s", c.terraformPluginCacheDir))
Expand Down
10 changes: 5 additions & 5 deletions server/events/command_runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,7 @@ func TestRunAutoplanCommand_DeletePlans(t *testing.T) {
When(workingDir.GetPullDir(matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest())).ThenReturn(tmp, nil)
testdata.Pull.BaseRepo = testdata.GithubRepo
ch.RunAutoplanCommand(testdata.GithubRepo, testdata.GithubRepo, testdata.Pull, testdata.User)
pendingPlanFinder.VerifyWasCalledOnce().DeletePlans(tmp)
pendingPlanFinder.VerifyWasCalledOnce().DeletePlans(testdata.Pull)
lockingLocker.VerifyWasCalledOnce().UnlockByPull(testdata.Pull.BaseRepo.FullName, testdata.Pull.Num)
}

Expand All @@ -622,7 +622,7 @@ func TestRunGenericPlanCommand_DeletePlans(t *testing.T) {
When(eventParsing.ParseGithubPull(pull)).ThenReturn(modelPull, modelPull.BaseRepo, testdata.GithubRepo, nil)
testdata.Pull.BaseRepo = testdata.GithubRepo
ch.RunCommentCommand(testdata.GithubRepo, nil, nil, testdata.User, testdata.Pull.Num, &events.CommentCommand{Name: command.Plan})
pendingPlanFinder.VerifyWasCalledOnce().DeletePlans(tmp)
pendingPlanFinder.VerifyWasCalledOnce().DeletePlans(modelPull)
lockingLocker.VerifyWasCalledOnce().UnlockByPull(testdata.Pull.BaseRepo.FullName, testdata.Pull.Num)
}

Expand All @@ -640,7 +640,7 @@ func TestRunSpecificPlanCommandDoesnt_DeletePlans(t *testing.T) {
When(workingDir.GetPullDir(matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest())).ThenReturn(tmp, nil)
testdata.Pull.BaseRepo = testdata.GithubRepo
ch.RunCommentCommand(testdata.GithubRepo, nil, nil, testdata.User, testdata.Pull.Num, &events.CommentCommand{Name: command.Plan, ProjectName: "default"})
pendingPlanFinder.VerifyWasCalled(Never()).DeletePlans(tmp)
pendingPlanFinder.VerifyWasCalled(Never()).DeletePlans(testdata.Pull)
}

// Test that if one plan fails and we are using automerge, that
Expand Down Expand Up @@ -689,7 +689,7 @@ func TestRunAutoplanCommandWithError_DeletePlans(t *testing.T) {
testdata.Pull.BaseRepo = testdata.GithubRepo
ch.RunAutoplanCommand(testdata.GithubRepo, testdata.GithubRepo, testdata.Pull, testdata.User)
// gets called twice: the first time before the plan starts, the second time after the plan errors
pendingPlanFinder.VerifyWasCalled(Times(2)).DeletePlans(tmp)
pendingPlanFinder.VerifyWasCalled(Times(2)).DeletePlans(testdata.Pull)

vcsClient.VerifyWasCalled(Times(0)).DiscardReviews(matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest())
}
Expand All @@ -715,7 +715,7 @@ func TestRunGenericPlanCommand_DiscardApprovals(t *testing.T) {
When(eventParsing.ParseGithubPull(pull)).ThenReturn(modelPull, modelPull.BaseRepo, testdata.GithubRepo, nil)
testdata.Pull.BaseRepo = testdata.GithubRepo
ch.RunCommentCommand(testdata.GithubRepo, nil, nil, testdata.User, testdata.Pull.Num, &events.CommentCommand{Name: command.Plan})
pendingPlanFinder.VerifyWasCalledOnce().DeletePlans(tmp)
pendingPlanFinder.VerifyWasCalledOnce().DeletePlans(modelPull)
lockingLocker.VerifyWasCalledOnce().UnlockByPull(testdata.Pull.BaseRepo.FullName, testdata.Pull.Num)

vcsClient.VerifyWasCalledOnce().DiscardReviews(matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest())
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 @@ -71,7 +71,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.DeleteWorkspaceForPath(lock.Pull.BaseRepo, lock.Pull, lock.Project.Path, lock.Workspace); err != nil {
l.Logger.Err("unable to delete workspace: %s", err)
}
}
Expand Down
Loading