core: Require variables correctly set during NewContext

We previously deferred this to Validate, but all of our operations require
a valid set of variables and so checking this up front makes it more
obvious when a call into Terraform Core from the CLI layer isn't
populating variables correctly, instead of having it fail downstream
somewhere.

It's the caller's responsibility to ensure that it has collected values
for all of the variables in some way before calling NewContext, because
in the main case driven by the CLI there are many different places that
variable values can be collected from and so handling the main user-facing
validation in the CLI allows us to return better error messages that take
into account which way a variable is (or is not) being set.

terraform/context.go

@@ -210,6 +210,18 @@ func NewContext(opts *ContextOpts) (*Context, tfdiags.Diagnostics) {
 
 	log.Printf("[TRACE] terraform.NewContext: complete")
 
+	// By the time we get here, we should have values defined for all of
+	// the root module variables, even if some of them are "unknown". It's the
+	// caller's responsibility to have already handled the decoding of these
+	// from the various ways the CLI allows them to be set and to produce
+	// user-friendly error messages if they are not all present, and so
+	// the error message from checkInputVariables should never be seen and
+	// includes language asking the user to report a bug.
+	if config != nil {
+		varDiags := checkInputVariables(config.Module.Variables, variables)
+		diags = diags.Append(varDiags)
+	}
+
 	return &Context{
 		components: components,
 		schemas:    schemas,
@@ -227,7 +239,7 @@ func NewContext(opts *ContextOpts) (*Context, tfdiags.Diagnostics) {
 		providerInputConfig: make(map[string]map[string]cty.Value),
 		providerSHA256s:     opts.ProviderSHA256s,
 		sh:                  sh,
-	}, nil
+	}, diags
 }
 
 func (c *Context) Schemas() *Schemas {
@@ -658,14 +670,6 @@ func (c *Context) Validate() tfdiags.Diagnostics {
 
 	var diags tfdiags.Diagnostics
 
-	// Validate input variables. We do this only for the values supplied
-	// by the root module, since child module calls are validated when we
-	// visit their graph nodes.
-	if c.config != nil {
-		varDiags := checkInputVariables(c.config.Module.Variables, c.variables)
-		diags = diags.Append(varDiags)
-	}
-
 	// If we have errors at this point then we probably won't be able to
 	// construct a graph without producing redundant errors, so we'll halt early.
 	if diags.HasErrors() {

terraform/context_apply_test.go

@@ -4803,12 +4803,6 @@ func TestContext2Apply_provisionerFail(t *testing.T) {
 		Provisioners: map[string]ProvisionerFactory{
 			"shell": testProvisionerFuncFixed(pr),
 		},
-		Variables: InputValues{
-			"value": &InputValue{
-				Value:      cty.NumberIntVal(1),
-				SourceType: ValueFromCaller,
-			},
-		},
 	})
 
 	if _, diags := ctx.Plan(); diags.HasErrors() {
@@ -6650,12 +6644,6 @@ func TestContext2Apply_destroyModuleWithAttrsReferencingResource(t *testing.T) {
 					"aws": testProviderFuncFixed(p),
 				},
 			),
-			Variables: InputValues{
-				"key_name": &InputValue{
-					Value:      cty.StringVal("foobarkey"),
-					SourceType: ValueFromCaller,
-				},
-			},
 		})
 
 		// First plan and apply a create operation

terraform/context_plan_test.go

@@ -213,12 +213,6 @@ func TestContext2Plan_createBefore_maintainRoot(t *testing.T) {
 				"aws": testProviderFuncFixed(p),
 			},
 		),
-		Variables: InputValues{
-			"in": &InputValue{
-				Value:      cty.StringVal("a,b,c"),
-				SourceType: ValueFromCaller,
-			},
-		},
 	})
 
 	plan, diags := ctx.Plan()
@@ -3388,8 +3382,8 @@ func TestContext2Plan_forEach(t *testing.T) {
 }
 
 func TestContext2Plan_forEachUnknownValue(t *testing.T) {
-	// This module has a variable defined, but it is not provided
+	// This module has a variable defined, but it's value is unknown. We
-	// in the context below and we expect the plan to error, but not panic
+	// expect this to produce an error, but not to panic.
 	m := testModule(t, "plan-for-each-unknown-value")
 	p := testProvider("aws")
 	p.DiffFn = testDiffFn
@@ -3400,6 +3394,12 @@ func TestContext2Plan_forEachUnknownValue(t *testing.T) {
 				"aws": testProviderFuncFixed(p),
 			},
 		),
+		Variables: InputValues{
+			"foo": {
+				Value:      cty.UnknownVal(cty.String),
+				SourceType: ValueFromCLIArg,
+			},
+		},
 	})
 
 	_, diags := ctx.Plan()

terraform/context_validate_test.go

@@ -87,35 +87,28 @@ func TestContext2Validate_varMapOverrideOld(t *testing.T) {
 		},
 	}
 
-	c := testContext2(t, &ContextOpts{
+	_, diags := NewContext(&ContextOpts{
 		Config: m,
 		ProviderResolver: providers.ResolverFixed(
 			map[string]providers.Factory{
 				"aws": testProviderFuncFixed(p),
 			},
 		),
-		Variables: InputValues{
+		Variables: InputValues{},
-			"foo.foo": &InputValue{
-				Value:      cty.StringVal("bar"),
-				SourceType: ValueFromCaller,
-			},
-		},
 	})
-
-	diags := c.Validate()
 	if !diags.HasErrors() {
+		// Error should be: The input variable "provider_var" has not been assigned a value.
 		t.Fatalf("succeeded; want error")
 	}
 }
 
 func TestContext2Validate_varNoDefaultExplicitType(t *testing.T) {
 	m := testModule(t, "validate-var-no-default-explicit-type")
-	c := testContext2(t, &ContextOpts{
+	_, diags := NewContext(&ContextOpts{
 		Config: m,
 	})
-
-	diags := c.Validate()
 	if !diags.HasErrors() {
+		// Error should be: The input variable "maybe_a_map" has not been assigned a value.
 		t.Fatalf("succeeded; want error")
 	}
 }
@@ -314,7 +307,7 @@ func TestContext2Validate_countVariableNoDefault(t *testing.T) {
 		},
 	}
 
-	c := testContext2(t, &ContextOpts{
+	_, diags := NewContext(&ContextOpts{
 		Config: m,
 		ProviderResolver: providers.ResolverFixed(
 			map[string]providers.Factory{
@@ -322,9 +315,8 @@ func TestContext2Validate_countVariableNoDefault(t *testing.T) {
 			},
 		),
 	})
-
-	diags := c.Validate()
 	if !diags.HasErrors() {
+		// Error should be: The input variable "foo" has not been assigned a value.
 		t.Fatalf("succeeded; want error")
 	}
 }
@@ -847,7 +839,7 @@ func TestContext2Validate_requiredVar(t *testing.T) {
 		},
 	}
 
-	c := testContext2(t, &ContextOpts{
+	_, diags := NewContext(&ContextOpts{
 		Config: m,
 		ProviderResolver: providers.ResolverFixed(
 			map[string]providers.Factory{
@@ -855,9 +847,8 @@ func TestContext2Validate_requiredVar(t *testing.T) {
 			},
 		),
 	})
-
-	diags := c.Validate()
 	if !diags.HasErrors() {
+		// Error should be: The input variable "foo" has not been assigned a value.
 		t.Fatalf("succeeded; want error")
 	}
 }