Merge pull request #30171 from hashicorp/jbardin/revert-validate-for-each

use `cty.DynamicVal` for expanded resources during validation

internal/terraform/context_plan_test.go

@@ -5308,7 +5308,6 @@ func TestContext2Plan_selfRefMultiAll(t *testing.T) {
 		ResourceTypes: map[string]*configschema.Block{
 			"aws_instance": {
 				Attributes: map[string]*configschema.Attribute{
-					"id":  {Type: cty.String, Computed: true},
 					"foo": {Type: cty.List(cty.String), Optional: true},
 				},
 			},

internal/terraform/context_validate_test.go

@@ -2031,3 +2031,60 @@ resource "test_object" "t" {
 		t.Fatal(diags.ErrWithWarnings())
 	}
 }
+
+func TestContext2Plan_lookupMismatchedObjectTypes(t *testing.T) {
+	p := new(MockProvider)
+	p.GetProviderSchemaResponse = getProviderSchemaResponseFromProviderSchema(&ProviderSchema{
+		ResourceTypes: map[string]*configschema.Block{
+			"test_instance": {
+				Attributes: map[string]*configschema.Attribute{
+					"id": {
+						Type:     cty.String,
+						Computed: true,
+					},
+					"things": {
+						Type:     cty.List(cty.String),
+						Optional: true,
+					},
+				},
+			},
+		},
+	})
+
+	m := testModuleInline(t, map[string]string{
+		"main.tf": `
+variable "items" {
+  type = list(string)
+  default = []
+}
+
+resource "test_instance" "a" {
+  for_each = length(var.items) > 0 ? { default = {} } : {}
+}
+
+output "out" {
+  // Strictly speaking, this expression is incorrect because the map element
+  // type is a different type from the default value, and the lookup
+  // implementation expects to be able to convert the default to match the
+  // element type.
+  // There are two reasons this works which we need to maintain for
+  // compatibility. First during validation the 'test_instance.a' expression
+  // only returns a dynamic value, preventing any type comparison. Later during
+  // plan and apply 'test_instance.a' is an object and not a map, and the
+  // lookup implementation skips the type comparison when the keys are known
+  // statically.
+  value = lookup(test_instance.a, "default", { id = null })["id"]
+}
+`})
+
+	ctx := testContext2(t, &ContextOpts{
+		Providers: map[addrs.Provider]providers.Factory{
+			addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
+		},
+	})
+
+	diags := ctx.Validate(m)
+	if diags.HasErrors() {
+		t.Fatal(diags.ErrWithWarnings())
+	}
+}

internal/terraform/evaluate.go

@@ -696,21 +696,12 @@ func (d *evaluationStateData) GetResource(addr addrs.Resource, rng tfdiags.Sourc
 				log.Printf("[ERROR] unknown instance %q referenced during %s", addr.Absolute(d.ModulePath), d.Operation)
 				return cty.DynamicVal, diags
 			}
-		default:
-			if d.Operation != walkValidate {
-				log.Printf("[ERROR] missing state for %q while in %s\n", addr.Absolute(d.ModulePath), d.Operation)
-			}
 
-			// Validation is done with only the configuration, so generate
-			// unknown values of the correct shape for evaluation.
-			switch {
-			case config.Count != nil:
-				return cty.UnknownVal(cty.List(ty)), diags
-			case config.ForEach != nil:
-				return cty.UnknownVal(cty.Map(ty)), 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
+			// for all resources in the configuration.
+			return cty.DynamicVal, diags
 		}
 	}
 
@@ -803,7 +794,7 @@ func (d *evaluationStateData) GetResource(addr addrs.Resource, rng tfdiags.Sourc
 		instances[key] = val
 	}
 
-	var ret cty.Value
+	ret := cty.DynamicVal
 
 	switch {
 	case config.Count != nil:

internal/terraform/testdata/plan-self-ref-multi-all/main.tf

@@ -1,4 +1,4 @@
 resource "aws_instance" "web" {
-    foo = aws_instance.web[*].id
+    foo = "${aws_instance.web.*.foo}"
     count = 4
 }