Fix regression - Delete .terraform.lock.hcl if it was created by terr…

…aform init (#1701) * Delete .terraform.lock.hcl prior to terraform init if it is not staged * Changes suggeted by reviewer
lyft · Sep 27, 2021 · e4162fc · e4162fc
1 parent a93f63b
commit e4162fc
Show file tree

Hide file tree

Showing 10 changed files with 561 additions and 25 deletions.
diff --git a/server/controllers/events/events_controller_e2e_test.go b/server/controllers/events/events_controller_e2e_test.go
@@ -71,8 +71,8 @@ func TestGitHubWorkflow(t *testing.T) {
 	if testing.Short() {
 		t.SkipNow()
 	}
-	// Ensure we have >= TF 0.12 locally.
-	ensureRunning012(t)
+	// Ensure we have >= TF 0.14 locally.
+	ensureRunning014(t)
 
 	cases := []struct {
 		Description string
@@ -465,12 +465,165 @@ func TestGitHubWorkflow(t *testing.T) {
 	}
 }
 
+func TestSimlpleWorkflow_terraformLockFile(t *testing.T) {
+
+	if testing.Short() {
+		t.SkipNow()
+	}
+	// Ensure we have >= TF 0.14 locally.
+	ensureRunning014(t)
+
+	cases := []struct {
+		Description string
+		// RepoDir is relative to testfixtures/test-repos.
+		RepoDir string
+		// ModifiedFiles are the list of files that have been modified in this
+		// pull request.
+		ModifiedFiles []string
+		// ExpAutoplan is true if we expect Atlantis to autoplan.
+		ExpAutoplan bool
+		// Comments are what our mock user writes to the pull request.
+		Comments []string
+		// ExpReplies is a list of files containing the expected replies that
+		// Atlantis writes to the pull request in order. A reply from a parallel operation
+		// will be matched using a substring check.
+		ExpReplies [][]string
+		// LockFileTracked deterims if the `.terraform.lock.hcl` file is tracked in git
+		// if this is true we dont expect the lockfile to be modified by terraform init
+		// if false we expect the lock file to be updated
+		LockFileTracked bool
+	}{
+		{
+			Description:   "simple with plan comment lockfile staged",
+			RepoDir:       "simple-with-lockfile",
+			ModifiedFiles: []string{"main.tf"},
+			ExpAutoplan:   true,
+			Comments: []string{
+				"atlantis plan",
+			},
+			ExpReplies: [][]string{
+				{"exp-output-autoplan.txt"},
+				{"exp-output-plan.txt"},
+			},
+			LockFileTracked: true,
+		},
+		{
+			Description:   "simple with plan comment lockfile not staged",
+			RepoDir:       "simple-with-lockfile",
+			ModifiedFiles: []string{"main.tf"},
+			Comments: []string{
+				"atlantis plan",
+			},
+			ExpReplies: [][]string{
+				{"exp-output-autoplan.txt"},
+				{"exp-output-plan.txt"},
+			},
+			LockFileTracked: false,
+		},
+	}
+	for _, c := range cases {
+		t.Run(c.Description, func(t *testing.T) {
+			RegisterMockTestingT(t)
+
+			// reset userConfig
+			userConfig = server.UserConfig{}
+			userConfig.DisableApply = true
+
+			ctrl, vcsClient, githubGetter, atlantisWorkspace := setupE2E(t, c.RepoDir)
+			// Set the repo to be cloned through the testing backdoor.
+			repoDir, headSHA, cleanup := initializeRepo(t, c.RepoDir)
+			defer cleanup()
+
+			oldLockFilePath, err := filepath.Abs(filepath.Join("testfixtures", "null_provider_lockfile_old_version"))
+			Ok(t, err)
+			oldLockFileContent, err := ioutil.ReadFile(oldLockFilePath)
+			Ok(t, err)
+
+			if c.LockFileTracked {
+				runCmd(t, "", "cp", oldLockFilePath, fmt.Sprintf("%s/.terraform.lock.hcl", repoDir))
+				runCmd(t, repoDir, "git", "add", ".terraform.lock.hcl")
+				runCmd(t, repoDir, "git", "commit", "-am", "stage .terraform.lock.hcl")
+			}
+
+			atlantisWorkspace.TestingOverrideHeadCloneURL = fmt.Sprintf("file://%s", repoDir)
+
+			// Setup test dependencies.
+			w := httptest.NewRecorder()
+			When(githubGetter.GetPullRequest(AnyRepo(), AnyInt())).ThenReturn(GitHubPullRequestParsed(headSHA), nil)
+			When(vcsClient.GetModifiedFiles(AnyRepo(), matchers.AnyModelsPullRequest())).ThenReturn(c.ModifiedFiles, nil)
+
+			// First, send the open pull request event which triggers autoplan.
+			pullOpenedReq := GitHubPullRequestOpenedEvent(t, headSHA)
+			ctrl.Post(w, pullOpenedReq)
+			ResponseContains(t, w, 200, "Processing...")
+
+			// check lock file content
+			actualLockFileContent, err := ioutil.ReadFile(fmt.Sprintf("%s/repos/runatlantis/atlantis-tests/2/default/.terraform.lock.hcl", atlantisWorkspace.DataDir))
+			Ok(t, err)
+			if c.LockFileTracked {
+				if string(oldLockFileContent) != string(actualLockFileContent) {
+					t.Error("Expected terraform.lock.hcl file not to be different as it has been staged")
+					t.FailNow()
+				}
+			} else {
+				if string(oldLockFileContent) == string(actualLockFileContent) {
+					t.Error("Expected terraform.lock.hcl file to be different as it should have been updated")
+					t.FailNow()
+				}
+			}
+
+			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))
+			}
+
+			// Now send any other comments.
+			for _, comment := range c.Comments {
+				commentReq := GitHubCommentEvent(t, comment)
+				w = httptest.NewRecorder()
+				ctrl.Post(w, commentReq)
+				ResponseContains(t, w, 200, "Processing...")
+			}
+
+			// check lock file content
+			actualLockFileContent, err = ioutil.ReadFile(fmt.Sprintf("%s/repos/runatlantis/atlantis-tests/2/default/.terraform.lock.hcl", atlantisWorkspace.DataDir))
+			Ok(t, err)
+			if c.LockFileTracked {
+				if string(oldLockFileContent) != string(actualLockFileContent) {
+					t.Error("Expected terraform.lock.hcl file not to be different as it has been staged")
+					t.FailNow()
+				}
+			} else {
+				if string(oldLockFileContent) == string(actualLockFileContent) {
+					t.Error("Expected terraform.lock.hcl file to be different as it should have been updated")
+					t.FailNow()
+				}
+			}
+
+			// Let's verify the pre-workflow hook was called for each comment including the pull request opened event
+			mockPreWorkflowHookRunner.VerifyWasCalled(Times(2)).Run(runtimematchers.AnyModelsPreWorkflowHookCommandContext(), EqString("some dummy command"), AnyString())
+
+			// Now we're ready to verify Atlantis made all the comments back (or
+			// replies) that we expect.  We expect each plan to have 1 comment,
+			// and apply have 1 for each comment plus one for the locks deleted at the
+			// end.
+
+			_, _, actReplies, _ := vcsClient.VerifyWasCalled(Times(2)).CreateComment(AnyRepo(), AnyInt(), AnyString(), AnyString()).GetAllCapturedArguments()
+			Assert(t, len(c.ExpReplies) == len(actReplies), "missing expected replies, got %d but expected %d", len(actReplies), len(c.ExpReplies))
+			for i, expReply := range c.ExpReplies {
+				assertCommentEquals(t, expReply, actReplies[i], c.RepoDir, false)
+			}
+		})
+	}
+}
+
 func TestGitHubWorkflowWithPolicyCheck(t *testing.T) {
 	if testing.Short() {
 		t.SkipNow()
 	}
-	// Ensure we have >= TF 0.12 locally.
-	ensureRunning012(t)
+	// Ensure we have >= TF 0.14 locally.
+	ensureRunning014(t)
 	// Ensure we have >= Conftest 0.21 locally.
 	ensureRunningConftest(t)
 
@@ -1160,11 +1313,11 @@ func ensureRunningConftest(t *testing.T) {
 	}
 }
 
