From 041d9d3eec810bfa26bc27a4ca59a1a51f82a444 Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Tue, 27 Sep 2022 12:02:44 -0400
Subject: [PATCH] unknown evaluation of missing instances in import

Because import does not yet plan new instances as part of the import
process, we can end up evaluating references to resources which have no
state at all. The fallback for this situation could result in slightly
better values during import. The count and for_each values were
technically incorrect, since the length is not known to be zero, and the
single instance does have a concrete type which we can return.
---
 internal/terraform/context_import_test.go     | 70 +++++++++++++++++++
 internal/terraform/evaluate.go                | 23 ++++++
 .../import-provider-resources/main.tf         |  2 +-
 3 files changed, 94 insertions(+), 1 deletion(-)

diff --git a/internal/terraform/context_import_test.go b/internal/terraform/context_import_test.go
index dc773dcf1f..920fe23438 100644
--- a/internal/terraform/context_import_test.go
+++ b/internal/terraform/context_import_test.go
@@ -915,6 +915,76 @@ resource "test_resource" "unused" {
 	}
 }
 
+// New resources in the config during import won't exist for evaluation
+// purposes (until import is upgraded to using a complete plan). This means
+// that references to them are unknown, but in the case of single instances, we
+// can at least know the type of unknown value.
+func TestContextImport_newResourceUnknown(t *testing.T) {
+	p := testProvider("aws")
+	m := testModuleInline(t, map[string]string{
+		"main.tf": `
+resource "test_resource" "one" {
+}
+
+resource "test_resource" "two" {
+  count = length(flatten([test_resource.one.id]))
+}
+
+resource "test_resource" "test" {
+}
+`})
+
+	p.GetProviderSchemaResponse = getProviderSchemaResponseFromProviderSchema(&ProviderSchema{
+		ResourceTypes: map[string]*configschema.Block{
+			"test_resource": {
+				Attributes: map[string]*configschema.Attribute{
+					"id": {Type: cty.String, Computed: true},
+				},
+			},
+		},
+	})
+
+	p.ImportResourceStateResponse = &providers.ImportResourceStateResponse{
+		ImportedResources: []providers.ImportedResource{
+			{
+				TypeName: "test_resource",
+				State: cty.ObjectVal(map[string]cty.Value{
+					"id": cty.StringVal("test"),
+				}),
+			},
+		},
+	}
+
+	ctx := testContext2(t, &ContextOpts{
+		Providers: map[addrs.Provider]providers.Factory{
+			addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
+		},
+	})
+
+	state, diags := ctx.Import(m, states.NewState(), &ImportOpts{
+		Targets: []*ImportTarget{
+			{
+				Addr: addrs.RootModuleInstance.ResourceInstance(
+					addrs.ManagedResourceMode, "test_resource", "test", addrs.NoKey,
+				),
+				ID: "test",
+			},
+		},
+	})
+	if diags.HasErrors() {
+		t.Fatal(diags.ErrWithWarnings())
+	}
+
+	ri := state.ResourceInstance(mustResourceInstanceAddr("test_resource.test"))
+	expected := `{"id":"test"}`
+	if ri == nil || ri.Current == nil {
+		t.Fatal("no state is recorded for resource instance test_resource.test")
+	}
+	if string(ri.Current.AttrsJSON) != expected {
+		t.Fatalf("expected %q, got %q\n", expected, ri.Current.AttrsJSON)
+	}
+}
+
 const testImportStr = `
 aws_instance.foo:
   ID = foo
diff --git a/internal/terraform/evaluate.go b/internal/terraform/evaluate.go
index f8df6a2dc0..b841dbd863 100644
--- a/internal/terraform/evaluate.go
+++ b/internal/terraform/evaluate.go
@@ -695,6 +695,29 @@ func (d *evaluationStateData) GetResource(addr addrs.Resource, rng tfdiags.Sourc
 				return cty.DynamicVal, diags
 			}
 
+		case walkImport:
+			// Import does not yet plan resource changes, so new resources from
+			// config are not going to be found here. Once walkImport fully
+			// plans resources, this case should not longer be needed.
+			// In the single instance case, we can return a typed unknown value
+			// for the instance to better satisfy other expressions using the
+			// value. This of course will not help if statically known
+			// attributes are expected to be known elsewhere, but reduces the
+			// number of problematic configs for now.
+			// Unlike in plan and apply above we can't be sure the count or
+			// for_each instances are empty, so we return a DynamicVal. We
+			// don't really have a good value to return otherwise -- empty
+			// values will fail for direct index expressions, and unknown
+			// Lists and Maps could fail in some type unifications.
+			switch {
+			case config.Count != nil:
+				return cty.DynamicVal, diags
+			case config.ForEach != nil:
+				return cty.DynamicVal, diags
+			default:
+				return cty.UnknownVal(ty), diags
+			}
+
 		default:
 			// We should only end up here during the validate walk,
 			// since later walks should have at least partial states populated
diff --git a/internal/terraform/testdata/import-provider-resources/main.tf b/internal/terraform/testdata/import-provider-resources/main.tf
index cb92470fab..a99ee5e941 100644
--- a/internal/terraform/testdata/import-provider-resources/main.tf
+++ b/internal/terraform/testdata/import-provider-resources/main.tf
@@ -1,5 +1,5 @@
 provider "aws" {
-    value = "${test_instance.bar.value}"
+    value = "${test_instance.bar.id}"
 }
 
 resource "aws_instance" "foo" {