plannable import: add a provider argument to the import block (#33175)

* command: keep our promises

* remove some nil config checks

Remove some of the safety checks that ensure plan nodes have config attached at the appropriate time.

* add GeneratedConfig to plan changes objects

Add a new GeneratedConfig field alongside Importing in plan changes.

* add config generation package

The genconfig package implements HCL config generation from provider state values.

Thanks to @mildwonkey whose implementation of terraform add is the basis for this package.

* generate config during plan

If a resource is being imported and does not already have config, attempt to generate that config during planning. The config is generated from the state as an HCL string, and then parsed back into an hcl.Body to attach to the plan graph node.

The generated config string is attached to the change emitted by the plan.

* complete config generation prototype, and add tests

* plannable import: add a provider argument to the import block

* Update internal/configs/config.go

Co-authored-by: kmoe <5575356+kmoe@users.noreply.github.com>

* Update internal/configs/config.go

Co-authored-by: kmoe <5575356+kmoe@users.noreply.github.com>

* Update internal/configs/config.go

Co-authored-by: kmoe <5575356+kmoe@users.noreply.github.com>

* fix formatting and tests

---------

Co-authored-by: Katy Moe <katy@katy.moe>
Co-authored-by: kmoe <5575356+kmoe@users.noreply.github.com>

internal/configs/config.go

@@ -408,6 +408,83 @@ func (c *Config) addProviderRequirements(reqs getproviders.Requirements, recurse
 		// have been caught elsewhere.
 	}
 
+	// Import blocks that are generating config may also have a custom provider
+	// meta argument. Like the provider meta argument used in resource blocks,
+	// we use this opportunity to load any implicit providers.
+	//
+	// We'll also use this to validate that import blocks and targeted resource
+	// blocks agree on which provider they should be using. If they don't agree,
+	// this will be because the user has written explicit provider arguments
+	// that don't agree and we'll get them to fix it.
+	for _, i := range c.Module.Import {
+		if len(i.To.Module) > 0 {
+			// All provider information for imports into modules should come
+			// from the module block, so we don't need to load anything for
+			// import targets within modules.
+			continue
+		}
+
+		if target, exists := c.Module.ManagedResources[i.To.String()]; exists {
+			// This means the information about the provider for this import
+			// should come from the resource block itself and not the import
+			// block.
+			//
+			// In general, we say that you shouldn't set the provider attribute
+			// on import blocks in this case. But to make config generation
+			// easier, we will say that if it is set in both places and it's the
+			// same then that is okay.
+
+			if i.ProviderConfigRef != nil {
+				if target.ProviderConfigRef == nil {
+					// This means we have a provider specified in the import
+					// block and not in the resource block. This isn't the right
+					// way round so let's consider this a failure.
+					diags = append(diags, &hcl.Diagnostic{
+						Severity: hcl.DiagError,
+						Summary:  "Invalid import provider argument",
+						Detail:   "The provider argument can only be specified in import blocks that will generate configuration.\n\nUse the provider argument in the target resource block to configure the provider for a resource with explicit provider configuration.",
+						Subject:  i.ProviderDeclRange.Ptr(),
+					})
+					continue
+				}
+
+				if i.ProviderConfigRef.Name != target.ProviderConfigRef.Name || i.ProviderConfigRef.Alias != target.ProviderConfigRef.Alias {
+					// This means we have a provider specified in both the
+					// import block and the resource block, and they disagree.
+					// This is bad as Terraform now has different instructions
+					// about which provider to use.
+					//
+					// The general guidance is that only the resource should be
+					// specifying the provider as the import block provider
+					// attribute is just for generating config. So, let's just
+					// tell the user to only set the provider argument in the
+					// resource.
+					diags = append(diags, &hcl.Diagnostic{
+						Severity: hcl.DiagError,
+						Summary:  "Invalid import provider argument",
+						Detail:   "The provider argument can only be specified in import blocks that will generate configuration.\n\nUse the provider argument in the target resource block to configure the provider for a resource with explicit provider configuration.",
+						Subject:  i.ProviderDeclRange.Ptr(),
+					})
+					continue
+				}
+			}
+
+			// All the provider information should come from the target resource
+			// which has already been processed, so skip the rest of this
+			// processing.
+			continue
+		}
+
+		// Otherwise we are generating config for the resource being imported,
+		// so all the provider information must come from this import block.
+		fqn := i.Provider
+		if _, exists := reqs[fqn]; exists {
+			// Explicit dependency already present
+			continue
+		}
+		reqs[fqn] = nil
+	}
+
 	// "provider" block can also contain version constraints
 	for _, provider := range c.Module.ProviderConfigs {
 		fqn := c.Module.ProviderForLocalConfig(addrs.LocalProviderConfig{LocalName: provider.Name})

internal/configs/config_test.go

@@ -4,6 +4,7 @@
 package configs
 
 import (
+	"os"
 	"testing"
 
 	"github.com/go-test/deep"
@@ -423,3 +424,45 @@ func TestConfigAddProviderRequirements(t *testing.T) {
 	diags = cfg.addProviderRequirements(reqs, true)
 	assertNoDiagnostics(t, diags)
 }
+
+func TestConfigImportProviderClashesWithModules(t *testing.T) {
+	src, err := os.ReadFile("testdata/invalid-import-files/import-and-module-clash.tf")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	parser := testParser(map[string]string{
+		"main.tf": string(src),
+	})
+
+	_, diags := parser.LoadConfigFile("main.tf")
+	assertExactDiagnostics(t, diags, []string{
+		`main.tf:9,3-19: Invalid import provider argument; The provider argument can only be specified in import blocks that will generate configuration.
+
+Use the providers argument within the module block to configure providers for all resources within a module, including imported resources.`,
+	})
+}
+
+func TestConfigImportProviderClashesWithResources(t *testing.T) {
+	cfg, diags := testModuleConfigFromFile("testdata/invalid-import-files/import-and-resource-clash.tf")
+	assertNoDiagnostics(t, diags)
+
+	diags = cfg.addProviderRequirements(getproviders.Requirements{}, true)
+	assertExactDiagnostics(t, diags, []string{
+		`testdata/invalid-import-files/import-and-resource-clash.tf:9,3-19: Invalid import provider argument; The provider argument can only be specified in import blocks that will generate configuration.
+
+Use the provider argument in the target resource block to configure the provider for a resource with explicit provider configuration.`,
+	})
+}
+
+func TestConfigImportProviderWithNoResourceProvider(t *testing.T) {
+	cfg, diags := testModuleConfigFromFile("testdata/invalid-import-files/import-and-no-resource.tf")
+	assertNoDiagnostics(t, diags)
+
+	diags = cfg.addProviderRequirements(getproviders.Requirements{}, true)
+	assertExactDiagnostics(t, diags, []string{
+		`testdata/invalid-import-files/import-and-no-resource.tf:5,3-19: Invalid import provider argument; The provider argument can only be specified in import blocks that will generate configuration.
+
+Use the provider argument in the target resource block to configure the provider for a resource with explicit provider configuration.`,
+	})
+}

internal/configs/import.go

@@ -13,7 +13,11 @@ type Import struct {
 	ID string
 	To addrs.AbsResourceInstance
 
-	DeclRange hcl.Range
+	ProviderConfigRef *ProviderConfigRef
+	Provider          addrs.Provider
+
+	DeclRange         hcl.Range
+	ProviderDeclRange hcl.Range
 }
 
 func decodeImportBlock(block *hcl.Block) (*Import, hcl.Diagnostics) {
@@ -41,11 +45,30 @@ func decodeImportBlock(block *hcl.Block) (*Import, hcl.Diagnostics) {
 		}
 	}
 
+	if attr, exists := content.Attributes["provider"]; exists {
+		if len(imp.To.Module) > 0 {
+			diags = append(diags, &hcl.Diagnostic{
+				Severity: hcl.DiagError,
+				Summary:  "Invalid import provider argument",
+				Detail:   "The provider argument can only be specified in import blocks that will generate configuration.\n\nUse the providers argument within the module block to configure providers for all resources within a module, including imported resources.",
+				Subject:  attr.Range.Ptr(),
+			})
+		}
+
+		var providerDiags hcl.Diagnostics
+		imp.ProviderConfigRef, providerDiags = decodeProviderConfigRef(attr.Expr, "provider")
+		imp.ProviderDeclRange = attr.Range
+		diags = append(diags, providerDiags...)
+	}
+
 	return imp, diags
 }
 
 var importBlockSchema = &hcl.BodySchema{
 	Attributes: []hcl.AttributeSchema{
+		{
+			Name: "provider",
+		},
 		{
 			Name:     "id",
 			Required: true,

internal/configs/module.go

@@ -402,11 +402,28 @@ func (m *Module) appendFile(file *File) hcl.Diagnostics {
 		}
 	}
 
-	// "Moved" and "import" blocks just append, because they are all independent
-	// of one another at this level. (We handle any references between
-	// them at runtime.)
+	for _, i := range file.Import {
+		if i.ProviderConfigRef != nil {
+			i.Provider = m.ProviderForLocalConfig(addrs.LocalProviderConfig{
+				LocalName: i.ProviderConfigRef.Name,
+				Alias:     i.ProviderConfigRef.Alias,
+			})
+		} else {
+			implied, err := addrs.ParseProviderPart(i.To.Resource.Resource.ImpliedProvider())
+			if err == nil {
+				i.Provider = m.ImpliedProviderForUnqualifiedType(implied)
+			}
+			// We don't return a diagnostic because the invalid resource name
+			// will already have been caught.
+		}
+
+		m.Import = append(m.Import, i)
+	}
+
+	// "Moved" blocks just append, because they are all independent of one
+	// another at this level. (We handle any references between them at
+	// runtime.)
 	m.Moved = append(m.Moved, file.Moved...)
-	m.Import = append(m.Import, file.Import...)
 
 	return diags
 }

internal/configs/resource.go

@@ -116,7 +116,7 @@ func decodeResourceBlock(block *hcl.Block, override bool) (*Resource, hcl.Diagno
 		Managed:   &ManagedResource{},
 	}
 
-	content, remain, moreDiags := block.Body.PartialContent(resourceBlockSchema)
+	content, remain, moreDiags := block.Body.PartialContent(ResourceBlockSchema)
 	diags = append(diags, moreDiags...)
 	r.Config = remain
 
@@ -768,7 +768,12 @@ var commonResourceAttributes = []hcl.AttributeSchema{
 	},
 }
 
-var resourceBlockSchema = &hcl.BodySchema{
+// ResourceBlockSchema is the schema for a resource or data resource type within
+// Terraform.
+//
+// This schema is public as it is required elsewhere in order to validate and
+// use generated config.
+var ResourceBlockSchema = &hcl.BodySchema{
 	Attributes: commonResourceAttributes,
 	Blocks: []hcl.BlockHeaderSchema{
 		{Type: "locals"}, // reserved for future use

internal/configs/testdata/invalid-import-files/import-and-module-clash.tf

@@ -0,0 +1,12 @@
+
+module "importable_resource" {
+  source = "../valid-modules/importable-resource"
+}
+
+provider "local" {}
+
+import {
+  provider = local
+  id = "foo/bar"
+  to = module.importable_resource.local_file.foo
+}

internal/configs/testdata/invalid-import-files/import-and-no-resource.tf

@@ -0,0 +1,10 @@
+
+provider "local" {}
+
+import {
+  provider = local
+  id = "foo/bar"
+  to = local_file.foo_bar
+}
+
+resource "local_file" "foo_bar" {}

internal/configs/testdata/invalid-import-files/import-and-resource-clash.tf

@@ -0,0 +1,16 @@
+
+provider "local" {}
+
+provider "local" {
+  alias = "alternate"
+}
+
+import {
+  provider = local
+  id = "foo/bar"
+  to = local_file.foo_bar
+}
+
+resource "local_file" "foo_bar" {
+  provider = local.alternate
+}

internal/configs/testdata/valid-files/providers-explicit-implied.tf

@@ -19,6 +19,12 @@ import {
   to = local_file.foo
 }
 
+import {
+  provider = template.foo
+  id       = "directory/foo_filename"
+  to       = local_file.bar
+}
+
 terraform {
   required_providers {
     test = {

internal/configs/testdata/valid-modules/importable-resource/main.tf

@@ -0,0 +1,2 @@
+
+resource "local_file" "foo" {}

internal/configs/testdata/valid-modules/importable-resource/providers.tf

@@ -0,0 +1,7 @@
+terraform {
+  required_providers {
+    local = {
+      source = "hashicorp/local"
+    }
+  }
+}

internal/terraform/context_import.go

@@ -24,15 +24,16 @@ type ImportOpts struct {
 
 // ImportTarget is a single resource to import.
 type ImportTarget struct {
+	// Config is the original import block for this import. This might be null
+	// if the import did not originate in config.
+	Config *configs.Import
+
 	// Addr is the address for the resource instance that the new object should
 	// be imported into.
 	Addr addrs.AbsResourceInstance
 
 	// ID is the ID of the resource to import. This is resource-specific.
 	ID string
-
-	// ProviderAddr is the address of the provider that should handle the import.
-	ProviderAddr addrs.AbsProviderConfig
 }
 
 // Import takes already-created external resources and brings them

internal/terraform/context_plan.go

@@ -517,8 +517,9 @@ func (c *Context) findImportBlocks(config *configs.Config) []*ImportTarget {
 	var importTargets []*ImportTarget
 	for _, ic := range config.Module.Import {
 		importTargets = append(importTargets, &ImportTarget{
-			Addr: ic.To,
-			ID:   ic.ID,
+			Addr:   ic.To,
+			ID:     ic.ID,
+			Config: ic,
 		})
 	}
 	return importTargets

internal/terraform/context_plan2_test.go

@@ -4516,3 +4516,83 @@ import {
 		}
 	})
 }
+
+func TestContext2Plan_importResourceConfigGenWithAlias(t *testing.T) {
+	addr := mustResourceInstanceAddr("test_object.a")
+	m := testModuleInline(t, map[string]string{
+		"main.tf": `
+provider "test" {
+  alias = "backup"
+}
+
+import {
+  provider = test.backup
+  to       = test_object.a
+  id       = "123"
+}
+`,
+	})
+
+	p := simpleMockProvider()
+	ctx := testContext2(t, &ContextOpts{
+		Providers: map[addrs.Provider]providers.Factory{
+			addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
+		},
+	})
+	p.ReadResourceResponse = &providers.ReadResourceResponse{
+		NewState: cty.ObjectVal(map[string]cty.Value{
+			"test_string": cty.StringVal("foo"),
+		}),
+	}
+	p.ImportResourceStateResponse = &providers.ImportResourceStateResponse{
+		ImportedResources: []providers.ImportedResource{
+			{
+				TypeName: "test_object",
+				State: cty.ObjectVal(map[string]cty.Value{
+					"test_string": cty.StringVal("foo"),
+				}),
+			},
+		},
+	}
+
+	plan, diags := ctx.Plan(m, states.NewState(), DefaultPlanOpts)
+	if diags.HasErrors() {
+		t.Fatalf("unexpected errors\n%s", diags.Err().Error())
+	}
+
+	t.Run(addr.String(), func(t *testing.T) {
+		instPlan := plan.Changes.ResourceInstance(addr)
+		if instPlan == nil {
+			t.Fatalf("no plan for %s at all", addr)
+		}
+
+		if got, want := instPlan.Addr, addr; !got.Equal(want) {
+			t.Errorf("wrong current address\ngot:  %s\nwant: %s", got, want)
+		}
+		if got, want := instPlan.PrevRunAddr, addr; !got.Equal(want) {
+			t.Errorf("wrong previous run address\ngot:  %s\nwant: %s", got, want)
+		}
+		if got, want := instPlan.Action, plans.NoOp; got != want {
+			t.Errorf("wrong planned action\ngot:  %s\nwant: %s", got, want)
+		}
+		if got, want := instPlan.ActionReason, plans.ResourceInstanceChangeNoReason; got != want {
+			t.Errorf("wrong action reason\ngot:  %s\nwant: %s", got, want)
+		}
+		if instPlan.Importing.ID != "123" {
+			t.Errorf("expected import change from \"123\", got non-import change")
+		}
+
+		want := `resource "test_object" "a" {
+  provider    = test.backup
+  test_bool   = null
+  test_list   = null
+  test_map    = null
+  test_number = null
+  test_string = "foo"
+}`
+		got := instPlan.GeneratedConfig
+		if diff := cmp.Diff(want, got); len(diff) > 0 {
+			t.Errorf("got:\n%s\nwant:\n%s\ndiff:\n%s", got, want, diff)
+		}
+	})
+}

internal/terraform/node_resource_abstract.go

@@ -256,6 +256,21 @@ func (n *NodeAbstractResource) ProvidedBy() (addrs.ProviderConfig, bool) {
 		return n.storedProviderConfig, true
 	}
 
+	// We might have an import target that is providing a specific provider,
+	// this is okay as we know there is nothing else potentially providing a
+	// provider configuration.
+	if len(n.importTargets) > 0 {
+		// The import targets should either all be defined via config or none
+		// of them should be. They should also all have the same provider, so it
+		// shouldn't matter which we check here, as they'll all give the same.
+		if n.importTargets[0].Config != nil && n.importTargets[0].Config.ProviderConfigRef != nil {
+			return addrs.LocalProviderConfig{
+				LocalName: n.importTargets[0].Config.ProviderConfigRef.Name,
+				Alias:     n.importTargets[0].Config.ProviderConfigRef.Alias,
+			}, false
+		}
+	}
+
 	// No provider configuration found; return a default address
 	return addrs.AbsProviderConfig{
 		Provider: n.Provider(),
@@ -271,6 +286,16 @@ func (n *NodeAbstractResource) Provider() addrs.Provider {
 	if n.storedProviderConfig.Provider.Type != "" {
 		return n.storedProviderConfig.Provider
 	}
+
+	if len(n.importTargets) > 0 {
+		// The import targets should either all be defined via config or none
+		// of them should be. They should also all have the same provider, so it
+		// shouldn't matter which we check here, as they'll all give the same.
+		if n.importTargets[0].Config != nil {
+			return n.importTargets[0].Config.Provider
+		}
+	}
+
 	return addrs.ImpliedProviderForUnqualifiedType(n.Addr.Resource.ImpliedProvider())
 }
 

internal/terraform/node_resource_abstract_instance.go

@@ -696,15 +696,26 @@ func (n *NodeAbstractResourceInstance) plan(
 
 		// parse the "file" as HCL to get the hcl.Body
 		synthHCLFile, hclDiags := hclsyntax.ParseConfig([]byte(generatedHCLAttributes), "generated_resources.tf", hcl.Pos{Byte: 0, Line: 1, Column: 1})
+		diags = diags.Append(hclDiags)
 		if hclDiags.HasErrors() {
-			return plan, state, keyData, diags.Append(hclDiags)
+			return plan, state, keyData, diags
+		}
+
+		// We have to do a kind of mini parsing of the content here to correctly
+		// mark attributes like 'provider' as hidden. We only care about the
+		// resulting content, so it's remain that gets passed into the resource
+		// as the config.
+		_, remain, resourceDiags := synthHCLFile.Body.PartialContent(configs.ResourceBlockSchema)
+		diags = diags.Append(resourceDiags)
+		if resourceDiags.HasErrors() {
+			return plan, state, keyData, diags
 		}
 
 		generatedConfig = &configs.Resource{
 			Mode:     addrs.ManagedResourceMode,
 			Type:     n.Addr.Resource.Resource.Type,
 			Name:     n.Addr.Resource.Resource.Name,
-			Config:   synthHCLFile.Body,
+			Config:   remain,
 			Managed:  &configs.ManagedResource{},
 			Provider: n.ResolvedProvider.Provider,
 		}

internal/terraform/node_resource_plan.go

@@ -354,8 +354,9 @@ func (n *nodeExpandPlannableResource) resourceInstanceSubgraph(ctx EvalContext,
 				// If we get here, we're definitely not in legacy import mode,
 				// so go ahead and plan the resource changes including import.
 				m.importTarget = ImportTarget{
-					ID:   importTarget.ID,
-					Addr: importTarget.Addr,
+					ID:     importTarget.ID,
+					Addr:   importTarget.Addr,
+					Config: importTarget.Config,
 				}
 			}
 		}