From 207b49b0306717f999a58cc456b36573068393f6 Mon Sep 17 00:00:00 2001 From: Katy Moe Date: Tue, 25 Jul 2023 11:04:08 +0100 Subject: [PATCH] allow interpolation in import block id The import block id field can now reference variables, attributes, and module outputs, as long as the result is a known non-empty string at plan time. A null or unknown value will result in an error. This commit slightly modifies the legacy CLI terraform import code path to construct a synthetic hcl.Expression from the import id passed in from the command line, with no intended change of functionality. --- internal/command/import.go | 6 +- internal/configs/import.go | 7 +- internal/configs/import_test.go | 15 +- internal/terraform/context_import.go | 6 +- internal/terraform/context_import_test.go | 57 +++-- internal/terraform/context_plan2_test.go | 234 ++++++++++++++++++ internal/terraform/eval_import.go | 60 +++++ internal/terraform/node_resource_abstract.go | 11 +- internal/terraform/node_resource_plan.go | 13 +- .../terraform/node_resource_plan_instance.go | 29 ++- .../testdata/import-id-data-source/main.tf | 11 + .../terraform/testdata/import-id-func/main.tf | 8 + .../testdata/import-id-invalid-null/main.tf | 10 + .../import-id-invalid-unknown/main.tf | 12 + .../testdata/import-id-module/child/main.tf | 3 + .../testdata/import-id-module/main.tf | 10 + .../testdata/import-id-variable/main.tf | 11 + 17 files changed, 457 insertions(+), 46 deletions(-) create mode 100644 internal/terraform/eval_import.go create mode 100644 internal/terraform/testdata/import-id-data-source/main.tf create mode 100644 internal/terraform/testdata/import-id-func/main.tf create mode 100644 internal/terraform/testdata/import-id-invalid-null/main.tf create mode 100644 internal/terraform/testdata/import-id-invalid-unknown/main.tf create mode 100644 internal/terraform/testdata/import-id-module/child/main.tf create mode 100644 internal/terraform/testdata/import-id-module/main.tf create mode 100644 internal/terraform/testdata/import-id-variable/main.tf diff --git a/internal/command/import.go b/internal/command/import.go index e79054bba7c1..e910f51a4e8e 100644 --- a/internal/command/import.go +++ b/internal/command/import.go @@ -12,6 +12,7 @@ import ( "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/hclsyntax" + "github.com/zclconf/go-cty/cty" "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/backend" @@ -236,7 +237,10 @@ func (c *ImportCommand) Run(args []string) int { Targets: []*terraform.ImportTarget{ { Addr: addr, - ID: args[1], + + // In the import block, the ID can be an arbitrary hcl.Expression, + // but here it's always interpreted as a literal string. + ID: hcl.StaticExpr(cty.StringVal(args[1]), configs.SynthBody("import", nil).MissingItemRange()), }, }, diff --git a/internal/configs/import.go b/internal/configs/import.go index 5771f8d48563..639595b461f6 100644 --- a/internal/configs/import.go +++ b/internal/configs/import.go @@ -5,12 +5,11 @@ package configs import ( "github.com/hashicorp/hcl/v2" - "github.com/hashicorp/hcl/v2/gohcl" "github.com/hashicorp/terraform/internal/addrs" ) type Import struct { - ID string + ID hcl.Expression To addrs.AbsResourceInstance ProviderConfigRef *ProviderConfigRef @@ -30,9 +29,7 @@ func decodeImportBlock(block *hcl.Block) (*Import, hcl.Diagnostics) { diags = append(diags, moreDiags...) if attr, exists := content.Attributes["id"]; exists { - attrDiags := gohcl.DecodeExpression(attr.Expr, nil, &imp.ID) - diags = append(diags, attrDiags...) - + imp.ID = attr.Expr } if attr, exists := content.Attributes["to"]; exists { diff --git a/internal/configs/import_test.go b/internal/configs/import_test.go index 2b3bd6fc1c96..bb3ae41805ff 100644 --- a/internal/configs/import_test.go +++ b/internal/configs/import_test.go @@ -14,6 +14,11 @@ import ( "github.com/zclconf/go-cty/cty" ) +var ( + typeComparer = cmp.Comparer(cty.Type.Equals) + valueComparer = cmp.Comparer(cty.Value.RawEquals) +) + func TestImportBlock_decode(t *testing.T) { blockRange := hcl.Range{ Filename: "mock.tf", @@ -52,7 +57,7 @@ func TestImportBlock_decode(t *testing.T) { }, &Import{ To: mustAbsResourceInstanceAddr("test_instance.bar"), - ID: "foo", + ID: foo_str_expr, DeclRange: blockRange, }, ``, @@ -76,7 +81,7 @@ func TestImportBlock_decode(t *testing.T) { }, &Import{ To: mustAbsResourceInstanceAddr("test_instance.bar[\"one\"]"), - ID: "foo", + ID: foo_str_expr, DeclRange: blockRange, }, ``, @@ -100,7 +105,7 @@ func TestImportBlock_decode(t *testing.T) { }, &Import{ To: mustAbsResourceInstanceAddr("module.bar.test_instance.bar"), - ID: "foo", + ID: foo_str_expr, DeclRange: blockRange, }, ``, @@ -138,7 +143,7 @@ func TestImportBlock_decode(t *testing.T) { DefRange: blockRange, }, &Import{ - ID: "foo", + ID: foo_str_expr, DeclRange: blockRange, }, "Missing required argument", @@ -160,7 +165,7 @@ func TestImportBlock_decode(t *testing.T) { t.Fatal("expected error") } - if !cmp.Equal(got, test.want, cmp.AllowUnexported(addrs.MoveEndpoint{})) { + if !cmp.Equal(got, test.want, typeComparer, valueComparer) { t.Fatalf("wrong result: %s", cmp.Diff(got, test.want)) } }) diff --git a/internal/terraform/context_import.go b/internal/terraform/context_import.go index 2b638734986d..376a07b4ee1b 100644 --- a/internal/terraform/context_import.go +++ b/internal/terraform/context_import.go @@ -6,6 +6,7 @@ package terraform import ( "log" + "github.com/hashicorp/hcl/v2" "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/states" @@ -22,7 +23,8 @@ type ImportOpts struct { SetVariables InputValues } -// ImportTarget is a single resource to import. +// ImportTarget is a single resource to import, +// in legacy (CLI) import mode. type ImportTarget struct { // Config is the original import block for this import. This might be null // if the import did not originate in config. @@ -33,7 +35,7 @@ type ImportTarget struct { Addr addrs.AbsResourceInstance // ID is the ID of the resource to import. This is resource-specific. - ID string + ID hcl.Expression } // 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 b526ede951de..d4a8a230f777 100644 --- a/internal/terraform/context_import_test.go +++ b/internal/terraform/context_import_test.go @@ -11,7 +11,10 @@ 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" @@ -37,13 +40,14 @@ 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( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: "bar", + ID: barExpr, }, }, }) @@ -88,13 +92,14 @@ 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( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.IntKey(0), ), - ID: "bar", + ID: barExpr, }, }, }) @@ -149,13 +154,14 @@ 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( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: "bar", + ID: barExpr, }, }, }) @@ -193,13 +199,14 @@ 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( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: "bar", + ID: barExpr, }, }, }) @@ -244,13 +251,14 @@ 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( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: "bar", + ID: barExpr, }, }, }) @@ -299,13 +307,14 @@ 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( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: "bar", + ID: barExpr, }, }, }) @@ -355,13 +364,14 @@ 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( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: "bar", + ID: barExpr, }, }, SetVariables: InputValues{ @@ -415,13 +425,14 @@ 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( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: "bar", + ID: barExpr, }, }, }) @@ -486,13 +497,14 @@ 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( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: "bar", + ID: barExpr, }, }, }) @@ -537,13 +549,14 @@ 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( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: "bar", + ID: barExpr, }, }, }) @@ -578,13 +591,14 @@ 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( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: "bar", + ID: barExpr, }, }, }) @@ -619,13 +633,14 @@ 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( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: "baz", + ID: bazExpr, }, }, }) @@ -660,13 +675,14 @@ 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( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: "baz", + ID: bazExpr, }, }, }) @@ -728,13 +744,14 @@ 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( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: "bar", + ID: barExpr, }, }, }) @@ -802,13 +819,14 @@ 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( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: "bar", + ID: barExpr, }, }, }) @@ -896,13 +914,14 @@ 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( addrs.ManagedResourceMode, "test_resource", "test", addrs.NoKey, ), - ID: "test", + ID: testExpr, }, }, }) @@ -966,13 +985,14 @@ 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( addrs.ManagedResourceMode, "test_resource", "test", addrs.NoKey, ), - ID: "test", + ID: testExpr, }, }, }) @@ -993,6 +1013,7 @@ 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{ @@ -1017,7 +1038,7 @@ func TestContextImport_33572(t *testing.T) { Addr: addrs.RootModuleInstance.ResourceInstance( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: "bar", + ID: bar_expr, }, }, }) diff --git a/internal/terraform/context_plan2_test.go b/internal/terraform/context_plan2_test.go index 367b367e9244..6f05b8711854 100644 --- a/internal/terraform/context_plan2_test.go +++ b/internal/terraform/context_plan2_test.go @@ -15,8 +15,11 @@ import ( "github.com/google/go-cmp/cmp" "github.com/zclconf/go-cty/cty" + // "github.com/hashicorp/hcl/v2" "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/checks" + + // "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/configs/configschema" "github.com/hashicorp/terraform/internal/lang/marks" "github.com/hashicorp/terraform/internal/plans" @@ -4595,6 +4598,237 @@ import { } } +func TestContext2Plan_importIdVariable(t *testing.T) { + p := testProvider("aws") + m := testModule(t, "import-id-variable") + ctx := testContext2(t, &ContextOpts{ + Providers: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("aws"): testProviderFuncFixed(p), + }, + }) + + p.ImportResourceStateResponse = &providers.ImportResourceStateResponse{ + ImportedResources: []providers.ImportedResource{ + { + TypeName: "aws_instance", + State: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("foo"), + }), + }, + }, + } + + _, diags := ctx.Plan(m, states.NewState(), &PlanOpts{ + SetVariables: InputValues{ + "the_id": &InputValue{ + // let var take its default value + Value: cty.NilVal, + }, + }, + }) + if diags.HasErrors() { + t.Fatalf("unexpected errors: %s", diags.Err()) + } +} + +func TestContext2Plan_importIdFunc(t *testing.T) { + p := testProvider("aws") + m := testModule(t, "import-id-func") + ctx := testContext2(t, &ContextOpts{ + Providers: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("aws"): testProviderFuncFixed(p), + }, + }) + + p.ImportResourceStateResponse = &providers.ImportResourceStateResponse{ + ImportedResources: []providers.ImportedResource{ + { + TypeName: "aws_instance", + State: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("foo"), + }), + }, + }, + } + + _, diags := ctx.Plan(m, states.NewState(), DefaultPlanOpts) + if diags.HasErrors() { + t.Fatalf("unexpected errors: %s", diags.Err()) + } +} + +func TestContext2Plan_importIdDataSource(t *testing.T) { + p := testProvider("aws") + m := testModule(t, "import-id-data-source") + + p.GetProviderSchemaResponse = getProviderSchemaResponseFromProviderSchema(&ProviderSchema{ + ResourceTypes: map[string]*configschema.Block{ + "aws_subnet": { + Attributes: map[string]*configschema.Attribute{ + "id": { + Type: cty.String, + Computed: true, + }, + }, + }, + }, + DataSources: map[string]*configschema.Block{ + "aws_subnet": { + Attributes: map[string]*configschema.Attribute{ + "vpc_id": { + Type: cty.String, + Required: true, + }, + "cidr_block": { + Type: cty.String, + Computed: true, + }, + "id": { + Type: cty.String, + Computed: true, + }, + }, + }, + }, + }) + p.ReadDataSourceResponse = &providers.ReadDataSourceResponse{ + State: cty.ObjectVal(map[string]cty.Value{ + "vpc_id": cty.StringVal("abc"), + "cidr_block": cty.StringVal("10.0.1.0/24"), + "id": cty.StringVal("123"), + }), + } + p.ImportResourceStateResponse = &providers.ImportResourceStateResponse{ + ImportedResources: []providers.ImportedResource{ + { + TypeName: "aws_subnet", + State: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("foo"), + }), + }, + }, + } + ctx := testContext2(t, &ContextOpts{ + Providers: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("aws"): testProviderFuncFixed(p), + }, + }) + + _, diags := ctx.Plan(m, states.NewState(), DefaultPlanOpts) + if diags.HasErrors() { + t.Fatalf("unexpected errors: %s", diags.Err()) + } +} + +func TestContext2Plan_importIdModule(t *testing.T) { + p := testProvider("aws") + m := testModule(t, "import-id-module") + + p.GetProviderSchemaResponse = getProviderSchemaResponseFromProviderSchema(&ProviderSchema{ + ResourceTypes: map[string]*configschema.Block{ + "aws_lb": { + Attributes: map[string]*configschema.Attribute{ + "id": { + Type: cty.String, + Computed: true, + }, + }, + }, + }, + }) + p.ImportResourceStateResponse = &providers.ImportResourceStateResponse{ + ImportedResources: []providers.ImportedResource{ + { + TypeName: "aws_lb", + State: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("foo"), + }), + }, + }, + } + ctx := testContext2(t, &ContextOpts{ + Providers: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("aws"): testProviderFuncFixed(p), + }, + }) + + _, diags := ctx.Plan(m, states.NewState(), DefaultPlanOpts) + if diags.HasErrors() { + t.Fatalf("unexpected errors: %s", diags.Err()) + } +} + +func TestContext2Plan_importIdInvalidNull(t *testing.T) { + p := testProvider("test") + m := testModule(t, "import-id-invalid-null") + ctx := testContext2(t, &ContextOpts{ + Providers: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("test"): testProviderFuncFixed(p), + }, + }) + + _, diags := ctx.Plan(m, states.NewState(), &PlanOpts{ + SetVariables: InputValues{ + "the_id": &InputValue{ + Value: cty.NullVal(cty.String), + }, + }, + }) + if !diags.HasErrors() { + t.Fatal("succeeded; want errors") + } + if got, want := diags.Err().Error(), "The import ID cannot be null"; !strings.Contains(got, want) { + t.Fatalf("wrong error:\ngot: %s\nwant: message containing %q", got, want) + } +} + +func TestContext2Plan_importIdInvalidUnknown(t *testing.T) { + p := testProvider("test") + m := testModule(t, "import-id-invalid-unknown") + ctx := testContext2(t, &ContextOpts{ + Providers: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("test"): testProviderFuncFixed(p), + }, + }) + p.GetProviderSchemaResponse = getProviderSchemaResponseFromProviderSchema(&ProviderSchema{ + ResourceTypes: map[string]*configschema.Block{ + "test_resource": { + Attributes: map[string]*configschema.Attribute{ + "id": { + Type: cty.String, + Computed: true, + }, + }, + }, + }, + }) + p.PlanResourceChangeFn = func(req providers.PlanResourceChangeRequest) providers.PlanResourceChangeResponse { + return providers.PlanResourceChangeResponse{ + PlannedState: cty.UnknownVal(cty.Object(map[string]cty.Type{ + "id": cty.String, + })), + } + } + p.ImportResourceStateResponse = &providers.ImportResourceStateResponse{ + ImportedResources: []providers.ImportedResource{ + { + TypeName: "test_resource", + State: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("foo"), + }), + }, + }, + } + + _, diags := ctx.Plan(m, states.NewState(), DefaultPlanOpts) + if !diags.HasErrors() { + t.Fatal("succeeded; want errors") + } + if got, want := diags.Err().Error(), `The import block "id" argument depends on resource attributes that cannot be determined until apply`; !strings.Contains(got, want) { + t.Fatalf("wrong error:\ngot: %s\nwant: message containing %q", got, want) + } +} + func TestContext2Plan_importIntoModuleWithGeneratedConfig(t *testing.T) { m := testModuleInline(t, map[string]string{ "main.tf": ` diff --git a/internal/terraform/eval_import.go b/internal/terraform/eval_import.go new file mode 100644 index 000000000000..89d51f846b45 --- /dev/null +++ b/internal/terraform/eval_import.go @@ -0,0 +1,60 @@ +package terraform + +import ( + "fmt" + + "github.com/hashicorp/hcl/v2" + "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) { + var diags tfdiags.Diagnostics + + if expr == nil { + return "", diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid import id argument", + Detail: "The import ID cannot be null.", + Subject: expr.Range().Ptr(), + }) + } + + importIdVal, evalDiags := ctx.EvaluateExpr(expr, cty.String, nil) + diags = diags.Append(evalDiags) + + if importIdVal.IsNull() { + return "", diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid import id argument", + Detail: "The import ID cannot be null.", + Subject: expr.Range().Ptr(), + }) + } + + if !importIdVal.IsKnown() { + return "", diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid import id argument", + Detail: `The import block "id" argument depends on resource attributes that cannot be determined until apply, so Terraform cannot plan to import this resource.`, // FIXME and what should I do about that? + Subject: expr.Range().Ptr(), + // Expression: + // EvalContext: + Extra: diagnosticCausedByUnknown(true), + }) + } + + var importId string + err := gocty.FromCtyValue(importIdVal, &importId) + if err != nil { + return "", diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid import id argument", + Detail: fmt.Sprintf("The import ID value is unsuitable: %s.", err), + Subject: expr.Range().Ptr(), + }) + } + + return importId, diags +} diff --git a/internal/terraform/node_resource_abstract.go b/internal/terraform/node_resource_abstract.go index 3607979ece28..f94f2996381c 100644 --- a/internal/terraform/node_resource_abstract.go +++ b/internal/terraform/node_resource_abstract.go @@ -140,10 +140,9 @@ func (n *NodeAbstractResource) ReferenceableAddrs() []addrs.Referenceable { // GraphNodeReferencer func (n *NodeAbstractResource) References() []*addrs.Reference { + var result []*addrs.Reference // If we have a config then we prefer to use that. if c := n.Config; c != nil { - var result []*addrs.Reference - result = append(result, n.DependsOn()...) if n.Schema == nil { @@ -204,12 +203,14 @@ func (n *NodeAbstractResource) References() []*addrs.Reference { refs, _ = lang.ReferencesInExpr(addrs.ParseRef, check.ErrorMessage) result = append(result, refs...) } + } - return result + for _, importTarget := range n.importTargets { + refs, _ := lang.ReferencesInExpr(addrs.ParseRef, importTarget.ID) + result = append(result, refs...) } - // Otherwise, we have no references. - return nil + return result } func (n *NodeAbstractResource) DependsOn() []*addrs.Reference { diff --git a/internal/terraform/node_resource_plan.go b/internal/terraform/node_resource_plan.go index a2f235409536..c2bfbe05fe01 100644 --- a/internal/terraform/node_resource_plan.go +++ b/internal/terraform/node_resource_plan.go @@ -318,9 +318,20 @@ func (n *nodeExpandPlannableResource) resourceInstanceSubgraph(ctx EvalContext, 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())) + } + return &graphNodeImportState{ Addr: importTarget.Addr, - ID: importTarget.ID, + ID: importId, ResolvedProvider: n.ResolvedProvider, } } diff --git a/internal/terraform/node_resource_plan_instance.go b/internal/terraform/node_resource_plan_instance.go index 565054f160f7..94612aecb6f1 100644 --- a/internal/terraform/node_resource_plan_instance.go +++ b/internal/terraform/node_resource_plan_instance.go @@ -159,7 +159,18 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) } } - importing := n.importTarget.ID != "" + 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 + } + } if importing && n.Config == nil && len(n.generateConfigPath) == 0 { // Then the user wrote an import target to a target that didn't exist. @@ -187,7 +198,7 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) // If the resource is to be imported, we now ask the provider for an Import // and a Refresh, and save the resulting state to instanceRefreshState. if importing { - instanceRefreshState, diags = n.importState(ctx, addr, provider, providerSchema) + instanceRefreshState, diags = n.importState(ctx, addr, importId, provider, providerSchema) } else { var readDiags tfdiags.Diagnostics instanceRefreshState, readDiags = n.readResourceInstanceState(ctx, addr) @@ -297,7 +308,7 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) } if importing { - change.Importing = &plans.Importing{ID: n.importTarget.ID} + change.Importing = &plans.Importing{ID: importId} } // FIXME: here we udpate the change to reflect the reason for @@ -439,12 +450,12 @@ func (n *NodePlannableResourceInstance) replaceTriggered(ctx EvalContext, repDat return diags } -func (n *NodePlannableResourceInstance) importState(ctx EvalContext, addr addrs.AbsResourceInstance, provider providers.Interface, providerSchema providers.ProviderSchema) (*states.ResourceInstanceObject, tfdiags.Diagnostics) { +func (n *NodePlannableResourceInstance) importState(ctx EvalContext, addr addrs.AbsResourceInstance, importId string, provider providers.Interface, providerSchema providers.ProviderSchema) (*states.ResourceInstanceObject, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics absAddr := addr.Resource.Absolute(ctx.Path()) diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { - return h.PrePlanImport(absAddr, n.importTarget.ID) + return h.PrePlanImport(absAddr, importId) })) if diags.HasErrors() { return nil, diags @@ -452,7 +463,7 @@ func (n *NodePlannableResourceInstance) importState(ctx EvalContext, addr addrs. resp := provider.ImportResourceState(providers.ImportResourceStateRequest{ TypeName: addr.Resource.Resource.Type, - ID: n.importTarget.ID, + ID: importId, }) diags = diags.Append(resp.Diagnostics) if diags.HasErrors() { @@ -467,13 +478,13 @@ func (n *NodePlannableResourceInstance) importState(ctx EvalContext, addr addrs. "Import returned no resources", fmt.Sprintf("While attempting to import with ID %s, the provider"+ "returned no instance states.", - n.importTarget.ID, + importId, ), )) return nil, diags } for _, obj := range imported { - log.Printf("[TRACE] graphNodeImportState: import %s %q produced instance object of type %s", absAddr.String(), n.importTarget.ID, obj.TypeName) + log.Printf("[TRACE] graphNodeImportState: import %s %q produced instance object of type %s", absAddr.String(), importId, obj.TypeName) } if len(imported) > 1 { diags = diags.Append(tfdiags.Sourceless( @@ -482,7 +493,7 @@ func (n *NodePlannableResourceInstance) importState(ctx EvalContext, addr addrs. fmt.Sprintf("While attempting to import with ID %s, the provider "+ "returned multiple resource instance states. This "+ "is not currently supported.", - n.importTarget.ID, + importId, ), )) return nil, diags diff --git a/internal/terraform/testdata/import-id-data-source/main.tf b/internal/terraform/testdata/import-id-data-source/main.tf new file mode 100644 index 000000000000..ad524fe5fa11 --- /dev/null +++ b/internal/terraform/testdata/import-id-data-source/main.tf @@ -0,0 +1,11 @@ +data "aws_subnet" "bar" { + vpc_id = "abc" + cidr_block = "10.0.1.0/24" +} + +import { + to = aws_subnet.bar + id = data.aws_subnet.bar.id +} + +resource "aws_subnet" "bar" {} diff --git a/internal/terraform/testdata/import-id-func/main.tf b/internal/terraform/testdata/import-id-func/main.tf new file mode 100644 index 000000000000..f861fae06947 --- /dev/null +++ b/internal/terraform/testdata/import-id-func/main.tf @@ -0,0 +1,8 @@ +import { + to = aws_instance.foo + id = substr("hmm123", "2", "3") +} + +resource "aws_instance" "foo" { + +} diff --git a/internal/terraform/testdata/import-id-invalid-null/main.tf b/internal/terraform/testdata/import-id-invalid-null/main.tf new file mode 100644 index 000000000000..5de128d1ec66 --- /dev/null +++ b/internal/terraform/testdata/import-id-invalid-null/main.tf @@ -0,0 +1,10 @@ +variable "the_id" { + type = string +} + +import { + to = test_resource.foo + id = var.the_id +} + +resource "test_resource" "foo" {} diff --git a/internal/terraform/testdata/import-id-invalid-unknown/main.tf b/internal/terraform/testdata/import-id-invalid-unknown/main.tf new file mode 100644 index 000000000000..317e99f0061d --- /dev/null +++ b/internal/terraform/testdata/import-id-invalid-unknown/main.tf @@ -0,0 +1,12 @@ +resource "test_resource" "foo" { + +} + +import { + to = test_resource.bar + id = test_resource.foo.id +} + +resource "test_resource" "bar" { + +} diff --git a/internal/terraform/testdata/import-id-module/child/main.tf b/internal/terraform/testdata/import-id-module/child/main.tf new file mode 100644 index 000000000000..251f582ae5b5 --- /dev/null +++ b/internal/terraform/testdata/import-id-module/child/main.tf @@ -0,0 +1,3 @@ +output "lb_id" { + value = 1 +} diff --git a/internal/terraform/testdata/import-id-module/main.tf b/internal/terraform/testdata/import-id-module/main.tf new file mode 100644 index 000000000000..ef75c78ec191 --- /dev/null +++ b/internal/terraform/testdata/import-id-module/main.tf @@ -0,0 +1,10 @@ +module "child" { + source = "./child" +} + +import { + to = aws_lb.foo + id = module.child.lb_id +} + +resource "aws_lb" "foo" {} diff --git a/internal/terraform/testdata/import-id-variable/main.tf b/internal/terraform/testdata/import-id-variable/main.tf new file mode 100644 index 000000000000..85056efa8a73 --- /dev/null +++ b/internal/terraform/testdata/import-id-variable/main.tf @@ -0,0 +1,11 @@ +variable "the_id" { + default = "123" +} + +import { + to = aws_instance.foo + id = var.the_id +} + +resource "aws_instance" "foo" { +}