configs/configload: Don't panic when version constraint is added

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",

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)
+	}
+}

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"
+        }
+    ]
+}

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"
+}

internal/initwd/module_install_test.go

@@ -129,6 +129,42 @@ func TestModuleInstaller_invalid_version_constraint_error(t *testing.T) {
 	}
 }
 
+func TestModuleInstaller_invalidVersionConstraintGetter(t *testing.T) {
+	fixtureDir := filepath.Clean("test-fixtures/invalid-version-constraint")
+	dir, done := tempChdir(t, fixtureDir)
+	defer done()
+
+	hooks := &testInstallHooks{}
+
+	modulesDir := filepath.Join(dir, ".terraform/modules")
+	inst := NewModuleInstaller(modulesDir, nil)
+	_, diags := inst.InstallModules(".", false, hooks)
+
+	if !diags.HasErrors() {
+		t.Fatal("expected error")
+	} else {
+		assertDiagnosticSummary(t, diags, "Invalid version constraint")
+	}
+}
+
+func TestModuleInstaller_invalidVersionConstraintLocal(t *testing.T) {
+	fixtureDir := filepath.Clean("test-fixtures/invalid-version-constraint-local")
+	dir, done := tempChdir(t, fixtureDir)
+	defer done()
+
+	hooks := &testInstallHooks{}
+
+	modulesDir := filepath.Join(dir, ".terraform/modules")
+	inst := NewModuleInstaller(modulesDir, nil)
+	_, diags := inst.InstallModules(".", false, hooks)
+
+	if !diags.HasErrors() {
+		t.Fatal("expected error")
+	} else {
+		assertDiagnosticSummary(t, diags, "Invalid version constraint")
+	}
+}
+
 func TestModuleInstaller_symlink(t *testing.T) {
 	fixtureDir := filepath.Clean("test-fixtures/local-module-symlink")
 	dir, done := tempChdir(t, fixtureDir)

internal/initwd/test-fixtures/invalid-version-constraint-local/.gitignore

@@ -0,0 +1 @@
+.terraform/*

internal/initwd/test-fixtures/invalid-version-constraint-local/root.tf

@@ -0,0 +1,9 @@
+# This fixture references the github repo at:
+#     https://github.com/hashicorp/terraform-aws-module-installer-acctest
+# However, due to the nature of this test (verifying early error), the URL will not be contacted,
+# and the test is safe to execute as part of the normal test suite.
+
+module "acctest_root" {
+  source  = "github.com/hashicorp/terraform-aws-module-installer-acctest"
+  version = "0.0.1"
+}

internal/initwd/test-fixtures/invalid-version-constraint/root.tf

@@ -1,9 +1,4 @@
-# This fixture references the github repo at:
-#     https://github.com/hashicorp/terraform-aws-module-installer-acctest
-# However, due to the nature of this test (verifying early error), the URL will not be contacted,
-# and the test is safe to execute as part of the normal test suite.
-
-module "acctest_root" {
-  source  = "github.com/hashicorp/terraform-aws-module-installer-acctest"
-  version = "0.0.1"
-}
+module "local" {
+  source  = "./local"
+  version = "0.0.1" # Version constraint not allowed for a local module
+}