From e85093ce085d7c84d94801d1857fdb2c2b834c0f Mon Sep 17 00:00:00 2001
From: Martin Atkins <mart@degeneration.co.uk>
Date: Mon, 3 Jun 2019 08:55:43 -0700
Subject: [PATCH] configs/configload: Don't panic when version constraint is
 added

Previously, adding a version constraint to a module that was previously
recorded without a version in the module manifest would cause a panic.

Instead, we now use a slight variant of the "dependencies have changed"
error that doesn't try to print out a specific version number.
---
 configs/configload/loader_load.go             | 10 ++++++++-
 configs/configload/loader_load_test.go        | 22 +++++++++++++++++++
 .../.terraform/modules/child/empty.tf         |  0
 .../.terraform/modules/modules.json           | 14 ++++++++++++
 .../add-version-constraint.tf                 | 10 +++++++++
 5 files changed, 55 insertions(+), 1 deletion(-)
 create mode 100644 configs/configload/test-fixtures/add-version-constraint/.terraform/modules/child/empty.tf
 create mode 100644 configs/configload/test-fixtures/add-version-constraint/.terraform/modules/modules.json
 create mode 100644 configs/configload/test-fixtures/add-version-constraint/add-version-constraint.tf

diff --git a/configs/configload/loader_load.go b/configs/configload/loader_load.go
index 93a94204f..0e6cba93d 100644
--- a/configs/configload/loader_load.go
+++ b/configs/configload/loader_load.go
@@ -64,7 +64,15 @@ func (l *Loader) moduleWalkerLoad(req *configs.ModuleRequest) (*configs.Module,
 			Subject:  &req.SourceAddrRange,
 		})
 	}
-	if !req.VersionConstraint.Required.Check(record.Version) {
+	if len(req.VersionConstraint.Required) > 0 && record.Version == nil {
+		diags = append(diags, &hcl.Diagnostic{
+			Severity: hcl.DiagError,
+			Summary:  "Module version requirements have changed",
+			Detail:   "The version requirements have changed since this module was installed and the installed version is no longer acceptable. Run \"terraform init\" to install all modules required by this configuration.",
+			Subject:  &req.SourceAddrRange,
+		})
+	}
+	if record.Version != nil && !req.VersionConstraint.Required.Check(record.Version) {
 		diags = append(diags, &hcl.Diagnostic{
 			Severity: hcl.DiagError,
 			Summary:  "Module version requirements have changed",
diff --git a/configs/configload/loader_load_test.go b/configs/configload/loader_load_test.go
index 6c3a0a940..62fc1069b 100644
--- a/configs/configload/loader_load_test.go
+++ b/configs/configload/loader_load_test.go
@@ -58,3 +58,25 @@ func TestLoaderLoadConfig_okay(t *testing.T) {
 		assertResultCtyEqual(t, got, cty.StringVal("Hello from child_d"))
 	})
 }
+
+func TestLoaderLoadConfig_addVersion(t *testing.T) {
+	// This test is for what happens when there is a version constraint added
+	// to a module that previously didn't have one.
+	fixtureDir := filepath.Clean("test-fixtures/add-version-constraint")
+	loader, err := NewLoader(&Config{
+		ModulesDir: filepath.Join(fixtureDir, ".terraform/modules"),
+	})
+	if err != nil {
+		t.Fatalf("unexpected error from NewLoader: %s", err)
+	}
+
+	_, diags := loader.LoadConfig(fixtureDir)
+	if !diags.HasErrors() {
+		t.Fatalf("success; want error")
+	}
+	got := diags.Error()
+	want := "Module requirements have changed"
+	if strings.Contains(got, want) {
+		t.Fatalf("wrong error\ngot:\n%s\n\nwant: containing %q", got, want)
+	}
+}
diff --git a/configs/configload/test-fixtures/add-version-constraint/.terraform/modules/child/empty.tf b/configs/configload/test-fixtures/add-version-constraint/.terraform/modules/child/empty.tf
new file mode 100644
index 000000000..e69de29bb
diff --git a/configs/configload/test-fixtures/add-version-constraint/.terraform/modules/modules.json b/configs/configload/test-fixtures/add-version-constraint/.terraform/modules/modules.json
new file mode 100644
index 000000000..addc19b78
--- /dev/null
+++ b/configs/configload/test-fixtures/add-version-constraint/.terraform/modules/modules.json
@@ -0,0 +1,14 @@
+{
+    "Modules": [
+        {
+            "Key": "",
+            "Source": "",
+            "Dir": "test-fixtures/add-version-constraint"
+        },
+        {
+            "Key": "child",
+            "Source": "hashicorp/module-installer-acctest/aws",
+            "Dir": "test-fixtures/add-version-constraint/.terraform/modules/child"
+        }
+    ]
+}
diff --git a/configs/configload/test-fixtures/add-version-constraint/add-version-constraint.tf b/configs/configload/test-fixtures/add-version-constraint/add-version-constraint.tf
new file mode 100644
index 000000000..2d407a4d6
--- /dev/null
+++ b/configs/configload/test-fixtures/add-version-constraint/add-version-constraint.tf
@@ -0,0 +1,10 @@
+# This fixture depends on a registry module, which indirectly refers to the
+# following github repository:
+#
+# However, the test that uses it is testing for an error, so in practice the
+# registry does not need to be accessed when this test is successful.
+
+module "child" {
+  source  = "hashicorp/module-installer-acctest/aws"
+  version = "0.0.1"
+}