Merge pull request #25420 from hashicorp/alisdair/fix-import-provider-config-references

terraform: Relax provider config ref constraints

terraform/context_import_test.go

@@ -290,73 +290,85 @@ func TestContextImport_providerModule(t *testing.T) {
 
 // Test that import will interpolate provider configuration and use
 // that configuration for import.
-func TestContextImport_providerVarConfig(t *testing.T) {
-	p := testProvider("aws")
-	m := testModule(t, "import-provider-vars")
-	ctx := testContext2(t, &ContextOpts{
-		Config: m,
-		Providers: map[addrs.Provider]providers.Factory{
-			addrs.NewDefaultProvider("aws"): testProviderFuncFixed(p),
+func TestContextImport_providerConfig(t *testing.T) {
+	testCases := map[string]struct {
+		module string
+		value  string
+	}{
+		"variables": {
+			module: "import-provider-vars",
+			value:  "bar",
 		},
-		Variables: InputValues{
-			"foo": &InputValue{
-				Value:      cty.StringVal("bar"),
-				SourceType: ValueFromCaller,
-			},
-		},
-	})
-
-	configured := false
-	p.ConfigureFn = func(c *ResourceConfig) error {
-		configured = true
-
-		if v, ok := c.Get("foo"); !ok || v.(string) != "bar" {
-			return fmt.Errorf("bad value %#v; want %#v", v, "bar")
-		}
-
-		return nil
-	}
-
-	p.ImportStateReturn = []*InstanceState{
-		&InstanceState{
-			ID:        "foo",
-			Ephemeral: EphemeralState{Type: "aws_instance"},
+		"locals": {
+			module: "import-provider-locals",
+			value:  "baz-bar",
 		},
 	}
+	for name, test := range testCases {
+		t.Run(name, func(t *testing.T) {
+			p := testProvider("aws")
+			m := testModule(t, test.module)
+			ctx := testContext2(t, &ContextOpts{
+				Config: m,
+				Providers: map[addrs.Provider]providers.Factory{
+					addrs.NewDefaultProvider("aws"): testProviderFuncFixed(p),
+				},
+				Variables: InputValues{
+					"foo": &InputValue{
+						Value:      cty.StringVal("bar"),
+						SourceType: ValueFromCaller,
+					},
+				},
+			})
 
-	state, diags := ctx.Import(&ImportOpts{
-		Targets: []*ImportTarget{
-			&ImportTarget{
-				Addr: addrs.RootModuleInstance.ResourceInstance(
-					addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey,
-				),
-				ID: "bar",
-			},
-		},
-	})
-	if diags.HasErrors() {
-		t.Fatalf("unexpected errors: %s", diags.Err())
-	}
+			p.ImportStateReturn = []*InstanceState{
+				&InstanceState{
+					ID:        "foo",
+					Ephemeral: EphemeralState{Type: "aws_instance"},
+				},
+			}
 
-	if !configured {
-		t.Fatal("didn't configure provider")
-	}
+			state, diags := ctx.Import(&ImportOpts{
+				Targets: []*ImportTarget{
+					&ImportTarget{
+						Addr: addrs.RootModuleInstance.ResourceInstance(
+							addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey,
+						),
+						ID: "bar",
+					},
+				},
+			})
+			if diags.HasErrors() {
+				t.Fatalf("unexpected errors: %s", diags.Err())
+			}
 
-	actual := strings.TrimSpace(state.String())
-	expected := strings.TrimSpace(testImportStr)
-	if actual != expected {
-		t.Fatalf("bad: \n%s", actual)
+			if !p.ConfigureCalled {
+				t.Fatal("didn't configure provider")
+			}
+
+			if foo := p.ConfigureRequest.Config.GetAttr("foo").AsString(); foo != test.value {
+				t.Fatalf("bad value %#v; want %#v", foo, test.value)
+			}
+
+			actual := strings.TrimSpace(state.String())
+			expected := strings.TrimSpace(testImportStr)
+			if actual != expected {
+				t.Fatalf("bad: \n%s", actual)
+			}
+		})
 	}
 }
 
 // Test that provider configs can't reference resources.
-func TestContextImport_providerNonVarConfig(t *testing.T) {
+func TestContextImport_providerConfigResources(t *testing.T) {
 	p := testProvider("aws")
-	m := testModule(t, "import-provider-non-vars")
+	pTest := testProvider("test")
+	m := testModule(t, "import-provider-resources")
 	ctx := testContext2(t, &ContextOpts{
 		Config: m,
 		Providers: map[addrs.Provider]providers.Factory{
-			addrs.NewDefaultProvider("aws"): testProviderFuncFixed(p),
+			addrs.NewDefaultProvider("aws"):  testProviderFuncFixed(p),
+			addrs.NewDefaultProvider("test"): testProviderFuncFixed(pTest),
 		},
 	})
 
@@ -380,6 +392,9 @@ func TestContextImport_providerNonVarConfig(t *testing.T) {
 	if !diags.HasErrors() {
 		t.Fatal("should error")
 	}
+	if got, want := diags.Err().Error(), `The configuration for provider["registry.terraform.io/hashicorp/aws"] depends on values that cannot be determined until apply.`; !strings.Contains(got, want) {
+		t.Errorf("wrong error\n got: %s\nwant: %s", got, want)
+	}
 }
 
 func TestContextImport_refresh(t *testing.T) {

terraform/eval_provider.go

@@ -49,9 +49,10 @@ func buildProviderConfig(ctx EvalContext, addr addrs.AbsProviderConfig, config *
 // EvalConfigProvider is an EvalNode implementation that configures
 // a provider that is already initialized and retrieved.
 type EvalConfigProvider struct {
-	Addr     addrs.AbsProviderConfig
-	Provider *providers.Interface
-	Config   *configs.Provider
+	Addr                addrs.AbsProviderConfig
+	Provider            *providers.Interface
+	Config              *configs.Provider
+	VerifyConfigIsKnown bool
 }
 
 func (n *EvalConfigProvider) Eval(ctx EvalContext) (interface{}, error) {
@@ -78,6 +79,16 @@ func (n *EvalConfigProvider) Eval(ctx EvalContext) (interface{}, error) {
 		return nil, diags.NonFatalErr()
 	}
 
+	if n.VerifyConfigIsKnown && !configVal.IsWhollyKnown() {
+		diags = diags.Append(&hcl.Diagnostic{
+			Severity: hcl.DiagError,
+			Summary:  "Invalid provider configuration",
+			Detail:   fmt.Sprintf("The configuration for %s depends on values that cannot be determined until apply.", n.Addr),
+			Subject:  &config.DeclRange,
+		})
+		return nil, diags.NonFatalErr()
+	}
+
 	configDiags := ctx.ConfigureProvider(n.Addr, configVal)
 	configDiags = configDiags.InConfigBody(configBody)
 

terraform/eval_provider_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/hashicorp/terraform/configs"
 	"github.com/hashicorp/terraform/configs/configschema"
 	"github.com/hashicorp/terraform/providers"
+	"github.com/hashicorp/terraform/tfdiags"
 )
 
 func TestBuildProviderConfig(t *testing.T) {
@@ -97,6 +98,99 @@ func TestEvalConfigProvider(t *testing.T) {
 	}
 }
 
+func TestEvalConfigProvider_unknownImport(t *testing.T) {
+	config := &configs.Provider{
+		Name: "foo",
+		Config: configs.SynthBody("", map[string]cty.Value{
+			"test_string": cty.UnknownVal(cty.String),
+		}),
+	}
+	provider := mockProviderWithConfigSchema(simpleTestSchema())
+	rp := providers.Interface(provider)
+	providerAddr := addrs.AbsProviderConfig{
+		Module:   addrs.RootModule,
+		Provider: addrs.NewDefaultProvider("foo"),
+	}
+	n := &EvalConfigProvider{
+		Addr:                providerAddr,
+		Config:              config,
+		Provider:            &rp,
+		VerifyConfigIsKnown: true,
+	}
+
+	ctx := &MockEvalContext{ProviderProvider: provider}
+	ctx.installSimpleEval()
+
+	_, err := n.Eval(ctx)
+
+	var diags tfdiags.Diagnostics
+	switch e := err.(type) {
+	case tfdiags.NonFatalError:
+		diags = e.Diagnostics
+	default:
+		t.Fatalf("expected err to be NonFatalError, was %T", err)
+	}
+
+	if len(diags) != 1 {
+		t.Fatalf("expected 1 diagnostic, got %d", len(diags))
+	}
+
+	if got, want := diags[0].Severity(), tfdiags.Error; got != want {
+		t.Errorf("wrong diagnostic severity %#v; want %#v", got, want)
+	}
+	if got, want := diags[0].Description().Summary, "Invalid provider configuration"; got != want {
+		t.Errorf("wrong diagnostic summary %#v; want %#v", got, want)
+	}
+	detail := `The configuration for provider["registry.terraform.io/hashicorp/foo"] depends on values that cannot be determined until apply.`
+	if got, want := diags[0].Description().Detail, detail; got != want {
+		t.Errorf("wrong diagnostic detail\n got: %q\nwant: %q", got, want)
+	}
+
+	if ctx.ConfigureProviderCalled {
+		t.Fatal("should not be called")
+	}
+}
+
+func TestEvalConfigProvider_unknownApply(t *testing.T) {
+	config := &configs.Provider{
+		Name: "foo",
+		Config: configs.SynthBody("", map[string]cty.Value{
+			"test_string": cty.UnknownVal(cty.String),
+		}),
+	}
+	provider := mockProviderWithConfigSchema(simpleTestSchema())
+	rp := providers.Interface(provider)
+	providerAddr := addrs.AbsProviderConfig{
+		Module:   addrs.RootModule,
+		Provider: addrs.NewDefaultProvider("foo"),
+	}
+	n := &EvalConfigProvider{
+		Addr:                providerAddr,
+		Config:              config,
+		Provider:            &rp,
+		VerifyConfigIsKnown: false,
+	}
+
+	ctx := &MockEvalContext{ProviderProvider: provider}
+	ctx.installSimpleEval()
+
+	if _, err := n.Eval(ctx); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	if !ctx.ConfigureProviderCalled {
+		t.Fatal("should be called")
+	}
+
+	gotObj := ctx.ConfigureProviderConfig
+	if !gotObj.Type().HasAttribute("test_string") {
+		t.Fatal("configuration object does not have \"test_string\" attribute")
+	}
+	if got, want := gotObj.GetAttr("test_string"), cty.UnknownVal(cty.String); !got.RawEquals(want) {
+		t.Errorf("wrong configuration value\ngot:  %#v\nwant: %#v", got, want)
+	}
+}
+
 func TestEvalInitProvider_impl(t *testing.T) {
 	var _ EvalNode = new(EvalInitProvider)
 }

terraform/evaltree_provider.go

@@ -18,19 +18,6 @@ func ProviderEvalTree(n *NodeApplyableProvider, config *configs.Provider) EvalNo
 		Addr: addr,
 	})
 
-	// Input stuff
-	seq = append(seq, &EvalOpFilter{
-		Ops: []walkOperation{walkImport},
-		Node: &EvalSequence{
-			Nodes: []EvalNode{
-				&EvalGetProvider{
-					Addr:   addr,
-					Output: &provider,
-				},
-			},
-		},
-	})
-
 	seq = append(seq, &EvalOpFilter{
 		Ops: []walkOperation{walkValidate},
 		Node: &EvalSequence{
@@ -48,7 +35,6 @@ func ProviderEvalTree(n *NodeApplyableProvider, config *configs.Provider) EvalNo
 		},
 	})
 
-	// Apply stuff
 	seq = append(seq, &EvalOpFilter{
 		Ops: []walkOperation{walkRefresh, walkPlan, walkApply, walkDestroy, walkImport},
 		Node: &EvalSequence{
@@ -64,7 +50,7 @@ func ProviderEvalTree(n *NodeApplyableProvider, config *configs.Provider) EvalNo
 	// We configure on everything but validate, since validate may
 	// not have access to all the variables.
 	seq = append(seq, &EvalOpFilter{
-		Ops: []walkOperation{walkRefresh, walkPlan, walkApply, walkDestroy, walkImport},
+		Ops: []walkOperation{walkRefresh, walkPlan, walkApply, walkDestroy},
 		Node: &EvalSequence{
 			Nodes: []EvalNode{
 				&EvalConfigProvider{
@@ -75,6 +61,19 @@ func ProviderEvalTree(n *NodeApplyableProvider, config *configs.Provider) EvalNo
 			},
 		},
 	})
+	seq = append(seq, &EvalOpFilter{
+		Ops: []walkOperation{walkImport},
+		Node: &EvalSequence{
+			Nodes: []EvalNode{
+				&EvalConfigProvider{
+					Addr:                addr,
+					Provider:            &provider,
+					Config:              config,
+					VerifyConfigIsKnown: true,
+				},
+			},
+		},
+	})
 
 	return &EvalSequence{Nodes: seq}
 }

terraform/graph_builder_import.go

@@ -94,9 +94,6 @@ func (b *ImportGraphBuilder) Steps() []GraphTransformer {
 		// configuration
 		&attachDataResourceDependenciesTransformer{},
 
-		// This validates that the providers only depend on variables
-		&ImportProviderValidateTransformer{},
-
 		// Close opened plugin connections
 		&CloseProviderTransformer{},
 

terraform/testdata/import-provider-locals/main.tf

@@ -0,0 +1,13 @@
+variable "foo" {}
+
+locals {
+  baz = "baz-${var.foo}"
+}
+
+provider "aws" {
+  foo = "${local.baz}"
+}
+
+resource "aws_instance" "foo" {
+  id = "bar"
+}

terraform/testdata/import-provider-non-vars/main.tf

@@ -1,7 +0,0 @@
-provider "aws" {
-    foo = "${aws_instance.foo.bar}"
-}
-
-resource "aws_instance" "foo" {
-    bar = "value"
-}

terraform/testdata/import-provider-resource/main.tf

@@ -1,7 +0,0 @@
-provider "aws" {
-  foo = data.template_data_source.d.foo
-}
-
-data "template_data_source" "d" {
-  foo = "bar"
-}

terraform/testdata/import-provider-resources/main.tf

@@ -0,0 +1,11 @@
+provider "aws" {
+    value = "${test_instance.bar.value}"
+}
+
+resource "aws_instance" "foo" {
+    bar = "value"
+}
+
+resource "test_instance" "bar" {
+    value = "yes"
+}

terraform/transform_import_provider.go

@@ -1,44 +0,0 @@
-package terraform
-
-import (
-	"fmt"
-
-	"github.com/hashicorp/hcl/v2"
-	"github.com/hashicorp/terraform/addrs"
-	"github.com/hashicorp/terraform/tfdiags"
-)
-
-// ImportProviderValidateTransformer is a GraphTransformer that goes through
-// the providers in the graph and validates that they only depend on variables.
-type ImportProviderValidateTransformer struct{}
-
-func (t *ImportProviderValidateTransformer) Transform(g *Graph) error {
-	var diags tfdiags.Diagnostics
-
-	for _, v := range g.Vertices() {
-		// We only care about providers
-		pv, ok := v.(GraphNodeProvider)
-		if !ok {
-			continue
-		}
-
-		// We only care about providers that reference things
-		rn, ok := pv.(GraphNodeReferencer)
-		if !ok {
-			continue
-		}
-
-		for _, ref := range rn.References() {
-			if _, ok := ref.Subject.(addrs.InputVariable); !ok {
-				diags = diags.Append(&hcl.Diagnostic{
-					Severity: hcl.DiagError,
-					Summary:  "Invalid provider dependency for import",
-					Detail:   fmt.Sprintf("The configuration for %s depends on %s. Providers used with import must either have literal configuration or refer only to input variables.", pv.ProviderAddr(), ref.Subject.String()),
-					Subject:  ref.SourceRange.ToHCL().Ptr(),
-				})
-			}
-		}
-	}
-
-	return diags.Err()
-}