From 21d6fb5a3741d85bba6e100c42bfcaee4a6c3a9c Mon Sep 17 00:00:00 2001
From: Martin Atkins <mart@degeneration.co.uk>
Date: Thu, 10 Dec 2020 16:21:10 -0800
Subject: [PATCH] depsfile: Don't panic when lock file is unreadable

Previously we were expecting that the *hcl.File would always be non-nil,
even in error cases. That isn't always true, so now we'll be more robust
about it and explicitly return an empty locks object in that case, along
with the error diagnostics.

In particular this avoids a panic in a strange situation where the user
created a directory where the lock file would normally go. There's no
meaning to such a directory, so it would always be a mistake and so now
we'll return an error message about it, rather than panicking as before.

The error message for the situation where the lock file is a directory is
currently not very specific, but since it's HCL responsible for generating
that message we can't really fix that at this layer. Perhaps in future
we can change HCL to have a specialized error message for that particular
error situation, but for the sake of this commit the goal is only to
stop the panic and return a normal error message.
---
 internal/depsfile/locks_file.go      |  6 +++++
 internal/depsfile/locks_file_test.go | 39 ++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/internal/depsfile/locks_file.go b/internal/depsfile/locks_file.go
index bf7fd26abe..3866152009 100644
--- a/internal/depsfile/locks_file.go
+++ b/internal/depsfile/locks_file.go
@@ -59,6 +59,12 @@ func loadLocks(loadParse func(*hclparse.Parser) (*hcl.File, hcl.Diagnostics)) (*
 	f, hclDiags := loadParse(parser)
 	ret.sources = parser.Sources()
 	diags = diags.Append(hclDiags)
+	if f == nil {
+		// If we encountered an error loading the file then those errors
+		// should already be in diags from the above, but the file might
+		// also be nil itself and so we can't decode from it.
+		return ret, diags
+	}
 
 	moreDiags := decodeLocksFromHCL(ret, f.Body)
 	diags = diags.Append(moreDiags)
diff --git a/internal/depsfile/locks_file_test.go b/internal/depsfile/locks_file_test.go
index 6019a07157..88d9532ba2 100644
--- a/internal/depsfile/locks_file_test.go
+++ b/internal/depsfile/locks_file_test.go
@@ -159,6 +159,45 @@ func TestLoadLocksFromFile(t *testing.T) {
 	}
 }
 
+func TestLoadLocksFromFileAbsent(t *testing.T) {
+	t.Run("lock file is a directory", func(t *testing.T) {
+		// This can never happen when Terraform is the one generating the
+		// lock file, but might arise if the user makes a directory with the
+		// lock file's name for some reason. (There is no actual reason to do
+		// so, so that would always be a mistake.)
+		locks, diags := LoadLocksFromFile("testdata")
+		if len(locks.providers) != 0 {
+			t.Errorf("returned locks has providers; expected empty locks")
+		}
+		if !diags.HasErrors() {
+			t.Fatalf("LoadLocksFromFile succeeded; want error")
+		}
+		// This is a generic error message from HCL itself, so upgrading HCL
+		// in future might cause a different error message here.
+		want := `Failed to read file: The configuration file "testdata" could not be read.`
+		got := diags.Err().Error()
+		if got != want {
+			t.Errorf("wrong error message\ngot:  %s\nwant: %s", got, want)
+		}
+	})
+	t.Run("lock file doesn't exist", func(t *testing.T) {
+		locks, diags := LoadLocksFromFile("testdata/nonexist.hcl")
+		if len(locks.providers) != 0 {
+			t.Errorf("returned locks has providers; expected empty locks")
+		}
+		if !diags.HasErrors() {
+			t.Fatalf("LoadLocksFromFile succeeded; want error")
+		}
+		// This is a generic error message from HCL itself, so upgrading HCL
+		// in future might cause a different error message here.
+		want := `Failed to read file: The configuration file "testdata/nonexist.hcl" could not be read.`
+		got := diags.Err().Error()
+		if got != want {
+			t.Errorf("wrong error message\ngot:  %s\nwant: %s", got, want)
+		}
+	})
+}
+
 func TestSaveLocksToFile(t *testing.T) {
 	locks := NewLocks()