diff --git a/internal/command/import.go b/internal/command/import.go index fc61671f9e..ff7637c1f5 100644 --- a/internal/command/import.go +++ b/internal/command/import.go @@ -236,7 +236,7 @@ func (c *ImportCommand) Run(args []string) int { newState, importDiags := lr.Core.Import(lr.Config, lr.InputState, &terraform.ImportOpts{ Targets: []*terraform.ImportTarget{ { - Addr: addr, + LegacyAddr: addr, // In the import block, the ID can be an arbitrary hcl.Expression, // but here it's always interpreted as a literal string. diff --git a/internal/configs/config.go b/internal/configs/config.go index 63fb34639f..531feba200 100644 --- a/internal/configs/config.go +++ b/internal/configs/config.go @@ -430,8 +430,10 @@ func (c *Config) addProviderRequirements(reqs getproviders.Requirements, recurse reqs[fqn] = nil } for _, i := range c.Module.Import { - implied, err := addrs.ParseProviderPart(i.To.Resource.Resource.ImpliedProvider()) + implied, err := addrs.ParseProviderPart(i.ToResource.Resource.ImpliedProvider()) if err == nil { + // FIXME: this will not resolve correctly if the target module uses + // a different local name from the root module. provider := c.Module.ImpliedProviderForUnqualifiedType(implied) if _, exists := reqs[provider]; exists { // Explicit dependency already present @@ -452,14 +454,14 @@ func (c *Config) addProviderRequirements(reqs getproviders.Requirements, recurse // 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 { + if len(i.ToResource.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 { + if target, exists := c.Module.ManagedResources[i.ToResource.Resource.String()]; exists { // This means the information about the provider for this import // should come from the resource block itself and not the import // block. diff --git a/internal/configs/import.go b/internal/configs/import.go index f77c04ab91..8d83cb4a97 100644 --- a/internal/configs/import.go +++ b/internal/configs/import.go @@ -5,12 +5,21 @@ package configs import ( "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/hclsyntax" "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/tfdiags" ) type Import struct { ID hcl.Expression - To addrs.AbsResourceInstance + + To hcl.Expression + // The To address may not be resolvable immediately if it contains dynamic + // index expressions, so we will extract the ConfigResource address and + // store it here for reference. + ToResource addrs.ConfigResource + + ForEach hcl.Expression ProviderConfigRef *ProviderConfigRef Provider addrs.Provider @@ -33,17 +42,19 @@ func decodeImportBlock(block *hcl.Block) (*Import, hcl.Diagnostics) { } if attr, exists := content.Attributes["to"]; exists { - traversal, traversalDiags := hcl.AbsTraversalForExpr(attr.Expr) - diags = append(diags, traversalDiags...) - if !traversalDiags.HasErrors() { - to, toDiags := addrs.ParseAbsResourceInstance(traversal) - diags = append(diags, toDiags.ToHCL()...) - imp.To = to - } + imp.To = attr.Expr + + addr, toDiags := parseConfigResourceFromExpression(attr.Expr) + diags = diags.Extend(toDiags.ToHCL()) + imp.ToResource = addr + } + + if attr, exists := content.Attributes["for_each"]; exists { + imp.ForEach = attr.Expr } if attr, exists := content.Attributes["provider"]; exists { - if len(imp.To.Module) > 0 { + if len(imp.ToResource.Module) > 0 { diags = append(diags, &hcl.Diagnostic{ Severity: hcl.DiagError, Summary: "Invalid import provider argument", @@ -66,6 +77,9 @@ var importBlockSchema = &hcl.BodySchema{ { Name: "provider", }, + { + Name: "for_each", + }, { Name: "id", Required: true, @@ -76,3 +90,63 @@ var importBlockSchema = &hcl.BodySchema{ }, }, } + +// parseResourceInstanceFromExpression takes an arbitrary expression +// representing a resource instance, and parses out the static ConfigResource +// skipping an variable index expressions. This is used to connect an import +// block's "to" to the configuration address before the full instance +// expressions are evaluated. +func parseConfigResourceFromExpression(expr hcl.Expression) (addrs.ConfigResource, tfdiags.Diagnostics) { + traversal, hcdiags := exprToResourceTraversal(expr) + if hcdiags.HasErrors() { + return addrs.ConfigResource{}, tfdiags.Diagnostics(nil).Append(hcdiags) + } + + addr, diags := addrs.ParseAbsResourceInstance(traversal) + if diags.HasErrors() { + return addrs.ConfigResource{}, diags + } + + return addr.ConfigResource(), diags +} + +// exprToResourceTraversal is used to parse the import block's to expression, +// which must be a resource instance, but may contain limited variables with +// index expressions. Since we only need the ConfigResource to connect the +// import to the configuration, we skip any index expressions. +func exprToResourceTraversal(expr hcl.Expression) (hcl.Traversal, hcl.Diagnostics) { + var trav hcl.Traversal + var diags hcl.Diagnostics + + switch e := expr.(type) { + case *hclsyntax.RelativeTraversalExpr: + t, d := exprToResourceTraversal(e.Source) + diags = diags.Extend(d) + trav = append(trav, t...) + trav = append(trav, e.Traversal...) + + case *hclsyntax.ScopeTraversalExpr: + // a static reference, we can just append the traversal + trav = append(trav, e.Traversal...) + + case *hclsyntax.IndexExpr: + // Get the collection from the index expression, we don't need the + // index for a ConfigResource + t, d := exprToResourceTraversal(e.Collection) + diags = diags.Extend(d) + if diags.HasErrors() { + return nil, diags + } + trav = append(trav, t...) + + default: + // if we don't recognise the expression type (which means we are likely + // dealing with a test mock), try and interpret this as an absolute + // traversal + t, d := hcl.AbsTraversalForExpr(e) + diags = diags.Extend(d) + trav = append(trav, t...) + } + + return trav, diags +} diff --git a/internal/configs/import_test.go b/internal/configs/import_test.go index d00acccecb..909b061b57 100644 --- a/internal/configs/import_test.go +++ b/internal/configs/import_test.go @@ -5,19 +5,59 @@ package configs import ( "fmt" + "reflect" "testing" - "github.com/google/go-cmp/cmp" "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/hclsyntax" "github.com/hashicorp/hcl/v2/hcltest" "github.com/hashicorp/terraform/internal/addrs" "github.com/zclconf/go-cty/cty" ) -var ( - typeComparer = cmp.Comparer(cty.Type.Equals) - valueComparer = cmp.Comparer(cty.Value.RawEquals) -) +func TestParseConfigResourceFromExpression(t *testing.T) { + mustExpr := func(expr hcl.Expression, diags hcl.Diagnostics) hcl.Expression { + if diags != nil { + panic(diags.Error()) + } + return expr + } + + tests := []struct { + expr hcl.Expression + expect addrs.ConfigResource + }{ + { + mustExpr(hclsyntax.ParseExpression([]byte("test_instance.bar"), "my_traversal", hcl.Pos{})), + mustAbsResourceInstanceAddr("test_instance.bar").ConfigResource(), + }, + + // parsing should skip the each.key variable + { + mustExpr(hclsyntax.ParseExpression([]byte("test_instance.bar[each.key]"), "my_traversal", hcl.Pos{})), + mustAbsResourceInstanceAddr("test_instance.bar").ConfigResource(), + }, + + // nested modules must work too + { + mustExpr(hclsyntax.ParseExpression([]byte("module.foo[each.key].test_instance.bar[each.key]"), "my_traversal", hcl.Pos{})), + mustAbsResourceInstanceAddr("module.foo.test_instance.bar").ConfigResource(), + }, + } + + for i, tc := range tests { + t.Run(fmt.Sprintf("%d-%s", i, tc.expect), func(t *testing.T) { + + got, diags := parseConfigResourceFromExpression(tc.expr) + if diags.HasErrors() { + t.Fatal(diags.ErrWithWarnings()) + } + if !got.Equal(tc.expect) { + t.Fatalf("got %s, want %s", got, tc.expect) + } + }) + } +} func TestImportBlock_decode(t *testing.T) { blockRange := hcl.Range{ @@ -56,9 +96,9 @@ func TestImportBlock_decode(t *testing.T) { DefRange: blockRange, }, &Import{ - To: mustAbsResourceInstanceAddr("test_instance.bar"), - ID: foo_str_expr, - DeclRange: blockRange, + ToResource: mustAbsResourceInstanceAddr("test_instance.bar").ConfigResource(), + ID: foo_str_expr, + DeclRange: blockRange, }, ``, }, @@ -80,9 +120,9 @@ func TestImportBlock_decode(t *testing.T) { DefRange: blockRange, }, &Import{ - To: mustAbsResourceInstanceAddr("test_instance.bar[\"one\"]"), - ID: foo_str_expr, - DeclRange: blockRange, + ToResource: mustAbsResourceInstanceAddr("test_instance.bar[\"one\"]").ConfigResource(), + ID: foo_str_expr, + DeclRange: blockRange, }, ``, }, @@ -104,9 +144,9 @@ func TestImportBlock_decode(t *testing.T) { DefRange: blockRange, }, &Import{ - To: mustAbsResourceInstanceAddr("module.bar.test_instance.bar"), - ID: foo_str_expr, - DeclRange: blockRange, + ToResource: mustAbsResourceInstanceAddr("module.bar.test_instance.bar").ConfigResource(), + ID: foo_str_expr, + DeclRange: blockRange, }, ``, }, @@ -124,8 +164,8 @@ func TestImportBlock_decode(t *testing.T) { DefRange: blockRange, }, &Import{ - To: mustAbsResourceInstanceAddr("test_instance.bar"), - DeclRange: blockRange, + ToResource: mustAbsResourceInstanceAddr("test_instance.bar").ConfigResource(), + DeclRange: blockRange, }, "Missing required argument", }, @@ -165,8 +205,12 @@ func TestImportBlock_decode(t *testing.T) { t.Fatal("expected error") } - if !cmp.Equal(got, test.want, typeComparer, valueComparer) { - t.Fatalf("wrong result: %s", cmp.Diff(got, test.want)) + if !got.ToResource.Equal(test.want.ToResource) { + t.Errorf("expected resource %q got %q", test.want.ToResource, got.ToResource) + } + + if !reflect.DeepEqual(got.ID, test.want.ID) { + t.Errorf("expected ID %q got %q", test.want.ID, got.ID) } }) } diff --git a/internal/configs/module.go b/internal/configs/module.go index c7696013b2..52faf4a7d2 100644 --- a/internal/configs/module.go +++ b/internal/configs/module.go @@ -422,7 +422,8 @@ func (m *Module) appendFile(file *File) hcl.Diagnostics { for _, i := range file.Import { for _, mi := range m.Import { - if i.To.Equal(mi.To) { + // FIXME: this doesn't allow multiple blocks for individual instances + if i.ToResource.Equal(mi.ToResource) { diags = append(diags, &hcl.Diagnostic{ Severity: hcl.DiagError, Summary: fmt.Sprintf("Duplicate import configuration for %q", i.To), @@ -439,7 +440,7 @@ func (m *Module) appendFile(file *File) hcl.Diagnostics { Alias: i.ProviderConfigRef.Alias, }) } else { - implied, err := addrs.ParseProviderPart(i.To.Resource.Resource.ImpliedProvider()) + implied, err := addrs.ParseProviderPart(i.ToResource.Resource.ImpliedProvider()) if err == nil { i.Provider = m.ImpliedProviderForUnqualifiedType(implied) } @@ -450,10 +451,10 @@ func (m *Module) appendFile(file *File) hcl.Diagnostics { // It is invalid for any import block to have a "to" argument matching // any moved block's "from" argument. for _, mb := range m.Moved { - // Comparing string serialisations is good enough here, because we - // only care about equality in the case that both addresses are - // AbsResourceInstances. - if mb.From.String() == i.To.String() { + // FIXME: This is not correct for moved modules, and won't catch + // all combinations of expanded imports (though preventing + // collisions based on ConfigResource alone may be sufficient) + if mb.From.String() == i.ToResource.String() { diags = append(diags, &hcl.Diagnostic{ Severity: hcl.DiagError, Summary: "Cannot import to a move source", diff --git a/internal/terraform/context_import.go b/internal/terraform/context_import.go index 09741e5c04..f946c37942 100644 --- a/internal/terraform/context_import.go +++ b/internal/terraform/context_import.go @@ -32,10 +32,13 @@ type ImportTarget struct { // Addr is the address for the resource instance that the new object should // be imported into. - Addr addrs.AbsResourceInstance + // TODO: remove + LegacyAddr addrs.AbsResourceInstance // ID is the ID of the resource to import. This is resource-specific. - ID hcl.Expression + // TODO: the expression is evaluated from Config. + ID hcl.Expression + idString string } // Import takes already-created external resources and brings them diff --git a/internal/terraform/context_import_test.go b/internal/terraform/context_import_test.go index 6625c1bca7..4bcf9eeb62 100644 --- a/internal/terraform/context_import_test.go +++ b/internal/terraform/context_import_test.go @@ -11,10 +11,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/zclconf/go-cty/cty" - "github.com/hashicorp/hcl/v2" - "github.com/hashicorp/hcl/v2/hcltest" "github.com/hashicorp/terraform/internal/addrs" - "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/configs/configschema" "github.com/hashicorp/terraform/internal/providers" "github.com/hashicorp/terraform/internal/states" @@ -40,14 +37,13 @@ func TestContextImport_basic(t *testing.T) { }, } - barExpr := hcl.StaticExpr(cty.StringVal("bar"), configs.SynthBody("import", nil).MissingItemRange()) state, diags := ctx.Import(m, states.NewState(), &ImportOpts{ Targets: []*ImportTarget{ { - Addr: addrs.RootModuleInstance.ResourceInstance( + LegacyAddr: addrs.RootModuleInstance.ResourceInstance( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: barExpr, + idString: "bar", }, }, }) @@ -92,14 +88,13 @@ resource "aws_instance" "foo" { }, } - barExpr := hcl.StaticExpr(cty.StringVal("bar"), configs.SynthBody("import", nil).MissingItemRange()) state, diags := ctx.Import(m, states.NewState(), &ImportOpts{ Targets: []*ImportTarget{ { - Addr: addrs.RootModuleInstance.ResourceInstance( + LegacyAddr: addrs.RootModuleInstance.ResourceInstance( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.IntKey(0), ), - ID: barExpr, + idString: "bar", }, }, }) @@ -154,14 +149,13 @@ func TestContextImport_collision(t *testing.T) { }, } - barExpr := hcl.StaticExpr(cty.StringVal("bar"), configs.SynthBody("import", nil).MissingItemRange()) state, diags := ctx.Import(m, state, &ImportOpts{ Targets: []*ImportTarget{ { - Addr: addrs.RootModuleInstance.ResourceInstance( + LegacyAddr: addrs.RootModuleInstance.ResourceInstance( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: barExpr, + idString: "bar", }, }, }) @@ -199,14 +193,13 @@ func TestContextImport_missingType(t *testing.T) { }, }) - barExpr := hcl.StaticExpr(cty.StringVal("bar"), configs.SynthBody("import", nil).MissingItemRange()) state, diags := ctx.Import(m, states.NewState(), &ImportOpts{ Targets: []*ImportTarget{ { - Addr: addrs.RootModuleInstance.ResourceInstance( + LegacyAddr: addrs.RootModuleInstance.ResourceInstance( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: barExpr, + idString: "bar", }, }, }) @@ -251,14 +244,13 @@ func TestContextImport_moduleProvider(t *testing.T) { }, }) - barExpr := hcl.StaticExpr(cty.StringVal("bar"), configs.SynthBody("import", nil).MissingItemRange()) state, diags := ctx.Import(m, states.NewState(), &ImportOpts{ Targets: []*ImportTarget{ { - Addr: addrs.RootModuleInstance.ResourceInstance( + LegacyAddr: addrs.RootModuleInstance.ResourceInstance( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: barExpr, + idString: "bar", }, }, }) @@ -307,14 +299,13 @@ func TestContextImport_providerModule(t *testing.T) { return } - barExpr := hcl.StaticExpr(cty.StringVal("bar"), configs.SynthBody("import", nil).MissingItemRange()) _, diags := ctx.Import(m, states.NewState(), &ImportOpts{ Targets: []*ImportTarget{ { - Addr: addrs.RootModuleInstance.Child("child", addrs.NoKey).ResourceInstance( + LegacyAddr: addrs.RootModuleInstance.Child("child", addrs.NoKey).ResourceInstance( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: barExpr, + idString: "bar", }, }, }) @@ -364,14 +355,13 @@ func TestContextImport_providerConfig(t *testing.T) { }, } - barExpr := hcl.StaticExpr(cty.StringVal("bar"), configs.SynthBody("import", nil).MissingItemRange()) state, diags := ctx.Import(m, states.NewState(), &ImportOpts{ Targets: []*ImportTarget{ { - Addr: addrs.RootModuleInstance.ResourceInstance( + LegacyAddr: addrs.RootModuleInstance.ResourceInstance( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: barExpr, + idString: "bar", }, }, SetVariables: InputValues{ @@ -425,14 +415,13 @@ func TestContextImport_providerConfigResources(t *testing.T) { }, } - barExpr := hcl.StaticExpr(cty.StringVal("bar"), configs.SynthBody("import", nil).MissingItemRange()) _, diags := ctx.Import(m, states.NewState(), &ImportOpts{ Targets: []*ImportTarget{ { - Addr: addrs.RootModuleInstance.ResourceInstance( + LegacyAddr: addrs.RootModuleInstance.ResourceInstance( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: barExpr, + idString: "bar", }, }, }) @@ -497,14 +486,13 @@ data "aws_data_source" "bar" { }), } - barExpr := hcl.StaticExpr(cty.StringVal("bar"), configs.SynthBody("import", nil).MissingItemRange()) state, diags := ctx.Import(m, states.NewState(), &ImportOpts{ Targets: []*ImportTarget{ { - Addr: addrs.RootModuleInstance.ResourceInstance( + LegacyAddr: addrs.RootModuleInstance.ResourceInstance( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: barExpr, + idString: "bar", }, }, }) @@ -549,14 +537,13 @@ func TestContextImport_refreshNil(t *testing.T) { } } - barExpr := hcl.StaticExpr(cty.StringVal("bar"), configs.SynthBody("import", nil).MissingItemRange()) state, diags := ctx.Import(m, states.NewState(), &ImportOpts{ Targets: []*ImportTarget{ { - Addr: addrs.RootModuleInstance.ResourceInstance( + LegacyAddr: addrs.RootModuleInstance.ResourceInstance( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: barExpr, + idString: "bar", }, }, }) @@ -591,14 +578,13 @@ func TestContextImport_module(t *testing.T) { }, } - barExpr := hcl.StaticExpr(cty.StringVal("bar"), configs.SynthBody("import", nil).MissingItemRange()) state, diags := ctx.Import(m, states.NewState(), &ImportOpts{ Targets: []*ImportTarget{ { - Addr: addrs.RootModuleInstance.Child("child", addrs.IntKey(0)).ResourceInstance( + LegacyAddr: addrs.RootModuleInstance.Child("child", addrs.IntKey(0)).ResourceInstance( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: barExpr, + idString: "bar", }, }, }) @@ -633,14 +619,13 @@ func TestContextImport_moduleDepth2(t *testing.T) { }, } - bazExpr := hcl.StaticExpr(cty.StringVal("baz"), configs.SynthBody("import", nil).MissingItemRange()) state, diags := ctx.Import(m, states.NewState(), &ImportOpts{ Targets: []*ImportTarget{ { - Addr: addrs.RootModuleInstance.Child("child", addrs.IntKey(0)).Child("nested", addrs.NoKey).ResourceInstance( + LegacyAddr: addrs.RootModuleInstance.Child("child", addrs.IntKey(0)).Child("nested", addrs.NoKey).ResourceInstance( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: bazExpr, + idString: "baz", }, }, }) @@ -675,14 +660,13 @@ func TestContextImport_moduleDiff(t *testing.T) { }, } - bazExpr := hcl.StaticExpr(cty.StringVal("baz"), configs.SynthBody("import", nil).MissingItemRange()) state, diags := ctx.Import(m, states.NewState(), &ImportOpts{ Targets: []*ImportTarget{ { - Addr: addrs.RootModuleInstance.Child("child", addrs.IntKey(0)).ResourceInstance( + LegacyAddr: addrs.RootModuleInstance.Child("child", addrs.IntKey(0)).ResourceInstance( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: bazExpr, + idString: "baz", }, }, }) @@ -744,14 +728,13 @@ func TestContextImport_multiState(t *testing.T) { }, }) - barExpr := hcl.StaticExpr(cty.StringVal("bar"), configs.SynthBody("import", nil).MissingItemRange()) state, diags := ctx.Import(m, states.NewState(), &ImportOpts{ Targets: []*ImportTarget{ { - Addr: addrs.RootModuleInstance.ResourceInstance( + LegacyAddr: addrs.RootModuleInstance.ResourceInstance( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: barExpr, + idString: "bar", }, }, }) @@ -819,14 +802,13 @@ func TestContextImport_multiStateSame(t *testing.T) { }, }) - barExpr := hcl.StaticExpr(cty.StringVal("bar"), configs.SynthBody("import", nil).MissingItemRange()) state, diags := ctx.Import(m, states.NewState(), &ImportOpts{ Targets: []*ImportTarget{ { - Addr: addrs.RootModuleInstance.ResourceInstance( + LegacyAddr: addrs.RootModuleInstance.ResourceInstance( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: barExpr, + idString: "bar", }, }, }) @@ -914,14 +896,13 @@ resource "test_resource" "unused" { }, }) - testExpr := hcl.StaticExpr(cty.StringVal("test"), configs.SynthBody("import", nil).MissingItemRange()) state, diags := ctx.Import(m, states.NewState(), &ImportOpts{ Targets: []*ImportTarget{ { - Addr: addrs.RootModuleInstance.ResourceInstance( + LegacyAddr: addrs.RootModuleInstance.ResourceInstance( addrs.ManagedResourceMode, "test_resource", "test", addrs.NoKey, ), - ID: testExpr, + idString: "test", }, }, }) @@ -985,14 +966,13 @@ resource "test_resource" "test" { }, }) - testExpr := hcl.StaticExpr(cty.StringVal("test"), configs.SynthBody("import", nil).MissingItemRange()) state, diags := ctx.Import(m, states.NewState(), &ImportOpts{ Targets: []*ImportTarget{ { - Addr: addrs.RootModuleInstance.ResourceInstance( + LegacyAddr: addrs.RootModuleInstance.ResourceInstance( addrs.ManagedResourceMode, "test_resource", "test", addrs.NoKey, ), - ID: testExpr, + idString: "test", }, }, }) @@ -1013,7 +993,6 @@ resource "test_resource" "test" { func TestContextImport_33572(t *testing.T) { p := testProvider("aws") m := testModule(t, "issue-33572") - bar_expr := hcltest.MockExprLiteral(cty.StringVal("bar")) ctx := testContext2(t, &ContextOpts{ Providers: map[addrs.Provider]providers.Factory{ @@ -1035,10 +1014,10 @@ func TestContextImport_33572(t *testing.T) { state, diags := ctx.Import(m, states.NewState(), &ImportOpts{ Targets: []*ImportTarget{ { - Addr: addrs.RootModuleInstance.ResourceInstance( + LegacyAddr: addrs.RootModuleInstance.ResourceInstance( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: bar_expr, + idString: "bar", }, }, }) diff --git a/internal/terraform/context_plan.go b/internal/terraform/context_plan.go index 32cf42438d..5c49b8442f 100644 --- a/internal/terraform/context_plan.go +++ b/internal/terraform/context_plan.go @@ -529,25 +529,27 @@ func (c *Context) postPlanValidateMoves(config *configs.Config, stmts []refactor // relaxed. func (c *Context) postPlanValidateImports(config *configs.Config, importTargets []*ImportTarget, allInst instances.Set) tfdiags.Diagnostics { var diags tfdiags.Diagnostics - for _, it := range importTargets { - // We only care about import target addresses that have a key. - // If the address does not have a key, we don't need it to be in config - // because are able to generate config. - if it.Addr.Resource.Key == nil { - continue - } - if !allInst.HasResourceInstance(it.Addr) { - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Cannot import to non-existent resource address", - fmt.Sprintf( - "Importing to resource address %s is not possible, because that address does not exist in configuration. Please ensure that the resource key is correct, or remove this import block.", - it.Addr, - ), - )) - } - } + // FIXME + //for _, it := range importTargets { + // // We only care about import target addresses that have a key. + // // If the address does not have a key, we don't need it to be in config + // // because are able to generate config. + // if it.Addr.Resource.Key == nil { + // continue + // } + + // if !allInst.HasResourceInstance(it.Addr) { + // diags = diags.Append(tfdiags.Sourceless( + // tfdiags.Error, + // "Cannot import to non-existent resource address", + // fmt.Sprintf( + // "Importing to resource address %s is not possible, because that address does not exist in configuration. Please ensure that the resource key is correct, or remove this import block.", + // it.Addr, + // ), + // )) + // } + //} return diags } @@ -556,13 +558,13 @@ func (c *Context) postPlanValidateImports(config *configs.Config, importTargets func (c *Context) findImportTargets(config *configs.Config, priorState *states.State) []*ImportTarget { var importTargets []*ImportTarget for _, ic := range config.Module.Import { - if priorState.ResourceInstance(ic.To) == nil { - importTargets = append(importTargets, &ImportTarget{ - Addr: ic.To, - ID: ic.ID, - Config: ic, - }) - } + // TODO: partial filtering here + //if priorState.ResourceInstance(ic.To) == nil { + importTargets = append(importTargets, &ImportTarget{ + ID: ic.ID, + Config: ic, + }) + //} } return importTargets } diff --git a/internal/terraform/eval_import.go b/internal/terraform/eval_import.go index ed024f31bc..e8118e7ec3 100644 --- a/internal/terraform/eval_import.go +++ b/internal/terraform/eval_import.go @@ -8,12 +8,13 @@ import ( "github.com/hashicorp/hcl/v2" "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/instances" "github.com/hashicorp/terraform/internal/tfdiags" "github.com/zclconf/go-cty/cty" "github.com/zclconf/go-cty/cty/gocty" ) -func evaluateImportIdExpression(expr hcl.Expression, ctx EvalContext) (string, tfdiags.Diagnostics) { +func evaluateImportIdExpression(expr hcl.Expression, ctx EvalContext, keyData instances.RepetitionData) (string, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics // import blocks only exist in the root module, and must be evaluated in @@ -29,7 +30,8 @@ func evaluateImportIdExpression(expr hcl.Expression, ctx EvalContext) (string, t }) } - importIdVal, evalDiags := ctx.EvaluateExpr(expr, cty.String, nil) + scope := ctx.EvaluationScope(nil, nil, keyData) + importIdVal, evalDiags := scope.EvalExpr(expr, cty.String) diags = diags.Append(evalDiags) if importIdVal.IsNull() { diff --git a/internal/terraform/node_resource_abstract.go b/internal/terraform/node_resource_abstract.go index 4bf7e98959..dae9de717b 100644 --- a/internal/terraform/node_resource_abstract.go +++ b/internal/terraform/node_resource_abstract.go @@ -206,11 +206,15 @@ func (n *NodeAbstractResource) References() []*addrs.Reference { } } +<<<<<<< HEAD return result } func (n *NodeAbstractResource) ImportReferences() []*addrs.Reference { var result []*addrs.Reference +======= + // TODO: for_each +>>>>>>> 2aac20391a (STASH) for _, importTarget := range n.importTargets { refs, _ := lang.ReferencesInExpr(addrs.ParseRef, importTarget.ID) result = append(result, refs...) diff --git a/internal/terraform/node_resource_plan.go b/internal/terraform/node_resource_plan.go index 81ba8f46c6..11e454fac7 100644 --- a/internal/terraform/node_resource_plan.go +++ b/internal/terraform/node_resource_plan.go @@ -7,10 +7,13 @@ import ( "fmt" "strings" + "github.com/hashicorp/hcl/v2" "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/dag" + "github.com/hashicorp/terraform/internal/instances" "github.com/hashicorp/terraform/internal/states" "github.com/hashicorp/terraform/internal/tfdiags" + "github.com/zclconf/go-cty/cty" ) // nodeExpandPlannableResource represents an addrs.ConfigResource and implements @@ -302,9 +305,84 @@ func (n *nodeExpandPlannableResource) expandResourceInstances(globalCtx EvalCont return diags.ErrWithWarnings() } +// Import blocks are expanded in conjunction with their associated resource block. +func (n nodeExpandPlannableResource) expandResourceImports(ctx EvalContext, addr addrs.AbsResource) (addrs.Map[addrs.AbsResourceInstance, string], tfdiags.Diagnostics) { + imports := addrs.MakeMap[addrs.AbsResourceInstance, string]() + var diags tfdiags.Diagnostics + + for _, imp := range n.importTargets { + if imp.Config == nil { + continue + } + + if !imp.Config.ToResource.Equal(addr.Config()) { + continue + } + + if imp.Config.ForEach == nil { + importID, evalDiags := evaluateImportIdExpression(imp.ID, ctx, EvalDataForNoInstanceKey) + diags = diags.Append(evalDiags) + if diags.HasErrors() { + return imports, diags + } + + // we already parsed this loading the config, so don't bother with diagnostics again + traversal, _ := hcl.AbsTraversalForExpr(imp.Config.To) + to, _ := addrs.ParseAbsResourceInstance(traversal) + imports.Put(to, importID) + } + + // FIXME: This won't work for lists like a dynamic block, but we still + // want the same errors because the values must be known for import. + forEach, forEachDiags := evaluateForEachExpression(imp.Config.ForEach, ctx) + diags = diags.Append(forEachDiags) + if forEachDiags.HasErrors() { + return imports, diags + } + + for k, v := range forEach { + keyData := instances.RepetitionData{ + EachKey: cty.StringVal(k), + EachValue: v, + } + + importID, evalDiags := evaluateImportIdExpression(imp.ID, ctx, keyData) + diags = diags.Append(evalDiags) + if diags.HasErrors() { + return imports, diags + } + + // FIXME: evalReplaceTriggeredByExpr works within a module scope + ref, evalDiags := evalReplaceTriggeredByExpr(imp.Config.To, keyData) + diags = diags.Append(evalDiags) + if diags.HasErrors() { + return imports, diags + } + // The reference is either a resource or resource instance + switch sub := ref.Subject.(type) { + case addrs.Resource: + imports.Put(sub.Absolute(ctx.Path()).Instance(addrs.NoKey), importID) + case addrs.ResourceInstance: + imports.Put(sub.Absolute(ctx.Path()), importID) + default: + // FIXME: + panic(fmt.Sprintf("invalid import address: %#v", sub)) + } + } + } + + return imports, diags +} + +// TODO: incorporate diagnostics func (n *nodeExpandPlannableResource) resourceInstanceSubgraph(ctx EvalContext, addr addrs.AbsResource, instanceAddrs []addrs.AbsResourceInstance) (*Graph, error) { var diags tfdiags.Diagnostics + // Now that the resources are all expanded, we can expand the imports for + // this resource. + imports, importDiags := n.expandResourceImports(ctx, addr) + diags = diags.Append(importDiags) + // Our graph transformers require access to the full state, so we'll // temporarily lock it while we work on this. state := ctx.State().Lock() @@ -318,21 +396,10 @@ func (n *nodeExpandPlannableResource) resourceInstanceSubgraph(ctx EvalContext, // to return the import node, not a plannable resource node. if n.legacyImportMode { for _, importTarget := range n.importTargets { - if importTarget.Addr.Equal(a.Addr) { - - // The import ID was supplied as a string on the command - // line and made into a synthetic HCL expression. - importId, diags := evaluateImportIdExpression(importTarget.ID, ctx) - if diags.HasErrors() { - // This should be impossible, because the import command - // arg parsing builds the synth expression from a - // non-null string. - panic(fmt.Sprintf("Invalid import id: %s. This is a bug in Terraform; please report it!", diags.Err())) - } - + if importTarget.LegacyAddr.Equal(a.Addr) { return &graphNodeImportState{ - Addr: importTarget.Addr, - ID: importId, + Addr: importTarget.LegacyAddr, + ID: importTarget.idString, ResolvedProvider: n.ResolvedProvider, } } @@ -362,15 +429,10 @@ func (n *nodeExpandPlannableResource) resourceInstanceSubgraph(ctx EvalContext, forceReplace: n.forceReplace, } - for _, importTarget := range n.importTargets { - if importTarget.Addr.Equal(a.Addr) { - // 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, - Config: importTarget.Config, - } + importID, ok := imports.GetOk(a.Addr) + if ok { + m.importTarget = ImportTarget{ + idString: importID, } } @@ -429,6 +491,9 @@ func (n *nodeExpandPlannableResource) resourceInstanceSubgraph(ctx EvalContext, Steps: steps, Name: "nodeExpandPlannableResource", } - graph, diags := b.Build(addr.Module) + graph, graphDiags := b.Build(addr.Module) + diags = diags.Append(graphDiags) + + // FIXME: diagnostics! return graph, diags.ErrWithWarnings() } diff --git a/internal/terraform/node_resource_plan_instance.go b/internal/terraform/node_resource_plan_instance.go index ecdd5a5d64..37e933a55b 100644 --- a/internal/terraform/node_resource_plan_instance.go +++ b/internal/terraform/node_resource_plan_instance.go @@ -159,18 +159,8 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) } } - importing := n.importTarget.ID != nil - var importId string - - if importing { - var evalDiags tfdiags.Diagnostics - - importId, evalDiags = evaluateImportIdExpression(n.importTarget.ID, ctx) - if evalDiags.HasErrors() { - diags = diags.Append(evalDiags) - return diags - } - } + importing := n.importTarget.idString != "" + importId := n.importTarget.idString if importing && n.Config == nil && len(n.generateConfigPath) == 0 { // Then the user wrote an import target to a target that didn't exist. @@ -505,7 +495,7 @@ func (n *NodePlannableResourceInstance) importState(ctx EvalContext, addr addrs. })) if imported[0].TypeName == "" { - diags = diags.Append(fmt.Errorf("import of %s didn't set type", n.importTarget.Addr.String())) + diags = diags.Append(fmt.Errorf("import of %s didn't set type", n.Addr.String())) return nil, diags } @@ -524,7 +514,7 @@ func (n *NodePlannableResourceInstance) importState(ctx EvalContext, addr addrs. // refresh riNode := &NodeAbstractResourceInstance{ - Addr: n.importTarget.Addr, + Addr: n.Addr, NodeAbstractResource: NodeAbstractResource{ ResolvedProvider: n.ResolvedProvider, }, @@ -548,7 +538,7 @@ func (n *NodePlannableResourceInstance) importState(ctx EvalContext, addr addrs. "is correct and that it is associated with the provider's "+ "configured region or endpoint, or use \"terraform apply\" to "+ "create a new remote object for this resource.", - n.importTarget.Addr, + n.Addr, ), )) return instanceRefreshState, diags diff --git a/internal/terraform/transform_config.go b/internal/terraform/transform_config.go index 2afa0b9e30..a590135464 100644 --- a/internal/terraform/transform_config.go +++ b/internal/terraform/transform_config.go @@ -7,6 +7,7 @@ import ( "fmt" "log" + "github.com/hashicorp/hcl/v2" "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/dag" @@ -102,8 +103,16 @@ func (t *ConfigTransformer) transformSingle(g *Graph, config *configs.Config, ge // Only include import targets that are targeting the current module. var importTargets []*ImportTarget for _, target := range t.importTargets { - if targetModule := target.Addr.Module.Module(); targetModule.Equal(config.Path) { - importTargets = append(importTargets, target) + // FIXME legacy mode handling is ugly + switch { + case target.Config == nil: + if target.LegacyAddr.Module.Module().Equal(config.Path) { + importTargets = append(importTargets, target) + } + default: + if target.Config.ToResource.Module.Equal(config.Path) { + importTargets = append(importTargets, target) + } } } @@ -122,7 +131,13 @@ func (t *ConfigTransformer) transformSingle(g *Graph, config *configs.Config, ge var matchedIndices []int for ix, i := range importTargets { - if target := i.Addr.ContainingResource().Config(); target.Equal(configAddr) { + // FIXME + if i.LegacyAddr.ConfigResource().Equal(configAddr) { + matchedIndices = append(matchedIndices, ix) + imports = append(imports, i) + + } + if i.Config != nil && i.Config.ToResource.Equal(configAddr) { // This import target has been claimed by an actual resource, // let's make a note of this to remove it from the targets. matchedIndices = append(matchedIndices, ix) @@ -171,14 +186,18 @@ func (t *ConfigTransformer) transformSingle(g *Graph, config *configs.Config, ge // TODO: We could actually catch and process these kind of problems earlier, // this is something that could be done during the Validate process. for _, i := range importTargets { + // we already parsed this loading the config, so don't bother with diagnostics again + // FIXME: legacy? + traversal, _ := hcl.AbsTraversalForExpr(i.Config.To) + to, _ := addrs.ParseAbsResourceInstance(traversal) // The case in which an unmatched import block targets an expanded // resource instance can error here. Others can error later. - if i.Addr.Resource.Key != addrs.NoKey { - return fmt.Errorf("Config generation for count and for_each resources not supported.\n\nYour configuration contains an import block with a \"to\" address of %s. This resource instance does not exist in configuration.\n\nIf you intended to target a resource that exists in configuration, please double-check the address. Otherwise, please remove this import block or re-run the plan without the -generate-config-out flag to ignore the import block.", i.Addr) + if to.Resource.Key != addrs.NoKey { + return fmt.Errorf("Config generation for count and for_each resources not supported.\n\nYour configuration contains an import block with a \"to\" address of %s. This resource instance does not exist in configuration.\n\nIf you intended to target a resource that exists in configuration, please double-check the address. Otherwise, please remove this import block or re-run the plan without the -generate-config-out flag to ignore the import block.", to) } abstract := &NodeAbstractResource{ - Addr: i.Addr.ConfigResource(), + Addr: to.ConfigResource(), importTargets: []*ImportTarget{i}, generateConfigPath: generateConfigPath, }