Merge pull request #33634 from hashicorp/jbardin/init-from-module-warnings

Allow `get` and `init -from-module` to complete when there are configuration validation errors

internal/checks/state_test.go

@@ -19,7 +19,7 @@ func TestChecksHappyPath(t *testing.T) {
 	loader, close := configload.NewLoaderForTests(t)
 	defer close()
 	inst := initwd.NewModuleInstaller(loader.ModulesDir(), loader, nil)
-	_, instDiags := inst.InstallModules(context.Background(), fixtureDir, "tests", true, initwd.ModuleInstallHooksImpl{})
+	_, instDiags := inst.InstallModules(context.Background(), fixtureDir, "tests", true, false, initwd.ModuleInstallHooksImpl{})
 	if instDiags.HasErrors() {
 		t.Fatal(instDiags.Err())
 	}

internal/command/command_test.go

@@ -159,7 +159,7 @@ func testModuleWithSnapshot(t *testing.T, name string) (*configs.Config, *config
 	// sources only this ultimately just records all of the module paths
 	// in a JSON file so that we can load them below.
 	inst := initwd.NewModuleInstaller(loader.ModulesDir(), loader, registry.NewClient(nil, nil))
-	_, instDiags := inst.InstallModules(context.Background(), dir, "tests", true, initwd.ModuleInstallHooksImpl{})
+	_, instDiags := inst.InstallModules(context.Background(), dir, "tests", true, false, initwd.ModuleInstallHooksImpl{})
 	if instDiags.HasErrors() {
 		t.Fatal(instDiags.Err())
 	}

internal/command/get.go

@@ -90,5 +90,5 @@ func getModules(ctx context.Context, m *Meta, path string, testsDir string, upgr
 		Ui:             m.Ui,
 		ShowLocalPaths: true,
 	}
-	return m.installModules(ctx, path, testsDir, upgrade, hooks)
+	return m.installModules(ctx, path, testsDir, upgrade, true, hooks)
 }

internal/command/init.go

@@ -374,7 +374,7 @@ func (c *InitCommand) getModules(ctx context.Context, path, testsDir string, ear
 		ShowLocalPaths: true,
 	}
 
-	installAbort, installDiags := c.installModules(ctx, path, testsDir, upgrade, hooks)
+	installAbort, installDiags := c.installModules(ctx, path, testsDir, upgrade, false, hooks)
 	diags = diags.Append(installDiags)
 
 	// At this point, installModules may have generated error diags or been

internal/command/meta_config.go

@@ -183,7 +183,7 @@ func (m *Meta) loadHCLFile(filename string) (hcl.Body, tfdiags.Diagnostics) {
 // can then be relayed to the end-user. The uiModuleInstallHooks type in
 // this package has a reasonable implementation for displaying notifications
 // via a provided cli.Ui.
-func (m *Meta) installModules(ctx context.Context, rootDir, testsDir string, upgrade bool, hooks initwd.ModuleInstallHooks) (abort bool, diags tfdiags.Diagnostics) {
+func (m *Meta) installModules(ctx context.Context, rootDir, testsDir string, upgrade, installErrsOnly bool, hooks initwd.ModuleInstallHooks) (abort bool, diags tfdiags.Diagnostics) {
 	ctx, span := tracer.Start(ctx, "install modules")
 	defer span.End()
 
@@ -203,7 +203,7 @@ func (m *Meta) installModules(ctx context.Context, rootDir, testsDir string, upg
 
 	inst := initwd.NewModuleInstaller(m.modulesDir(), loader, m.registryClient())
 
-	_, moreDiags := inst.InstallModules(ctx, rootDir, testsDir, upgrade, hooks)
+	_, moreDiags := inst.InstallModules(ctx, rootDir, testsDir, upgrade, installErrsOnly, hooks)
 	diags = diags.Append(moreDiags)
 
 	if ctx.Err() == context.Canceled {

internal/initwd/from_module.go

@@ -49,6 +49,7 @@ const initFromModuleRootKeyPrefix = initFromModuleRootCallName + "."
 // are produced in that case, to prompt the user to rewrite the source strings
 // to be absolute references to the original remote module.
 func DirFromModule(ctx context.Context, loader *configload.Loader, rootDir, modulesDir, sourceAddrStr string, reg *registry.Client, hooks ModuleInstallHooks) tfdiags.Diagnostics {
+
 	var diags tfdiags.Diagnostics
 
 	// The way this function works is pretty ugly, but we accept it because
@@ -157,11 +158,18 @@ func DirFromModule(ctx context.Context, loader *configload.Loader, rootDir, modu
 	wrapHooks := installHooksInitDir{
 		Wrapped: hooks,
 	}
+	// Create a manifest record for the root module. This will be used if
+	// there are any relative-pathed modules in the root.
+	instManifest[""] = modsdir.Record{
+		Key: "",
+		Dir: rootDir,
+	}
 	fetcher := getmodules.NewPackageFetcher()
-	_, instDiags := inst.installDescendentModules(ctx, fakeRootModule, rootDir, instManifest, true, wrapHooks, fetcher)
-	diags = append(diags, instDiags...)
-	if instDiags.HasErrors() {
-		return diags
+
+	walker := inst.moduleInstallWalker(ctx, instManifest, true, wrapHooks, fetcher)
+	_, cDiags := inst.installDescendentModules(fakeRootModule, instManifest, walker, true)
+	if cDiags.HasErrors() {
+		return diags.Append(cDiags)
 	}
 
 	// If all of that succeeded then we'll now migrate what was installed

internal/initwd/from_module_test.go

@@ -212,6 +212,38 @@ func TestDirFromModule_submodules(t *testing.T) {
 	assertResultDeepEqual(t, gotTraces, wantTraces)
 }
 
+// submodulesWithProvider is identical to above, except that the configuration
+// would fail to load for some reason. We still want the module to be installed
+// for use cases like testing or CDKTF, and will only emit warnings for config
+// errors.
+func TestDirFromModule_submodulesWithProvider(t *testing.T) {
+	fixtureDir := filepath.Clean("testdata/empty")
+	fromModuleDir, err := filepath.Abs("./testdata/local-module-missing-provider")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	tmpDir, done := tempChdir(t, fixtureDir)
+	defer done()
+
+	hooks := &testInstallHooks{}
+	dir, err := filepath.EvalSymlinks(tmpDir)
+	if err != nil {
+		t.Error(err)
+	}
+	modInstallDir := filepath.Join(dir, ".terraform/modules")
+
+	loader, cleanup := configload.NewLoaderForTests(t)
+	defer cleanup()
+	diags := DirFromModule(context.Background(), loader, dir, modInstallDir, fromModuleDir, nil, hooks)
+
+	for _, d := range diags {
+		if d.Severity() != tfdiags.Warning {
+			t.Errorf("expected warning, got %v", diags.Err())
+		}
+	}
+}
+
 // TestDirFromModule_rel_submodules is similar to the test above, but the
 // from-module is relative to the install dir ("../"):
 // https://github.com/hashicorp/terraform/issues/23010

internal/initwd/module_install.go

@@ -77,6 +77,12 @@ func NewModuleInstaller(modsDir string, loader *configload.Loader, reg *registry
 // needs to replace a directory that is already present with a newly-extracted
 // package.
 //
+// installErrsOnly installs modules but converts validation errors from
+// building the configuration after installation to warnings. This is used by
+// commands like `get` or `init -from-module` where the established behavior
+// was only to install the requested module, and extra validation can break
+// compatibility.
+//
 // If the returned diagnostics contains errors then the module installation
 // may have wholly or partially completed. Modules must be loaded in order
 // to find their dependencies, so this function does many of the same checks
@@ -85,7 +91,7 @@ func NewModuleInstaller(modsDir string, loader *configload.Loader, reg *registry
 // If successful (the returned diagnostics contains no errors) then the
 // first return value is the early configuration tree that was constructed by
 // the installation process.
-func (i *ModuleInstaller) InstallModules(ctx context.Context, rootDir, testsDir string, upgrade bool, hooks ModuleInstallHooks) (*configs.Config, tfdiags.Diagnostics) {
+func (i *ModuleInstaller) InstallModules(ctx context.Context, rootDir, testsDir string, upgrade, installErrsOnly bool, hooks ModuleInstallHooks) (*configs.Config, tfdiags.Diagnostics) {
 	log.Printf("[TRACE] ModuleInstaller: installing child modules for %s into %s", rootDir, i.modsDir)
 	var diags tfdiags.Diagnostics
 
@@ -115,14 +121,6 @@ func (i *ModuleInstaller) InstallModules(ctx context.Context, rootDir, testsDir
 	}
 
 	fetcher := getmodules.NewPackageFetcher()
-	cfg, instDiags := i.installDescendentModules(ctx, rootMod, rootDir, manifest, upgrade, hooks, fetcher)
-	diags = append(diags, instDiags...)
-
-	return cfg, diags
-}
-
-func (i *ModuleInstaller) installDescendentModules(ctx context.Context, rootMod *configs.Module, rootDir string, manifest modsdir.Manifest, upgrade bool, hooks ModuleInstallHooks, fetcher *getmodules.PackageFetcher) (*configs.Config, tfdiags.Diagnostics) {
-	var diags tfdiags.Diagnostics
 
 	if hooks == nil {
 		// Use our no-op implementation as a placeholder
@@ -135,8 +133,16 @@ func (i *ModuleInstaller) installDescendentModules(ctx context.Context, rootMod
 		Key: "",
 		Dir: rootDir,
 	}
+	walker := i.moduleInstallWalker(ctx, manifest, upgrade, hooks, fetcher)
 
-	cfg, cDiags := configs.BuildConfig(rootMod, configs.ModuleWalkerFunc(
+	cfg, instDiags := i.installDescendentModules(rootMod, manifest, walker, installErrsOnly)
+	diags = append(diags, instDiags...)
+
+	return cfg, diags
+}
+
+func (i *ModuleInstaller) moduleInstallWalker(ctx context.Context, manifest modsdir.Manifest, upgrade bool, hooks ModuleInstallHooks, fetcher *getmodules.PackageFetcher) configs.ModuleWalker {
+	return configs.ModuleWalkerFunc(
 		func(req *configs.ModuleRequest) (*configs.Module, *version.Version, hcl.Diagnostics) {
 			var diags hcl.Diagnostics
 
@@ -273,10 +279,49 @@ func (i *ModuleInstaller) installDescendentModules(ctx context.Context, rootMod
 				// of addrs.ModuleSource.
 				panic(fmt.Sprintf("unsupported module source address %#v", addr))
 			}
-
 		},
-	))
+	)
+}
+
+func (i *ModuleInstaller) installDescendentModules(rootMod *configs.Module, manifest modsdir.Manifest, installWalker configs.ModuleWalker, installErrsOnly bool) (*configs.Config, tfdiags.Diagnostics) {
+	var diags tfdiags.Diagnostics
+
+	// When attempting to initialize the current directory with a module
+	// source, some use cases may want to ignore configuration errors from the
+	// building of the entire configuration structure, but we still need to
+	// capture installation errors. Because the actual module installation
+	// happens in the ModuleWalkFunc callback while building the config, we
+	// need to create a closure to capture the installation diagnostics
+	// separately.
+	var instDiags hcl.Diagnostics
+	walker := installWalker
+	if installErrsOnly {
+		walker = configs.ModuleWalkerFunc(func(req *configs.ModuleRequest) (*configs.Module, *version.Version, hcl.Diagnostics) {
+			mod, version, diags := installWalker.LoadModule(req)
+			instDiags = instDiags.Extend(diags)
+			return mod, version, diags
+		})
+	}
+
+	cfg, cDiags := configs.BuildConfig(rootMod, walker)
 	diags = diags.Append(cDiags)
+	if installErrsOnly {
+		// We can't continue if there was an error during installation, but
+		// return all diagnostics in case there happens to be anything else
+		// useful when debugging the problem. Any instDiags will be included in
+		// diags already.
+		if instDiags.HasErrors() {
+			return cfg, diags
+		}
+
+		// If there are any errors here, they must be only from building the
+		// config structures. We don't want to block initialization at this
+		// point, so convert these into warnings. Any actual errors in the
+		// configuration will be raised as soon as the config is loaded again.
+		// We continue below because writing the manifest is required to finish
+		// module installation.
+		diags = tfdiags.OverrideAll(diags, tfdiags.Warning, nil)
+	}
 
 	err := manifest.WriteSnapshotToDir(i.modsDir)
 	if err != nil {

internal/initwd/module_install_test.go

@@ -46,7 +46,7 @@ func TestModuleInstaller(t *testing.T) {
 	loader, close := configload.NewLoaderForTests(t)
 	defer close()
 	inst := NewModuleInstaller(modulesDir, loader, nil)
-	_, diags := inst.InstallModules(context.Background(), ".", "tests", false, hooks)
+	_, diags := inst.InstallModules(context.Background(), ".", "tests", false, false, hooks)
 	assertNoDiagnostics(t, diags)
 
 	wantCalls := []testInstallHookCall{
@@ -110,7 +110,7 @@ func TestModuleInstaller_error(t *testing.T) {
 	loader, close := configload.NewLoaderForTests(t)
 	defer close()
 	inst := NewModuleInstaller(modulesDir, loader, nil)
-	_, diags := inst.InstallModules(context.Background(), ".", "tests", false, hooks)
+	_, diags := inst.InstallModules(context.Background(), ".", "tests", false, false, hooks)
 
 	if !diags.HasErrors() {
 		t.Fatal("expected error")
@@ -131,7 +131,7 @@ func TestModuleInstaller_emptyModuleName(t *testing.T) {
 	loader, close := configload.NewLoaderForTests(t)
 	defer close()
 	inst := NewModuleInstaller(modulesDir, loader, nil)
-	_, diags := inst.InstallModules(context.Background(), ".", "tests", false, hooks)
+	_, diags := inst.InstallModules(context.Background(), ".", "tests", false, false, hooks)
 
 	if !diags.HasErrors() {
 		t.Fatal("expected error")
@@ -169,7 +169,7 @@ func TestModuleInstaller_packageEscapeError(t *testing.T) {
 	loader, close := configload.NewLoaderForTests(t)
 	defer close()
 	inst := NewModuleInstaller(modulesDir, loader, nil)
-	_, diags := inst.InstallModules(context.Background(), ".", "tests", false, hooks)
+	_, diags := inst.InstallModules(context.Background(), ".", "tests", false, false, hooks)
 
 	if !diags.HasErrors() {
 		t.Fatal("expected error")
@@ -207,7 +207,7 @@ func TestModuleInstaller_explicitPackageBoundary(t *testing.T) {
 	loader, close := configload.NewLoaderForTests(t)
 	defer close()
 	inst := NewModuleInstaller(modulesDir, loader, nil)
-	_, diags := inst.InstallModules(context.Background(), ".", "tests", false, hooks)
+	_, diags := inst.InstallModules(context.Background(), ".", "tests", false, false, hooks)
 
 	if diags.HasErrors() {
 		t.Fatalf("unexpected errors\n%s", diags.Err().Error())
@@ -230,7 +230,7 @@ func TestModuleInstaller_ExactMatchPrerelease(t *testing.T) {
 	loader, close := configload.NewLoaderForTests(t)
 	defer close()
 	inst := NewModuleInstaller(modulesDir, loader, registry.NewClient(nil, nil))
-	cfg, diags := inst.InstallModules(context.Background(), ".", "tests", false, hooks)
+	cfg, diags := inst.InstallModules(context.Background(), ".", "tests", false, false, hooks)
 
 	if diags.HasErrors() {
 		t.Fatalf("found unexpected errors: %s", diags.Err())
@@ -257,7 +257,7 @@ func TestModuleInstaller_PartialMatchPrerelease(t *testing.T) {
 	loader, close := configload.NewLoaderForTests(t)
 	defer close()
 	inst := NewModuleInstaller(modulesDir, loader, registry.NewClient(nil, nil))
-	cfg, diags := inst.InstallModules(context.Background(), ".", "tests", false, hooks)
+	cfg, diags := inst.InstallModules(context.Background(), ".", "tests", false, false, hooks)
 
 	if diags.HasErrors() {
 		t.Fatalf("found unexpected errors: %s", diags.Err())
@@ -280,7 +280,7 @@ func TestModuleInstaller_invalid_version_constraint_error(t *testing.T) {
 	loader, close := configload.NewLoaderForTests(t)
 	defer close()
 	inst := NewModuleInstaller(modulesDir, loader, nil)
-	_, diags := inst.InstallModules(context.Background(), ".", "tests", false, hooks)
+	_, diags := inst.InstallModules(context.Background(), ".", "tests", false, false, hooks)
 
 	if !diags.HasErrors() {
 		t.Fatal("expected error")
@@ -306,7 +306,7 @@ func TestModuleInstaller_invalidVersionConstraintGetter(t *testing.T) {
 	loader, close := configload.NewLoaderForTests(t)
 	defer close()
 	inst := NewModuleInstaller(modulesDir, loader, nil)
-	_, diags := inst.InstallModules(context.Background(), ".", "tests", false, hooks)
+	_, diags := inst.InstallModules(context.Background(), ".", "tests", false, false, hooks)
 
 	if !diags.HasErrors() {
 		t.Fatal("expected error")
@@ -332,7 +332,7 @@ func TestModuleInstaller_invalidVersionConstraintLocal(t *testing.T) {
 	loader, close := configload.NewLoaderForTests(t)
 	defer close()
 	inst := NewModuleInstaller(modulesDir, loader, nil)
-	_, diags := inst.InstallModules(context.Background(), ".", "tests", false, hooks)
+	_, diags := inst.InstallModules(context.Background(), ".", "tests", false, false, hooks)
 
 	if !diags.HasErrors() {
 		t.Fatal("expected error")
@@ -358,7 +358,7 @@ func TestModuleInstaller_symlink(t *testing.T) {
 	loader, close := configload.NewLoaderForTests(t)
 	defer close()
 	inst := NewModuleInstaller(modulesDir, loader, nil)
-	_, diags := inst.InstallModules(context.Background(), ".", "tests", false, hooks)
+	_, diags := inst.InstallModules(context.Background(), ".", "tests", false, false, hooks)
 	assertNoDiagnostics(t, diags)
 
 	wantCalls := []testInstallHookCall{
@@ -434,7 +434,7 @@ func TestLoaderInstallModules_registry(t *testing.T) {
 	loader, close := configload.NewLoaderForTests(t)
 	defer close()
 	inst := NewModuleInstaller(modulesDir, loader, registry.NewClient(nil, nil))
-	_, diags := inst.InstallModules(context.Background(), dir, "tests", false, hooks)
+	_, diags := inst.InstallModules(context.Background(), dir, "tests", false, false, hooks)
 	assertNoDiagnostics(t, diags)
 
 	v := version.Must(version.NewVersion("0.0.1"))
@@ -597,7 +597,7 @@ func TestLoaderInstallModules_goGetter(t *testing.T) {
 	loader, close := configload.NewLoaderForTests(t)
 	defer close()
 	inst := NewModuleInstaller(modulesDir, loader, registry.NewClient(nil, nil))
-	_, diags := inst.InstallModules(context.Background(), dir, "tests", false, hooks)
+	_, diags := inst.InstallModules(context.Background(), dir, "tests", false, false, hooks)
 	assertNoDiagnostics(t, diags)
 
 	wantCalls := []testInstallHookCall{
@@ -715,7 +715,7 @@ func TestModuleInstaller_fromTests(t *testing.T) {
 	loader, close := configload.NewLoaderForTests(t)
 	defer close()
 	inst := NewModuleInstaller(modulesDir, loader, nil)
-	_, diags := inst.InstallModules(context.Background(), ".", "tests", false, hooks)
+	_, diags := inst.InstallModules(context.Background(), ".", "tests", false, false, hooks)
 	assertNoDiagnostics(t, diags)
 
 	wantCalls := []testInstallHookCall{
@@ -772,7 +772,7 @@ func TestLoadInstallModules_registryFromTest(t *testing.T) {
 	loader, close := configload.NewLoaderForTests(t)
 	defer close()
 	inst := NewModuleInstaller(modulesDir, loader, registry.NewClient(nil, nil))
-	_, diags := inst.InstallModules(context.Background(), dir, "tests", false, hooks)
+	_, diags := inst.InstallModules(context.Background(), dir, "tests", false, false, hooks)
 	assertNoDiagnostics(t, diags)
 
 	v := version.Must(version.NewVersion("0.0.1"))
@@ -944,7 +944,7 @@ func assertNoDiagnostics(t *testing.T, diags tfdiags.Diagnostics) bool {
 
 func assertDiagnosticCount(t *testing.T, diags tfdiags.Diagnostics, want int) bool {
 	t.Helper()
-	if len(diags) != 0 {
+	if len(diags) != want {
 		t.Errorf("wrong number of diagnostics %d; want %d", len(diags), want)
 		for _, diag := range diags {
 			t.Logf("- %#v", diag)

internal/initwd/testdata/local-module-missing-provider/main.tf

@@ -0,0 +1,14 @@
+terraform {
+  required_providers {
+    foo = {
+      source = "hashicorp/foo"
+      // since this module declares an alias with no config, it is not valid as
+      // a root module.
+      configuration_aliases = [ foo.alternate ]
+    }
+  }
+}
+
+resource "foo_instance" "bam" {
+  provider = foo.alternate
+}

internal/initwd/testing.go

@@ -39,7 +39,7 @@ func LoadConfigForTests(t *testing.T, rootDir string, testsDir string) (*configs
 	loader, cleanup := configload.NewLoaderForTests(t)
 	inst := NewModuleInstaller(loader.ModulesDir(), loader, registry.NewClient(nil, nil))
 
-	_, moreDiags := inst.InstallModules(context.Background(), rootDir, testsDir, true, ModuleInstallHooksImpl{})
+	_, moreDiags := inst.InstallModules(context.Background(), rootDir, testsDir, true, false, ModuleInstallHooksImpl{})
 	diags = diags.Append(moreDiags)
 	if diags.HasErrors() {
 		cleanup()

internal/lang/globalref/analyzer_test.go

@@ -25,7 +25,7 @@ func testAnalyzer(t *testing.T, fixtureName string) *Analyzer {
 	defer cleanup()
 
 	inst := initwd.NewModuleInstaller(loader.ModulesDir(), loader, registry.NewClient(nil, nil))
-	_, instDiags := inst.InstallModules(context.Background(), configDir, "tests", true, initwd.ModuleInstallHooksImpl{})
+	_, instDiags := inst.InstallModules(context.Background(), configDir, "tests", true, false, initwd.ModuleInstallHooksImpl{})
 	if instDiags.HasErrors() {
 		t.Fatalf("unexpected module installation errors: %s", instDiags.Err().Error())
 	}

internal/refactoring/move_validate_test.go

@@ -538,7 +538,7 @@ func loadRefactoringFixture(t *testing.T, dir string) (*configs.Config, instance
 	defer cleanup()
 
 	inst := initwd.NewModuleInstaller(loader.ModulesDir(), loader, registry.NewClient(nil, nil))
-	_, instDiags := inst.InstallModules(context.Background(), dir, "tests", true, initwd.ModuleInstallHooksImpl{})
+	_, instDiags := inst.InstallModules(context.Background(), dir, "tests", true, false, initwd.ModuleInstallHooksImpl{})
 	if instDiags.HasErrors() {
 		t.Fatal(instDiags.Err())
 	}

internal/terraform/terraform_test.go

@@ -67,7 +67,7 @@ func testModuleWithSnapshot(t *testing.T, name string) (*configs.Config, *config
 	// sources only this ultimately just records all of the module paths
 	// in a JSON file so that we can load them below.
 	inst := initwd.NewModuleInstaller(loader.ModulesDir(), loader, registry.NewClient(nil, nil))
-	_, instDiags := inst.InstallModules(context.Background(), dir, "tests", true, initwd.ModuleInstallHooksImpl{})
+	_, instDiags := inst.InstallModules(context.Background(), dir, "tests", true, false, initwd.ModuleInstallHooksImpl{})
 	if instDiags.HasErrors() {
 		t.Fatal(instDiags.Err())
 	}
@@ -124,7 +124,7 @@ func testModuleInline(t *testing.T, sources map[string]string) *configs.Config {
 	// sources only this ultimately just records all of the module paths
 	// in a JSON file so that we can load them below.
 	inst := initwd.NewModuleInstaller(loader.ModulesDir(), loader, registry.NewClient(nil, nil))
-	_, instDiags := inst.InstallModules(context.Background(), cfgPath, "tests", true, initwd.ModuleInstallHooksImpl{})
+	_, instDiags := inst.InstallModules(context.Background(), cfgPath, "tests", true, false, initwd.ModuleInstallHooksImpl{})
 	if instDiags.HasErrors() {
 		t.Fatal(instDiags.Err())
 	}