-// Will fail test if terraform isn't in path and isn't version >= 0.12
-func ensureRunning012(t *testing.T) {
+// Will fail test if terraform isn't in path and isn't version >= 0.14
+func ensureRunning014(t *testing.T) {
 	localPath, err := exec.LookPath("terraform")
 	if err != nil {
-		t.Log("terraform >= 0.12 must be installed to run this test")
+		t.Log("terraform >= 0.14 must be installed to run this test")
 		t.FailNow()
 	}
 	versionOutBytes, err := exec.Command(localPath, "version").Output() // #nosec
@@ -1180,7 +1333,7 @@ func ensureRunning012(t *testing.T) {
 	}
 	localVersion, err := version.NewVersion(match[1])
 	Ok(t, err)
-	minVersion, err := version.NewVersion("0.12.0")
+	minVersion, err := version.NewVersion("0.14.0")
 	Ok(t, err)
 	if localVersion.LessThan(minVersion) {
 		t.Logf("must have terraform version >= %s, you have %s", minVersion, localVersion)

diff --git a/server/controllers/events/testfixtures/null_provider_lockfile_old_version b/server/controllers/events/testfixtures/null_provider_lockfile_old_version
@@ -0,0 +1,20 @@
+# This file is maintained automatically by "terraform init".
+# Manual edits may be lost in future updates.
+
+provider "registry.terraform.io/hashicorp/null" {
+  version     = "3.0.0"
+  constraints = "3.0.0"
+  hashes = [
+    "h1:ysHGBhBNkIiJLEpthB/IVCLpA1Qoncp3KbCTFGFZTO0=",
+    "zh:05fb7eab469324c97e9b73a61d2ece6f91de4e9b493e573bfeda0f2077bc3a4c",
+    "zh:1688aa91885a395c4ae67636d411475d0b831e422e005dcf02eedacaafac3bb4",
+    "zh:24a0b1292e3a474f57c483a7a4512d797e041bc9c2fbaac42fe12e86a7fb5a3c",
+    "zh:2fc951bd0d1b9b23427acc93be09b6909d72871e464088171da60fbee4fdde03",
+    "zh:6db825759425599a326385a68acc6be2d9ba0d7d6ef587191d0cdc6daef9ac63",
+    "zh:85985763d02618993c32c294072cc6ec51f1692b803cb506fcfedca9d40eaec9",
+    "zh:a53186599c57058be1509f904da512342cfdc5d808efdaf02dec15f0f3cb039a",
+    "zh:c2e07b49b6efa676bdc7b00c06333ea1792a983a5720f9e2233db27323d2707c",
+    "zh:cdc8fe1096103cf5374751e2e8408ec4abd2eb67d5a1c5151fe2c7ecfd525bef",
+    "zh:dbdef21df0c012b0d08776f3d4f34eb0f2f229adfde07ff252a119e52c0f65b7",
+  ]
+}
diff --git a/...r/controllers/events/testfixtures/test-repos/simple-with-lockfile/exp-output-autoplan.txt b/...r/controllers/events/testfixtures/test-repos/simple-with-lockfile/exp-output-autoplan.txt
@@ -0,0 +1,48 @@
+Ran Plan for dir: `.` workspace: `default`
+
+<details><summary>Show Output</summary>
+
+```diff
+
+Terraform used the selected providers to generate the following execution
+plan. Resource actions are indicated with the following symbols:
++ create
+
+Terraform will perform the following actions:
+
+  # null_resource.simple[0] will be created
++ resource "null_resource" "simple" {
+      + id = (known after apply)
+    }
+
+  # null_resource.simple2 will be created
++ resource "null_resource" "simple2" {
+      + id = (known after apply)
+    }
+
+  # null_resource.simple3 will be created
++ resource "null_resource" "simple3" {
+      + id = (known after apply)
+    }
+
+Plan: 3 to add, 0 to change, 0 to destroy.
+
+Changes to Outputs:
++ var       = "default"
++ workspace = "default"
+
+```
+
+* :arrow_forward: To **apply** this plan, comment:
+    * `atlantis apply -d .`
+* :put_litter_in_its_place: To **delete** this plan click [here](lock-url)
+* :repeat: To **plan** this project again, comment:
+    * `atlantis plan -d .`
+</details>
+Plan: 3 to add, 0 to change, 0 to destroy.
+
+---
+* :fast_forward: To **apply** all unapplied plans from this pull request, comment:
+    * `atlantis apply`
+* :put_litter_in_its_place: To delete all plans and locks for the PR, comment:
+    * `atlantis unlock`
diff --git a/server/controllers/events/testfixtures/test-repos/simple-with-lockfile/exp-output-plan.txt b/server/controllers/events/testfixtures/test-repos/simple-with-lockfile/exp-output-plan.txt
@@ -0,0 +1,48 @@
+Ran Plan for dir: `.` workspace: `default`
+
+<details><summary>Show Output</summary>
+
+```diff
+
+Terraform used the selected providers to generate the following execution
+plan. Resource actions are indicated with the following symbols:
++ create
+
+Terraform will perform the following actions:
+
+  # null_resource.simple[0] will be created
++ resource "null_resource" "simple" {
+      + id = (known after apply)
+    }
+
+  # null_resource.simple2 will be created
++ resource "null_resource" "simple2" {
+      + id = (known after apply)
+    }
+
+  # null_resource.simple3 will be created
++ resource "null_resource" "simple3" {
+      + id = (known after apply)
+    }
+
+Plan: 3 to add, 0 to change, 0 to destroy.
+
+Changes to Outputs:
++ var       = "default"
++ workspace = "default"
+
+```
+
+* :arrow_forward: To **apply** this plan, comment:
+    * `atlantis apply -d .`
+* :put_litter_in_its_place: To **delete** this plan click [here](lock-url)
+* :repeat: To **plan** this project again, comment:
+    * `atlantis plan -d .`
+</details>
+Plan: 3 to add, 0 to change, 0 to destroy.
+
+---
+* :fast_forward: To **apply** all unapplied plans from this pull request, comment:
+    * `atlantis apply`
+* :put_litter_in_its_place: To delete all plans and locks for the PR, comment:
+    * `atlantis unlock`
diff --git a/server/controllers/events/testfixtures/test-repos/simple-with-lockfile/main.tf b/server/controllers/events/testfixtures/test-repos/simple-with-lockfile/main.tf
@@ -0,0 +1,18 @@
+resource "null_resource" "simple" {
+  count = 1
+}
+
+resource "null_resource" "simple2" {}
+resource "null_resource" "simple3" {}
+
+variable "var" {
+  default = "default"
+}
+
+output "var" {
+  value = var.var
+}
+
+output "workspace" {
+  value = terraform.workspace
+}
diff --git a/server/core/runtime/init_step_runner.go b/server/core/runtime/init_step_runner.go
@@ -16,6 +16,24 @@ type InitStepRunner struct {
 }
 
 func (i *InitStepRunner) Run(ctx models.ProjectCommandContext, extraArgs []string, path string, envs map[string]string) (string, error) {
+	lockFileName := ".terraform.lock.hcl"
+	terraformLockfilePath := filepath.Join(path, lockFileName)
+	terraformLockFileTracked, err := common.IsFileTracked(path, lockFileName)
+	if err != nil {
+		ctx.Log.Warn("Error checking if %s is tracked in %s", lockFileName, path)
+
+	}
+	// If .terraform.lock.hcl is not tracked in git and it exists prior to init
+	// delete it as it probably has been created by a previous run of
+	// terraform init
+	if common.FileExists(terraformLockfilePath) && !terraformLockFileTracked {
+		ctx.Log.Debug("Deleting `%s` that was generated by previous terraform init", terraformLockfilePath)
+		delErr := os.Remove(terraformLockfilePath)
+		if delErr != nil {
+			ctx.Log.Info("Error Deleting `%s`", lockFileName)
+		}
+	}
+
 	tfVersion := i.DefaultTFVersion
 	if ctx.TerraformVersion != nil {
 		tfVersion = ctx.TerraformVersion
@@ -33,8 +51,7 @@ func (i *InitStepRunner) Run(ctx models.ProjectCommandContext, extraArgs []strin
 
 	terraformInitArgs = append(terraformInitArgs, "-no-color")
 
-	lockfilePath := filepath.Join(path, ".terraform.lock.hcl")
-	if MustConstraint("< 0.14.0").Check(tfVersion) || fileDoesNotExists(lockfilePath) {
+	if MustConstraint("< 0.14.0").Check(tfVersion) || !common.FileExists(terraformLockfilePath) {
 		terraformInitArgs = append(terraformInitArgs, "-upgrade")
 	}
 
@@ -50,12 +67,3 @@ func (i *InitStepRunner) Run(ctx models.ProjectCommandContext, extraArgs []strin
 	}
 	return "", nil
 }
-
-func fileDoesNotExists(name string) bool {
-	if _, err := os.Stat(name); err != nil {
-		if os.IsNotExist(err) {
-			return true
-		}
-	}
-	return false
-}